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

Allow granting network user role on host project from project module and factory #1930

Merged
merged 7 commits into from
Dec 15, 2023

Conversation

simonebruzzechesse
Copy link
Collaborator

@simonebruzzechesse simonebruzzechesse commented Dec 14, 2023

This PR updates Shared VPC config for project factory and project module for fine grained Shared VPC configuration. It adds the following attributes to shared_vpc_service_config variable:

  • subnets_iam: for subnet level IAM binding for non API services identities
  • host_project_iam: for granting network user role to identities other than API services

It update project module documentation adding reference for Shared VPC configuration (IAM permissions assigned at subnet level + org policy for restricting subnets available on the newly created project). It provides example usage of the new attributes in 2 examples (with new test cases).

Fixes #1929

This PR

I applicable, I acknowledge that I have:

  • Read the contributing guide
  • Ran terraform fmt on all modified files
  • Regenerated the relevant README.md files using tools/tfdoc.py
  • Made sure all relevant tests pass

@ludoo
Copy link
Collaborator

ludoo commented Dec 14, 2023

wow you're fast :)

@simonebruzzechesse
Copy link
Collaborator Author

wow you're fast :)

Not that fast.. I started workingon this yesterday! :)

@ludoo ludoo changed the title Update shared vpc config for Project Factory and Project module Allow granting network user role on host project from project module and factory Dec 14, 2023
Copy link
Collaborator

@ludoo ludoo left a comment

Choose a reason for hiding this comment

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

Great work Simo, thanks for this. Dropped a few comments mainly on variable naming and the README wording, once those are addressed this is good to go from my PoV.

@simonebruzzechesse
Copy link
Collaborator Author

Great work Simo, thanks for this. Dropped a few comments mainly on variable naming and the README wording, once those are addressed this is good to go from my PoV.

Thanks for the feedback and comments, I updated the PR with your improvements! WRT what you suggested I just kept the reference example for the Shared VPC being, IMHO, the best configuration which suits most of the use cases. WDYT?

@simonebruzzechesse simonebruzzechesse marked this pull request as ready for review December 15, 2023 12:25
@ludoo
Copy link
Collaborator

ludoo commented Dec 15, 2023

I don't think it's a reference as I wrote twice :) I would just move the org policy to the project-level IAM example, and scrap the last example.

Copy link
Collaborator

@ludoo ludoo left a comment

Choose a reason for hiding this comment

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

Thanks Simo!!!

@simonebruzzechesse simonebruzzechesse merged commit c50b732 into master Dec 15, 2023
@simonebruzzechesse simonebruzzechesse deleted the bruzz/pf-shared-vpc-update branch December 15, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IAM Permissions for Shared VPC Usage in Project Factory
2 participants