Commit graph

2 commits

Author SHA1 Message Date
Brennan Kinney 1b1877f025
refactor: letsencrypt implicit location discovery (#2525)
* chore: Extract letsencrypt logic into methods

This allows other scripts to share the functionality to discover the correct letsencrypt folder from the 3 possible locations (where specific order is important).

As these methods should now return a string value, the `return 1` after a panic is now dropped.

* chore: Update comments

The todo is resolved with this PR, `_setup_ssl` will be called by both cert conditional statements with purpose for each better documented to maintainers at the start of the logic block.

* refactor: Defer most logic to helper/ssl.sh

The loop is no longer required, extraction is delegated to `_setup_ssl` now.

For the change event prevention, we retrieve the relevant FQDN via the new helper method, beyond that it's just indentation diff.

`check-for-changes.sh` adjusted to allow locally scoped var declarations by wrapping a function. Presently no loop control flow is needed so this seems fine. Made it clear that `CHANGED` is local and `CHKSUM_FILE` is not.

Panic scope doesn't require `SSL_TYPE` for context, it's clearly`letsencrypt`.

* fix: Correctly match wildcard results

Now that the service configs are properly updated, when the services restart they will return a cert with the SAN `DNS:*.example.test`,  which is valid for `mail.example.test`, however the test function did not properly account for this in the regexp query.

Resolved by truncating the left-most DNS label from FQDN and adding a third check to match a returned wildcard DNS result.

Extracted out the common logic to create the regexp query and renamed the methods to communicate more clearly that they check the FQDN is supported, not necessarily explicitly listed by the cert.

* tests(letsencrypt): Enable remaining tests

These will now pass. Adjusted comments accordingly.

Added an additional test on a fake FQDN that should still be valid to a wildcard cert (SNI validation in a proper setup would reject the connection afterwards).

Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2022-04-18 22:52:50 +12:00
Brennan Kinney 7ca056852f
tests(feat): Complete rewrite of letsencrypt tests (#2286)
* chore: Normalize container setup

Easier to grok what is different between configurations.

- Container name usage replaced with variable
- Volumes defined earlier and redeclared when relevant (only real difference is `VOLUME_LETSENCRYPT`)
- Contextual comment about the `acme.json` copy.
- Quoting `SSL_TYPE`, `SSL_DOMAIN` and `-h` values for syntax highlighting.
- Moved `-t` and `${NAME}` to separate line.
- Consistent indentation.

* chore: DRY test logic

Extracts out repeated test logic into methods

* chore: Scope configs to individual test cases (1/3)

- Preparation step for shifting out the container configs to their own scoped test cases. Split into multiple commits to ease reviewing by diffs for this change.
- Re-arrange the hostname and domain configs to match the expected order of the new test cases.
- Shuffle the hostname and domainname grouped tests into tests per container config scope.
- Collapse the `acme.json` test cases into single test case.

* chore: Scope configs to individual test cases (2/3)

- Shifts the hostname and domainname container configs into their respective scoped test cases.
- Moving the `acme.json` container config produces a less favorable diff, so is deferred to a follow-up commit.
- Test cases updated to refer to their `${CONTAINER_NAME}` var instead of the hard-coded string name.

* chore: Scope configs to individual test cases (3/3)

Final commit to shift out the container configs.

- Common vars are exported in `setup_file()` for the test cases to use without needing to repeat the declaration in each test case.
- `teardown_file()` shifts container removal at end of scoped test case.

* chore: Adapt to `common_container_setup` template

- `CONTAINER_NAME` becomes `TEST_NAME` (`common.bash` helper via `init_with_defaults`).
- `docker run ...` and related configuration is now outsourced to the `common.bash` helper, only extra args that the default template does not cover are defined in the test case.
- `TARGET_DOMAIN`establishes the domain folder name for `/etc/letsencrypt/live`.
- `_should*` methods no longer manage a `CONTAINER_NAME` arg, instead using the `TEST_NAME` global that should be valid as test is run as a sequence of test cases.
- `PRIVATE_CONFIG` and the `private_config_path ...` are now using the global `TEST_TMP_CONFIG` initialized at the start of each test case, slightly different as not locally defined/scoped like `PRIVATE_CONFIG` would be within the test case, hence the explicit choice of a different name for context.

* chore: Minor tweaks

- Test case comment descriptions.
- DRY: `docker rm -f` lines moved to `teardown()`
- Use `wait_for_service` helper instead of checking the `changedetector` script itself is running.
- There is a startup delay before the `changedetector` begins monitoring, wait until it ready event is logged.
- Added a helper to query logs for a service (useful later).
- `/bin/sh` commands reduced to `sh`.
- Change the config check to match and compare output, not number of lines returned. Provides better failure output by bats to debug against.

* chore: Add more test functions for `acme.json`

This just extracts out existing logic from the test case to functions to make the test case itself more readable/terse.

* chore: Housekeeping

No changes, just moving logic around and grouping into inline functions, with some added comments.

* chore: Switch to `example.test` certs

This also required copying the source files to match the expected letsencrypt file structure expected in the test/container usage.

* chore: Delete `test/config/letsencrypt/`

No longer necessary, using the `example.test/` certs instead.

These letsencrypt certs weren't for the domains they were used for, and of course long expired.

* chore: Housekeeping

Add more maintainer comments, rename some functions.

* tests: Expand `acme.json` extraction coverage

Finally able to add more test coverage! :)

- Two new methods to validate expected success/failure of extraction for a given FQDN.
- Added an RSA test prior to the wildcard to test a renewal simulation (just with different cert type).
- Added extra method to make sure we're detecting multiple successful change events, not just a previous logged success (false positive).

* tests: Refactor the negotiate_tls functionality

Covers all ports (except POP) and correctly tests against expected verification status with new `example.test` certs.

The `FQDN` var will be put to use in a follow-up commit.

* tests: Verify the certs contain the expected FQDNs

* chore: Extract TLS test methods into a separate helper script

Can be useful for other TLS tests to utilize.

* chore: Housekeeping

* chore: Fix test typo

There was a mismatch between the output and expected output between these two files "find key for" and "find key & cert for". Changed to "find key and/or cert for" to make the warning more clear that it's issued for either or both failure conditions.

Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2021-11-16 16:00:16 +01:00