From b868da5ad4fd60106a8b7397be686c5173eade12 Mon Sep 17 00:00:00 2001 From: Bartosz Chwila <103247439+barchw@users.noreply.github.com> Date: Thu, 2 May 2024 16:48:58 +0200 Subject: [PATCH] [release-1.5] hotfix: Disable new behaviour for ENABLE_EXTERNAL_NAME_ALIAS (#789) * hotfix: Disable new behaviour for ENABLE_EXTERNAL_NAME_ALIAS * wip: use annotation to configure external name * wip: Add nil pointer check and remove the env from iop * Add release notes * Add docu * Update predicate * Add unit tests * Update experimental test * Remove unneded docs * Delete docs/release-notes/1.7.0.md * CP natalia review * Update 04-00-istio-custom-resource.md * Update deploy-latest-release-to-cluster.sh --------- Co-authored-by: Marek Kolodziejczak --- api/v1alpha2/istio_merge.go | 50 ++++++++++- api/v1alpha2/merge_test.go | 84 +++++++++++++++++++ controllers/istio_controller.go | 2 +- docs/release-notes/1.5.3.md | 3 + docs/user/04-00-istio-custom-resource.md | 9 ++ .../istiooperator/merge_experimental_test.go | 2 +- pkg/lib/annotations/annotations.go | 11 ++- .../deploy-latest-release-to-cluster.sh | 3 +- 8 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 docs/release-notes/1.5.3.md diff --git a/api/v1alpha2/istio_merge.go b/api/v1alpha2/istio_merge.go index 7f7b5c760..37001fc4f 100644 --- a/api/v1alpha2/istio_merge.go +++ b/api/v1alpha2/istio_merge.go @@ -3,6 +3,7 @@ package v1alpha2 import ( "encoding/json" + "github.com/kyma-project/istio/operator/pkg/lib/annotations" "google.golang.org/protobuf/types/known/wrapperspb" "istio.io/api/operator/v1alpha1" "k8s.io/apimachinery/pkg/util/intstr" @@ -38,13 +39,60 @@ func (i *Istio) MergeInto(op iopv1alpha1.IstioOperator) (iopv1alpha1.IstioOperat return op, err } - return mergedResourcesOp, nil + externalNameAliasAnnotationFixOp := manageExternalNameAlias(i, mergedResourcesOp) + + return externalNameAliasAnnotationFixOp, nil } type meshConfigBuilder struct { c *meshv1alpha1.MeshConfig } +func manageExternalNameAlias(i *Istio, op iopv1alpha1.IstioOperator) iopv1alpha1.IstioOperator { + if op.Spec == nil { + op.Spec = &v1alpha1.IstioOperatorSpec{} + } + if op.Spec.Components == nil { + op.Spec.Components = &v1alpha1.IstioComponentSetSpec{} + } + if op.Spec.Components.Pilot == nil { + op.Spec.Components.Pilot = &v1alpha1.ComponentSpec{} + } + if op.Spec.Components.Pilot.K8S == nil { + op.Spec.Components.Pilot.K8S = &v1alpha1.KubernetesResourcesSpec{} + } + + shouldDisable := annotations.ShouldDisableExternalNameAlias(i.Annotations) + found := false + for _, v := range op.Spec.Components.Pilot.K8S.Env { + if v.Name == "ENABLE_EXTERNAL_NAME_ALIAS" { + if shouldDisable { + v.Value = "false" + } else { + v.Value = "true" + } + found = true + break + } + } + + if !found { + if shouldDisable { + op.Spec.Components.Pilot.K8S.Env = append(op.Spec.Components.Pilot.K8S.Env, &v1alpha1.EnvVar{ + Name: "ENABLE_EXTERNAL_NAME_ALIAS", + Value: "false", + }) + } else { + op.Spec.Components.Pilot.K8S.Env = append(op.Spec.Components.Pilot.K8S.Env, &v1alpha1.EnvVar{ + Name: "ENABLE_EXTERNAL_NAME_ALIAS", + Value: "true", + }) + } + } + + return op +} + func newMeshConfigBuilder(op iopv1alpha1.IstioOperator) (*meshConfigBuilder, error) { if op.Spec.MeshConfig == nil { op.Spec.MeshConfig = &structpb.Struct{} diff --git a/api/v1alpha2/merge_test.go b/api/v1alpha2/merge_test.go index e10df4c5b..b2bd64f8b 100644 --- a/api/v1alpha2/merge_test.go +++ b/api/v1alpha2/merge_test.go @@ -408,6 +408,90 @@ var _ = Describe("Merge", func() { Expect(iopCpuLimit).To(Equal(cpuLimit)) }) }) + Context("Istio CR annotation to disable external name alias feature", func() { + It("should set env variable to true when there was no annotation", func() { + //given + iop := iopv1alpha1.IstioOperator{ + Spec: &operatorv1alpha1.IstioOperatorSpec{}, + } + istioCR := Istio{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + Spec: IstioSpec{}, + } + + // when + out, err := istioCR.MergeInto(iop) + + var env *operatorv1alpha1.EnvVar + //then + Expect(err).ShouldNot(HaveOccurred()) + for _, v := range out.Spec.Components.Pilot.K8S.Env { + if v.Name == "ENABLE_EXTERNAL_NAME_ALIAS" { + env = v + } + } + Expect(env).ToNot(BeNil()) + Expect(env.Value).To(Equal("true")) + }) + It("should set env variable to true when the annotation value is false", func() { + //given + iop := iopv1alpha1.IstioOperator{ + Spec: &operatorv1alpha1.IstioOperatorSpec{}, + } + istioCR := Istio{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "istio-operator.kyma-project.io/disable-external-name-alias": "false", + }, + }, + Spec: IstioSpec{}, + } + + // when + out, err := istioCR.MergeInto(iop) + + //then + Expect(err).ShouldNot(HaveOccurred()) + var env *operatorv1alpha1.EnvVar + for _, v := range out.Spec.Components.Pilot.K8S.Env { + if v.Name == "ENABLE_EXTERNAL_NAME_ALIAS" { + env = v + } + } + Expect(env).ToNot(BeNil()) + Expect(env.Value).To(Equal("true")) + }) + It("should set env variable to false when the annotation value is true", func() { + //given + iop := iopv1alpha1.IstioOperator{ + Spec: &operatorv1alpha1.IstioOperatorSpec{}, + } + istioCR := Istio{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "istio-operator.kyma-project.io/disable-external-name-alias": "true", + }, + }, + Spec: IstioSpec{}, + } + + // when + out, err := istioCR.MergeInto(iop) + + var env *operatorv1alpha1.EnvVar + //then + Expect(err).ShouldNot(HaveOccurred()) + for _, v := range out.Spec.Components.Pilot.K8S.Env { + if v.Name == "ENABLE_EXTERNAL_NAME_ALIAS" { + env = v + } + } + Expect(env).ToNot(BeNil()) + Expect(env.Value).To(Equal("false")) + }) + }) }) Context("IngressGateway", func() { Context("When Istio CR has 500m configured for CPU and 500Mi for memory limits", func() { diff --git a/controllers/istio_controller.go b/controllers/istio_controller.go index e137094dd..cd132d2c4 100644 --- a/controllers/istio_controller.go +++ b/controllers/istio_controller.go @@ -225,7 +225,7 @@ func (r *IstioReconciler) SetupWithManager(mgr ctrl.Manager, rateLimiter RateLim return ctrl.NewControllerManagedBy(mgr). For(&operatorv1alpha2.Istio{}). - WithEventFilter(predicate.GenerationChangedPredicate{}). + WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{})). WithOptions(controller.Options{ RateLimiter: TemplateRateLimiter( rateLimiter.BaseDelay, diff --git a/docs/release-notes/1.5.3.md b/docs/release-notes/1.5.3.md new file mode 100644 index 000000000..a19a7000a --- /dev/null +++ b/docs/release-notes/1.5.3.md @@ -0,0 +1,3 @@ +## New Features + +- Allow for opting out of the **ENABLE_EXTERNAL_NAME_ALIAS** Istio pilot environment variable in the Istio custom resource. This allows for retaining behavior that was present in Istio prior to version 1.21. See issue [#787](https://github.com/kyma-project/istio/issues/787 ). diff --git a/docs/user/04-00-istio-custom-resource.md b/docs/user/04-00-istio-custom-resource.md index e5dcb4074..a9599c1e0 100644 --- a/docs/user/04-00-istio-custom-resource.md +++ b/docs/user/04-00-istio-custom-resource.md @@ -76,3 +76,12 @@ This table lists all the possible parameters of the given resource together with | **conditions.​reason** | string | Defines the reason for the condition status change. | | **conditions.​status** (required) | string | Represents the status of the condition. The value is either `True`, `False`, or `Unknown`. | | **conditions.​type** | string | Provides a short description of the condition. | + +### Annotations + +To retain the behavior of `EXTERNAL_NAME` that was present in versions of Istio prior to 1.21, you can configure the `ENABLE_EXTERNAL_NAME_ALIAS` environment variable in the Istio pilot. To do this, add the following annotation to the Istio CR: + +```yaml +istio-operator.kyma-project.io/disable-external-name-alias: "true" +``` +For more information, see [ExternalName support changes](https://istio.io/latest/news/releases/1.21.x/announcing-1.21/upgrade-notes/#externalname-support-changes). diff --git a/internal/istiooperator/merge_experimental_test.go b/internal/istiooperator/merge_experimental_test.go index 8271c23d3..29060904a 100644 --- a/internal/istiooperator/merge_experimental_test.go +++ b/internal/istiooperator/merge_experimental_test.go @@ -37,7 +37,7 @@ var _ = Describe("Merge", func() { // populates the state object which invalidates strict reflect // validation. If loaded CR from file (Evaluation) changes, this // size needs to be updated... - Expect(len(iop.Spec.Components.Pilot.K8S.Env)).To(Equal(3)) + Expect(len(iop.Spec.Components.Pilot.K8S.Env)).To(Equal(4)) }) Context("ParseExperimentalFeatures", func() { It("should update IstioOperator with managed environment variables when all experimental options are set to true and source struct is populated", func() { diff --git a/pkg/lib/annotations/annotations.go b/pkg/lib/annotations/annotations.go index e9da03ba4..86f1cec36 100644 --- a/pkg/lib/annotations/annotations.go +++ b/pkg/lib/annotations/annotations.go @@ -5,7 +5,8 @@ import ( ) const ( - restartAnnotationName = "istio-operator.kyma-project.io/restartedAt" + restartAnnotationName = "istio-operator.kyma-project.io/restartedAt" + disableExternalNameAnnotation = "istio-operator.kyma-project.io/disable-external-name-alias" ) func AddRestartAnnotation(annotations map[string]string) map[string]string { @@ -21,3 +22,11 @@ func HasRestartAnnotation(annotations map[string]string) bool { _, found := annotations[restartAnnotationName] return found } + +func ShouldDisableExternalNameAlias(annotations map[string]string) bool { + val, found := annotations[disableExternalNameAnnotation] + if found && val == "true" { + return true + } + return false +} diff --git a/tests/integration/scripts/deploy-latest-release-to-cluster.sh b/tests/integration/scripts/deploy-latest-release-to-cluster.sh index bf4737d6b..394202155 100755 --- a/tests/integration/scripts/deploy-latest-release-to-cluster.sh +++ b/tests/integration/scripts/deploy-latest-release-to-cluster.sh @@ -11,6 +11,7 @@ TARGET_BRANCH="$1" TAG=$(git describe --tags --abbrev=0) if [ "${TAG%.*}" == "${TARGET_BRANCH#release\-}" ] then + TAG="${TAG%-experimental}" echo "Installing Istio ${TAG}" RELEASE_MANIFEST_URL="https://github.com/kyma-project/istio/releases/download/${TAG}/istio-manager.yaml" curl -L "$RELEASE_MANIFEST_URL" | kubectl apply -f - @@ -18,4 +19,4 @@ else echo "Installing Istio from latest release" RELEASE_MANIFEST_URL="https://github.com/kyma-project/istio/releases/latest/download/istio-manager.yaml" curl -L "$RELEASE_MANIFEST_URL" | kubectl apply -f - -fi \ No newline at end of file +fi