-
Notifications
You must be signed in to change notification settings - Fork 16.8k
[stable/traefik] adding support for traefik wildcard certificates #6015
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. |
Signed CLA |
/assign @krancour |
Added support for multiple sets of domains |
When testing this change locally this is a subset of the output to demonstrate it working as intended:
|
/ok-to-test |
/assign |
ping @krancour |
/assign @dtomcej |
@billimek: GitHub didn't allow me to assign the following users: dtomcej. Note that only kubernetes members and repo collaborators can be assigned. In response to this:
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. |
/assign @emilevauge |
@billimek: GitHub didn't allow me to assign the following users: emilevauge. Note that only kubernetes members and repo collaborators can be assigned. In response to this:
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. |
@unguiculus with @krancour unfortunately stepping away from Traefik review duties, do you know how to get another person in the OWNERS file assigned to this PR? |
/assign @dtomcej |
@ldez: GitHub didn't allow me to assign the following users: dtomcej. Note that only kubernetes members and repo collaborators can be assigned. In response to this:
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 have been trying! but I'm not part of the proper org it seems... |
@dtomcej apart from being in the owner's file, you also need to be in the Kubernetes github org. I hadn't realized that you and other Traefik maintainers were not. The process for requesting membership is here: https://github.com/kubernetes/community/blob/master/community-membership.md#requirements You can use me as a sponsor. If you know anyone else who's already a member, as them as well. |
Tested on my development kub8s -> Works like a charm 👏 👏 👏 👏 |
@dtomcej wondering if you got a chance to approve this? |
/lgtm |
@dtomcej: changing LGTM is restricted to assignees, and only kubernetes/charts repo collaborators may be assigned issues. In response to this:
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. |
enabled: false | ||
# List of sets of main and (optional) SANs to generate for | ||
# for wildcard certificates see https://docs.traefik.io/configuration/acme/#wildcard-domains | ||
domainsList: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the examples in this section should be commented out - this is the norm for examples and avoids you accidentally attempting to request certs for example domains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally some feedback, thank you @grugnog!
Changes pushed.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: billimek, dtomcej, unguiculus 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:
Approvers can indicate their approval by writing |
…labels * kubernetes/master: (411 commits) [stable/efs-provisioner] Chart for efs-provisioner, a Kubernetes external-storage provisioner (helm#3233) [stable/filebeat] filebeat fixes (helm#6332) [stable/stolon] Add support for priorityClasses (helm#6607) [stable/external-dns] Add support for priorityClasses (helm#6606) [stable/minio] Add support for priorityClasses (helm#6604) [stable/cluster-autoscaler] Add support for priorityClasses (helm#6603) [stable/oauth2-proxy] Add support for priorityClasses (helm#6586) [stable/elasticsearch-exporter] add support for priorityClasses (helm#6584) [stable/traefik] adding support for traefik wildcard certificates (helm#6015) gcloud-sqlproxy: add tolerations and affinity to the Deployment (helm#6495) Review readme (helm#6399) [stable/mongodb-replicaset] Prometheus Metrics export (helm#6282) [stable/artifactory-ha] Typo fix: livessProbed->livenessProbed (helm#6462) [stable/artifactory] livessProbed->livenessProbed (helm#6461) [incubator/kube-spot-termination-notice-handler] Add Support for Tolerations (helm#5813) [stable/kanister-operator] RBAC changes and kanister profile creation (helm#6280) fix redis-ha NOTE.txt, stable/redis-ha don't create master-0 pod (helm#6131) [stable/concourse] add support for custom envvars for the web containers (helm#6441) upgrade to latest prometheus release 2.3.2 and alertmanager 0.15.1 (helm#6623) cert-manager: fast-forward to upstream 777ce6f4 (helm#6625) ...
stable/traefik/README.md
Outdated
@@ -122,8 +122,9 @@ The following table lists the configurable parameters of the Traefik chart and t | |||
| `acme.staging` | Whether to get certs from Let's Encrypt's staging environment | `true` | | |||
| `acme.logging` | Display debug log messages from the ACME client library | `false` | | |||
| `acme.domains.enabled` | Enable certificate creation by default for specific domain | `false` | | |||
| `acme.domains.main` | Main domain name of the generated certificate | `*.example.com` | | |||
| `acme.domains.sans` | List of alternative subject name to give to the certificate | `[]` | | |||
| `acme.domains.domainList` | List of domains & (optional) subject names | `[]` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@billimek Does it suppose to be acme.domains.domainsList
? Missing s
in the Readme
…lm#6015) * adding support for traefik wildcard certificates * adding support for multiple sets of domains * making values sans example more readable * bumping version of Chart * commenting domain examples from values.yml Signed-off-by: Jakob Niggel <info@jakobniggel.de>
…lm#6015) * adding support for traefik wildcard certificates * adding support for multiple sets of domains * making values sans example more readable * bumping version of Chart * commenting domain examples from values.yml
What this PR does / why we need it: This PR is to include wildcard certificate support for the traefik chart. See https://docs.traefik.io/configuration/acme/#wildcard-domains for more details on this capability.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #5379Special notes for your reviewer: Based on the original work provided by @farfeduc