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.

Sign up for GitHub

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

Jump to bottom

Adds variables for data governance project and bigquery project #124

Merged
merged 40 commits into from
Sep 10, 2021
Merged

Adds variables for data governance project and bigquery project #124

merged 40 commits into from
Sep 10, 2021

Conversation

amandakarina
Copy link
Collaborator

@amandakarina amandakarina commented Aug 31, 2021 •
edited by daniel-cit
Loading

Fixes #24

  • Tests pass
  • Appropriate changes to README are included in PR

@amandakarina
Adds variable for data governance project
5722e14
@amandakarina
Reduces data governance project name
7d6b16e
@amandakarina
Reduces data ingestion project name
edc54d2
@amandakarina
Adds ensure_access_policy.sh script usage
5cc87e9
@amandakarina
Adds project roles for data governance project
74afdfe
@amandakarina
Uses topic full id to grant roles
0b1dbea
@amandakarina
Adds dependency for iam roles assign because of perimiter
11d893a
@amandakarina
Adds dependency for wait vpc propagation
9f8a213
@amandakarina
Expose resources variables on dwt-networking module
e90c141
@amandakarina
Adds dependency for explicit output
4f97efe
@amandakarina
Changes build order
d55fcf7
@amandakarina
Create key on data governance project
3ee2df6
@amandakarina
Removes dependency
a6e04ea
@amandakarina
Adds services to vpc
8f53bfd
@amandakarina
Adds sleep to wait vpc destroy propagation
c9587bb
@amandakarina
Forces wait propagation on destroy
320859d
@amandakarina
Adds dependency for dlp regional, increase time sleep on destroy
423064f
@amandakarina
Adds dependency for external resources on test
ad7fc21
@amandakarina
Removes dependency
c36fa29
@amandakarina
Remove dlp from perimeter
7192c06
@amandakarina
Removes dependency on test
38d3eab
@amandakarina
Removes dependency on test
21bacb8
@amandakarina
Removes dependency
37e8477
@amandakarina amandakarina changed the title Adds variable for data governance project [DO NOT MERGE] Adds variable for data governance project Sep 6, 2021
@amandakarina amandakarina changed the title [DO NOT MERGE] Adds variable for data governance project Adds variables for data governance project and bigquery project Sep 9, 2021
@amandakarina amandakarina marked this pull request as ready for review September 9, 2021 14:48
@amandakarina amandakarina marked this pull request as draft September 9, 2021 17:51
@amandakarina amandakarina marked this pull request as ready for review September 9, 2021 19:17
@daniel-cit daniel-cit self-requested a review September 9, 2021 20:45
daniel-cit
daniel-cit approved these changes Sep 9, 2021
View reviewed changes
Copy link
Collaborator

@daniel-cit daniel-cit left a comment

Choose a reason for hiding this comment

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

LGTM

erlanderlo
erlanderlo reviewed Sep 10, 2021
View reviewed changes
Copy link
Member

@erlanderlo erlanderlo left a comment

Choose a reason for hiding this comment

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

initial review comments

modules/base-data-ingestion/vpc_service_control.tf Show resolved Hide resolved
modules/base-data-ingestion/vpc_service_control.tf
@@ -34,13 +34,15 @@ module "dwh_networking" {
subnet_ip = var.subnet_ip
perimeter_members = local.perimeter_members
commom_suffix = random_id.suffix.hex
resources = [data.google_project.ingestion_project.number, data.google_project.governance_project.number, data.google_project.datalake_project.number]

restricted_services = [
Copy link
Member

Choose a reason for hiding this comment

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

consider adding the other services mentioned here:
https://github.com/GoogleCloudPlatform/notebooks-blueprint-security/blob/main/main.tf#L200

That are relevant (for instance notebook api's prob are not)

test/setup/main.tf Outdated
"serviceusage.googleapis.com",
"iam.googleapis.com",
"dns.googleapis.com",
"pubsub.googleapis.com",
Copy link
Member

Choose a reason for hiding this comment

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

wondering if this is copy/pasted for the project for BQ...

Are the following really needed for BQ project?

  • pubsub
  • dataflow
  • dlp
  • cloud scheduler
  • appengine
  • cloudbuild
  • compute

erlanderlo
erlanderlo approved these changes Sep 10, 2021
View reviewed changes
Copy link
Member

@erlanderlo erlanderlo left a comment

Choose a reason for hiding this comment

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

LGTM

@bharathkkb bharathkkb merged commit e68e760 into GoogleCloudPlatform:main Sep 10, 2021
@amandakarina amandakarina deleted the feat/enables-all-vpc-service-perimeter-bridges branch September 20, 2021 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniel-cit daniel-cit daniel-cit approved these changes

@erlanderlo erlanderlo erlanderlo approved these changes

@bharathkkb bharathkkb bharathkkb approved these changes

@morgante morgante Awaiting requested review from morgante

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

enable all vpc-sc bridges
4 participants
@amandakarina @bharathkkb @erlanderlo @daniel-cit