diff --git a/api/v1alpha1/external_secrets_config_types.go b/api/v1alpha1/external_secrets_config_types.go index 6d24f518..0beb2574 100644 --- a/api/v1alpha1/external_secrets_config_types.go +++ b/api/v1alpha1/external_secrets_config_types.go @@ -116,11 +116,12 @@ type ControllerConfig struct { // annotations are for adding custom annotations to all the resources created for external-secrets deployment. // The annotations are merged with any default annotations set by the operator. User-specified annotations take precedence over defaults in case of conflicts. - // Annotation keys with prefixes `kubernetes.io/`, `app.kubernetes.io/`, `openshift.io/`, or `k8s.io/` are not allowed. + // Annotation keys containing domains `kubernetes.io/`, `openshift.io/`, `cert-manager.io/` or `k8s.io/` (including subdomains like `*.kubernetes.io/`) are not allowed. // +kubebuilder:validation:XValidation:rule="self.all(key, key.matches('^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\\\\/)?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$'))",message="Annotation keys must consist of alphanumeric characters, '-', '_' or '.', starting and ending with alphanumeric, with an optional lowercase DNS subdomain prefix and '/' (e.g., 'my-key' or 'example.com/my-key')" // +kubebuilder:validation:XValidation:rule="self.all(key, !key.contains('/') || key.split('/')[0].size() <= 253)",message="Annotation key prefix (DNS subdomain) must be no more than 253 characters" // +kubebuilder:validation:XValidation:rule="self.all(key, key.contains('/') ? key.split('/')[1].size() <= 63 : key.size() <= 63)",message="Annotation key name part must be no more than 63 characters" - // +kubebuilder:validation:XValidation:rule="self.all(key, !['kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', 'k8s.io/'].exists(p, key.startsWith(p)))",message="Annotation keys with reserved prefixes 'kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not allowed" + // +kubebuilder:validation:XValidation:rule="self.all(key, !key.matches('^([^/]*\\\\.)?(kubernetes\\\\.io|k8s\\\\.io|openshift\\\\.io)/'))",message="Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" + // +kubebuilder:validation:XValidation:rule="self.all(key, !key.matches('^(cert-manager\\\\.io)/'))",message="Annotation keys containing reserved domain 'cert-manager.io/' are not allowed" // +kubebuilder:validation:MinProperties=0 // +kubebuilder:validation:MaxProperties=20 // +kubebuilder:validation:Optional diff --git a/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml b/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml index c6dea6b5..ba33bda9 100644 --- a/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml +++ b/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml @@ -511,7 +511,7 @@ tests: annotations: custom.company.io/team: "platform" custom.company.io/app: "external-secrets" - - name: Should fail with annotation key starting with kubernetes.io + - name: Should allow custom annotations with cert-manager subdomain resourceName: cluster initial: | apiVersion: operator.openshift.io/v1alpha1 @@ -519,19 +519,17 @@ tests: spec: controllerConfig: annotations: - kubernetes.io/service-account: "test" - expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys with reserved prefixes 'kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not allowed" - - name: Should fail with annotation key starting with app.kubernetes.io - resourceName: cluster - initial: | + nothing.cert-manager.io: "space" + something.cert-manager.io/app: "platform" + expected: | apiVersion: operator.openshift.io/v1alpha1 kind: ExternalSecretsConfig spec: controllerConfig: annotations: - app.kubernetes.io/name: "test" - expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys with reserved prefixes 'kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not allowed" - - name: Should fail with annotation key starting with openshift.io + nothing.cert-manager.io: "space" + something.cert-manager.io/app: "platform" + - name: Should fail with annotation key starting with cert-manager.io resourceName: cluster initial: | apiVersion: operator.openshift.io/v1alpha1 @@ -539,9 +537,9 @@ tests: spec: controllerConfig: annotations: - openshift.io/cluster: "test" - expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys with reserved prefixes 'kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not allowed" - - name: Should fail with annotation key starting with k8s.io + cert-manager.io/inject-ca-from: "self-signed-issuer" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domain 'cert-manager.io/' are not allowed" + - name: Should fail with annotation key starting with kubernetes.io resourceName: cluster initial: | apiVersion: operator.openshift.io/v1alpha1 @@ -549,9 +547,9 @@ tests: spec: controllerConfig: annotations: - k8s.io/component: "test" - expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys with reserved prefixes 'kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not allowed" - - name: Should allow annotation key containing but not starting with kubernetes.io + kubernetes.io/service-account: "test" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" + - name: Should fail with annotation key starting with app.kubernetes.io resourceName: cluster initial: | apiVersion: operator.openshift.io/v1alpha1 @@ -559,15 +557,19 @@ tests: spec: controllerConfig: annotations: - custom.kubernetes.io/annotation: "allowed" - expected: | + app.kubernetes.io/name: "test" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" + - name: Should fail with annotation key starting with openshift.io + resourceName: cluster + initial: | apiVersion: operator.openshift.io/v1alpha1 kind: ExternalSecretsConfig spec: controllerConfig: annotations: - custom.kubernetes.io/annotation: "allowed" - - name: Should allow annotation key with subdomain of kubernetes.io + openshift.io/cluster: "test" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" + - name: Should fail with annotation key starting with k8s.io resourceName: cluster initial: | apiVersion: operator.openshift.io/v1alpha1 @@ -575,14 +577,18 @@ tests: spec: controllerConfig: annotations: - monitoring.kubernetes.io/enabled: "true" - expected: | + k8s.io/component: "test" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" + - name: Should not allow annotation key with subdomain of kubernetes.io + resourceName: cluster + initial: | apiVersion: operator.openshift.io/v1alpha1 kind: ExternalSecretsConfig spec: controllerConfig: annotations: - monitoring.kubernetes.io/enabled: "true" + custom.kubernetes.io/annotation: "denied" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" - name: Should allow annotation key ending with kubernetes.io resourceName: cluster initial: | @@ -599,7 +605,7 @@ tests: controllerConfig: annotations: test-kubernetes.io/value: "allowed" - - name: Should allow annotation key with k8s in the middle + - name: Should not allow annotation key with subdomain of k8s.io resourceName: cluster initial: | apiVersion: operator.openshift.io/v1alpha1 @@ -607,14 +613,8 @@ tests: spec: controllerConfig: annotations: - company.k8s.io/annotation: "allowed" - expected: | - apiVersion: operator.openshift.io/v1alpha1 - kind: ExternalSecretsConfig - spec: - controllerConfig: - annotations: - company.k8s.io/annotation: "allowed" + service.k8s.io/annotation: "denied" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" - name: Should allow annotation key similar but not matching reserved prefix resourceName: cluster initial: | @@ -644,7 +644,7 @@ tests: allowed-annotation: "good" kubernetes.io/forbidden: "bad" another-allowed: "good" - expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys with reserved prefixes 'kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not allowed" + expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys containing reserved domains 'kubernetes.io/', 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') are not allowed" - name: Should allow exactly 20 annotations (max limit) resourceName: cluster initial: | @@ -759,16 +759,6 @@ tests: controllerConfig: annotations: custom-company.io/my_annotation.name: "allowed" - - name: Should fail with annotation key exactly matching reserved prefix - resourceName: cluster - initial: | - apiVersion: operator.openshift.io/v1alpha1 - kind: ExternalSecretsConfig - spec: - controllerConfig: - annotations: - kubernetes.io/test: "test" - expectedError: "ExternalSecretsConfig.operator.openshift.io \"cluster\" is invalid: spec.controllerConfig.annotations: Invalid value: \"object\": Annotation keys with reserved prefixes 'kubernetes.io/', 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not allowed" - name: Should allow annotation key that looks similar to reserved but is not resourceName: cluster initial: | @@ -799,7 +789,6 @@ tests: prometheus.io/port: "8080" argocd.argoproj.io/sync-wave: "1" vault.hashicorp.com/agent-inject: "true" - cert-manager.io/cluster-issuer: "letsencrypt" expected: | apiVersion: operator.openshift.io/v1alpha1 kind: ExternalSecretsConfig @@ -810,7 +799,6 @@ tests: prometheus.io/port: "8080" argocd.argoproj.io/sync-wave: "1" vault.hashicorp.com/agent-inject: "true" - cert-manager.io/cluster-issuer: "letsencrypt" - name: Should allow annotation with empty value resourceName: cluster initial: | diff --git a/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml b/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml index d3d52c96..fba37ddf 100644 --- a/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml +++ b/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml @@ -220,7 +220,7 @@ metadata: categories: Security console.openshift.io/disable-operand-delete: "true" containerImage: openshift.io/external-secrets-operator:latest - createdAt: "2026-01-13T09:33:27Z" + createdAt: "2026-01-30T12:44:22Z" features.operators.openshift.io/cnf: "false" features.operators.openshift.io/cni: "false" features.operators.openshift.io/csi: "false" diff --git a/bundle/manifests/operator.openshift.io_externalsecretsconfigs.yaml b/bundle/manifests/operator.openshift.io_externalsecretsconfigs.yaml index 83607c73..34626485 100644 --- a/bundle/manifests/operator.openshift.io_externalsecretsconfigs.yaml +++ b/bundle/manifests/operator.openshift.io_externalsecretsconfigs.yaml @@ -1171,7 +1171,7 @@ spec: description: |- annotations are for adding custom annotations to all the resources created for external-secrets deployment. The annotations are merged with any default annotations set by the operator. User-specified annotations take precedence over defaults in case of conflicts. - Annotation keys with prefixes `kubernetes.io/`, `app.kubernetes.io/`, `openshift.io/`, or `k8s.io/` are not allowed. + Annotation keys containing domains `kubernetes.io/`, `openshift.io/`, `cert-manager.io/` or `k8s.io/` (including subdomains like `*.kubernetes.io/`) are not allowed. maxProperties: 20 minProperties: 0 type: object @@ -1188,11 +1188,13 @@ spec: - message: Annotation key name part must be no more than 63 characters rule: 'self.all(key, key.contains(''/'') ? key.split(''/'')[1].size() <= 63 : key.size() <= 63)' - - message: Annotation keys with reserved prefixes 'kubernetes.io/', - 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not - allowed - rule: self.all(key, !['kubernetes.io/', 'app.kubernetes.io/', - 'openshift.io/', 'k8s.io/'].exists(p, key.startsWith(p))) + - message: Annotation keys containing reserved domains 'kubernetes.io/', + 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') + are not allowed + rule: self.all(key, !key.matches('^([^/]*\\.)?(kubernetes\\.io|k8s\\.io|openshift\\.io)/')) + - message: Annotation keys containing reserved domain 'cert-manager.io/' + are not allowed + rule: self.all(key, !key.matches('^(cert-manager\\.io)/')) certProvider: description: certProvider is for defining the configuration for certificate providers used to manage TLS certificates for webhook diff --git a/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml b/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml index 5de68e0c..a8964aff 100644 --- a/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml +++ b/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml @@ -1171,7 +1171,7 @@ spec: description: |- annotations are for adding custom annotations to all the resources created for external-secrets deployment. The annotations are merged with any default annotations set by the operator. User-specified annotations take precedence over defaults in case of conflicts. - Annotation keys with prefixes `kubernetes.io/`, `app.kubernetes.io/`, `openshift.io/`, or `k8s.io/` are not allowed. + Annotation keys containing domains `kubernetes.io/`, `openshift.io/`, `cert-manager.io/` or `k8s.io/` (including subdomains like `*.kubernetes.io/`) are not allowed. maxProperties: 20 minProperties: 0 type: object @@ -1188,11 +1188,13 @@ spec: - message: Annotation key name part must be no more than 63 characters rule: 'self.all(key, key.contains(''/'') ? key.split(''/'')[1].size() <= 63 : key.size() <= 63)' - - message: Annotation keys with reserved prefixes 'kubernetes.io/', - 'app.kubernetes.io/', 'openshift.io/', or 'k8s.io/' are not - allowed - rule: self.all(key, !['kubernetes.io/', 'app.kubernetes.io/', - 'openshift.io/', 'k8s.io/'].exists(p, key.startsWith(p))) + - message: Annotation keys containing reserved domains 'kubernetes.io/', + 'openshift.io/', 'k8s.io/' (including subdomains like '*.kubernetes.io/') + are not allowed + rule: self.all(key, !key.matches('^([^/]*\\.)?(kubernetes\\.io|k8s\\.io|openshift\\.io)/')) + - message: Annotation keys containing reserved domain 'cert-manager.io/' + are not allowed + rule: self.all(key, !key.matches('^(cert-manager\\.io)/')) certProvider: description: certProvider is for defining the configuration for certificate providers used to manage TLS certificates for webhook diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 2066b0a1..c00208f2 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,7 +5,6 @@ kind: Kustomization images: - name: controller newName: openshift.io/external-secrets-operator - newTag: latest generatorOptions: disableNameSuffixHash: true configMapGenerator: diff --git a/docs/api_reference.md b/docs/api_reference.md index 11cae900..bcb5d1e2 100644 --- a/docs/api_reference.md +++ b/docs/api_reference.md @@ -16,23 +16,6 @@ Package v1alpha1 contains API Schema definitions for the operator v1alpha1 API g -#### Annotation - - - -Annotation is for adding custom annotations to the resources created for external-secrets deployment. - - - -_Appears in:_ -- [ControllerConfig](#controllerconfig) - -| Field | Description | Default | Validation | -| --- | --- | --- | --- | -| `key` _string_ | key is the annotation key. It must follow Kubernetes qualified name syntax: an optional lowercase DNS subdomain prefix (max 253 chars) followed by '/' and a name segment (max 63 chars). The name must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character. Examples: 'my-key', 'example.com/my-key'. | | MaxLength: 317
MinLength: 1
Required: \{\}
| -| `value` _string_ | value is the annotation value. It can be any string, including empty. | | Optional: \{\}
| - - #### ApplicationConfig @@ -218,7 +201,7 @@ _Appears in:_ | --- | --- | --- | --- | | `certProvider` _[CertProvidersConfig](#certprovidersconfig)_ | certProvider is for defining the configuration for certificate providers used to manage TLS certificates for webhook and plugins. | | Optional: \{\}
| | `labels` _object (keys:string, values:string)_ | labels to apply to all resources created for the external-secrets operand deployment.
This field can have a maximum of 20 entries. | | MaxProperties: 20
MinProperties: 0
Optional: \{\}
| -| `annotations` _[Annotation](#annotation) array_ | annotations are for adding custom annotations to all the resources created for external-secrets deployment. The annotations are merged with any default annotations set by the operator. User-specified annotations take precedence over defaults in case of conflicts.
Annotation keys with prefixes `kubernetes.io/`, `app.kubernetes.io/`, `openshift.io/`, or `k8s.io/` are not allowed. | | MaxItems: 20
MinItems: 0
| +| `annotations` _object (keys:string, values:string)_ | annotations are for adding custom annotations to all the resources created for external-secrets deployment.
The annotations are merged with any default annotations set by the operator. User-specified annotations take precedence over defaults in case of conflicts.
Annotation keys with prefixes `kubernetes.io/`, `app.kubernetes.io/`, `openshift.io/`, or `k8s.io/` are not allowed. | | MaxProperties: 20
MinProperties: 0
Optional: \{\}
| | `networkPolicies` _[NetworkPolicy](#networkpolicy) array_ | networkPolicies specifies the list of network policy configurations
to be applied to external-secrets pods.
Each entry allows specifying a name for the generated NetworkPolicy object,
along with its full Kubernetes NetworkPolicy definition.
If this field is not provided, external-secrets components will be isolated
with deny-all network policies, which will prevent proper operation. | | MaxItems: 50
MinItems: 0
Optional: \{\}
| | `componentConfigs` _[ComponentConfig](#componentconfig) array_ | componentConfigs allows specifying deployment-level configuration overrides for individual external-secrets components. This field enables fine-grained control over deployment settings for each component independently.
Each component can only have one configuration entry. | | MaxItems: 4
MinItems: 0
Optional: \{\}
| diff --git a/pkg/controller/common/utils.go b/pkg/controller/common/utils.go index 029daf71..90949a3b 100644 --- a/pkg/controller/common/utils.go +++ b/pkg/controller/common/utils.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strings" "sync" "sync/atomic" @@ -71,6 +72,19 @@ func UpdateResourceLabels(obj client.Object, labels map[string]string) { obj.SetLabels(l) } +// UpdateResourceAnnotations merges provided annotations into the object's existing annotations. +// User-provided annotations take precedence over existing ones. +func UpdateResourceAnnotations(obj client.Object, annotations map[string]string) { + a := obj.GetAnnotations() + if a == nil { + a = make(map[string]string, len(annotations)) + } + for k, v := range annotations { + a[k] = v + } + obj.SetAnnotations(a) +} + func DecodeCertificateObjBytes(objBytes []byte) *certmanagerv1.Certificate { obj, err := runtime.Decode(codecs.UniversalDecoder(certmanagerv1.SchemeGroupVersion), objBytes) if err != nil { @@ -186,14 +200,67 @@ func HasObjectChanged(desired, fetched client.Object) bool { objectModified = networkPolicySpecModified(desired.(*networkingv1.NetworkPolicy), fetched.(*networkingv1.NetworkPolicy)) case *webhook.ValidatingWebhookConfiguration: objectModified = validatingWebHookSpecModified(desired.(*webhook.ValidatingWebhookConfiguration), fetched.(*webhook.ValidatingWebhookConfiguration)) + default: - panic(fmt.Sprintf("unsupported object type: %T", desired)) + return ObjectMetadataModified(desired, fetched) } + return objectModified || ObjectMetadataModified(desired, fetched) } func ObjectMetadataModified(desired, fetched client.Object) bool { - return !reflect.DeepEqual(desired.GetLabels(), fetched.GetLabels()) + // Check if labels have changed + if !reflect.DeepEqual(desired.GetLabels(), fetched.GetLabels()) { + return true + } + // Check if annotations have changed (ignoring system-managed annotations) + // Filter both desired and fetched to avoid infinite reconcile loops caused by + // annotations managed by external controllers (e.g., cert-manager.io/inject-ca-from) + desiredAnnotates := FilterReservedAnnotations(desired.GetAnnotations()) + fetchedAnnotates := FilterReservedAnnotations(fetched.GetAnnotations()) + // Handle nil vs empty map: both are semantically equivalent + if len(desiredAnnotates) == 0 && len(fetchedAnnotates) == 0 { + return false + } + if !reflect.DeepEqual(desiredAnnotates, fetchedAnnotates) { + return true + } + return false +} + +// FilterReservedAnnotations filters out Kubernetes/OpenShift reserved domain annotations +// and annotations managed by external controllers that should not trigger reconciliation. +// Reserved domains: kubernetes.io, openshift.io, k8s.io +func FilterReservedAnnotations(annotations map[string]string) map[string]string { + if len(annotations) == 0 { + return annotations + } + + // Reserved domain patterns: blocks both "kubernetes.io/*" and "*.kubernetes.io/*" + reservedDomains := []string{"kubernetes.io", "openshift.io", "k8s.io"} + + result := make(map[string]string, len(annotations)) + for key, value := range annotations { + isReserved := false + + for _, domain := range reservedDomains { + // Checks for patterns like kubernetes.io/* and *.kubernetes.io/* only + if strings.HasPrefix(key, domain+"/") || strings.Contains(key, "."+domain+"/") { + isReserved = true + break + } + if strings.HasPrefix(key, "cert-manager.io/") { + isReserved = true + break + } + } + + if !isReserved { + result[key] = value + } + } + + return result } func certificateSpecModified(desired, fetched *certmanagerv1.Certificate) bool { @@ -220,6 +287,14 @@ func deploymentSpecModified(desired, fetched *appsv1.Deployment) bool { return true } + // Check pod template annotations + if desired.Spec.Template.Annotations != nil && !reflect.DeepEqual( + desired.Spec.Template.Annotations, + FilterReservedAnnotations(fetched.Spec.Template.Annotations), + ) { + return true + } + // Check volumes if !volumesEqual(desired.Spec.Template.Spec.Volumes, fetched.Spec.Template.Spec.Volumes) { return true @@ -463,12 +538,6 @@ func validatingWebHookSpecModified(desired, fetched *webhook.ValidatingWebhookCo return true } - if desiredVal, exists := desired.GetAnnotations()[CertManagerInjectCAFromAnnotation]; exists { - if desiredVal != fetched.GetAnnotations()[CertManagerInjectCAFromAnnotation] { - return true - } - } - fetchedWebhooksMap := make(map[string]webhook.ValidatingWebhook) for _, wh := range fetched.Webhooks { fetchedWebhooksMap[wh.Name] = wh diff --git a/pkg/controller/common/utils_test.go b/pkg/controller/common/utils_test.go new file mode 100644 index 00000000..17986679 --- /dev/null +++ b/pkg/controller/common/utils_test.go @@ -0,0 +1,73 @@ +package common + +import ( + "reflect" + "testing" + + "github.com/google/go-cmp/cmp" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestFilterAnnotations(t *testing.T) { + source := map[string]string{ + "kubernetes.io/foo": "bar", + "deployment.kubernetes.io/revision": "4", + + "k8s.io/foo": "bar", + "friends.k8s.io/xyz": "infinity", + + "openshift.io/foo": "p", + "services.openshift.io/apiserver": "self", + + "cert-manager.io/secret-name": "x-cert", + } + + // the legit annotations should be retained. + retained := map[string]string{ + "platform.openshift.site/eso": "included", + "platform-team.company.io/component": "eso", + + "kubernetes.io": "foobar", + "k8s.io": "foobar", + + "app.kubernetes.io": "none", + "none.k8s.io": "foo", + + "openshift.io": "enterprise", + "beta.openshift.io": "foobar", + + "cert-manager.io": "ready", + "acme.cert-manager.io": "cert", + "acme.cert-manager.io/dns01-challenge": "dns", + } + + // include all non-rejected annotations in source. + for k, v := range retained { + source[k] = v + } + + filtered := FilterReservedAnnotations(source) + if !reflect.DeepEqual(filtered, retained) { + t.Fatalf("expected filtered annotations mismatch: %s", cmp.Diff(filtered, retained)) + } +} +func TestDeploymentObjectChanged(t *testing.T) { + x := appsv1.Deployment{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "deployment.kubernetes.io/revision": "4", + }, + }, + } + + y := appsv1.Deployment{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{}, + }, + } + + if HasObjectChanged(&y, &x) { + t.Fatal("expected mismatch") + } +} diff --git a/pkg/controller/external_secrets/certificate.go b/pkg/controller/external_secrets/certificate.go index f68c3cfb..8b90072f 100644 --- a/pkg/controller/external_secrets/certificate.go +++ b/pkg/controller/external_secrets/certificate.go @@ -90,6 +90,14 @@ func (r *Reconciler) getCertificateObject(esc *operatorv1alpha1.ExternalSecretsC updateNamespace(certificate, esc) common.UpdateResourceLabels(certificate, resourceLabels) + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(certificate, annotationsMap) + } + } + if err := r.updateCertificateParams(esc, certificate); err != nil { return nil, common.NewIrrecoverableError(err, "failed to update certificate resource for %s/%s deployment", getNamespace(esc), esc.GetName()) } diff --git a/pkg/controller/external_secrets/certificate_test.go b/pkg/controller/external_secrets/certificate_test.go index 580ac7ae..4f16fae7 100644 --- a/pkg/controller/external_secrets/certificate_test.go +++ b/pkg/controller/external_secrets/certificate_test.go @@ -441,6 +441,87 @@ func TestCreateOrApplyCertificates(t *testing.T) { }, recon: false, }, + { + name: "certificate with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + if ns.Name == "test-issuer" { + return true, nil + } + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if cert, ok := obj.(*certmanagerv1.Certificate); ok { + // Verify annotations are applied + if cert.Annotations == nil { + t.Error("certificate annotations should not be nil") + return nil + } + if cert.Annotations["app.io/issue-temporary-certificate"] != "true" { + t.Errorf("expected annotation 'app.io/issue-temporary-certificate'='true', got '%s'", + cert.Annotations["app.io/issue-temporary-certificate"]) + } + } + return nil + }) + m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + if ns.Name == "test-issuer" { + testIssuer().DeepCopyInto(obj.(*certmanagerv1.Issuer)) + return nil + } + return fmt.Errorf("object not found") + }) + }, + esc: func(esc *v1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.CertProvider.CertManager.Mode = v1alpha1.Enabled + esc.Spec.ControllerConfig.CertProvider.CertManager.IssuerRef.Name = "test-issuer" + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "app.io/issue-temporary-certificate": "true", + "team/owner": "security", + } + }, + recon: false, + }, + { + name: "certificate filters reserved annotation prefixes", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + if ns.Name == "test-issuer" { + return true, nil + } + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if cert, ok := obj.(*certmanagerv1.Certificate); ok { + // Verify only allowed annotation is present + if cert.Annotations["allowed-cert-annotation"] != "value" { + t.Errorf("expected 'allowed-cert-annotation'") + } + // Verify reserved prefixes were filtered + if _, exists := cert.Annotations["app.kubernetes.io/component"]; exists { + t.Error("reserved prefix 'app.kubernetes.io/' should have been filtered") + } + } + return nil + }) + m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error { + if ns.Name == "test-issuer" { + testIssuer().DeepCopyInto(obj.(*certmanagerv1.Issuer)) + return nil + } + return fmt.Errorf("object not found") + }) + }, + esc: func(esc *v1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.CertProvider.CertManager.Mode = v1alpha1.Enabled + esc.Spec.ControllerConfig.CertProvider.CertManager.IssuerRef.Name = "test-issuer" + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "allowed-cert-annotation": "value", + "app.kubernetes.io/component": "filtered", + } + }, + recon: false, + }, } for _, tt := range tests { diff --git a/pkg/controller/external_secrets/configmap.go b/pkg/controller/external_secrets/configmap.go index e8ec2433..31927424 100644 --- a/pkg/controller/external_secrets/configmap.go +++ b/pkg/controller/external_secrets/configmap.go @@ -15,7 +15,7 @@ import ( // in the operand namespace when proxy configuration is present. The ConfigMap is labeled // with the injection label required by the Cluster Network Operator (CNO), which watches // for this label and injects the cluster's trusted CA bundle into the ConfigMap's data. -// This function ensures the correct labels are present so that CNO can manage the CA bundle +// This function ensures the correct Testinglabels are present so that CNO can manage the CA bundle // content as expected. func (r *Reconciler) ensureTrustedCABundleConfigMap(esc *operatorv1alpha1.ExternalSecretsConfig, resourceLabels map[string]string) error { proxyConfig := r.getProxyConfiguration(esc) @@ -39,6 +39,14 @@ func (r *Reconciler) ensureTrustedCABundleConfigMap(esc *operatorv1alpha1.Extern }, } + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + desiredConfigMap.Annotations = annotationsMap + } + } + configMapName := fmt.Sprintf("%s/%s", desiredConfigMap.GetNamespace(), desiredConfigMap.GetName()) r.log.V(4).Info("reconciling trusted CA bundle ConfigMap resource", "name", configMapName) @@ -58,17 +66,26 @@ func (r *Reconciler) ensureTrustedCABundleConfigMap(esc *operatorv1alpha1.Extern return nil } - // ConfigMap exists, ensure it has the correct labels + // ConfigMap exists, ensure it has the correct labels and annotations // Do not update the data of the ConfigMap since it is managed by CNO - // Check if metadata (labels) has been modified. - // NOTE: Currently ObjectMetadataModified only checks labels, but if it's extended - // in the future to check annotations as well, CNO may race with this update since - // CNO adds `openshift.io/owning-component: Networking / cluster-network-operator` annotations on this ConfigMap. + // Check if metadata (labels/annotations) has been modified. + // NOTE: CNO adds `openshift.io/owning-component: Networking / cluster-network-operator` annotations on this ConfigMap. + // validateAndFilterAnnotations function filters out reserved prefixes like openshift.io/, so we won't overwrite CNO's annotations. if exist && common.ObjectMetadataModified(desiredConfigMap, existingConfigMap) { r.log.V(1).Info("trusted CA bundle ConfigMap has been modified, updating to desired state", "name", configMapName) - // Update the labels since + // Update the labels existingConfigMap.Labels = desiredConfigMap.Labels + // Merge annotations - preserve existing annotations that are not managed by us + if desiredConfigMap.Annotations != nil { + if existingConfigMap.Annotations == nil { + existingConfigMap.Annotations = make(map[string]string) + } + for k, v := range desiredConfigMap.Annotations { + existingConfigMap.Annotations[k] = v + } + } + if err := r.UpdateWithRetry(r.ctx, existingConfigMap); err != nil { return common.FromClientError(err, "failed to update %s trusted CA bundle ConfigMap resource", configMapName) } diff --git a/pkg/controller/external_secrets/deployments.go b/pkg/controller/external_secrets/deployments.go index f66e7d11..2ef4b4d1 100644 --- a/pkg/controller/external_secrets/deployments.go +++ b/pkg/controller/external_secrets/deployments.go @@ -5,8 +5,10 @@ import ( "os" "unsafe" + "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/apis/core" @@ -103,6 +105,15 @@ func (r *Reconciler) getDeploymentObject(assetName string, esc *operatorv1alpha1 common.UpdateResourceLabels(deployment, resourceLabels) updatePodTemplateLabels(deployment, resourceLabels) + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(deployment, annotationsMap) + updatePodTemplateAnnotations(deployment, annotationsMap) + } + } + image := os.Getenv(externalsecretsImageEnvVarName) if image == "" { return nil, common.NewIrrecoverableError(fmt.Errorf("%s environment variable with externalsecrets image not set", externalsecretsImageEnvVarName), "failed to update image in %s deployment object", deployment.GetName()) @@ -163,6 +174,44 @@ func updatePodTemplateLabels(deployment *appsv1.Deployment, labels map[string]st deployment.Spec.Template.SetLabels(l) } +// validateAndFilterAnnotations validates annotations using Kubernetes validation and filters out reserved domains using the common utility function. +func validateAndFilterAnnotations(annotations map[string]string, logger logr.Logger) map[string]string { + if len(annotations) == 0 { + return annotations + } + + // Validate annotations using Kubernetes built-in validation + if errs := apivalidation.ValidateAnnotations(annotations, field.NewPath("annotations")); len(errs) > 0 { + logger.Error(errs.ToAggregate(), "invalid annotations detected, skipping all annotations") + return make(map[string]string) + } + + // Filter reserved domains kubernetes.io/*, *.kubernetes.io/*, openshift.io/*, *.openshift.io/*, etc. + filtered := common.FilterReservedAnnotations(annotations) + + if len(filtered) < len(annotations) { + for key := range annotations { + if _, exists := filtered[key]; !exists { + logger.V(1).Info("filtered annotation with reserved domain", "key", key) + } + } + } + + return filtered +} + +// updatePodTemplateAnnotations sets annotations on the pod template spec. +func updatePodTemplateAnnotations(deployment *appsv1.Deployment, annotations map[string]string) { + a := deployment.Spec.Template.GetAnnotations() + if a == nil { + a = make(map[string]string, len(annotations)) + } + for k, v := range annotations { + a[k] = v + } + deployment.Spec.Template.SetAnnotations(a) +} + func updateContainerSecurityContext(container *corev1.Container) { container.SecurityContext = &corev1.SecurityContext{ AllowPrivilegeEscalation: ptr.To(false), diff --git a/pkg/controller/external_secrets/install_external_secrets.go b/pkg/controller/external_secrets/install_external_secrets.go index f60ce85e..c738effd 100644 --- a/pkg/controller/external_secrets/install_external_secrets.go +++ b/pkg/controller/external_secrets/install_external_secrets.go @@ -118,10 +118,19 @@ func (r *Reconciler) createOrApplyNamespace(esc *operatorv1alpha1.ExternalSecret namespace := getNamespace(esc) obj := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: namespace, + Name: namespace, + Labels: resourceLabels, }, } + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + obj.Annotations = annotationsMap + } + } + if err := r.Create(r.ctx, obj); err != nil { if errors.IsAlreadyExists(err) { r.log.V(4).Info("namespace already exists", "namespace", namespace) diff --git a/pkg/controller/external_secrets/networkpolicy.go b/pkg/controller/external_secrets/networkpolicy.go index 2e735c2f..527312ab 100644 --- a/pkg/controller/external_secrets/networkpolicy.go +++ b/pkg/controller/external_secrets/networkpolicy.go @@ -137,6 +137,14 @@ func (r *Reconciler) createOrApplyNetworkPolicyFromAsset(esc *operatorv1alpha1.E updateNamespace(networkPolicy, esc) common.UpdateResourceLabels(networkPolicy, resourceLabels) + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(networkPolicy, annotationsMap) + } + } + networkPolicyName := fmt.Sprintf("%s/%s", networkPolicy.GetNamespace(), networkPolicy.GetName()) r.log.V(4).Info("Reconciling static network policy", "name", networkPolicyName) diff --git a/pkg/controller/external_secrets/networkpolicy_test.go b/pkg/controller/external_secrets/networkpolicy_test.go index 55eab12e..84f588d3 100644 --- a/pkg/controller/external_secrets/networkpolicy_test.go +++ b/pkg/controller/external_secrets/networkpolicy_test.go @@ -181,6 +181,61 @@ func TestCreateOrApplyStaticNetworkPolicies(t *testing.T) { }, wantErr: "failed to update network policy external-secrets/deny-all-traffic: test client error", }, + { + name: "network policy with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok { + // Verify annotations are applied + if np.Annotations == nil { + t.Error("networkpolicy annotations should not be nil") + return nil + } + if np.Annotations["security/policy-type"] != "deny-all" { + t.Errorf("expected annotation 'security/policy-type'='deny-all', got '%s'", + np.Annotations["security/policy-type"]) + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "security/policy-type": "deny-all", + "team/owner": "security", + } + }, + }, + { + name: "network policy filters reserved annotation prefixes", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok { + // Verify only allowed annotation is present + if np.Annotations["custom-policy"] != "value" { + t.Errorf("expected 'custom-policy' annotation") + } + // Verify reserved prefixes were filtered + if _, exists := np.Annotations["kubernetes.io/test"]; exists { + t.Error("reserved prefix 'kubernetes.io/' should have been filtered") + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "custom-policy": "value", + "kubernetes.io/test": "filtered", + } + }, + }, } for _, tt := range tests { diff --git a/pkg/controller/external_secrets/rbacs.go b/pkg/controller/external_secrets/rbacs.go index 664fac6c..adfd7d65 100644 --- a/pkg/controller/external_secrets/rbacs.go +++ b/pkg/controller/external_secrets/rbacs.go @@ -45,7 +45,7 @@ func (r *Reconciler) createOrApplyControllerRBACResources(esc *operatorv1alpha1. controllerClusterRoleServiceBindingsAssetName, controllerClusterRoleViewAssetName, } { - clusterRoleObj := r.getClusterRoleObject(asset, resourceLabels) + clusterRoleObj := r.getClusterRoleObject(esc, asset, resourceLabels) if err := r.createOrApplyClusterRole(esc, clusterRoleObj, recon); err != nil { r.log.Error(err, "failed to reconcile controller clusterrole resources") return err @@ -82,7 +82,7 @@ func (r *Reconciler) createOrApplyCertControllerRBACResources(esc *operatorv1alp return nil } - clusterRoleObj := r.getClusterRoleObject(certControllerClusterRoleAssetName, resourceLabels) + clusterRoleObj := r.getClusterRoleObject(esc, certControllerClusterRoleAssetName, resourceLabels) if err := r.createOrApplyClusterRole(esc, clusterRoleObj, recon); err != nil { r.log.Error(err, "failed to reconcile cert-controller clusterrole resources") return err @@ -135,9 +135,18 @@ func (r *Reconciler) createOrApplyClusterRole(esc *operatorv1alpha1.ExternalSecr // getClusterRoleObject is for obtaining the content of given ClusterRole static asset, and // then updating it with desired values. -func (r *Reconciler) getClusterRoleObject(assetName string, resourceLabels map[string]string) *rbacv1.ClusterRole { +func (r *Reconciler) getClusterRoleObject(esc *operatorv1alpha1.ExternalSecretsConfig, assetName string, resourceLabels map[string]string) *rbacv1.ClusterRole { clusterRole := common.DecodeClusterRoleObjBytes(assets.MustAsset(assetName)) common.UpdateResourceLabels(clusterRole, resourceLabels) + + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(clusterRole, annotationsMap) + } + } + return clusterRole } @@ -183,6 +192,15 @@ func (r *Reconciler) getClusterRoleBindingObject(esc *operatorv1alpha1.ExternalS clusterRoleBinding := common.DecodeClusterRoleBindingObjBytes(assets.MustAsset(assetName)) clusterRoleBinding.RoleRef.Name = clusterRoleName common.UpdateResourceLabels(clusterRoleBinding, resourceLabels) + + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(clusterRoleBinding, annotationsMap) + } + } + updateServiceAccountNamespaceInRBACBindingObject[*rbacv1.ClusterRoleBinding](clusterRoleBinding, serviceAccountName, getNamespace(esc)) return clusterRoleBinding } @@ -225,6 +243,15 @@ func (r *Reconciler) getRoleObject(esc *operatorv1alpha1.ExternalSecretsConfig, role := common.DecodeRoleObjBytes(assets.MustAsset(assetName)) updateNamespace(role, esc) common.UpdateResourceLabels(role, resourceLabels) + + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(role, annotationsMap) + } + } + return role } @@ -268,6 +295,15 @@ func (r *Reconciler) getRoleBindingObject(esc *operatorv1alpha1.ExternalSecretsC roleBinding.RoleRef.Name = roleName updateNamespace(roleBinding, esc) common.UpdateResourceLabels(roleBinding, resourceLabels) + + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(roleBinding, annotationsMap) + } + } + updateServiceAccountNamespaceInRBACBindingObject[*rbacv1.RoleBinding](roleBinding, serviceAccountName, roleBinding.GetNamespace()) return roleBinding } diff --git a/pkg/controller/external_secrets/rbacs_test.go b/pkg/controller/external_secrets/rbacs_test.go index 56c565f5..5bc95ddc 100644 --- a/pkg/controller/external_secrets/rbacs_test.go +++ b/pkg/controller/external_secrets/rbacs_test.go @@ -297,6 +297,133 @@ func TestCreateOrApplyRBACResource(t *testing.T) { { name: "rolebindings creation successful", }, + { + name: "clusterrole with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if cr, ok := obj.(*rbacv1.ClusterRole); ok { + // Verify annotations are applied + if cr.Annotations != nil { + if cr.Annotations["rbac/managed-by"] != "operator" { + t.Errorf("expected annotation 'rbac/managed-by'='operator', got '%s'", + cr.Annotations["rbac/managed-by"]) + } + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "rbac/managed-by": "operator", + "team/owner": "platform", + } + }, + }, + { + name: "clusterrolebinding with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if crb, ok := obj.(*rbacv1.ClusterRoleBinding); ok { + // Verify annotations are applied + if crb.Annotations != nil { + if crb.Annotations["binding/type"] != "cluster" { + t.Errorf("expected annotation 'binding/type'='cluster'") + } + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "binding/type": "cluster", + } + }, + }, + { + name: "role with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if role, ok := obj.(*rbacv1.Role); ok { + // Verify annotations are applied + if role.Annotations != nil { + if role.Annotations["role/purpose"] != "leader-election" { + t.Errorf("expected annotation 'role/purpose'='leader-election'") + } + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "role/purpose": "leader-election", + } + }, + }, + { + name: "rolebinding with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if rb, ok := obj.(*rbacv1.RoleBinding); ok { + // Verify annotations are applied + if rb.Annotations != nil { + if rb.Annotations["binding/scope"] != "namespace" { + t.Errorf("expected annotation 'binding/scope'='namespace'") + } + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "binding/scope": "namespace", + } + }, + }, + { + name: "rbac resources filter reserved annotation prefixes", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + switch obj := obj.(type) { + case *rbacv1.ClusterRole, *rbacv1.ClusterRoleBinding, *rbacv1.Role, *rbacv1.RoleBinding: + // Verify reserved prefixes were filtered + metaObj := obj.(client.Object) + annotations := metaObj.GetAnnotations() + if _, exists := annotations["openshift.io/test"]; exists { + t.Error("reserved prefix 'openshift.io/' should have been filtered") + } + if annotations["allowed-rbac"] != "value" { + t.Errorf("expected 'allowed-rbac' annotation") + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "allowed-rbac": "value", + "openshift.io/test": "filtered", + } + }, + }, } for _, tt := range tests { diff --git a/pkg/controller/external_secrets/secret.go b/pkg/controller/external_secrets/secret.go index 0f673ff4..ebf41510 100644 --- a/pkg/controller/external_secrets/secret.go +++ b/pkg/controller/external_secrets/secret.go @@ -61,5 +61,14 @@ func (r *Reconciler) getSecretObject(esc *operatorv1alpha1.ExternalSecretsConfig updateNamespace(secret, esc) common.UpdateResourceLabels(secret, resourceLabels) + + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(secret, annotationsMap) + } + } + return secret, nil } diff --git a/pkg/controller/external_secrets/secret_test.go b/pkg/controller/external_secrets/secret_test.go index d8a9a851..a9c7fb36 100644 --- a/pkg/controller/external_secrets/secret_test.go +++ b/pkg/controller/external_secrets/secret_test.go @@ -184,6 +184,61 @@ func TestCreateOrApplySecret(t *testing.T) { preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { }, }, + { + name: "secret with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if secret, ok := obj.(*corev1.Secret); ok { + // Verify annotations are applied + if secret.Annotations == nil { + t.Error("secret annotations should not be nil") + return nil + } + if secret.Annotations["vault.hashicorp.com/secret-type"] != "webhook-cert" { + t.Errorf("expected annotation 'vault.hashicorp.com/secret-type'='webhook-cert', got '%s'", + secret.Annotations["vault.hashicorp.com/secret-type"]) + } + } + return nil + }) + }, + esc: func(esc *v1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "vault.hashicorp.com/secret-type": "webhook-cert", + "security/classification": "confidential", + } + }, + }, + { + name: "secret filters reserved annotation prefixes", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if secret, ok := obj.(*corev1.Secret); ok { + // Verify only allowed annotation is present + if secret.Annotations["allowed-secret-annotation"] != "value" { + t.Errorf("expected 'allowed-secret-annotation'") + } + // Verify reserved prefixes were filtered + if _, exists := secret.Annotations["k8s.io/test"]; exists { + t.Error("reserved prefix 'k8s.io/' should have been filtered") + } + } + return nil + }) + }, + esc: func(esc *v1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "allowed-secret-annotation": "value", + "k8s.io/test": "filtered", + } + }, + }, } for _, tt := range tests { diff --git a/pkg/controller/external_secrets/service_test.go b/pkg/controller/external_secrets/service_test.go index 644b6ef0..62a04300 100644 --- a/pkg/controller/external_secrets/service_test.go +++ b/pkg/controller/external_secrets/service_test.go @@ -114,6 +114,80 @@ func TestCreateOrApplyServices(t *testing.T) { }, wantErr: `failed to create service external-secrets/external-secrets-webhook: test client error`, }, + { + name: "service with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + var capturedService *corev1.Service + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + switch svc := obj.(type) { + case *corev1.Service: + capturedService = svc.DeepCopy() + // Verify annotations are applied + if capturedService.Annotations == nil { + t.Error("service annotations should not be nil") + return nil + } + if capturedService.Annotations["prometheus.io/scrape"] != "true" { + t.Errorf("expected annotation 'prometheus.io/scrape'='true', got '%s'", + capturedService.Annotations["prometheus.io/scrape"]) + } + if capturedService.Annotations["team/owner"] != "platform" { + t.Errorf("expected annotation 'team/owner'='platform', got '%s'", + capturedService.Annotations["team/owner"]) + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "prometheus.io/scrape": "true", + "team/owner": "platform", + } + }, + }, + { + name: "service filters reserved annotation prefixes", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + var capturedService *corev1.Service + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + switch svc := obj.(type) { + case *corev1.Service: + capturedService = svc.DeepCopy() + // Verify only allowed annotation is present + if capturedService.Annotations == nil { + t.Error("service annotations should not be nil") + return nil + } + if capturedService.Annotations["allowed-key"] != "allowed-value" { + t.Errorf("expected 'allowed-key' annotation, got '%s'", + capturedService.Annotations["allowed-key"]) + } + // Verify reserved prefixes were filtered + if _, exists := capturedService.Annotations["kubernetes.io/test"]; exists { + t.Error("reserved prefix 'kubernetes.io/' should have been filtered") + } + if _, exists := capturedService.Annotations["app.kubernetes.io/component"]; exists { + t.Error("reserved prefix 'app.kubernetes.io/' should have been filtered") + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "allowed-key": "allowed-value", + "kubernetes.io/test": "filtered", + "app.kubernetes.io/component": "filtered", + } + }, + }, } for _, tt := range tests { diff --git a/pkg/controller/external_secrets/serviceaccounts.go b/pkg/controller/external_secrets/serviceaccounts.go index 14ba33f8..6bfa80ee 100644 --- a/pkg/controller/external_secrets/serviceaccounts.go +++ b/pkg/controller/external_secrets/serviceaccounts.go @@ -44,6 +44,14 @@ func (r *Reconciler) createOrApplyServiceAccounts(esc *operatorv1alpha1.External updateNamespace(desired, esc) common.UpdateResourceLabels(desired, resourceLabels) + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(desired, annotationsMap) + } + } + serviceAccountName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName()) r.log.V(4).Info("reconciling serviceaccount resource", "name", serviceAccountName) @@ -57,7 +65,15 @@ func (r *Reconciler) createOrApplyServiceAccounts(esc *operatorv1alpha1.External if externalSecretsConfigCreateRecon { r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s serviceaccount already exists, possibly from a previous install", serviceAccountName) } - r.log.V(4).Info("serviceaccount exists", "name", serviceAccountName) + if common.HasObjectChanged(desired, fetched) { + r.log.V(1).Info("ServiceAccount modified, updating", "name", serviceAccountName) + if err := r.UpdateWithRetry(r.ctx, desired); err != nil { + return common.FromClientError(err, "failed to update serviceaccount %s", serviceAccountName) + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "ServiceAccount %s updated", serviceAccountName) + } else { + r.log.V(4).Info("serviceaccount already up-to-date", "name", serviceAccountName) + } } else { if err := r.Create(r.ctx, desired); err != nil { return common.FromClientError(err, "failed to create serviceaccount %s", serviceAccountName) diff --git a/pkg/controller/external_secrets/serviceaccounts_test.go b/pkg/controller/external_secrets/serviceaccounts_test.go index 70809fc9..986dc03d 100644 --- a/pkg/controller/external_secrets/serviceaccounts_test.go +++ b/pkg/controller/external_secrets/serviceaccounts_test.go @@ -122,6 +122,89 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) { }, wantErr: "failed to create serviceaccount external-secrets/external-secrets: test client error", }, + { + name: "serviceaccount with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if sa, ok := obj.(*corev1.ServiceAccount); ok { + // Verify annotations are applied + if sa.Annotations == nil { + t.Error("serviceaccount annotations should not be nil") + return nil + } + if sa.Annotations["vault.hashicorp.com/agent-inject"] != "true" { + t.Errorf("expected annotation 'vault.hashicorp.com/agent-inject'='true', got '%s'", + sa.Annotations["vault.hashicorp.com/agent-inject"]) + } + if sa.Annotations["team/owner"] != "platform" { + t.Errorf("expected annotation 'team/owner'='platform', got '%s'", + sa.Annotations["team/owner"]) + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "vault.hashicorp.com/agent-inject": "true", + "team/owner": "platform", + } + }, + }, + { + name: "serviceaccount filters reserved annotation prefixes with wildcard", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if sa, ok := obj.(*corev1.ServiceAccount); ok { + if sa.Annotations == nil { + t.Error("serviceaccount annotations should not be nil") + return nil + } + + // Verify only allowed annotation is present + if sa.Annotations["allowed-annotation"] != "value" { + t.Errorf("expected 'allowed-annotation', got '%s'", + sa.Annotations["allowed-annotation"]) + } + + reservedKeys := []string{ + "kubernetes.io/test", + "app.kubernetes.io/component", + "deployment.kubernetes.io/revision", + "openshift.io/test", + "console.openshift.io/route", + "k8s.io/test", + } + + for _, key := range reservedKeys { + if val, exists := sa.Annotations[key]; exists && val == "filtered" { + t.Errorf("reserved annotation %q should have been filtered but found with value %q", key, val) + } + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "allowed-annotation": "value", + // Direct domain patterns + "kubernetes.io/test": "filtered", + "openshift.io/test": "filtered", + "k8s.io/test": "filtered", + // Wildcard subdomain patterns (should also be filtered) + "app.kubernetes.io/component": "filtered", + "deployment.kubernetes.io/revision": "filtered", + "console.openshift.io/route": "filtered", + } + }, + }, } for _, tt := range tests { diff --git a/pkg/controller/external_secrets/services.go b/pkg/controller/external_secrets/services.go index cb023865..e3fbaf0d 100644 --- a/pkg/controller/external_secrets/services.go +++ b/pkg/controller/external_secrets/services.go @@ -53,6 +53,14 @@ func (r *Reconciler) createOrApplyServiceFromAsset(esc *operatorv1alpha1.Externa updateNamespace(service, esc) common.UpdateResourceLabels(service, resourceLabels) + // Apply annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(service, annotationsMap) + } + } + serviceName := fmt.Sprintf("%s/%s", service.GetNamespace(), service.GetName()) r.log.V(4).Info("Reconciling service", "name", serviceName) diff --git a/pkg/controller/external_secrets/validatingwebhook.go b/pkg/controller/external_secrets/validatingwebhook.go index d5013e9d..5bc96148 100644 --- a/pkg/controller/external_secrets/validatingwebhook.go +++ b/pkg/controller/external_secrets/validatingwebhook.go @@ -59,6 +59,15 @@ func (r *Reconciler) getValidatingWebhookObjects(esc *operatorv1alpha1.ExternalS validatingWebhook := common.DecodeValidatingWebhookConfigurationObjBytes(assets.MustAsset(assetName)) common.UpdateResourceLabels(validatingWebhook, resourceLabels) + + // Apply custom annotations from ControllerConfig + if len(esc.Spec.ControllerConfig.Annotations) > 0 { + annotationsMap := validateAndFilterAnnotations(esc.Spec.ControllerConfig.Annotations, r.log) + if len(annotationsMap) > 0 { + common.UpdateResourceAnnotations(validatingWebhook, annotationsMap) + } + } + if err := updateValidatingWebhookAnnotation(esc, validatingWebhook); err != nil { return nil, fmt.Errorf("failed to update validatingWebhook resource for %s external secrets: %s", esc.GetName(), err.Error()) } diff --git a/pkg/controller/external_secrets/validatingwebhook_test.go b/pkg/controller/external_secrets/validatingwebhook_test.go index be17bfbd..9673371a 100644 --- a/pkg/controller/external_secrets/validatingwebhook_test.go +++ b/pkg/controller/external_secrets/validatingwebhook_test.go @@ -20,9 +20,10 @@ var ( func TestCreateOrApplyValidatingWebhookConfiguration(t *testing.T) { tests := []struct { - name string - preReq func(*Reconciler, *fakes.FakeCtrlClient) - wantErr string + name string + preReq func(*Reconciler, *fakes.FakeCtrlClient) + updateExternalSecretsConfig func(*v1alpha1.ExternalSecretsConfig) + wantErr string }{ { name: "validatingWebhookConfiguration reconciliation successful", @@ -97,6 +98,61 @@ func TestCreateOrApplyValidatingWebhookConfiguration(t *testing.T) { }) }, }, + { + name: "validatingWebhookConfiguration with custom annotations applied successfully", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if vwc, ok := obj.(*webhook.ValidatingWebhookConfiguration); ok { + // Verify annotations are applied + if vwc.Annotations == nil { + t.Error("validatingwebhook annotations should not be nil") + return nil + } + if vwc.Annotations["monitoring/enabled"] != "true" { + t.Errorf("expected annotation 'monitoring/enabled'='true', got '%s'", + vwc.Annotations["monitoring/enabled"]) + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *v1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "monitoring/enabled": "true", + "team/owner": "platform", + } + }, + }, + { + name: "validatingWebhookConfiguration filters reserved annotation prefixes", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if vwc, ok := obj.(*webhook.ValidatingWebhookConfiguration); ok { + // Verify only allowed annotation is present + if vwc.Annotations["custom-key"] != "custom-value" { + t.Errorf("expected 'custom-key' annotation") + } + // Verify reserved prefixes were filtered + if _, exists := vwc.Annotations["kubernetes.io/test"]; exists { + t.Error("reserved prefix 'kubernetes.io/' should have been filtered") + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *v1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.Annotations = map[string]string{ + "custom-key": "custom-value", + "kubernetes.io/test": "filtered", + } + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -105,10 +161,13 @@ func TestCreateOrApplyValidatingWebhookConfiguration(t *testing.T) { if tt.preReq != nil { tt.preReq(r, mock) } - r.CtrlClient = mock - externalSecretsForValidateWebhook := testExternalSecretsForValidateWebhookConfiguration() + r.CtrlClient = mock + externalSecretsForValidateWebhook := testExternalSecretsForValidateWebhookConfiguration() + if tt.updateExternalSecretsConfig != nil { + tt.updateExternalSecretsConfig(externalSecretsForValidateWebhook) + } - err := r.createOrApplyValidatingWebhookConfiguration(externalSecretsForValidateWebhook, controllerDefaultResourceLabels, false) + err := r.createOrApplyValidatingWebhookConfiguration(externalSecretsForValidateWebhook, controllerDefaultResourceLabels, false) if (tt.wantErr != "" || err != nil) && (err == nil || err.Error() != tt.wantErr) { t.Errorf("createOrApplyValidatingWebhookConfiguration() err: %v, wantErr: %v", err, tt.wantErr) } diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 332474f2..16f75979 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -23,18 +23,25 @@ import ( "embed" "encoding/base64" "fmt" + "strings" "testing" "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1" "github.com/openshift/external-secrets-operator/test/utils" ) @@ -43,9 +50,9 @@ var testassets embed.FS const ( // test bindata - externalSecretsFile = "testdata/external_secret.yaml" + externalSecretsFile = "testdata/external_secret.yaml" externalSecretsFileWithRevisionLimit = "testdata/external_secret_with_revision_limits.yaml" - expectedSecretValueFile = "testdata/expected_value.yaml" + expectedSecretValueFile = "testdata/expected_value.yaml" ) const ( @@ -73,6 +80,7 @@ var _ = Describe("External Secrets Operator End-to-End test scenarios", Ordered, var ( clientset *kubernetes.Clientset dynamicClient *dynamic.DynamicClient + runtimeClient client.Client loader utils.DynamicResourceLoader awsSecretName string testNamespace string @@ -88,6 +96,14 @@ var _ = Describe("External Secrets Operator End-to-End test scenarios", Ordered, dynamicClient, err = dynamic.NewForConfig(cfg) Expect(err).Should(BeNil()) + // Create scheme and register types + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(operatorv1alpha1.AddToScheme(scheme)) + + runtimeClient, err = client.New(cfg, client.Options{Scheme: scheme}) + Expect(err).Should(BeNil()) + awsSecretName = fmt.Sprintf("eso-e2e-secret-%s", utils.GetRandomString(5)) namespace := &corev1.Namespace{ @@ -113,15 +129,6 @@ var _ = Describe("External Secrets Operator End-to-End test scenarios", Ordered, loader.CreateFromFile(testassets.ReadFile, externalSecretsFile, "") }) - AfterAll(func() { - By("Deleting the externalsecrets.openshift.operator.io/cluster CR") - loader.DeleteFromFile(testassets.ReadFile, externalSecretsFile, "") - - By("Deleting the test namespace") - Expect(clientset.CoreV1().Namespaces().Delete(ctx, testNamespace, metav1.DeleteOptions{})). - NotTo(HaveOccurred(), "failed to delete test namespace") - }) - BeforeEach(func() { By("Verifying external-secrets operand pods are ready") Expect(utils.VerifyPodsReadyByPrefix(ctx, clientset, operandNamespace, []string{ @@ -303,4 +310,143 @@ var _ = Describe("External Secrets Operator End-to-End test scenarios", Ordered, }, time.Minute, 5*time.Second).Should(Succeed()) }) }) + + Context("Annotations", func() { + It("should apply custom annotations to created resources", func() { + // Define test annotations + testAnnotations := map[string]string{ + "example.com/custom-annotation": "test-value", + "mycompany.io/owner": "platform-team", + } + + By("Updating ExternalSecretsConfig with custom annotations") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR); err != nil { + return err + } + + updatedCR := existingCR.DeepCopy() + updatedCR.Spec.ControllerConfig.Annotations = testAnnotations + + return runtimeClient.Update(ctx, updatedCR) + }) + Expect(err).NotTo(HaveOccurred(), "should update ExternalSecretsConfig with annotations") + + defer func() { + // Remove the test annotations + By("Removing test annotations from ExternalSecretsConfig CR") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + currentCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, currentCR); err != nil { + return err + } + for key := range testAnnotations { + delete(currentCR.Spec.ControllerConfig.Annotations, key) + } + return runtimeClient.Update(ctx, currentCR) + }) + Expect(err).NotTo(HaveOccurred(), "should remove test annotations from ExternalSecretsConfig") + }() + + By("Waiting for external-secrets operand pods to be ready") + Expect(utils.VerifyPodsReadyByPrefix(ctx, clientset, operandNamespace, []string{ + operandCoreControllerPodPrefix, + operandCertControllerPodPrefix, + operandWebhookPodPrefix, + })).To(Succeed()) + + By("Verifying annotations are applied to Deployment resources") + Eventually(func(g Gomega) { + deployments, err := clientset.AppsV1().Deployments(operandNamespace).List(ctx, metav1.ListOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "should list deployments in %s namespace", operandNamespace) + g.Expect(deployments.Items).NotTo(BeEmpty(), "should have at least one deployment") + + for _, deployment := range deployments.Items { + if !strings.HasPrefix(deployment.Name, "external-secrets") { + continue + } + + // annotations on deployment.metadata + annotations := deployment.GetAnnotations() + for key, value := range testAnnotations { + g.Expect(annotations).To(HaveKeyWithValue(key, value), + "deployment %s should have annotation %s=%s", deployment.Name, key, value) + } + + // annotations on deployment.spec.template + annotations = deployment.Spec.Template.Annotations + for key, value := range testAnnotations { + g.Expect(annotations).To(HaveKeyWithValue(key, value), + "deployment %s should have annotation %s=%s", deployment.Name, key, value) + } + } + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + + By("Verifying annotations are applied to Service resources") + Eventually(func(g Gomega) { + services, err := clientset.CoreV1().Services(operandNamespace).List(ctx, metav1.ListOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "should list services in %s namespace", operandNamespace) + + for _, service := range services.Items { + if !strings.HasPrefix(service.Name, "external-secrets") { + continue + } + + annotations := service.GetAnnotations() + for key, value := range testAnnotations { + g.Expect(annotations).To(HaveKeyWithValue(key, value), + "service %s should have annotation %s=%s", service.Name, key, value) + } + } + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + + By("Verifying annotations are applied to ServiceAccount resources") + Eventually(func(g Gomega) { + serviceAccounts, err := clientset.CoreV1().ServiceAccounts(operandNamespace).List(ctx, metav1.ListOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "should list service accounts in %s namespace", operandNamespace) + + for _, sa := range serviceAccounts.Items { + if !strings.HasPrefix(sa.Name, "external-secrets") { + continue + } + + annotations := sa.GetAnnotations() + for key, value := range testAnnotations { + g.Expect(annotations).To(HaveKeyWithValue(key, value), + "service account %s should have annotation %s=%s", sa.Name, key, value) + } + } + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should not allow updating annotations with reserved domain prefix", func() { + By("Getting the existing ExternalSecretsConfig CR") + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR) + Expect(err).NotTo(HaveOccurred(), "should get ExternalSecretsConfig CR") + + By("Attempting to update with a reserved domain annotation") + updatedCR := existingCR.DeepCopy() + if updatedCR.Spec.ControllerConfig.Annotations == nil { + updatedCR.Spec.ControllerConfig.Annotations = make(map[string]string) + } + + // Add two reserved annotations that should be rejected + updatedCR.Spec.ControllerConfig.Annotations["deployment.kubernetes.io/revision"] = "9" + updatedCR.Spec.ControllerConfig.Annotations["k8s.io/not-allowed"] = "denied" + + err = runtimeClient.Update(ctx, updatedCR) + Expect(err).To(HaveOccurred(), "update with reserved domain annotations should fail") + }) + }) + + AfterAll(func() { + By("Deleting the externalsecrets.openshift.operator.io/cluster CR") + loader.DeleteFromFile(testassets.ReadFile, externalSecretsFile, "") + + By("Deleting the test namespace") + Expect(clientset.CoreV1().Namespaces().Delete(ctx, testNamespace, metav1.DeleteOptions{})). + NotTo(HaveOccurred(), "failed to delete test namespace") + }) }) diff --git a/test/e2e/testdata/external_secret.yaml b/test/e2e/testdata/external_secret.yaml index 2fa38abb..b1866026 100644 --- a/test/e2e/testdata/external_secret.yaml +++ b/test/e2e/testdata/external_secret.yaml @@ -22,3 +22,5 @@ spec: port: 5353 # DNS - protocol: UDP port: 5353 # DNS + +