Skip to content

Commit

Permalink
allowedSecretParameterPaths prefix but not parent of secretParameters (
Browse files Browse the repository at this point in the history
…#3242)

trying to make a test for another example of #3227.  In this test, we have `foo` in `allowedSecretParameterPaths` but the `secretParameters` has `path = "foobar/baz"`.

I think this means validation should fail in `_validate_allowed_secret_parameter_paths` with the log message `secret parameter path 'foobar/baz' does not match any of allowedSecretParameterPaths`.

But this is based on a naive reading of the code + test cases, and I could be wrong.  Somebody who uses this function should review.

https://issues.redhat.com/browse/APPSRE-7196
  • Loading branch information
d-hat authored Mar 1, 2023
1 parent cbd8c33 commit 81210a0
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 2 deletions.
76 changes: 76 additions & 0 deletions reconcile/test/test_saasherder_allowed_secret_paths.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import pytest

from reconcile.utils.saasherder import SaasHerder


@pytest.mark.parametrize(
"allowed_secret_parameter_path,referenced_secret_path,expected_valid",
[
# covered by parent directory
("foobar", "foobar/baz", True),
# not covered by parent directory even though there is a common name prefix
("foo", "foobar/baz", False),
# multilevel allowed path
("foo/bar", "foo/bar/baz", True),
# multilevel but different intermediary path
("foo/bar", "foo/baz/bar", False),
],
)
def test_saasherder_allowed_secret_paths(
allowed_secret_parameter_path: str,
referenced_secret_path: str,
expected_valid: bool,
):
"""
ensure a parent directory in allowed_secret_parameter_paths matches correctly
"""
saas_files = [
{
"path": "path1",
"name": "a1",
"managedResourceTypes": [],
"allowedSecretParameterPaths": [allowed_secret_parameter_path],
"resourceTemplates": [
{
"name": "test",
"url": "url",
"targets": [
{
"namespace": {
"name": "ns",
"environment": {"name": "env1", "parameters": "{}"},
"cluster": {"name": "cluster"},
},
"ref": "main",
"upstream": {"instance": {"name": "ci"}, "name": "job"},
"parameters": {},
"secretParameters": [
{
"name": "secret",
"secret": {
"path": referenced_secret_path,
"field": "db.endpoint",
},
},
],
},
],
},
],
"selfServiceRoles": [
{"users": [{"org_username": "theirname"}], "bots": []}
],
},
]

saasherder = SaasHerder(
saas_files,
thread_pool_size=1,
gitlab=None,
integration="",
integration_version="",
settings={},
validate=True,
)

assert saasherder.valid == expected_valid
8 changes: 6 additions & 2 deletions reconcile/utils/saasherder.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,11 @@ def _validate_allowed_secret_parameter_paths(
return
for sp in secret_parameters:
path = sp["secret"]["path"]
match = [a for a in allowed_secret_parameter_paths if path.startswith(a)]
match = [
a
for a in allowed_secret_parameter_paths
if (os.path.commonpath([path, a]) == a)
]
if not match:
self.valid = False
logging.error(
Expand Down Expand Up @@ -465,7 +469,7 @@ def _check_promotions_have_same_source(
else:
(pub_saas, pub_rt_name, pub_rt_url) = pub_channel_ref

for (sub_saas, sub_rt_name, sub_rt_url) in sub_targets:
for sub_saas, sub_rt_name, sub_rt_url in sub_targets:
if not pub_channel_ref:
logging.error(
"Channel is not published by any target\n"
Expand Down

0 comments on commit 81210a0

Please sign in to comment.