Fixes #10518.
`tools/pinning/current/repin.sh` is not run; only pytest version is
updated. This is because `pypinning` had a bunch of syntax changes that
seem simply but I believe should be in a separate PR, which I think
should be done after this to collect all repin changes.
As discussed further in #10518, these issues were caused by pytest's
internalization of pytest-subtest, which had several implementation
changes.
To fix these, we simply no longer use subtest in the failing tests. The
test in acme is now parametrized instead, and the tests in apache only
ever had a single parameter.
To use parametrization in the acme test, I converted `DNSTest` from
unittest to pytest style, which was pretty straightforward. The only
note there is that while it would be nice to make `ec_secp384r1_key` a
fixture, you [can't use fixtures in
parameters](https://github.com/pytest-dev/pytest/issues/349). You could
use requests, but that seemed less clear and messier, because then you'd
be checking the value of the parameter and only sometimes loading it.
Could also make it a global variable, but that didn't really seem
necessary, as it's only called twice. Happy to consider other options,
not strongly tied to this one, just seemed nicest to me.
This field is optional to maintain backwards compatibility. Note that
`AnnotatedChallenge` inherits from `jose.ImmutableMap`, which has a
[check in
__init__](4b74747670/src/josepy/util.py (L125-L131))
that all slots are provided. That check would not allow us to do a
backwards-compatible addition, so I implemented an `__init__` for each
of these subclasses that fills the fields without calling the parent
`__init__`, and so doesn't hit an error when `identifier` is absent.
I chose to use `acme.messages.Identifier` rather than
`certbot._internal.san.SAN` here because these are wrapped ACME types,
so they should use the ACME representation. Also, `AnnotatedChallenge`
is passed to plugins, so we need to pass a type that the plugins can
understand.
Additionally, `domain` is marked as deprecated.
Part of #10346
/cc @bmw, who noticed the issue with `AnnotatedChallenge`
[here](https://github.com/certbot/certbot/pull/10468#issuecomment-3403294394)
and provided additional feedback
[here](https://github.com/jsha/certbot/pull/2#issuecomment-3534895793).
Note that there's still some work to do to finish excising `domain`
assumptions from this portion of the code.
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
The Apache configuration `Include`d in automatically created
`[sitename]-le-ssl.conf` files was redefining the `vhost_combined`
`LogFormat`, but contrary to the comment before the redefinition, did
not include the virtual host server name in the log format. This is
particularly confusing because this redefinition is hard to find when
debugging logging issues, as log formats are not related to SSL/TLS
configuration, and the included configuration file is outside of
`/etc/apache2`.
Additionally, a `vhost_common` `LogFormat` was defined, but not used
anywhere.
The `LogFormat` directives were introduced in commit
68f85d9f1a. Several other directives that
do not directly pertain to configuring SSL/TLS were added in that
commit, and have gradually been removed over the years. This should be
the last such removal.
Delete the `LogFormat` directives from the Apache configuration files
(both old and current), and update the `ALL_SSL_OPTIONS_HASHES`.
Fixes#9769 File 'options-ssl-apache.conf' included in autocreated
'[sitename]-le-ssl.conf' has potentially problematic vhost_combined
LogFormat
This was causing oldest tests to fail on my mac, which has an open file
limit of 256. Locks were being released at exit, but there were more
than 256 tests being run at once. Holding onto the file descriptor for
temporary files was making us keep the files open.
I also removed unnecessary setUps and tearDowns in subclasses so that
this could be fixed in only one spot.
If you wanted to do any testing locally, I was throwing this in places:
```
import errno, os, resource
open_file_handles = []
for fd in range(resource.getrlimit(resource.RLIMIT_NOFILE)[0]):
try: os.fstat(fd)
except OSError as e:
if e.errno == errno.EBADF: continue
open_file_handles.append(fd)
print(f'location description: {len(open_file_handles)}')
```
this is another part of https://github.com/certbot/certbot/issues/10195
these changes were done automatically with the command:
```
ruff check --fix --extend-select UP006 --unsafe-fixes certbot-apache
```
Alternative implementation for #7908.
In this PR:
- set up ruff in CI (add to `tox.ini`, mark dep in `certbot/setup.py`)
- add a `ruff.toml` that ignores particularly annoying errors. I think
line length isn't actually necessary to set with this workflow since
we're not checking it but putting it there for future usage.
- either fix or ignore the rest of the errors that come with the default
linting configuration. fixed errors are mostly unused variables. ignored
are usually where we're doing weird import things for a specific reason.
Part of #10183
> Option 4. Stop updating old files with security improvements. If
people want to be on old software they can but then they're not getting
the nice new things. We can either warn or not warn if we see people
using them, either on certbot install (what, who's installing new
certbot on these machines), new cert, cert renewal, or certbot update.
The second two would require code changes, I'm pretty sure. I don't
think we should warn too often because that's how we get people to
silence all output. This is a little weird because we don't usually keep
around deprecated things. We could also warn loudly and see if people
complain. Or do some sort of brownout.
This PR warns every time certbot is run. We could make it run less often
(only when a new config file is installed, probably), but that's a more
extensive code change, and honestly I think it's probably fine? But I
can change it.
Another token of gratitude for a super useful tool and service.
More about codespell: https://github.com/codespell-project/codespell .
I personally introduced it to dozens if not hundreds of projects already
and so far only positive feedback.
CI workflow has 'permissions' set only to 'read' so also should be safe.
---------
Signed-off-by: Yaroslav O. Halchenko <debian@onerussian.com>
Fixes#10252.
See further discussion here: https://github.com/pypa/pip/issues/11457
We are doing option:
> Alternatively, enable the --use-pep517 pip option, possibly with
--no-build-isolation. The --use-pip517 flag will force pip to use the
modern mechanism for editable installs. --no-build-isolation may be
needed if your project has build-time requirements beyond setuptools and
wheel. By passing this flag, you are responsible for making sure your
environment already has the required dependencies to build your package.
Once the legacy mechanism is removed, --use-pep517 will have no effect
and will essentially be enabled by default in this context.
Major changes made here include:
- Add `--use-pep517` to use the modern mechanism, which will be the only
mechanism in future pip releases
- Change to `/src` layout to appease mypy, and because for editable
installs that really is the normal way these days.
- `cd acme && mkdir src && mv acme src/` etc.
- add `where='src'` argument to `find_packages` and add
`package_dir={'': 'src'},` in `setup.py`s
- update `MANIFEST.in` files with new path locations
- Update our many hardcoded filepaths
- Update `importlib-metadata` requirement to fix
double-plugin-entry-point problem in oldest tests
