Before, a fallback http server was created to handle requests for
unknown virtual hosts even when `HTTPS_METHOD=nohttp`. (In this case,
all http vhosts would be unknown.) Likewise, a catch-all fallback
https server was still created even if `HTTPS_METHOD=nohttps`.
Now the fallback servers are created only if needed. This brings the
behavior in line with the documentation and user expectation. It will
also make it easier to implement a planned feature: different servers
on different ports.
Before, if neither the vhost-specific cert nor `default.crt` existed,
nginx-proxy would not create the https vhost. This resulted in nginx
either refusing the connection or serving the wrong vhost depending on
whether there was another https vhost with a certificate.
Now nginx-proxy always creates an https server for a vhost, even if
the vhost-specific certificate and the default certificate are both
missing. When both certs are missing, nginx is given empty
certificate data to make it possible for it to start up without an
error. The empty certificate data causes the user to see a TLS error,
which is much easier to troubleshoot than a connection refused error
or serving the wrong vhost.
Add initial tests
Newlines
Remove unused variable
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
Change comment value
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
add missing services line
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
Use deploy.replicas
Remove details about choosing a load balancing method
Feedback note
Co-authored-by: Nicolas Duchon <nicolas.duchon@gmail.com>
This makes it possible to bring up different compose files for
different tests in the same test module.
This change does not negatively affect performance because the fixture
is a no-op if the docker compose filename is unchanged between tests.
Rationale for eliminating the check to see if the `DEBUG` environment
variable holds a true value:
* The `DEBUG` environment variable might be set on a container (for
purposes specific to that container, not `nginx-proxy`) to a value
that cannot be parsed as a bool, which would break `nginx-proxy`.
* It simplifies the template.
* It eliminates a cold code path.
* It avoids heisenbugs.
* It makes debugging easier for users.
Also delete the debug info tests, as they are fragile and they provide
limited value.
Alternatively, we could avoid collision with the container's use of
the `DEBUG` environment variable by using a container label [1] such
as `com.google.nginx-proxy.nginx-proxy.debug`. I think doing so has
dubious value, especially if we want to attempt backwards
compatibility with the `DEBUG` environment variable.
Fixes#2139
[1] https://docs.docker.com/engine/reference/commandline/run/#-set-metadata-on-container--l---label---label-file
Co-authored-by: Nicolas Duchon <nicolas.duchon@gmail.com>
If header values from a malicious client are passed to the backend
server unchecked and unchanged, the client may be able to subvert
security checks done by the backend server.
Move required files but 'nginx.tmpl' into a local 'app' folder and copy the
folder content into the image.
'nginx.tmpl' should be moved as well, but this is a breaking change for
configuration with a separate 'docker-gen' container.
This features allows the custom location blocks to be added to the
virtual path based routing. The custom config can be specified for each
container individually.
This commit removes the automatic path stripping and replaces it with a
user configurable environment variable. This can be set individually for
each container.
A test on raw IP addresses doesn't reach the existing IPv6 skip logic, added that to avoid a test failing when only IPv4 is available (eg: standard docker container networks).
Additionally some other tests set the `none` network and connecting to this fails as it's not allowed? Preventing that from happening resolves the final failing tests within containerized pytest.
The `network` object would never be in a list of network names (strings), and without `greedy=True` arg as the `docker-py` API docs note, the containers will not be part of the results, thus always returning an empty list which was not intended..
Now the network will properly match the current networks for pytest container, avoiding duplicate connect attempts, and the network list result will actually have containers to count when filtering by length.
When the container runs with host networking instead of the default bridge, the `$HOSTNAME` / `/etc/hostname` reflects that of the host instead of the container ID , which causes the pytest container to get removed accidentally.
Using a container name instead we can more reliably target the container to avoid removing it, should we need to run with host networking instead.
The original `/.dockerenv` approach is no longer valid, and context wise we're only using this for the test suite, so using an ENV in that container is a better solution.
This addition requires usage of `DEFAULT_HOST` on containers tested to ensure they don't accidentally use `web2` as their default fallback (due to no SNI / `-servername` requested in openssl queries), otherwise they would be testing against the incorrect DH params response.
They could alternatively request an FQDN explicitly as well, instead of relying on implicit fallback/default server selection behaviour.
---
`web2.nginx-proxy.tld.dhparam.pem` is a copy of `ffdhe2048.pem`.
- Use a DRY method instead.
- ENV test changed from 2048-bit to 3072-bit to avoid confusion in a future test that should not be mixed up accidentally with 2048-bit elsewhere.
- Custom DH file test comparison changed to match other comparisons for equality against the expected DH param content.
- Related comments revised, additional comment for context added by the test definition.
- Minor white-space adjustments.
Use YAML anchors for repeated values providing a single source of truth.
I would use `x-*` convention to store anchors above service containers, but this seems to require a compose config that defines the services (and version?) keys, which this test setup was failing to be compatible with for some reason..
- 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.
Check that when multiple containers have the same WIRTUAL_HOST and one of
them is unreachable, the resulting `upstream` block has no
`server 127.0.0.1 down;` entry.
