From bd823f99899b315fa61a3dc084440fc4d8d5baa1 Mon Sep 17 00:00:00 2001 From: Knative Prow Robot Date: Mon, 4 Dec 2023 18:32:39 +0000 Subject: [PATCH] Update istio gateway from v1alpha3 to v1beta1 (#1647) Co-authored-by: Vincent Hou --- .../operator/base/ingressconfiguration.go | 4 +- .../operator/base/zz_generated.deepcopy.go | 4 +- .../knativeserving/ingress/istio.go | 27 ++- .../knativeserving/ingress/istio_test.go | 156 +++++++++++++++++- 4 files changed, 173 insertions(+), 18 deletions(-) diff --git a/pkg/apis/operator/base/ingressconfiguration.go b/pkg/apis/operator/base/ingressconfiguration.go index 9da7337dcd..048ad92611 100644 --- a/pkg/apis/operator/base/ingressconfiguration.go +++ b/pkg/apis/operator/base/ingressconfiguration.go @@ -17,7 +17,7 @@ limitations under the License. package base import ( - istiov1alpha3 "istio.io/api/networking/v1alpha3" + istiov1beta1 "istio.io/api/networking/v1beta1" v1 "k8s.io/api/core/v1" ) @@ -65,5 +65,5 @@ type IstioGatewayOverride struct { Selector map[string]string `json:"selector,omitempty"` // A list of server specifications. - Servers []*istiov1alpha3.Server `json:"servers,omitempty"` + Servers []*istiov1beta1.Server `json:"servers,omitempty"` } diff --git a/pkg/apis/operator/base/zz_generated.deepcopy.go b/pkg/apis/operator/base/zz_generated.deepcopy.go index 5b95ca1125..a899033ed8 100644 --- a/pkg/apis/operator/base/zz_generated.deepcopy.go +++ b/pkg/apis/operator/base/zz_generated.deepcopy.go @@ -22,7 +22,7 @@ limitations under the License. package base import ( - v1alpha3 "istio.io/api/networking/v1alpha3" + v1beta1 "istio.io/api/networking/v1beta1" v1 "k8s.io/api/core/v1" ) @@ -310,7 +310,7 @@ func (in *IstioGatewayOverride) DeepCopyInto(out *IstioGatewayOverride) { } if in.Servers != nil { in, out := &in.Servers, &out.Servers - *out = make([]*v1alpha3.Server, len(*in)) + *out = make([]*v1beta1.Server, len(*in)) for i := range *in { if (*in)[i] != nil { in, out := &(*in)[i], &(*out)[i] diff --git a/pkg/reconciler/knativeserving/ingress/istio.go b/pkg/reconciler/knativeserving/ingress/istio.go index 2255a9ba82..cd379b0886 100644 --- a/pkg/reconciler/knativeserving/ingress/istio.go +++ b/pkg/reconciler/knativeserving/ingress/istio.go @@ -18,10 +18,11 @@ package ingress import ( "context" + "strings" mf "github.com/manifestival/manifestival" "go.uber.org/zap" - istionetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" + istionetworkingv1beta "istio.io/client-go/pkg/apis/networking/v1beta1" "istio.io/client-go/pkg/clientset/versioned/scheme" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "knative.dev/operator/pkg/apis/operator/base" @@ -38,9 +39,19 @@ func istioTransformers(ctx context.Context, instance *v1beta1.KnativeServing) [] func gatewayTransform(instance *servingv1beta1.KnativeServing, log *zap.SugaredLogger) mf.Transformer { return func(u *unstructured.Unstructured) error { // Update the deployment with the new registry and tag - if u.GetAPIVersion() == "networking.istio.io/v1alpha3" && u.GetKind() == "Gateway" { - gateway := &istionetworkingv1alpha3.Gateway{} - if err := scheme.Scheme.Convert(u, gateway, nil); err != nil { + if u.GetKind() == "Gateway" { + if !strings.HasPrefix(u.GetAPIVersion(), "networking.istio.io/") { + return nil + } + beta := true + if strings.HasSuffix(u.GetAPIVersion(), "v1alpha3") { + u.SetAPIVersion("networking.istio.io/v1beta1") + beta = false + } + + gateway := &istionetworkingv1beta.Gateway{} + err := scheme.Scheme.Convert(u, gateway, nil) + if err != nil { return err } @@ -50,7 +61,7 @@ func gatewayTransform(instance *servingv1beta1.KnativeServing, log *zap.SugaredL } } // TODO: cluster-local-gateway was removed since v0.20 https://github.com/knative-extensions/net-istio/commit/058432d749435ef1fc61aa2b1fd048d0c75460ee - // Reomove it once operator stops v0.20 support. + // Remove it once operator stops v0.20 support. if u.GetName() == "cluster-local-gateway" || u.GetName() == "knative-local-gateway" { if err := updateIstioGateway(localGateway(instance), gateway, log); err != nil { return err @@ -60,6 +71,10 @@ func gatewayTransform(instance *servingv1beta1.KnativeServing, log *zap.SugaredL if err := scheme.Scheme.Convert(gateway, u, nil); err != nil { return err } + + if !beta { + u.SetAPIVersion("networking.istio.io/v1alpha3") + } } return nil } @@ -79,7 +94,7 @@ func localGateway(instance *servingv1beta1.KnativeServing) *base.IstioGatewayOve return nil } -func updateIstioGateway(override *base.IstioGatewayOverride, gateway *istionetworkingv1alpha3.Gateway, log *zap.SugaredLogger) error { +func updateIstioGateway(override *base.IstioGatewayOverride, gateway *istionetworkingv1beta.Gateway, log *zap.SugaredLogger) error { if override != nil && len(override.Selector) > 0 { log.Debugw("Updating Gateway", "name", gateway.GetName(), "gatewayOverrides", override) gateway.Spec.Selector = override.Selector diff --git a/pkg/reconciler/knativeserving/ingress/istio_test.go b/pkg/reconciler/knativeserving/ingress/istio_test.go index 7fe2ac98d3..5026f7207b 100644 --- a/pkg/reconciler/knativeserving/ingress/istio_test.go +++ b/pkg/reconciler/knativeserving/ingress/istio_test.go @@ -21,7 +21,9 @@ import ( "go.uber.org/zap" istiov1alpha3 "istio.io/api/networking/v1alpha3" + istiov1beta1 "istio.io/api/networking/v1beta1" istionetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" + istionetworkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" "istio.io/client-go/pkg/clientset/versioned/scheme" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "knative.dev/operator/pkg/apis/operator/base" @@ -31,14 +33,14 @@ import ( var log = zap.NewNop().Sugar() -func gatewayOverride(selector map[string]string, servers []*istiov1alpha3.Server) *base.IstioGatewayOverride { +func gatewayOverride(selector map[string]string, servers []*istiov1beta1.Server) *base.IstioGatewayOverride { return &base.IstioGatewayOverride{ Selector: selector, Servers: servers, } } -func TestGatewayTransform(t *testing.T) { +func TestGatewayTransformV1alpha3(t *testing.T) { serverIn := []*istiov1alpha3.Server{ { Hosts: []string{"localhost"}, @@ -48,13 +50,13 @@ func TestGatewayTransform(t *testing.T) { Port: &istiov1alpha3.Port{Name: "test"}, }} - serverUpdate := []*istiov1alpha3.Server{ + serverUpdate := []*istiov1beta1.Server{ { Hosts: []string{"localhost-1"}, - Port: &istiov1alpha3.Port{Name: "test-1", Protocol: "proto-1", Number: 25, TargetPort: 53}, + Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-1", Number: 25, TargetPort: 53}, }, { Hosts: []string{"localhost-1"}, - Port: &istiov1alpha3.Port{Name: "test-1", Protocol: "proto-2", Number: 45, TargetPort: 23}, + Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-2", Number: 45, TargetPort: 23}, }} tests := []struct { @@ -67,7 +69,7 @@ func TestGatewayTransform(t *testing.T) { deprecatedKnativeIngressGateway base.IstioGatewayOverride deprecatedClusterLocalGateway base.IstioGatewayOverride expected map[string]string - expectedServersIn []*istiov1alpha3.Server + expectedServersIn []*istiov1beta1.Server }{{ name: "update ingress gateway", gatewayName: "knative-ingress-gateway", @@ -132,7 +134,7 @@ func TestGatewayTransform(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gateway := makeUnstructuredGateway(t, tt.gatewayName, tt.in, tt.serversIn) + gateway := makeUnstructuredGatewayAlpha(tt.gatewayName, tt.in, tt.serversIn) instance := &servingv1beta1.KnativeServing{ Spec: servingv1beta1.KnativeServingSpec{ Ingress: &servingv1beta1.IngressConfigs{ @@ -162,7 +164,145 @@ func TestGatewayTransform(t *testing.T) { } } -func makeUnstructuredGateway(t *testing.T, name string, selector map[string]string, servers []*istiov1alpha3.Server) *unstructured.Unstructured { +func TestGatewayTransform(t *testing.T) { + serverIn := []*istiov1beta1.Server{ + { + Hosts: []string{"localhost"}, + Port: &istiov1beta1.Port{Name: "test"}, + }, { + Hosts: []string{"localhost"}, + Port: &istiov1beta1.Port{Name: "test"}, + }} + + serverUpdate := []*istiov1beta1.Server{ + { + Hosts: []string{"localhost-1"}, + Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-1", Number: 25, TargetPort: 53}, + }, { + Hosts: []string{"localhost-1"}, + Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-2", Number: 45, TargetPort: 23}, + }} + + tests := []struct { + name string + gatewayName string + in map[string]string + serversIn []*istiov1beta1.Server + knativeIngressGateway *base.IstioGatewayOverride + clusterLocalGateway *base.IstioGatewayOverride + deprecatedKnativeIngressGateway base.IstioGatewayOverride + deprecatedClusterLocalGateway base.IstioGatewayOverride + expected map[string]string + expectedServersIn []*istiov1beta1.Server + }{{ + name: "update ingress gateway", + gatewayName: "knative-ingress-gateway", + in: map[string]string{ + "istio": "old-istio", + }, + serversIn: serverIn, + knativeIngressGateway: gatewayOverride(map[string]string{"istio": "knative-ingress"}, serverUpdate), + clusterLocalGateway: gatewayOverride(map[string]string{"istio": "cluster-local"}, nil), + expected: map[string]string{ + "istio": "knative-ingress", + }, + expectedServersIn: serverUpdate, + }, { + name: "update local gateway", + gatewayName: "cluster-local-gateway", + in: map[string]string{ + "istio": "old-istio", + }, + knativeIngressGateway: gatewayOverride(map[string]string{"istio": "knative-ingress"}, nil), + clusterLocalGateway: gatewayOverride(map[string]string{"istio": "cluster-local"}, serverUpdate), + expected: map[string]string{ + "istio": "cluster-local", + }, + expectedServersIn: serverUpdate, + }, { + name: "update ingress gateway with both new and deprecate config", + gatewayName: "knative-ingress-gateway", + in: map[string]string{ + "istio": "old-istio", + }, + knativeIngressGateway: gatewayOverride(map[string]string{"istio": "win"}, nil), + deprecatedKnativeIngressGateway: *gatewayOverride(map[string]string{"istio": "lose"}, nil), + expected: map[string]string{ + "istio": "win", + }, + }, { + name: "update local gateway with both new and deprecate config", + gatewayName: "cluster-local-gateway", + in: map[string]string{ + "istio": "old-istio", + }, + clusterLocalGateway: gatewayOverride(map[string]string{"istio": "win"}, nil), + deprecatedClusterLocalGateway: *gatewayOverride(map[string]string{"istio": "lose"}, nil), + expected: map[string]string{ + "istio": "win", + }, + }, { + name: "do not update unknown gateway", + gatewayName: "not-knative-ingress-gateway", + in: map[string]string{ + "istio": "old-istio", + }, + knativeIngressGateway: gatewayOverride(map[string]string{"istio": "knative-ingress"}, nil), + clusterLocalGateway: gatewayOverride(map[string]string{"istio": "cluster-local"}, nil), + deprecatedKnativeIngressGateway: *gatewayOverride(map[string]string{"istio": "lose"}, nil), + deprecatedClusterLocalGateway: *gatewayOverride(map[string]string{"istio": "cluster-local"}, nil), + expected: map[string]string{ + "istio": "old-istio", + }, + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gateway := makeUnstructuredGateway(tt.gatewayName, tt.in, tt.serversIn) + instance := &servingv1beta1.KnativeServing{ + Spec: servingv1beta1.KnativeServingSpec{ + Ingress: &servingv1beta1.IngressConfigs{ + Istio: base.IstioIngressConfiguration{ + Enabled: true, + KnativeIngressGateway: tt.knativeIngressGateway, + KnativeLocalGateway: tt.clusterLocalGateway, + }, + }, + }, + } + + gatewayTransform(instance, log)(gateway) + + gatewayResult := &istionetworkingv1beta1.Gateway{} + err := scheme.Scheme.Convert(gateway, gatewayResult, nil) + util.AssertEqual(t, err, nil) + util.AssertDeepEqual(t, gatewayResult.Spec.Selector, tt.expected) + for i, server := range gatewayResult.Spec.Servers { + util.AssertDeepEqual(t, server.Hosts, tt.expectedServersIn[i].Hosts) + util.AssertDeepEqual(t, server.Port.Name, tt.expectedServersIn[i].Port.Name) + util.AssertDeepEqual(t, server.Port.Number, tt.expectedServersIn[i].Port.Number) + util.AssertDeepEqual(t, server.Port.TargetPort, tt.expectedServersIn[i].Port.TargetPort) + util.AssertDeepEqual(t, server.Port.Protocol, tt.expectedServersIn[i].Port.Protocol) + } + }) + } +} + +func makeUnstructuredGateway(name string, selector map[string]string, servers []*istiov1beta1.Server) *unstructured.Unstructured { + gateway := &istionetworkingv1beta1.Gateway{} + result := &unstructured.Unstructured{} + gateway.SetName(name) + gateway.Spec.Selector = selector + gateway.Spec.Servers = servers + + if err := scheme.Scheme.Convert(gateway, result, nil); err != nil { + panic(err) + } + + return result +} + +func makeUnstructuredGatewayAlpha(name string, selector map[string]string, servers []*istiov1alpha3.Server) *unstructured.Unstructured { gateway := &istionetworkingv1alpha3.Gateway{} result := &unstructured.Unstructured{} gateway.SetName(name)