From 2bffb9f11904ac866a15261605ded64b4c125348 Mon Sep 17 00:00:00 2001 From: siddhi bhor Date: Mon, 8 Sep 2025 12:47:46 +0530 Subject: [PATCH 1/3] Removes resources created for bitwarden server when bitwardenSecretManagerProvider is disabled --- ...ecrets-operator.clusterserviceversion.yaml | 17 ++-- config/rbac/role.yaml | 15 ++-- pkg/controller/common/utils.go | 4 + .../external_secrets/certificate.go | 17 +++- .../external_secrets/certificate_test.go | 85 +++++++++++++++++++ pkg/controller/external_secrets/controller.go | 11 +-- .../external_secrets/service_test.go | 32 +++++++ pkg/controller/external_secrets/services.go | 3 + 8 files changed, 164 insertions(+), 20 deletions(-) diff --git a/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml b/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml index 68e75fcb1..a743edc4a 100644 --- a/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml +++ b/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-08T07:15:06Z" 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: @@ -435,7 +445,6 @@ spec: - external-secrets.io resources: - clusterexternalsecrets - - clusterpushsecrets - clustersecretstores - externalsecrets - pushsecrets @@ -454,8 +463,6 @@ spec: resources: - clusterexternalsecrets/finalizers - clusterexternalsecrets/status - - clusterpushsecrets/finalizers - - clusterpushsecrets/status - clustersecretstores/finalizers - clustersecretstores/status - externalsecrets/finalizers @@ -479,10 +486,8 @@ spec: - generatorstates - githubaccesstokens - grafanas - - mfas - passwords - quayaccesstokens - - sshkeys - stssessiontokens - uuids - vaultdynamicsecrets diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index c790aa378..cdc5e2b3f 100644 --- a/config/rbac/role.yaml +++ b/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: @@ -97,7 +107,6 @@ rules: - external-secrets.io resources: - clusterexternalsecrets - - clusterpushsecrets - clustersecretstores - externalsecrets - pushsecrets @@ -116,8 +125,6 @@ rules: resources: - clusterexternalsecrets/finalizers - clusterexternalsecrets/status - - clusterpushsecrets/finalizers - - clusterpushsecrets/status - clustersecretstores/finalizers - clustersecretstores/status - externalsecrets/finalizers @@ -141,10 +148,8 @@ rules: - generatorstates - githubaccesstokens - grafanas - - mfas - passwords - quayaccesstokens - - sshkeys - stssessiontokens - uuids - vaultdynamicsecrets diff --git a/pkg/controller/common/utils.go b/pkg/controller/common/utils.go index 1f3886a3d..cdea9da3e 100644 --- a/pkg/controller/common/utils.go +++ b/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)) } diff --git a/pkg/controller/external_secrets/certificate.go b/pkg/controller/external_secrets/certificate.go index 359ebf394..456168602 100644 --- a/pkg/controller/external_secrets/certificate.go +++ b/pkg/controller/external_secrets/certificate.go @@ -21,12 +21,16 @@ 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 - } + // Only handle certificates if cert-manager is enabled + if !isCertManagerConfigEnabled(es) { + // If cert-manager is not enabled, we don't create or delete any certificates + return nil } + // Create webhook certificate when cert-manager is enabled + if err := r.createOrApplyCertificate(es, resourceLabels, webhookCertificateAssetName, recon); err != nil { + return err + } if isBitwardenConfigEnabled(es) { bitwardenConfig := es.Spec.ExternalSecretsConfig.BitwardenSecretManagerProvider if bitwardenConfig.SecretRef.Name != "" { @@ -35,7 +39,12 @@ func (r *Reconciler) createOrApplyCertificates(es *operatorv1alpha1.ExternalSecr if err := r.createOrApplyCertificate(es, resourceLabels, bitwardenCertificateAssetName, recon); err != nil { return err } + } else { + 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 } diff --git a/pkg/controller/external_secrets/certificate_test.go b/pkg/controller/external_secrets/certificate_test.go index 12883d64f..6f708930d 100644 --- a/pkg/controller/external_secrets/certificate_test.go +++ b/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 { diff --git a/pkg/controller/external_secrets/controller.go b/pkg/controller/external_secrets/controller.go index dd0a88fee..58348e1ce 100644 --- a/pkg/controller/external_secrets/controller.go +++ b/pkg/controller/external_secrets/controller.go @@ -100,17 +100,18 @@ 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 // +kubebuilder:rbac:groups="",resources=serviceaccounts/token,verbs=create // +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets;clustersecretstores;clusterpushsecrets;externalsecrets;secretstores;pushsecrets,verbs=get;list;watch;create;update;patch;delete;deletecollection -// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/finalizers;clustersecretstores/finalizers;externalsecrets/finalizers;pushsecrets/finalizers;secretstores/finalizers;clusterpushsecrets/finalizers,verbs=get;update;patch -// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/status;clustersecretstores/status;externalsecrets/status;pushsecrets/status;secretstores/status;clusterpushsecrets/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=generators.external-secrets.io,resources=acraccesstokens;clustergenerators;ecrauthorizationtokens;fakes;gcraccesstokens;generatorstates;sshkeys;mfas,verbs=get;list;watch;create;delete;update;patch;deletecollection +// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets;clustersecretstores;externalsecrets;pushsecrets;secretstores,verbs=get;list;watch;create;update;patch;delete;deletecollection +// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/finalizers;clustersecretstores/finalizers;externalsecrets/finalizers;pushsecrets/finalizers;secretstores/finalizers,verbs=get;update;patch +// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/status;clustersecretstores/status;externalsecrets/status;pushsecrets/status;secretstores/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=generators.external-secrets.io,resources=acraccesstokens;clustergenerators;ecrauthorizationtokens;fakes;gcraccesstokens;generatorstates,verbs=get;list;watch;create;delete;update;patch;deletecollection // +kubebuilder:rbac:groups=generators.external-secrets.io,resources=githubaccesstokens;grafanas;passwords;quayaccesstokens;stssessiontokens;uuids;vaultdynamicsecrets;webhooks,verbs=get;list;watch;create;delete;update;patch;deletecollection // New is for building the reconciler instance consumed by the Reconcile method. diff --git a/pkg/controller/external_secrets/service_test.go b/pkg/controller/external_secrets/service_test.go index a43fcac32..f1a1d50e3 100644 --- a/pkg/controller/external_secrets/service_test.go +++ b/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 bitwarden-server service: test client error`, + }, } for _, tt := range tests { diff --git a/pkg/controller/external_secrets/services.go b/pkg/controller/external_secrets/services.go index 15ed73649..b7705419f 100644 --- a/pkg/controller/external_secrets/services.go +++ b/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 bitwarden-server service: %w", err) + } continue } if err := r.createOrApplyServiceFromAsset(externalsecrets, service.assetName, resourceLabels, externalsecretsCreateRecon); err != nil { From f5b78a1b700f5be2a897e828c55c0e79a95b6a91 Mon Sep 17 00:00:00 2001 From: siddhi bhor Date: Mon, 8 Sep 2025 14:18:06 +0530 Subject: [PATCH 2/3] RBAC changes --- .../external-secrets-operator.clusterserviceversion.yaml | 7 ++++++- config/rbac/role.yaml | 5 +++++ pkg/controller/external_secrets/controller.go | 8 ++++---- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml b/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml index a743edc4a..c00b4f55a 100644 --- a/bundle/manifests/external-secrets-operator.clusterserviceversion.yaml +++ b/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-09-08T07:15:06Z" + createdAt: "2025-09-08T08:44:22Z" features.operators.openshift.io/cnf: "false" features.operators.openshift.io/cni: "false" features.operators.openshift.io/csi: "false" @@ -445,6 +445,7 @@ spec: - external-secrets.io resources: - clusterexternalsecrets + - clusterpushsecrets - clustersecretstores - externalsecrets - pushsecrets @@ -463,6 +464,8 @@ spec: resources: - clusterexternalsecrets/finalizers - clusterexternalsecrets/status + - clusterpushsecrets/finalizers + - clusterpushsecrets/status - clustersecretstores/finalizers - clustersecretstores/status - externalsecrets/finalizers @@ -486,8 +489,10 @@ spec: - generatorstates - githubaccesstokens - grafanas + - mfas - passwords - quayaccesstokens + - sshkeys - stssessiontokens - uuids - vaultdynamicsecrets diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index cdc5e2b3f..82d648ed4 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -107,6 +107,7 @@ rules: - external-secrets.io resources: - clusterexternalsecrets + - clusterpushsecrets - clustersecretstores - externalsecrets - pushsecrets @@ -125,6 +126,8 @@ rules: resources: - clusterexternalsecrets/finalizers - clusterexternalsecrets/status + - clusterpushsecrets/finalizers + - clusterpushsecrets/status - clustersecretstores/finalizers - clustersecretstores/status - externalsecrets/finalizers @@ -148,8 +151,10 @@ rules: - generatorstates - githubaccesstokens - grafanas + - mfas - passwords - quayaccesstokens + - sshkeys - stssessiontokens - uuids - vaultdynamicsecrets diff --git a/pkg/controller/external_secrets/controller.go b/pkg/controller/external_secrets/controller.go index 58348e1ce..55e9321df 100644 --- a/pkg/controller/external_secrets/controller.go +++ b/pkg/controller/external_secrets/controller.go @@ -108,10 +108,10 @@ type Reconciler struct { // +kubebuilder:rbac:groups="",resources=serviceaccounts/token,verbs=create // +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets;clustersecretstores;externalsecrets;pushsecrets;secretstores,verbs=get;list;watch;create;update;patch;delete;deletecollection -// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/finalizers;clustersecretstores/finalizers;externalsecrets/finalizers;pushsecrets/finalizers;secretstores/finalizers,verbs=get;update;patch -// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/status;clustersecretstores/status;externalsecrets/status;pushsecrets/status;secretstores/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=generators.external-secrets.io,resources=acraccesstokens;clustergenerators;ecrauthorizationtokens;fakes;gcraccesstokens;generatorstates,verbs=get;list;watch;create;delete;update;patch;deletecollection +// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets;clustersecretstores;clusterpushsecrets;externalsecrets;secretstores;pushsecrets,verbs=get;list;watch;create;update;patch;delete;deletecollection +// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/finalizers;clustersecretstores/finalizers;externalsecrets/finalizers;pushsecrets/finalizers;secretstores/finalizers;clusterpushsecrets/finalizers,verbs=get;update;patch +// +kubebuilder:rbac:groups=external-secrets.io,resources=clusterexternalsecrets/status;clustersecretstores/status;externalsecrets/status;pushsecrets/status;secretstores/status;clusterpushsecrets/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=generators.external-secrets.io,resources=acraccesstokens;clustergenerators;ecrauthorizationtokens;fakes;gcraccesstokens;generatorstates;sshkeys;mfas,verbs=get;list;watch;create;delete;update;patch;deletecollection // +kubebuilder:rbac:groups=generators.external-secrets.io,resources=githubaccesstokens;grafanas;passwords;quayaccesstokens;stssessiontokens;uuids;vaultdynamicsecrets;webhooks,verbs=get;list;watch;create;delete;update;patch;deletecollection // New is for building the reconciler instance consumed by the Reconcile method. From f4a78c798a359bb14d3126545ce68138786b6809 Mon Sep 17 00:00:00 2001 From: siddhi bhor Date: Wed, 10 Sep 2025 19:00:01 +0530 Subject: [PATCH 3/3] added the seperate functions for webhook and bitwarden --- .../external_secrets/certificate.go | 52 ++++++++++++++----- .../external_secrets/service_test.go | 2 +- pkg/controller/external_secrets/services.go | 2 +- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/pkg/controller/external_secrets/certificate.go b/pkg/controller/external_secrets/certificate.go index 456168602..a745c28f4 100644 --- a/pkg/controller/external_secrets/certificate.go +++ b/pkg/controller/external_secrets/certificate.go @@ -21,30 +21,54 @@ var ( ) func (r *Reconciler) createOrApplyCertificates(es *operatorv1alpha1.ExternalSecrets, resourceLabels map[string]string, recon bool) error { - // Only handle certificates if cert-manager is enabled - if !isCertManagerConfigEnabled(es) { - // If cert-manager is not enabled, we don't create or delete any certificates - return nil + // Handle webhook certificate + if err := r.handleWebhookCertificate(es, resourceLabels, recon); err != nil { + return err } - // Create webhook certificate when cert-manager is enabled - if err := r.createOrApplyCertificate(es, resourceLabels, webhookCertificateAssetName, recon); err != nil { + // 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 - } - } else { - if err := common.DeleteObject(r.ctx, r.CtrlClient, &certmanagerv1.Certificate{}, bitwardenCertificateAssetName); err != nil { - return fmt.Errorf("failed to delete bitwarden certificate: %w", 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 } diff --git a/pkg/controller/external_secrets/service_test.go b/pkg/controller/external_secrets/service_test.go index f1a1d50e3..2b29e6347 100644 --- a/pkg/controller/external_secrets/service_test.go +++ b/pkg/controller/external_secrets/service_test.go @@ -142,7 +142,7 @@ func TestCreateOrApplyServices(t *testing.T) { }, } }, - wantErr: `failed to delete bitwarden-server service: test client error`, + wantErr: `failed to delete service object: test client error`, }, } diff --git a/pkg/controller/external_secrets/services.go b/pkg/controller/external_secrets/services.go index b7705419f..cdbee50af 100644 --- a/pkg/controller/external_secrets/services.go +++ b/pkg/controller/external_secrets/services.go @@ -30,7 +30,7 @@ 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 bitwarden-server service: %w", err) + return fmt.Errorf("failed to delete service object: %w", err) } continue }