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

fix: grant membership to bootstrap terraform SA to access required groups #1301

Open lpezet opened 2 months ago

daniel-cit commented 2 months ago

/gcbrun

eeaton commented 2 months ago

@daniel-cit looks like all the CI tests are failing again, due to a breaking issue in the upstream provider.

I found another repo that identified the perma-diff introduced by Cloud Functions API changing it's behavior to add LOG_EXECUTION_ID to the env variables if not explicitly specified. I expect the same fix should work for us.

Incidentally, this issue might be avoided by the PR #1304 that is hiding this module behind an enablement flag, so will avoid deploying the Function, but we should still find a way to address it for those who explicitly enable the module. I'll do a quick PR to add the explicit LOG_EXECUTION_ID and see if that helps.

daniel-cit commented 2 months ago

@daniel-cit looks like all the CI tests are failing again, due to a breaking issue in the upstream provider.

I found another repo that identified the perma-diff introduced by Cloud Functions API changing it's behavior to add LOG_EXECUTION_ID to the env variables if not explicitly specified. I expect the same fix should work for us.

Incidentally, this issue might be avoided by the PR #1304 that is hiding this module behind an enablement flag, so will avoid deploying the Function, but we should still find a way to address it for those who explicitly enable the module. I'll do a quick PR to add the explicit LOG_EXECUTION_ID and see if that helps.

I think that the flag option is a good one as a temporary fix.

there are two sets of environment variables in the cloud function, build time and run time.

For the run time we already have code that sets the LOG_EXECUTION_ID here. The fabric example you linked is adding the LOG_EXECUTION_ID to both the runtime and build env vars.

the strange thing is that the error is on the runtime part that has the configuration just presented, like if something was messing up with the env vars:

When expanding the plan for module.cai_monitoring.module.cloud_function.google_cloudfunctions2_function.function to include new values learned so far during apply, 
provider "registry.terraform.io/hashicorp/google" produced an invalid new value for .service_config[0].environment_variables:

was null, 

but now

cty.MapVal(map[string]cty.Value{
  "LOG_EXECUTION_ID":cty.StringVal("true"),
"ROLES":cty.StringVal("roles/owner,roles/editor,roles/resourcemanager.organizationAdmin,roles/compute.networkAdmin,roles/compute.orgFirewallPolicyAdmin"),
  "SOURCE_ID":cty.StringVal("organizations/REDACTED/sources/17068193223143848275")
}).
eeaton commented 2 months ago

The unrelated issues from Functions provider in the last few comments have been addressed by #1311 , I'll resume running tests on this PR. /gcbrun

eeaton commented 2 months ago

CI tests are green, but I don't think our tests were accurately checking for the scenario of creating groups through terraform config because the automation never encountered the reported issues. I'll try to reproduce locally to verify the changes.

lpezet commented 1 month ago

Getting that error when running make docker_test_lint:

...
44      :       1723318373.673       3.278      0       55      0       0       terraform_validate ./4-projects/modules/infra_pipelines
45      :       1723318373.743       3.404      0       55      0       0       terraform_validate ./4-projects/modules/single_project
46      :       1723318374.022       3.067      0       55      0       0       terraform_validate ./5-app-infra/business_unit_1/development
47      :       1723318374.281       3.052      0       55      0       0       terraform_validate ./5-app-infra/business_unit_1/nonproduction
48      :       1723318374.422       3.049      0       55      0       0       terraform_validate ./5-app-infra/business_unit_1/production
49      :       1723318374.887       2.751      0       55      0       0       terraform_validate ./5-app-infra/modules/env_base
50      :       1723318375.013       2.905      0       55      0       0       terraform_validate ./test/setup
ENABLE_BPMETADATA not set to 1. Skipping metadata validation.
Error: The following tests have failed: check_headers
make: *** [Makefile:28: docker_test_lint] Error 1

Where can I find this check_headers test?