fix: respect namespace in structured local-gateways config#2217
Conversation
|
Welcome @DCchoudhury15! It looks like this is your first PR to knative/operator 🎉 |
|
Hi @DCchoudhury15. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@houshengbo i have created the pr up for review. |
|
/ok-to-test |
|
@houshengbo all tests have passed waiting for approval |
| @@ -76,11 +85,25 @@ func updateIstioService(ks *v1beta1.KnativeServing, u *unstructured.Unstructured | |||
|
|
|||
| // UpdateNamespace set correct namespace of istio to the service knative-local-gateway | |||
| func UpdateNamespace(u *unstructured.Unstructured, data map[string]string, ns string) { | |||
There was a problem hiding this comment.
This func can be refactored a bit for better logical separation:
var (
knativeLocalGateway = "knative-local-gateway"
localGateways = "local-gateways"
)
func UpdateNamespace(u *unstructured.Unstructured, data map[string]string, namespace string) {
if ns := resolveGatewayNamespace(data, namespace); ns != "" {
u.SetNamespace(ns)
}
}
func resolveGatewayNamespace(data map[string]string, ns string) string {
if ns := fromStructuredGateways(data); ns != "" {
return ns
}
return fromLegacyGateway(data, ns)
}
func fromStructuredGateways(data map[string]string) string {
raw, ok := data[localGateways]
if !ok {
return ""
}
var gateways []localGatewayConfig
if err := yaml.Unmarshal([]byte(raw), &gateways); err != nil {
return ""
}
for _, gw := range gateways {
if gw.Name == knativeLocalGateway {
return gw.Namespace
}
}
return ""
}
func fromLegacyGateway(data map[string]string, ns string) string {
key := fmt.Sprintf("local-gateway.%s.%s", ns, knativeLocalGateway)
raw, ok := data[key]
if !ok {
return ""
}
fields := strings.Split(raw, ".")
if len(fields) < 2 {
return ""
}
return fields[1]
}
There was a problem hiding this comment.
@DCchoudhury15 Could you refactor this func?
There was a problem hiding this comment.
@houshengbo i have done the changes up for review
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2217 +/- ##
==========================================
+ Coverage 63.31% 63.45% +0.14%
==========================================
Files 49 49
Lines 1878 1899 +21
==========================================
+ Hits 1189 1205 +16
- Misses 597 600 +3
- Partials 92 94 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DCchoudhury15, houshengbo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@houshengbo some ci tests were failing so made some changes up for review. |
|
@DCchoudhury15 As long as you rebase the main branch, it should be good, since I updated the deps and manifests. |
Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>
Co-authored-by: Vincent <shou73@bloomberg.net>
48da06b to
a482109
Compare
i have rebased the main branch. |
|
/lgtm |
Changes
namespacefield when using the structuredlocal-gatewaysconfiguration format.UpdateNamespacelogic iningress_service.goto unmarshal and prioritize the plurallocal-gatewaysYAML array.ingress_service_test.goto verify structured config parsing and ensure it takes precedence over legacy dot-notation config./kind bug
Fixes #2201
Release Note