Add TransformOptions Configuration Block to SecretProviderClass

[manedurphy]

PR Type

/kind feature

Related Issues & Pull Requests

Issues

• Get all k/v pairs from endpoint
• Mount key values from json/yaml secret files

Pull Requests

• Sync All Secrets to K8s

Purpose

• The purpose for these changes is to add functionality that eases the processes of syncing secrets to K8s.
• In order to achieve this, the user can configure the secrets-store-driver to transform a JSON response into individual files based on the key/value pairs found within that JSON response.
• We will see in the examples below that the configuration for the mounted file's format is generic, and we will be able to support other formats in the future (e.g. YAML), but for now I just want to show the current changes and have an open discussion on the direction we need to take with this feature.
• Previous discussions that led to the implementation of this functionality can be seen in a previous PR that I raised related to an alternative functionality, SyncAll. I tried to be thorough enough in this document so referencing my previous work will not be necessary, but it is there if it helps to read it as well.

Implementation

API Type Definitions

• The following structures below have been defined for the V1 API.
• The transformOptions field is a new configuration block that is set in the SPC's definition.
• Looking at the SecretProviderClass dropdown below, we can see that communication is established with a Vault provider.
• For the Vault Provider, items without a specified secretKey will retrieve a JSON object with all secrets on the secret path and all metadata included with Vault's JSON response.
• The transformOptions configuration block allows the user to target a jsonPath on the JSON response body and create a file for each key/value pair in that nested object. In this example, a user will likely want to mount both key/value pairs stored within the .data.data JSON path in the Vault JSON response as individual files, rather than syncing the full JSON response with all of its metadata.
• Allowing the driver to handle JSON responses makes it easier for the user to retrieve and work with multiple secrets that are on the same path of their secrets store of choice.

V1 Definitions

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 {
	// 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"`
}

// SecretProviderClassSpec defines the desired state of SecretProviderClass
type SecretProviderClassSpec struct {
	// Configuration for provider name
	Provider Provider `json:"provider,omitempty"`
	// Configuration for specific provider
	Parameters    map[string]string `json:"parameters,omitempty"`
	SecretObjects []*SecretObject   `json:"secretObjects,omitempty"`
	// Configuration for secret transformation
	TransformOptions TransformOptions `json:"transformOptions,omitempty"`
}

SecretProviderClass

apiVersion: secrets-store.csi.x-k8s.io/v1
kind: SecretProviderClass
metadata:
  name: vault-spc
spec:
  provider: vault
  parameters:
    roleName: "csi"
    vaultAddress: "http://vault.vault:8200"
    objects: |
      - secretPath: "secret/data/database"
        objectName: "db-creds"
  transformOptions:
    format: json
    jsonPath: $.data.data

Vault JSON Response

{
  "request_id": "4142b613-78c7-3d78-1e5b-5d6dd9115386",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "data": {
      "password": "BnNcWA2Lt8",
      "username": "db-user"
    },
    "metadata": {
      "created_time": "2022-05-27T06:56:21.297437205Z",
      "custom_metadata": null,
      "deletion_time": "",
      "destroyed": false,
      "version": 1
    }
  },
  "warnings": null
}

Mounting Secret Content

• When the driver sees that files are returned from the provider, it will know how to handle the response based on the transformOptions.Format value.
• The default behavior is to handle a plaintext format, which is what the driver does already. The format can be configured in the SecretProviderClass as shown in the example previously.

MountContent

