Control cert-manager installation with a separate config value

[rkrzewski]

Fixes #971

[rkrzewski]

I'd appreciate a review, @brokenpip3

[brokenpip3]

hei @rkrzewski thanks a lot for the PR, I will do a proper review asap.

In the mean time I saw that you bumped the certmanager dependency, however the previous maintainers have decided to add the certmanager crds into the crds folder in the chart: https://github.com/jenkinsci/kubernetes-operator/blob/master/chart/jenkins-operator/crds/cert-manager.crds.yaml

I tried previously to fix that by using some helm hooks but then I was forced to work on more urgent stuff and I planned to take a look on that after the 0.9.0 (which I will finally restart to handle in these days): #810

If you want to take a look it will be great, otherwise even if we put your new condition to off the helm chart will try to deploy the old certamanager crds anyway

[sbandaru08]

Version: 0.8.0-beta.2
We have our own certs that gets pulled by adding tls.secretName: xxxx-tst-wildcard. i tried running Version: 0.8.0-beta.2 by setting the webhook.false but still i get issues not sure if your fix can fix this issue. can you help me with it

[rkrzewski]

If you want to take a look it will be great, otherwise even if we put your new condition to off the helm chart will try to deploy the old certamanager crds anyway

Sure I'll take look at it. @brokenpip3, do you think if the CRDs can be just wrapped in a conditional clause based on cert-manager.enabled value? Would that work?

@sbandaru08 if we sort out the above, it'll fix the problem you describe.

[sbandaru08]

Sure thank you

[brokenpip3]

Sure I'll take look at it. @brokenpip3, do you think if the CRDs can be just wrapped in a conditional clause based on cert-manager.enabled value? Would that work?

yes, but be aware that you can't have a go template inside the crds directory in helm, so you can't add guards, that's why I was trying with the helm hook

[rkrzewski]

@brokenpip3 oh, I didn't know that. I've read up on https://helm.sh/docs/chart_best_practices/custom_resource_definitions/ and maybe the "Method 2" would work? We could place the cert-manager CRDs into a separate sub-chart inside of jenkins-operator chart that use the same guard condition as cert-manager chart.

[brokenpip3]

@brokenpip3 oh, I didn't know that. I've read up on https://helm.sh/docs/chart_best_practices/custom_resource_definitions/ and maybe the "Method 2" would work? We could place the cert-manager CRDs into a separate sub-chart inside of jenkins-operator chart that use the same guard condition as cert-manager chart.

Indeed could be a good tentative :)

[rkrzewski]

@brokenpip3 I've moved the CRDs to separate subchart as discussed and it seems to work as expected:

helm template --include-crds --set cert-manager.enabled=false jenkins-operator .

generates manifests that include Jenkins CRD but none of cert-manager CRDs.

[brokenpip3]

This is great, ty. I will try to find the time to review this PR during this weekend

[sbandaru08]

@brokenpip3 Just wanted to know to see when can this change live ?

[brokenpip3]

@brokenpip3 Just wanted to know to see when can this change live ?

I need to add some helm tests to be sure that this change will not destroy the current upgrade/install flow from both with/without validation webhook. It could take a couple of days or a couple of weeks, will depend on my free time

[rkrzewski]

I need to add some helm tests
If this is something I could help you with, please let me know.

[brokenpip3]

Thanks for your contribution!