gcp-iam-audit-log-v1 Policy Is Not Working

[blueandgold]

Opening this in Forseti repo, since this affects Forseti and not other Config Validator clients.

The problem is that gcp-iam-audit-log-v1 policy does not work in forseti because the audit_config field gets ignored when converting from CAI json to proto.

Edit: Moving this to the policy-library since this is affecting not just Forseti.

[blueandgold]

@joecheuk is there a reason why we do the ignoring of fields here, while the other CV clients does not?

@katze120 for FYI.

[katze120]

In cft scorecard, the conversion leverages terraform validator converter which only takes bindings field from iam_policy and implicitly dropping audit_configs field.
And similarly in terraform validator itself.
FYI @morgante

Is there any CV client that works fine with audit_configs field?

[morgante]

@katze120 Are you sure scorecard doesn't provide it? Scorecard uses the type from tfvalidator but nothing else.

[katze120]

yes, from actual test results and also the Asset struct definition in addition to links provided above

When I bypass tfvalidator and directly try to convert interface{} variable to proto, it gives me Error: validating: FCV: converting asset to proto: unmarshaling to proto: unknown field "audit_configs" in iam.Policy

[morgante]

Got it, so we're likely encountering the same issue as Forseti (the protodef for IAM policies doesn't include audit_configs).

[xingao267]

Is this due to the same issue as #367?

[xingao267]

From my understanding on the discussion, to fix this issue, we need

1. Add audit_configs in the public IAM proto https://github.com/googleapis/googleapis/blob/master/google/iam/v1/policy.proto. I've contacted the IAM team for that. This should fix CV, CFT Scorecard and Forseti (when using the version of CV which has this fix).

2. Add support for audit_configs in https://github.com/GoogleCloudPlatform/cloud-foundation-toolkit/blob/master/cli/vendor/github.com/GoogleCloudPlatform/terraform-validator/converters/google/convert.go#L226-L235, which should then let Terraform Validator also see audit_configs field.

Is that correct?

[morgante]

@xingao267 Your analysis is correct. I'd be happy to look at a PR if you have a chance to fix this.