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

Merge challtestsrv package into Pebble #466

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

mcpherrinm
Copy link
Contributor

@mcpherrinm mcpherrinm commented May 24, 2024

This moves the challtestsrv package into the Pebble source tree.

All users of challtestsrv that I'm aware of also use Pebble, and having them in two repos means they're harder to maintain.

cpu and others added 27 commits December 5, 2018 12:27
Boulder has a nice handy [`challtestsrv` package and
command](https://github.com/letsencrypt/boulder/tree/9e39680e3f78c410e2d780a7badfe200a31698eb/test/challtestsrv)
used for integration tests. Its small and useful enough to live in its
own repo. This will make it easy for Boulder's load-generator to use the
common package and for Pebble's pebble-challtestsrv command to use it as
well.

The `challtestsrv` package is ported over from Boulder mostly-as is with
a few small improvements. Notably:

* The TLS-ALPN-01 and HTTPS HTTP-01 features were split into two
separate binds. This helps us preserve strict TLS-ALPN-01 challenge
responses while also supporting HTTP-01 -> HTTPS HTTP-01 redirects. (See
letsencrypt/boulder#3962)
* The "FAKE_DNS" env var is removed. Now there is a default IPv4 and
a default IPv6 address that can be set via the management API. These
default addresses are used for A/AAAA query responses when there is not
a more specific mock.
* Hardcoded Boulder specific mock DNS data is removed. In its place are
new management API functions for adding/removing mock A, AAAA, and CAA
records
* The output is less noisy now. The DNS server no longer prints a line
per reply.
Boulder has a nice handy [`challtestsrv` package and command][1] used for integration tests.
It's small, stand-alone, and useful enough to live in its own repo. This will make it
easy for Boulder's load-generator to use the common package and for Pebble's
pebble-challtestsrv command to use it as well.

The `challtestsrv` package is ported over from Boulder mostly-as is with a few
small improvements. Notably:

* The TLS-ALPN-01 and HTTPS HTTP-01 features were split into two separate binds.
  This helps us preserve strict TLS-ALPN-01 challenge responses while also supporting
  HTTP-01 -> HTTPS HTTP-01 redirects. (See letsencrypt/boulder#3962)
* The "FAKE_DNS" env var is removed. Now there is a default IPv4 and a default
  IPv6 address that can be set via the management API. These default addresses are
  used for A/AAAA query responses when there is not a more specific mock.
* Hardcoded Boulder specific mock DNS data is removed. In its place are new
  management API functions for adding/removing mock A, AAAA, and CAA records
* The output is less noisy now. The DNS server no longer prints a line per reply.

[1]: https://github.com/letsencrypt/boulder/tree/9e39680e3f78c410e2d780a7badfe200a31698eb/test/challtestsrv
Fix some style/clarity things in the comments.

Also, remove the "if" guards around several delete statements. The
delete builtin is a no-op if the field doesn't exist, so we don't need
those guards.
Its useful for testing purposes to be able to find out what requests have been processed by the challenge test servers.

For example it may be useful to see that redirects were properly followed or that CAA tree climbing resulted in the expected DNS queries.
* fix: don't panic with unknown DNS question type.
* fix: typos.
* refactor: use dns.Fqdn
…ypt#5)

This PR breaks API compatibility by renaming the exported `IdPeAcmeIdentifier` field to `IDPeAcmeIdentifier`.

I'll cut a new major release the next time something beyond tidying up comes along.
- supports create/read/destroy and DNS lookup of CNAME record for hosts
- supports one level of "aliasing" by resolving CNAME in mock DNS server
* chore: update golangci-lint.
* chore: validate go modules and vendor.
* fix: go modules files.
* fix: vendor folder content.
Go's ListenAndServe methods return an error with the text "Server
close", even on a clean shutdown. We should suppress this spurious error
so it doesn't mask more meaningful ones.

Also, update to a more recent golangci-lint and exempt some lints in order
to fix the build.
This allows testing Boulder's code to log when old TLS versions are
used.
@aarongable
Copy link
Contributor

When this is fully in shape, let's do a little bit of grooming to preserve history:

  • Squash all commits after the merge commit into a single commit, force-push to this PR
  • Turn off "require linear history" in the repo settings
  • Merge as-is (neither squashing nor merging) so that we get two commits on pebble/main: the merge commit itself, and the fixups commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants