Draft for a new feature to support Secrets as a type of parameters

[umi0410]

Related to #5506

Disclaimer

This is just a draft, and the purpose of this PR is to look for a mentor who could help me with detailed implementations. As the test code has not been modified, the tests will fail, and this PR lacks test code at moment.

Motivation

Currently, we can use parameters referring to ConfigMaps but cannot use parameters referring to Secrets.

Modifications

I added a new type of parameters that refer to Secrets based on the code for the case where a parameter refers to ConfigMaps.

Verification

1. Create a Secret which will be referred to by a Workflow.

kubectl create secret generic workflow-parameter --from-literal credential=THIS_IS_SAMPLE_CREDENTIAL

2. Create a Workflow whose parameter refers to the Secret.

❯ cat << EOF | argo submit --log -
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: secret-parameter-
spec:
  entrypoint: echo
  arguments:
    parameters:
    - name: credential
      valueFrom:
         secretKeyRef:
           name: workflow-parameter
           key: credential
  templates:
  - name: echo
    inputs:
      parameters:
      - name: credential
    container:
      image: alpine:3
      command: [echo]
      args: ["Parameter: {{inputs.parameters.credential}}"]
EOF

3. In the logs, you can see the parameter has been passed to the Workflow and its Pod. In this example, the value of the parameter is THIS_IS_SAMPLE_CREDENTIAL.

Name:                secret-parameter-vhbln
Namespace:           argo
ServiceAccount:      unset (will run with the default ServiceAccount)
Status:              Pending
Created:             Sun Jul 23 21:36:50 +0900 (now)
Progress:
Parameters:
secret-parameter-vhbln: time="2023-07-23T12:36:52.292Z" level=info msg="capturing logs" argo=true
secret-parameter-vhbln: Parameter: THIS_IS_SAMPLE_CREDENTIAL
secret-parameter-vhbln: time="2023-07-23T12:36:53.304Z" level=info msg="sub-process exited" argo=true error="<nil>"

Concerns

1. Credentials are not redacted properly.

For the current implementation, the plain value of secrets are shown in logs and Pod manifests as follows:

❯ kubectl logs secret-parameter-vhbln -c init | grep THIS_IS_SAMPLE_CREDENTIAL
time="2023-07-23T12:36:51.470Z" level=info msg="Executor initialized" deadline="2023-07-23 12:41:50 +0000 UTC" includeScriptOutput=false namespace=argo podName=secret-parameter-vhbln template="{\"name\":\"echo\",\"inputs\":{\"parameters\":[{\"name\":\"credential\",\"value\":\"THIS_IS_SAMPLE_CREDENTIAL\",\"valueFrom\":{}}]},\"outputs\":{},\"metadata\":{},\"container\":{\"name\":\"\",\"image\":\"alpine:3\",\"command\":[\"echo\"],\"args\":[\"Parameter: THIS_IS_SAMPLE_CREDENTIAL\"],\"resources\":{}},\"archiveLocation\":{\"archiveLogs\":true,\"s3\":{\"endpoint\":\"minio:9000\",\"bucket\":\"my-bucket\",\"insecure\":true,\"accessKeySecret\":{\"name\":\"my-minio-cred\",\"key\":\"accesskey\"},\"secretKeySecret\":{\"name\":\"my-minio-cred\",\"key\":\"secretkey\"},\"key\":\"secret-parameter-vhbln/secret-parameter-vhbln\"}}}" version="&Version{Version:untagged,BuildDate:2023-07-21T16:09:45Z,GitCommit:5b31ca18b306c4bb1c7c218a59cbc75dceb77fd9,GitTag:untagged,GitTreeState:clean,GoVersion:go1.20.4,Compiler:gc,Platform:linux/arm64,}"

❯ kubectl get pod secret-parameter-vhbln -o yaml | grep -C 3 THIS_IS_SAMPLE_CREDENTIAL
    - name: ARGO_CONTAINER_NAME
      value: wait
    - name: ARGO_TEMPLATE
      value: '{"name":"echo","inputs":{"parameters":[{"name":"credential","value":"THIS_IS_SAMPLE_CREDENTIAL","valueFrom":{"secretKeyRef":{"name":"workflow-parameter","key":"credential"}}}]},"outputs":{},"metadata":{},"container":{"name":"","image":"alpine:3","command":["echo"],"args":["Parameter:
        THIS_IS_SAMPLE_CREDENTIAL"],"resources":{}},"archiveLocation":{"archiveLogs":true,"s3":{"endpoint":"minio:9000","bucket":"my-bucket","insecure":true,"accessKeySecret":{"name":"my-minio-cred","key":"accesskey"},"secretKeySecret":{"name":"my-minio-cred","key":"secretkey"},"key":"secret-parameter-vhbln/secret-parameter-vhbln"}}}'
    - name: ARGO_NODE_ID
      value: secret-parameter-vhbln
    - name: ARGO_INCLUDE_SCRIPT_OUTPUT
