fix(VPCSC): enable dryrun mode #1210
Conversation
| restricted_services = var.restricted_services | ||
| vpc_accessible_services = ["RESTRICTED-SERVICES"] | ||
| # configurations for a perimeter in dry run mode. | ||
| resources_dry_run = [var.project_number] |
There was a problem hiding this comment.
the way the projects are added to the perimeter in step 4-projects also needs to be updated.
vpc_service_control_attach_enabled = var.vpc_service_control_attach_enabled
vpc_service_control_perimeter_name = var.vpc_service_control_perimeter_name
vpc_service_control_sleep_duration = var.vpc_service_control_sleep_duration
This cloud be a new flag like how it is coded in the project factory module
There was a problem hiding this comment.
hey Eliot, just wondering if this change could be done with a flag instead of comment/uncomment the code, export in outputs to be read by other steps, like 4-projects as Daniel mentioned. It would also be less tricky for others blueprints that have documentation of how to deploy on top of foundation.
There was a problem hiding this comment.
- This fix also needs to be implemented in the hub and spoke flow
- and also in the simple project module in step 4-projects
- It would be helpful for the user to have a note on the main README and the READMEs from step 3 that the perimeter is created in dry mode and that it need to be updated to start enforcing the rules.
is it possible to have a flag vcp_sc_dry_run that could be set to true in the test instead of editing the code?
3-networks-dual-svpc/modules/restricted_shared_vpc/service_control.tf
Outdated
Show resolved
Hide resolved
…trol.tf Co-authored-by: Daniel Andrade <[email protected]>
|
Thanks Daniel & Amanda for the input. ACK to add the changes consistently across step 4-projects, simple project module, and hub-and-spoke flow re: the flag, like our discussion over chat I realize now we need separate input variables, because it is a valid use case for customers to set the enforced perimeter and dryrun perimeter to separate configurations (they are two different resources managed by the same CFT module). I'll draft this proposal and update the PR. |
|
Quick update: I'm blocked by a provider error on CFT. The project factory module has partially designed support for an argument I've experimented locally with a few different patterns consistently that set the perimeter to dry_run mode in the 3-networks and 4-projects stage, but until the provider is fixed I don't think I can implement it using the current project factory module. |
|
…d outputs, and the enforce_vpcsc flag
…tomatically added to the appropriate dry-run perimeter
…rks-hub-and-spoke, and regenerate variable docs for the hub-and-spoke pattern.
|
I've made a few new commits, but no need to do a full review yet. I've got a working version of configuring the dry-run perimeter in my local environment for the dualsvpc pattern, but haven't tested it fully for hub-and-spoke (just copied off the key differences manually). I want to run the full CI tests to identify any configuration errors I might have introduced. Once that is working well I'll add the readme changes and then ask for a review |
…raform-example-foundation into fix-1209-vpcsc-dryrun
|
/gcbrun |
… Result from ` gcloud beta access-context-manager supported-services list --format='get(NAME)' --filter="SUPPORT_STAGE=GA" `" This reverts commit 600ce9e.
|
/gcbrun |
|
Looks like CI is repeatedly failing here, but I haven't been able to identify a more specific issue in the logs regarding what parts of the TestOrg specifically are failing:
|
|
@eeaton the attribute with the changed value We may try to setup to see if this will solve the issue. I will do a local test to see if this will solve the problem. |
|
Got it, I've started seeing the same error on the other PR about fixing the SA, even though I don't expect that either PR changes things that should impact the TestOrg step, so that does suggest it's a change from the API itself that will break the CI on every branch. |
…alues for plan and apply due to underlying changes in GCF API
Co-authored-by: Daniel Andrade <[email protected]>
|
The tests in networks_test.go passed, but now CI failed for a similar test in projects_tests.go. I've applied similar logic that we used in networks_test.go, to check project membership against the dry-run diff object. |
Co-authored-by: Daniel Andrade <[email protected]>
|
@daniel-cit Woo hoo, all tests are finally green! Thanks again for all the help with this PR. When you have a chance can you review and approve please? |
I will do a deploy to test the upgrade path |
|
@apeabody can I please get your formal review as a code owner to merge? |
|
/gcbrun |
Address issue #1209 .
CI tests have high flaky failure rate due in part to propagation delays with VPCSC perimeter. Changing this to dry mode better adheres to best practices for enablement and will avoid this issue in CI tests.