Skip to content

Commit

Permalink
[SRVKS-1179] enable secret filter informer by default (#2445)
Browse files Browse the repository at this point in the history 
* enable secret filter informer by default

* lint

* address comments
  • Loading branch information
@skonto
skonto authored Feb 1, 2024
1 parent 95af918 commit 88d09c3
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 182 deletions.
27 changes: 9 additions & 18 deletions openshift-knative-operator/pkg/serving/secretinformerfiltering.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
180 changes: 16 additions & 164 deletions openshift-knative-operator/pkg/serving/secretinformerfilterting_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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))
Expand Down

0 comments on commit 88d09c3

Please sign in to comment.