--
      name: kube-api-access-m7zrp
      readOnly: true
  - args:
    - 'Parameter: THIS_IS_SAMPLE_CREDENTIAL'
    command:
    - /var/run/argo/argoexec
    - emissary
--
    - name: ARGO_CONTAINER_NAME
      value: main
    - name: ARGO_TEMPLATE
      value: '{"name":"echo","inputs":{"parameters":[{"name":"credential","value":"THIS_IS_SAMPLE_CREDENTIAL","valueFrom":{"secretKeyRef":{"name":"workflow-parameter","key":"credential"}}}]},"outputs":{},"metadata":{},"container":{"name":"","image":"alpine:3","command":["echo"],"args":["Parameter:
        THIS_IS_SAMPLE_CREDENTIAL"],"resources":{}},"archiveLocation":{"archiveLogs":true,"s3":{"endpoint":"minio:9000","bucket":"my-bucket","insecure":true,"accessKeySecret":{"name":"my-minio-cred","key":"accesskey"},"secretKeySecret":{"name":"my-minio-cred","key":"secretkey"},"key":"secret-parameter-vhbln/secret-parameter-vhbln"}}}'
    - name: ARGO_NODE_ID
      value: secret-parameter-vhbln
    - name: ARGO_INCLUDE_SCRIPT_OUTPUT
--
    - name: ARGO_CONTAINER_NAME
      value: init
    - name: ARGO_TEMPLATE
      value: '{"name":"echo","inputs":{"parameters":[{"name":"credential","value":"THIS_IS_SAMPLE_CREDENTIAL","valueFrom":{"secretKeyRef":{"name":"workflow-parameter","key":"credential"}}}]},"outputs":{},"metadata":{},"container":{"name":"","image":"alpine:3","command":["echo"],"args":["Parameter:
        THIS_IS_SAMPLE_CREDENTIAL"],"resources":{}},"archiveLocation":{"archiveLogs":true,"s3":{"endpoint":"minio:9000","bucket":"my-bucket","insecure":true,"accessKeySecret":{"name":"my-minio-cred","key":"accesskey"},"secretKeySecret":{"name":"my-minio-cred","key":"secretkey"},"key":"secret-parameter-vhbln/secret-parameter-vhbln"}}}'
    - name: ARGO_NODE_ID
      value: secret-parameter-vhbln
    - name: ARGO_INCLUDE_SCRIPT_OUTPUT

I have a concern about the current implementation not properly masking the actual value of credentials. However, making significant changes to how we pass arguments and log them would be required to fully redact these credentials.
Therefore, my suggestion is to introduce this implementation initially. If it's found insufficient in terms of redacting credentials, we can consider modifying the fundamental implementation of passing arguments. Ultimately, this will enable us to achieve proper redaction of credentials.

2. The interfaces of APIs and definitions of CRD will be updated.

Since I added a field secretKeyRef to parameters, the interfaces of APIs and definitions of CRD will be updated. I'd like to confirm if this is allowed and whether the CRD version should be increased.
From my research on a previous case involving the introduction of configMapKeyRef¹, I noticed that while the API and CRD were modified, the CRD version didn't increase.

Footnotes

1. https://github.com/argoproj/argo-workflows/pull/6662 ↩

[umi0410]

@terrytangyuan Could you please take a look at the current approach?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

[terrytangyuan]

Looks like this slipped through my inbox. Would you like to go through your proposal during the contributors' meeting? It's totally ok if not but you'll get feedback faster from others.

[umi0410]

@terrytangyuan Thanks for the comment! Unfortunately, I'm quite busy these days, so I don't think I have time to attend the contirubtor meeting. For now, I feel like focusing more on other simpler issues.

[stale]

This issue has been closed due to inactivity. Feel free to re-open if you still encounter this issue.

[noam-codefresh]

should probably mention secret and not configmap

[noam-codefresh]

is there any chance this PR can be revived? i think secretKeyRef can be very handy in different workflow templates. i'd be happy to try and make any required fixes or changes to it, if @umi0410 doesn't have time for it.

[terrytangyuan]

Feel free to pick it up.