added secret resolver for handling annotations directly on secrets#30
added secret resolver for handling annotations directly on secrets#30danieldonoghue wants to merge 2 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for handling annotations directly on Secret resources, expanding the operator's capability beyond deployment-based synchronization. The changes enable secrets to be synced to Vault using annotations on the Secret objects themselves.
- Adds a new
SecretReconcilercontroller to handle Secret resources with vault-sync annotations - Extracts common synchronization logic into shared functions to avoid code duplication
- Updates metric labels from "deployment" to "resource" to accommodate both deployment and secret sync operations
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/metrics/metrics.go | Updated metric labels from "deployment" to "resource" to support both deployment and secret sync |
| internal/controller/sync_common.go | New shared sync functionality for both deployment and secret controllers |
| internal/controller/secret_controller_test.go | Comprehensive test suite for the SecretReconciler |
| internal/controller/secret_controller.go | New SecretReconciler implementation for handling Secret resources |
| internal/controller/deployment_controller.go | Added comment clarifying this file implements DeploymentReconciler |
| examples/secret-level-sync-example.yaml | Example configurations for direct secret synchronization |
| examples/README.md | Updated documentation to include secret-level sync examples |
| docs/PROJECT_SUMMARY.md | Updated project documentation to reflect dual sync modes |
| config/rbac/role.yaml | Updated RBAC permissions to support secret management |
| cmd/main.go | Registered the new SecretReconciler controller |
| README.md | Enhanced documentation with secret sync usage examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Type string // "deployment" or "secret" | ||
| } | ||
|
|
||
| // Note: SecretConfig is defined in deployment_controller.go to avoid duplication |
There was a problem hiding this comment.
The comment references SecretConfig being defined in deployment_controller.go, but this creates a dependency that should be documented or the type should be defined in this shared file instead.
| // Note: SecretConfig is defined in deployment_controller.go to avoid duplication | |
| // SecretConfig defines the structure for custom secret configuration. | |
| type SecretConfig struct { | |
| Name string `json:"name"` | |
| Keys []string `json:"keys"` | |
| Prefix string `json:"prefix,omitempty"` | |
| } |
| return changed | ||
| } | ||
|
|
||
| // Note: getSecretKeys is defined in deployment_controller.go to avoid duplication |
There was a problem hiding this comment.
This comment indicates getSecretKeys is defined elsewhere to avoid duplication, but since this is shared functionality, it should be defined in this common file or the dependency should be clearly documented.
| // Note: getSecretKeys is defined in deployment_controller.go to avoid duplication | |
| // getSecretKeys returns the keys of the provided Kubernetes Secret. | |
| func getSecretKeys(secret *corev1.Secret) []string { | |
| keys := make([]string, 0, len(secret.Data)) | |
| for k := range secret.Data { | |
| keys = append(keys, k) | |
| } | |
| return keys | |
| } |
|
|
||
| // getLastKnownSecretVersions retrieves the last known secret versions from secret annotations. | ||
| func (r *SecretReconciler) getLastKnownSecretVersions(secret *corev1.Secret) map[string]string { | ||
| versionsAnnotation, exists := secret.Annotations[VaultSecretVersionsAnnotation] |
There was a problem hiding this comment.
The constant VaultSecretVersionsAnnotation is used but not defined in this file. It should be imported from a constants file or defined locally to improve maintainability.
| } | ||
|
|
||
| // Check if vault-sync is enabled for this secret (presence of vault path annotation) | ||
| vaultPath, vaultSyncEnabled := secret.Annotations[VaultPathAnnotation] |
There was a problem hiding this comment.
Multiple annotation constants (VaultPathAnnotation, VaultSyncFinalizer, etc.) are used throughout the file but not defined here. These should be imported from a constants file or defined locally to improve code maintainability and reduce coupling.
| vaultPath, vaultSyncEnabled := secret.Annotations[VaultPathAnnotation] | ||
| if !vaultSyncEnabled || vaultPath == "" { | ||
| // Remove finalizer if it exists but sync is disabled | ||
| if controllerutil.ContainsFinalizer(secret, VaultSyncFinalizer) { |
There was a problem hiding this comment.
Multiple annotation constants (VaultPathAnnotation, VaultSyncFinalizer, etc.) are used throughout the file but not defined here. These should be imported from a constants file or defined locally to improve code maintainability and reduce coupling.
| preserveOnDelete := secret.Annotations[VaultPreserveOnDeleteAnnotation] == "true" | ||
|
|
||
| // Get the vault path | ||
| vaultPath, exists := secret.Annotations[VaultPathAnnotation] |
There was a problem hiding this comment.
Multiple annotation constants (VaultPathAnnotation, VaultSyncFinalizer, etc.) are used throughout the file but not defined here. These should be imported from a constants file or defined locally to improve code maintainability and reduce coupling.
| } | ||
|
|
||
| // Check if custom secrets configuration is provided | ||
| secretsToSync, hasCustomConfig := secret.Annotations[VaultSecretsAnnotation] |
There was a problem hiding this comment.
Multiple annotation constants (VaultPathAnnotation, VaultSyncFinalizer, etc.) are used throughout the file but not defined here. These should be imported from a constants file or defined locally to improve code maintainability and reduce coupling.
|
|
||
| // getLastKnownSecretVersions retrieves the last known secret versions from secret annotations. | ||
| func (r *SecretReconciler) getLastKnownSecretVersions(secret *corev1.Secret) map[string]string { | ||
| versionsAnnotation, exists := secret.Annotations[VaultSecretVersionsAnnotation] |
There was a problem hiding this comment.
Multiple annotation constants (VaultPathAnnotation, VaultSyncFinalizer, etc.) are used throughout the file but not defined here. These should be imported from a constants file or defined locally to improve code maintainability and reduce coupling.
|
|
||
| // isRotationCheckDisabled checks if secret rotation detection is disabled for this secret. | ||
| func (r *SecretReconciler) isRotationCheckDisabled(secret *corev1.Secret) bool { | ||
| rotationCheck, exists := secret.Annotations[VaultRotationCheckAnnotation] |
There was a problem hiding this comment.
Multiple annotation constants (VaultPathAnnotation, VaultSyncFinalizer, etc.) are used throughout the file but not defined here. These should be imported from a constants file or defined locally to improve code maintainability and reduce coupling.
| // getReconcileInterval parses the reconciliation interval from the vault-sync.io/reconcile annotation. | ||
| // Returns the duration if valid, or zero duration if disabled or invalid. | ||
| func (r *SecretReconciler) getReconcileInterval(secret *corev1.Secret) time.Duration { | ||
| reconcileValue, exists := secret.Annotations[VaultReconcileAnnotation] |
There was a problem hiding this comment.
Multiple annotation constants (VaultPathAnnotation, VaultSyncFinalizer, etc.) are used throughout the file but not defined here. These should be imported from a constants file or defined locally to improve code maintainability and reduce coupling.
No description provided.