Anonymous volumes are discouraged for reliable persistence.
Users should use named volumes or bind mounts instead. Potentially breaking change, users can also use explicit anonymous volumes instead of relying on implicit anonymous volumes.
`nginx-proxy` really should not be creating implicit anonymous volumes as in most cases it is undesirable.
`git blame` reveals this was added in 2014 by jwilder, with a message that implies implicit anonymous volumes was never intended..
- Added clarification comment of the DIR command
- Quoted `ARGS` usage required wrapping `ARGS` assignment in an array to properly expand. This wasn't broken before, but is required change to keep ShellCheck lint happy.
- Quote wrapped `DIR` usage, the volume target had an extra `/` before the `DIR` which seems unnecessary as `pwd` should return absolute path.
- Expanded `docker run` options to long-form.
As this project isn't exactly python focused apart from the test suite, I'll assume other contributors are probably not as experienced with python either. Since this is a rather technical test, the extra comments should help grok the functionality without floundering around with the docs.
When the subprocess raises an exception due to an issue with the command (_eg using `-CAfile` arg to `openssl` with an invalid path_), the tests would output large walls of text that wasn't particularly helpful in troubleshooting the issue. `stderr` was also leaking out inbetween the test case results in the terminal, this has been resolved by ensuring that output is caught and piped, which keeps it available to python when an exception is raised. Identifying the actual error cause and location is now much nicer.
Updated the output to be plain string content instead of byte strings, this works fine :)
Adds back the ability to avoid using DH params, provided no file was explicitly supplied.
This used to be `DHPARAM_GENERATION=false`, the equivalent is now `DHPARAM_SKIP=1` (default 0). Previous name was no longer appropriate.
Ensures that if a user has explicitly provided their own dhparam file to still output a warning instead of the skip message, since `DHPARAM_SKIP=1` doesn't disable the support in nginx.
- `dhparam_generation` tests are no longer necessary, dropped.
Modified the remaining `dhparam` test to use multiple `nginx-proxy` images to verify correct behavior for different configs.
Tests now cover:
- Default (ffdhe4096) is used.
- Alternative via ENV (ffdhe2048) works correctly.
- Invalid group via ENV (1024-bit) fails.
- Custom DH params provided via file mount works with warning emitted.
---
- `assert_log_contains`: added a `container_name` arg with `nginxproxy` as the default value. This allows multiple nginx-proxy containers to utilize this method instead.
- Extracted out the `openssl` test (_to `negotiate_cipher()`_) and modified it to be a bit more flexible. It now takes a container with optional extra args to pass to `openssl` command called, as well as the `grep` string to match. This made the original test redundant, so I've dropped it.
- Added two methods to use `negotiate_cipher()`, one verifies a DHE cipher suite was negotiated and checks that a DH emphermal key was also mentioned in the output. The other method verifies the expectation of failing to negotiate a valid cipher if DH params have not been set, while verifying that non-DHE cipher suites can be successfully negotiated.
- Added a `get_env()` method for extracting attached environments on a container. This is useful for verifying invalid `DHPARAM_BITS` values (eg `1024`-bit).
- The original `Server Temp Key` assertion was incorrect, it was expecting a value that is unrelated to DHE cipher suite support (_`X25519` is related to ECDHE_). This is due to TLS 1.3 being negotiated where you cannot use custom DH params, nor influence the negotiated cipher due to this mechanism changing from TLS 1.3. TLS 1.3 does support DH params, but it internally negotiates RFC 7919 group between server and client instead. Thus to verify expectations, the connection via `openssl` is made explicitly with TLS 1.2 instead.
- `DHPARAM_FILE` is a local var not intended for overriding via ENV. Clarified that with `local` declaration.
- `FFDHE_GROUP` var uses default assignment (_`:=4096` instead of only substitute `:-4096`_), so that `DHPARAM_BITS` retains the default 4096 value in subsequent references if no custom size was provided.
- Refactored the conditional statements to only handle early failure conditions. Shifting out the RFC7919 support that can run after all checks have passed.
- Revised comments.
Implicit URL is unnecessary (_and presently relies on Github redirecting from it's original mapped URL_).
Use an explicit URL instead to reduce the guesswork/trust of where the shortener was going to redirect to.
- `==` for string equality since we're using bash `[[ test ]]` already.
- Uppercase `socket_file` variable to be consistent with other internal variables used in the script.
- Convert `[ test ]` to `[[ test ]]` for consistency, improving maintenance. Double-bracket (_not posix compatible_) does not require quoted variables, ShellCheck lint knows this is safe too :)
- `-z` test for `$RESOLVERS` is native syntax to check for empty string value.
- Referenced variables should generally be wrapped like so `"${VAR}"`.
- Variable assignments with string values should be double quotes for content with variables, otherwise use single quotes (_no interpolation_).
- Converted my if statements to use the same style used in the rest of the file.
- While the anonymous VOLUME can be dropped from Dockerfile, the path needs to be valid at run-time, might as well ensure it's available by creating the dhparam folder at build.
- Generation logic no longer necessary, dropped.
- Standardized RFC 7919 groups added (2048, 3072, 4096), with 4096-bit remaining the default size. The DH logic can live in the entrypoint script as well.
- Third-party supplied pre-generated DH params removed as they're not considered trustworthy compared to RFC 7919 groups.
