pathogen-repo-build: support assuming AWS role for runtime permissions #81

joverlee521 · 2024-05-10T20:38:36Z

Assumes repo-specific roles, GitHubActionsRoleNextstrainRepo@zika for example, which are managed by the Terraform configuration in the nextstrain/infra repo. The repo here is always the calling repository, regardless of if the "repo" input was provided to use workflows from another place.

The runtime credentials are then saved in an envdir that is passed to the build command via NEXTSTRAIN_RUNTIME_ENVDIRS.

Namespaced NEXTSTRAIN_RUNTIME_ENVDIR with ./git/nextstrain as suggested by @tsibley in review¹

¹ #81 (comment)

Related-to: nextstrain/infra#4
Co-authored-by: Thomas Sibley [email protected]

Checklist

Checks pass

Resolves: <#4> Related-to: <nextstrain/.github#81>

.github/workflows/pathogen-repo-build.yaml

.github/workflows/pathogen-repo-build.yaml.in

text-templates/repo-aws-iam-policy.json

Resolves: <#4> Related-to: <nextstrain/.github#81>

@tsibley

Assumes the role `GitHubActionsRoleNextstrainS3Access` (just a placeholder name, final name TDB in nextstrain/infra#4) and scopes repo specific permissions with an inline session policy. The runtime credentials are then saved in an envdir that is passed to the build command via `NEXTSTRAIN_RUNTIME_ENVDIRS`. Namespaced `NEXTSTRAIN_RUNTIME_ENVDIR` with `./git/nextstrain` as suggested by @tsibley in review¹ ¹ <#81 (comment)> pathogen-repo-build:

Static role permissions policies which are expected to be narrowed/scoped-down by an inline session policy set by the pathogen-repo-build workflow. Problems with this approach noted in review commentary: - <nextstrain/.github#81 (comment)> - <nextstrain/.github#81 (comment)> Resolves: <#4> Related-to: <nextstrain/.github#81>

Let's us map GitHub token claims to principal tags, which is great. No need to customize the "sub" claim any more, and granular matching. But ultimately not workable because GitHub OIDC doesn't include the repository name _sans_ owner in its token claims (e.g. "zika" for "nextstrain/zika"), so we don't have any policy variable to interpolate into policy conditions. I thought maybe we could (ab)use role session names for this and pass the unqualified repo name via it ourselves, but that hit a wall because while sts:RoleSessionName is available in the AssumeRoleWithWebIdentity request, it's not available on subsequent requests which we need to be able to use in policy conditions. Then I thought maybe we could use sts:SourceIdentity instead in the role assumption request which is available as aws:SourceIdentity in subsequent requests, except that GitHub's token would have to include an "https://aws.amazonaws.com/source_identity" claim, and it ofc does not provide any way to do that. If it did, it would likely also let us provide a custom "https://aws.amazonaws.com/tags" claim at which point all of this nonsense would be moot anyway. Finally, I thought maybe we could use a custom audience ("aud" claim) which GitHub _does_ let you specify. This is available in AWS requests as the "token.actions.githubusercontent.com:aud" key and we can use it as a policy variable. \o/ But to use it we'd need to cut out Cognito Identity (e.g. go back to where we started without it) and lose the token claims → principal tags mapping. So we'd need to customize the GitHub token's "sub" claim and match against it. We'd also need to explicitly list out all possible valid "aud" on the AWS OIDC IdP configuration (as "client_id_list"), and that would be frustrating. This all feels more complex than it's worth at this point. Resolves: <#4> Related-to: <nextstrain/.github#81>

tsibley · 2024-05-17T19:08:50Z

For posterity, two false starts (failed prototypes/ideas) on the AWS side of this:

@tsibley

Assumes repo-specific roles, `GitHubActionsRoleNextstrainBuild@zika` for example, which are managed by the Terraform configuration in the nextstrain/infra repo. The runtime credentials are then saved in an envdir that is passed to the build command via `NEXTSTRAIN_RUNTIME_ENVDIRS`. Namespaced `NEXTSTRAIN_RUNTIME_ENVDIR` with `./git/nextstrain` as suggested by @tsibley in review¹ ¹ <#81 (comment)> Related-to: <nextstrain/infra#4> Co-authored-by: Thomas Sibley <[email protected]>

tsibley · 2024-05-17T22:14:59Z

Re-pushed with the (not yet working) code to do what I'm planning on supporting over on the AWS side.

TKTK Resolves: <#4> Related-to: <nextstrain/.github#81>

TKTK Resolves: <#4> Related-to: <nextstrain/.github#81> Supersedes: <#6>, <#7>

TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>

@tsibley

Assumes repo-specific roles, `GitHubActionsRoleNextstrainBuild@zika` for example, which are managed by the Terraform configuration in the nextstrain/infra repo. The runtime credentials are then saved in an envdir that is passed to the build command via `NEXTSTRAIN_RUNTIME_ENVDIRS`. Namespaced `NEXTSTRAIN_RUNTIME_ENVDIR` with `./git/nextstrain` as suggested by @tsibley in review¹ ¹ <#81 (comment)> Related-to: <nextstrain/infra#4> Co-authored-by: Thomas Sibley <[email protected]>

tsibley · 2024-05-20T22:46:57Z

nextstrain/infra#8 is where I ended up.

TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>

@tsibley

Assumes repo-specific roles, `GitHubActionsRoleNextstrainRepo@zika` for example, which are managed by the Terraform configuration in the nextstrain/infra repo. The repo here is always the _calling_ repository, regardless of if the "repo" input was provided to use workflows from another place. The runtime credentials are then saved in an envdir that is passed to the build command via `NEXTSTRAIN_RUNTIME_ENVDIRS`. Namespaced `NEXTSTRAIN_RUNTIME_ENVDIR` with `./git/nextstrain` as suggested by @tsibley in review¹ ¹ <#81 (comment)> Related-to: <nextstrain/infra#4> Co-authored-by: Thomas Sibley <[email protected]>

@tsibley

Assumes repo-specific roles, `GitHubActionsRoleNextstrainRepo@zika` for example, which are managed by the Terraform configuration in the nextstrain/infra repo. The repo here is always the _calling_ repository, regardless of if the "repo" input was provided to use workflows from another place. The runtime credentials are then saved in an envdir that is passed to the build command via `NEXTSTRAIN_RUNTIME_ENVDIRS`. Namespaced `NEXTSTRAIN_RUNTIME_ENVDIR` with `./git/nextstrain` as suggested by @tsibley in review¹ ¹ <#81 (comment)> Related-to: <nextstrain/infra#4> Co-authored-by: Thomas Sibley <[email protected]>

The script was copied from the ncov-ingest repo <https://github.com/nextstrain/ncov-ingest/blob/20b432624ad2c4bf24c85756bd04bae1e067bde8/bin/write-envdir> Adding in preparation for configuring separate AWS credentials for the build runtime in pathogen-repo-build.

Renamed to `setup-aws-batch-credentials` in preparation for adding another step that sets up AWS credentials for the runtime. The new step for setting up runtime AWS credentials probably won't need an anchor as it should only run once before the build starts. I just wanted to be specific with the existing anchor name to prevent any potential confusion.

@tsibley

Assumes repo-specific roles, `GitHubActionsRoleNextstrainRepo@zika` for example, which are managed by the Terraform configuration in the nextstrain/infra repo. The repo here is always the _calling_ repository, regardless of if the "repo" input was provided to use workflows from another place. The runtime credentials are then saved in an envdir that is passed to the build command via `NEXTSTRAIN_RUNTIME_ENVDIRS`. Namespaced `NEXTSTRAIN_RUNTIME_ENVDIR` with `./git/nextstrain` as suggested by @tsibley in review¹ ¹ <#81 (comment)> Related-to: <nextstrain/infra#4> Co-authored-by: Thomas Sibley <[email protected]>

tsibley · 2024-05-21T00:00:54Z

I've reworked this a bit to successfully integrate with nextstrain/infra#8.

A collection of templated repo-specific roles for inside a Nextstrain runtime and one role for the GitHub Actions job itself, i.e. outside the runtime. The inside-the-runtime roles are given pathogen-specific permissions necessary for the ingest and phylogenetic workflows of a pathogen repo. The outside-the-runtime role is only necessary/used at the moment for access to AWS Batch. The roles can only be assumed by specific repos when performed by our centralized pathogen-repo-build workflow. While this doesn't completely prevent off-label use by other GitHub Actions workflows launched from a pathogen repo, it does make it more involved to do so, hopefully to the point of discouragement. The associated GitHub repository configuration is managed by Terraform now as well since the customization of the "sub" claim in GitHub Action's OIDC token is tightly coupled to our AWS role trust policies. Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>

joverlee521

Thanks for pushing this over the finish line @tsibley 🙌

.github/workflows/pathogen-repo-build.yaml.in

@tsibley

Assumes repo-specific roles, `GitHubActionsRoleNextstrainRepo@zika` for example, which are managed by the Terraform configuration in the nextstrain/infra repo. The repo here is always the _calling_ repository, regardless of if the "repo" input was provided to use workflows from another place. The runtime credentials are then saved in an envdir that is passed to the build command via `NEXTSTRAIN_RUNTIME_ENVDIRS`. Namespaced `NEXTSTRAIN_RUNTIME_ENVDIR` with `./git/nextstrain` as suggested by @tsibley in review¹ ¹ <#81 (comment)> Related-to: <nextstrain/infra#4> Co-authored-by: Thomas Sibley <[email protected]>

…g credentials Very helpful for troubleshooting when looking at logs. Note that when using the AWS Batch runtime the credential identity will only be emitted for the initial job submission, not subsequent wait-N jobs. This seems fine.

…ials steps So they are more easily differentiated/recognized in the job logs. I found myself easily mixing them up when trying to find the right one.

tsibley · 2024-05-21T23:54:16Z

I think I'll plan to merge this in the morning... so I can deal with the fallout.

tsibley · 2024-05-22T19:19:30Z

Merged. I believe the new work will really only start being used once we remove existing static credentials from repos.

tsibley · 2024-05-22T21:01:00Z

Test run of zika's ingest-to-phylogenetic.yaml failed during the write-envdir step with:

mkdir: created directory '.git/nextstrain/env.d'
Wrote .git/nextstrain/env.d/AWS_ACCESS_KEY_ID
.git/nextstrain/.github/bin/write-envdir: line 20: !name: unbound variable
Wrote .git/nextstrain/env.d/AWS_SECRET_ACCESS_KEY

The out-of-order stdout/stderr merge to logs makes it a little confusing, but the access keys were successfully written and it failed on AWS_SESSION_TOKEN. That env var doesn't exist because static IAM user access keys were used instead of temporary session keys.

I'll push a fix.

tsibley · 2024-05-22T21:45:55Z

Fixed by #88 (and a better error with #87).

This comment was marked as outdated.

Sign in to view

joverlee521 force-pushed the aws-role-policies branch from 1fae489 to 59027ff Compare May 10, 2024 20:42

joverlee521 mentioned this pull request May 10, 2024

Create new role for GH Action access to S3 nextstrain/infra#4

Closed

tsibley self-requested a review May 14, 2024 23:50

tsibley added a commit to nextstrain/infra that referenced this pull request May 15, 2024

wip! Roles for our pathogen-repo-build GitHub Actions workflow

002e2d7

Resolves: <#4> Related-to: <nextstrain/.github#81>

tsibley mentioned this pull request May 15, 2024

Roles for our pathogen-repo-build GitHub Actions workflow nextstrain/infra#6

Closed

1 task

tsibley reviewed May 15, 2024

View reviewed changes

tsibley added a commit to nextstrain/infra that referenced this pull request May 16, 2024

wip! Roles for our pathogen-repo-build GitHub Actions workflow

9a30261

Resolves: <#4> Related-to: <nextstrain/.github#81>

joverlee521 force-pushed the aws-role-policies branch from 30fd636 to b2c38a0 Compare May 16, 2024 22:56

tsibley mentioned this pull request May 17, 2024

Roles for our pathogen-repo-build GitHub Actions workflow, take 2 nextstrain/infra#7

Closed

tsibley force-pushed the aws-role-policies branch from b2c38a0 to 58ba6a0 Compare May 17, 2024 22:13

tsibley added a commit to nextstrain/infra that referenced this pull request May 20, 2024

wip! Roles for our pathogen-repo-build GitHub Actions workflow

374d37e

TKTK Resolves: <#4> Related-to: <nextstrain/.github#81>

tsibley mentioned this pull request May 20, 2024

Roles for our pathogen-repo-build GitHub Actions workflow, take 3 nextstrain/infra#8

Merged

1 task

tsibley added a commit to nextstrain/infra that referenced this pull request May 20, 2024

wip! Roles for our pathogen-repo-build GitHub Actions workflow

4d2114e

TKTK Resolves: <#4> Related-to: <nextstrain/.github#81> Supersedes: <#6>, <#7>

tsibley added a commit to nextstrain/infra that referenced this pull request May 20, 2024

wip! Roles for our pathogen-repo-build GitHub Actions workflow

4dac01b

TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>

tsibley force-pushed the aws-role-policies branch from 58ba6a0 to 619606b Compare May 20, 2024 22:46

tsibley added a commit to nextstrain/infra that referenced this pull request May 20, 2024

wip! Roles for our pathogen-repo-build GitHub Actions workflow

868c7e9

TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>

tsibley added a commit to nextstrain/infra that referenced this pull request May 20, 2024

wip! Roles for our pathogen-repo-build GitHub Actions workflow

ddc08cf

TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>

tsibley force-pushed the aws-role-policies branch from 619606b to 83abe0d Compare May 20, 2024 23:34

tsibley force-pushed the aws-role-policies branch from 83abe0d to eec1d61 Compare May 20, 2024 23:36

joverlee521 added 2 commits May 20, 2024 16:41

Add bin/write-envdir

7430cef

The script was copied from the ncov-ingest repo <https://github.com/nextstrain/ncov-ingest/blob/20b432624ad2c4bf24c85756bd04bae1e067bde8/bin/write-envdir> Adding in preparation for configuring separate AWS credentials for the build runtime in pathogen-repo-build.

tsibley force-pushed the aws-role-policies branch from eec1d61 to 009b8d8 Compare May 20, 2024 23:42

tsibley requested a review from a team May 21, 2024 16:08

tsibley marked this pull request as ready for review May 21, 2024 16:09

joverlee521 commented May 21, 2024

View reviewed changes

.github/workflows/pathogen-repo-build.yaml.in Outdated Show resolved Hide resolved

.github/workflows/pathogen-repo-build.yaml.in Outdated Show resolved Hide resolved

joverlee521 and others added 3 commits May 21, 2024 15:37

pathogen-repo-build: Rename the two aws-actions/configure-aws-credent…

3a2d597

…ials steps So they are more easily differentiated/recognized in the job logs. I found myself easily mixing them up when trying to find the right one.

tsibley force-pushed the aws-role-policies branch from cf5c523 to 3a2d597 Compare May 21, 2024 22:43

tsibley merged commit d1f65a9 into master May 22, 2024
38 checks passed

tsibley deleted the aws-role-policies branch May 22, 2024 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pathogen-repo-build: support assuming AWS role for runtime permissions #81

pathogen-repo-build: support assuming AWS role for runtime permissions #81

joverlee521 commented May 10, 2024 •

edited by tsibley

Loading

This comment was marked as outdated.

tsibley commented May 17, 2024

tsibley commented May 17, 2024

tsibley commented May 20, 2024

tsibley commented May 21, 2024

joverlee521 left a comment

tsibley commented May 21, 2024 •

edited

Loading

tsibley commented May 22, 2024

tsibley commented May 22, 2024

tsibley commented May 22, 2024

pathogen-repo-build: support assuming AWS role for runtime permissions #81

pathogen-repo-build: support assuming AWS role for runtime permissions #81

Conversation

joverlee521 commented May 10, 2024 • edited by tsibley Loading

Checklist

This comment was marked as outdated.

tsibley commented May 17, 2024

tsibley commented May 17, 2024

tsibley commented May 20, 2024

tsibley commented May 21, 2024

joverlee521 left a comment

Choose a reason for hiding this comment

tsibley commented May 21, 2024 • edited Loading

tsibley commented May 22, 2024

tsibley commented May 22, 2024

tsibley commented May 22, 2024

joverlee521 commented May 10, 2024 •

edited by tsibley

Loading

tsibley commented May 21, 2024 •

edited

Loading