Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

feat(delivery): document roles used in CI pipeline #489

Merged
merged 13 commits into from
Jun 8, 2022
Merged

Conversation

ace-n
Copy link
Contributor

@ace-n ace-n commented Jun 6, 2022

DO NOT MERGE until this is tested (which requires some other PRs)

Fixes #473

@github-actions github-actions bot added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. component: delivery Related to automation, testing, deployment of the application. labels Jun 6, 2022
@ace-n ace-n added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 6, 2022
@github-actions github-actions bot added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jun 6, 2022
@ace-n ace-n self-assigned this Jun 6, 2022
terraform/modules/emblem-app/main.tf Outdated Show resolved Hide resolved
terraform/modules/emblem-app/main.tf Outdated Show resolved Hide resolved
]
}

resource "google_project_iam_member" "delivery_storage_object_admin_granting_iam_member" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use google_storage_bucket_iam_member or similar to bind this role to a single bucket without the need for as many conditions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EDIT: This is actually a granting permission (permission to grant the storage role), not a storage permission.

AFAICT, this should be kept as-is. 🙂

terraform/modules/emblem-app/main.tf Outdated Show resolved Hide resolved
terraform/modules/emblem-app/main.tf Outdated Show resolved Hide resolved
terraform/modules/emblem-app/main.tf Outdated Show resolved Hide resolved
terraform/modules/ops/main.tf Outdated Show resolved Hide resolved
@ace-n ace-n requested a review from grayside June 8, 2022 00:41
@ace-n
Copy link
Contributor Author

ace-n commented Jun 8, 2022

I've confirmed this works (locally) when run with tf apply.

Once this and #482 are merged, I'll confirm this works on a "from-scratch" E2E test run.

@ace-n ace-n marked this pull request as ready for review June 8, 2022 00:44
@ace-n ace-n requested a review from a team as a code owner June 8, 2022 00:44
@ace-n ace-n removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 8, 2022
@github-actions github-actions bot added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jun 8, 2022
@ace-n
Copy link
Contributor Author

ace-n commented Jun 8, 2022

@grayside AFAICT (based on mirroring these changes into #482) this largely works now, and should be good to review.

(As mentioned above, I'll do "acceptance testing" post-merge. If changes are needed, they'll be in a separate PR.)

@ace-n ace-n dismissed grayside’s stale review June 8, 2022 02:36

Subsequent review requested

@ace-n ace-n merged commit e794665 into main Jun 8, 2022
@ace-n ace-n deleted the delivery-iam branch June 8, 2022 05:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: delivery Related to automation, testing, deployment of the application. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ops: document Delivery CI-required permissions in Terraform
2 participants