DopplerHQ / kubernetes-operator

Apache License 2.0
44 stars 18 forks source link

Configurable type support and tls-var transform work-around #41

Closed emmeowzing closed 1 year ago

emmeowzing commented 1 year ago

This probably shouldn't get merged, but it includes support for any secret types with an added work-around for TLS-type vars / string manipulation since the API doesn't support this yet.

https://docs.doppler.com/docs/accessing-secrets#name-transformers

emmeowzing commented 1 year ago

25

emmeowzing commented 1 year ago

Semi-related to this work is https://github.com/DopplerHQ/helm-charts/issues/12

emmeowzing commented 1 year ago

This seems to work just fine ~

$ k get secret https-cert -n argocd
NAME         TYPE                DATA   AGE
https-cert   kubernetes.io/tls   5      12m
emmeowzing commented 1 year ago
$ kgs
NAME                         TYPE                             DATA   AGE
cronitor                     Opaque                           4      20m
nexus-docker-registry        kubernetes.io/dockerconfigjson   4      105m

seems to work well for kubernetes.io/dockerconfigjson-type secrets as well with the same transform and adding env vars _DOCKERCONFIGJSON to Doppler's UI. Not sure about the other types or if they'd need additional transforms as I hardly use those types.

emmeowzing commented 1 year ago

