support using secret names instead of UUID #121
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
+ Coverage 62.40% 63.94% +1.53%
==========================================
Files 11 11
Lines 963 1004 +41
==========================================
+ Hits 601 642 +41
Misses 343 343
Partials 19 19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
maxkpower
changed the title
| and must be unique across all accessible secrets. Validation errors will prevent secret synchronization. | ||
| Defaults to false. |
There was a problem hiding this comment.
❓ What validation is this referring to, I see the POSIX validation here, https://github.com/bitwarden/sm-kubernetes/pull/121/files#diff-66a708935ecfb1c2ce30bb5554633406245e96c20bc00bd85e0c8ccbaa0068deR348-R355, but that won't prevent secret sync (just logs), as this description is saying.
|
|
||
| # 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) |
There was a problem hiding this comment.
Now that we've settled on merely warning for non-POSIX secret names, rather than failing, we should update this doc string, as well as the others that @mimartin12 pointed out. Maybe something like:
| # 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) | |
| # 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) |
| } | ||
|
|
||
| // ValidateSecretKeyName validates that a secret key name is POSIX-compliant. | ||
| // POSIX-compliant names are required for environment variable compatibility: |
There was a problem hiding this comment.
| // POSIX-compliant names are required for environment variable compatibility: | |
| // POSIX-compliant names are recommended for maximum compatibility: |
| return smSecretResponse.HasChanges, secrets, nil | ||
| } | ||
|
|
||
| // New mode: Use secret names with validation and duplicate detection |
There was a problem hiding this comment.
| // New mode: Use secret names with validation and duplicate detection | |
| // Use secret names with validation and duplicate detection |
|
|
||
| smSecretVals := smSecretResponse.Secrets | ||
|
|
||
| // Legacy mode: Use UUIDs as keys |
There was a problem hiding this comment.
| // Legacy mode: Use UUIDs as keys | |
| // Use UUIDs as keys |
| identifier := sdk.SecretIdentifierResponse{ | ||
| ID: uuid.NewString(), | ||
| Key: uuid.NewString(), | ||
| Key: fmt.Sprintf("secret_%d", i), // Use secret name |
There was a problem hiding this comment.
Could we make a new test for the useSecretName functionality and add that, rather than replacing this one?
| fixture.Teardown() | ||
| }) | ||
|
|
||
| Describe("ValidateSecretKeyName", func() { |
There was a problem hiding this comment.
Question: should these tests be adjusted now that we've decided to simply warn for non-compliant secret names? Do they still pass?
🎟️ Tracking
https://bitwarden.atlassian.net/browse/SM-1768
📔 Objective
Adds an optional useSecretNames field to the BitwardenSecret CRD that allows using secret names from Bitwarden Secrets Manager as Kubernetes secret keys instead of UUIDs.
Changes
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes