ESO-166:Removes resources created for bitwarden server when bitwardenSecretManager is disable

bundle/manifests/external-secrets-operator.clusterserviceversion.yaml

@@ -204,7 +204,7 @@ metadata:
     categories: Security
     console.openshift.io/disable-operand-delete: "true"
     containerImage: openshift.io/external-secrets-operator:latest
-    createdAt: "2025-08-20T18:17:42Z"
+    createdAt: "2025-09-08T08:44:22Z"
     features.operators.openshift.io/cnf: "false"
     features.operators.openshift.io/cni: "false"
     features.operators.openshift.io/csi: "false"
@@ -412,6 +412,16 @@ spec:
           - cert-manager.io
           resources:
           - certificates
+          verbs:
+          - create
+          - delete
+          - get
+          - list
+          - update
+          - watch
+        - apiGroups:
+          - cert-manager.io
+          resources:
           - clusterissuers
           - issuers
           verbs:

config/rbac/role.yaml

@@ -74,6 +74,16 @@ rules:
   - cert-manager.io
   resources:
   - certificates
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - update
+  - watch
+- apiGroups:
+  - cert-manager.io
+  resources:
   - clusterissuers
   - issuers
   verbs:

pkg/controller/common/utils.go

@@ -480,6 +480,10 @@ func DeleteObject(ctx context.Context, ctrlClient operatorclient.CtrlClient, obj
 		o = DecodeSecretObjBytes(assets.MustAsset(assetName))
 	case *corev1.ServiceAccount:
 		o = DecodeServiceAccountObjBytes(assets.MustAsset(assetName))
+	case *corev1.Service:
+		o = DecodeServiceObjBytes(assets.MustAsset(assetName))
+	case *certmanagerv1.Certificate:
+		o = DecodeCertificateObjBytes(assets.MustAsset(assetName))
 	default:
 		panic(fmt.Sprintf("unsupported object type: %T", obj))
 	}

pkg/controller/external_secrets/certificate.go

@@ -21,20 +21,53 @@ var (
 )
 
 func (r *Reconciler) createOrApplyCertificates(es *operatorv1alpha1.ExternalSecrets, resourceLabels map[string]string, recon bool) error {
-	if isCertManagerConfigEnabled(es) {
-		if err := r.createOrApplyCertificate(es, resourceLabels, webhookCertificateAssetName, recon); err != nil {
-			return err
-		}
+	// Handle webhook certificate
+	if err := r.handleWebhookCertificate(es, resourceLabels, recon); err != nil {
+		return err
+	}
+
+	// Handle bitwarden certificate
+	if err := r.handleBitwardenCertificate(es, resourceLabels, recon); err != nil {
+		return err
 	}
 
+	return nil
+}
+
+// handleWebhookCertificate manages the webhook certificate lifecycle
+func (r *Reconciler) handleWebhookCertificate(es *operatorv1alpha1.ExternalSecrets, resourceLabels map[string]string, recon bool) error {
+	// Only create webhook certificate if cert-manager is enabled
+	if !isCertManagerConfigEnabled(es) {
+		return nil
+	}
+
+	return r.createOrApplyCertificate(es, resourceLabels, webhookCertificateAssetName, recon)
+}
+
+// handleBitwardenCertificate manages the bitwarden certificate lifecycle
+func (r *Reconciler) handleBitwardenCertificate(es *operatorv1alpha1.ExternalSecrets, resourceLabels map[string]string, recon bool) error {
+	// Bitwarden certificate handling is independent of cert-manager configuration
+	// Only handle bitwarden certificates when bitwarden is enabled
 	if isBitwardenConfigEnabled(es) {
 		bitwardenConfig := es.Spec.ExternalSecretsConfig.BitwardenSecretManagerProvider
+
+		// If a secret reference is provided, validate it exists instead of creating a certificate
 		if bitwardenConfig.SecretRef.Name != "" {
-			return r.assertSecretRefExists(es, es.Spec.ExternalSecretsConfig.BitwardenSecretManagerProvider)
-		}
-		if err := r.createOrApplyCertificate(es, resourceLabels, bitwardenCertificateAssetName, recon); err != nil {
-			return err
+			return r.assertSecretRefExists(es, bitwardenConfig)
 		}
+
+		// Create or update bitwarden certificate
+		return r.createOrApplyCertificate(es, resourceLabels, bitwardenCertificateAssetName, recon)
+	}
+
+	// If bitwarden is not enabled, clean up any existing bitwarden certificate
+	return r.cleanupBitwardenCertificate()
+}
+
+// cleanupBitwardenCertificate removes the bitwarden certificate when bitwarden is disabled
+func (r *Reconciler) cleanupBitwardenCertificate() error {
+	if err := common.DeleteObject(r.ctx, r.CtrlClient, &certmanagerv1.Certificate{}, bitwardenCertificateAssetName); err != nil {
+		return fmt.Errorf("failed to delete bitwarden certificate: %w", err)
 	}
 	return nil
 }

pkg/controller/external_secrets/certificate_test.go

@@ -440,6 +440,91 @@ func TestCreateOrApplyCertificates(t *testing.T) {
 			},
 			recon: false,
 		},
