Allow config of http01 solver pod security context

[aidy]

This allows configuration of the http01 solver PodSecurityContext as
part of the Issuer specification.

Pull Request Motivation

#5295

Kind

/kind feature

Release Note

You can now configure the pod security context of HTTP-01 solver pods.

[jetstack-bot]

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

[aidy]

This is feels like a slightly clumsy way to allow this configuration, but I think it's the cleanest approach.

Other approaches involve radically altering the way in which the cm controller takes configuration, or providing configuration flags for all the possible PodSecurityContext options. I suspect that the default is fine for nearly all use-cases, and for the few cases where overriding the default is necessary, doing so as a native k8s construct is likely to be preferable.

If necessary, helm templates could be modified to enable more intuitive usage.

I couldn't spot any prior art in the e2e tests for http01 solvers, and it was a little beyond my available time to implement - but if I've just missed it, please do point me in the right direction.

[SgtCoDFish]

/ok-to-test

Thanks for raising this! I probably won't be able to review but hopefully someone will!

[irbekrm]

We do already have one mechanism how to configure some pod spec values (for Ingress solvers only at the moment) via issuer.spec.acme.solvers.http01.ingress.podTemplate.spec.
Do we specifically want to make this a flag as opposed to adding another config field to the issuer pod template to make this a global config option?
I'm a little concerned that it could be confusing for users that there would be config options for pod spec in two different places.

[aidy]

I'm a little concerned that it could be confusing for users that there would be config options for pod spec in two different places.

Maybe this isn't the right approach, but - Isn't that already the case? The options from buildDefaultPod all come from extraArgs, I think.

[irbekrm]

Maybe this isn't the right approach, but - Isn't that already the case? The options from buildDefaultPod all come from extraArgs, I think.

I think that the existing flags are an older approach of configuring the solver pod and the pod template was added later to avoid having to add more and more flags to configure various options, see #1097 but I will verify this, if that's the case, we probably want to document it somewhere

x-post https://kubernetes.slack.com/archives/CDEQJ0Q8M/p1660041935692369

[aidy]

I've looked a bit harder, and I think I agree - setting via the issuer spec is a cleaner and more intuitive approach. I'll have a look at reworking this.

[irbekrm]

I've looked a bit harder, and I think I agree - setting via the issuer spec is a cleaner and more intuitive approach. I'll have a look at reworking this.

Thanks @aidy and thanks for the work you've done already!

[jetstack-bot]

@aidy: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cert-manager-master-e2e-v1-27-upgrade	b52c5e9	link	true	/test pull-cert-manager-master-e2e-v1-27-upgrade
pull-cert-manager-master-e2e-v1-27	b52c5e9	link	true	/test pull-cert-manager-master-e2e-v1-27
pull-cert-manager-master-make-test	91bf83a	link	true	/test pull-cert-manager-master-make-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[gnd]

hello, any progress on this ?

[cert-manager-bot]

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

[cert-manager-prow]

@cert-manager-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.
/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-sigs/prow repository.

[phandox]

Hi @aidy @maelvls it looks like it slip through cracks. Can we get this reopened / merged? So much work was already done and we would really appreciate to have it in the upstream!

/reopen
/remove-lifecycle rotten

[cert-manager-prow]

@phandox: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

Hi @aidy @maelvls it looks like it slip through cracks. Can we get this reopened / merged? So much work was already done and we would really appreciate to have it in the upstream!

/reopen
/remove-lifecycle rotten

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-sigs/prow repository.

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maelvls

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 [maelvls]

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

[phandox]

Bump @aidy any chance for fixing the merge conflict? It would be highly appreciated as the PR is already approved 🙏 Thank you!

[aidy]

Will try to have a look this week

[phandox]

Hi @maelvls would you be able to give it LGTM label? conflicts seems to be fixed again by @aidy . Thank you! 🙏

[maelvls]

Hey, sorry about the delay! The PR seems all good. Well done @aidy!

/lgtm

[phandox]

Awesome @maelvls , only removing do-not-merge/hold label is missing from merging 🙏

[wallrj]

A beta-release is now available which contains the fix for this issue. Please test and feedback if you have time.

• https://github.com/cert-manager/cert-manager/releases/tag/v1.16.0-beta.0