Allow acme boolean flag to enable lego behavior with default issuer

[fabn]

Signed-off-by: Fabio Napoleoni [email protected]

Description

Implement a --acme flag for cert-manager install subcommand that automates helm values

--set=ingressShim.defaultIssuerName=letsencrypt-prod \
--set=ingressShim.defaultIssuerKind=ClusterIssuer

and create a global ClusterIssuer that is used to generate letsencrypt certificates when an ingress is annotated with kubernetes.io/tls-acme: "true"

Let me know if you're interested and if I should change something to make this merged.

Motivation and Context

As explained in #271 it would be useful to have an app specific flag that implements kubernetes.io/tls-acme: "true" configuration.

I think is in the motivation of this project to have some shorthand to automate per cluster tasks that are tedious and error prone like the one above.

Also the implementation is completely optin.

How Has This Been Tested?

I built the binary locally and tested on my own cluster, I'm not so skilled with go (actually my first lines in go) thus I'm still not able to implement an automated test. If you give me some hint on how to do that I'll be glad to try.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I've read the CONTRIBUTION guide
• [x] I have signed-off my commits with git commit -s
• [ ] I have added tests to cover my changes.
• [x] All new and existing tests passed.

• [x] I have tested this on arm, or have added code to prevent deployment

[Waterdrips]

Hey, we like to discuss changes like this on issues, I know we had one to discuss the missing --set flag from cert-manager app. I think you also proposed adding the functionality in this PR but it didnt really get addressed.

I'm not sure of the implementation of this feature, we have some apps creating LE issuers (openfaas-ingress and docker-registry-ingress). They can install a staging or production issuer with different ingressClass annotations (traefik for example) and they have templated the YAML used to create the issuer, this lets us do the above modifications as well as not require using yaml from the internet (which may change between cli versions).

Additionally the --acme flag you have added is true by default, which is either a breaking change (if someone has an issuer called that already) or will start to install cluster-scoped resources into people's clusters without them knowing.

[fabn]

Good points, I'll try to answer:

• true by default was obviously a mistake, my proposed change should be optin. I'll change it
• for the yaml part I completely agree that that the proposed approach is not so clean and my first idea was to embed some kind of yaml template in the command, also because that will allow further customization (e.g. to register a letsencrypt email address in issuer) but as I told you my go skills are pretty limited. If you point me to the right direction I'll change it to use a template instead of downloading the issuer template from git repo.

[fabn]

I've update PR with suggested changes, let me know if they aree good.

[fabn]

@Waterdrips feedback?

[Waterdrips]

Looks good, can we add a --staging flag like we have for the other apps that create issuers?

Good for testing and not using your rate limit up.

[derek[bot]]

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

[fabn]

@Waterdrips done. I have a test failure but is unrelated to my changes. I think is because of openfaas new releasees.

[fabn]

@Waterdrips up

[fabn]

Done, please check it out.

[fabn]

Is still this valuable for you?

[fabn]

Up

[Shikachuu]

@fabn are you still intrested, to move the PR forward?

[fabn]

I used this fork on more than one cluster and it works as is. What should I do to get it merged?

[derek[bot]]

Thank you for your contribution. unfortunately, one or more of your commits are missing the required "Signed-off-by:" statement. Signing off is part of the Developer Certificate of Origin (DCO) which is used by this project.

Read the DCO and project contributing guide carefully, and amend your commits using the git CLI. Note that this does not require any cryptography, keys or special steps to be taken.

:bulb: Shall we fix this?

This will only take a few moments.

First, clone your fork and checkout this branch using the git CLI.

Next, set up your real name and email address:

git config --global user.name "Your Full Name" git config --global user.email "[email protected]"

Finally, run one of these commands to add the "Signed-off-by" line to your commits.

If you only have one commit so far then run: git commit --amend --signoff and then git push --force. If you have multiple commits, watch this video.

Check that the message has been added properly by running "git log".

[Shikachuu]

@fabn can you rebase your commits and sign-off your commit please?

[fabn]

@fabn can you rebase your commits and sign-off your commit please?

Done

[fabn]

Also fixed some compile error when rebased.

[Shikachuu]

LGTM