forseti-security / terraform-google-forseti

A Terraform module for installing Forseti on GCP
Apache License 2.0
132 stars 126 forks source link

Enable the ability to turn on firewall logs and add labels to resources #587

Closed upodroid closed 4 years ago

upodroid commented 4 years ago

Hi

I have a requirement to enable firewall logging on rules that is created in our network. Please accept this change and release a new patch version once it has been merged.

Thank you

upodroid commented 4 years ago

I will also need to set labels on all created resources. I'll push a commit that can do that later today.

gkowalski-google commented 4 years ago

Hi @upodroid, your changes look good to me. Let me fix the build triggers so the tests run and I can approve afterwards.

upodroid commented 4 years ago

I'm ready to merge this now, I have:

upodroid commented 4 years ago

Another thing i just noticed, forseti service account is missing metric writer in the project. I'm thinking the server should push metrics to Stackdriver.

image

upodroid commented 4 years ago

image

Much better now :)

gkowalski-google commented 4 years ago

@upodroid Here's the error from the tests. It doesn't seem to be related to your changes, perhaps it is a race condition and we just need to update the forseti-private-zone to depend on the google_project_service resource?

Error: Error creating ManagedZone: googleapi: Error 403: Cloud DNS API has not been used in project {{ PROJECT_NUMBER }} before or it is disabled. Enable it by visiting https://console.developers.google.com/apis/api/dns.googleapis.com/overview?project={{ PROJECT_NUMBER }} then retry. If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry., accessNotConfigured

         on ../../../examples/private_connection/main.tf line 83, in resource "google_dns_managed_zone" "forseti-private-zone":
         83: resource "google_dns_managed_zone" "forseti-private-zone" {
upodroid commented 4 years ago

Looks like it is ready to be merged now.