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

feat!: adds support to multiple service projects and Shared VPC #115

Merged
merged 8 commits into from
May 5, 2023
Merged

feat!: adds support to multiple service projects and Shared VPC #115

merged 8 commits into from
May 5, 2023

Conversation

amandakarina
Copy link
Collaborator

Hey folks,

this PR adds support to multiple services project and the creation of a Shared VPC in a network project in the harness sub-module.

Could you, please, take a look on it?

@amandakarina amandakarina changed the title feat!: adds support to multiple services project and Shared VPC feat!: adds support to multiple service projects and Shared VPC Apr 20, 2023
@amandakarina amandakarina force-pushed the feat/adds-support-to-multiple-services-project-and-shared-vpc branch 15 times, most recently from afe6228 to 060e732 Compare April 24, 2023 17:47
@amandakarina amandakarina force-pushed the feat/adds-support-to-multiple-services-project-and-shared-vpc branch from 060e732 to a9a8297 Compare April 24, 2023 19:29
@amandakarina amandakarina force-pushed the feat/adds-support-to-multiple-services-project-and-shared-vpc branch from 3896c72 to 7bbffea Compare April 25, 2023 19:44
@amandakarina amandakarina mentioned this pull request Apr 26, 2023
Closed
gtsorbo
gtsorbo requested changes Apr 27, 2023
View reviewed changes
Copy link
Collaborator

@gtsorbo gtsorbo left a comment

Choose a reason for hiding this comment

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

A few suggestions, and waiting on @bharathkkb with regards to renaming submodules

modules/secure-serverless-harness/versions.tf Outdated Show resolved Hide resolved
modules/secure-serverless-net/variables.tf Outdated Show resolved Hide resolved
@bharathkkb
Copy link
Member

I have not tested this before but I believe what will happen is TF registry will stop tracking old version after current release and these will be added as new submodules. I generally don't recommend this unless there is a strong rationale.

Like Grant mentioned, we should rename the versions.tf to reflect new name though.

gtsorbo
gtsorbo approved these changes May 1, 2023
View reviewed changes
Copy link
Collaborator

@gtsorbo gtsorbo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Add validation to harness as well, then LGTM

Please write some upgrade notes regarding the submodule name change to inform users of navigating the breaking change

modules/secure-serverless-harness/variables.tf Outdated Show resolved Hide resolved
@amandakarina
Copy link
Collaborator Author

Add validation to harness as well, then LGTM

Please write some upgrade notes regarding the submodule name change to inform users of navigating the breaking change

hey @gtsorbo where should I put these notes? in the modules readmes?

@gtsorbo
Copy link
Collaborator

gtsorbo commented May 2, 2023

Add validation to harness as well, then LGTM
Please write some upgrade notes regarding the submodule name change to inform users of navigating the breaking change

hey @gtsorbo where should I put these notes? in the modules readmes?
@amandakarina Can you make a short upgrade guide similar to this: https://github.com/terraform-google-modules/terraform-google-lb-http/blob/master/docs/upgrading_to_v8.0.md

@amandakarina amandakarina requested a review from gtsorbo May 2, 2023 16:55
@amandakarina amandakarina force-pushed the feat/adds-support-to-multiple-services-project-and-shared-vpc branch 6 times, most recently from 75a63c0 to 72102ab Compare May 3, 2023 18:29
@amandakarina amandakarina force-pushed the feat/adds-support-to-multiple-services-project-and-shared-vpc branch from 72102ab to c430803 Compare May 3, 2023 18:35
@amandakarina
Copy link
Collaborator Author

/gcbrun

@gtsorbo gtsorbo merged commit bc1b8b1 into GoogleCloudPlatform:main May 5, 2023
@release-please release-please bot mentioned this pull request May 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtsorbo gtsorbo gtsorbo approved these changes

@prabhu34 prabhu34 Awaiting requested review from prabhu34 prabhu34 is a code owner

@anamer anamer Awaiting requested review from anamer anamer is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@amandakarina @bharathkkb @gtsorbo