-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release v2025.1 #881
base: master
Are you sure you want to change the base?
Release v2025.1 #881
Conversation
…ps (#879) Allow connections to e.g. https://maps.googleapis.com/$rpc/google.internal.maps.mapsjs (as seen in CSP reports).
If check-migrations fails with non-1 exit code, the server will continue to start. The existing check-migrations call and test for exit code 1 was originally introduced as a fix for #461. The change in this PR should be more robust than the existing approach, but `set -e` and `set -o pipefail` might provide even more safety in handling failures in this script. These latter options would also allow for complete removal of `make check-migrations` from production. There may also be other critical scripts in this repo which would benefit from similar changes.
This change brings consistency: all other executables in `files/**` are marked executable in git rather than `chmod`'d in `.dockerfile` files. The `chmod` call was originally introduced in #676 without discussion. There is a wider debate whether git can be trusted to manage file permissions, touched on at #830 (comment)
Progress towards #677 - this will eventually allow the GHCR upload job to depend on the outcome of tests previously run in CircleCI.
Noted while working on #882: `files/postgres/upgrade-postgres.sh` is not actually run with `/bin/bash -eu` as its shebang declares, but with `/bin/sh`. ----- This commit 1. fixes the shell used for `upgrade-postgres.sh` 2. introduces a test which would have detected this issue, and should prevent similar issues in future 3. fixes use of deprecated dockerfile syntax detected by the new test 4. pulls `shellcheck` invocation into a script file to allow easier use locally, and possible future extension ----- Docker build checks Currently in beta, "docker build checks" are best-practice lint checks for dockerfiles. See: https://docs.docker.com/build/checks/#check-a-build-without-building ----- Dockerfile fixes * `postgres-upgrade.dockerfile`: https://docs.docker.com/reference/build-checks/json-args-recommended/ * `postgres14.dockerfile`: https://docs.docker.com/go/dockerfile/rule/legacy-key-value-format/ ----- Related: #342
Allowing access to enketo's redis instances locally allows this docker-compose environment to be used to work on enketo-express as well as other central services: 1. run in this repo: make stop && make dev && docker compose stop enketo 2. run in `enketo` repo: yarn workspace enketo-express grunt develop
@alxndrsn, GitHub notes that CircleCI is failing here. We've gotten rid of CircleCI, right? Is there a config change that needs to happen in the GitHub settings for the repo? |
That sounds right, but I can't find where to (un)configure the check. Maybe it needs to be deactivated through the CircleCI UI? |
Hmm I think you might be right. If it were configured within GitHub, I'd expect to see it under branch protection rules, but I'm not seeing CircleCI there. I am seeing an option within CircleCI to "Stop Building", maybe that's it. But I think we should probably hold off on that until we're closer to the release of v2025.1. If we were to put out a patch release before then, we'd still rely on CircleCI for that, so I don't think we should turn off CircleCI completely until we're sure that we're done with patch releases. |
In commit b2a9d4f, the directory name containing the nginx tests was changed from `test` to `nginx`. `docker compose` uses the parent directory name as the default name for containers, so this change broke log output which refers to specific docker logs by container name. The approach in this commit should be more robust as it uses the docker compose service names instead of directory-derived container names.
Whitespace changes are often introduced inadvertently due to different editor configurations. They then add noise to PRs, and if committed, add noise to future diffs and the git log. This PR adds checks for the following in `.sh` files: * trailing spaces on lines * trailing newlines in files * tabs vs spaces
In some circumstances, `npx` will transparently install packages when asked to run a command. From https://docs.npmjs.com/cli/v8/commands/npx#description: > If any requested packages are not present in the local project dependencies, then they are installed to a folder in the npm cache, which is added to the PATH environment variable in the executed process. A prompt is printed (which can be suppressed by providing either `--yes` or `--no`). Ref vuejs/vue-cli@f36a4ed
* use bash everywhere, instead of a mixture of `sh`, `bash`, and (implicitly) `dash` * for consistency, always set flags/options: * `-e`: "Exit immediately if a pipeline...returns a non-zero status." * `-u`: "Treat unset variables...as an error" * `set -o pipefail`: "the return value of a pipeline is the value of the last (rightmost) command to exit with a non-zero status" * `shopt -s inherit_errexit`: "command substitution inherits the value of the errexit [`-e`] option" See: https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#The-Set-Builtin See: https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#The-Shopt-Builtin In the service container's `start-odk.sh` script, this also removes the need for the explicit `make check-migrations` check, as all steps in scripts are now checked implicitly. Closes #341 Closes #342
Approach suggested in #818 (comment) ----- For nginx config, a new approach is implemented with mawk. This is similar to envsubst, but more ergonomic: * no need to explicitly list all variables * throw error on missing variables * do not replace nginx vars like $host, $request_uri with empty strings (in contrast to envsubst when executed without an explicit variable list) There are a couple of changes which may break existing deployments: * changing client-config.json.template * requiring all substituted variable to be defined Closes #473
This PR prepares the release of v2025.1. It should only contain changes from other PRs that have already been approved and merged (and possibly merge commits from the
master
branch).