Skip to content

Commit

Permalink
Merge pull request #2530 from guardian/support-multiple-GuGithubActio…
Browse files Browse the repository at this point in the history
…nsRole-instances-in-a-single-account

Support multiple usages of `GuGithubActionsRole` in a single AWS account
  • Loading branch information
rtyley authored Dec 5, 2024
2 parents c5b94ef + d3259e7 commit 19be12c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 16 deletions.
16 changes: 16 additions & 0 deletions .changeset/friendly-trainers-cry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@guardian/cdk": major
---

Support multiple usages of `GuGithubActionsRole` in a single AWS account

This significantly changes the resources constructed by `GuGithubActionsRole`,
specifically, **the construct will no longer instantiate a `GitHubOidcProvider`**.
This is because you can only ever have one `GitHubOidcProvider` per provider
domain (ie `token.actions.githubusercontent.com`) - while we may want multiple
instances of `GuGithubActionsRole` in an AWS account, we can't have the
`GuGithubActionsRole` construct trying to make a new `GitHubOidcProvider` with
each instance.

Consequently, you will need to instantiate the `GitHubOidcProvider` elsewhere
as a singleton. At the Guardian, we do this with https://github.com/guardian/aws-account-setup.
24 changes: 10 additions & 14 deletions src/constructs/iam/roles/__snapshots__/github-actions.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,6 @@ exports[`The GitHubActionsRole construct should create the correct resources wit
},
"Type": "AWS::IAM::Policy",
},
"GithubActionsOidc": {
"Properties": {
"ClientIdList": [
"sts.amazonaws.com",
],
"ThumbprintList": [
"6938fd4d98bab03faadb97b34396831e3780aea1",
"1c58a3a8518e8759bf075b76b750d4f2df264fcd",
],
"Url": "https://token.actions.githubusercontent.com",
},
"Type": "AWS::IAM::OIDCProvider",
},
"GithubActionsRoleF5CC769F": {
"Properties": {
"AssumeRolePolicyDocument": {
Expand All @@ -69,7 +56,16 @@ exports[`The GitHubActionsRole construct should create the correct resources wit
"Effect": "Allow",
"Principal": {
"Federated": {
"Ref": "GithubActionsOidc",
"Fn::Join": [
"",
[
"arn:aws:iam::",
{
"Ref": "AWS::AccountId",
},
":oidc-provider/token.actions.githubusercontent.com",
],
],
},
},
},
Expand Down
8 changes: 6 additions & 2 deletions src/constructs/iam/roles/github-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ export interface GuGithubActionsRoleProps {
}

/*
Note you can only have one of these per AWS account - `OIDCProvider`s are keyed by the
provider domain, ie `token.actions.githubusercontent.com`, so this must be instantiated
as a singleton. At the Guardian we do this in https://github.com/guardian/aws-account-setup .
AWS CDK implements an OIDCProvider as a custom resource.
This requires a lambda to be deployed into the account.
As far as I can tell, the lambda is automating the setting of `ThumbprintList`, which is quite generic.
Expand All @@ -63,7 +67,7 @@ See:
- https://github.com/aws/aws-cdk/blob/851c8ca9989856fa61496ff113f9cb8c66d02f3b/packages/%40aws-cdk/aws-iam/lib/oidc-provider.ts
- https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc.html
*/
class GitHubOidcProvider extends CfnResource {
export class GitHubOidcProvider extends CfnResource {
constructor(scope: GuStack) {
super(scope, "GithubActionsOidc", {
type: "AWS::IAM::OIDCProvider",
Expand All @@ -89,7 +93,7 @@ export class GuGithubActionsRole extends GuRole {
constructor(scope: GuStack, props: GuGithubActionsRoleProps) {
super(scope, "GithubActionsRole", {
assumedBy: new FederatedPrincipal(
new GitHubOidcProvider(scope).ref,
`arn:aws:iam::${scope.account}:oidc-provider/${GITHUB_ACTIONS_ID_TOKEN_REQUEST_DOMAIN}`,
{
StringLike: {
[`${GITHUB_ACTIONS_ID_TOKEN_REQUEST_DOMAIN}:sub`]: GuGithubActionsRepositoryCondition.toString(
Expand Down

0 comments on commit 19be12c

Please sign in to comment.