`start_arg_parser` reads like a helper that mutates a parser, when it
actually builds the argument parser for the start/restart-server
scripts. Rename it for consistency with the sibling
`upgrade_script_arg_parser` helper.
The wrapper accepts a `refname` plus its own `--remote-url`/`--local-ref`
flags and quietly forwards anything else through to upgrade-zulip-stage-3.
That meant `upgrade-zulip-from-git --help` advertised none of the
operationally important options (`--skip-restart`, `--skip-puppet`,
`--audit-fts-indexes`, etc.), and admins had to read the stage-3 source
to discover them.
Extract stage-3's forwardable options into a shared
`upgrade_script_arg_parser` helper in zulip_tools, used as a parent
parser in both stage-3 and the git wrapper. The wrapper now parses these
options itself (so they appear in `--help`) and reconstructs them when
invoking stage-2.
When the destination of an extracted file is a symlink to a
directory (e.g. in the docker-zulip container, where
/home/zulip/uploads -> /data/uploads), modern GNU tar replaces the
symlink with a real directory before extracting through it, as a
security precaution. As a result, uploads (and potentially
configuration files) get restored improper (and, in the case of
docker-zulip, non-persistent) paths.
Pass `--keep-directory-symlink` to both tar invocations so the
extractor follows symlinks-to-directories rather than replacing them.
The non-docker case is unaffected (these paths are real directories
in a standard install), and the security mitigation is not
meaningfully weakened: destinations are pinned to /etc/zulip,
/home/zulip/uploads, and zproject, and the archive is one the
operator just produced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Running `sudo scripts/stop-server --help` previously stopped the
server, since the script ignored sys.argv entirely. Add a no-arg
argparse parse_args() call before any side effects so --help prints
usage and unknown arguments are rejected.
The other server-control scripts (start-server, restart-server) are
already safe; both go through start_arg_parser(), which uses argparse.
Zulip production suite / ${{ matrix.name }} (zulip/ci:bookworm, --test-custom-db, Debian 12 production install with custom db name and user, bookworm) (push) Has been cancelled
Zulip production suite / ${{ matrix.name }} (zulip/ci:jammy, , Ubuntu 22.04 production install and PostgreSQL upgrade with pgroonga, jammy) (push) Has been cancelled
The previous commit makes Puppet create the local uploads directory
on install, but the upgrade flow runs `manage.py check --database
default` in stage-3 before applying Puppet. Servers upgrading from
images that lack the directory would fail check_uploads_settings
before Puppet got a chance to create it.
Mirror Puppet's behavior here: only create the directory if it
isn't already present, so symlinks pointing to alternate storage
are left alone.
Some networks may block access to the host -- put a timeout on it, so
it doesn't hang forever, and log what's happening so folks have a
sense of what may be wrong.
See #38284.
Fedora 38 reached end of life upstream; replace it with Fedora 43,
which ships dnf5 and a recent enough groonga to skip the source
build. Concretely, to make `tools/provision` run cleanly on
Fedora 43:
* dnf5 (Fedora 41+) dropped the `groupinstall` alias and no longer
resolves the "Development Tools" display name, so switch the
Fedora branch of `setup-yum-repo` to
`dnf group install development-tools` (by group ID). Move the
existing CentOS/RHEL `groupinstall` calls into their own branches
so they keep working on yum/dnf4.
* Map Fedora 43 to PostgreSQL 17, and add `groonga-devel`,
`xxhash-devel`, `meson`, and `redhat-rpm-config` to the Fedora
package list. PGroonga 4.0.6 switched its build system to meson;
on Fedora, PostgreSQL's `pg_config` exports CFLAGS containing
`-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1` and
`-specs=.../redhat-annobin-cc1`, and without `redhat-rpm-config`
the PGroonga build fails with "cannot read spec file".
`xxhash-devel` is required because `xxhash-libs` is often pulled
in transitively (by blosc2, pyarrow, etc.) without its headers,
in which case meson detects libxxhash via pkg-config and skips
the vendored fallback that would otherwise build it from source.
* Use the packaged groonga from Fedora's main repo (15.0.4 on
Fedora 43, satisfying PGroonga 4.0.6's >= 14.1.0 requirement)
instead of compiling groonga from source. This removes the only
remaining caller of `scripts/lib/build-groonga` and the
BUILD_GROONGA_FROM_SOURCE plumbing in provision.py, which the
prior Fedora 38 path was the only user of.
Verified end-to-end on a Fedora 43 host and in a fresh fedora:43
podman container: setup-yum-repo, the package install, and the
PGroonga source build against the system groonga all complete
cleanly.
This is the default, but if we ever want to switch to uv’s managed
Python later, it will be necessary to have this earlier to stop uv
from keeping it after checking out an earlier branch.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
PostgreSQL's `search_path` sets the order in which schemas are
searched, and the first one is the one in which unqualified objects
are created[^1].
Lower in the file, we create an explicit `zulip` schema -- so we
should not use the database name in the `search_path`. Switch to the
correct constant `zulip`.
[^1]: https://www.postgresql.org/docs/current/ddl-schemas.html#DDL-SCHEMAS-PATH
Rather than require a configuration option, detect the `tsearch_data`
error, in conjunction with a remote PostgreSQL host, and warn and
continue. It is pretty much impossible to arrive at that combination
and then _not_ want to try again with the flag, so we should handle
that degradation gracefully, instead of requiring an explicit
install and configuration flag.
`certbot certonly`, when obtaining a new certificate, does not
run deploy hooks that were installed into
`/etc/letsencrypt/renewal-hooks` (certbot/certbot#9978). Because of
this, we added an explicit `service nginx reload`.
This turns out to be awkward for a few reasons -- firstly, this does
not handle the restarting of our SMTP server, which also needs to pick
up the new certificates. It requires that Docker installs explicitly
restart nginx their own way. And it also means that if certbot
already had a certificate, we reload nginx twice.
We would ideally just have certbot just run all of the deploy hooks,
as usual, no matter if the certificate is new or a renewal; however,
it has no flag to do so.
We move to approximating running the deploy hooks, if the certificate
is new. This makes it easier to have identical behaviour between
initial install and later renewals.
This requires moving the symlinking step itself to a deploy hook, so
it can run before nginx and the email server are restarted. Our one
consumer of the `--skip-symlink` flag, Docker, can work around its
removal. The symlink step is a no-op if `$ZULIP_DOMAIN` is unset,
i.e. during automated renewals. This ensures that system that
installed certbot, but has since changed to manual certificate
management, does not have those certificates overridden next time
certbot renews itself.
The use of `--force-interactive` was because it "suppresses a
warning", which current certbot shows no sign of displaying without
the flag.
We remove `--force-interactive`, and instead add
`--keep-until-expiring` if we are non-interactive, to allow
`setup-certbot` to default to not unnecessarily renewing certificates.
When run interactively, it may still prompt if a not-near-expiry
certificate is to be force-renewed.
Zulip production suite / ${{ matrix.name }} (zulip/ci:bookworm, --test-custom-db, Debian 12 production install with custom db name and user, bookworm) (push) Has been cancelled
Zulip production suite / ${{ matrix.name }} (zulip/ci:jammy, , Ubuntu 22.04 production install and PostgreSQL upgrade with pgroonga, jammy) (push) Has been cancelled
restart-server has grown more complex since
efresh-sharding-and-restart was written -- use that complexity. For
instance, workers are stopped one-at-a-time, rather than requiring
that they all stop sequentially before being started again.
It also means that Django restarts will be rolling, which trades off
500s during login for realms which are moving, in exchange for leaving
other realms unaffected.
