Add support for labels in managed secrets

[tommatime]

This adds the ability to specify labels to be synced to the managed secret. There is currently no way to ensure a label is added to the managed secret which is required, for example, to set the secret-type for ArgoCD resources. If omitted, the existing "secrets.doppler.com/subtype": "dopplerSecret" label is applied to the managed Secret. If labels are included, the existing subtype label will be appended to the user-supplied labels to ensure it is not overwritten.

Syntax:

apiVersion: secrets.doppler.com/v1alpha1
kind: DopplerSecret
metadata:
  name: dopplersecret-test
  namespace: doppler-operator-system
spec:
  tokenSecret:
    name: doppler-token-secret
  managedSecret:
    name: doppler-test-secret
    namespace: default
    labels:
      doppler-secret-label: test

Fixes #64

[tommatime]

This is my first contribution to this project (and my first OSS contribution in general 🥳 ), so please let me know if any additional updates or documentation changes are necessary.

A comment on the original issue also requests support for annotations on the managed secret. This should be very similar to labels, so let me know if this functionality is desired and I can add that to this PR as well.

Thanks!

[nmanoogian]

Thanks so much for your submission, @tommatime! And congrats on your first OSS contribution! 🎉

We'll take a look at this PR and get back to you with feedback shortly.

[nmanoogian]

Hey @tommatime,

I'm so sorry for letting this sit as long as it has! We aim to get contributions like this turned around a lot faster but the review slipped through the cracks.

Your changes look great! I have a few recommendations and during my testing, I incorporated a few of them in the patch below.

If you're up for it, I think that annotation support would also be excellent to add at the same time. I'd completely understand if you'd like to stick with just labels for this PR though.

If you haven't used these before you can paste the patch directly into git apply (or use pbpaste | git apply if you're on macOS). Feel free to make modifications and commit (or rebase) the changes as you see fit. Thanks again and sorry for the delay!

diff --git a/api/v1alpha1/dopplersecret_types.go b/api/v1alpha1/dopplersecret_types.go
index a373018..3243e49 100644
--- a/api/v1alpha1/dopplersecret_types.go
+++ b/api/v1alpha1/dopplersecret_types.go
@@ -50,6 +50,8 @@ type ManagedSecretReference struct {
 	// +optional
 	Type string `json:"type,omitempty"`
 
+	// Labels to add or update on the managed secret
+	// +optional
 	Labels map[string]string `json:"labels,omitempty"`
 }
 
diff --git a/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml b/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml
index 1e2bc5c..7bc2952 100644
--- a/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml
+++ b/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml
@@ -59,6 +59,7 @@ spec:
                   labels:
                     additionalProperties:
                       type: string
+                    description: Labels to add or update on the managed secret
                     type: object
                   name:
                     description: The name of the Secret resource
diff --git a/controllers/dopplersecret_controller_secrets.go b/controllers/dopplersecret_controller_secrets.go
index 536d90c..d8dac2e 100644
--- a/controllers/dopplersecret_controller_secrets.go
+++ b/controllers/dopplersecret_controller_secrets.go
@@ -148,7 +148,7 @@ func GetKubeSecretAnnotations(secretsResult models.SecretsResult, processorsVers
 	return annotations
 }
 
