fix: different tls secret per ingress

[yelhouti]

Signed-off-by: Youssef El Houti youssef.elhouti@gmail.com

For reviewers

The goal here is for lets-encrypt to automatically generate certificates per ingress and avoid having to handle that manually, specially if your cluster already have issuers installed.
This is done by editing the jx-requirements and adding

  ingress:
    annotations:
      cert-manager.io/cluster-issuer: letsencrypt-prod

in env/jxboot-resources/values.tmpl.yaml (or editing env/values.tmpl.yaml with the extra key)
This would work for both clusters with already installed cert-managers and new ones with cert-managers and issuers created by jenkins-x

fixes #31 , jenkins-x/jx#5310

[jenkins-x-bot]

Hi @yelhouti. Thanks for your PR.

I'm waiting for a jenkins-x-charts or jenkins-x 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.

[yelhouti]

/assign @jstrachan

[abayer]

cc @daveconde @dgozalo @ccojocar since I don't feel qualified to make changes to TLS behavior. =)

[yelhouti]

@jstrachan could you review this please, it helps with use case where not global certificate is possible (one per ingress) and keeps previous behavior working. please :)

[yelhouti]

/assign @jstrachan

[gazal-k]

within the similar https://github.com/jenkins-x-charts/jxboot-helmfile-resources/ what they have done is to use a common secret; https://github.com/jenkins-x-charts/jxboot-helmfile-resources/blob/master/jxboot-helmfile-resources/values.yaml#L1573. And that secret is expected to be for a wildcard certificate.

[yelhouti]

@gazal-k thank you very much for getting back to me, this is the same thing we have here, unfortunately this is not an option for me, as I can't do dns validation (required fro wildcard), and not everyone can. so this my only option. I can update the PR have both: if a secretname is given, use it, else use one certificate per ingress, what do you think?

[gazal-k]

Yes, making it more configurable would be ideal.

PS: I'm not part of the JX team

[daveconde]

@yelhouti I think making this more configurable as @gazal-k suggested is a good approach for this one, I like the idea of being able to specifically specify the secret used.

[yelhouti]

@daveconde would you merge it if:
1- tries to read first the secretName from values.yaml per ingress (ex: hook.ingress.secretName, nexus.ingress.secretName...)
2- if empty or missing uses the global one from cluster.ingress.secretName
3- else fallsback to -p or -s using the certmanager.production
EDIT: .ingress.tls.secretName instead of .ingress.secretName

[daveconde]

That sounds sensible to me, @garethjevans would you have any concerns with this behaviour?

[garethjevans]

Hi @daveconde, @yelhouti i think that suggestion should work. It needs to handle the case for a global certificate, individual certificates and user provided certificates (eventually), am not sure what the best order for resolution would be, but as long as we don't break compatibility that would be fine.

[daveconde]

@yelhouti Could you please also open a docs PR that explains the expected behaviour?

[yelhouti]

@daveconde PR updated, as @garethjevans mentionned it should ne break backward compatibility. working on the doc right away (and on jenkins-x-charts/jxboot-helmfile-resources#151)

[daveconde]

/lgtm

[daveconde]

/ok-to-test

[yelhouti]

are there any automated tests for this?
I think it just needs a lgtm

[daveconde]

/lgtm

[daveconde]

/lgtm

[jenkins-x-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: daveconde

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

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

[daveconde]

/ok-to-test