// 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, 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})
	}

	req := &v1alpha1.MountRequest{
		Attributes:           attributes,
		Secrets:              secrets,
		TargetPath:           targetPath,
		Permission:           permission,
		CurrentObjectVersion: objVersions,
	}

	resp, err := client.Mount(ctx, req)
	if err != nil {
		if isMaxRecvMsgSizeError(err) {
			klog.ErrorS(err, "Set --max-call-recv-msg-size to configure larger maximum size in bytes of gRPC response")
		}
		return nil, internalerrors.GRPCProviderError, err
	}
	if resp != nil && resp.GetError() != nil && len(resp.GetError().Code) > 0 {
		return nil, resp.GetError().Code, fmt.Errorf("mount request failed with provider error code %s", resp.GetError().Code)
	}

	ov := resp.GetObjectVersion()
	if ov == nil {
		return nil, internalerrors.GRPCProviderError, errors.New("missing object versions")
	}
	objectVersions := make(map[string]string)
	for _, v := range ov {
		objectVersions[v.Id] = v.Version
	}

	// warn if the proto response size is over 1 MiB.
	// Individual k8s secrets are limited to 1MiB in size.
	// Ref: https://kubernetes.io/docs/concepts/configuration/secret/#restrictions
	if size := proto.Size(resp); size > 1048576 {
		klog.InfoS("proto above 1MiB, secret sync may fail", "size", size)
	}

	if len(resp.GetFiles()) > 0 {
		klog.V(5).Info("writing mount response files")
		if err := fileutil.Validate(resp.GetFiles()); err != nil {
			return nil, internalerrors.FileWriteError, err
		}

		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, 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 {
					var content []byte

					switch val := v.(type) {
					case string:
						content = []byte(val)
					default:
						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{
						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
		}
	} else {
		// when no files are returned we assume that the plugin has not migrated
		// grpc responses for writing files yet.
		klog.V(5).Info("mount response has no files")
	}

	return objectVersions, "", nil
}

Example

• This example shows how to configure a SecretProviderClass handle secrets with multiple formats.
• We can use the .spec.transformOptions.secrets list to specify which secrets should be handled differently than what is configured at a higher level in the hierarchy.
• In this example, we are declaring that the 2 secrets in the secrets list should be handled as plaintext, and everything else should be handled as json.

SecretProviderClass

apiVersion: secrets-store.csi.x-k8s.io/v1
kind: SecretProviderClass
metadata:
  name: vault-spc
spec:
  provider: vault
  parameters:
    roleName: "csi"
    vaultAddress: "http://vault.vault:8200"
    objects: |
      - secretPath: "secret/data/database"
        objectName: "database/username"
        secretKey: "username"
      - secretPath: "secret/data/database"
        objectName: "database/password"
        secretKey: "password"
      - secretPath: "secret/data/database"
        objectName: "db-creds"
  secretObjects:
    - secretName: db-creds
      type: Opaque
      data:
        - objectName: db-creds/username
          key: username
        - objectName: db-creds/password
          key: password
  transformOptions:
    format: json
    jsonPath: $.data.data
    secrets:
      - objectName: database/username
        format: plaintext
      - objectName: database/password
        format: plaintext

• We can see within the container that we were able to retrieve the username and password from Vault with 2 strategies.
• The first strategy was to specify the secretKey, which gets a specfic key on a path in Vault.
• The second strategy was to get all keys on a path in Vault by not specifying the secretKey. The absence of the secretKey field on an item in .spec.parameters.objects tells Vault to return all secrets on the secretPath as a JSON object.

container

# Shell session in the container

# Command
ls /mnt/secrets-store/database
# Output
password  username

# Command
ls /mnt/secrets-store/db-creds
# Output
password  username

kubectl get secrets

# Output
NAME       TYPE     DATA   AGE
db-creds   Opaque   2      111s

Addtional Notes

• Please let me know if there is anything that I can make more clear. I tried to leave as much detail as possible.
• If these changes are going in the right direction, then I can continue to add the necessary testing and comments to the code.

Todos

• squashed commits
• includes documentation
• adds unit tests

[k8s-ci-robot]

Hi @manedurphy. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[manedurphy]

@fubar @tam7t @aramase

[tam7t]

My major concern here is that this PR adds this great templating/translation feature but only in the context of secret syncing.

I would like to see this totally separate from secret syncing as this provides useful transforms for our main usecase of writing secrets to files.

Re-writing this as something like TransformOptions then performing the transform between the RPC response from the provider but before the data is written to with the AtomicWriter will make this feature useful in many more usecases.

[tam7t]

Between client.Mount() RPC to the provider and the fileutil.WritePayloads

Is I think the idea spot to inject the concept of a "transform" where 1 file can map to multiple files based on json path or other future format extractors.

If we are able to have this concept of a 'transform' directly in the SPC then it could be useful for other issues such as: #948

[manedurphy]

I don't think I am understanding the changes you are looking for @tam7t. My understanding of the issue that you've linked (#948) is that the user wants the ability to hardcode secret k/v pairs directly in secretObjects. The functionality I am adding with this feature is meant for targeting a set of secrets when the mounted content is a json object with multiple k/v pairs. Can you provide an example to demonstrate the idea behind the concept of a "transform" where 1 file can map to multiple files based on json path or other future format extractors?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: manedurphy
Once this PR has been reviewed and has the lgtm label, please assign tam7t for approval by writing /assign @tam7t in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[manedurphy]

@aramase @tam7t The example below shows the SecretProviderClass with the recommended TransformOptions. With this implementation, the driver knows that the provider will be returning a json response. When it receives that response from the provider, it extracts the secrets on the $.data.data jsonPath, and mounts each of them to their own file. The file is mounted as <objectName>/<jsonKey>. In this case, Vault returns two secrets, username and password, on the $.data.data path, and each is mounted as db-creds/username and db-creds/password. For the sake of verbosity, I've also mounted each of those secrets the way we normally would now, and configured the driver to recognize them as plaintext secrets.

SecretProviderClass

• Here we can see that we are asking for individual secrets as well as a full json response depending on whether the secretKey is set in the objects list.

apiVersion: secrets-store.csi.x-k8s.io/v1
kind: SecretProviderClass
metadata:
  name: vault-spc
spec:
  provider: vault
  parameters:
    roleName: "csi"
    vaultAddress: "http://vault.vault:8200"
    objects: |
      - secretPath: "secret/data/database"
        objectName: "database/username"
        secretKey: "username"
      - secretPath: "secret/data/database"
        objectName: "database/password"
        secretKey: "password"
      - secretPath: "secret/data/database"
        objectName: "db-creds"
  secretObjects:
    - secretName: db-creds
      type: Opaque
      data:
        - objectName: db-creds/username
          key: username
        - objectName: db-creds/password
          key: password
  transformOptions:
    format: json
    jsonPath: $.data.data
    secrets:
      - objectName: database/username
        format: plaintext
      - objectName: database/password
        format: plaintext

Mount

• Here we can see that we achieve the same effect for getting secrets individually as well as getting full json responses.

# /mnt/secrets-store
ls -l

# output
total 0
lrwxrwxrwx    1 root     root            15 Jul 31 20:05 database -> ..data/database
lrwxrwxrwx    1 root     root            15 Jul 31 20:05 db-creds -> ..data/db-creds

ls -l database/

# output
total 8
-rw-r--r--    1 root     root            10 Jul 31 20:08 password
-rw-r--r--    1 root     root             7 Jul 31 20:08 username

ls -l db-creds/

# output
total 8
-rw-r--r--    1 root     root            10 Jul 31 20:08 password
-rw-r--r--    1 root     root             7 Jul 31 20:08 username

[tam7t]

JSONPath instead of JsonPath per https://github.com/golang/go/wiki/CodeReviewComments#initialisms

[tam7t]

JSONPath

[tam7t]

We don't follow this very well in a lot of the code but per https://github.com/golang/go/wiki/CodeReviewComments#comment-sentences comments should be full sentences. Don't try to fix the other stuff in this PR but would be good to try to apply to the comments added here.

[tam7t]

I don't really understand what this represents and it doesn't seem like the test cases cover where TransformOptions.Secrets is populated?

I see the example in #963 (comment) but I'm not clear why the secret: part there is needed.

[tam7t]

I think we actually want the ProviderFilePath or InFilePath here to note the input data for the translation. I could see a single SecretProviderClass that has a number of different secrets that you want to split out in different ways.

[tam7t]

I think we want this to be a list like []*TransformOptions so that multiple transforms can be done on the secrets before presenting in the pod filesystem.

[tam7t]

I think we can actually omit this just determine behavior based on whether JsonPath is populated.

[tam7t]

I notice from the implementation that it expects the JsonPath to resolve to string key's that become filenames. I think this comment should be explicit about that behavior.

Additionally I think it might be nice to support extracting a single value if the path resolves to a string instead of a json object. It may be a bit more verbose but I think more explicit to support resolving to a json value and adding an OutFilePath parameter. Consider a response from the provider that says file my_secret has value {"username":"admin", "password": "hunter2"}

You could then write an SPC like:

apiVersion: secrets-store.csi.x-k8s.io/v1
kind: SecretProviderClass
metadata:
  name: app-secrets
spec:
  provider: vault
  parameters:
    objects: |
      - objectName: "my_secret"
        secretPath: "secret/data/my-kv"
        secretKey: "blob"
  transforms:
    - inFilePath: "my_secret"
      jsonPath: "$.username"
      outFilePath: "username"
    - inFilePath: "my_secret"
      jsonPath: "$.password"
      outFilePath: "password"

Later we could add a transform like the this to resolve #948

  transforms:
    - staticValue: "admin"
      outFilePath: "username"

Where we could generate a file based just on SPC data for non-secret static value admin to file username.

[manedurphy]

commenting to acknowledge feedback. haven't had much time in the last few weeks. will try to revisit this as soon as i can.

[enj]

Curious if we have any desire to support CEL here (maybe instead of JSON path). CEL is used by Kubernetes in a few places now, and the list is growing.

[vitordeap]

Hello guys! Thanks for your collaboration with this! I'm really excited about this feature, it will save a lot of work for my team with a more reliable process :)

[esparta86]

hey team, that feature will be perfect for our integrations. example rabbitmq and prometheus. to have sync with our vault

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[zentavr]

When would this PR accepted?

[aramase]

When would this PR accepted?

This PR needs rebase and comments addressed. @tam7t is working on a POC that might help accomplish this.,

I'm going to close this PR due to inactivity.

/close

[k8s-ci-robot]

@aramase: Closed this PR.

In response to this:

When would this PR accepted?

This PR needs rebase and comments addressed. @tam7t is working on a POC that might help accomplish this.,

I'm going to close this PR due to inactivity.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bencat-sixense]

Hello @tam7t, did you have the time to poc this ?
Thanks :)