-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/eli 731 add regression role to tf #642
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
base: main
Are you sure you want to change the base?
Changes from all commits
14d275e
271901b
6b225d4
7f4609e
8afd37c
a67ee36
9541bc5
41229b1
b2a9a45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -678,49 +678,6 @@ resource "aws_iam_policy" "iam_management" { | |
| tags = merge(local.tags, { Name = "iam-management" }) | ||
| } | ||
|
|
||
| # Assume role policy document for GitHub Actions | ||
| data "aws_iam_policy_document" "github_actions_assume_role" { | ||
| statement { | ||
| sid = "OidcAssumeRoleWithWebIdentity" | ||
| effect = "Allow" | ||
| actions = ["sts:AssumeRoleWithWebIdentity"] | ||
|
|
||
| principals { | ||
| type = "Federated" | ||
| identifiers = [ | ||
| aws_iam_openid_connect_provider.github.arn | ||
| ] | ||
| } | ||
|
|
||
| condition { | ||
| test = "StringLike" | ||
| variable = "token.actions.githubusercontent.com:sub" | ||
| values = ["repo:${var.github_org}/${var.github_repo}:*"] | ||
| } | ||
|
|
||
| condition { | ||
| test = "StringEquals" | ||
| variable = "token.actions.githubusercontent.com:aud" | ||
| values = ["sts.amazonaws.com"] | ||
| } | ||
| } | ||
| dynamic "statement" { | ||
| for_each = var.environment == "dev" ? [1] : [] | ||
| content { | ||
| sid = "AllowDevSSORoleToAssumeIamBootstrap" | ||
| effect = "Allow" | ||
| actions = ["sts:AssumeRole"] | ||
|
|
||
| principals { | ||
| type = "AWS" | ||
| identifiers = [ | ||
| local.dev_role_arn | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| resource "aws_iam_policy" "stream_management" { | ||
| name = "stream-management" | ||
| description = "Allow GitHub Actions to manage project Firehose delivery streams and Kinesis streams" | ||
|
|
@@ -845,6 +802,190 @@ resource "aws_iam_policy" "cloudwatch_management" { | |
| tags = merge(local.tags, { Name = "cloudwatch-management" }) | ||
| } | ||
|
|
||
| data "aws_iam_policy_document" "regression_test_permissions" { | ||
| statement { | ||
| sid = "S3Access" | ||
| effect = "Allow" | ||
| actions = [ | ||
| "s3:ListBucket", | ||
| "s3:GetObject", | ||
| "s3:PutObject", | ||
| "s3:DeleteObject", | ||
| "s3:GetBucketTagging", | ||
| "s3:GetObjectTagging", | ||
| "s3:PutObjectTagging", | ||
| "s3:GetObjectVersion" | ||
| ], | ||
| resources = [ | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-eli-rules", | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-eli-rules/*", | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-consumer-map", | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-consumer-map/*" | ||
| ] | ||
| } | ||
|
|
||
| statement { | ||
| sid = "DynamoAccess" | ||
| effect = "Allow" | ||
| actions = [ | ||
| "dynamodb:GetItem", | ||
|
eddalmond1 marked this conversation as resolved.
|
||
| "dynamodb:PutItem", | ||
| "dynamodb:Query", | ||
| "dynamodb:Scan", | ||
| "dynamodb:UpdateItem", | ||
| "dynamodb:DeleteItem", | ||
| "dynamodb:DescribeTable", | ||
| "dynamodb:ListTables", | ||
| "dynamodb:DeleteTable", | ||
| "dynamodb:CreateTable", | ||
| "dynamodb:TagResource", | ||
| "dynamodb:UntagResource", | ||
| "dynamodb:ListTagsOfResource" | ||
| ] | ||
| resources = [ | ||
| "arn:aws:dynamodb:*:${data.aws_caller_identity.current.account_id}:table/*eligibility-signposting-api-${var.environment}-eligibility_datastore" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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"
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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" | ||
| actions = [ | ||
| "secretsmanager:GetSecretValue", | ||
| "secretsmanager:PutSecretValue", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do the tests change the secret values or versions?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought they might do or may do in future, i will discuss with adam as i'd rather they didnt |
||
| "secretsmanager:DescribeSecret", | ||
| "secretsmanager:UpdateSecretVersionStage" | ||
| ] | ||
| resources = ["*"] | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to add a resource here for the specific secrets
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. happy to be convinced otherwise though
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just realised i'm missing the resource tab entirely! will add a *
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could probably still specify a wildcarded path based on the 'path' of the secrets we save.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "arn:aws:secretsmanager:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:secret:eligibility-signposting-api-*" |
||
|
|
||
| statement { | ||
| sid = "CloudWatchLogsRead" | ||
| effect = "Allow" | ||
| actions = [ | ||
| "logs:DescribeLogGroups", | ||
| "logs:DescribeLogStreams", | ||
| "logs:GetLogEvents", | ||
| "logs:FilterLogEvents", | ||
| "logs:StartQuery", | ||
| "logs:GetQueryResults", | ||
| "logs:StopQuery" | ||
| ] | ||
| resources = ["*"] | ||
| } | ||
|
|
||
| statement { | ||
| sid = "XRayRead" | ||
| effect = "Allow" | ||
| actions = [ | ||
| "xray:GetTraceSummaries", | ||
| "xray:BatchGetTraces", | ||
| "xray:GetServiceGraph", | ||
| "xray:GetGroups", | ||
| "xray:GetGroup", | ||
| "xray:GetSamplingRules", | ||
| "xray:GetSamplingTargets", | ||
| "xray:GetSamplingStatisticSummaries", | ||
| "xray:UpdateSamplingRule" | ||
| ] | ||
| resources = ["*"] | ||
| } | ||
|
|
||
| statement { | ||
| sid = "SSMRead" | ||
| effect = "Allow" | ||
| actions = [ | ||
| "ssm:GetParameter", | ||
| "ssm:GetParameters", | ||
| "ssm:GetParametersByPath" | ||
| ] | ||
| 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/*", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah this is another case of just trying to give what they had, but agree its not needed i will remove |
||
| "arn:aws:ssm:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:parameter/ptl/*", | ||
| "arn:aws:ssm:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:parameter/prod/*" | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| resource "aws_iam_policy" "regression_test_permissions" { | ||
| name = "regression-test-permissions" | ||
| description = "Permissions for the regression test GitHub Actions role" | ||
| path = "/service-policies/" | ||
| policy = data.aws_iam_policy_document.regression_test_permissions.json | ||
| } | ||
|
|
||
| # Assume role policy document for GitHub Actions | ||
| data "aws_iam_policy_document" "github_actions_assume_role" { | ||
| statement { | ||
| sid = "OidcAssumeRoleWithWebIdentity" | ||
| effect = "Allow" | ||
| actions = ["sts:AssumeRoleWithWebIdentity"] | ||
|
|
||
| principals { | ||
| type = "Federated" | ||
| identifiers = [ | ||
| aws_iam_openid_connect_provider.github.arn | ||
| ] | ||
| } | ||
|
|
||
| condition { | ||
| test = "StringLike" | ||
| variable = "token.actions.githubusercontent.com:sub" | ||
| values = ["repo:${var.github_org}/${var.github_repo}:*"] | ||
| } | ||
|
|
||
| condition { | ||
| test = "StringEquals" | ||
| variable = "token.actions.githubusercontent.com:aud" | ||
| values = ["sts.amazonaws.com"] | ||
| } | ||
| } | ||
| dynamic "statement" { | ||
| for_each = var.environment == "dev" ? [1] : [] | ||
| content { | ||
| sid = "AllowDevSSORoleToAssumeIamBootstrap" | ||
| effect = "Allow" | ||
| actions = ["sts:AssumeRole"] | ||
|
|
||
| principals { | ||
| type = "AWS" | ||
| identifiers = [ | ||
| local.dev_role_arn | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| # Assume role policy document for GitHub Actions | ||
| data "aws_iam_policy_document" "regression_repo_assume_role" { | ||
| statement { | ||
| sid = "OidcAssumeRoleWithWebIdentity" | ||
| effect = "Allow" | ||
| actions = ["sts:AssumeRoleWithWebIdentity"] | ||
|
|
||
| principals { | ||
| type = "Federated" | ||
| identifiers = [ | ||
| aws_iam_openid_connect_provider.github.arn | ||
| ] | ||
| } | ||
|
|
||
| condition { | ||
| test = "StringLike" | ||
| variable = "token.actions.githubusercontent.com:sub" | ||
| values = ["repo:${var.github_org}/${var.regression_repo}:*"] | ||
| } | ||
|
|
||
| condition { | ||
| test = "StringEquals" | ||
| variable = "token.actions.githubusercontent.com:aud" | ||
| values = ["sts.amazonaws.com"] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| # Attach the policies to the role | ||
| resource "aws_iam_role_policy_attachment" "api_infrastructure" { | ||
| role = aws_iam_role.github_actions.name | ||
|
|
@@ -885,3 +1026,13 @@ resource "aws_iam_role_policy_attachment" "cloudwatch_management" { | |
| role = aws_iam_role.github_actions.name | ||
| policy_arn = aws_iam_policy.cloudwatch_management.arn | ||
| } | ||
|
|
||
| resource "aws_iam_role_policy_attachment" "regression_test_permissions" { | ||
| role = aws_iam_role.regression_test_role.name | ||
| policy_arn = aws_iam_policy.regression_test_permissions.arn | ||
| } | ||
|
|
||
| resource "aws_iam_role_policy_attachment" "regression_security_management" { | ||
| role = aws_iam_role.regression_test_role.name | ||
| policy_arn = aws_iam_policy.security_management.arn | ||
|
eddalmond1 marked this conversation as resolved.
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bin comma