Control locale during Ansible runs #4252

rmol · 2019-03-08T23:51:40Z

Status

Ready for review

Description of Changes

To avoid non-English system locales from breaking tasks that parse command output expecting English, set the environment variable LC_ALL=C in each play of securedrop-prod.yml.

Fixes #4219.

Testing

Install app and mon with a non-English locale. Verify that running securedrop-admin install completes without errors. If it breaks on tasks in install_tor.yml, or anywhere else, this fix has not worked.

Checklist

If you made changes to `securedrop-admin`:

Linting and tests (make -C admin test) pass in the admin development container

If you made non-trivial code changes:

I have written a test plan and validated it for this PR

emkll · 2019-03-12T21:31:11Z

I will be testing this will the following plan

Set up Prod VMs (14.04) with french locale (setting the contents of /etc/default/locale [0]) and running sudo locale-gen fr_FR && sudo locale-gen fr_FR.UTF-8 && sudo update-locale
Reboot and ensure locale returns [0], that various tooling is in French
Install on this branch from Tails, and ensure the testing completes successfully
Upgrade to 16.04 and re-run the playbook

[0] Contents of /etc/default/locale:

LANG=fr_FR.UTF-8
LANGUAGE="fr_FR.UTF-8"
LC_CTYPE="fr_FR.UTF-8"
LC_NUMERIC="fr_FR.UTF-8"
LC_TIME="fr_FR.UTF-8"
LC_COLLATE="fr_FR.UTF-8"
LC_MONETARY="fr_FR.UTF-8"
LC_MESSAGES="fr_FR.UTF-8"
LC_PAPER="fr_FR.UTF-8"
LC_NAME="fr_FR.UTF-8"
LC_ADDRESS="fr_FR.UTF-8"
LC_TELEPHONE="fr_FR.UTF-8"
LC_MEASUREMENT="fr_FR.UTF-8"
LC_IDENTIFICATION="fr_FR.UTF-8"
LC_ALL="fr_FR.UTF-8"

emkll

Looks good to me so far (one comment inline). I have tested this branch using the above test plan:

Set up Prod VMs (14.04) with french locale (setting the contents of /etc/default/locale [0]) and running sudo locale-gen fr_FR && sudo locale-gen fr_FR.UTF-8 && sudo update-locale
Reboot and ensure locale returns [0], that various tooling is in French
Install on this branch from Tails, and ensure the testing completes successfully
Upgrade to 16.04 and re-run the playbook

I will test on hardware tomorrow morning, based on the instructions provided in #4219 (comment)

emkll · 2019-03-12T22:00:38Z

install_files/ansible-base/securedrop-prod.yml

@@ -2,6 +2,8 @@
 ---


Would it make sense to also apply these changes to securedrop-staging.yml?

Summarizing out of band discussion, we should also add the locale blocks to the other "prod" SD playbooks, such as -logs and -backup. Technically only -logs uses register, which is how we first encountered the error we're aiming to resolve here, but adding coverage to the other playbooks will also let us broaden the config test and reuse the existing "check all prod SD playbooks" logic.

I'll tack on a commit to implement this before final review.

emkll · 2019-03-12T22:44:28Z

install_files/ansible-base/securedrop-prod.yml

@@ -2,6 +2,8 @@
 ---
 - name: Ensure validation is run before prod install
  hosts: localhost
+  environment:
+    LC_ALL: C


What do you think about validating that this variable is set (via assert or otherwise) at the start of each play?

Rather than an assert statement, we can add a config test (to be run in CI) that confirms the environment is set as expected on every play. See here for an example: https://github.com/freedomofpress/securedrop/blob/10a2eeedc3c22fd39e6a7eda4ca8f4bca1ca6024/molecule/ansible-config/tests/test_max_fail_percentage.py

@rmol has already added the corresponding config test here, which will help guard against regressions in the future: if new plays are added without the explicit locale declaration, CI will report an error.

To avoid non-English system locales from breaking tasks that parse command output expecting English, set the environment variable LC_ALL=C in each play of securedrop-prod.yml.

Add configuration test to check locale is set for all plays there and in securedrop-prod.

@rmol

Porting the locale override logic by @rmol to the usual medley of SecureDrop prod playbooks. Correspondingly updates the config tests to reuse the "all prod playbooks" lookup logic, so we're sure to retain coverage into the future.

