From 2599999b84a0abb16d90b863ef667af772692ef9 Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Thu, 26 May 2022 19:59:21 -0700 Subject: [PATCH 01/12] json path work in progress --- apis/v1/secretproviderclass_types.go | 20 +++ apis/v1/zz_generated.deepcopy.go | 36 +++++ ...re.csi.x-k8s.io_secretproviderclasses.yaml | 26 ++++ ...secretproviderclasspodstatus_controller.go | 9 +- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 26 ++++ ...re.csi.x-k8s.io_secretproviderclasses.yaml | 26 ++++ pkg/rotation/reconciler.go | 9 +- pkg/util/secretutil/secret.go | 123 ++++++++++++++++-- 8 files changed, 265 insertions(+), 10 deletions(-) diff --git a/apis/v1/secretproviderclass_types.go b/apis/v1/secretproviderclass_types.go index 4645c8b0a..36238644c 100644 --- a/apis/v1/secretproviderclass_types.go +++ b/apis/v1/secretproviderclass_types.go @@ -46,6 +46,25 @@ type SecretObject struct { Data []*SecretObjectData `json:"data,omitempty"` } +type SyncOptions struct { + // type of K8s secret object + Type string `json:"type,omitempty"` + // the format of the secret (plaintext|json|yaml) + Format string `json:"format,omitempty"` + // the nested object to target for syncing + JsonPath string `json:"jsonPath,omitempty"` + Secrets []Secret `json:"secrets,omitempty"` +} + +type Secret struct { + // name of the K8s secret object + SecretName string `json:"secretName,omitempty"` + // the format of the secret (plaintext|json) + Format string `json:"format,omitempty"` + // the nested object to target for syncing + JsonPath string `json:"jsonPath,omitempty"` +} + // SecretProviderClassSpec defines the desired state of SecretProviderClass type SecretProviderClassSpec struct { // Configuration for provider name @@ -53,6 +72,7 @@ type SecretProviderClassSpec struct { // Configuration for specific provider Parameters map[string]string `json:"parameters,omitempty"` SecretObjects []*SecretObject `json:"secretObjects,omitempty"` + SyncOptions SyncOptions `json:"syncOptions,omitempty"` } // ByPodStatus defines the state of SecretProviderClass as seen by diff --git a/apis/v1/zz_generated.deepcopy.go b/apis/v1/zz_generated.deepcopy.go index d4a1d9715..c822fae92 100644 --- a/apis/v1/zz_generated.deepcopy.go +++ b/apis/v1/zz_generated.deepcopy.go @@ -40,6 +40,21 @@ func (in *ByPodStatus) DeepCopy() *ByPodStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Secret) DeepCopyInto(out *Secret) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Secret. +func (in *Secret) DeepCopy() *Secret { + if in == nil { + return nil + } + out := new(Secret) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecretObject) DeepCopyInto(out *SecretObject) { *out = *in @@ -268,6 +283,7 @@ func (in *SecretProviderClassSpec) DeepCopyInto(out *SecretProviderClassSpec) { } } } + in.SyncOptions.DeepCopyInto(&out.SyncOptions) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretProviderClassSpec. @@ -305,3 +321,23 @@ func (in *SecretProviderClassStatus) DeepCopy() *SecretProviderClassStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SyncOptions) DeepCopyInto(out *SyncOptions) { + *out = *in + if in.Secrets != nil { + in, out := &in.Secrets, &out.Secrets + *out = make([]Secret, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SyncOptions. +func (in *SyncOptions) DeepCopy() *SyncOptions { + if in == nil { + return nil + } + out := new(SyncOptions) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 80fa563a1..b3a037fb5 100644 --- a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,6 +80,32 @@ spec: type: string type: object type: array + syncOptions: + properties: + format: + description: the format of the secret (plaintext|json|yaml) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secrets: + items: + properties: + format: + description: the format of the secret (plaintext|json) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secretName: + description: name of the K8s secret object + type: string + type: object + type: array + type: + description: type of K8s secret object + type: string + type: object type: object status: description: SecretProviderClassStatus defines the observed state of SecretProviderClass diff --git a/controllers/secretproviderclasspodstatus_controller.go b/controllers/secretproviderclasspodstatus_controller.go index c9302daf1..fdc0f985c 100644 --- a/controllers/secretproviderclasspodstatus_controller.go +++ b/controllers/secretproviderclasspodstatus_controller.go @@ -287,6 +287,13 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context, errs := make([]error, 0) for _, secretObj := range spc.Spec.SecretObjects { secretName := strings.TrimSpace(secretObj.SecretName) + jsonPath := secretutil.GetJsonPath(secretName, spc.Spec.SyncOptions) + secretFormat, err := secretutil.GetSecretFormat(secretName, spc.Spec.SyncOptions) + if err != nil { + klog.ErrorS(err, "failed to get format for secret", "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus)) + errs = append(errs, fmt.Errorf("failed to get format for secret %s, err: %w", secretName, err)) + continue + } if err = secretutil.ValidateSecretObject(*secretObj); err != nil { klog.ErrorS(err, "failed to validate secret object in spc", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus)) @@ -311,7 +318,7 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context, secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type)) var datamap map[string][]byte - if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil { + if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files, secretFormat, jsonPath); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err)) klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus)) errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err)) diff --git a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 80fa563a1..b3a037fb5 100644 --- a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,6 +80,32 @@ spec: type: string type: object type: array + syncOptions: + properties: + format: + description: the format of the secret (plaintext|json|yaml) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secrets: + items: + properties: + format: + description: the format of the secret (plaintext|json) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secretName: + description: name of the K8s secret object + type: string + type: object + type: array + type: + description: type of K8s secret object + type: string + type: object type: object status: description: SecretProviderClassStatus defines the observed state of SecretProviderClass diff --git a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 80fa563a1..b3a037fb5 100644 --- a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,6 +80,32 @@ spec: type: string type: object type: array + syncOptions: + properties: + format: + description: the format of the secret (plaintext|json|yaml) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secrets: + items: + properties: + format: + description: the format of the secret (plaintext|json) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secretName: + description: name of the K8s secret object + type: string + type: object + type: array + type: + description: type of K8s secret object + type: string + type: object type: object status: description: SecretProviderClassStatus defines the observed state of SecretProviderClass diff --git a/pkg/rotation/reconciler.go b/pkg/rotation/reconciler.go index a71d03801..5566ba190 100644 --- a/pkg/rotation/reconciler.go +++ b/pkg/rotation/reconciler.go @@ -455,6 +455,13 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret } for _, secretObj := range spc.Spec.SecretObjects { secretName := strings.TrimSpace(secretObj.SecretName) + jsonPath := secretutil.GetJsonPath(secretName, spc.Spec.SyncOptions) + secretFormat, err := secretutil.GetSecretFormat(secretName, spc.Spec.SyncOptions) + if err != nil { + klog.ErrorS(err, "failed to get format for secret", "secret", secretName, "spc", klog.KObj(spc), "controller", "rotation") + errs = append(errs, fmt.Errorf("failed to get format for secret %s, err: %w", secretName, err)) + continue + } if err = secretutil.ValidateSecretObject(*secretObj); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, k8sSecretRotationFailedReason, fmt.Sprintf("failed validation for secret object in spc %s/%s, err: %+v", spc.Namespace, spc.Name, err)) @@ -465,7 +472,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type)) var datamap map[string][]byte - if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil { + if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files, secretFormat, jsonPath); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, k8sSecretRotationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", spc.Namespace, spc.Name, secretName, err)) klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "secret", klog.ObjectRef{Namespace: spc.Namespace, Name: secretName}, "controller", "rotation") errs = append(errs, err) diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index 1d317bb4e..4c36893bd 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -21,6 +21,7 @@ import ( "crypto/rsa" "crypto/sha256" "crypto/x509" + "encoding/json" "encoding/pem" "fmt" "io" @@ -40,6 +41,12 @@ const ( privateKeyTypeEC = "EC PRIVATE KEY" ) +const ( + formatJSON = "json" + formatPlaintext = "plaintext" + FormatAuto = "auto" +) + // getCertPart returns the certificate or the private key part of the cert func GetCertPart(data []byte, key string) ([]byte, error) { if key == corev1.TLSPrivateKeyKey { @@ -148,18 +155,19 @@ func ValidateSecretObject(secretObj secretsstorev1.SecretObject) error { // GetSecretData gets the object contents from the pods target path and returns a // map that will be populated in the Kubernetes secret data field -func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType corev1.SecretType, files map[string]string) (map[string][]byte, error) { +func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType corev1.SecretType, files map[string]string, format, jsonPath string) (map[string][]byte, error) { datamap := make(map[string][]byte) for _, data := range secretObjData { objectName := strings.TrimSpace(data.ObjectName) dataKey := strings.TrimSpace(data.Key) if len(objectName) == 0 { - return datamap, fmt.Errorf("object name in secretObjects.data") + return datamap, fmt.Errorf("object name in secretObjects.data is empty") } if len(dataKey) == 0 { return datamap, fmt.Errorf("key in secretObjects.data is empty") } + file, ok := files[objectName] if !ok { return datamap, fmt.Errorf("file matching objectName %s not found in the pod", objectName) @@ -168,13 +176,64 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType if err != nil { return datamap, fmt.Errorf("failed to read file %s, err: %w", objectName, err) } - datamap[dataKey] = content - if secretType == corev1.SecretTypeTLS { - c, err := GetCertPart(content, dataKey) - if err != nil { - return datamap, fmt.Errorf("failed to get cert data from file %s, err: %w", file, err) + + // TODO (manedurphy) Take auto-detection into consideration + switch format { + case formatJSON: + var ( + jsonContent map[string]interface{} + valBytes []byte + err error + ) + + if err = json.Unmarshal(content, &jsonContent); err == nil { + if jsonPath != "" { + var ( + jsonPathSplit []string + valid bool + ) + jsonPathSplit = strings.Split(jsonPath, ".")[1:] + for _, path := range jsonPathSplit { + if jsonContent, valid = jsonContent[path].(map[string]interface{}); !valid { + return datamap, fmt.Errorf("invalid json path: %s", jsonPath) + } + } + } + + // extract key-value pairs from the json object file content + for key, val := range jsonContent { + switch val := val.(type) { + case string: + valBytes = []byte(val) + default: + // TODO (manedurphy) Describe the behavior here + if valBytes, err = json.Marshal(val); err != nil { + return datamap, fmt.Errorf("failed to marshal value %v, err: %w", val, err) + } + } + datamap[key] = valBytes + } + continue } - datamap[dataKey] = c + return datamap, fmt.Errorf("failed to unmarshal JSON file contents %s, err: %w", file, err) + default: + // set the contents of the mounted secret as the value + datamap[dataKey] = content + if secretType == corev1.SecretTypeTLS { + c, err := GetCertPart(content, dataKey) + if err != nil { + return datamap, fmt.Errorf("failed to get cert data from file %s, err: %w", file, err) + } + datamap[dataKey] = c + } + // TODO (manedurphy) handle basic auth data, perhaps differently + // if secretType == corev1.SecretTypeBasicAuth { + // username, password := getBasicAuthCredentials(content) + // delete(datamap, dataKey) + + // datamap[basicAuthUsername] = []byte(username) + // datamap[basicAuthPassword] = []byte(password) + // } } } return datamap, nil @@ -202,3 +261,51 @@ func generateSHA(data string) (string, error) { sha := hasher.Sum(nil) return fmt.Sprintf("%x", sha), nil } + +// TODO (manedurphy) Add description, write unit tests +func GetSecretFormat(secretName string, syncOptions secretsstorev1.SyncOptions) (string, error) { + var format string + + // top level value for secret format + if syncOptions.Format != "" { + format = syncOptions.Format + } + for _, secret := range syncOptions.Secrets { + if secret.SecretName == secretName { + if secret.Format != "" { + // secret format is set on a specific secret + format = secret.Format + } + } + } + if format == "" { + format = formatPlaintext + } + + // TODO (manedurphy) Add support for YAML + if format != formatPlaintext && format != formatJSON { + return format, fmt.Errorf("unsupported secret format: %s", format) + } + + return format, nil +} + +// TODO (manedurphy) Add description, write unit tests +func GetJsonPath(secretName string, syncOptions secretsstorev1.SyncOptions) string { + var jsonPath string + + // top level value for json path + if syncOptions.JsonPath != "" { + jsonPath = syncOptions.JsonPath + } + for _, secret := range syncOptions.Secrets { + if secret.SecretName == secretName { + if secret.JsonPath != "" { + // json path is set on a specific secret + jsonPath = secret.JsonPath + } + } + } + + return jsonPath +} From 5021a7dfa50eff052dde7249cec28e3e621c2e46 Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Fri, 27 May 2022 00:54:45 -0700 Subject: [PATCH 02/12] implemented json path templating --- apis/v1/secretproviderclass_types.go | 2 ++ ...re.csi.x-k8s.io_secretproviderclasses.yaml | 29 +++++++++++++++++ ...re.csi.x-k8s.io_secretproviderclasses.yaml | 3 ++ ...secretproviderclasspodstatus_controller.go | 4 +-- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 29 +++++++++++++++++ ...re.csi.x-k8s.io_secretproviderclasses.yaml | 3 ++ ...re.csi.x-k8s.io_secretproviderclasses.yaml | 3 ++ pkg/rotation/reconciler.go | 2 +- pkg/util/secretutil/secret.go | 31 ++++++++++++++----- 9 files changed, 96 insertions(+), 10 deletions(-) diff --git a/apis/v1/secretproviderclass_types.go b/apis/v1/secretproviderclass_types.go index 36238644c..f16aa0e15 100644 --- a/apis/v1/secretproviderclass_types.go +++ b/apis/v1/secretproviderclass_types.go @@ -57,6 +57,8 @@ type SyncOptions struct { } type Secret struct { + // type of K8s secret object + Type string `json:"type,omitempty"` // name of the K8s secret object SecretName string `json:"secretName,omitempty"` // the format of the secret (plaintext|json) diff --git a/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 80fa563a1..5fd5fc86c 100644 --- a/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,6 +80,35 @@ spec: type: string type: object type: array + syncOptions: + properties: + format: + description: the format of the secret (plaintext|json|yaml) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secrets: + items: + properties: + format: + description: the format of the secret (plaintext|json) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secretName: + description: name of the K8s secret object + type: string + type: + description: type of K8s secret object + type: string + type: object + type: array + type: + description: type of K8s secret object + type: string + type: object type: object status: description: SecretProviderClassStatus defines the observed state of SecretProviderClass diff --git a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b3a037fb5..5fd5fc86c 100644 --- a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -100,6 +100,9 @@ spec: secretName: description: name of the K8s secret object type: string + type: + description: type of K8s secret object + type: string type: object type: array type: diff --git a/controllers/secretproviderclasspodstatus_controller.go b/controllers/secretproviderclasspodstatus_controller.go index fdc0f985c..f5b3eb005 100644 --- a/controllers/secretproviderclasspodstatus_controller.go +++ b/controllers/secretproviderclasspodstatus_controller.go @@ -287,6 +287,8 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context, errs := make([]error, 0) for _, secretObj := range spc.Spec.SecretObjects { secretName := strings.TrimSpace(secretObj.SecretName) + // TODO (manedurphy) can we migrate the configuration for the secret type to be in just syncOptions? + secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type), secretName, spc.Spec.SyncOptions) jsonPath := secretutil.GetJsonPath(secretName, spc.Spec.SyncOptions) secretFormat, err := secretutil.GetSecretFormat(secretName, spc.Spec.SyncOptions) if err != nil { @@ -315,8 +317,6 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context, var funcs []func() (bool, error) if !exists { - secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type)) - var datamap map[string][]byte if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files, secretFormat, jsonPath); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err)) diff --git a/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 80fa563a1..5fd5fc86c 100644 --- a/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,6 +80,35 @@ spec: type: string type: object type: array + syncOptions: + properties: + format: + description: the format of the secret (plaintext|json|yaml) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secrets: + items: + properties: + format: + description: the format of the secret (plaintext|json) + type: string + jsonPath: + description: the nested object to target for syncing + type: string + secretName: + description: name of the K8s secret object + type: string + type: + description: type of K8s secret object + type: string + type: object + type: array + type: + description: type of K8s secret object + type: string + type: object type: object status: description: SecretProviderClassStatus defines the observed state of SecretProviderClass diff --git a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b3a037fb5..5fd5fc86c 100644 --- a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -100,6 +100,9 @@ spec: secretName: description: name of the K8s secret object type: string + type: + description: type of K8s secret object + type: string type: object type: array type: diff --git a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b3a037fb5..5fd5fc86c 100644 --- a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -100,6 +100,9 @@ spec: secretName: description: name of the K8s secret object type: string + type: + description: type of K8s secret object + type: string type: object type: array type: diff --git a/pkg/rotation/reconciler.go b/pkg/rotation/reconciler.go index 5566ba190..2169a2801 100644 --- a/pkg/rotation/reconciler.go +++ b/pkg/rotation/reconciler.go @@ -455,6 +455,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret } for _, secretObj := range spc.Spec.SecretObjects { secretName := strings.TrimSpace(secretObj.SecretName) + secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type), secretName, spc.Spec.SyncOptions) jsonPath := secretutil.GetJsonPath(secretName, spc.Spec.SyncOptions) secretFormat, err := secretutil.GetSecretFormat(secretName, spc.Spec.SyncOptions) if err != nil { @@ -470,7 +471,6 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret continue } - secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type)) var datamap map[string][]byte if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files, secretFormat, jsonPath); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, k8sSecretRotationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", spc.Namespace, spc.Name, secretName, err)) diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index 4c36893bd..a1108ffc5 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -131,11 +131,27 @@ func getPrivateKey(data []byte) ([]byte, error) { // GetSecretType returns a k8s secret type. // Kubernetes doesn't impose any constraints on the type name: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types // If the secret type is empty, then default is Opaque. -func GetSecretType(sType string) corev1.SecretType { - if sType == "" { - return corev1.SecretTypeOpaque +func GetSecretType(secretType, secretName string, syncOptions secretsstorev1.SyncOptions) corev1.SecretType { + // if secretType == "" { + // return corev1.SecretTypeOpaque + // } + // return corev1.SecretType(secretType) + + if secretType != "" { + return corev1.SecretType(secretType) } - return corev1.SecretType(sType) + + for _, secret := range syncOptions.Secrets { + if secret.SecretName == secretName && secret.Type == secretType { + return corev1.SecretType(secret.Type) + } + } + + if syncOptions.Type != "" { + return corev1.SecretType(syncOptions.Type) + } + + return corev1.SecretTypeOpaque } // ValidateSecretObject performs basic validation of the secret provider class @@ -144,9 +160,10 @@ func ValidateSecretObject(secretObj secretsstorev1.SecretObject) error { if len(secretObj.SecretName) == 0 { return fmt.Errorf("secret name is empty") } - if len(secretObj.Type) == 0 { - return fmt.Errorf("secret type is empty") - } + // TODO (manedurphy) remove this because we are determining the secret type between the secret object & sync options + // if len(secretObj.Type) == 0 { + // return fmt.Errorf("secret type is empty") + // } if len(secretObj.Data) == 0 { return fmt.Errorf("data is empty") } From ed2fcbef0c008e446cc2ff6731b7621fad424d92 Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Fri, 27 May 2022 01:38:49 -0700 Subject: [PATCH 03/12] removed "todo" comments --- .../secretproviderclasspodstatus_controller.go | 1 - pkg/util/secretutil/secret.go | 14 +------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/controllers/secretproviderclasspodstatus_controller.go b/controllers/secretproviderclasspodstatus_controller.go index f5b3eb005..63ce38988 100644 --- a/controllers/secretproviderclasspodstatus_controller.go +++ b/controllers/secretproviderclasspodstatus_controller.go @@ -287,7 +287,6 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context, errs := make([]error, 0) for _, secretObj := range spc.Spec.SecretObjects { secretName := strings.TrimSpace(secretObj.SecretName) - // TODO (manedurphy) can we migrate the configuration for the secret type to be in just syncOptions? secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type), secretName, spc.Spec.SyncOptions) jsonPath := secretutil.GetJsonPath(secretName, spc.Spec.SyncOptions) secretFormat, err := secretutil.GetSecretFormat(secretName, spc.Spec.SyncOptions) diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index a1108ffc5..c7462d9ea 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -160,10 +160,6 @@ func ValidateSecretObject(secretObj secretsstorev1.SecretObject) error { if len(secretObj.SecretName) == 0 { return fmt.Errorf("secret name is empty") } - // TODO (manedurphy) remove this because we are determining the secret type between the secret object & sync options - // if len(secretObj.Type) == 0 { - // return fmt.Errorf("secret type is empty") - // } if len(secretObj.Data) == 0 { return fmt.Errorf("data is empty") } @@ -223,7 +219,7 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType case string: valBytes = []byte(val) default: - // TODO (manedurphy) Describe the behavior here + // we can marshal non-string types to get unquoted values as well as handle nested objects if valBytes, err = json.Marshal(val); err != nil { return datamap, fmt.Errorf("failed to marshal value %v, err: %w", val, err) } @@ -243,14 +239,6 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType } datamap[dataKey] = c } - // TODO (manedurphy) handle basic auth data, perhaps differently - // if secretType == corev1.SecretTypeBasicAuth { - // username, password := getBasicAuthCredentials(content) - // delete(datamap, dataKey) - - // datamap[basicAuthUsername] = []byte(username) - // datamap[basicAuthPassword] = []byte(password) - // } } } return datamap, nil From 0f94d4cc31be38b9ce7d09f884e8d9bbe961741e Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Fri, 27 May 2022 13:12:03 -0700 Subject: [PATCH 04/12] small bug fix and code formatting --- pkg/util/secretutil/secret.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index c7462d9ea..3ce48b3ce 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -132,17 +132,12 @@ func getPrivateKey(data []byte) ([]byte, error) { // Kubernetes doesn't impose any constraints on the type name: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types // If the secret type is empty, then default is Opaque. func GetSecretType(secretType, secretName string, syncOptions secretsstorev1.SyncOptions) corev1.SecretType { - // if secretType == "" { - // return corev1.SecretTypeOpaque - // } - // return corev1.SecretType(secretType) - if secretType != "" { return corev1.SecretType(secretType) } for _, secret := range syncOptions.Secrets { - if secret.SecretName == secretName && secret.Type == secretType { + if secret.SecretName == secretName && secret.Type != "" { return corev1.SecretType(secret.Type) } } @@ -205,8 +200,10 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType jsonPathSplit []string valid bool ) + jsonPathSplit = strings.Split(jsonPath, ".")[1:] for _, path := range jsonPathSplit { + // set the value of "jsonContent" to the nested object at the key "path" if jsonContent, valid = jsonContent[path].(map[string]interface{}); !valid { return datamap, fmt.Errorf("invalid json path: %s", jsonPath) } @@ -215,16 +212,18 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType // extract key-value pairs from the json object file content for key, val := range jsonContent { - switch val := val.(type) { - case string: - valBytes = []byte(val) - default: - // we can marshal non-string types to get unquoted values as well as handle nested objects - if valBytes, err = json.Marshal(val); err != nil { - return datamap, fmt.Errorf("failed to marshal value %v, err: %w", val, err) + if data.Key == key { + switch val := val.(type) { + case string: + valBytes = []byte(val) + default: + // we can marshal non-string types to get unquoted values as well as handle nested objects + if valBytes, err = json.Marshal(val); err != nil { + return datamap, fmt.Errorf("failed to marshal value %v, err: %w", val, err) + } } + datamap[key] = valBytes } - datamap[key] = valBytes } continue } From 70f8e3ecd4784835bb15b6492980d769e762a1b9 Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 18:25:21 -0700 Subject: [PATCH 05/12] cleanup --- apis/v1/secretproviderclass_types.go | 22 +-- apis/v1/zz_generated.deepcopy.go | 10 +- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 16 +- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 16 +- ...secretproviderclasspodstatus_controller.go | 12 +- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 16 +- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 16 +- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 16 +- pkg/rotation/reconciler.go | 13 +- pkg/secrets-store/nodeserver.go | 7 +- pkg/secrets-store/provider_client.go | 111 +++++++++++++- pkg/util/secretutil/secret.go | 141 ++---------------- 12 files changed, 157 insertions(+), 239 deletions(-) diff --git a/apis/v1/secretproviderclass_types.go b/apis/v1/secretproviderclass_types.go index f16aa0e15..2a8f074e6 100644 --- a/apis/v1/secretproviderclass_types.go +++ b/apis/v1/secretproviderclass_types.go @@ -46,25 +46,16 @@ type SecretObject struct { Data []*SecretObjectData `json:"data,omitempty"` } -type SyncOptions struct { - // type of K8s secret object - Type string `json:"type,omitempty"` - // the format of the secret (plaintext|json|yaml) - Format string `json:"format,omitempty"` - // the nested object to target for syncing +type TransformOptions struct { + Format string `json:"format,omitempty"` JsonPath string `json:"jsonPath,omitempty"` Secrets []Secret `json:"secrets,omitempty"` } type Secret struct { - // type of K8s secret object - Type string `json:"type,omitempty"` - // name of the K8s secret object - SecretName string `json:"secretName,omitempty"` - // the format of the secret (plaintext|json) - Format string `json:"format,omitempty"` - // the nested object to target for syncing - JsonPath string `json:"jsonPath,omitempty"` + ObjectName string `json:"objectName,omitempty"` + Format string `json:"format,omitempty"` + JsonPath string `json:"jsonPath,omitempty"` } // SecretProviderClassSpec defines the desired state of SecretProviderClass @@ -74,7 +65,8 @@ type SecretProviderClassSpec struct { // Configuration for specific provider Parameters map[string]string `json:"parameters,omitempty"` SecretObjects []*SecretObject `json:"secretObjects,omitempty"` - SyncOptions SyncOptions `json:"syncOptions,omitempty"` + // Configuration for secret transformation + TransformOptions TransformOptions `json:"transformOptions,omitempty"` } // ByPodStatus defines the state of SecretProviderClass as seen by diff --git a/apis/v1/zz_generated.deepcopy.go b/apis/v1/zz_generated.deepcopy.go index c822fae92..529a38aee 100644 --- a/apis/v1/zz_generated.deepcopy.go +++ b/apis/v1/zz_generated.deepcopy.go @@ -283,7 +283,7 @@ func (in *SecretProviderClassSpec) DeepCopyInto(out *SecretProviderClassSpec) { } } } - in.SyncOptions.DeepCopyInto(&out.SyncOptions) + in.TransformOptions.DeepCopyInto(&out.TransformOptions) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretProviderClassSpec. @@ -323,7 +323,7 @@ func (in *SecretProviderClassStatus) DeepCopy() *SecretProviderClassStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SyncOptions) DeepCopyInto(out *SyncOptions) { +func (in *TransformOptions) DeepCopyInto(out *TransformOptions) { *out = *in if in.Secrets != nil { in, out := &in.Secrets, &out.Secrets @@ -332,12 +332,12 @@ func (in *SyncOptions) DeepCopyInto(out *SyncOptions) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SyncOptions. -func (in *SyncOptions) DeepCopy() *SyncOptions { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TransformOptions. +func (in *TransformOptions) DeepCopy() *TransformOptions { if in == nil { return nil } - out := new(SyncOptions) + out := new(TransformOptions) in.DeepCopyInto(out) return out } diff --git a/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 5fd5fc86c..b73bd04ee 100644 --- a/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,34 +80,24 @@ spec: type: string type: object type: array - syncOptions: + transformOptions: + description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` properties: format: - description: the format of the secret (plaintext|json|yaml) type: string jsonPath: - description: the nested object to target for syncing type: string secrets: items: properties: format: - description: the format of the secret (plaintext|json) type: string jsonPath: - description: the nested object to target for syncing type: string - secretName: - description: name of the K8s secret object - type: string - type: - description: type of K8s secret object + objectName: type: string type: object type: array - type: - description: type of K8s secret object - type: string type: object type: object status: diff --git a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 5fd5fc86c..b73bd04ee 100644 --- a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,34 +80,24 @@ spec: type: string type: object type: array - syncOptions: + transformOptions: + description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` properties: format: - description: the format of the secret (plaintext|json|yaml) type: string jsonPath: - description: the nested object to target for syncing type: string secrets: items: properties: format: - description: the format of the secret (plaintext|json) type: string jsonPath: - description: the nested object to target for syncing type: string - secretName: - description: name of the K8s secret object - type: string - type: - description: type of K8s secret object + objectName: type: string type: object type: array - type: - description: type of K8s secret object - type: string type: object type: object status: diff --git a/controllers/secretproviderclasspodstatus_controller.go b/controllers/secretproviderclasspodstatus_controller.go index 63ce38988..c9302daf1 100644 --- a/controllers/secretproviderclasspodstatus_controller.go +++ b/controllers/secretproviderclasspodstatus_controller.go @@ -287,14 +287,6 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context, errs := make([]error, 0) for _, secretObj := range spc.Spec.SecretObjects { secretName := strings.TrimSpace(secretObj.SecretName) - secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type), secretName, spc.Spec.SyncOptions) - jsonPath := secretutil.GetJsonPath(secretName, spc.Spec.SyncOptions) - secretFormat, err := secretutil.GetSecretFormat(secretName, spc.Spec.SyncOptions) - if err != nil { - klog.ErrorS(err, "failed to get format for secret", "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus)) - errs = append(errs, fmt.Errorf("failed to get format for secret %s, err: %w", secretName, err)) - continue - } if err = secretutil.ValidateSecretObject(*secretObj); err != nil { klog.ErrorS(err, "failed to validate secret object in spc", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus)) @@ -316,8 +308,10 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context, var funcs []func() (bool, error) if !exists { + secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type)) + var datamap map[string][]byte - if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files, secretFormat, jsonPath); err != nil { + if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err)) klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus)) errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err)) diff --git a/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 5fd5fc86c..b73bd04ee 100644 --- a/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,34 +80,24 @@ spec: type: string type: object type: array - syncOptions: + transformOptions: + description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` properties: format: - description: the format of the secret (plaintext|json|yaml) type: string jsonPath: - description: the nested object to target for syncing type: string secrets: items: properties: format: - description: the format of the secret (plaintext|json) type: string jsonPath: - description: the nested object to target for syncing type: string - secretName: - description: name of the K8s secret object - type: string - type: - description: type of K8s secret object + objectName: type: string type: object type: array - type: - description: type of K8s secret object - type: string type: object type: object status: diff --git a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 5fd5fc86c..b73bd04ee 100644 --- a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,34 +80,24 @@ spec: type: string type: object type: array - syncOptions: + transformOptions: + description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` properties: format: - description: the format of the secret (plaintext|json|yaml) type: string jsonPath: - description: the nested object to target for syncing type: string secrets: items: properties: format: - description: the format of the secret (plaintext|json) type: string jsonPath: - description: the nested object to target for syncing type: string - secretName: - description: name of the K8s secret object - type: string - type: - description: type of K8s secret object + objectName: type: string type: object type: array - type: - description: type of K8s secret object - type: string type: object type: object status: diff --git a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index 5fd5fc86c..b73bd04ee 100644 --- a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -80,34 +80,24 @@ spec: type: string type: object type: array - syncOptions: + transformOptions: + description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` properties: format: - description: the format of the secret (plaintext|json|yaml) type: string jsonPath: - description: the nested object to target for syncing type: string secrets: items: properties: format: - description: the format of the secret (plaintext|json) type: string jsonPath: - description: the nested object to target for syncing type: string - secretName: - description: name of the K8s secret object - type: string - type: - description: type of K8s secret object + objectName: type: string type: object type: array - type: - description: type of K8s secret object - type: string type: object type: object status: diff --git a/pkg/rotation/reconciler.go b/pkg/rotation/reconciler.go index 2169a2801..cb615f8d2 100644 --- a/pkg/rotation/reconciler.go +++ b/pkg/rotation/reconciler.go @@ -384,7 +384,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, fmt.Sprintf("failed to lookup provider client: %q", providerName)) return fmt.Errorf("failed to lookup provider client: %q", providerName) } - newObjectVersions, errorReason, err := secretsstore.MountContent(ctx, providerClient, string(paramsJSON), string(secretsJSON), spcps.Status.TargetPath, string(permissionJSON), oldObjectVersions) + newObjectVersions, errorReason, err := secretsstore.MountContent(ctx, providerClient, string(paramsJSON), string(secretsJSON), spcps.Status.TargetPath, string(permissionJSON), oldObjectVersions, &spc.Spec.TransformOptions) if err != nil { r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, fmt.Sprintf("provider mount err: %+v", err)) return fmt.Errorf("failed to rotate objects for pod %s/%s, err: %w", spcps.Namespace, spcps.Status.PodName, err) @@ -455,14 +455,6 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret } for _, secretObj := range spc.Spec.SecretObjects { secretName := strings.TrimSpace(secretObj.SecretName) - secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type), secretName, spc.Spec.SyncOptions) - jsonPath := secretutil.GetJsonPath(secretName, spc.Spec.SyncOptions) - secretFormat, err := secretutil.GetSecretFormat(secretName, spc.Spec.SyncOptions) - if err != nil { - klog.ErrorS(err, "failed to get format for secret", "secret", secretName, "spc", klog.KObj(spc), "controller", "rotation") - errs = append(errs, fmt.Errorf("failed to get format for secret %s, err: %w", secretName, err)) - continue - } if err = secretutil.ValidateSecretObject(*secretObj); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, k8sSecretRotationFailedReason, fmt.Sprintf("failed validation for secret object in spc %s/%s, err: %+v", spc.Namespace, spc.Name, err)) @@ -471,8 +463,9 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret continue } + secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type)) var datamap map[string][]byte - if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files, secretFormat, jsonPath); err != nil { + if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil { r.generateEvent(pod, corev1.EventTypeWarning, k8sSecretRotationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", spc.Namespace, spc.Name, secretName, err)) klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "secret", klog.ObjectRef{Namespace: spc.Namespace, Name: secretName}, "controller", "rotation") errs = append(errs, err) diff --git a/pkg/secrets-store/nodeserver.go b/pkg/secrets-store/nodeserver.go index 407384787..bbd33c335 100644 --- a/pkg/secrets-store/nodeserver.go +++ b/pkg/secrets-store/nodeserver.go @@ -25,6 +25,7 @@ import ( "path/filepath" "time" + v1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" internalerrors "sigs.k8s.io/secrets-store-csi-driver/pkg/errors" "sigs.k8s.io/secrets-store-csi-driver/pkg/k8s" "sigs.k8s.io/secrets-store-csi-driver/pkg/util/fileutil" @@ -238,7 +239,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis } mounted = true var objectVersions map[string]string - if objectVersions, errorReason, err = ns.mountSecretsStoreObjectContent(ctx, providerName, string(parametersStr), string(secretStr), targetPath, string(permissionStr), podName); err != nil { + if objectVersions, errorReason, err = ns.mountSecretsStoreObjectContent(ctx, providerName, string(parametersStr), string(secretStr), targetPath, string(permissionStr), podName, &spc.Spec.TransformOptions); err != nil { klog.ErrorS(err, "failed to mount secrets store object content", "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}) return nil, fmt.Errorf("failed to mount secrets store objects for pod %s/%s, err: %w", podNamespace, podName, err) } @@ -335,7 +336,7 @@ func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag return &csi.NodeUnstageVolumeResponse{}, nil } -func (ns *nodeServer) mountSecretsStoreObjectContent(ctx context.Context, providerName, attributes, secrets, targetPath, permission, podName string) (map[string]string, string, error) { +func (ns *nodeServer) mountSecretsStoreObjectContent(ctx context.Context, providerName, attributes, secrets, targetPath, permission, podName string, transformOptions *v1.TransformOptions) (map[string]string, string, error) { if len(attributes) == 0 { return nil, "", errors.New("missing attributes") } @@ -358,7 +359,7 @@ func (ns *nodeServer) mountSecretsStoreObjectContent(ctx context.Context, provid klog.InfoS("Using gRPC client", "provider", providerName, "pod", podName) - return MountContent(ctx, client, attributes, secrets, targetPath, permission, nil) + return MountContent(ctx, client, attributes, secrets, targetPath, permission, nil, transformOptions) } func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRequest) (*csi.NodeGetInfoResponse, error) { diff --git a/pkg/secrets-store/provider_client.go b/pkg/secrets-store/provider_client.go index 24820b9d3..fbd55da8f 100644 --- a/pkg/secrets-store/provider_client.go +++ b/pkg/secrets-store/provider_client.go @@ -18,6 +18,7 @@ package secretsstore import ( "context" + "encoding/json" "errors" "fmt" "net" @@ -28,6 +29,7 @@ import ( "sync" "time" + v1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" internalerrors "sigs.k8s.io/secrets-store-csi-driver/pkg/errors" "sigs.k8s.io/secrets-store-csi-driver/pkg/util/fileutil" "sigs.k8s.io/secrets-store-csi-driver/pkg/util/runtimeutil" @@ -65,6 +67,11 @@ const ServiceConfig = ` } ` +const ( + formatJSON = "json" + formatPlaintext = "plaintext" +) + var ( // PluginNameRe is the regular expression used to validate plugin names. PluginNameRe = regexp.MustCompile(`^[a-zA-Z0-9_-]{0,30}$`) @@ -222,7 +229,7 @@ func (p *PluginClientBuilder) HealthCheck(ctx context.Context, interval time.Dur // MountContent calls the client's Mount() RPC with helpers to format the // request and interpret the response. -func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, attributes, secrets, targetPath, permission string, oldObjectVersions map[string]string) (map[string]string, string, error) { +func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, attributes, secrets, targetPath, permission string, oldObjectVersions map[string]string, transformOptions *v1.TransformOptions) (map[string]string, string, error) { var objVersions []*v1alpha1.ObjectVersion for obj, version := range oldObjectVersions { objVersions = append(objVersions, &v1alpha1.ObjectVersion{Id: obj, Version: version}) @@ -268,9 +275,109 @@ func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, if err := fileutil.Validate(resp.GetFiles()); err != nil { return nil, internalerrors.FileWriteError, err } - if err := fileutil.WritePayloads(targetPath, resp.GetFiles()); err != nil { + + getSecret := func(path string) v1.Secret { + for _, secret := range transformOptions.Secrets { + if secret.ObjectName == path { + return secret + } + } + return v1.Secret{} + } + + var files []*v1alpha1.File + for _, file := range resp.GetFiles() { + format := formatPlaintext + jsonPath := transformOptions.JsonPath + secret := getSecret(file.Path) + + if transformOptions.Format != "" { + format = transformOptions.Format + } + if secret.Format != "" { + format = secret.Format + } + if secret.JsonPath != "" { + jsonPath = secret.JsonPath + } + + switch format { + case formatJSON: + var fileContent map[string]interface{} + if err := json.Unmarshal(file.Contents, &fileContent); err != nil { + return nil, "UnmarshalFileContentError", fmt.Errorf("could not unmarshal file contents: %w", err) + } + + for _, path := range strings.Split(jsonPath, ".")[1:] { + if content, valid := fileContent[path]; valid { + fileContent = content.(map[string]interface{}) + } + } + + for k, v := range fileContent { + var content []byte + + switch val := v.(type) { + case string: + content = []byte(val) + default: + content, _ = json.Marshal(val) + } + + files = append(files, &v1alpha1.File{ + Path: fmt.Sprintf("%s/%s", file.Path, k), + Mode: file.Mode, + Contents: content, + }) + } + case formatPlaintext: + files = append(files, file) + } + } + if err := fileutil.WritePayloads(targetPath, files); err != nil { return nil, internalerrors.FileWriteError, err } + + // switch transformOptions.Format { + // case "json": + // var files []*v1alpha1.File + // for _, file := range resp.GetFiles() { + // var fileContent map[string]interface{} + // if err := json.Unmarshal(file.Contents, &fileContent); err != nil { + // return nil, "UnmarshalFileContentError", fmt.Errorf("could not unmarshal file contents: %w", err) + // } + + // for _, path := range strings.Split(transformOptions.JsonPath, ".")[1:] { + // if content, valid := fileContent[path]; valid { + // fileContent = content.(map[string]interface{}) + // } + // } + + // for k, v := range fileContent { + // var content []byte + + // switch val := v.(type) { + // case string: + // content = []byte(val) + // default: + // content, _ = json.Marshal(val) + // } + + // files = append(files, &v1alpha1.File{ + // Path: fmt.Sprintf("%s/%s", file.Path, k), + // Mode: file.Mode, + // Contents: content, + // }) + // } + // } + // if err := fileutil.WritePayloads(targetPath, files); err != nil { + // return nil, internalerrors.FileWriteError, err + // } + // default: + // if err := fileutil.WritePayloads(targetPath, resp.GetFiles()); err != nil { + // return nil, internalerrors.FileWriteError, err + // } + // } } else { // when no files are returned we assume that the plugin has not migrated // grpc responses for writing files yet. diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index 3ce48b3ce..fe6073155 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -21,7 +21,6 @@ import ( "crypto/rsa" "crypto/sha256" "crypto/x509" - "encoding/json" "encoding/pem" "fmt" "io" @@ -41,12 +40,6 @@ const ( privateKeyTypeEC = "EC PRIVATE KEY" ) -const ( - formatJSON = "json" - formatPlaintext = "plaintext" - FormatAuto = "auto" -) - // getCertPart returns the certificate or the private key part of the cert func GetCertPart(data []byte, key string) ([]byte, error) { if key == corev1.TLSPrivateKeyKey { @@ -128,25 +121,11 @@ func getPrivateKey(data []byte) ([]byte, error) { return pem.EncodeToMemory(block), nil } -// GetSecretType returns a k8s secret type. -// Kubernetes doesn't impose any constraints on the type name: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types -// If the secret type is empty, then default is Opaque. -func GetSecretType(secretType, secretName string, syncOptions secretsstorev1.SyncOptions) corev1.SecretType { - if secretType != "" { - return corev1.SecretType(secretType) +func GetSecretType(sType string) corev1.SecretType { + if sType == "" { + return corev1.SecretTypeOpaque } - - for _, secret := range syncOptions.Secrets { - if secret.SecretName == secretName && secret.Type != "" { - return corev1.SecretType(secret.Type) - } - } - - if syncOptions.Type != "" { - return corev1.SecretType(syncOptions.Type) - } - - return corev1.SecretTypeOpaque + return corev1.SecretType(sType) } // ValidateSecretObject performs basic validation of the secret provider class @@ -161,9 +140,7 @@ func ValidateSecretObject(secretObj secretsstorev1.SecretObject) error { return nil } -// GetSecretData gets the object contents from the pods target path and returns a -// map that will be populated in the Kubernetes secret data field -func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType corev1.SecretType, files map[string]string, format, jsonPath string) (map[string][]byte, error) { +func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType corev1.SecretType, files map[string]string) (map[string][]byte, error) { datamap := make(map[string][]byte) for _, data := range secretObjData { objectName := strings.TrimSpace(data.ObjectName) @@ -175,7 +152,6 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType if len(dataKey) == 0 { return datamap, fmt.Errorf("key in secretObjects.data is empty") } - file, ok := files[objectName] if !ok { return datamap, fmt.Errorf("file matching objectName %s not found in the pod", objectName) @@ -184,60 +160,13 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType if err != nil { return datamap, fmt.Errorf("failed to read file %s, err: %w", objectName, err) } - - // TODO (manedurphy) Take auto-detection into consideration - switch format { - case formatJSON: - var ( - jsonContent map[string]interface{} - valBytes []byte - err error - ) - - if err = json.Unmarshal(content, &jsonContent); err == nil { - if jsonPath != "" { - var ( - jsonPathSplit []string - valid bool - ) - - jsonPathSplit = strings.Split(jsonPath, ".")[1:] - for _, path := range jsonPathSplit { - // set the value of "jsonContent" to the nested object at the key "path" - if jsonContent, valid = jsonContent[path].(map[string]interface{}); !valid { - return datamap, fmt.Errorf("invalid json path: %s", jsonPath) - } - } - } - - // extract key-value pairs from the json object file content - for key, val := range jsonContent { - if data.Key == key { - switch val := val.(type) { - case string: - valBytes = []byte(val) - default: - // we can marshal non-string types to get unquoted values as well as handle nested objects - if valBytes, err = json.Marshal(val); err != nil { - return datamap, fmt.Errorf("failed to marshal value %v, err: %w", val, err) - } - } - datamap[key] = valBytes - } - } - continue - } - return datamap, fmt.Errorf("failed to unmarshal JSON file contents %s, err: %w", file, err) - default: - // set the contents of the mounted secret as the value - datamap[dataKey] = content - if secretType == corev1.SecretTypeTLS { - c, err := GetCertPart(content, dataKey) - if err != nil { - return datamap, fmt.Errorf("failed to get cert data from file %s, err: %w", file, err) - } - datamap[dataKey] = c + datamap[dataKey] = content + if secretType == corev1.SecretTypeTLS { + c, err := GetCertPart(content, dataKey) + if err != nil { + return datamap, fmt.Errorf("failed to get cert data from file %s, err: %w", file, err) } + datamap[dataKey] = c } } return datamap, nil @@ -265,51 +194,3 @@ func generateSHA(data string) (string, error) { sha := hasher.Sum(nil) return fmt.Sprintf("%x", sha), nil } - -// TODO (manedurphy) Add description, write unit tests -func GetSecretFormat(secretName string, syncOptions secretsstorev1.SyncOptions) (string, error) { - var format string - - // top level value for secret format - if syncOptions.Format != "" { - format = syncOptions.Format - } - for _, secret := range syncOptions.Secrets { - if secret.SecretName == secretName { - if secret.Format != "" { - // secret format is set on a specific secret - format = secret.Format - } - } - } - if format == "" { - format = formatPlaintext - } - - // TODO (manedurphy) Add support for YAML - if format != formatPlaintext && format != formatJSON { - return format, fmt.Errorf("unsupported secret format: %s", format) - } - - return format, nil -} - -// TODO (manedurphy) Add description, write unit tests -func GetJsonPath(secretName string, syncOptions secretsstorev1.SyncOptions) string { - var jsonPath string - - // top level value for json path - if syncOptions.JsonPath != "" { - jsonPath = syncOptions.JsonPath - } - for _, secret := range syncOptions.Secrets { - if secret.SecretName == secretName { - if secret.JsonPath != "" { - // json path is set on a specific secret - jsonPath = secret.JsonPath - } - } - } - - return jsonPath -} From 28f2fc05d6548bb120e2cc313e192517e0e19fda Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 18:45:14 -0700 Subject: [PATCH 06/12] regenerate charts --- apis/v1/secretproviderclass_types.go | 19 ++++++++++++------- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 11 ++++++++++- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 11 ++++++++++- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 11 ++++++++++- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 11 ++++++++++- ...re.csi.x-k8s.io_secretproviderclasses.yaml | 11 ++++++++++- 6 files changed, 62 insertions(+), 12 deletions(-) diff --git a/apis/v1/secretproviderclass_types.go b/apis/v1/secretproviderclass_types.go index 2a8f074e6..c77527d95 100644 --- a/apis/v1/secretproviderclass_types.go +++ b/apis/v1/secretproviderclass_types.go @@ -46,18 +46,23 @@ type SecretObject struct { Data []*SecretObjectData `json:"data,omitempty"` } +type Secret struct { + // name of the object to sync + ObjectName string `json:"objectName,omitempty"` + // expected format of the secret received from the provider (e.g. plaintext, json) + Format string `json:"format,omitempty"` + // JSON path to target for a secret received in JSON format + JsonPath string `json:"jsonPath,omitempty"` +} + type TransformOptions struct { - Format string `json:"format,omitempty"` + // expected format of the secret received from the provider (e.g. plaintext, json) + Format string `json:"format,omitempty"` + // JSON path to target for a secret received in JSON format JsonPath string `json:"jsonPath,omitempty"` Secrets []Secret `json:"secrets,omitempty"` } -type Secret struct { - ObjectName string `json:"objectName,omitempty"` - Format string `json:"format,omitempty"` - JsonPath string `json:"jsonPath,omitempty"` -} - // SecretProviderClassSpec defines the desired state of SecretProviderClass type SecretProviderClassSpec struct { // Configuration for provider name diff --git a/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b73bd04ee..d1b535ebd 100644 --- a/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -81,20 +81,29 @@ spec: type: object type: array transformOptions: - description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` + description: Configuration for secret transformation properties: format: + description: expected format of the secret received from the provider + (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in JSON + format type: string secrets: items: properties: format: + description: expected format of the secret received from + the provider (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in + JSON format type: string objectName: + description: name of the object to sync type: string type: object type: array diff --git a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b73bd04ee..d1b535ebd 100644 --- a/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/config/crd/bases/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -81,20 +81,29 @@ spec: type: object type: array transformOptions: - description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` + description: Configuration for secret transformation properties: format: + description: expected format of the secret received from the provider + (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in JSON + format type: string secrets: items: properties: format: + description: expected format of the secret received from + the provider (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in + JSON format type: string objectName: + description: name of the object to sync type: string type: object type: array diff --git a/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b73bd04ee..d1b535ebd 100644 --- a/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -81,20 +81,29 @@ spec: type: object type: array transformOptions: - description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` + description: Configuration for secret transformation properties: format: + description: expected format of the secret received from the provider + (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in JSON + format type: string secrets: items: properties: format: + description: expected format of the secret received from + the provider (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in + JSON format type: string objectName: + description: name of the object to sync type: string type: object type: array diff --git a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b73bd04ee..d1b535ebd 100644 --- a/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -81,20 +81,29 @@ spec: type: object type: array transformOptions: - description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` + description: Configuration for secret transformation properties: format: + description: expected format of the secret received from the provider + (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in JSON + format type: string secrets: items: properties: format: + description: expected format of the secret received from + the provider (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in + JSON format type: string objectName: + description: name of the object to sync type: string type: object type: array diff --git a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml index b73bd04ee..d1b535ebd 100644 --- a/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml +++ b/manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -81,20 +81,29 @@ spec: type: object type: array transformOptions: - description: SyncOptions SyncOptions `json:"syncOptions,omitempty"` + description: Configuration for secret transformation properties: format: + description: expected format of the secret received from the provider + (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in JSON + format type: string secrets: items: properties: format: + description: expected format of the secret received from + the provider (e.g. plaintext, json) type: string jsonPath: + description: JSON path to target for a secret received in + JSON format type: string objectName: + description: name of the object to sync type: string type: object type: array From 06e579f3df80901619d978735c937cea6ce5d7aa Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 20:15:58 -0700 Subject: [PATCH 07/12] add unit tests --- pkg/secrets-store/nodeserver_test.go | 2 +- pkg/secrets-store/provider_client.go | 43 +------------------- pkg/secrets-store/provider_client_test.go | 49 ++++++++++++++++++----- 3 files changed, 42 insertions(+), 52 deletions(-) diff --git a/pkg/secrets-store/nodeserver_test.go b/pkg/secrets-store/nodeserver_test.go index ba9594cfb..d0d0743ea 100644 --- a/pkg/secrets-store/nodeserver_test.go +++ b/pkg/secrets-store/nodeserver_test.go @@ -405,7 +405,7 @@ func TestMountSecretsStoreObjectContent(t *testing.T) { if err != nil { t.Fatalf("expected error to be nil, got: %+v", err) } - _, errorReason, err := ns.mountSecretsStoreObjectContent(context.TODO(), "provider1", test.attributes, test.secrets, test.targetPath, test.permission, "pod") + _, errorReason, err := ns.mountSecretsStoreObjectContent(context.TODO(), "provider1", test.attributes, test.secrets, test.targetPath, test.permission, "pod", nil) if errorReason != test.expectedErrorReason { t.Fatalf("expected error reason to be %s, got: %s", test.expectedErrorReason, errorReason) } diff --git a/pkg/secrets-store/provider_client.go b/pkg/secrets-store/provider_client.go index fbd55da8f..be24e9941 100644 --- a/pkg/secrets-store/provider_client.go +++ b/pkg/secrets-store/provider_client.go @@ -93,7 +93,7 @@ type PluginClientBuilder struct { // plugins in the provided absolute path to a folder. Plugin servers must listen // to the unix domain socket at: // -// /.sock +// /.sock // // where must match the PluginNameRe regular expression. // @@ -337,47 +337,6 @@ func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, if err := fileutil.WritePayloads(targetPath, files); err != nil { return nil, internalerrors.FileWriteError, err } - - // switch transformOptions.Format { - // case "json": - // var files []*v1alpha1.File - // for _, file := range resp.GetFiles() { - // var fileContent map[string]interface{} - // if err := json.Unmarshal(file.Contents, &fileContent); err != nil { - // return nil, "UnmarshalFileContentError", fmt.Errorf("could not unmarshal file contents: %w", err) - // } - - // for _, path := range strings.Split(transformOptions.JsonPath, ".")[1:] { - // if content, valid := fileContent[path]; valid { - // fileContent = content.(map[string]interface{}) - // } - // } - - // for k, v := range fileContent { - // var content []byte - - // switch val := v.(type) { - // case string: - // content = []byte(val) - // default: - // content, _ = json.Marshal(val) - // } - - // files = append(files, &v1alpha1.File{ - // Path: fmt.Sprintf("%s/%s", file.Path, k), - // Mode: file.Mode, - // Contents: content, - // }) - // } - // } - // if err := fileutil.WritePayloads(targetPath, files); err != nil { - // return nil, internalerrors.FileWriteError, err - // } - // default: - // if err := fileutil.WritePayloads(targetPath, resp.GetFiles()); err != nil { - // return nil, internalerrors.FileWriteError, err - // } - // } } else { // when no files are returned we assume that the plugin has not migrated // grpc responses for writing files yet. diff --git a/pkg/secrets-store/provider_client_test.go b/pkg/secrets-store/provider_client_test.go index 90890e912..3af741d6c 100644 --- a/pkg/secrets-store/provider_client_test.go +++ b/pkg/secrets-store/provider_client_test.go @@ -27,6 +27,7 @@ import ( "testing" "time" + v1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" "sigs.k8s.io/secrets-store-csi-driver/pkg/test_utils/tmpdir" "sigs.k8s.io/secrets-store-csi-driver/pkg/util/fileutil" "sigs.k8s.io/secrets-store-csi-driver/provider/fake" @@ -58,7 +59,8 @@ func TestMountContent(t *testing.T) { cases := []struct { name string // inputs - permission string + permission string + transformOptions *v1.TransformOptions // mock outputs objectVersions map[string]string providerError error @@ -68,10 +70,11 @@ func TestMountContent(t *testing.T) { skipon string }{ { - name: "provider successful response (no files)", - permission: "420", - objectVersions: map[string]string{"secret/secret1": "v1", "secret/secret2": "v2"}, - expectedFiles: map[string]os.FileMode{}, + name: "provider successful response (no files)", + permission: "420", + objectVersions: map[string]string{"secret/secret1": "v1", "secret/secret2": "v2"}, + expectedFiles: map[string]os.FileMode{}, + transformOptions: &v1.TransformOptions{}, }, { name: "provider response with file", @@ -87,6 +90,7 @@ func TestMountContent(t *testing.T) { expectedFiles: map[string]os.FileMode{ "foo": 0666, }, + transformOptions: &v1.TransformOptions{}, }, { name: "provider response with multiple files", @@ -108,6 +112,7 @@ func TestMountContent(t *testing.T) { "foo": 0666, "bar": 0444, }, + transformOptions: &v1.TransformOptions{}, }, { name: "provider response with nested files (linux)", @@ -135,7 +140,8 @@ func TestMountContent(t *testing.T) { "baz/bar": 0777, "baz/qux": 0777, }, - skipon: "windows", + skipon: "windows", + transformOptions: &v1.TransformOptions{}, }, { // note: this is a bit weird because the path `baz\bar` on windows @@ -167,6 +173,31 @@ func TestMountContent(t *testing.T) { "baz\\bar": 0444, "baz\\qux": 0666, }, + transformOptions: &v1.TransformOptions{}, + }, + { + name: "provider response with JSON format", + permission: "777", + files: []*v1alpha1.File{ + { + Path: "db-creds", + Mode: 0666, + Contents: []byte("{\"username\": \"db-user\", \"password\": \"db-password\"}"), + }, + { + Path: "tls", + Mode: 0400, + Contents: []byte("{\"tls.key\": \"some-key\", \"tls.crt\": \"some-certificate\"}"), + }, + }, + objectVersions: map[string]string{"foo": "v1"}, + expectedFiles: map[string]os.FileMode{ + "db-creds/username": 0666, + "db-creds/password": 0666, + "tls/tls.key": 0400, + "tls/tls.crt": 0400, + }, + transformOptions: &v1.TransformOptions{Format: formatJSON}, }, } @@ -196,7 +227,7 @@ func TestMountContent(t *testing.T) { t.Fatalf("expected err to be nil, got: %+v", err) } - objectVersions, _, err := MountContent(context.TODO(), client, "{}", "{}", targetPath, test.permission, nil) + objectVersions, _, err := MountContent(context.TODO(), client, "{}", "{}", targetPath, test.permission, nil, test.transformOptions) if err != nil { t.Errorf("expected err to be nil, got: %+v", err) } @@ -254,7 +285,7 @@ func TestMountContent_TooLarge(t *testing.T) { } // rpc error: code = ResourceExhausted desc = grpc: received message larger than max (28 vs. 5) - _, errorCode, err := MountContent(context.TODO(), client, "{}", "{}", targetPath, "777", nil) + _, errorCode, err := MountContent(context.TODO(), client, "{}", "{}", targetPath, "777", nil, &v1.TransformOptions{}) if err == nil { t.Errorf("expected err to be not nil") } @@ -348,7 +379,7 @@ func TestMountContentError(t *testing.T) { t.Fatalf("expected err to be nil, got: %+v", err) } - objectVersions, errorCode, err := MountContent(context.TODO(), client, test.attributes, test.secrets, test.targetPath, test.permission, nil) + objectVersions, errorCode, err := MountContent(context.TODO(), client, test.attributes, test.secrets, test.targetPath, test.permission, nil, nil) if err == nil { t.Errorf("expected err to be not nil") } From f10e694979429996729550b7490730c93c19dd86 Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 20:20:18 -0700 Subject: [PATCH 08/12] wip --- pkg/util/secretutil/secret.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index fe6073155..6ca4a45e9 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -121,6 +121,9 @@ func getPrivateKey(data []byte) ([]byte, error) { return pem.EncodeToMemory(block), nil } +// GetSecretType returns a k8s secret type. +// Kubernetes doesn't impose any constraints on the type name: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types +// If the secret type is empty, then default is Opaque. func GetSecretType(sType string) corev1.SecretType { if sType == "" { return corev1.SecretTypeOpaque From 0b53f7c186495dbc7431fe8527fe3fa330c1d37f Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 20:21:03 -0700 Subject: [PATCH 09/12] added comments back --- pkg/util/secretutil/secret.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index 6ca4a45e9..0d756d20d 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -143,6 +143,8 @@ func ValidateSecretObject(secretObj secretsstorev1.SecretObject) error { return nil } +// GetSecretData gets the object contents from the pods target path and returns a +// map that will be populated in the Kubernetes secret data field func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType corev1.SecretType, files map[string]string) (map[string][]byte, error) { datamap := make(map[string][]byte) for _, data := range secretObjData { From d302f220318825177025ec608558c68495fffb29 Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 20:22:48 -0700 Subject: [PATCH 10/12] validate secret type --- pkg/util/secretutil/secret.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/util/secretutil/secret.go b/pkg/util/secretutil/secret.go index 0d756d20d..8ca871ed9 100644 --- a/pkg/util/secretutil/secret.go +++ b/pkg/util/secretutil/secret.go @@ -137,6 +137,9 @@ func ValidateSecretObject(secretObj secretsstorev1.SecretObject) error { if len(secretObj.SecretName) == 0 { return fmt.Errorf("secret name is empty") } + if len(secretObj.Type) == 0 { + return fmt.Errorf("secret type is empty") + } if len(secretObj.Data) == 0 { return fmt.Errorf("data is empty") } From 3d42b90483d1827320e107678389f8b32eea3b0e Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 20:38:03 -0700 Subject: [PATCH 11/12] added unit test for transformOptions with json path --- pkg/errors/errors.go | 4 ++++ pkg/secrets-store/provider_client.go | 7 +++++-- pkg/secrets-store/provider_client_test.go | 24 +++++++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index dc3a56ef4..98206bfde 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -45,4 +45,8 @@ const ( PodVolumeNotFound = "PodVolumeNotFound" // FileWriteError error FileWriteError = "FileWriteError" + // MarhsalError error + MarshalError = "MarshalError" + // UnmarhsalError error + UnmarshalError = "UnmarshalError" ) diff --git a/pkg/secrets-store/provider_client.go b/pkg/secrets-store/provider_client.go index be24e9941..dfbedc275 100644 --- a/pkg/secrets-store/provider_client.go +++ b/pkg/secrets-store/provider_client.go @@ -305,7 +305,7 @@ func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, case formatJSON: var fileContent map[string]interface{} if err := json.Unmarshal(file.Contents, &fileContent); err != nil { - return nil, "UnmarshalFileContentError", fmt.Errorf("could not unmarshal file contents: %w", err) + return nil, internalerrors.UnmarshalError, fmt.Errorf("failed to unmarshal file contents: %w", err) } for _, path := range strings.Split(jsonPath, ".")[1:] { @@ -321,7 +321,10 @@ func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, case string: content = []byte(val) default: - content, _ = json.Marshal(val) + content, err = json.Marshal(val) + if err != nil { + return nil, internalerrors.MarshalError, fmt.Errorf("failed to marshal file contents: %w", err) + } } files = append(files, &v1alpha1.File{ diff --git a/pkg/secrets-store/provider_client_test.go b/pkg/secrets-store/provider_client_test.go index 3af741d6c..0abe12bb9 100644 --- a/pkg/secrets-store/provider_client_test.go +++ b/pkg/secrets-store/provider_client_test.go @@ -199,6 +199,30 @@ func TestMountContent(t *testing.T) { }, transformOptions: &v1.TransformOptions{Format: formatJSON}, }, + { + name: "provider response with JSON format with JSON path", + permission: "777", + files: []*v1alpha1.File{ + { + Path: "db-creds", + Mode: 0666, + Contents: []byte("{\"data\": {\"data\": {\"username\": \"db-user\", \"password\": \"db-password\"}}}"), + }, + { + Path: "tls", + Mode: 0400, + Contents: []byte("{\"data\": {\"data\": {\"tls.key\": \"some-key\", \"tls.crt\": \"some-certificate\"}}}"), + }, + }, + objectVersions: map[string]string{"foo": "v1"}, + expectedFiles: map[string]os.FileMode{ + "db-creds/username": 0666, + "db-creds/password": 0666, + "tls/tls.key": 0400, + "tls/tls.crt": 0400, + }, + transformOptions: &v1.TransformOptions{Format: formatJSON, JsonPath: "$.data.data"}, + }, } for _, test := range cases { From a4c59fee80c6c64f96e3a56f119fe9ca1180fb76 Mon Sep 17 00:00:00 2001 From: Dane Murphy Date: Tue, 16 Aug 2022 21:45:19 -0700 Subject: [PATCH 12/12] more error handling --- pkg/errors/errors.go | 2 ++ pkg/secrets-store/provider_client.go | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 98206bfde..ec255e466 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -49,4 +49,6 @@ const ( MarshalError = "MarshalError" // UnmarhsalError error UnmarshalError = "UnmarshalError" + // InvalidJSONPathError error + InvalidJSONPathError = "InvalidJSONPathError" ) diff --git a/pkg/secrets-store/provider_client.go b/pkg/secrets-store/provider_client.go index dfbedc275..85a42de2c 100644 --- a/pkg/secrets-store/provider_client.go +++ b/pkg/secrets-store/provider_client.go @@ -308,10 +308,14 @@ func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, return nil, internalerrors.UnmarshalError, fmt.Errorf("failed to unmarshal file contents: %w", err) } + // The JSON path is configured in the SecretProviderClass: $.data.data -> ["data", "data"] for _, path := range strings.Split(jsonPath, ".")[1:] { if content, valid := fileContent[path]; valid { + // Update the value of "fileContent" to the nested path fileContent = content.(map[string]interface{}) + continue } + return nil, internalerrors.InvalidJSONPathError, fmt.Errorf("invalid json path: %s", transformOptions.JsonPath) } for k, v := range fileContent {