Skip to content

Support for SSM Parameter Store Hierarchy #1128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andreas-mueller-bb opened this issue May 14, 2024 · 3 comments
Closed

Support for SSM Parameter Store Hierarchy #1128

andreas-mueller-bb opened this issue May 14, 2024 · 3 comments
Labels
stale Issue/PR is stale and closed automatically

Comments

@andreas-mueller-bb
Copy link

andreas-mueller-bb commented May 14, 2024

Describe the bug

Using the latest version with the preregistered token workflow.

We're applying a hierarchy to our parameter names like described here: https://docs.aws.amazon.com/systems-manager/latest/userguide/sysman-paramstore-hierarchies.html

Unfortunately that doesn't go well with the ssm policy created for accessing that parameter.
When the parameter is called "/terraform/prod/preregistered_gitlab_runner_token" the policy looks like this:

              ~ Statement = [
                  ~ {
                      ~ Action   = [
                          - "ssm:PutParameter",
                          + "ssm:GetParameters",
                          + "ssm:GetParameter",
                        ]
                      ~ Resource = "*" -> [
                          + [...]
                          + "arn:aws:ssm:eu-central-1:913735344111:parameter//terraform/prod/preregistered_gitlab_runner_token",
                        ]

To Reproduce

Steps to reproduce the behavior:

  1. Use a parameter with a leading slash
  2. Apply the module
  3. Check the ssm policy

Expected behavior

As using a hierarchy is encouraged by AWS it would be awesome if this would be compatible with the module :)

Additional context

Thanks for you work!

@bwells-scripps
Copy link

Here is how we worked around the issue with our local copy. It may not be the best or most correct way, so I haven't submitted it as a PR yet:

---
 main.tf | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/main.tf b/main.tf
index 5a018ab..6b60f85 100644
--- a/main.tf
+++ b/main.tf
@@ -604,7 +604,7 @@ data "aws_iam_policy_document" "ssm" {
       "ssm:GetParameters",
     ]
     resources = [
-      for name in compact(
+      for name in [for nm in compact(
         [
           aws_ssm_parameter.runner_sentry_dsn.name,
           local.runner_gitlab_registration_token_secure_parameter_store_name,
@@ -612,14 +612,19 @@ data "aws_iam_policy_document" "ssm" {
           var.runner_gitlab.preregistered_runner_token_ssm_parameter_name,
           aws_ssm_parameter.runner_registration_token.name
         ]
-      ) : "arn:${data.aws_partition.current.partition}:ssm:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:parameter/${name}"
+        ) : startswith(nm, "/") ? nm : "/${nm}"
+      ] : "arn:${data.aws_partition.current.partition}:ssm:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:parameter${name}"
     ]
   }
 
   statement {
     actions = ["ssm:PutParameter"]
     resources = [
-      "arn:${data.aws_partition.current.partition}:ssm:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:parameter/${aws_ssm_parameter.runner_registration_token.name}"
+      for name in [
+        for nm in compact(
+          [aws_ssm_parameter.runner_registration_token.name]
+        ) : startswith(nm, "/") ? nm : "/${nm}"
+      ] : "arn:${data.aws_partition.current.partition}:ssm:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:parameter${name}"
     ]
   }
 }
-- 

Copy link
Contributor

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 15 days.

@github-actions github-actions bot added the stale Issue/PR is stale and closed automatically label Jul 17, 2024
kayman-mk added a commit that referenced this issue Jul 22, 2024
## Description

Using SSM parameter hierarchies (as described in
https://docs.aws.amazon.com/systems-manager/latest/userguide/sysman-paramstore-hierarchies.html)
results in an error. See #1128

Several workarounds exist, but using `trimprefix` makes them
superfluous. Having a `/` as first character results in the above
mentioned error.

## Verification

Checked the ARN of parameter `/test/test`. It showed up as
`arn:aws:ssm:eu-central-1:123456789012:parameter/test/test` in the
console. So no `//` at first place.

---------

Co-authored-by: kirkchong <[email protected]>
Co-authored-by: Matthias Kay <[email protected]>
Co-authored-by: Matthias Kay <[email protected]>
@kayman-mk
Copy link
Collaborator

Closed via #1146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issue/PR is stale and closed automatically
Projects
None yet
Development

No branches or pull requests

3 participants