One shortcoming I've sighted so far of this approach is that, if the secret type changes the controller doesn't delete and re-create secrets (pretty sure secret types can't be patched). So it just errors and fails to update the secret in the respective namespaces.

nmanoogian commented 1 year ago

Hey @bjd2385, thank you for contributing! We're really excited to see community engagement and interest in improving the operator.

I really like that you've added a top-level type field; I think that's definitely the most intuitive way to update the DopplerSecret interface to support this. However, I'm wondering if name transformers are the best way to map Doppler secrets to the fields for non-opaque secret types. We have the processors construct which today allows users to load Base64 secret data from Doppler as bytes into Kubernetes secrets. We've had the idea before of allowing processors to also do custom name transformations/mappings and that might work well here. This might also fit more closely with how we'd anticipate teams will want to organize their secrets. I'd imagine that a team might want to keep their TLS cert, SSH secrets, or other k8s-native creds alongside other secrets and create two or more DopplerSecret resources (pointing to the same Doppler config) to sync most secrets to one opaque k8s secret and the sync others to their respectively-typed k8s secrets.

For backward compatibility, I'd imagine that if the secret being sync'd is opaque, we'd default to using the plain processor but for non-opaque secrets, we'd omit secrets that were not explicitly defined in processors.

I think something like this could work:

diff --git a/api/v1alpha1/dopplersecret_types.go b/api/v1alpha1/dopplersecret_types.go
index 1e64a16..b26e1fc 100644
--- a/api/v1alpha1/dopplersecret_types.go
+++ b/api/v1alpha1/dopplersecret_types.go
@@ -37,8 +37,10 @@ type SecretReference struct {

 type SecretProcessor struct {
    // The type of process to be performed, either "plain" or "base64"
-   // +kubebuilder:validation:Enum=plain;base64
    Type string `json:"type"`
+
+   // The mapped name of the field in the managed secret, defaults to the original Doppler secret name.
+   AsName string `json:"asName,omitempty"`
 }

 type SecretProcessors map[string]*SecretProcessor
diff --git a/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml b/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml
index 6e95425..9643bca 100644
--- a/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml
+++ b/config/crd/bases/secrets.doppler.com_dopplersecrets.yaml
@@ -80,12 +80,13 @@ spec:
               processors:
                 additionalProperties:
                   properties:
+                    asName:
+                      description: The mapped name of the field in the managed secret,
+                        defaults to the original Doppler secret name.
+                      type: string
                     type:
                       description: The type of process to be performed, either "plain"
                         or "base64"
-                      enum:
-                      - plain
-                      - base64
                       type: string
                   required:
                   - type
@@ -116,8 +117,8 @@ spec:
                 type: object
               type:
                 default: Opaque
-                description: The Doppler secret type transferred to the managed secret
-                  type
+                description: The Doppler secret type is transferred to the managed
+                  secret type
                 enum:
                 - Opaque
                 - kubernetes.io/tls
diff --git a/controllers/dopplersecret_controller_secrets.go b/controllers/dopplersecret_controller_secrets.go
index 5901a99..136d2c6 100644
--- a/controllers/dopplersecret_controller_secrets.go
+++ b/controllers/dopplersecret_controller_secrets.go
@@ -95,7 +95,7 @@ func (r *DopplerSecretReconciler) GetDopplerToken(ctx context.Context, dopplerSe
 }

 // GetKubeSecretData generates Kube secret data from a Doppler API secrets result
-func GetKubeSecretData(secretsResult models.SecretsResult, processors secretsv1alpha1.SecretProcessors) (map[string][]byte, error) {
+func GetKubeSecretData(secretsResult models.SecretsResult, processors secretsv1alpha1.SecretProcessors, useDefaultProcessor bool) (map[string][]byte, error) {
    kubeSecretData := map[string][]byte{}
    for _, secret := range secretsResult.Secrets {
        secretName := secret.Name
@@ -103,8 +103,14 @@ func GetKubeSecretData(secretsResult models.SecretsResult, processors secretsv1a
        // Processors
        processor := processors[secret.Name]
        if processor == nil {
+           if !useDefaultProcessor {
+               continue
+           }
            processor = &secretsv1alpha1.DefaultProcessor
        }
+       if processor.AsName != "" {
+           secretName = processor.AsName
+       }
        processorFunc := procs.All[processor.Type]
        if processorFunc == nil {
            return nil, fmt.Errorf("Failed to process data with unknown processor: %v", processor.Type)
@@ -151,7 +157,8 @@ func GetProcessorsVersion(processors secretsv1alpha1.SecretProcessors) (string,

 // CreateManagedSecret creates a managed Kubernetes secret
 func (r *DopplerSecretReconciler) CreateManagedSecret(ctx context.Context, dopplerSecret secretsv1alpha1.DopplerSecret, secretsResult models.SecretsResult) error {
-   secretData, dataErr := GetKubeSecretData(secretsResult, dopplerSecret.Spec.Processors)
+   useDefaultProcessor := dopplerSecret.Spec.SecretType == string(corev1.SecretTypeOpaque)
+   secretData, dataErr := GetKubeSecretData(secretsResult, dopplerSecret.Spec.Processors, useDefaultProcessor)
    if dataErr != nil {
        return fmt.Errorf("Failed to build Kubernetes secret data: %w", dataErr)
    }
@@ -181,7 +188,8 @@ func (r *DopplerSecretReconciler) CreateManagedSecret(ctx context.Context, doppl

 // UpdateManagedSecret updates a managed Kubernetes secret
 func (r *DopplerSecretReconciler) UpdateManagedSecret(ctx context.Context, secret corev1.Secret, dopplerSecret secretsv1alpha1.DopplerSecret, secretsResult models.SecretsResult) error {
-   secretData, dataErr := GetKubeSecretData(secretsResult, dopplerSecret.Spec.Processors)
+   useDefaultProcessor := dopplerSecret.Spec.SecretType == string(corev1.SecretTypeOpaque)
+   secretData, dataErr := GetKubeSecretData(secretsResult, dopplerSecret.Spec.Processors, useDefaultProcessor)
    if dataErr != nil {
        return fmt.Errorf("Failed to build Kubernetes secret data: %w", dataErr)
    }

(If you want to play around with that, you should be able to paste the snippet directly into git apply).

Thanks again for putting the work in to move this feature forward! We really appreciate it!

nmanoogian commented 1 year ago

@bjd2385, thanks again for submitting this! I've just opened #47 to add this behavior to the operator. If you get a chance, I'd love your feedback to make sure that the new behavior covers your use cases.

emmeowzing commented 1 year ago

I'll give it a try tomorrow night!

emmeowzing commented 1 year ago

47