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

refactor: remove base vpc and change in perimeter #5

Open
wants to merge 114 commits into
base: master
Choose a base branch
from

Conversation

mariammartins
Copy link
Owner

Hi folks, this PR contains the following network refactoring:

  • perimeter change, now with a single perimeter that includes all foundation projects

  • the removal of the base project, now containing only restricted project

Copy link

@caetano-colin caetano-colin left a comment

Choose a reason for hiding this comment

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

thanks for the PR! I did a first skimming and have some questions / minor changes

@@ -744,7 +744,7 @@ An environment variable `GOOGLE_IMPERSONATE_SERVICE_ACCOUNT` will be set with th

1. Update `common.auto.tfvars` file with values from your GCP environment.
See any of the envs folder [README.md](../3-networks-hub-and-spoke/envs/production/README.md#inputs) files for additional information on the values in the `common.auto.tfvars` file.
1. You must add your user email in the variable `perimeter_additional_members` to be able to see the resources created in the restricted project.
1. You must add your user email in the variable `perimeter_additional_members` to be able to see the resources created in the shared vpc project.

Choose a reason for hiding this comment

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

NIT: maybe this would be more concise?

Suggested change
1. You must add your user email in the variable `perimeter_additional_members` to be able to see the resources created in the shared vpc project.
1. You must add your user email in the variable `perimeter_additional_members` to be able to see the resources that are created in the shared vpc-sc perimeter.

@@ -371,6 +371,23 @@ For example, to create a new business unit similar to business_unit_1, run the f
./tf-wrapper.sh apply development
```

### Include APP Infra Pipeline terraform service account in the perimeter

App Infra Pipeline terraform service account needs to be added to the VPC-SC perimeter.

Choose a reason for hiding this comment

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

suggestion: adding a reason why the app infra pipeline should be added to the perimeter

Choose a reason for hiding this comment

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

question:

  • does this change need to be made for both the cloudbuild and local terraform deployments?
  • it is a pre-requisite?

@@ -168,4 +168,4 @@ default_region_kms = "us"
// For GitHub OAuth see: https://developer.hashicorp.com/terraform/cloud-docs/vcs/github
// For GitHub Enterprise see: https://developer.hashicorp.com/terraform/cloud-docs/vcs/github-enterprise
// For GitLab.com see: https://developer.hashicorp.com/terraform/cloud-docs/vcs/gitlab-com
// For GitLab EE/CE see: https://developer.hashicorp.com/terraform/cloud-docs/vcs/gitlab-eece
// For GitLab EE/CE see: https://developer.hashicorp.com/terraform/cloud-docs/vcs/gitlab-eece
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing empty line at the end of file

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