-// GetKubeSecretAnnotations generates Kube labels from a Doppler API secrets result
+// GetKubeSecretLabels generates Kube labels from the provided managed secret spec
 func GetKubeSecretLabels(additionalLabels map[string]string) map[string]string {
 	labels := map[string]string{}
 
@@ -268,37 +268,38 @@ func (r *DopplerSecretReconciler) UpdateSecret(ctx context.Context, dopplerSecre
 	// Secret processors
 	processorsVersion := ""
 	formatVersion := ""
+	existingLabels := map[string]string{}
 	if existingKubeSecret != nil {
 		secretVersion = existingKubeSecret.Annotations[kubeSecretVersionAnnotation]
 		processorsVersion = existingKubeSecret.Annotations[kubeSecretProcessorsVersionAnnotation]
 		formatVersion = existingKubeSecret.Annotations[kubeSecretFormatVersionAnnotation]
+		existingLabels = existingKubeSecret.Labels
 	}
 
-	processorsVersionChanged := currentProcessorsVersion != processorsVersion
-	formatVersionChanged := dopplerSecret.Spec.Format != formatVersion
-	requestedSecretVersion := secretVersion
+	changes := []string{}
 
-	// - Processors transform secret values so if they've changed, we need to re-fetch the secrets so they can be re-processed.
-	// - The format is computed by the API and it defaults to "json". However, the operator uses the presence of the `format` field
-	//   to determine whether or not to process the JSON as separate k/v pairs or save the whole payload into a single DOPPLER_SECRETS_FILE secret.
-	//   If the format changed, we need to re-fetch secrets so we can redetermine this.
+	// Processors transform secret values so if they've changed, we need to re-fetch the secrets so they can be re-processed.
+	if currentProcessorsVersion != processorsVersion {
+		changes = append(changes, "processors")
+	}
 
-	// If either have changed, set requestedSecretVersion to an empty secret version to reload the secrets.
-	if processorsVersionChanged || formatVersionChanged {
-		log.Info("[/] Processor or format version changed, reloading secrets.", "processorsChanged", processorsVersionChanged, "formatChanged", formatVersionChanged)
-		requestedSecretVersion = ""
+	// The format is computed by the API and it defaults to "json". However, the operator uses the presence of the `format` field
+	// to determine whether or not to process the JSON as separate k/v pairs or save the whole payload into a single DOPPLER_SECRETS_FILE secret.
+	// If the format changed, we need to re-fetch secrets so we can redetermine this.
+	if dopplerSecret.Spec.Format != formatVersion {
+		changes = append(changes, "format")
 	}
 
-	if existingKubeSecret != nil {
-		// Compare existing managed secret labels to labels defined in the doppler secret.
-		// If they are not equal, we will update the managed secret with the new labels.
-		if reflect.DeepEqual(existingKubeSecret.Labels, GetKubeSecretLabels(dopplerSecret.Spec.ManagedSecretRef.Labels)) {
-			log.Info("[-] Managed secret labels not modified.")
-		} else {
-			// If labels have changed, set requestedSecretVersion to an empty secret version to reload the secrets.
-			log.Info("[/] Labels changed, reloading secrets.")
-			requestedSecretVersion = ""
-		}
+	// If the labels have been changed, we don't technically need to reload the secrets but it's simpler to do.
+	if !reflect.DeepEqual(existingLabels, GetKubeSecretLabels(dopplerSecret.Spec.ManagedSecretRef.Labels)) {
+		changes = append(changes, "labels")
+	}
+
+	// If any relevant attributes have been changed, set requestedSecretVersion to an empty secret version to reload the secrets.
+	requestedSecretVersion := secretVersion
+	if len(changes) > 0 {
+		log.Info("[/] Attributes have changed, reloading secrets.", "changes", changes)
+		requestedSecretVersion = ""
 	}
 
 	secretsResult, apiErr := api.GetSecrets(GetAPIContext(dopplerSecret, dopplerToken), requestedSecretVersion, dopplerSecret.Spec.Project, dopplerSecret.Spec.Config, dopplerSecret.Spec.NameTransformer, dopplerSecret.Spec.Format, dopplerSecret.Spec.Secrets)
@@ -310,7 +311,7 @@ func (r *DopplerSecretReconciler) UpdateSecret(ctx context.Context, dopplerSecre
 		return nil
 	}
 
-	log.Info("[/] Secrets have been modified", "oldVersion", secretVersion, "newVersion", secretsResult.ETag, "processorsChanged", processorsVersionChanged)
+	log.Info("[/] Secrets have been modified", "oldVersion", secretVersion, "newVersion", secretsResult.ETag, "changes", changes)
 
 	if existingKubeSecret == nil {
 		return r.CreateManagedSecret(ctx, dopplerSecret, *secretsResult)

[nmanoogian]

nit/copypasta: Can we update this comment to match the function?

[nmanoogian]

Can we add a docs comment here as well as the +optional annotation?

[nmanoogian]

I think this might be a good opportunity to refactor the UpdateSecret logic. Let me know what you think of the attached patch.