diff --git a/openshift-knative-operator/pkg/serving/secretinformerfiltering.go b/openshift-knative-operator/pkg/serving/secretinformerfiltering.go index 9396120719..d269b2ea60 100644 --- a/openshift-knative-operator/pkg/serving/secretinformerfiltering.go +++ b/openshift-knative-operator/pkg/serving/secretinformerfiltering.go @@ -8,18 +8,13 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "knative.dev/networking/pkg/apis/networking" + "knative.dev/networking/pkg/config" "knative.dev/operator/pkg/apis/operator/base" operatorv1beta1 "knative.dev/operator/pkg/apis/operator/v1beta1" ) -const ( - // TODO: Remove when available in knative.dev/networking/config - ServingInternalCertName = "knative-serving-certs" - // TODO: Maybe decide to fetch from net-kourier deps instead - EnableSecretInformerFilteringByCertUIDEnv = "ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UID" - // TODO: Annotation is deprecated, remove in future releases - secretInformerFilteringAnnotation = "serverless.openshift.io/enable-secret-informer-filtering" -) +// TODO: Maybe decide to fetch from net-kourier deps instead +const EnableSecretInformerFilteringByCertUIDEnv = "ENABLE_SECRET_INFORMER_FILTERING_BY_CERT_UID" func enableSecretInformerFilteringTransformers(ks base.KComponent) []mf.Transformer { shouldInject := false @@ -41,7 +36,8 @@ func enableSecretInformerFilteringTransformers(ks base.KComponent) []mf.Transfor func injectLabelIntoInternalEncryptionSecret() mf.Transformer { return func(u *unstructured.Unstructured) error { - if u.GetKind() == "Secret" && u.GetName() == ServingInternalCertName { + //nolint:staticcheck // ignore the deprecation until internal encryption is implemented downstream + if u.GetKind() == "Secret" && u.GetName() == config.ServingInternalCertName { labels := u.GetLabels() if labels == nil { labels = make(map[string]string, 1) @@ -74,15 +70,10 @@ func configIfUnsetAndCheckIfShouldInject(comp *operatorv1beta1.KnativeServing, d } } - // The annotation is deprecated - // TODO: Remove this block in future releases - if deployment == "net-istio-controller" { - if v, ok := comp.GetAnnotations()[secretInformerFilteringAnnotation]; ok { - b, _ := strconv.ParseBool(v) - // Keep the same behavior as in 1.27 - return b, common.InjectEnvironmentIntoDeployment("net-istio-controller", "controller", - corev1.EnvVar{Name: EnableSecretInformerFilteringByCertUIDEnv, Value: v}) - } + // TODO: remove when set to true at the net-* repos upstream/midstream + if deployment == "net-istio-controller" || deployment == "net-kourier-controller" { + return true, common.InjectEnvironmentIntoDeployment(deployment, "controller", + corev1.EnvVar{Name: EnableSecretInformerFilteringByCertUIDEnv, Value: "true"}) } return false, nil } diff --git a/openshift-knative-operator/pkg/serving/secretinformerfilterting_test.go b/openshift-knative-operator/pkg/serving/secretinformerfilterting_test.go index e828f7717e..e897de4bd7 100644 --- a/openshift-knative-operator/pkg/serving/secretinformerfilterting_test.go +++ b/openshift-knative-operator/pkg/serving/secretinformerfilterting_test.go @@ -11,12 +11,12 @@ import ( func TestSecretInformerFilteringOverride(t *testing.T) { cases := []struct { - name string - in *operatorv1beta1.KnativeServing - expected *operatorv1beta1.KnativeServing - shouldAddLabelToSecret bool + name string + in *operatorv1beta1.KnativeServing + expected *operatorv1beta1.KnativeServing + shouldEnableSecretInformerFiltering bool }{{ - name: "by default no overrides, disabled secret filtering, with kourier enabled", + name: "by default no overrides, enabled secret informer filtering, with kourier enabled", in: ks(func(ks *operatorv1beta1.KnativeServing) { ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Kourier: base.KourierIngressConfiguration{ Enabled: true, @@ -27,9 +27,9 @@ func TestSecretInformerFilteringOverride(t *testing.T) { Enabled: true, }} }), - shouldAddLabelToSecret: false, + shouldEnableSecretInformerFiltering: true, }, { - name: "by default no overrides, disabled secret filtering, with istio enabled", + name: "by default no overrides, enabled secret informer filtering, with istio enabled", in: ks(func(ks *operatorv1beta1.KnativeServing) { ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ Enabled: true, @@ -40,71 +40,9 @@ func TestSecretInformerFilteringOverride(t *testing.T) { Enabled: true, }} }), - shouldAddLabelToSecret: false, + shouldEnableSecretInformerFiltering: true, }, { - name: "by default no overrides, deprecated annotation set to true, enabled secret filtering, istio enabled", - in: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "true"} - }), - expected: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "true"} - }), - shouldAddLabelToSecret: true, - }, { - name: "by default no overrides, istio deprecated annotation set to true but has no effect, disabled secret filtering, kourier enabled", - in: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Kourier: base.KourierIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "true"} - }), - expected: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Kourier: base.KourierIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "true"} - }), - shouldAddLabelToSecret: false, - }, { - name: "by default no overrides, istio deprecated annotation set to false, disabled secret filtering, istio enabled", - in: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "false"} - }), - expected: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "false"} - }), - shouldAddLabelToSecret: false, - }, { - name: "by default no overrides, istio deprecated annotation value with bad string, disabled secret filtering, istio enabled", - in: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "wrong"} - }), - expected: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "wrong"} - }), - shouldAddLabelToSecret: false, - }, { - name: "disabled secret filtering with kourier enabled", + name: "disabled secret informer filtering with kourier enabled", in: ks(func(ks *operatorv1beta1.KnativeServing) { ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Kourier: base.KourierIngressConfiguration{ Enabled: true, @@ -143,55 +81,13 @@ func TestSecretInformerFilteringOverride(t *testing.T) { }}, }) }), - shouldAddLabelToSecret: false, - }, { - name: "disabled secret filtering with istio enabled", - in: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Spec.DeploymentOverride = append(ks.Spec.DeploymentOverride, base.WorkloadOverride{ - Name: "net-istio-controller", - Env: []base.EnvRequirementsOverride{ - { - Container: "controller", - EnvVars: []corev1.EnvVar{{ - Name: EnableSecretInformerFilteringByCertUIDEnv, - Value: "false", - }, { - Name: "foo", - Value: "foo", - }}, - }}, - }) - }), - expected: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Spec.DeploymentOverride = append(ks.Spec.DeploymentOverride, base.WorkloadOverride{ - Name: "net-istio-controller", - Env: []base.EnvRequirementsOverride{ - { - Container: "controller", - EnvVars: []corev1.EnvVar{{ - Name: EnableSecretInformerFilteringByCertUIDEnv, - Value: "false", - }, { - Name: "foo", - Value: "foo", - }}, - }}, - }) - }), - shouldAddLabelToSecret: false, + shouldEnableSecretInformerFiltering: false, }, { - name: "deprecated annotation is set to true but env var takes precedence, disabled secret filtering, istio enabled", + name: "disabled secret informer filtering with istio enabled", in: ks(func(ks *operatorv1beta1.KnativeServing) { ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ Enabled: true, }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "true"} ks.Spec.DeploymentOverride = append(ks.Spec.DeploymentOverride, base.WorkloadOverride{ Name: "net-istio-controller", Env: []base.EnvRequirementsOverride{ @@ -211,7 +107,6 @@ func TestSecretInformerFilteringOverride(t *testing.T) { ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ Enabled: true, }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "true"} ks.Spec.DeploymentOverride = append(ks.Spec.DeploymentOverride, base.WorkloadOverride{ Name: "net-istio-controller", Env: []base.EnvRequirementsOverride{ @@ -227,59 +122,16 @@ func TestSecretInformerFilteringOverride(t *testing.T) { }}, }) }), - shouldAddLabelToSecret: false, - }, { - name: "deprecated annotation is set to false but env var takes precedence, enabled secret filtering, istio enabled", - in: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "false"} - ks.Spec.DeploymentOverride = append(ks.Spec.DeploymentOverride, base.WorkloadOverride{ - Name: "net-istio-controller", - Env: []base.EnvRequirementsOverride{ - { - Container: "controller", - EnvVars: []corev1.EnvVar{{ - Name: EnableSecretInformerFilteringByCertUIDEnv, - Value: "true", - }, { - Name: "foo", - Value: "foo", - }}, - }}, - }) - }), - expected: ks(func(ks *operatorv1beta1.KnativeServing) { - ks.Spec.Ingress = &operatorv1beta1.IngressConfigs{Istio: base.IstioIngressConfiguration{ - Enabled: true, - }} - ks.Annotations = map[string]string{secretInformerFilteringAnnotation: "false"} - ks.Spec.DeploymentOverride = append(ks.Spec.DeploymentOverride, base.WorkloadOverride{ - Name: "net-istio-controller", - Env: []base.EnvRequirementsOverride{ - { - Container: "controller", - EnvVars: []corev1.EnvVar{{ - Name: EnableSecretInformerFilteringByCertUIDEnv, - Value: "true", - }, { - Name: "foo", - Value: "foo", - }}, - }}, - }) - }), - shouldAddLabelToSecret: false, + shouldEnableSecretInformerFiltering: false, }} for _, c := range cases { t.Run(c.name, func(t *testing.T) { tf := enableSecretInformerFilteringTransformers(c.in) - if c.shouldAddLabelToSecret { - if tf == nil { - t.Errorf("Secret transformer should not be nil") - } + if c.shouldEnableSecretInformerFiltering && tf == nil { + t.Errorf("Secret informer filtering transformer should not be nil") + } else if !c.shouldEnableSecretInformerFiltering && tf != nil { + t.Errorf("Secret informer filtering transformer should be nil") } if !cmp.Equal(c.in, c.expected) { t.Errorf("Resource was not as expected:\n%s", cmp.Diff(c.in, c.expected))