From b3780e9135285b591cc391663affc04568881018 Mon Sep 17 00:00:00 2001 From: openshift-app-platform-shift-bot <267347085+openshift-app-platform-shift-bot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:08:12 +0000 Subject: [PATCH 1/2] feat: add integration tests for network policy API fields Add comprehensive test coverage for the networkPolicies field in ExternalSecretsConfig controllerConfig, covering the NetworkPolicy struct validation (name, componentName, egress), ComponentName enum values, boundary validation (maxItems, minLength, maxLength), required field enforcement, and immutability rules for name and componentName on update. Enhancement: openshift/enhancements#1834 (ESO-165) Co-Authored-By: Claude Opus 4.6 --- .../externalsecretsconfig.testsuite.yaml | 548 +++++++++++++++++- 1 file changed, 547 insertions(+), 1 deletion(-) 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 720d7fc23..e5c2b724d 100644 --- a/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml +++ b/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml @@ -493,6 +493,387 @@ tests: webhookConfig: certificateCheckInterval: "15m" operatingNamespace: "test-ns" + - name: Should be able to create ExternalSecretsConfig with networkPolicies for CoreController + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-external-secrets-egress + componentName: ExternalSecretsCoreController + egress: + - ports: + - protocol: TCP + port: 443 + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-external-secrets-egress + componentName: ExternalSecretsCoreController + egress: + - ports: + - protocol: TCP + port: 443 + - name: Should be able to create ExternalSecretsConfig with networkPolicies for BitwardenSDKServer + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-bitwarden-egress + componentName: BitwardenSDKServer + egress: + - ports: + - protocol: TCP + port: 6443 + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-bitwarden-egress + componentName: BitwardenSDKServer + egress: + - ports: + - protocol: TCP + port: 6443 + - name: Should be able to create ExternalSecretsConfig with multiple networkPolicies + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-core-egress + componentName: ExternalSecretsCoreController + egress: + - ports: + - protocol: TCP + port: 443 + - name: allow-bitwarden-egress + componentName: BitwardenSDKServer + egress: + - ports: + - protocol: TCP + port: 6443 + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-core-egress + componentName: ExternalSecretsCoreController + egress: + - ports: + - protocol: TCP + port: 443 + - name: allow-bitwarden-egress + componentName: BitwardenSDKServer + egress: + - ports: + - protocol: TCP + port: 6443 + - name: Should be able to create ExternalSecretsConfig with networkPolicy allowing all egress + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-all-egress + componentName: ExternalSecretsCoreController + egress: + - {} + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-all-egress + componentName: ExternalSecretsCoreController + egress: + - {} + - name: Should be able to create ExternalSecretsConfig with empty networkPolicies list + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: [] + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: {} + - name: Should fail with invalid componentName in networkPolicies + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: InvalidComponent + egress: + - {} + expectedError: "spec.controllerConfig.networkPolicies[0].componentName: Unsupported value: \"InvalidComponent\": supported values: \"ExternalSecretsCoreController\", \"BitwardenSDKServer\"" + - name: Should fail with empty name in networkPolicies + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: "" + componentName: ExternalSecretsCoreController + egress: + - {} + expectedError: "spec.controllerConfig.networkPolicies[0].name in body should be at least 1 chars long" + - name: Should fail with name too long in networkPolicies + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: "this-network-policy-name-is-extremely-long-and-exceeds-the-kubernetes-maximum-name-length-limit-of-two-hundred-fifty-three-characters-which-is-quite-a-lot-of-characters-but-we-need-to-test-this-validation-constraint-properly-to-ensure-it-works-as-expected-in-all-scenarios" + componentName: ExternalSecretsCoreController + egress: + - {} + expectedError: "spec.controllerConfig.networkPolicies[0].name: Too long: may not be more than 253 bytes" + - name: Should fail with missing componentName in networkPolicies + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + egress: + - {} + expectedError: "spec.controllerConfig.networkPolicies[0].componentName: Required value" + - name: Should fail with missing egress in networkPolicies + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: ExternalSecretsCoreController + expectedError: "spec.controllerConfig.networkPolicies[0].egress: Required value" + - name: Should fail with too many networkPolicies exceeding maxItems + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: "np-0" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-1" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-2" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-3" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-4" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-5" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-6" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-7" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-8" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-9" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-10" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-11" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-12" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-13" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-14" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-15" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-16" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-17" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-18" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-19" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-20" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-21" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-22" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-23" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-24" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-25" + componentName: ExternalSecretsCoreController + egress: [{}] + - name: "np-26" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-27" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-28" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-29" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-30" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-31" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-32" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-33" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-34" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-35" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-36" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-37" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-38" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-39" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-40" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-41" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-42" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-43" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-44" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-45" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-46" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-47" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-48" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-49" + componentName: BitwardenSDKServer + egress: [{}] + - name: "np-50" + componentName: BitwardenSDKServer + egress: [{}] + expectedError: "spec.controllerConfig.networkPolicies: Too many: 51: must have at most 50 items" + - name: Should be able to create ExternalSecretsConfig with networkPolicies using CIDR-based egress + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-specific-cidr + componentName: ExternalSecretsCoreController + egress: + - to: + - ipBlock: + cidr: 10.0.0.0/8 + ports: + - protocol: TCP + port: 443 + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-specific-cidr + componentName: ExternalSecretsCoreController + egress: + - to: + - ipBlock: + cidr: 10.0.0.0/8 + ports: + - protocol: TCP + port: 443 onUpdate: - name: Should be able to update labels in controller config resourceName: cluster @@ -596,4 +977,169 @@ tests: bitwardenSecretManagerProvider: mode: Enabled secretRef: - name: "bitwarden-certs" \ No newline at end of file + name: "bitwarden-certs" + - name: Should be able to add networkPolicies on update + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: {} + updated: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: ExternalSecretsCoreController + egress: + - {} + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: ExternalSecretsCoreController + egress: + - {} + - name: Should be able to update egress rules in existing networkPolicy + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: ExternalSecretsCoreController + egress: + - ports: + - protocol: TCP + port: 443 + updated: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: ExternalSecretsCoreController + egress: + - ports: + - protocol: TCP + port: 443 + - ports: + - protocol: TCP + port: 8443 + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: ExternalSecretsCoreController + egress: + - ports: + - protocol: TCP + port: 443 + - ports: + - protocol: TCP + port: 8443 + - name: Should be able to add additional networkPolicy entries on update + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-core-egress + componentName: ExternalSecretsCoreController + egress: + - {} + updated: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-core-egress + componentName: ExternalSecretsCoreController + egress: + - {} + - name: allow-bitwarden-egress + componentName: BitwardenSDKServer + egress: + - ports: + - protocol: TCP + port: 6443 + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-core-egress + componentName: ExternalSecretsCoreController + egress: + - {} + - name: allow-bitwarden-egress + componentName: BitwardenSDKServer + egress: + - ports: + - protocol: TCP + port: 6443 + - name: Should not be able to remove existing networkPolicy entry on update + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-core-egress + componentName: ExternalSecretsCoreController + egress: + - {} + - name: allow-bitwarden-egress + componentName: BitwardenSDKServer + egress: + - {} + updated: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-core-egress + componentName: ExternalSecretsCoreController + egress: + - {} + expectedError: "name and componentName fields in networkPolicies are immutable" + - name: Should not be able to change componentName in existing networkPolicy on update + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: ExternalSecretsCoreController + egress: + - {} + updated: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + controllerConfig: + networkPolicies: + - name: allow-egress + componentName: BitwardenSDKServer + egress: + - {} + expectedError: "name and componentName fields in networkPolicies are immutable" \ No newline at end of file From 09e34392fa7dd44a3b7d91c309bac64b6aa2ac43 Mon Sep 17 00:00:00 2001 From: openshift-app-platform-shift-bot <267347085+openshift-app-platform-shift-bot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:15:03 +0000 Subject: [PATCH 2/2] feat: add runtime validation for network policy misconfiguration Add a warning log and event when BitwardenSDKServer network policies are configured but the Bitwarden plugin is not enabled. This helps users detect misconfigurations early without blocking reconciliation. Also adds unit tests for the validateExternalSecretsConfig function. Enhancement: openshift/enhancements#1834 (ESO-165) Co-Authored-By: Claude Opus 4.6 --- pkg/controller/external_secrets/utils.go | 18 +++ pkg/controller/external_secrets/utils_test.go | 113 ++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 pkg/controller/external_secrets/utils_test.go diff --git a/pkg/controller/external_secrets/utils.go b/pkg/controller/external_secrets/utils.go index cb96719f0..7c5250643 100644 --- a/pkg/controller/external_secrets/utils.go +++ b/pkg/controller/external_secrets/utils.go @@ -5,6 +5,7 @@ import ( "fmt" "os" + corev1 "k8s.io/api/core/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" @@ -83,6 +84,23 @@ func (r *Reconciler) validateExternalSecretsConfig(esc *operatorv1alpha1.Externa return fmt.Errorf("spec.controllerConfig.certProvider.certManager.mode is set, but cert-manager is not installed") } } + + // Warn if BitwardenSDKServer network policies are configured but Bitwarden plugin is not enabled. + // The policies will still be created (targeting non-existent pods is harmless), but this helps + // users detect misconfigurations early. + if !isBitwardenConfigEnabled(esc) { + for _, np := range esc.Spec.ControllerConfig.NetworkPolicies { + if np.ComponentName == operatorv1alpha1.BitwardenSDKServer { + r.log.V(1).Info("network policy targets BitwardenSDKServer component but bitwarden plugin is not enabled", + "networkPolicy", np.Name, + "hint", "enable the bitwarden plugin or remove the BitwardenSDKServer network policy") + r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "NetworkPolicyMisconfiguration", + "NetworkPolicy %q targets BitwardenSDKServer but the bitwarden plugin is not enabled", np.Name) + break + } + } + } + return nil } diff --git a/pkg/controller/external_secrets/utils_test.go b/pkg/controller/external_secrets/utils_test.go new file mode 100644 index 000000000..8dfdc6a18 --- /dev/null +++ b/pkg/controller/external_secrets/utils_test.go @@ -0,0 +1,113 @@ +package external_secrets + +import ( + "testing" + + networkingv1 "k8s.io/api/networking/v1" + + operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1" + "github.com/openshift/external-secrets-operator/pkg/controller/commontest" +) + +func TestValidateExternalSecretsConfig(t *testing.T) { + tests := []struct { + name string + setupReconciler func(*Reconciler) + updateExternalSecretsConfig func(*operatorv1alpha1.ExternalSecretsConfig) + wantErr string + }{ + { + name: "valid config with no network policies", + }, + { + name: "valid config with CoreController network policy", + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.NetworkPolicies = []operatorv1alpha1.NetworkPolicy{ + { + Name: "allow-egress", + ComponentName: operatorv1alpha1.CoreController, + Egress: []networkingv1.NetworkPolicyEgressRule{{}}, + }, + } + }, + }, + { + name: "valid config with BitwardenSDKServer network policy and bitwarden enabled", + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.Plugins.BitwardenSecretManagerProvider = &operatorv1alpha1.BitwardenSecretManagerProvider{ + Mode: operatorv1alpha1.Enabled, + } + esc.Spec.ControllerConfig.NetworkPolicies = []operatorv1alpha1.NetworkPolicy{ + { + Name: "allow-bitwarden-egress", + ComponentName: operatorv1alpha1.BitwardenSDKServer, + Egress: []networkingv1.NetworkPolicyEgressRule{{}}, + }, + } + }, + }, + { + name: "warn when BitwardenSDKServer network policy configured but bitwarden disabled", + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.NetworkPolicies = []operatorv1alpha1.NetworkPolicy{ + { + Name: "allow-bitwarden-egress", + ComponentName: operatorv1alpha1.BitwardenSDKServer, + Egress: []networkingv1.NetworkPolicyEgressRule{{}}, + }, + } + }, + // No error expected - this is a warning only + }, + { + name: "cert-manager mode enabled but not installed", + setupReconciler: func(r *Reconciler) { + // optionalResourcesList does NOT contain certificateCRDGKV + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.CertProvider = &operatorv1alpha1.CertProvidersConfig{ + CertManager: &operatorv1alpha1.CertManagerConfig{ + Mode: operatorv1alpha1.Enabled, + }, + } + }, + wantErr: "spec.controllerConfig.certProvider.certManager.mode is set, but cert-manager is not installed", + }, + { + name: "cert-manager mode enabled and installed", + setupReconciler: func(r *Reconciler) { + r.optionalResourcesList[certificateCRDGKV] = struct{}{} + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ControllerConfig.CertProvider = &operatorv1alpha1.CertProvidersConfig{ + CertManager: &operatorv1alpha1.CertManagerConfig{ + Mode: operatorv1alpha1.Enabled, + }, + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + if tt.setupReconciler != nil { + tt.setupReconciler(r) + } + + esc := commontest.TestExternalSecretsConfig() + if tt.updateExternalSecretsConfig != nil { + tt.updateExternalSecretsConfig(esc) + } + + err := r.validateExternalSecretsConfig(esc) + if tt.wantErr != "" { + if err == nil || err.Error() != tt.wantErr { + t.Errorf("Expected error: %v, got: %v", tt.wantErr, err) + } + } else if err != nil { + t.Errorf("Unexpected error: %v", err) + } + }) + } +}