Skip to content

Commit

Permalink
Merge pull request #96 from lkubb/fix/unwrap-client-config
Browse files Browse the repository at this point in the history
Fixes #95
lkubb authored Nov 7, 2024
2 parents d4734ff + 585bc52 commit 0e4d84e
Showing 13 changed files with 242 additions and 289 deletions.
1 change: 1 addition & 0 deletions changelog/95.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed the client used for unwrapping authentication credentials not respecting `client` configuration when no cached configuration is available
12 changes: 9 additions & 3 deletions src/saltext/vault/utils/vault/factory.py
Original file line number Diff line number Diff line change
@@ -500,8 +500,14 @@ def _get_connection_config(cbank, opts, context, force_local=False, pre_flush=Fa
config_cache.flush(cbank=False)

config_cache.store(new_config)
if unwrap_client is None:
unwrap_client = vclient.VaultClient(**new_config["server"], **new_config["client"])
# Always reinitialize client in case the unwrap_client `client` config is
# out of sync with the new config. This is a theoretical precaution since
# the client should be instantiated in _query_master using the new config.
unwrap_client = vclient.VaultClient(
**new_config["server"],
**new_config["client"],
session=unwrap_client.session if unwrap_client is not None else None,
)
return new_config, embedded_token, unwrap_client


@@ -710,7 +716,7 @@ def check_result(

if result.get("wrap_info") or result.get("wrap_info_nested"):
if unwrap_client is None:
unwrap_client = vclient.VaultClient(**result["server"])
unwrap_client = vclient.VaultClient(**result["server"], **result.get("client", {}))

for key in [""] + result.get("wrap_info_nested", []):
if key:
4 changes: 1 addition & 3 deletions tests/unit/pillar/test_vault.py
Original file line number Diff line number Diff line change
@@ -101,9 +101,7 @@ def test_get_paths(pattern, expected):
previous_pillar = {
"role": "foo",
}
result = vault._get_paths( # pylint: disable=protected-access
pattern, "test-minion", previous_pillar
)
result = vault._get_paths(pattern, "test-minion", previous_pillar)
assert result == expected


97 changes: 38 additions & 59 deletions tests/unit/runners/vault/test_vault.py
Original file line number Diff line number Diff line change
@@ -370,14 +370,12 @@ def test_generate_token(
token_serialized,
wrapped_serialized,
metadata_secret_default,
): # pylint: disable-msg=too-many-arguments
):
"""
Ensure _generate_token calls the API as expected
"""
wrap = config("issue:wrap")
res_token, res_num_uses = vault._generate_token( # pylint: disable=protected-access
"test-minion", issue_params=None, wrap=wrap
)
res_token, res_num_uses = vault._generate_token("test-minion", issue_params=None, wrap=wrap)
endpoint = "auth/token/create"
role_name = config("issue:token:role_name")
payload = {}
@@ -407,17 +405,13 @@ def test_generate_token_no_policies_denied():
Ensure generated tokens need at least one attached policy
"""
with pytest.raises(salt.exceptions.SaltRunnerError, match=".*No policies matched minion.*"):
vault._generate_token( # pylint: disable=protected-access
"test-minion", issue_params=None, wrap=False
)
vault._generate_token("test-minion", issue_params=None, wrap=False)


@pytest.mark.parametrize("ttl", [None, 1337])
@pytest.mark.parametrize("uses", [None, 1, 30])
@pytest.mark.parametrize("config", [{}, {"issue:type": "approle"}], indirect=True)
def test_generate_token_deprecated(
ttl, uses, token_serialized, config, validate_signature, caplog
): # pylint: disable-msg=too-many-arguments
def test_generate_token_deprecated(ttl, uses, token_serialized, config, validate_signature, caplog):
"""
Ensure the deprecated generate_token function returns data in the old format
"""
@@ -715,15 +709,13 @@ def test_get_role_id(
manage_entity,
manage_entity_alias,
wrapped_serialized,
): # pylint: disable-msg=too-many-arguments
):
"""
Ensure _get_role_id returns data in the expected format and does not
try to generate a new AppRole if it exists and is configured correctly
"""
wrap = config("issue:wrap")
res = vault._get_role_id( # pylint: disable=protected-access
"test-minion", issue_params=None, wrap=wrap
)
res = vault._get_role_id("test-minion", issue_params=None, wrap=wrap)
lookup_approle.assert_called_with("test-minion")
lookup_roleid.assert_called_with("test-minion", wrap=wrap)
manage_approle.assert_not_called()
@@ -750,33 +742,32 @@ def test_get_role_id(
)
def test_get_role_id_generate_new(
self,
config, # pylint: disable=unused-argument
config,
lookup_approle,
lookup_roleid,
manage_approle,
manage_entity,
manage_entity_alias,
wrapped_serialized,
issue_params,
): # pylint: disable-msg=too-many-arguments
):
"""
Ensure _get_role_id returns data in the expected format and does not
try to generate a new AppRole if it exists and is configured correctly
"""
lookup_approle.return_value = False
wrap = config("issue:wrap")
res = vault._get_role_id( # pylint: disable=protected-access
"test-minion", issue_params=issue_params, wrap=wrap
)
res = vault._get_role_id("test-minion", issue_params=issue_params, wrap=wrap)
assert res == wrapped_serialized
lookup_roleid.assert_called_with("test-minion", wrap=wrap)
manage_approle.assert_called_once_with("test-minion", issue_params)
manage_entity.assert_called_once_with("test-minion")
manage_entity_alias.assert_called_once_with("test-minion")

@pytest.mark.usefixtures("config")
@pytest.mark.parametrize("config", [{"issue:type": "approle"}], indirect=True)
def test_get_role_id_generate_new_errors_on_generation_failure(
self, config, lookup_approle, lookup_roleid # pylint: disable=unused-argument
self, lookup_approle, lookup_roleid
):
"""
Ensure _get_role_id returns an error if the AppRole generation failed
@@ -787,9 +778,7 @@ def test_get_role_id_generate_new_errors_on_generation_failure(
salt.exceptions.SaltRunnerError,
match="Failed to create AppRole for minion.*",
):
vault._get_role_id( # pylint: disable=protected-access
"test-minion", issue_params=None, wrap=False
)
vault._get_role_id("test-minion", issue_params=None, wrap=False)


@pytest.mark.parametrize(
@@ -951,9 +940,7 @@ def test_get_policies(expected, grains, pillar):
"saltext.vault.utils.vault.helpers.expand_pattern_lists",
Mock(side_effect=lambda x, *args, **kwargs: [x]),
):
res = vault._get_policies( # pylint: disable=protected-access
"test-minion", refresh_pillar=False
)
res = vault._get_policies("test-minion", refresh_pillar=False)
assert res == expected


@@ -979,9 +966,7 @@ def test_get_policies_does_not_render_pillar_unnecessarily(config, grains, pilla
):
with patch("salt.pillar.get_pillar", autospec=True) as get_pillar:
get_pillar.return_value.compile_pillar.return_value = pillar
vault._get_policies( # pylint: disable=protected-access
"test-minion", refresh_pillar=True
)
vault._get_policies("test-minion", refresh_pillar=True)
assert get_pillar.call_count == int("pillar" in config("policies:assign")[0])


@@ -1005,9 +990,7 @@ def test_get_policies_for_nonexisting_minions(expected):
"saltext.vault.utils.vault.helpers.expand_pattern_lists",
Mock(side_effect=lambda x, *args, **kwargs: [x]),
):
res = vault._get_policies( # pylint: disable=protected-access
"test-minion", refresh_pillar=False
)
res = vault._get_policies("test-minion", refresh_pillar=False)
assert res == expected


