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: disabling SCC notifications for now #1304

Closed lpezet closed 2 months ago

eeaton commented 2 months ago

@lpezet the results of the Lint test show specific errors in your code changes: https://github.com/terraform-google-modules/terraform-example-foundation/actions/runs/10129013421/job/28008555085?pr=1304

│ Error: Unsupported attribute
│ 
│   on iam.tf line 173, in resource "google_project_iam_member" "project_scc_admin":
│  173:   project = module.scc_notifications.project_id
│     ├────────────────
│     │ module.scc_notifications is a list of object
│ 
│ Can't access attributes on a list of objects. Did you mean to access
│ attribute "project_id" for a specific element of the list, or across all
│ elements of the list?
╵
╷
│ Error: Unsupported attribute
│ 
│   on sa.tf line 18, in resource "google_service_account" "cai_monitoring_builder":
│   18:   project                      = module.scc_notifications.project_id
│     ├────────────────
│     │ module.scc_notifications is a list of object
│ 
│ Can't access attributes on a list of objects. Did you mean to access
│ attribute "project_id" for a specific element of the list, or across all
│ elements of the list?

Once you get the Lint tests passing I'll review the specific changes

eeaton commented 2 months ago

/gcbrun

lpezet commented 2 months ago

@eeaton I believe I addressed everything you reported. On my own setup, I'm still getting that converting TF resource to CAI/exit code 33 error. Maybe I need to teardown 1-org and re-apply everything.

eeaton commented 2 months ago

/gcbrun

daniel-cit commented 2 months ago

/gcbrun

eeaton commented 2 months ago

Code changes look good, but the unit tests in CI are failing with the error below.

TestOrg 2024-07-31T15:03:30Z command.go:185: ERROR: (gcloud.pubsub.topics.describe) NOT_FOUND: Resource not found (resource=top-scc-notification). This command is authenticated as ci-account@ci-foundation-0snp73-kolx.iam.gserviceaccount.com using the credentials in /tmp/3595579577, specified by the [auth/credential_file_override] property.
Step #8 - "verify-org":     gcloud.go:84: error while running command: exit status 1; ERROR: (gcloud.pubsub.topics.describe) NOT_FOUND: Resource not found (resource=top-scc-notification).

We have an internal environment to automatically build and test all the resources before. The logs and errors from this are internal-only as a control against leaked secrets, so unfortunately there's a bit of back-and-forth required to convey the error and fix it.

To recreate these tests in your local environment, see contributing. The relevant test for the scope of changes in this PR is just test/integration/org/org_test.go.

In this case, I think the simplest fix is to comment out the lines that assert the resources related to SCC and CAI-monitoring have been created. Since we're now treating them as optional, it makes sense to disable the tests that require these resources. Add a commit that comments out these lines from org_test.go:

I haven't yet run this myself, I'm just comparing the test file against the resources I know we've changed, give that a try. If you're having a hard time with the tests locally I'll try to recreate it in my environment to confirm what's missing in the tests.

lpezet commented 2 months ago

@eeaton I'm trying to go through the tests myself, but the make docker_test_prepare fails at some point with:

│ Error: Request `Enable Project Service "sourcerepo.googleapis.com" for project "ci-foundation-77uj5a-q58j"` returned error: Batch request and retried single request "Enable Project Service \"sourcerepo.googleapis.com\" for project \"ci-foundation-77uj5a-q58j\"" both failed. Final error: failed to enable services: failed on request preconditions: googleapi: Error 403: Permission denied to enable service [sourcerepo.googleapis.com]
│ Help Token: ARZIt87RX7leTepHHK1jRqfguQNIdBkkfU0LPxizvbwU-FQaOJgrL4izifLpQnwnjrHVPMIIMFyp9P7haV0pXuO1pxhXNFbfJfwpvQAkHM8jW2KB
│ Details:
│ [
│   {
│     "@type": "type.googleapis.com/google.rpc.PreconditionFailure",
│     "violations": [
│       {
│         "subject": "?error_code=110002\u0026service=servicemanagement.googleapis.com\u0026permission=servicemanagement.services.bind\u0026resource=ci-foundation-77uj5a-q58j",
│         "type": "googleapis.com"
│       }
│     ]
│   },
│   {
│     "@type": "type.googleapis.com/google.rpc.ErrorInfo",
│     "domain": "serviceusage.googleapis.com",
│     "metadata": {
│       "permission": "servicemanagement.services.bind",
│       "resource": "ci-foundation-77uj5a-q58j",
│       "service": "servicemanagement.googleapis.com"
│     },
│     "reason": "AUTH_PERMISSION_DENIED"
│   }
│ ]
│ , forbidden
│ 
│   with module.project.module.project-factory.module.project_services.google_project_service.project_services["sourcerepo.googleapis.com"],
│   on .terraform/modules/project/modules/project_services/main.tf line 31, in resource "google_project_service" "project_services":
│   31: resource "google_project_service" "project_services" {
│ 
╵
make: *** [Makefile:66: docker_test_prepare] Error 1

My SA has the roles mentioned in the CONTRIBUTING file. I tried adding more roles (Service Usage Admin and Service Management Admin) but no luck. I'll keep trying but what am I missing?

eeaton commented 2 months ago

The issue you're encountering is about the CSR deprecation. It looks like the docker tests also have a dependency on CSR that needs to be unpicked. Docker tests don't meaningfully use CSR, but there are places where the CSR API is enabled, which will now fail.

CSR is still available to orgs that used it prior to deprecation, but not to new orgs who hadn't used it. So unfortunately that means your environment and mine have different behavior so it hasn't been caught by our automated tests. I expect you can get around this by removing / commenting out any references to "sourcerepo.googleapis.com". The following files are likely culprits (there are a few more in later stages, but those aren't blockers for tests in the Bootstrap and Org stages.

eeaton commented 2 months ago

/gcbrun