From 0c6c2c0b5c4d16d951c56a46cea32e088b1a4cd1 Mon Sep 17 00:00:00 2001 From: maxkpower Date: Mon, 24 Nov 2025 19:16:09 +0100 Subject: [PATCH 1/6] support using secret names instead of UUID --- README.md | 42 +- api/v1/bitwardensecret_types.go | 8 + .../k8s.bitwarden.com_bitwardensecrets.yaml | 9 + config/samples/k8s_v1_bitwardensecret.yaml | 6 + .../controller/bitwardensecret_controller.go | 107 ++++- .../test/reconciler_edge_cases_test.go | 2 +- internal/controller/test/testutils/fixture.go | 2 +- internal/controller/test/validation_test.go | 375 ++++++++++++++++++ 8 files changed, 543 insertions(+), 8 deletions(-) create mode 100644 internal/controller/test/validation_test.go diff --git a/README.md b/README.md index 731f661..452737a 100644 --- a/README.md +++ b/README.md @@ -94,12 +94,52 @@ Our operator is designed to look for the creation of a custom resource called a - **spec.organizationId**: The Bitwarden organization ID you are pulling Secrets Manager data from - **spec.secretName**: The name of the Kubernetes secret that will be created and injected with Secrets Manager data. - **spec.authToken**: The name of a secret inside of the Kubernetes namespace that the BitwardenSecrets object is being deployed into that contains the Secrets Manager machine account authorization token being used to access secrets. +- **spec.useSecretNames** (optional): When set to `true`, uses secret names from Bitwarden Secrets Manager as Kubernetes secret keys instead of UUIDs. Default: `false` (uses UUIDs for backward compatibility). -Secrets Manager does not guarantee unique secret names across projects, so by default secrets will be created with the Secrets Manager secret UUID used as the key. To make your generated secret easier to use, you can create a map of Bitwarden Secret IDs to Kubernetes secret keys. The generated secret will replace the Bitwarden Secret IDs with the mapped friendly name you provide. Below are the map settings available: +#### Secret Key Naming + +By default, secrets are created with the Secrets Manager secret UUID used as the key. This ensures backward compatibility and handles cases where secret names may not be unique or POSIX-compliant. + +**Option 1: Use Secret Names (Recommended for new deployments)** + +Set `useSecretNames: true` to use the secret names from Bitwarden as Kubernetes secret keys: + +```yaml +spec: + useSecretNames: true +``` + +When enabled: +- Secret names from Bitwarden Secrets Manager will be used as Kubernetes secret keys +- Secret names must be POSIX-compliant (start with letter or underscore, contain only alphanumeric and underscore characters) +- Secret names must be unique across all accessible secrets +- Validation errors will prevent secret synchronization with detailed error messages + +Example result: +```yaml +Data: + DATABASE_PASSWORD: + API_KEY: + REDIS_URL: +``` + +**Option 2: Use UUID Keys with Mapping (Default)** + +By default, secrets use UUIDs as keys. To make your generated secret easier to use, you can create a map of Bitwarden Secret IDs to Kubernetes secret keys. The generated secret will replace the Bitwarden Secret IDs with the mapped friendly name you provide. Below are the map settings available: - **bwSecretId**: This is the UUID of the secret in Secrets Manager. This can found under the secret name in the Secrets Manager web portal or by using the [Bitwarden Secrets Manager CLI](https://github.com/bitwarden/sdk/releases). - **secretKeyName**: The resulting key inside the Kubernetes secret that replaces the UUID +Example: +```yaml +spec: + map: + - bwSecretId: e30f88bd-9e9c-42ae-83b7-b155012da672 + secretKeyName: DATABASE_PASSWORD + - bwSecretId: 9f66ccaf-998e-4e5d-9294-b155012db579 + secretKeyName: API_KEY +``` + Note that the custom mapping is made available on the generated secret for informational purposes in the `k8s.bitwarden.com/custom-map` annotation. #### Creating a BitwardenSecret object diff --git a/api/v1/bitwardensecret_types.go b/api/v1/bitwardensecret_types.go index 40fd0ec..ff97188 100644 --- a/api/v1/bitwardensecret_types.go +++ b/api/v1/bitwardensecret_types.go @@ -51,6 +51,14 @@ type BitwardenSecretSpec struct { // +kubebuilder:validation:Optional // +kubebuilder:default=true OnlyMappedSecrets bool `json:"onlyMappedSecrets"` + // UseSecretNames, when true, uses the secret names from Bitwarden Secrets Manager as Kubernetes secret keys. + // When false or unset (default), uses secret UUIDs as keys (preserving backward compatibility). + // When enabled, secret names must be POSIX-compliant (start with letter/underscore, contain only alphanumeric/underscore) + // and must be unique across all accessible secrets. Validation errors will prevent secret synchronization. + // Defaults to false. + // +kubebuilder:validation:Optional + // +kubebuilder:default=false + UseSecretNames bool `json:"useSecretNames,omitempty"` } type AuthToken struct { diff --git a/config/crd/bases/k8s.bitwarden.com_bitwardensecrets.yaml b/config/crd/bases/k8s.bitwarden.com_bitwardensecrets.yaml index bed8914..4850e76 100644 --- a/config/crd/bases/k8s.bitwarden.com_bitwardensecrets.yaml +++ b/config/crd/bases/k8s.bitwarden.com_bitwardensecrets.yaml @@ -86,6 +86,15 @@ spec: secretName: description: The name of the secret for the type: string + useSecretNames: + default: false + description: |- + UseSecretNames, when true, uses the secret names from Bitwarden Secrets Manager as Kubernetes secret keys. + When false or unset (default), uses secret UUIDs as keys (preserving backward compatibility). + When enabled, secret names must be POSIX-compliant (start with letter/underscore, contain only alphanumeric/underscore) + and must be unique across all accessible secrets. Validation errors will prevent secret synchronization. + Defaults to false. + type: boolean required: - authToken - organizationId diff --git a/config/samples/k8s_v1_bitwardensecret.yaml b/config/samples/k8s_v1_bitwardensecret.yaml index cb470ce..df7e60c 100644 --- a/config/samples/k8s_v1_bitwardensecret.yaml +++ b/config/samples/k8s_v1_bitwardensecret.yaml @@ -11,6 +11,12 @@ metadata: spec: organizationId: "a08a8157-129e-4002-bab4-b118014ca9c7" secretName: bw-sample-secret + + # Optional: Use secret names from Bitwarden as Kubernetes secret keys + # When enabled, secret names must be POSIX-compliant and unique + # Default: false (uses UUIDs as keys for backward compatibility) + # useSecretNames: true + # map: [] map: - bwSecretId: e30f88bd-9e9c-42ae-83b7-b155012da672 diff --git a/internal/controller/bitwardensecret_controller.go b/internal/controller/bitwardensecret_controller.go index d287cdc..21c4148 100644 --- a/internal/controller/bitwardensecret_controller.go +++ b/internal/controller/bitwardensecret_controller.go @@ -128,7 +128,7 @@ func (r *BitwardenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Requ //Get the secrets from the Bitwarden API based on lastSync and organizationId //This will also indicate if the Bitwarden secret needs to be refreshed - refresh, secrets, err := r.PullSecretManagerSecretDeltas(logger, orgId, authToken, lastSync.Time) + refresh, secrets, err := r.PullSecretManagerSecretDeltas(logger, orgId, authToken, lastSync.Time, bwSecret.Spec.UseSecretNames) if err != nil { logErr := r.LogError(logger, ctx, bwSecret, err, fmt.Sprintf("Error pulling Secret Manager secrets from API => API: %s -- Identity: %s -- State: %s -- OrgId: %s ", r.BitwardenClientFactory.GetApiUrl(), r.BitwardenClientFactory.GetIdentityApiUrl(), r.StatePath, orgId)) @@ -266,10 +266,41 @@ func (r *BitwardenSecretReconciler) LogCompletion(logger logr.Logger, ctx contex return nil } +// ValidateSecretKeyName validates that a secret key name is POSIX-compliant. +// POSIX-compliant names are required for environment variable compatibility: +// - Must start with a letter (a-z, A-Z) or underscore (_) +// - Can only contain letters, digits (0-9), and underscores +// - Cannot be empty +func ValidateSecretKeyName(key string) error { + if key == "" { + return fmt.Errorf("secret key cannot be empty") + } + + // Check first character + firstChar := key[0] + if !((firstChar >= 'a' && firstChar <= 'z') || + (firstChar >= 'A' && firstChar <= 'Z') || + firstChar == '_') { + return fmt.Errorf("secret key '%s' must start with a letter or underscore", key) + } + + // Check remaining characters + for i, char := range key { + if !((char >= 'a' && char <= 'z') || + (char >= 'A' && char <= 'Z') || + (char >= '0' && char <= '9') || + char == '_') { + return fmt.Errorf("secret key '%s' contains invalid character '%c' at position %d (only alphanumeric and underscore allowed)", key, char, i) + } + } + + return nil +} + // This function will determine if any secrets have been updated and return all secrets assigned to the machine account if so. // First returned value is a boolean stating if something changed or not. -// The second returned value is a mapping of secret IDs and their values from Secrets Manager -func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Logger, orgId string, authToken string, lastSync time.Time) (bool, map[string][]byte, error) { +// The second returned value is a mapping of secret IDs (or names if useSecretNames is true) and their values from Secrets Manager +func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Logger, orgId string, authToken string, lastSync time.Time, useSecretNames bool) (bool, map[string][]byte, error) { bitwardenClient, err := r.BitwardenClientFactory.GetBitwardenClient() if err != nil { logger.Error(err, "Failed to create client") @@ -298,12 +329,78 @@ func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Lo smSecretVals := smSecretResponse.Secrets + // Legacy mode: Use UUIDs as keys (default behavior for backward compatibility) + if !useSecretNames { + for _, smSecretVal := range smSecretVals { + secrets[smSecretVal.ID] = []byte(smSecretVal.Value) + } + defer bitwardenClient.Close() + return smSecretResponse.HasChanges, secrets, nil + } + + // New mode: Use secret names with validation and duplicate detection + seenKeys := make(map[string][]string) // Track duplicates: key -> []secretIDs + var invalidKeys []string + + // First pass: validate and detect duplicates for _, smSecretVal := range smSecretVals { - secrets[smSecretVal.ID] = []byte(smSecretVal.Value) + secretKey := smSecretVal.Key + + // Validate POSIX compliance + if err := ValidateSecretKeyName(secretKey); err != nil { + logger.Error(err, "Invalid secret key name", + "secretId", smSecretVal.ID, + "secretKey", secretKey) + invalidKeys = append(invalidKeys, + fmt.Sprintf("%s (ID: %s): %v", secretKey, smSecretVal.ID, err)) + continue + } + + // Track for duplicate detection + seenKeys[secretKey] = append(seenKeys[secretKey], smSecretVal.ID) } - defer bitwardenClient.Close() + // Check for duplicates + var duplicates []string + for key, ids := range seenKeys { + if len(ids) > 1 { + duplicates = append(duplicates, + fmt.Sprintf("'%s' (IDs: %v)", key, ids)) + } + } + + // Fail if validation or duplicate errors found + if len(invalidKeys) > 0 || len(duplicates) > 0 { + var errMsg string + + if len(invalidKeys) > 0 { + errMsg += "Invalid secret key names found:\n" + for _, invalid := range invalidKeys { + errMsg += fmt.Sprintf(" - %s\n", invalid) + } + } + + if len(duplicates) > 0 { + if len(invalidKeys) > 0 { + errMsg += "\n" + } + errMsg += "Duplicate secret key names detected:\n" + for _, dup := range duplicates { + errMsg += fmt.Sprintf(" - %s\n", dup) + } + errMsg += "\nEach secret must have a unique name. Please rename the conflicting secrets in Secrets Manager." + } + + defer bitwardenClient.Close() + return false, nil, fmt.Errorf(errMsg) + } + // Second pass: build the secrets map using names + for _, smSecretVal := range smSecretVals { + secrets[smSecretVal.Key] = []byte(smSecretVal.Value) + } + + defer bitwardenClient.Close() return smSecretResponse.HasChanges, secrets, nil } diff --git a/internal/controller/test/reconciler_edge_cases_test.go b/internal/controller/test/reconciler_edge_cases_test.go index d5d190d..7125b97 100644 --- a/internal/controller/test/reconciler_edge_cases_test.go +++ b/internal/controller/test/reconciler_edge_cases_test.go @@ -166,7 +166,7 @@ var _ = Describe("BitwardenSecret Reconciler - Edge Case Tests", Ordered, func() for i := range largeNumOfSecrets { identifier := sdk.SecretIdentifierResponse{ ID: uuid.NewString(), - Key: uuid.NewString(), + Key: fmt.Sprintf("secret_%d", i), // Use realistic POSIX-compliant secret name OrganizationID: fixture.OrgId, } largeSecretMap = append(largeSecretMap, operatorsv1.SecretMap{ diff --git a/internal/controller/test/testutils/fixture.go b/internal/controller/test/testutils/fixture.go index a1cf5ab..ce4a4f4 100644 --- a/internal/controller/test/testutils/fixture.go +++ b/internal/controller/test/testutils/fixture.go @@ -80,7 +80,7 @@ func (f *TestFixture) setup(t *testing.T, runner *EnvTestRunner) { for secretCount := range ExpectedNumOfSecrets { identifier := sdk.SecretIdentifierResponse{ ID: uuid.NewString(), - Key: uuid.NewString(), + Key: fmt.Sprintf("secret_%d", secretCount), // Use realistic POSIX-compliant secret name } //build a map mapping each Identifier to an human readable name based on index diff --git a/internal/controller/test/validation_test.go b/internal/controller/test/validation_test.go new file mode 100644 index 0000000..4021c09 --- /dev/null +++ b/internal/controller/test/validation_test.go @@ -0,0 +1,375 @@ +/* +Source code in this repository is covered by one of two licenses: (i) the +GNU General Public License (GPL) v3.0 (ii) the Bitwarden License v1.0. The +default license throughout the repository is GPL v3.0 unless the header +specifies another license. Bitwarden Licensed code is found only in the +/bitwarden_license directory. + +GPL v3.0: +https://github.com/bitwarden/server/blob/main/LICENSE_GPL.txt + +Bitwarden License v1.0: +https://github.com/bitwarden/server/blob/main/LICENSE_BITWARDEN.txt + +No grant of any rights in the trademarks, service marks, or logos of Bitwarden is +made (except as may be necessary to comply with the notice requirements as +applicable), and use of any Bitwarden trademarks must comply with Bitwarden +Trademark Guidelines +. + +*/ + +package controller_test + +import ( + "time" + + sdk "github.com/bitwarden/sdk-go" + operatorsv1 "github.com/bitwarden/sm-kubernetes/api/v1" + "github.com/bitwarden/sm-kubernetes/internal/controller" + "github.com/bitwarden/sm-kubernetes/internal/controller/test/testutils" + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +var _ = Describe("Secret Name Validation Tests", Ordered, func() { + var ( + namespace string + fixture testutils.TestFixture + ) + + BeforeEach(func() { + fixture = *testutils.NewTestFixture(testContext, envTestRunner) + namespace = fixture.CreateNamespace() + }) + + AfterAll(func() { + fixture.Cancel() + }) + + AfterEach(func() { + fixture.Teardown() + }) + + Describe("ValidateSecretKeyName", func() { + It("should accept valid POSIX-compliant names", func() { + validNames := []string{ + "DATABASE_PASSWORD", + "API_KEY", + "_private_key", + "secret123", + "MY_SECRET_2", + "a", + "_", + "test_secret_name_123", + } + + for _, name := range validNames { + err := controller.ValidateSecretKeyName(name) + Expect(err).NotTo(HaveOccurred(), "Expected '%s' to be valid", name) + } + }) + + It("should reject empty names", func() { + err := controller.ValidateSecretKeyName("") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cannot be empty")) + }) + + It("should reject names starting with a digit", func() { + err := controller.ValidateSecretKeyName("123secret") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must start with a letter or underscore")) + }) + + It("should reject names with hyphens", func() { + err := controller.ValidateSecretKeyName("my-secret") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid character")) + Expect(err.Error()).To(ContainSubstring("-")) + }) + + It("should reject names with spaces", func() { + err := controller.ValidateSecretKeyName("my secret") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid character")) + }) + + It("should reject names with special characters", func() { + invalidChars := []string{"@", "#", "$", "%", "^", "&", "*", "(", ")", "+", "=", "[", "]", "{", "}", "|", "\\", ";", ":", "'", "\"", "<", ">", ",", ".", "?", "/"} + + for _, char := range invalidChars { + name := "secret" + char + "name" + err := controller.ValidateSecretKeyName(name) + Expect(err).To(HaveOccurred(), "Expected '%s' to be invalid", name) + Expect(err.Error()).To(ContainSubstring("invalid character")) + } + }) + }) + + Describe("UseSecretNames mode with validation", func() { + It("should successfully sync with valid secret names", func() { + // Create mock response with valid secret names + validSecretsData := []sdk.SecretResponse{ + { + CreationDate: time.Now().String(), + ID: uuid.NewString(), + Key: "DATABASE_PASSWORD", + Value: "db-secret-value", + OrganizationID: fixture.OrgId, + }, + { + CreationDate: time.Now().String(), + ID: uuid.NewString(), + Key: "API_KEY", + Value: "api-key-value", + OrganizationID: fixture.OrgId, + }, + } + validSecretsResponse := sdk.SecretsSyncResponse{ + HasChanges: true, + Secrets: validSecretsData, + } + + fixture.SetupDefaultCtrlMocks(false, &validSecretsResponse) + + _, err := fixture.CreateDefaultAuthSecret(namespace) + Expect(err).NotTo(HaveOccurred()) + + // Create BitwardenSecret with useSecretNames enabled + bwSecret, err := fixture.CreateBitwardenSecret( + testutils.BitwardenSecretName, + namespace, + fixture.OrgId, + testutils.SynchronizedSecretName, + testutils.AuthSecretName, + testutils.AuthSecretKey, + []operatorsv1.SecretMap{}, // No mapping needed + false, // onlyMappedSecrets = false + ) + Expect(err).NotTo(HaveOccurred()) + + // Enable useSecretNames + bwSecret.Spec.UseSecretNames = true + err = fixture.K8sClient.Update(fixture.Ctx, bwSecret) + Expect(err).NotTo(HaveOccurred()) + + // Trigger reconciliation + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} + _, err = fixture.Reconciler.Reconcile(fixture.Ctx, req) + Expect(err).NotTo(HaveOccurred()) + + // Verify K8s secret was created with name-based keys + k8sSecret := &corev1.Secret{} + err = fixture.K8sClient.Get(fixture.Ctx, + types.NamespacedName{Name: testutils.SynchronizedSecretName, Namespace: namespace}, + k8sSecret) + Expect(err).NotTo(HaveOccurred()) + + // Verify keys are secret names, not UUIDs + Expect(k8sSecret.Data).To(HaveKey("DATABASE_PASSWORD")) + Expect(k8sSecret.Data).To(HaveKey("API_KEY")) + Expect(string(k8sSecret.Data["DATABASE_PASSWORD"])).To(Equal("db-secret-value")) + Expect(string(k8sSecret.Data["API_KEY"])).To(Equal("api-key-value")) + }) + + It("should fail with invalid secret names", func() { + // Create mock response with invalid secret names + invalidSecretsData := []sdk.SecretResponse{ + { + CreationDate: time.Now().String(), + ID: uuid.NewString(), + Key: "123-invalid", // Starts with digit + Value: "some-value", + OrganizationID: fixture.OrgId, + }, + { + CreationDate: time.Now().String(), + ID: uuid.NewString(), + Key: "my-secret", // Contains hyphen + Value: "another-value", + OrganizationID: fixture.OrgId, + }, + } + invalidSecretsResponse := sdk.SecretsSyncResponse{ + HasChanges: true, + Secrets: invalidSecretsData, + } + + fixture.SetupDefaultCtrlMocks(false, &invalidSecretsResponse) + + _, err := fixture.CreateDefaultAuthSecret(namespace) + Expect(err).NotTo(HaveOccurred()) + + // Create BitwardenSecret with useSecretNames enabled + bwSecret, err := fixture.CreateBitwardenSecret( + testutils.BitwardenSecretName, + namespace, + fixture.OrgId, + testutils.SynchronizedSecretName, + testutils.AuthSecretName, + testutils.AuthSecretKey, + []operatorsv1.SecretMap{}, + false, + ) + Expect(err).NotTo(HaveOccurred()) + + bwSecret.Spec.UseSecretNames = true + err = fixture.K8sClient.Update(fixture.Ctx, bwSecret) + Expect(err).NotTo(HaveOccurred()) + + // Trigger reconciliation - should fail + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} + _, err = fixture.Reconciler.Reconcile(fixture.Ctx, req) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Invalid secret key names found")) + }) + + It("should fail with duplicate secret names", func() { + // Create mock response with duplicate secret names + duplicateId1 := uuid.NewString() + duplicateId2 := uuid.NewString() + duplicateSecretsData := []sdk.SecretResponse{ + { + CreationDate: time.Now().String(), + ID: duplicateId1, + Key: "DUPLICATE_NAME", + Value: "value1", + OrganizationID: fixture.OrgId, + }, + { + CreationDate: time.Now().String(), + ID: duplicateId2, + Key: "DUPLICATE_NAME", // Same name as above + Value: "value2", + OrganizationID: fixture.OrgId, + }, + } + duplicateSecretsResponse := sdk.SecretsSyncResponse{ + HasChanges: true, + Secrets: duplicateSecretsData, + } + + fixture.SetupDefaultCtrlMocks(false, &duplicateSecretsResponse) + + _, err := fixture.CreateDefaultAuthSecret(namespace) + Expect(err).NotTo(HaveOccurred()) + + // Create BitwardenSecret with useSecretNames enabled + bwSecret, err := fixture.CreateBitwardenSecret( + testutils.BitwardenSecretName, + namespace, + fixture.OrgId, + testutils.SynchronizedSecretName, + testutils.AuthSecretName, + testutils.AuthSecretKey, + []operatorsv1.SecretMap{}, + false, + ) + Expect(err).NotTo(HaveOccurred()) + + bwSecret.Spec.UseSecretNames = true + err = fixture.K8sClient.Update(fixture.Ctx, bwSecret) + Expect(err).NotTo(HaveOccurred()) + + // Trigger reconciliation - should fail + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} + _, err = fixture.Reconciler.Reconcile(fixture.Ctx, req) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Duplicate secret key names detected")) + Expect(err.Error()).To(ContainSubstring("DUPLICATE_NAME")) + }) + }) + + Describe("Backward compatibility - UUID mode (default)", func() { + It("should use UUIDs as keys when useSecretNames is false (default)", func() { + // Use default fixture setup (useSecretNames = false by default) + fixture.SetupDefaultCtrlMocks(false, nil) + + _, err := fixture.CreateDefaultAuthSecret(namespace) + Expect(err).NotTo(HaveOccurred()) + + // Create BitwardenSecret without setting useSecretNames (defaults to false) + _, err = fixture.CreateDefaultBitwardenSecret(namespace, fixture.SecretMap) + Expect(err).NotTo(HaveOccurred()) + + // Trigger reconciliation + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} + _, err = fixture.Reconciler.Reconcile(fixture.Ctx, req) + Expect(err).NotTo(HaveOccurred()) + + // Verify K8s secret was created with UUID keys (mapped via SecretMap) + k8sSecret := &corev1.Secret{} + err = fixture.K8sClient.Get(fixture.Ctx, + types.NamespacedName{Name: testutils.SynchronizedSecretName, Namespace: namespace}, + k8sSecret) + Expect(err).NotTo(HaveOccurred()) + + // Verify keys are the mapped names (from SecretMap), not the secret names + Expect(k8sSecret.Data).To(HaveKey("secret_0_key")) + Expect(k8sSecret.Data).To(HaveKey("secret_1_key")) + Expect(k8sSecret.Data).To(HaveKey("secret_2_key")) + }) + + It("should ignore invalid secret names in UUID mode", func() { + // Create secrets with invalid names that would fail in name mode + invalidNameSecretsData := []sdk.SecretResponse{ + { + CreationDate: time.Now().String(), + ID: uuid.NewString(), + Key: "123-invalid-name", // Would be invalid in name mode + Value: "value1", + OrganizationID: fixture.OrgId, + }, + { + CreationDate: time.Now().String(), + ID: uuid.NewString(), + Key: "my-secret", // Would be invalid in name mode + Value: "value2", + OrganizationID: fixture.OrgId, + }, + } + invalidNameSecretsResponse := sdk.SecretsSyncResponse{ + HasChanges: true, + Secrets: invalidNameSecretsData, + } + + fixture.SetupDefaultCtrlMocks(false, &invalidNameSecretsResponse) + + _, err := fixture.CreateDefaultAuthSecret(namespace) + Expect(err).NotTo(HaveOccurred()) + + // Create BitwardenSecret with useSecretNames = false (or unset) + _, err = fixture.CreateBitwardenSecret( + testutils.BitwardenSecretName, + namespace, + fixture.OrgId, + testutils.SynchronizedSecretName, + testutils.AuthSecretName, + testutils.AuthSecretKey, + []operatorsv1.SecretMap{}, + false, + ) + Expect(err).NotTo(HaveOccurred()) + + // Trigger reconciliation - should succeed (ignores invalid names in UUID mode) + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} + _, err = fixture.Reconciler.Reconcile(fixture.Ctx, req) + Expect(err).NotTo(HaveOccurred()) + + // Verify K8s secret was created with UUID keys + k8sSecret := &corev1.Secret{} + err = fixture.K8sClient.Get(fixture.Ctx, + types.NamespacedName{Name: testutils.SynchronizedSecretName, Namespace: namespace}, + k8sSecret) + Expect(err).NotTo(HaveOccurred()) + + // Should have 2 entries with UUID keys + Expect(len(k8sSecret.Data)).To(Equal(2)) + }) + }) +}) From e87be142a64029a9525e17310808c4ee76e7b50e Mon Sep 17 00:00:00 2001 From: maxkpower Date: Mon, 24 Nov 2025 19:21:33 +0100 Subject: [PATCH 2/6] readme cleanup --- README.md | 8 +++---- .../controller/bitwardensecret_controller.go | 2 +- internal/controller/test/validation_test.go | 21 ------------------- 3 files changed, 5 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 452737a..11aab3c 100644 --- a/README.md +++ b/README.md @@ -94,13 +94,13 @@ Our operator is designed to look for the creation of a custom resource called a - **spec.organizationId**: The Bitwarden organization ID you are pulling Secrets Manager data from - **spec.secretName**: The name of the Kubernetes secret that will be created and injected with Secrets Manager data. - **spec.authToken**: The name of a secret inside of the Kubernetes namespace that the BitwardenSecrets object is being deployed into that contains the Secrets Manager machine account authorization token being used to access secrets. -- **spec.useSecretNames** (optional): When set to `true`, uses secret names from Bitwarden Secrets Manager as Kubernetes secret keys instead of UUIDs. Default: `false` (uses UUIDs for backward compatibility). +- **spec.useSecretNames** (optional): When set to `true`, uses secret names from Bitwarden Secrets Manager as Kubernetes secret keys instead of UUIDs. Default: `false`. #### Secret Key Naming -By default, secrets are created with the Secrets Manager secret UUID used as the key. This ensures backward compatibility and handles cases where secret names may not be unique or POSIX-compliant. +By default, secrets are created with the Secrets Manager secret UUID used as the key. -**Option 1: Use Secret Names (Recommended for new deployments)** +**Option 1: Use Secret Names** Set `useSecretNames: true` to use the secret names from Bitwarden as Kubernetes secret keys: @@ -113,7 +113,7 @@ When enabled: - Secret names from Bitwarden Secrets Manager will be used as Kubernetes secret keys - Secret names must be POSIX-compliant (start with letter or underscore, contain only alphanumeric and underscore characters) - Secret names must be unique across all accessible secrets -- Validation errors will prevent secret synchronization with detailed error messages +- Validation errors will prevent secret synchronization with error messages Example result: ```yaml diff --git a/internal/controller/bitwardensecret_controller.go b/internal/controller/bitwardensecret_controller.go index 21c4148..9f8a606 100644 --- a/internal/controller/bitwardensecret_controller.go +++ b/internal/controller/bitwardensecret_controller.go @@ -329,7 +329,7 @@ func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Lo smSecretVals := smSecretResponse.Secrets - // Legacy mode: Use UUIDs as keys (default behavior for backward compatibility) + // Legacy mode: Use UUIDs as keys if !useSecretNames { for _, smSecretVal := range smSecretVals { secrets[smSecretVal.ID] = []byte(smSecretVal.Value) diff --git a/internal/controller/test/validation_test.go b/internal/controller/test/validation_test.go index 4021c09..e488952 100644 --- a/internal/controller/test/validation_test.go +++ b/internal/controller/test/validation_test.go @@ -1,24 +1,3 @@ -/* -Source code in this repository is covered by one of two licenses: (i) the -GNU General Public License (GPL) v3.0 (ii) the Bitwarden License v1.0. The -default license throughout the repository is GPL v3.0 unless the header -specifies another license. Bitwarden Licensed code is found only in the -/bitwarden_license directory. - -GPL v3.0: -https://github.com/bitwarden/server/blob/main/LICENSE_GPL.txt - -Bitwarden License v1.0: -https://github.com/bitwarden/server/blob/main/LICENSE_BITWARDEN.txt - -No grant of any rights in the trademarks, service marks, or logos of Bitwarden is -made (except as may be necessary to comply with the notice requirements as -applicable), and use of any Bitwarden trademarks must comply with Bitwarden -Trademark Guidelines -. - -*/ - package controller_test import ( From cb1020400fe842f5682ceeb9e93e7639e4b206e0 Mon Sep 17 00:00:00 2001 From: maxkpower Date: Mon, 24 Nov 2025 19:30:04 +0100 Subject: [PATCH 3/6] fix tests --- internal/controller/test/validation_test.go | 40 +++++++++++++-------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/internal/controller/test/validation_test.go b/internal/controller/test/validation_test.go index e488952..425dca5 100644 --- a/internal/controller/test/validation_test.go +++ b/internal/controller/test/validation_test.go @@ -11,6 +11,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -238,22 +239,33 @@ var _ = Describe("Secret Name Validation Tests", Ordered, func() { _, err := fixture.CreateDefaultAuthSecret(namespace) Expect(err).NotTo(HaveOccurred()) - // Create BitwardenSecret with useSecretNames enabled - bwSecret, err := fixture.CreateBitwardenSecret( - testutils.BitwardenSecretName, - namespace, - fixture.OrgId, - testutils.SynchronizedSecretName, - testutils.AuthSecretName, - testutils.AuthSecretKey, - []operatorsv1.SecretMap{}, - false, - ) + // Create BitwardenSecret directly with useSecretNames enabled + bwSecret := &operatorsv1.BitwardenSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testutils.BitwardenSecretName, + Namespace: namespace, + }, + Spec: operatorsv1.BitwardenSecretSpec{ + AuthToken: operatorsv1.AuthToken{ + SecretName: testutils.AuthSecretName, + SecretKey: testutils.AuthSecretKey, + }, + SecretName: testutils.SynchronizedSecretName, + OrganizationId: fixture.OrgId, + SecretMap: []operatorsv1.SecretMap{}, + OnlyMappedSecrets: false, + UseSecretNames: true, // Enable name-based mode from the start + }, + } + err = fixture.K8sClient.Create(fixture.Ctx, bwSecret) Expect(err).NotTo(HaveOccurred()) - bwSecret.Spec.UseSecretNames = true - err = fixture.K8sClient.Update(fixture.Ctx, bwSecret) - Expect(err).NotTo(HaveOccurred()) + // Wait for BitwardenSecret to be created + Eventually(func(g Gomega) { + fetched := &operatorsv1.BitwardenSecret{} + err := fixture.K8sClient.Get(fixture.Ctx, types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}, fetched) + g.Expect(err).NotTo(HaveOccurred()) + }).WithTimeout(10 * time.Second).WithPolling(100 * time.Millisecond).Should(Succeed()) // Trigger reconciliation - should fail req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} From b1d5431fe682c9a721687b02d9ea777c8d0effe1 Mon Sep 17 00:00:00 2001 From: maxkpower Date: Mon, 24 Nov 2025 23:15:16 +0100 Subject: [PATCH 4/6] warn but succeed with non-POSIX secret names --- README.md | 10 +- api/v1/bitwardensecret_types.go | 4 +- .../controller/bitwardensecret_controller.go | 37 ++----- .../test/reconciler_edge_cases_test.go | 2 +- internal/controller/test/testutils/fixture.go | 2 +- internal/controller/test/validation_test.go | 97 ++++++++++++------- 6 files changed, 85 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 11aab3c..5b01c63 100644 --- a/README.md +++ b/README.md @@ -111,9 +111,13 @@ spec: When enabled: - Secret names from Bitwarden Secrets Manager will be used as Kubernetes secret keys -- Secret names must be POSIX-compliant (start with letter or underscore, contain only alphanumeric and underscore characters) -- Secret names must be unique across all accessible secrets -- Validation errors will prevent secret synchronization with error messages +- Secret names should be POSIX-compliant for best environment variable compatibility: + - Should start with a letter (`a-z`, `A-Z`) or underscore (`_`) + - Should contain only letters, digits (`0-9`), and underscores (`_`) + - Warnings will be logged for non-compliant names (e.g., names with dashes, dots, or starting with digits) +- Secret names **must be unique** across all accessible secrets (duplicates will cause sync failure) + +**Note:** While Kubernetes accepts various characters in Secret keys, the operator warns about non-POSIX-compliant names that may not work optimally as environment variables. The secrets will still sync, but you may encounter issues when using them in certain contexts. Example result: ```yaml diff --git a/api/v1/bitwardensecret_types.go b/api/v1/bitwardensecret_types.go index ff97188..f0004f3 100644 --- a/api/v1/bitwardensecret_types.go +++ b/api/v1/bitwardensecret_types.go @@ -53,8 +53,8 @@ type BitwardenSecretSpec struct { OnlyMappedSecrets bool `json:"onlyMappedSecrets"` // UseSecretNames, when true, uses the secret names from Bitwarden Secrets Manager as Kubernetes secret keys. // When false or unset (default), uses secret UUIDs as keys (preserving backward compatibility). - // When enabled, secret names must be POSIX-compliant (start with letter/underscore, contain only alphanumeric/underscore) - // and must be unique across all accessible secrets. Validation errors will prevent secret synchronization. + // When enabled, warnings are logged for non-POSIX-compliant names (e.g., containing dashes, dots, or starting with digits). + // Secret names must be unique across all accessible secrets - duplicates will cause synchronization failure. // Defaults to false. // +kubebuilder:validation:Optional // +kubebuilder:default=false diff --git a/internal/controller/bitwardensecret_controller.go b/internal/controller/bitwardensecret_controller.go index 9f8a606..bd1ad9b 100644 --- a/internal/controller/bitwardensecret_controller.go +++ b/internal/controller/bitwardensecret_controller.go @@ -340,20 +340,17 @@ func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Lo // New mode: Use secret names with validation and duplicate detection seenKeys := make(map[string][]string) // Track duplicates: key -> []secretIDs - var invalidKeys []string - // First pass: validate and detect duplicates + // First pass: validate POSIX compliance (warn) and detect duplicates (error) for _, smSecretVal := range smSecretVals { secretKey := smSecretVal.Key // Validate POSIX compliance if err := ValidateSecretKeyName(secretKey); err != nil { - logger.Error(err, "Invalid secret key name", + logger.Info("Secret name is not POSIX-compliant and may not work as an environment variable", "secretId", smSecretVal.ID, - "secretKey", secretKey) - invalidKeys = append(invalidKeys, - fmt.Sprintf("%s (ID: %s): %v", secretKey, smSecretVal.ID, err)) - continue + "secretKey", secretKey, + "warning", err.Error()) } // Track for duplicate detection @@ -369,27 +366,13 @@ func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Lo } } - // Fail if validation or duplicate errors found - if len(invalidKeys) > 0 || len(duplicates) > 0 { - var errMsg string - - if len(invalidKeys) > 0 { - errMsg += "Invalid secret key names found:\n" - for _, invalid := range invalidKeys { - errMsg += fmt.Sprintf(" - %s\n", invalid) - } - } - - if len(duplicates) > 0 { - if len(invalidKeys) > 0 { - errMsg += "\n" - } - errMsg += "Duplicate secret key names detected:\n" - for _, dup := range duplicates { - errMsg += fmt.Sprintf(" - %s\n", dup) - } - errMsg += "\nEach secret must have a unique name. Please rename the conflicting secrets in Secrets Manager." + // Fail if duplicates found + if len(duplicates) > 0 { + errMsg := "Duplicate secret key names detected:\n" + for _, dup := range duplicates { + errMsg += fmt.Sprintf(" - %s\n", dup) } + errMsg += "\nMultiple secrets with the same name. Use unique names for secrets or disable useSecretNames." defer bitwardenClient.Close() return false, nil, fmt.Errorf(errMsg) diff --git a/internal/controller/test/reconciler_edge_cases_test.go b/internal/controller/test/reconciler_edge_cases_test.go index 7125b97..9b57863 100644 --- a/internal/controller/test/reconciler_edge_cases_test.go +++ b/internal/controller/test/reconciler_edge_cases_test.go @@ -166,7 +166,7 @@ var _ = Describe("BitwardenSecret Reconciler - Edge Case Tests", Ordered, func() for i := range largeNumOfSecrets { identifier := sdk.SecretIdentifierResponse{ ID: uuid.NewString(), - Key: fmt.Sprintf("secret_%d", i), // Use realistic POSIX-compliant secret name + Key: fmt.Sprintf("secret_%d", i), // Use secret name OrganizationID: fixture.OrgId, } largeSecretMap = append(largeSecretMap, operatorsv1.SecretMap{ diff --git a/internal/controller/test/testutils/fixture.go b/internal/controller/test/testutils/fixture.go index ce4a4f4..32cc372 100644 --- a/internal/controller/test/testutils/fixture.go +++ b/internal/controller/test/testutils/fixture.go @@ -80,7 +80,7 @@ func (f *TestFixture) setup(t *testing.T, runner *EnvTestRunner) { for secretCount := range ExpectedNumOfSecrets { identifier := sdk.SecretIdentifierResponse{ ID: uuid.NewString(), - Key: fmt.Sprintf("secret_%d", secretCount), // Use realistic POSIX-compliant secret name + Key: fmt.Sprintf("secret_%d", secretCount), // Use secret name } //build a map mapping each Identifier to an human readable name based on index diff --git a/internal/controller/test/validation_test.go b/internal/controller/test/validation_test.go index 425dca5..bc25c3d 100644 --- a/internal/controller/test/validation_test.go +++ b/internal/controller/test/validation_test.go @@ -120,23 +120,33 @@ var _ = Describe("Secret Name Validation Tests", Ordered, func() { _, err := fixture.CreateDefaultAuthSecret(namespace) Expect(err).NotTo(HaveOccurred()) - // Create BitwardenSecret with useSecretNames enabled - bwSecret, err := fixture.CreateBitwardenSecret( - testutils.BitwardenSecretName, - namespace, - fixture.OrgId, - testutils.SynchronizedSecretName, - testutils.AuthSecretName, - testutils.AuthSecretKey, - []operatorsv1.SecretMap{}, // No mapping needed - false, // onlyMappedSecrets = false - ) + // Create BitwardenSecret directly with useSecretNames enabled + bwSecret := &operatorsv1.BitwardenSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testutils.BitwardenSecretName, + Namespace: namespace, + }, + Spec: operatorsv1.BitwardenSecretSpec{ + AuthToken: operatorsv1.AuthToken{ + SecretName: testutils.AuthSecretName, + SecretKey: testutils.AuthSecretKey, + }, + SecretName: testutils.SynchronizedSecretName, + OrganizationId: fixture.OrgId, + SecretMap: []operatorsv1.SecretMap{}, + OnlyMappedSecrets: false, + UseSecretNames: true, // Enable name-based mode from the start + }, + } + err = fixture.K8sClient.Create(fixture.Ctx, bwSecret) Expect(err).NotTo(HaveOccurred()) - // Enable useSecretNames - bwSecret.Spec.UseSecretNames = true - err = fixture.K8sClient.Update(fixture.Ctx, bwSecret) - Expect(err).NotTo(HaveOccurred()) + // Wait for BitwardenSecret to be created + Eventually(func(g Gomega) { + fetched := &operatorsv1.BitwardenSecret{} + err := fixture.K8sClient.Get(fixture.Ctx, types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}, fetched) + g.Expect(err).NotTo(HaveOccurred()) + }).WithTimeout(10 * time.Second).WithPolling(100 * time.Millisecond).Should(Succeed()) // Trigger reconciliation req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} @@ -157,7 +167,7 @@ var _ = Describe("Secret Name Validation Tests", Ordered, func() { Expect(string(k8sSecret.Data["API_KEY"])).To(Equal("api-key-value")) }) - It("should fail with invalid secret names", func() { + It("should warn but succeed with non-POSIX secret names", func() { // Create mock response with invalid secret names invalidSecretsData := []sdk.SecretResponse{ { @@ -185,28 +195,49 @@ var _ = Describe("Secret Name Validation Tests", Ordered, func() { _, err := fixture.CreateDefaultAuthSecret(namespace) Expect(err).NotTo(HaveOccurred()) - // Create BitwardenSecret with useSecretNames enabled - bwSecret, err := fixture.CreateBitwardenSecret( - testutils.BitwardenSecretName, - namespace, - fixture.OrgId, - testutils.SynchronizedSecretName, - testutils.AuthSecretName, - testutils.AuthSecretKey, - []operatorsv1.SecretMap{}, - false, - ) + // Create BitwardenSecret directly with useSecretNames enabled + bwSecret := &operatorsv1.BitwardenSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testutils.BitwardenSecretName, + Namespace: namespace, + }, + Spec: operatorsv1.BitwardenSecretSpec{ + AuthToken: operatorsv1.AuthToken{ + SecretName: testutils.AuthSecretName, + SecretKey: testutils.AuthSecretKey, + }, + SecretName: testutils.SynchronizedSecretName, + OrganizationId: fixture.OrgId, + SecretMap: []operatorsv1.SecretMap{}, + OnlyMappedSecrets: false, + UseSecretNames: true, // Enable name-based mode from the start + }, + } + err = fixture.K8sClient.Create(fixture.Ctx, bwSecret) Expect(err).NotTo(HaveOccurred()) - bwSecret.Spec.UseSecretNames = true - err = fixture.K8sClient.Update(fixture.Ctx, bwSecret) - Expect(err).NotTo(HaveOccurred()) + // Wait for BitwardenSecret to be created + Eventually(func(g Gomega) { + fetched := &operatorsv1.BitwardenSecret{} + err := fixture.K8sClient.Get(fixture.Ctx, types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}, fetched) + g.Expect(err).NotTo(HaveOccurred()) + }).WithTimeout(10 * time.Second).WithPolling(100 * time.Millisecond).Should(Succeed()) - // Trigger reconciliation - should fail + // Trigger reconciliation - should succeed with warnings req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} _, err = fixture.Reconciler.Reconcile(fixture.Ctx, req) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Invalid secret key names found")) + Expect(err).NotTo(HaveOccurred()) + + // Verify K8s secret was created with non-POSIX keys (warnings logged but sync succeeded) + k8sSecret := &corev1.Secret{} + err = fixture.K8sClient.Get(fixture.Ctx, + types.NamespacedName{Name: testutils.SynchronizedSecretName, Namespace: namespace}, + k8sSecret) + Expect(err).NotTo(HaveOccurred()) + + // Verify keys are present even though they're not POSIX-compliant + Expect(k8sSecret.Data).To(HaveKey("123-invalid")) + Expect(k8sSecret.Data).To(HaveKey("my-secret")) }) It("should fail with duplicate secret names", func() { From 12618f74a80554438a804ad71627c913f941e83b Mon Sep 17 00:00:00 2001 From: maxkpower Date: Mon, 8 Dec 2025 23:36:22 +0100 Subject: [PATCH 5/6] updated comments and tests --- config/samples/k8s_v1_bitwardensecret.yaml | 4 +- .../controller/bitwardensecret_controller.go | 6 +- .../test/reconciler_edge_cases_test.go | 84 +++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/config/samples/k8s_v1_bitwardensecret.yaml b/config/samples/k8s_v1_bitwardensecret.yaml index df7e60c..91ea969 100644 --- a/config/samples/k8s_v1_bitwardensecret.yaml +++ b/config/samples/k8s_v1_bitwardensecret.yaml @@ -13,7 +13,9 @@ spec: secretName: bw-sample-secret # Optional: Use secret names from Bitwarden as Kubernetes secret keys - # When enabled, secret names must be POSIX-compliant and unique + # When enabled, secret names must be unique. Secret names that are not + # POSIX-compliant will trigger a warning, but will be set on a best-effort + # basis. # Default: false (uses UUIDs as keys for backward compatibility) # useSecretNames: true diff --git a/internal/controller/bitwardensecret_controller.go b/internal/controller/bitwardensecret_controller.go index bd1ad9b..872c547 100644 --- a/internal/controller/bitwardensecret_controller.go +++ b/internal/controller/bitwardensecret_controller.go @@ -267,7 +267,7 @@ func (r *BitwardenSecretReconciler) LogCompletion(logger logr.Logger, ctx contex } // ValidateSecretKeyName validates that a secret key name is POSIX-compliant. -// POSIX-compliant names are required for environment variable compatibility: +// POSIX-compliant names are recommended for maximum compatibility: // - Must start with a letter (a-z, A-Z) or underscore (_) // - Can only contain letters, digits (0-9), and underscores // - Cannot be empty @@ -329,7 +329,7 @@ func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Lo smSecretVals := smSecretResponse.Secrets - // Legacy mode: Use UUIDs as keys + // Use UUIDs as keys if !useSecretNames { for _, smSecretVal := range smSecretVals { secrets[smSecretVal.ID] = []byte(smSecretVal.Value) @@ -338,7 +338,7 @@ func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Lo return smSecretResponse.HasChanges, secrets, nil } - // New mode: Use secret names with validation and duplicate detection + // Use secret names with validation and duplicate detection seenKeys := make(map[string][]string) // Track duplicates: key -> []secretIDs // First pass: validate POSIX compliance (warn) and detect duplicates (error) diff --git a/internal/controller/test/reconciler_edge_cases_test.go b/internal/controller/test/reconciler_edge_cases_test.go index 9b57863..35127fe 100644 --- a/internal/controller/test/reconciler_edge_cases_test.go +++ b/internal/controller/test/reconciler_edge_cases_test.go @@ -233,4 +233,88 @@ var _ = Describe("BitwardenSecret Reconciler - Edge Case Tests", Ordered, func() Expect(condition.Status).To(Equal(metav1.ConditionTrue)) Expect(updatedBwSecret.Status.LastSuccessfulSyncTime.Time).NotTo(BeZero()) }) + + It("should successfully sync using secret names (useSecretNames mode)", func() { + // Configure mocks with secret names instead of UUIDs + secretNamesData := []sdk.SecretResponse{} + for i := range 10 { + identifier := sdk.SecretIdentifierResponse{ + ID: uuid.NewString(), + Key: fmt.Sprintf("secret_%d", i), // Use secret names + OrganizationID: fixture.OrgId, + } + projectId := uuid.NewString() + secretNamesData = append(secretNamesData, sdk.SecretResponse{ + CreationDate: time.Now().String(), + ID: identifier.ID, + Key: identifier.Key, + Note: uuid.NewString(), + OrganizationID: fixture.OrgId, + ProjectID: &projectId, + RevisionDate: time.Now().String(), + Value: uuid.NewString(), + }) + } + secretNamesResponse := sdk.SecretsSyncResponse{ + HasChanges: true, + Secrets: secretNamesData, + } + + fixture.SetupDefaultCtrlMocks(false, &secretNamesResponse) + + _, err := fixture.CreateDefaultAuthSecret(namespace) + Expect(err).NotTo(HaveOccurred()) + + // Create BitwardenSecret with useSecretNames enabled and no SecretMap + bwSecret, err := fixture.CreateBitwardenSecret( + testutils.BitwardenSecretName, + namespace, + fixture.OrgId, + testutils.SynchronizedSecretName, + testutils.AuthSecretName, + testutils.AuthSecretKey, + []operatorsv1.SecretMap{}, // No SecretMap needed with useSecretNames + false, // OnlyMappedSecrets + ) + Expect(err).NotTo(HaveOccurred()) + Expect(bwSecret).NotTo(BeNil()) + + // Enable useSecretNames + bwSecret.Spec.UseSecretNames = true + err = fixture.K8sClient.Update(fixture.Ctx, bwSecret) + Expect(err).NotTo(HaveOccurred()) + + // Trigger reconciliation + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}} + result, err := fixture.Reconciler.Reconcile(fixture.Ctx, req) + + // Verify reconciliation succeeded + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(time.Duration(fixture.Reconciler.RefreshIntervalSeconds) * time.Second)) + + Eventually(func(g Gomega) { + // Verify created Kubernetes secret + createdTargetSecret := &corev1.Secret{} + g.Expect(fixture.K8sClient.Get(fixture.Ctx, types.NamespacedName{Name: testutils.SynchronizedSecretName, Namespace: namespace}, createdTargetSecret)).Should(Succeed()) + + // Check secret metadata and type + g.Expect(createdTargetSecret.Labels[controller.LabelBwSecret]).To(Equal(string(bwSecret.UID))) + g.Expect(createdTargetSecret.Type).To(Equal(corev1.SecretTypeOpaque)) + + // Verify all secrets are synced using their names as keys + g.Expect(len(createdTargetSecret.Data)).To(Equal(10)) + for i := range 10 { + expectedKey := fmt.Sprintf("secret_%d", i) + g.Expect(createdTargetSecret.Data).To(HaveKey(expectedKey)) + } + + // Verify BitwardenSecret status + updatedBwSecret := &operatorsv1.BitwardenSecret{} + g.Expect(fixture.K8sClient.Get(fixture.Ctx, types.NamespacedName{Name: testutils.BitwardenSecretName, Namespace: namespace}, updatedBwSecret)).Should(Succeed()) + condition := apimeta.FindStatusCondition(updatedBwSecret.Status.Conditions, "SuccessfulSync") + g.Expect(condition).NotTo(BeNil()) + g.Expect(condition.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(updatedBwSecret.Status.LastSuccessfulSyncTime.Time).NotTo(BeZero()) + }) + }) }) From 26d67cbd90b6b3698f6b03f1470e8dd58938db13 Mon Sep 17 00:00:00 2001 From: maxkpower Date: Wed, 10 Dec 2025 18:58:08 +0100 Subject: [PATCH 6/6] Revert test change --- internal/controller/test/reconciler_edge_cases_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/test/reconciler_edge_cases_test.go b/internal/controller/test/reconciler_edge_cases_test.go index 35127fe..c4cb999 100644 --- a/internal/controller/test/reconciler_edge_cases_test.go +++ b/internal/controller/test/reconciler_edge_cases_test.go @@ -166,7 +166,7 @@ var _ = Describe("BitwardenSecret Reconciler - Edge Case Tests", Ordered, func() for i := range largeNumOfSecrets { identifier := sdk.SecretIdentifierResponse{ ID: uuid.NewString(), - Key: fmt.Sprintf("secret_%d", i), // Use secret name + Key: uuid.NewString(), OrganizationID: fixture.OrgId, } largeSecretMap = append(largeSecretMap, operatorsv1.SecretMap{