@@ -1047,9 +1030,7 @@ def test_get_metadata(metadata_patterns, expected, pillar):
"saltext.vault.utils.vault.helpers.expand_pattern_lists",
Mock(side_effect=lambda x, *args, **kwargs: [x]),
):
res = vault._get_metadata( # pylint: disable=protected-access
"test-minion", metadata_patterns, refresh_pillar=False
)
res = vault._get_metadata("test-minion", metadata_patterns, refresh_pillar=False)
assert res == expected


@@ -1065,7 +1046,7 @@ def test_get_metadata_list():
"saltext.vault.utils.vault.helpers.expand_pattern_lists", autospec=True
) as expand:
expand.return_value = ["salt_role_foo", "salt_role_bar"]
res = vault._get_metadata( # pylint: disable=protected-access
res = vault._get_metadata(
"test-minion",
{"salt_role": "salt_role_{pillar[roles]}"},
refresh_pillar=False,
@@ -1277,7 +1258,7 @@ def test_parse_issue_params(issue_params, expected):
Ensure all known parameters can only be overridden if it was configured
on the master. Also ensure the mapping to API requests is correct (for tokens).
"""
res = vault._parse_issue_params(issue_params) # pylint: disable=protected-access
res = vault._parse_issue_params(issue_params)
assert res == expected


@@ -1315,7 +1296,7 @@ def test_parse_issue_params_does_not_allow_bind_secret_id_override(issue_params,
"""
Ensure bind_secret_id can only be set on the master.
"""
res = vault._parse_issue_params(issue_params) # pylint: disable=protected-access
res = vault._parse_issue_params(issue_params)
assert res.get("bind_secret_id", False) == expected


@@ -1324,7 +1305,7 @@ def test_manage_approle(approle_api, policies_default):
"""
Ensure _manage_approle calls the API as expected.
"""
vault._manage_approle("test-minion", None) # pylint: disable=protected-access
vault._manage_approle("test-minion", None)
approle_api.write_approle.assert_called_once_with(
"test-minion",
mount="salt-minions",
@@ -1339,7 +1320,7 @@ def test_delete_approle(approle_api):
"""
Ensure _delete_approle calls the API as expected.
"""
vault._delete_approle("test-minion") # pylint: disable=protected-access
vault._delete_approle("test-minion")
approle_api.delete_approle.assert_called_once_with("test-minion", mount="salt-minions")


@@ -1349,7 +1330,7 @@ def test_lookup_approle(approle_api, approle_meta):
Ensure _lookup_approle calls the API as expected.
"""
approle_api.read_approle.return_value = approle_meta
res = vault._lookup_approle("test-minion") # pylint: disable=protected-access
res = vault._lookup_approle("test-minion")
assert res == approle_meta
approle_api.read_approle.assert_called_once_with("test-minion", mount="salt-minions")

@@ -1360,7 +1341,7 @@ def test_lookup_approle_nonexistent(approle_api):
Ensure _lookup_approle catches VaultNotFoundErrors and returns False.
"""
approle_api.read_approle.side_effect = vaultutil.VaultNotFoundError
res = vault._lookup_approle("test-minion") # pylint: disable=protected-access
res = vault._lookup_approle("test-minion")
assert res is False


@@ -1370,7 +1351,7 @@ def test_lookup_role_id(approle_api, wrap):
"""
Ensure _lookup_role_id calls the API as expected.
"""
vault._lookup_role_id("test-minion", wrap=wrap) # pylint: disable=protected-access
vault._lookup_role_id("test-minion", wrap=wrap)
approle_api.read_role_id.assert_called_once_with("test-minion", mount="salt-minions", wrap=wrap)


@@ -1380,7 +1361,7 @@ def test_lookup_role_id_nonexistent(approle_api):
Ensure _lookup_role_id catches VaultNotFoundErrors and returns False.
"""
approle_api.read_role_id.side_effect = vaultutil.VaultNotFoundError
res = vault._lookup_role_id("test-minion", wrap=False) # pylint: disable=protected-access
res = vault._lookup_role_id("test-minion", wrap=False)
assert res is False


@@ -1390,7 +1371,7 @@ def test_get_secret_id(approle_api, wrap):
"""
Ensure _get_secret_id calls the API as expected.
"""
vault._get_secret_id("test-minion", wrap=wrap) # pylint: disable=protected-access
vault._get_secret_id("test-minion", wrap=wrap)
approle_api.generate_secret_id.assert_called_once_with(
"test-minion",
metadata=ANY,
@@ -1405,7 +1386,7 @@ def test_lookup_entity_by_alias(identity_api):
Ensure _lookup_entity_by_alias calls the API as expected.
"""
with patch("saltext.vault.runners.vault._lookup_role_id", return_value="test-role-id"):
vault._lookup_entity_by_alias("test-minion") # pylint: disable=protected-access
vault._lookup_entity_by_alias("test-minion")
identity_api.read_entity_by_alias.assert_called_once_with(
alias="test-role-id", mount="salt-minions"
)
@@ -1418,7 +1399,7 @@ def test_lookup_entity_by_alias_failed(identity_api):
"""
with patch("saltext.vault.runners.vault._lookup_role_id", return_value="test-role-id"):
identity_api.read_entity_by_alias.side_effect = vaultutil.VaultNotFoundError
res = vault._lookup_entity_by_alias("test-minion") # pylint: disable=protected-access
res = vault._lookup_entity_by_alias("test-minion")
assert res is False


@@ -1427,7 +1408,7 @@ def test_fetch_entity_by_name(identity_api):
"""
Ensure _fetch_entity_by_name calls the API as expected.
"""
vault._fetch_entity_by_name("test-minion") # pylint: disable=protected-access
vault._fetch_entity_by_name("test-minion")
identity_api.read_entity.assert_called_once_with(name="salt_minion_test-minion")


@@ -1437,18 +1418,16 @@ def test_fetch_entity_by_name_failed(identity_api):
Ensure _fetch_entity_by_name returns False if the lookup fails.
"""
identity_api.read_entity.side_effect = vaultutil.VaultNotFoundError
res = vault._fetch_entity_by_name("test-minion") # pylint: disable=protected-access
res = vault._fetch_entity_by_name("test-minion")
assert res is False


@pytest.mark.usefixtures("config")
def test_manage_entity(
identity_api, metadata, metadata_entity_default
): # pylint: disable=unused-argument
@pytest.mark.usefixtures("config", "metadata")
def test_manage_entity(identity_api, metadata_entity_default):
"""
Ensure _manage_entity calls the API as expected.
"""
vault._manage_entity("test-minion") # pylint: disable=protected-access
vault._manage_entity("test-minion")
identity_api.write_entity.assert_called_with(
"salt_minion_test-minion", metadata=metadata_entity_default
)
@@ -1459,7 +1438,7 @@ def test_delete_entity(identity_api):
"""
Ensure _delete_entity calls the API as expected.
"""
vault._delete_entity("test-minion") # pylint: disable=protected-access
vault._delete_entity("test-minion")
identity_api.delete_entity.assert_called_with("salt_minion_test-minion")


@@ -1469,7 +1448,7 @@ def test_manage_entity_alias(identity_api):
Ensure _manage_entity_alias calls the API as expected.
"""
with patch("saltext.vault.runners.vault._lookup_role_id", return_value="test-role-id"):
vault._manage_entity_alias("test-minion") # pylint: disable=protected-access
vault._manage_entity_alias("test-minion")
identity_api.write_entity_alias.assert_called_with(
"salt_minion_test-minion", alias_name="test-role-id", mount="salt-minions"
)
@@ -1486,22 +1465,22 @@ def test_manage_entity_alias_raises_errors(identity_api):
salt.exceptions.SaltRunnerError,
match="Cannot create alias.* no entity found.",
):
vault._manage_entity_alias("test-minion") # pylint: disable=protected-access
vault._manage_entity_alias("test-minion")


def test_revoke_token_by_token(client):
"""
Ensure _revoke_token calls the API as expected.
"""
vault._revoke_token(token="test-token") # pylint: disable=protected-access
vault._revoke_token(token="test-token")
client.post.assert_called_once_with("auth/token/revoke", payload={"token": "test-token"})


def test_revoke_token_by_accessor(client):
"""
Ensure _revoke_token calls the API as expected.
"""
vault._revoke_token(accessor="test-accessor") # pylint: disable=protected-access
vault._revoke_token(accessor="test-accessor")
client.post.assert_called_once_with(
"auth/token/revoke-accessor", payload={"accessor": "test-accessor"}
)
Loading

0 comments on commit 0e4d84e

Please sign in to comment.