Conversation
| policy_arn = aws_iam_policy.regression_test_permissions.arn | ||
| } | ||
|
|
||
| resource "aws_iam_role_policy_attachment" "security_management" { |
There was a problem hiding this comment.
I think we already have a policy attachment called security_management on line 1006? Should this be regression_security_management ?
| "secretsmanager:DescribeSecret", | ||
| "secretsmanager:UpdateSecretVersionStage" | ||
| ] | ||
| } |
There was a problem hiding this comment.
do we need to add a resource here for the specific secrets
There was a problem hiding this comment.
the reason I haven't is because we only have 1 and also because secrets that we add later will likely need to be tested by regression anyway so its just gunna end up listing all the secrets here.
There was a problem hiding this comment.
happy to be convinced otherwise though
There was a problem hiding this comment.
just realised i'm missing the resource tab entirely! will add a *
There was a problem hiding this comment.
We could probably still specify a wildcarded path based on the 'path' of the secrets we save.
There was a problem hiding this comment.
"arn:aws:secretsmanager:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:secret:eligibility-signposting-api-*"
| data "aws_iam_policy_document" "regression_test_permissions" { | ||
| statement { | ||
| sid = "S3Access" | ||
| Effect = "Allow", |
There was a problem hiding this comment.
for data sources, I think Terraform uses lower case effect, actions, resources (for this and below)
There was a problem hiding this comment.
how annoyingly inconsistent, but true, will amend
| ] | ||
| resources = [ | ||
| "arn:aws:ssm:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:parameter/${var.environment}/*", | ||
| "arn:aws:ssm:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:parameter/splunk/*", |
There was a problem hiding this comment.
I can't think why regression tests would need either our splunk or proxygen tokens (the ones below).
Presumably needs mtls to send requests to our api, so the first resource is probably ok
There was a problem hiding this comment.
yeah this is another case of just trying to give what they had, but agree its not needed i will remove
| effect = "Allow" | ||
| action = [ | ||
| "secretsmanager:GetSecretValue", | ||
| "secretsmanager:PutSecretValue", |
There was a problem hiding this comment.
Do the tests change the secret values or versions?
There was a problem hiding this comment.
I thought they might do or may do in future, i will discuss with adam as i'd rather they didnt
| "dynamodb:ListTagsOfResource" | ||
| ] | ||
| resources = [ | ||
| "arn:aws:dynamodb:*:${data.aws_caller_identity.current.account_id}:table/*eligibility-signposting-api-${var.environment}-eligibility_datastore" |
There was a problem hiding this comment.
think this needs the region var
"arn:aws:dynamodb:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:table/*eligibility-signposting-api-${var.environment}-eligibility_datastore"
There was a problem hiding this comment.
i did think that but no where else in this file is that given? so i just went with the convention, happy to change though
| statement { | ||
| sid = "SecretsManagerAccess" | ||
| effect = "Allow" | ||
| action = [ |
| data "aws_iam_policy_document" "regression_test_permissions" { | ||
| statement { | ||
| sid = "S3Access" | ||
| effect = "Allow", |
| "s3:PutObjectTagging", | ||
| "s3:GetObjectVersion", | ||
| ], | ||
| resource = [ |
| "s3:GetObjectTagging", | ||
| "s3:PutObjectTagging", | ||
| "s3:GetObjectVersion", | ||
| ], |
| statement { | ||
| sid = "DynamoAccess" | ||
| effect = "Allow" | ||
| action = [ |
| statement { | ||
| sid = "CloudWatchLogsRead" | ||
| effect = "Allow" | ||
| action = [ |
| statement { | ||
| sid = "XRayRead" | ||
| effect = "Allow" | ||
| action = [ |
| statement { | ||
| sid = "SSMRead" | ||
| effect = "Allow" | ||
| action = [ |
2 participants
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.