kubernetes-sigs / gateway-api

Repository for the next iteration of composite service (e.g. Ingress) and load balancing APIs.
https://gateway-api.sigs.k8s.io
Apache License 2.0
1.86k stars 482 forks source link

override unmarshalJSON for supportedfeature to ensure comptability #3454

Closed LiorLieberman closed 5 days ago

LiorLieberman commented 1 week ago

I messed up the branch name so disregard it.

What type of PR is this? /kind bug

What this PR does / why we need it:

This change is following an issue with the breaking change we introduced in https://github.com/kubernetes-sigs/gateway-api/pull/3200/. Issue link - https://github.com/istio/istio/issues/53846

I added unitests which hopefully provide enough coverage but happy to add more tests if you have ideas.

Which issue(s) this PR fixes:

Fixes #3464

Does this PR introduce a user-facing change?:

Add backward compatibility for the older version of SupportedFeature

/cc @howardjohn @youngnick @mikemorris @robscott

LiorLieberman commented 1 week ago

/cc @howardjohn @youngnick @robscott @mikemorris

robscott commented 6 days ago

/cherry-pick release-1.2

k8s-infra-cherrypick-robot commented 6 days ago

@robscott: once the present PR merges, I will cherry-pick it on top of release-1.2 in a new PR and assign it to you.

In response to [this](https://github.com/kubernetes-sigs/gateway-api/pull/3454#issuecomment-2486255325): >/cherry-pick release-1.2 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
howardjohn commented 6 days ago

Looks like we need codegen:

diff --git a/apis/v1/zz_generated.deepcopy.go b/apis/v1/zz_generated.deepcopy.go
index 62fb99b0..fe1fb89b 100644
--- a/apis/v1/zz_generated.deepcopy.go
+++ b/apis/v1/zz_generated.deepcopy.go
@@ -1744,3 +1744,18 @@ func (in *SupportedFeature) DeepCopy() *SupportedFeature {
    in.DeepCopyInto(out)
    return out
 }
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *SupportedFeatureInternal) DeepCopyInto(out *SupportedFeatureInternal) {
+   *out = *in
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SupportedFeatureInternal.
+func (in *SupportedFeatureInternal) DeepCopy() *SupportedFeatureInternal {
+   if in == nil {
+       return nil
+   }
+   out := new(SupportedFeatureInternal)
+   in.DeepCopyInto(out)
+   return out
+}
diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go
index a613f40a..ca69f4d3 100644
--- a/pkg/generated/openapi/zz_generated.openapi.go
+++ b/pkg/generated/openapi/zz_generated.openapi.go
@@ -145,6 +145,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA
        "sigs.k8s.io/gateway-api/apis/v1.SecretObjectReference":                           schema_sigsk8sio_gateway_api_apis_v1_SecretObjectReference(ref),
        "sigs.k8s.io/gateway-api/apis/v1.SessionPersistence":                              schema_sigsk8sio_gateway_api_apis_v1_SessionPersistence(ref),
        "sigs.k8s.io/gateway-api/apis/v1.SupportedFeature":                                schema_sigsk8sio_gateway_api_apis_v1_SupportedFeature(ref),
+       "sigs.k8s.io/gateway-api/apis/v1.SupportedFeatureInternal":                        schema_sigsk8sio_gateway_api_apis_v1_SupportedFeatureInternal(ref),
        "sigs.k8s.io/gateway-api/apis/v1alpha2.BackendLBPolicy":                           schema_sigsk8sio_gateway_api_apis_v1alpha2_BackendLBPolicy(ref),
        "sigs.k8s.io/gateway-api/apis/v1alpha2.BackendLBPolicyList":                       schema_sigsk8sio_gateway_api_apis_v1alpha2_BackendLBPolicyList(ref),
        "sigs.k8s.io/gateway-api/apis/v1alpha2.BackendLBPolicySpec":                       schema_sigsk8sio_gateway_api_apis_v1alpha2_BackendLBPolicySpec(ref),
@@ -3797,8 +3798,7 @@ func schema_sigsk8sio_gateway_api_apis_v1_GatewayClassStatus(ref common.Referenc
                            Items: &spec.SchemaOrArray{
                                Schema: &spec.Schema{
                                    SchemaProps: spec.SchemaProps{
-                                       Default: map[string]interface{}{},
-                                       Ref:     ref("sigs.k8s.io/gateway-api/apis/v1.SupportedFeature"),
+                                       Ref: ref("sigs.k8s.io/gateway-api/apis/v1.SupportedFeature"),
                                    },
                                },
                            },
@@ -5650,6 +5650,27 @@ func schema_sigsk8sio_gateway_api_apis_v1_SupportedFeature(ref common.ReferenceC
    }
 }

+func schema_sigsk8sio_gateway_api_apis_v1_SupportedFeatureInternal(ref common.ReferenceCallback) common.OpenAPIDefinition {
+   return common.OpenAPIDefinition{
+       Schema: spec.Schema{
+           SchemaProps: spec.SchemaProps{
+               Description: "This is solely for the purpose of ensuring backward compatibility and SHOULD NOT be used elsewhere.",
+               Type:        []string{"object"},
+               Properties: map[string]spec.Schema{
+                   "name": {
+                       SchemaProps: spec.SchemaProps{
+                           Default: "",
+                           Type:    []string{"string"},
+                           Format:  "",
+                       },
+                   },
+               },
+               Required: []string{"name"},
+           },
+       },
+   }
+}
+
 func schema_sigsk8sio_gateway_api_apis_v1alpha2_BackendLBPolicy(ref common.ReferenceCallback) common.OpenAPIDefinition {
    return common.OpenAPIDefinition{
        Schema: spec.Schema{

We can do a followup release if needed, but fwiw we are targeting an Istio release tomorrow afternoon that would be great to include this in. Not a big deal if we miss it, though.

LiorLieberman commented 6 days ago

@howardjohn Will ask for approval in the slack chat later today, wdyt about my comment https://github.com/kubernetes-sigs/gateway-api/pull/3454#discussion_r1845242178 ?

howardjohn commented 5 days ago

/lgtm

k8s-ci-robot commented 5 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LiorLieberman, robscott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/gateway-api/blob/main/OWNERS)~~ [robscott] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
k8s-infra-cherrypick-robot commented 5 days ago

@robscott: new pull request created: #3465

In response to [this](https://github.com/kubernetes-sigs/gateway-api/pull/3454#issuecomment-2486255325): >/cherry-pick release-1.2 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.