You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/11/13 16:18:20 UTC

[GitHub] [apisix-ingress-controller] dickens7 opened a new pull request, #1451: feat: ingress annotations supports the specified upstream schema

dickens7 opened a new pull request, #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451

   <!-- Please answer these questions before submitting a pull request -->
   
   ### Type of change:
   
   #1420
   
   <!-- Please delete options that are not relevant. -->
   
   - [ ] Bugfix
   - [x] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   <!--- If it fixes an open issue, please link to the issue here. -->
   
   ### Pre-submission checklist:
   
   <!--
   Please follow the requirements:
   1. Use Draft if the PR is not ready to be reviewed
   2. Test is required for the feat/fix PR, unless you have a good reason
   3. Doc is required for the feat PR
   4. Use a new commit to resolve review instead of `push -f`
   5. Use "request review" to notify the reviewer once you have resolved the review
   -->
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [x] Have you added corresponding test cases?
   * [x] Have you modified the corresponding document?
   * [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix-ingress-controller#community) first**
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] dickens7 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
dickens7 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1039715755


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+var schemeMap map[string]struct{} = map[string]struct{}{
+	"http":  {},

Review Comment:
   We define SchemeMap to `pkg/types/apisix/v1/types.go`
   Or just use constants like `SchemeHTTP` , `SchemeGRPC`
   
   > pkg/types/apisix/v1/types.go
   
   ```go
   var SchemeMap map[string]struct{} = map[string]struct{}{
   	SchemeHTTP:  {},
   	SchemeHTTPS: {},
   	SchemeGRPC:  {},
   	SchemeGRPCS: {},
   }
   ```
   
   or
   
   > pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go
   
   ```go
   var schemeMap map[string]struct{} = map[string]struct{}{
   	types.SchemeHTTP:  {},
   	types.SchemeHTTPS: {},
   	types.SchemeGRPC:  {},
   	types.SchemeGRPCS: {},
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#issuecomment-1312768629

   # [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1451?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1451](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1451?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6493ec9) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/febeab4cac49dd97a6b257324ba9ba3b5396c514?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (febeab4) will **decrease** coverage by `0.03%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1451      +/-   ##
   ==========================================
   - Coverage   41.30%   41.26%   -0.04%     
   ==========================================
     Files          82       82              
     Lines        7271     7277       +6     
   ==========================================
     Hits         3003     3003              
   - Misses       3921     3927       +6     
     Partials      347      347              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1451?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [pkg/providers/ingress/translation/annotations.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9pbmdyZXNzL3RyYW5zbGF0aW9uL2Fubm90YXRpb25zLmdv) | `58.82% <ø> (ø)` | |
   | [pkg/providers/ingress/translation/translator.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3Byb3ZpZGVycy9pbmdyZXNzL3RyYW5zbGF0aW9uL3RyYW5zbGF0b3IuZ28=) | `0.00% <0.00%> (ø)` | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1040363687


##########
pkg/types/apisix/v1/types.go:
##########
@@ -81,6 +81,13 @@ const (
 	DefaultUpstreamTimeout = 60
 )
 
+var IngressSchemeMap map[string]struct{} = map[string]struct{}{

Review Comment:
   Yes, I also agree with naming `ValidSchemes`  then we can merge  it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] lingsamuel commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1037873211


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+var schemeMap map[string]int = map[string]int{
+	"http":  1,

Review Comment:
   If the map values are useless, I prefer to use an empty struct value here. Otherwise, other readers may get confused.



##########
docs/en/latest/concepts/annotations.md:
##########
@@ -269,3 +269,29 @@ spec:
                 port:
                   number: 80
 ```
+
+## Upstram scheme

Review Comment:
   `Upstream`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] dickens7 commented on pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
dickens7 commented on PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#issuecomment-1328134601

   Please take a look at this `unit-test-ci`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1033677371


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,49 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"fmt"
+
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+// string in slice
+func inSchemes(a string) bool {
+	list := []string{"http", "https", "grpc", "grpcs"}
+
+	for _, b := range list {
+		if b == a {
+			return true
+		}
+	}
+	return false
+}

Review Comment:
   we can just construct a map, the check if the key exist.
   
   ```
   _, ok := fooMap[scheme]
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1033752911


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -26,24 +27,24 @@ func NewParser() annotations.IngressAnnotationsParser {
 	return &upstreamscheme{}
 }
 
-// string in slice
-func inSchemes(a string) bool {
-	list := []string{"http", "https", "grpc", "grpcs"}
-
-	for _, b := range list {
-		if b == a {
-			return true
-		}
-	}
-	return false
+var schemeMap map[string]int = map[string]int{
+	"http":  1,
+	"https": 2,
+	"grpc":  3,
+	"grpcs": 4,
 }
 
 func (w *upstreamscheme) Parse(e annotations.Extractor) (interface{}, error) {
 	scheme := e.GetStringAnnotation(annotations.AnnotationsUpstreamScheme)
-
-	if inSchemes(scheme) {
+	_, ok := schemeMap[scheme]

Review Comment:
   I thought we could ignore case?
   both `HTTP` and `http` can be used.
   
   You can convert the obtained configuration items to lowercase



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] lingsamuel commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1040324481


##########
pkg/types/apisix/v1/types.go:
##########
@@ -81,6 +81,13 @@ const (
 	DefaultUpstreamTimeout = 60
 )
 
+var IngressSchemeMap map[string]struct{} = map[string]struct{}{

Review Comment:
   The name doesn't quite fit because these valid schemes are not just for Ingress. `ValidSchemes` would be better, but I can approve this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1039096909


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+var schemeMap map[string]struct{} = map[string]struct{}{
+	"http":  {},

Review Comment:
   good catch, we have https://github.com/apache/apisix-ingress-controller/blob/c8d3bd52fd8820475be763cd52b51b981382f285/pkg/types/apisix/v1/types.go#L52-L59



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1032705067


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,29 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+func (w *upstreamscheme) Parse(e annotations.Extractor) (interface{}, error) {
+	return e.GetStringAnnotation(annotations.AnnotationsUpstreamScheme), nil
+}

Review Comment:
   We need to check that its value is within these available ranges
   
   https://github.com/apache/apisix-ingress-controller/blob/c8d3bd52fd8820475be763cd52b51b981382f285/pkg/types/apisix/v1/types.go#L52-L59



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] dickens7 commented on pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
dickens7 commented on PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#issuecomment-1330106673

   Please help me re-run ci.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] lingsamuel commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
lingsamuel commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1039093517


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+var schemeMap map[string]struct{} = map[string]struct{}{
+	"http":  {},

Review Comment:
   A small suggestion. Can we make this variable global? Together with the schemes in `pkg/types/apisix/v1/types.go`.
   Otherwise, we may need to deal with cross-file constant dependencies.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] dickens7 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
dickens7 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1037981390


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+var schemeMap map[string]int = map[string]int{
+	"http":  1,

Review Comment:
   Indeed an `empty struct` is better
   ```go
   map[string]struct{}
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#issuecomment-1333019498

   could you please merge master branch first?
   I have sovled the markdown lint issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#issuecomment-1327919660

   sorry for delay, could you please merge master branch? thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] dickens7 commented on pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
dickens7 commented on PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#issuecomment-1329936199

   please help me re-run ci.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451#discussion_r1037885071


##########
pkg/providers/ingress/translation/annotations/upstreamscheme/upstreamscheme.go:
##########
@@ -0,0 +1,50 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package upstreamscheme
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
+)
+
+type upstreamscheme struct{}
+
+func NewParser() annotations.IngressAnnotationsParser {
+	return &upstreamscheme{}
+}
+
+var schemeMap map[string]int = map[string]int{
+	"http":  1,

Review Comment:
   +1



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-ingress-controller] tao12345666333 merged pull request #1451: feat: ingress annotations supports the specified upstream schema

Posted by GitBox <gi...@apache.org>.
tao12345666333 merged PR #1451:
URL: https://github.com/apache/apisix-ingress-controller/pull/1451


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org