Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(certificateResolvers)!: 💥 🐛 use same syntax in Chart and in Traefik #1214

Merged
merged 15 commits into from
Oct 16, 2024
Merged

fix(certificateResolvers)!: 💥 🐛 use same syntax in Chart and in Traefik #1214

merged 15 commits into from
Oct 16, 2024

Conversation

mloiseleur
Copy link
Contributor

@mloiseleur mloiseleur commented Oct 10, 2024 •
edited
Loading

What does this PR do?

Align syntax used in this Chart and in Traefik for certificatesResolvers.

Before

certResolvers:
  myAcmeResolver:
    email: [email protected]
    dnsChallenge:
      provider: myProvider
      resolvers:
        - 1.1.1.1
        - 8.8.8.8
    tlsChallenge: true

After

certificatesResolvers:
  myAcmeResolver:
    acme: 
      email: [email protected]
      dnsChallenge:
        provider: myProvider
        resolvers:
          - 1.1.1.1
          - 8.8.8.8
      tlsChallenge: true

Motivation

Nowadays, Traefik Proxy and Traefik Hub supports various certificateResolvers, not only ACME.
Current implementation do not allow to configure tailscale or distributedAcme, for instance.

More

  • Yes, I updated the tests accordingly
  • Yes, I updated the schema accordingly
  • Yes, I ran make test and all the tests passed

@mloiseleur mloiseleur changed the title fix(certificateResolvers)\!: 💥 use same syntax in Chart and in Traefik fix(certificateResolvers)!: 💥 use same syntax in Chart and in Traefik Oct 10, 2024
@mloiseleur mloiseleur added the kind/breaking This PR introduce a breaking change label Oct 10, 2024
@mloiseleur mloiseleur changed the title fix(certificateResolvers)!: 💥 use same syntax in Chart and in Traefik fix(certificateResolvers)!: 💥 🐛 use same syntax in Chart and in Traefik Oct 11, 2024
darkweaver87
darkweaver87 reviewed Oct 11, 2024
View reviewed changes
EXAMPLES.md Outdated Show resolved Hide resolved
traefik/VALUES.md Outdated Show resolved Hide resolved
traefik/VALUES.md Outdated Show resolved Hide resolved
traefik/tests/pod-config_test.yaml
- contains:
path: spec.template.spec.containers[0].args
content: "--certificatesresolvers.my-resolver.tailscale=true"

Copy link
Contributor

Choose a reason for hiding this comment

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

It should fail with unsupported resolver + (optional) test that we don't configure it by default.

traefik/values.yaml Outdated Show resolved Hide resolved
traefik/values.schema.json Outdated Show resolved Hide resolved
traefik/templates/_podtemplate.tpl Outdated Show resolved Hide resolved
traefik/templates/_helpers.tpl Outdated Show resolved Hide resolved
darkweaver87
darkweaver87 approved these changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@darkweaver87 darkweaver87 left a comment

Choose a reason for hiding this comment

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

LGTU

@traefiker traefiker merged commit 016822d into traefik:master Oct 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darkweaver87 darkweaver87 darkweaver87 approved these changes

Assignees
No one assigned
Labels
kind/breaking This PR introduce a breaking change kind/bug/fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mloiseleur @darkweaver87 @traefiker