Pass non-driver volumeAttributes to the created CertificateRequest

[Cisien]

This change provides alignment between the csi-driver and the Certificate resource by passing non-standard volumeAttributes defined on the csi volume through to the generated CertificateRequest. This behavior exists when cert-manager consumes the Certificate resource, allowing custom values to be passed on to the issuer that is signing the certificate.

In my use case, I depend on several values that need to be forwarded to the signing service. I have implemented this in the external issuer as annotations on the CertificateRequest.

A (sanitized) example of a Certificate resource:

apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: my-cert
  namespace: resource-namespace
  annotations:
    external.issuer.group.name/serviceId: 72d780c0-4c9d-4fbc-a54b-ad88f120e4c4
    external.issuer.group.name/teamName: TestTeam
    external.issuer.group.name/serviceName: TestService
    external.issuer.group.name/environment: DEV
spec:
  secretName: my-cert
  duration: 8760h # 1 year
  renewBefore: 8759h # 1 year -1h for testing
  commonName: this-field-is-intentionally-left-blank
  privateKey:
    rotationPolicy: Always
    algorithm: RSA
    encoding: PKCS1
    size: 2048
  issuerRef:
    name: internal-custom-issuer
    kind: CustomIssuer
    group: external.issuer.group.name

With this change, I will be able to define the same CertificateRequest using the following volume spec

        - name: another-bp-cert-csi-driver
          csi:
            readOnly: true
            driver: csi.cert-manager.io
            volumeAttributes:
              csi.cert-manager.io/issuer-name: internal-custom-issuer
              csi.cert-manager.io/issuer-kind: CustomIssuer
              csi.cert-manager.io/issuer-group: external.issuer.group.name
              csi.cert-manager.io/common-name: not-used
              csi.cert-manager.io/duration: 8760h
              csi.cert-manager.io/renew-before: 8759h
              external.issuer.group.name/serviceId: 12d780c0-4c9d-4fbc-a54b-ad88f120e4c7
              external.issuer.group.name/teamName: TestTeam1
              external.issuer.group.name/serviceName: TestService1
              external.issuer.group.name/environment: DEV

[jetstack-bot]

Hi @Cisien. Thanks for your PR.

I'm waiting for a cert-manager 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.

[ThatsMrTalbot]

Hi @Cisien, thanks for your contribution!

This all looks pretty good to me, especially since the code change itself is so small.

/ok-to-test

[Cisien]

unit-tests currently fail because annotations are being returned. Unless there's a reason not to, i'll update this test to not fail based on annotations not being nil

Edit: Instead i'll update the logic to exclude the "csi.storage.k8s.io" group name

[Cisien]

I have updated the tests to expect annotations to no longer be nil, and excluded the csi.storage.k8s.io group from being included in the annotations added to the CertificateRequest.

/retest

[Cisien]

the test failure of the e2e test appears to be unrelated to my change.

{"component":"entrypoint","file":"k8s.io/test-infra/prow/entrypoint/run.go:173","func":"k8s.io/test-infra/prow/entrypoint.Options.ExecuteProcess","level":"error","msg":"Entrypoint received interrupt: terminated","severity":"error","time":"2024-03-07T18:44:07Z"}

[Cisien]

requesting a re-test in case it was simply transient.
/retest

[Cisien]

@ThatsMrTalbot Tests are passing again. Are you okay with the most recent change?

[ThatsMrTalbot]

Tests look good, I've also tried it locally and it all works as expected

/lgtm

[Cisien]

For the purpose of my planning, what is a reasonable timeline to expect for getting this merged and into a release?

[SgtCoDFish]

/lgtm
/approve

Thansk for raising this and thank for the reviews!

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SgtCoDFish

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [SgtCoDFish]

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

[ThatsMrTalbot]

This was released with https://github.com/cert-manager/csi-driver/releases/tag/v0.8.0