Skip to content

chore(ci): aws runner: switch to OIDC-based assuming role #2081

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.

Sign up for GitHub

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

Jump to bottom
Merged
fuyufjh merged 4 commits into from
Apr 24, 2022
Merged

chore(ci): aws runner: switch to OIDC-based assuming role #2081

fuyufjh merged 4 commits into from
Apr 24, 2022

Conversation

@fuyufjh
Copy link
Collaborator

@fuyufjh fuyufjh commented Apr 24, 2022 •
edited
Loading

What's changed and what's your intention?

After this PR, EC2 for CI workflow will be created in the following way, which is recommended by AWS officially.

  1. GitHub Action servers use OIDC (OpenID Connect) to connect to AWS
  2. AWS checks the request must be from this organization and this repo
  3. AWS allocates a new short-term IAM account for this CI run only. It only lives for 1 hour.
  4. The GitHub Action continues to execute other steps with this IAM account

After that, the community developer should be able to run CI easily. I'll test this later.

References:

  1. https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-amazon-web-services
  2. https://github.com/aws-actions/configure-aws-credentials#assuming-a-role

Checklist

N/A

Refer to a related PR or issue link (optional)

closes #1339

@codecov
Copy link

codecov bot commented Apr 24, 2022 •
edited
Loading

Codecov Report

Merging #2081 (45c58d5) into main (abb87c3) will increase coverage by 0.10%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2081      +/-   ##
==========================================
+ Coverage   70.93%   71.04%   +0.10%     
==========================================
  Files         633      635       +2     
  Lines       81126    81633     +507     
==========================================
+ Hits        57548    57993     +445     
- Misses      23578    23640      +62
Flag Coverage Δ
rust 71.04% <ø> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/batch/src/executor/executor2_wrapper.rs 41.02% <0.00%> (-35.90%) ⬇️
src/cmd/src/bin/frontend_node.rs 0.00% <0.00%> (-16.67%) ⬇️
src/stream/src/executor_v2/hop_window.rs 66.82% <0.00%> (-13.74%) ⬇️
src/meta/src/model/hash_mapping.rs 75.00% <0.00%> (-8.73%) ⬇️
src/meta/src/stream/stream_manager.rs 61.04% <0.00%> (-8.52%) ⬇️
src/common/src/types/interval.rs 73.36% <0.00%> (-5.56%) ⬇️
src/meta/src/model/stream.rs 51.80% <0.00%> (-3.11%) ⬇️
src/meta/src/hummock/level_handler.rs 97.95% <0.00%> (-2.05%) ⬇️
src/meta/src/hummock/compaction.rs 76.50% <0.00%> (-0.79%) ⬇️
src/meta/src/stream/test_fragmenter.rs 99.26% <0.00%> (-0.37%) ⬇️
... and 78 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@fuyufjh fuyufjh marked this pull request as ready for review April 24, 2022 08:16
@fuyufjh fuyufjh requested review from neverchanje and skyzh April 24, 2022 08:16
@fuyufjh fuyufjh merged commit e5f1f8f into main Apr 24, 2022
@fuyufjh fuyufjh deleted the eric/use-assuming-role-in-aws-runner branch April 24, 2022 09:07
@fuyufjh
Copy link
Collaborator Author

fuyufjh commented Apr 24, 2022

Unluckily, this approach still does not works for pull request from forked repos (the community developers). 😇 Here is the reason:

  1. The job or workflow run requires a permissions setting with id-token: write. You won’t be able to request the OIDC JWT ID token if the permissions setting for id-token is set to read or none. Ref
  2. However, maximum access by forked repos of id-token is read.  Ref

It’s possible to send write tokens to workflows from pull requests - allows pull requests from forks to use a GITHUB_TOKEN with write permission. But, this is only available for private repository forks. Ref

@skyzh
Copy link
Contributor

skyzh commented Apr 24, 2022

Let's wait for switching to a new CI pipeline, or simply still go with the pull-and-push way.

@fuyufjh
Copy link
Collaborator Author

fuyufjh commented Apr 24, 2022

Let's wait for switching to a new CI pipeline, or simply still go with the pull-and-push way.

Agree. I think we can keep using the manual pull-and-push way mentioned in CONTRIBUTING.md, until switching to new CI pipeline later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@neverchanje neverchanje Awaiting requested review from neverchanje

1 more reviewer

@skyzh skyzh skyzh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/chore

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ci: setup community CI

3 participants

@fuyufjh @skyzh