diff --git a/osism/tasks/__init__.py b/osism/tasks/__init__.py index c0c464a19..adb9f0b2f 100644 --- a/osism/tasks/__init__.py +++ b/osism/tasks/__init__.py @@ -50,6 +50,10 @@ ISOLATED_ENV_PREFIXES = ("LC_",) +# The path the run scripts in the worker containers fall back to when VAULT is +# unset: CONFIGURATION_DIRECTORY/environments/.vault_pass, both hardcoded there. +ANSIBLE_VAULT_PASSWORD_FILE = "/opt/configuration/environments/.vault_pass" + class AnsibleFailure(Exception): """Raised when an Ansible run in a worker container exits non-zero. @@ -300,6 +304,35 @@ def run_ansible_in_environment( ansible_vault_password = utils.redis.get("ansible_vault_password") if ansible_vault_password: env["VAULT"] = "/ansible-vault.py" + elif not env.get("VAULT") and not os.path.exists(ANSIBLE_VAULT_PASSWORD_FILE): + # The run script resolves VAULT=${VAULT:-}, so both of its + # inputs have to be checked here. A non-empty inherited VAULT is an + # operator pointing at a custom password file or helper + # (manager_environment_extra reaches the worker containers as plain + # environment variables); the script prefers it and never looks at + # the file below. ${:-} substitutes on null as well as on unset, so + # an empty VAULT is the fallback case, not the operator one. + # + # With neither, the script falls back to the file, and every one of + # them passes --vault-password-file unconditionally, so the play + # dies with "The vault password file ... was not found". That + # message sends the operator looking for a file in the configuration + # repository, while the state that actually holds is "the worker has + # no vault password" -- and it recurs by design, because the password + # lives only in Redis, which the manager runs without a volume. This + # is the only layer that sees both sources, so it is the only one + # that can say so. Report it here and do not dispatch. + message = ( + "No Ansible Vault password is available. Run " + "'osism set vault password' on the manager, or add " + "environments/.vault_pass to the configuration repository " + "for unattended operation." + ) + logger.error(message) + if publish: + utils.push_task_output(request_id, f"{message}\n") + utils.finish_task_output(request_id, rc=1) + raise AnsibleFailure(message) # Log play execution start log_play_execution( diff --git a/tests/unit/tasks/test_init.py b/tests/unit/tasks/test_init.py index bfabb1055..9c420abbf 100644 --- a/tests/unit/tasks/test_init.py +++ b/tests/unit/tasks/test_init.py @@ -327,8 +327,16 @@ def runner_mocks(mocker, tmp_path, mock_redis, monkeypatch): popen = mocker.patch("osism.tasks.subprocess.Popen") popen.return_value = make_process(["ok: [node-1]\n"]) + # The default is a deployment that can run: no password in Redis, but the + # configuration repository carries the file the run scripts fall back to. + # Tests for the empty-Redis case unlink it. + vault_file = tmp_path / ".vault_pass" + vault_file.write_text("secret\n") + mocker.patch.object(tasks, "ANSIBLE_VAULT_PASSWORD_FILE", str(vault_file)) + return SimpleNamespace( popen=popen, + vault_file=vault_file, create_redlock=mocker.patch("osism.tasks.utils.create_redlock"), push=mocker.patch("osism.tasks.utils.push_task_output"), finish=mocker.patch("osism.tasks.utils.finish_task_output"), @@ -514,13 +522,124 @@ def test_run_ansible_vault_env_set_when_password_present(runner_mocks): assert env["VAULT"] == "/ansible-vault.py" -def test_run_ansible_vault_env_absent_without_password(runner_mocks): +def test_run_ansible_vault_env_absent_when_only_the_file_exists(runner_mocks): + """No password in Redis but a file in the configuration repository is a + working deployment: VAULT stays unset so the run script falls back to + ``$ENVIRONMENTS_DIRECTORY/.vault_pass`` itself.""" runner_mocks.redis.get.return_value = None run_ansible() env = runner_mocks.popen.call_args.kwargs["env"] assert "VAULT" not in env +def test_run_ansible_vault_file_not_consulted_when_redis_has_password(runner_mocks): + """The Redis password wins, so a configuration repository without the file + -- the state a freshly generated one is in -- still runs.""" + runner_mocks.redis.get.return_value = b"secret" + runner_mocks.vault_file.unlink() + run_ansible() + assert runner_mocks.popen.call_args.kwargs["env"]["VAULT"] == "/ansible-vault.py" + + +def test_run_ansible_no_vault_password_anywhere_raises_before_dispatch(runner_mocks): + """With neither source available the run script would abort with "The vault + password file ... was not found", pointing at the configuration repository + instead of at the worker state that actually holds. Both sources are only + visible here, so the play is not started at all.""" + runner_mocks.redis.get.return_value = None + runner_mocks.vault_file.unlink() + + with pytest.raises(tasks.AnsibleFailure): + run_ansible() + + runner_mocks.popen.assert_not_called() + + +def test_run_ansible_no_vault_password_names_both_remedies(runner_mocks): + """The interactive remedy is not discoverable from the state, and the file + is the only unattended one -- so the message has to name both.""" + runner_mocks.redis.get.return_value = None + runner_mocks.vault_file.unlink() + + with pytest.raises(tasks.AnsibleFailure) as excinfo: + run_ansible() + + message = str(excinfo.value) + assert "osism set vault password" in message + assert "environments/.vault_pass" in message + + +def test_run_ansible_inherited_vault_env_suppresses_the_check( + runner_mocks, monkeypatch +): + """An operator can point the run script at a custom password file or helper + by putting VAULT in manager_environment_extra, which reaches this process as + an inherited environment variable. The script prefers it over its own + fallback, so the fallback file being absent is not a failure then.""" + monkeypatch.setenv("VAULT", "/opt/configuration/environments/custom.vault_pass") + runner_mocks.redis.get.return_value = None + runner_mocks.vault_file.unlink() + + run_ansible() + + env = runner_mocks.popen.call_args.kwargs["env"] + assert env["VAULT"] == "/opt/configuration/environments/custom.vault_pass" + + +def test_run_ansible_empty_vault_env_does_not_suppress_the_check( + runner_mocks, monkeypatch +): + """``${VAULT:-}`` substitutes on null as well as on unset, so an + empty VAULT leaves the run script on the fallback file and the check has to + treat it exactly like an absent one.""" + monkeypatch.setenv("VAULT", "") + runner_mocks.redis.get.return_value = None + runner_mocks.vault_file.unlink() + + with pytest.raises(tasks.AnsibleFailure): + run_ansible() + + runner_mocks.popen.assert_not_called() + + +def test_run_ansible_no_vault_password_streams_message_and_rc(runner_mocks): + """``osism apply`` reads both the message and the rc back out of the Redis + output stream, so raising alone would leave it waiting for output that + never arrives.""" + runner_mocks.redis.get.return_value = None + runner_mocks.vault_file.unlink() + + with pytest.raises(tasks.AnsibleFailure): + run_ansible() + + pushed = "".join(call.args[1] for call in runner_mocks.push.call_args_list) + assert "osism set vault password" in pushed + runner_mocks.finish.assert_called_once_with("req-1", rc=1) + + +def test_run_ansible_no_vault_password_publish_false_does_not_stream(runner_mocks): + runner_mocks.redis.get.return_value = None + runner_mocks.vault_file.unlink() + + with pytest.raises(tasks.AnsibleFailure): + run_ansible(publish=False) + + runner_mocks.push.assert_not_called() + runner_mocks.finish.assert_not_called() + + +def test_run_ansible_no_vault_password_cleans_ssh_dir(runner_mocks): + """The raise happens inside the try whose ``finally`` removes the per-task + ControlPath directory.""" + runner_mocks.redis.get.return_value = None + runner_mocks.vault_file.unlink() + + with pytest.raises(tasks.AnsibleFailure): + run_ansible() + + runner_mocks.rmtree.assert_called_once() + + # -- worker dispatch --