terraform-google-modules / terraform-example-foundation

Shows how the CFT modules can be composed to build a secure cloud foundation
https://cloud.google.com/architecture/security-foundations
Apache License 2.0
1.22k stars 714 forks source link

chore: create access context manager policy ID in test if needed #1252

Closed daniel-cit closed 4 months ago

daniel-cit commented 4 months ago

This PR will toggle the creation of the access context manager policy ID in the test in step 1-org if it does not exist in the organization.

This will fix this errors in integration test

Step #13 - "converge-shared":           Error:          Received unexpected error:
Step #13 - "converge-shared":                           FatalError{Underlying: error while running command: exit status 1; 
Step #13 - "converge-shared":                           Error: Error creating AccessLevel: googleapi: Error 404: Policy not found

and

Step #12 - "create-shared": TestShared 2024-05-24T15:41:48Z command.go:100: Running command gcloud with args [access-context-manager policies list --organization REDACTED--filter parent:organizations/REDACTED--quiet --format json]
Step #12 - "create-shared": TestShared 2024-05-24T15:41:48Z command.go:185: []
Step #12 - "create-shared":     shared_test.go:45: 
Step #12 - "create-shared":             Error Trace:    /workspace/test/integration/shared/shared_test.go:45
Step #12 - "create-shared":             Error:          Should NOT be empty, but was 
Step #12 - "create-shared":             Test:           TestShared
Step #12 - "create-shared":             Messages:       Access Context Manager Policy ID must be configured in the organization for the test to proceed.
apeabody commented 4 months ago

I suspect this could result in a race condition with other PRs/Repos. But especially between terraform-example-foundation-int-trigger-HubAndSpoke terraform-example-foundation-int-trigger-default as they are trigger at the same time for a PR?

daniel-cit commented 4 months ago

I suspect this could result in a race condition with other PRs/Repos. But especially between terraform-example-foundation-int-trigger-HubAndSpoke terraform-example-foundation-int-trigger-default as they are trigger at the same time for a PR?

@apeabody

The normal configuration of the test org is to have an access context manager policy ID.

The policy ID is removed when an integration build for the repo terraform-google-vpc-service-controls runs a helper script.

an alternative would be to create the access context manager policy ID using gcloud or a helper script but not using the foundation Terraform configuration on step 1-org and this would run in only one of the two test flows.

This would fix the situation the first time the chosen test flow runs.

apeabody commented 4 months ago

I suspect this could result in a race condition with other PRs/Repos. But especially between terraform-example-foundation-int-trigger-HubAndSpoke terraform-example-foundation-int-trigger-default as they are trigger at the same time for a PR?

@apeabody

The normal configuration of the test org is to have an access context manager policy ID.

The policy ID is removed when an integration build for the repo terraform-google-vpc-service-controls runs a helper script.

an alternative would be to create the access context manager policy ID using gcloud or a helper script but not using the foundation Terraform configuration on step 1-org and this would run in only one of the two test flows.

This would fix the situation the first time the chosen test flow runs.

Yeah - A Terraform resource exists error would be unrecoverable, but a gcloud.Runf to create in org_test.gocould simply "fail" ok without needing complex pre-conditions. Anyway, just a thought to limit test failures.

daniel-cit commented 4 months ago

@apeabody Could you PTAL again