Update config validator dependency and fix incompatible changes.

[briantkennedy]

This commit is only intended to only roll forward the config validator
version and preserve status quo behavior. This commit is not intended
to improve performance, refactor code, or achieve any other goals and
those will be considered out of scope.

[morgante]

Thanks! Do you feel like FCV has stabilized to the point that we're ready to upgrade/roll out the new version?

Also I assume this would eliminate the need for #575, fyi @katze120.

[briantkennedy]

In terms of the high level user interfaces (Constraint Templates, Constraints, CAI input, Violation output) it's stable and can be rolled out.  Some of the programmatic interfaces may change.

[katze120]

Also I assume this would eliminate the need for #575, fyi @katze120.

Thanks!
#575 is still needed, but with this, we can call SanitizeAncestryPath function within cft scorecard instead of creating a duplicated function to build the ancestry path by joining the reversed "ancestors" array

[morgante]

Thanks, just one naming suggestion.

[morgante]

Since we're changing these functions to get and return the data instead of adding it directly, could we change the names?

Suggested change

	func addDataFromReader(config *ScoringConfig, reader io.Reader) ([]*validator.Asset, error) {
	func getDataFromReader(config *ScoringConfig, reader io.Reader) ([]*validator.Asset, error) {

[morgante]

To be clear, please rename all the updated methods to follow this.

[briantkennedy]

Oh, I thought that the suggestion was going to update all callsites. will fix.

[katze120]

With this PR, the original violation metadata field becomes metadata.details, and impacts the way scorecard outputs metadata.
This behaviour can be confirmed by running cft scorecard with --verbose,
Before:

DBUG[02-20|21:37:00] Violation metadata                       metadata="struct_value:<fields:<key:\"...

After:

DBUG[02-20|21:14:19] Violation metadata                       metadata="struct_value:<fields:<key:\"details\ .. (.. followed by the output before...)

I have checked recent changes in config-validator repo but was not able to identify yet from the code what caused this change. Would it be due to the change in opa version?

@briantkennedy any idea? I'd like to confirm whether this change is intentional and permanent.

And if so, I'll probably need do something like this in scorecard output, to obtain value of certain metadata fields:
From

v.Metadata.GetStructValue().Fields[field]

to

v.Metadata.GetStructValue().Fields["details"].GetStructValue().Fields[field]

[katze120]

Is there a list of all behaviour changes in the new config-validator?

Here are some that I've observed so far.

• all underscore in constraint names are replaced by dash.
  This has impact on downstream integration that relies on constraint names, while I understood why this was done..
• in previous version, only unique violations are reported. In new version there are duplicated violations due to a few reasons:
  • rego code that reports the same message & metadata for different underlying resources. This used to result in 1 violation, now many
  • duplicate violation from iam constraints due to the same identity assigned the same iam role for the same resource twice. Not sure how that can happen, but this is confirmed from raw CAI data.
  • duplicate violation from blacklist-public-users constraint. Nothing unusual in raw CAI data. The same violation is reported 4 times. (There are 4 IAM role - member mappings on the bucket but only one is public )
• data in Violation.ConstraintConfig is missing so we lost info such as constraint annotations including category. scorecard GetName function returns empty string now.

@morgante
Can we hold the release of new executable to https://storage.googleapis.com/cft-cli until the impact is clearer?
And make sure backup is made for the old executables?
I may need to update some internal documentation to use the specific version https://storage.googleapis.com/cft-cli/v0.3.0/ until downstream integrations are updated to be compatible with the new behaviours

[morgante]

@katze120 Sure, I can hold off on releasing now.

duplicate violation from iam constraints due to the same identity assigned the same iam role for the same resource twice. Not sure how that can happen, but this is confirmed from raw CAI data.
duplicate violation from iam constraints due to the same identity assigned the same iam role for the same resource twice. Not sure how that can happen, but this is confirmed from raw CAI data.

These definitely sound like bugs.

[katze120]

Opened #584 to keep working notes reg. scorecard behaviour changes