Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[stable/kube-lego] Adding options, fixing lint, bumping default version #2571

Merged
merged 3 commits into from
Nov 1, 2017

Conversation

venezia
Copy link
Contributor

@venezia venezia commented Oct 23, 2017

This PR does a couple of things

  • Makes clear it has support for multiple kube-legos running in a cluster by indicating there is a way to set LEGO_SUPPORTED_INGRESS_CLASS, LEGO_SUPPORTED_INGRESS_PROVIDER, and LEGO_DEFAULT_INGRESS_CLASS environmental variables.

These are needed in solutions for multiple ingress controllers Issue 233 and Issue 189

These variables have been listed on the project page's README.md - and while they are useful to an unreleased fix, they're still useful regardless

  • Fixes lint by having LEGO_EMAIL no longer commented out

  • Bumps default version of kube-lego to 0.1.5

I bumped the chart version to v0.2.0 to indicate the additional options that are available.

LMK if there's any problems - thanks

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 23, 2017
@k8s-ci-robot
Copy link
Contributor

Hi @venezia. Thanks for your PR.

I'm waiting for a kubernetes 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.

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

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 23, 2017
@mattfarina
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 24, 2017
@mattfarina
Copy link
Contributor

For anyone wanting to look at differences between the versions 0.1.4 and 0.1.5... jetstack/kube-lego@0.1.4...0.1.5

@mattfarina
Copy link
Contributor

/cc @mgoodness @jackzampolin

@k8s-ci-robot
Copy link
Contributor

@mattfarina: GitHub didn't allow me to request PR reviews from the following users: jackzampolin.

Note that only kubernetes members can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @mgoodness @jackzampolin

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.

Copy link
Contributor

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one little nit

@@ -5,6 +5,7 @@ config:
## Email address to use for registration with Let's Encrypt
##
# LEGO_EMAIL: my@email.tld
LEGO_EMAIL: foo@bar.com
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

foo@bar.com

Should use example.com instead of bar.com. example.com is spec'd for this while bar.com is a live domain.

Copy link
Contributor

@jackzampolin jackzampolin Oct 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is news to me! Also there is actually a foo@bar.com https://bar.com/ Maybe leave it as an easter egg?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See RFC 6761 for details. example in certain permutations is a reserved domain name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to foo@example.com - thanks!

@mattfarina mattfarina merged commit ffbe16c into helm:master Nov 1, 2017
@venezia venezia deleted the kube_lego_add_options branch November 1, 2017 23:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants