Add Venafi custom field support to cert-shim

[k0da]

Some setups have a requirement to have custom-field annotation set on Certificate object.

This commit adds support for scraping Venafi custom-fields annotation from ingressLike objects and pass them to Certificate object.

Pull Request Motivation

This PR adds support for Venafi custom-field annotation from ingLike resources

Kind

Feature

Release Note

NONE

[jetstack-bot]

Hi @k0da. 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.

[k0da]

/cc @inteon
HI, @inteon could you please review?

[k0da]

/cc @wallrj

[inteon]

/hold
I don't think we should be reading issuer-specific annotations for the certificate shim, what do you think @wallrj.
Ideally we have a more general solution for this.

[k0da]

@inteon I thought first to get all annotations as is. But then decided to narrow down a change to a specific usecase.

[inteon]

@inteon I thought first to get all annotations as is. But then decided to narrow down a change to a specific use case.

I think this is the best way forward, we would still have to filter the annotations (eg. only copy annotations with a specific prefix). It would be great if we could create something that aligns with our existing annotation-copying solutions (haven't had the time to investigate this yet).

[wallrj]

We discussed this PR in our Wednesday standup and my contributions to the discussion were:

1. I was initially opposed to adding more annotations to ingress-shim because we will end up having to create an annotation for every field of the Certificate, which is a maintenance burden. 1. Instead users with special Certificate requirements should not use ingress-shim and should pre-provision a Certificate resource with their required settings.
2. However I can see two arguments in favour of this change:
   1. Some platform engineers prefer Ingress and ingress-shim because they don't want application engineers to have to learn about and use the cert-manager custom resources directly.
   2. By configuring Ingress certificates using only annotations, the application manifests do not have a dependency on the cert-manager CRDs so applications and cert-manager can be installed in any order. Although @hawksight argued that cert-manager is necessarily part of the cluster platform and will usually be deployed in an earlier phase than the applications.
   3. I was conflating this PR with previous PRs which have introduced Ingress annotations for Certificate.Spec fields. This PR is about copying Certificate annotations, so I guess it seems reasonable that all the supported Certificate annotations can also be added to an Ingress and copied to the generated Certificate by ingress-shim.

@k0da Please also create a cert-manager website PR, documenting how this new feature will work and link it to this PR.

[k0da]

@wallrj Thanks for the review. Will do website change.

I have an implementation question though, should we take all annotations as is, like we do for Labels given Certificate is created out of IngLike object I feel natural to take them as is.

[inteon]

@wallrj Thanks for the review. Will do website change.

I have an implementation question though, should we take all annotations as is, like we do for Labels given Certificate is created out of IngLike object I feel natural to take them as is.

I'm not sure I understand your question.

[jetstack-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

[jetstack-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

[jetstack-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

[jetstack-bot]

@jetstack-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/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.

[inteon]

/remove-lifecycle rotten
/reopen

[jetstack-bot]

@inteon: Reopened this PR.

In response to this:

/remove-lifecycle rotten
/reopen

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.

[jetstack-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign irbekrm for approval. 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

[jetstack-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
/lifecycle stale

[hawksight]

/remove-lifecycle stale

[jetstack-bot]

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.