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.

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

liquidweb: detect zone automatically #2031

Merged
merged 25 commits into from Oct 14, 2023
Merged

liquidweb: detect zone automatically #2031

merged 25 commits into from Oct 14, 2023

Conversation

jakdept
Copy link
Contributor

@jakdept jakdept commented Oct 11, 2023

I work for LiquidWeb, and we'd like to get some updates into our provider in lego to simplify things a bit and change the API endpoint.

I've followed the guidelines at https://github.com/go-acme/lego/blob/2140e6befe2a46267c88c735a40f4791fe6036ab/CONTRIBUTING.md and will open an issue as well once I've got this open.

I'm skipping including make test cause of the long output, but it passes as well.

╰─> make fmt
make: Nothing to be done for `fmt'.
┬─{ jack:~/s/g/j/lego git:(refresh)
╰─> make checks
golangci-lint run
╰─> make build
BIN_OUTPUT: dist/lego
rm -rf dist/ builds/ cover.out
Version: a99af0b0e7dd2c9e01c62a129e21e1fad9732a56
go build -trimpath -ldflags '-X "main.version=a99af0b0e7dd2c9e01c62a129e21e1fad9732a56"' -o  dist/lego ./cmd/lego/

I'm guessing you'll squash my commits, which is fine if needed. Or I can submit another and squash them myself if needed.

@jakdept jakdept mentioned this pull request Oct 11, 2023
5 tasks
@ldez ldez self-requested a review October 11, 2023 10:13
@ldez
Copy link
Member

ldez commented Oct 11, 2023

Hello,

Why did you change the prefix?

@ldez
Copy link
Member

ldez commented Oct 11, 2023

All the PR should be rewritten to:

  • follow the same design as the other providers.
  • don't create regression (the exposed elements are a part of the lego API).

I will rewrite it.

@ldez ldez changed the title Update liquidweb provider liquidweb: use new API Oct 11, 2023
@ldez ldez changed the title liquidweb: use new API liquidweb: detect zone automatically Oct 11, 2023
@ldez ldez added this to the v4.15 milestone Oct 11, 2023
@jakdept
Copy link
Contributor Author

jakdept commented Oct 11, 2023

I changed the prefix so it matches our terraform provider and upcoming prefix for the CLI and other tooling. However, I changed it in a way so that the old prefix still works. I believe I have tests showing that as well.

I did not see anything specifying a standard around the prefix. If that's an issue, we can leave it as LIQUID_WEB_ and I can work around it, but it's going to differ from all other tooling we provide.

@ldez
Copy link
Member

ldez commented Oct 14, 2023

I will merge the PR without the LWAPI_.

I will extend the system I created to handle fallback env vars for the cloudflare provider to be usable in this context.

I will create a PR with the support of LWAPI_ with the right fallback system.

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez merged commit 8afdc9d into go-acme:master Oct 14, 2023
7 checks passed
pchanvallon pushed a commit to pchanvallon/lego that referenced this pull request Oct 18, 2023
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants