scripts: standardise shebangs

[alxndrsn]

This came up as part of work on getodk/central-backend#1363 when trying to understand the purpose of the check-migrations make target in odk-central-backend.

---

• 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

What has been done to verify that this works as intended?

CI

Why is this the best possible solution? Were any other approaches considered?

\There are various things in this PR which could be broken down to simplify understanding and reduce the risk of breaking 3rd-party production environments:

• don't standardise on -u flag - this flag has a higher likelihood than others of having surprising effects
• don't bother checking for indirect invocations - there currently aren't any, and the check isn't perfect

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

-u flag might cause problems in custom envs. The error messages generated should be quite obvious though, e.g.

$ bash -c 'set -u; echo $x'
bash: line 1: x: unbound variable

Does this change require updates to documentation? If so, please file an issue here and include the link below.

I don't think so.

Before submitting this PR, please make sure you have:

• branched off and targeted the next branch OR only changed documentation/infrastructure (master is stable and used in production)
• verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

[ktuite]

I think this should be fine? Just summarizing my own understanding of the PR:

Having every file have the same header seems good:

#!/bin/bash -eu
set -o pipefail
shopt -s inherit_errexit

The only other change in here was what we talked about with the check-migrations command, and how that was only introduced so that the server wouldn't start running if there were any problems with the migrations.

I'm approving this but I want someone else with more ops insight like @lognaturel to be aware of this change, too.

[lognaturel]

The original standard shebang was #!/bin/sh. The idea was to have maximal portability and specifically leave the door open to using alpine images which deliberately don't include bash for size reasons.

At some point, I think @alxndrsn started introducing #!/bin/bash shebangs, including for the frontend build script, and we didn't catch the inconsistency in review.

We could do #!/bin/sh -eu but the other error handling improvements are bash-specific.

Given we'd already accidentally been using some bash shebangs anyway and that we have no immediate plans to move to alpine images, I think we can go ahead with this.

[alxndrsn]

The original standard shebang was #!/bin/sh. The idea was to have maximal portability and specifically leave the door open to using alpine images which deliberately don't include bash for size reasons.

At some point, I think @alxndrsn started introducing #!/bin/bash shebangs, including for the frontend build script, and we didn't catch the inconsistency in review.

For the record, various bash shebangs and invocations have been used in this repo over time:

6c38322 (2018):

central/nginx.dockerfile

Line 14 in 6c38322

ENTRYPOINT [ "/bin/bash", "/scripts/odk-setup.sh" ]

central/files/nginx/odk-setup.sh

Line 25 in 6c38322

/bin/bash /scripts/entrypoint.sh

19c87ed (2018):

central/files/service/scripts/start-odk.sh

Line 5 in 19c87ed

/bin/bash -c "envsubst '\$DOMAIN' < /usr/share/odk/config.json.template > $CONFIG_PATH"

b6a6208 (2019):

central/files/nginx/run_certbot.sh

Line 1 in b6a6208

#!/bin/bash

52248f8 (2020):

central/files/enketo/start-enketo.sh

Line 3 in 52248f8

/bin/bash -c "SECRET=$(cat /etc/secrets/enketo-secret) LESS_SECRET=$(cat /etc/secrets/enketo-less-secret) API_KEY=$(cat /etc/secrets/enketo-api-key) envsubst '\$DOMAIN:\$SECRET:\$LESS_SECRET:\$API_KEY' < ${CONFIG_PATH}.template > $CONFIG_PATH"

central/files/service/scripts/start-odk.sh

Line 3 in 52248f8

/bin/bash -c "ENKETO_API_KEY=$(cat /etc/secrets/enketo-api-key) envsubst '\$DOMAIN:\$ENKETO_API_KEY' < /usr/share/odk/config.json.template > $CONFIG_PATH"

5a6b6cb (2022):

central/files/nginx/odk-setup.sh

Line 1 in 5a6b6cb

#!/bin/bash

[lognaturel]

Record redressed! Sorry, I didn't mean to sound blaming, thanks for the fact check!