conorsch · 2019-03-14T01:17:34Z

Tacked on a commit porting the locale overrides to all production playbooks, with corresponding config test update.

conorsch

Approving based on visual review of changes, as well as @emkll's detailed test report.

kushaldas · 2019-03-14T13:20:04Z

@emkll I tried sudo locale-gen fr_FR && sudo locale-gen fr_FR.UTF-8 && sudo update-locale many times, but, could not set the locale properly. What am I missing?

rmol · 2019-03-14T13:26:08Z

@kushaldas I think you need to pass to update-locale the environment variables you want active in /etc/default/locale, e.g. sudo update-locale LANG=fr_FR.UTF8. I'd add the --reset flag as well, which will comment out any variables present in the file that you don't pass.

emkll

Based on @rmol's instructions, a minimal way to change locale on VMs:

sudo locale-gen fr_FR.UTF-8
sudo update-locale --reset LANG=fr_FR.UTF-8
sudo reboot

Once that was done, followed this test plan:

Installs successfully on this branch
Infra tests pass
./securedrop-admin backup and ./securedrop-admin restore complete successfully
./securedrop-admin logs completes successfully
./securedrop-admin tailsconfig completes successfully

LGTM!

rmol requested review from conorsch, emkll, kushaldas and msheiny as code owners March 8, 2019 23:51

rmol force-pushed the fix-4219 branch from c6749ba to 5573305 Compare March 11, 2019 19:02

emkll reviewed Mar 12, 2019

View reviewed changes

rmol added 2 commits March 13, 2019 15:43

Control locale during Ansible runs

f445d83

To avoid non-English system locales from breaking tasks that parse command output expecting English, set the environment variable LC_ALL=C in each play of securedrop-prod.yml.

Control locale for securedrop-staging playbook as well

75d6965

Add configuration test to check locale is set for all plays there and in securedrop-prod.

rmol force-pushed the fix-4219 branch from 5573305 to 75d6965 Compare March 13, 2019 21:33

Enforces locale overrides on all playbooks

c71bf16

Porting the locale override logic by @rmol to the usual medley of SecureDrop prod playbooks. Correspondingly updates the config tests to reuse the "all prod playbooks" lookup logic, so we're sure to retain coverage into the future.

conorsch approved these changes Mar 14, 2019

View reviewed changes

emkll approved these changes Mar 14, 2019

View reviewed changes

emkll merged commit 5ce9fa1 into freedomofpress:develop Mar 14, 2019

eloquence mentioned this pull request Mar 21, 2019

[docs] Document how to run install playbooks when server locale is non-English #4242

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Control locale during Ansible runs #4252

Control locale during Ansible runs #4252

rmol commented Mar 8, 2019

emkll commented Mar 12, 2019 •

edited

Loading

emkll left a comment

emkll Mar 12, 2019

rmol Mar 12, 2019

conorsch Mar 14, 2019

emkll Mar 12, 2019

conorsch Mar 12, 2019

conorsch Mar 14, 2019

conorsch commented Mar 14, 2019

conorsch left a comment

kushaldas commented Mar 14, 2019

rmol commented Mar 14, 2019

emkll left a comment

Control locale during Ansible runs #4252

Control locale during Ansible runs #4252

Conversation

rmol commented Mar 8, 2019

Status

Description of Changes

Testing

Checklist

If you made changes to securedrop-admin:

If you made non-trivial code changes:

emkll commented Mar 12, 2019 • edited Loading

emkll left a comment

Choose a reason for hiding this comment

emkll Mar 12, 2019

Choose a reason for hiding this comment

rmol Mar 12, 2019

Choose a reason for hiding this comment

conorsch Mar 14, 2019

Choose a reason for hiding this comment

emkll Mar 12, 2019

Choose a reason for hiding this comment

conorsch Mar 12, 2019

Choose a reason for hiding this comment

conorsch Mar 14, 2019

Choose a reason for hiding this comment

conorsch commented Mar 14, 2019

conorsch left a comment

Choose a reason for hiding this comment

kushaldas commented Mar 14, 2019

rmol commented Mar 14, 2019

emkll left a comment

Choose a reason for hiding this comment

If you made changes to `securedrop-admin`:

emkll commented Mar 12, 2019 •

edited

Loading