+		{
+			name: "bitwarden certificate deleted when bitwarden disabled",
+			preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) {
+				m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) {
+					if ns.Name == serviceExternalSecretWebhookName {
+						return false, nil // webhook certificate doesn't exist, will be created
+					}
+					if ns.Name == "bitwarden-tls-certs" {
+						// Bitwarden certificate exists and needs to be deleted
+						cert := testCertificate(bitwardenCertificateAssetName)
+						cert.DeepCopyInto(obj.(*certmanagerv1.Certificate))
+						return true, nil
+					}
+					if ns.Name == "test-issuer" {
+						return true, nil // issuer exists
+					}
+					return false, nil
+				})
+				m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
+					return commontest.TestClientError // Simulates deletion failure
+				})
+				m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
+					cert, ok := obj.(*certmanagerv1.Certificate)
+					if !ok {
+						return fmt.Errorf("expected *certmanagerv1.Certificate, got %T", obj)
+					}
+					if cert.Name == serviceExternalSecretWebhookName {
+						return nil // webhook certificate creation succeeds
+					}
+					t.Errorf("Unexpected create call for %s", cert.Name)
+					return nil
+				})
+				m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error {
+					if ns.Name == "test-issuer" && ns.Namespace == commontest.TestExternalSecretsNamespace {
+						testIssuer().DeepCopyInto(obj.(*certmanagerv1.Issuer))
+						return nil
+					}
+					return fmt.Errorf("object not found")
+				})
+			},
+			es: func(es *v1alpha1.ExternalSecrets) {
+				es.Spec.ExternalSecretsConfig.CertManagerConfig.Enabled = "true"
+				es.Spec.ExternalSecretsConfig.CertManagerConfig.IssuerRef.Name = "test-issuer"
+				// Bitwarden is disabled (explicitly set to false), so deletion should be triggered
+				es.Spec.ExternalSecretsConfig.BitwardenSecretManagerProvider = &v1alpha1.BitwardenSecretManagerProvider{
+					Enabled: "false",
+				}
+			},
+			recon: false,
+			wantErr: "failed to delete bitwarden certificate: test client error",
+		},
+		{
+			name: "no certificate operations when cert-manager disabled",
+			preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) {
+				// Mock functions that should NOT be called
+				m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) {
+					t.Errorf("Exists() should not be called when cert-manager is disabled, but was called for %s", ns.Name)
+					return false, nil
+				})
+				m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
+					t.Errorf("Create() should not be called when cert-manager is disabled")
+					return nil
+				})
+				m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
+					t.Errorf("Delete() should not be called when cert-manager is disabled")
+					return nil
+				})
+				m.GetCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) error {
+					t.Errorf("Get() should not be called when cert-manager is disabled")
+					return nil
+				})
+			},
+			es: func(es *v1alpha1.ExternalSecrets) {
+				// cert-manager is explicitly disabled (or not configured)
+				// This should trigger early return in createOrApplyCertificates
+				es.Spec.ExternalSecretsConfig = &v1alpha1.ExternalSecretsConfig{
+					// No CertManagerConfig means cert-manager is disabled
+					BitwardenSecretManagerProvider: &v1alpha1.BitwardenSecretManagerProvider{
+						Enabled: "true", // Even with Bitwarden enabled, no cert operations should happen
+					},
+				}
+			},
+			recon: false,
+			// No error expected - should return successfully without any operations
+		},
 	}
 
 	for _, tt := range tests {

pkg/controller/external_secrets/controller.go

@@ -100,7 +100,8 @@ type Reconciler struct {
 // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;list;watch;create;update;patch
 // +kubebuilder:rbac:groups="",resources=events;secrets;services;serviceaccounts,verbs=get;list;watch;create;update;delete;patch
 // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;delete
-// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates;clusterissuers;issuers,verbs=get;list;watch;create;update
+// +kubebuilder:rbac:groups=cert-manager.io,resources=clusterissuers;issuers,verbs=get;list;watch;create;update
+// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch;create;update;delete
 // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch;create
 
 // +kubebuilder:rbac:groups="",resources=endpoints,verbs=get;list;watch;create

pkg/controller/external_secrets/service_test.go

@@ -112,6 +112,38 @@ func TestCreateOrApplyServices(t *testing.T) {
 			},
 			wantErr: `failed to create service external-secrets/external-secrets-webhook: test client error`,
 		},
+		{
+			name: "bitwarden service deleted when bitwarden disabled",
+			preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) {
+				m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) {
+					switch o := obj.(type) {
+					case *corev1.Service:
+						if ns.Name == "bitwarden-sdk-server" {
+							svc := testService(bitwardenServiceAssetName)
+							svc.DeepCopyInto(o)
+							return true, nil // Service exists and needs to be deleted
+						}
+						// For webhook service
+						svc := testService("external-secrets/resources/service_external-secrets-webhook.yml")
+						svc.DeepCopyInto(o)
+						return true, nil
+					}
+					return false, nil
+				})
+				m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
+					return commontest.TestClientError // Simulates deletion failure
+				})
+			},
+			updateExternalSecretsObj: func(es *operatorv1alpha1.ExternalSecrets) {
+				// Bitwarden is disabled (default), so deletion should be triggered
+				es.Spec.ExternalSecretsConfig = &operatorv1alpha1.ExternalSecretsConfig{
+					BitwardenSecretManagerProvider: &operatorv1alpha1.BitwardenSecretManagerProvider{
+						Enabled: "false", // This triggers deletion logic
+					},
+				}
+			},
+			wantErr: `failed to delete service object: test client error`,
+		},
 	}
 
 	for _, tt := range tests {

pkg/controller/external_secrets/services.go

@@ -29,6 +29,9 @@ func (r *Reconciler) createOrApplyServices(externalsecrets *operatorv1alpha1.Ext
 
 	for _, service := range servicesToCreate {
 		if !service.condition {
+			if err := common.DeleteObject(r.ctx, r.CtrlClient, &corev1.Service{}, service.assetName); err != nil {
+				return fmt.Errorf("failed to delete service object: %w", err)
+			}
 			continue
 		}
 		if err := r.createOrApplyServiceFromAsset(externalsecrets, service.assetName, resourceLabels, externalsecretsCreateRecon); err != nil {