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

chore(deps): consolidate renovate bot dependency fixes #1251

Merged
merged 14 commits into from
May 24, 2024

Conversation

@eeaton eeaton requested review from rjerrems, gtsorbo and a team as code owners May 23, 2024 13:04
@eeaton eeaton enabled auto-merge (squash) May 23, 2024 13:06
@eeaton
Copy link
Collaborator Author

eeaton commented May 23, 2024

@apeabody Can you approve this when you have a chance please?

eeaton added 2 commits May 23, 2024 13:35
…owed_action" argument. (11.0 treats it as required, but should be optional)
…ules/terraform-google-vm#406

No functional change: leaving most_disruptive_allowed_action blank by default should be equivalent to "REPLACE"
Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

LGTM - The only downside is if there is an issue it can be more difficult to pinpoint which update is the source.

@eeaton
Copy link
Collaborator Author

eeaton commented May 23, 2024

/gcbrun

@daniel-cit
Copy link
Contributor

@apeabody @eeaton
integration test need to be updated here

https://github.com/terraform-google-modules/terraform-example-foundation/blob/f22eb6682512468c3da638e1b9a03c5d74f4c2a4/test/integration/networks/networks_test.go#L291C1-L293C6

			"from": map[string]interface{}{
				"identity_type": "ANY_IDENTITY",
			},

to add the sources in the egressPolicies like is is used in the ingressPolicies

			"from": map[string]interface{}{
				"sources": map[string][]string{
					"access_levels": {"*"},
				},
				"identity_type": "ANY_IDENTITY",
			},

to fix

step #16 - "converge-networks":   on .terraform/modules/base_env.restricted_shared_vpc.regular_service_perimeter/modules/regular_service_perimeter/main.tf line 87, in resource "google_access_context_manager_service_perimeter" "regular_service_perimeter":
Step #16 - "converge-networks":   87:           source_restriction = egress_policies.value["from"]["sources"] != null ? "SOURCE_RESTRICTION_ENABLED" : null
Step #16 - "converge-networks":     ‚ egress_policies.value["from"] is object with 1 attribute "identity_type"

and usage of sources should be fixed in https://github.com/terraform-google-modules/terraform-google-vpc-service-controls/blob/master/modules/regular_service_perimeter/main.tf
so that it should not be required.

Maybe from

lookup(egress_policies.value["from"]["sources"], "access_levels", []) 

to

lookup(lookup(egress_policies.value["from"],"sources",{}),"access_levels", [])

…ck (same behavior as already present in ingressPolicies)
@eeaton
Copy link
Collaborator Author

eeaton commented May 24, 2024

Thanks for the guidance Daniel. I've made an additional commit to the tests on this PR and raised the upstream fix as well terraform-google-modules/terraform-google-vpc-service-controls#146

@eeaton eeaton merged commit ee6183f into terraform-google-modules:master May 24, 2024
7 checks passed
@eeaton eeaton deleted the renovatebot-combined branch May 24, 2024 12:12
@fmichaelobrien
Copy link
Contributor

thanks team for the update, I'll add this consolidated module update to a scheduled upstream sync for monday. I agree there may be issues with backtracking regressions - however it is also good to have an effective consolidated minor release out at once so a full clean org deploy to 5-app-infra can be done once. The reduce toil option is appreciated.

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.

4 participants