What is intended behavior when secrets stored in a vault are overridden?

[Tam-Lin]

Is your feature request related to a problem? Please describe. I'm adding some additional testcases so that when I make a actually functioning pull request for #209, I'm more assured it won't break anything else. And I've run into an issue I'm not sure how to resolve, because I'm not sure what the intended behavior is.

Specifically, if you have a secret specified in the environment, and then an override of it, the override wins. This is what the test_override_load_environmen_with_var tests for, so I'm assuming it's correct. So I created a corresponding testcase for overriding secrets from a vault with var, and that currently fails, without any changes to the secrets code. So you can override environmental secrets, but you can't override vault secrets. I'm assuming that's not the intended behavior, but I don't know that for sure.

Describe the solution you'd like I think that both secrets declared in the environment and secrets that come from a vault should both be able to be overridden, for consistencies sake.

Describe alternatives you've considered The other option is to retain current behavior, which is fine too, as long as it's documented that that's what happens, and you can't override secrets sourced from the vault.

Additional context This is the test case that currently fails:

def test_override_vault_with_var(hvac):
    config = {
        'vault_addr': 'http://someaddr.com',
        'vault_token': 'not_awesome_token',
        'vault_kv_version': '2'
    }

    vault_secret_payload = {
        "data": {
            "data": {
                "foo": "bar",
                "baz": "hello"
            },
            "metadata": {
                "auth": None,
                "lease_duration": 2764800,
                "lease_id": "",
                "renewable": False
            }
        }
    }

    fake_client = MagicMock()
    hvac.Client.return_value = fake_client
    fake_client.secrets.kv.v2.read_secret_version.return_value = vault_secret_payload

    secrets = load_secrets({
        "myapp": {
            "token": {
                "type": "vault",
                "path": "secrets/something",
                "key": "foo"
            }
        }
    }, config,
        {"myapp": {"token": "baz"}})
    assert secrets["myapp"]["token"] == "baz"

[Lawouach]

Hello,

Good catch. I hardly use Vault so I think this one escaped me. I also never considered the need to override vault secrets that way.

I think you are right, we need consistency. Overriding should always win, no matter the secret provider. That said, it still feels a bit odd that you'd want to override a vault secret with an env value.

Damn, now I'm thinking about it, I'm unsure. Would that be a security issue somehow. Afterall, if secrets are stored in vault, it's likely so that they get centralized and managed out of band. Would you not expect that chaos toolkit ensures you always go through vault if the secret is defined that way? From an operations perspective that is?

Mmmh. Unsure.

[Tam-Lin]

I have no idea. I'm not sure where the extra_vars is coming from, or why it was added, just that it's I think supposed to override any values that were specified in the experiment itself. I'd tend to argue for having extra_vars override things no matter what for consistency's sake; if the person running the experiment doesn't want extra_vars to override things, they shouldn't specify them in the first place.

[ciaransweet]

Closing because of @Tam-Lin 's PR