Skip to content

Navigation Menu

Sign in
Sign up

added example of github pushing to s3, using oidc - #295

Open
dlavender4 wants to merge 5 commits into
main from
github-oidc
Open

added example of github pushing to s3, using oidc #295
dlavender4 wants to merge 5 commits into
main from
github-oidc

Conversation

@dlavender4

@dlavender4 dlavender4 commented Apr 18, 2023

Copy link
Copy Markdown
Contributor

No description provided.

dlavender4 requested a review from a team as a code owner April 18, 2023 09:39

Copy link
Copy Markdown

@andyblundell andyblundell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good - needs referencing from somewhere though, not sure there's anywhere suitable yet, maybe we need a new list of handy guides for specific stuff like this?

11Error: Process completed with exit code 1.

Ensure Role: fails when has wrong GitHub Repo name:
- AWS: edit "GitHubS3DeployRole" Trust Policy. Change GitHub Repo in "token.actions.githubusercontent.com:sub" to something random: "repo:NHSDigitalWRONG/tech-radar:ref:refs/heads/main"

@andyblundell andyblundell Apr 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change to RandomOrg/RandomRepo or similar? (To remove reference to radar repo)

- But took 2min, seemed to be timing out / retrying

Ensure Role fails when has wrong GitHub Branch name:
- AWS: edit "GitHubS3DeployRole" Trust Policy. Change GitHub branch in "token.actions.githubusercontent.com:sub" to something random: "repo:NHSDigital/tech-radar:ref:refs/heads/mainWRONG"

@andyblundell andyblundell Apr 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change to RandomOrg/RandomRepo or similar? (To remove reference to radar repo)

@amaanibn-nasar1-nhs amaanibn-nasar1-nhs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth noting that I've done some work to create a reusable terraform module that does this. https://github.com/NHSDigital/ee-terraform-modules

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

Reviewers

@andyblundell andyblundell andyblundell requested changes
@walteck walteck Awaiting requested review from walteck
+1 more reviewer
@amaanibn-nasar1-nhs amaanibn-nasar1-nhs amaanibn-nasar1-nhs left review comments
Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /