Skip to content
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.

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

fix: Add roles and service enablements to bootstrap project #1175

Merged

Conversation

obriensystems
Copy link
Contributor

@obriensystems obriensystems commented Mar 27, 2024

Overview

This PR is an initial canary PR that addresses a couple minor service enablement and IAM role requirements during the 0-bootstrap and 1-org subsections that occurred on a clean GCP organization when following the readme instructions deploying the landing zone.
We as a team would like to also determine the formal process ( #1179 ) around PR submission as we add more ER or Bug PRs while we bring up the as-is TEF deployment for future additional functionality

see testing in #1133
specifically

#1136
#1139
#1140
#1142
#1143
#1161

Guidance

This first PR is a preliminary fix for service enablements and roles required during bootstrap of the TEF deployment.
Later in #1144 the readme instructions will be added to a bootstrap.sh script

@obriensystems obriensystems requested review from rjerrems, gtsorbo and a team as code owners March 27, 2024 14:25
@obriensystems obriensystems changed the title Add roles and service enablements to bootstrap project fix: Add roles and service enablements to bootstrap project Mar 27, 2024
@fmichaelobrien
Copy link
Contributor

@fmichaelobrien
Copy link
Contributor

Team, let me know what else needs to be provided in this initial PR to get it merged.

I am working on behalf of 2 Canadian Government organizations (one Federal, one Provincial) wishing to directly use and contribute to the TEF as a default GCP LZ

@fmichaelobrien
Copy link
Contributor

fmichaelobrien commented Apr 4, 2024

Lint ran - fixing whitespace issues in another commit
https://github.com/terraform-google-modules/terraform-example-foundation/actions/runs/8557468090/job/23449822771?pr=1175

./0-bootstrap/README.md:91:     ``` 
./0-bootstrap/README.md:99:     ```  
Error: Trailing whitespace found in the lines above.
Error: The following tests have failed: check_whitespace

@fmichaelobrien
Copy link
Contributor

@obriensystems
Copy link
Contributor Author

obriensystems commented Apr 13, 2024

I'll raise in a separate PR but 3-networks-hub-and-spoke also needs the compute.orgSecurityPolicyAdmin or User or more recently compute.orgFirewallPolicyAdmin IAM role on the super admin to be able to view Hierarchical Firewall Policies created under the common branch

https://cloud.google.com/firewall/docs/firewall-policies#iam
https://cloud.google.com/compute/docs/access/iam#compute.orgFirewallPolicyAdmin

As well as compute api on bootstrap project in step 11 of 3-networks-hub-and-spoke

michael@cloudshell:~/tef-olxyz/github/gcp-networks (tef-olxyz)$ gcloud services enable compute.googleapis.com
Operation "operations/acf.p2-438381210056-2415ed08-fad8-4333-8c8e-1017881efb60" finished successfully.

tested entire 3-networks-hub-and-spoke with these changes including the hierarchical firewall policy retry rename procedure

Screenshot 2024-04-13 at 19 34 33

@eeaton eeaton enabled auto-merge (squash) April 26, 2024 09:37
@eeaton
Copy link
Collaborator

eeaton commented Apr 26, 2024

Thanks for the submission, for the additional roles are LGTM. Can you re-sync the branch?

@obriensystems
Copy link
Contributor Author

sounds good, there are additional sections in the gitlab, github, jenkins build variants that i will separately test/pr later.
i will rebase this branch for the CB/CSR default..

@fmichaelobrien
Copy link
Contributor

main branch merged - waiting on "lint" task report...

@eeaton eeaton merged commit a759ee9 into terraform-google-modules:master Apr 26, 2024
5 checks passed
@fmichaelobrien
Copy link
Contributor

Thank you TEF team - we really appreciate this initial PR!!!
We will be moving over select changes from our fork over the next 4-8 weeks that benefit the wider TEF community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants