diff --git a/README.md b/README.md index 731f661..5b01c63 100644 --- a/README.md +++ b/README.md @@ -94,12 +94,56 @@ 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`. -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. + +**Option 1: Use Secret Names** + +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 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 +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..f0004f3 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, 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 + 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..91ea969 100644 --- a/config/samples/k8s_v1_bitwardensecret.yaml +++ b/config/samples/k8s_v1_bitwardensecret.yaml @@ -11,6 +11,14 @@ 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 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 + # map: [] map: - bwSecretId: e30f88bd-9e9c-42ae-83b7-b155012da672 diff --git a/internal/controller/bitwardensecret_controller.go b/internal/controller/bitwardensecret_controller.go index d287cdc..872c547 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 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 +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,61 @@ func (r *BitwardenSecretReconciler) PullSecretManagerSecretDeltas(logger logr.Lo smSecretVals := smSecretResponse.Secrets + // Use UUIDs as keys + if !useSecretNames { + for _, smSecretVal := range smSecretVals { + secrets[smSecretVal.ID] = []byte(smSecretVal.Value) + } + defer bitwardenClient.Close() + return smSecretResponse.HasChanges, secrets, nil + } + + // 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) for _, smSecretVal := range smSecretVals { - secrets[smSecretVal.ID] = []byte(smSecretVal.Value) + secretKey := smSecretVal.Key + + // Validate POSIX compliance + if err := ValidateSecretKeyName(secretKey); err != nil { + logger.Info("Secret name is not POSIX-compliant and may not work as an environment variable", + "secretId", smSecretVal.ID, + "secretKey", secretKey, + "warning", err.Error()) + } + + // 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 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) + } + + // 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..c4cb999 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()) + }) + }) }) diff --git a/internal/controller/test/testutils/fixture.go b/internal/controller/test/testutils/fixture.go index a1cf5ab..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: uuid.NewString(), + 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 new file mode 100644 index 0000000..bc25c3d --- /dev/null +++ b/internal/controller/test/validation_test.go @@ -0,0 +1,397 @@ +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" + metav1 "k8s.io/apimachinery/pkg/apis/meta/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 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()) + + // 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}} + _, 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 warn but succeed with non-POSIX 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 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()) + + // 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 succeed with warnings + 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 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() { + // 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 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()) + + // 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}} + _, 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)) + }) + }) +})