hotfix: Disable new behaviour for ENABLE_EXTERNAL_NAME_ALIAS (#788)

* 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 * Apply suggestions from code review Co-authored-by: Natalia Sitko <[email protected]> * Update 04-00-istio-custom-resource.md --------- Co-authored-by: Marek Kolodziejczak <[email protected]> Co-authored-by: Natalia Sitko <[email protected]>
kyma-project · May 2, 2024 · 5413f97 · 5413f97
1 parent bf6badc
commit 5413f97
Show file tree

Hide file tree

Showing 7 changed files with 157 additions and 4 deletions.
diff --git a/api/v1alpha2/istio_merge.go b/api/v1alpha2/istio_merge.go
@@ -2,6 +2,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"
@@ -37,13 +38,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
@@ -454,6 +454,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
@@ -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.7.0.md b/docs/release-notes/1.7.0.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
@@ -77,3 +77,12 @@ This table lists all the possible parameters of the given resource together with
 | **conditions.&#x200b;reason**             | string     | Defines the reason for the condition status change.                                                                      |
 | **conditions.&#x200b;status** (required)  | string     | Represents the status of the condition. The value is either `True`, `False`, or `Unknown`.                               |
 | **conditions.&#x200b;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
@@ -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(5))
+		Expect(len(iop.Spec.Components.Pilot.K8S.Env)).To(Equal(6))
 	})
 	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
@@ -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
+}
Original file line number	Diff line number	Diff line change
		@@ -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 ).