Repository: GoogleCloudPlatform/terraform-validator
Pull request #324 (2021-09-30 17:53:48)

Made structured output of validate command always set resource_body to a valid type
Simplified return value for ValidateAssetsFunc to be a slice of violations instead of an AuditResponse. This makes it easier to see that the violations shouldn't be a nil slice, which would be serialized to null instead of []. (resource_body must be an object or array, not null.)

This is related to https://github.com/GoogleCloudPlatform/terraform-validator/issues/255


Patch:
diff --git a/cmd/validate.go b/cmd/validate.go
index 405b16da..56df2cff 100644
--- a/cmd/validate.go
+++ b/cmd/validate.go
@@ -20,6 +20,7 @@ import (
 	"os"
 
 	"github.com/GoogleCloudPlatform/terraform-validator/tfgcv"
+	"github.com/forseti-security/config-validator/pkg/api/validator"
 	"github.com/golang/protobuf/jsonpb"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -107,36 +108,38 @@ func (o *validateOptions) run(plan string) error {
 		return errors.Wrap(err, "converting tfplan to CAI assets")
 	}
 
-	auditResult, err := o.validateAssets(ctx, assets, o.policyPath)
+	violations, err := o.validateAssets(ctx, assets, o.policyPath)
 	if err != nil {
 		return errors.Wrap(err, "validating: FCV")
 	}
 
 	if o.rootOptions.useStructuredLogging {
 		msg := "No violations found"
-		if len(auditResult.Violations) > 0 {
+		if len(violations) > 0 {
 			msg = "Violations found"
 		}
 		o.rootOptions.outputLogger.Info(
 			msg,
-			zap.Any("resource_body", auditResult.Violations),
+			zap.Any("resource_body", violations),
 		)
-		if len(auditResult.Violations) > 0 {
+		if len(violations) > 0 {
 			return errViolations
 		}
 		return nil
 	}
 
 	// Legacy behavior
-	if len(auditResult.Violations) > 0 {
+	if len(violations) > 0 {
 		if o.outputJSON {
 			marshaller := &jsonpb.Marshaler{}
+			auditResult := &validator.AuditResponse{}
+			auditResult.Violations = violations
 			if err := marshaller.Marshal(os.Stdout, auditResult); err != nil {
 				return errors.Wrap(err, "marshalling violations to json")
 			}
 		} else {
 			fmt.Print("Found Violations:\n\n")
-			for _, v := range auditResult.Violations {
+			for _, v := range violations {
 				fmt.Printf("Constraint %v on resource %v: %v\n\n",
 					v.Constraint,
 					v.Resource,
diff --git a/go.mod b/go.mod
index 3052695b..036a72cf 100644
--- a/go.mod
+++ b/go.mod
@@ -15,6 +15,7 @@ require (
 	go.uber.org/zap v1.19.1
 	google.golang.org/api v0.56.0
 	google.golang.org/genproto v0.0.0-20210828152312-66f60bf46e71
+	google.golang.org/protobuf v1.27.1 // indirect
 )
 
 go 1.14
diff --git a/tfgcv/validate_assets.go b/tfgcv/validate_assets.go
index 3be8d8d5..8798c69c 100644
--- a/tfgcv/validate_assets.go
+++ b/tfgcv/validate_assets.go
@@ -32,18 +32,18 @@ func BuildVersion() string {
 	return buildVersion
 }
 
-type ValidateAssetsFunc func(ctx context.Context, assets []google.Asset, policyRootPath string) (*validator.AuditResponse, error)
+type ValidateAssetsFunc func(ctx context.Context, assets []google.Asset, policyRootPath string) ([]*validator.Violation, error)
 
 // ValidateAssets instantiates GCV and audits CAI assets using "policies"
 // and "lib" folder under policyRootPath.
-func ValidateAssets(ctx context.Context, assets []google.Asset, policyRootPath string) (*validator.AuditResponse, error) {
+func ValidateAssets(ctx context.Context, assets []google.Asset, policyRootPath string) ([]*validator.Violation, error) {
 	return ValidateAssetsWithLibrary(ctx, assets,
 		[]string{filepath.Join(policyRootPath, "policies")},
 		filepath.Join(policyRootPath, "lib"))
 }
 
 // ValidateAssetsWithLibrary instantiates GCV and audits CAI assets.
-func ValidateAssetsWithLibrary(ctx context.Context, assets []google.Asset, policyPaths []string, policyLibraryDir string) (*validator.AuditResponse, error) {
+func ValidateAssetsWithLibrary(ctx context.Context, assets []google.Asset, policyPaths []string, policyLibraryDir string) ([]*validator.Violation, error) {
 	valid, err := gcv.NewValidator(policyPaths, policyLibraryDir)
 	if err != nil {
 		return nil, errors.Wrap(err, "initializing gcv validator")
@@ -59,17 +59,19 @@ func ValidateAssetsWithLibrary(ctx context.Context, assets []google.Asset, polic
 
 	pbSplitAssets := splitAssets(pbAssets)
 
-	auditResult := &validator.AuditResponse{}
+	// Make an empty slice, not a nil slice, so that this
+	// can be properly serialized to JSON.
+	violations := []*validator.Violation{}
 	for _, asset := range pbSplitAssets {
-		violations, err := valid.ReviewAsset(context.Background(), asset)
+		newViolations, err := valid.ReviewAsset(context.Background(), asset)
 
 		if err != nil {
 			return nil, errors.Wrapf(err, "reviewing asset %s", asset)
 		}
-		auditResult.Violations = append(auditResult.Violations, violations...)
+		violations = append(violations, newViolations...)
 	}
 
-	return auditResult, nil
+	return violations, nil
 }
 
 // splitAssets split assets because for the GCP target Constraint


Test patch:
diff --git a/cmd/validate_test.go b/cmd/validate_test.go
index bce72700..97d44da0 100644
--- a/cmd/validate_test.go
+++ b/cmd/validate_test.go
@@ -10,31 +10,27 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func testAuditResponseNoViolations() *validator.AuditResponse {
-	return &validator.AuditResponse{
-		Violations: []*validator.Violation{},
-	}
+func testNoViolations() []*validator.Violation {
+	return []*validator.Violation{}
 }
 
-func MockValidateAssetsNoViolations(ctx context.Context, assets []google.Asset, policyRootPath string) (*validator.AuditResponse, error) {
-	return testAuditResponseNoViolations(), nil
+func MockValidateAssetsNoViolations(ctx context.Context, assets []google.Asset, policyRootPath string) ([]*validator.Violation, error) {
+	return testNoViolations(), nil
 }
 
-func testAuditResponseWithViolations() *validator.AuditResponse {
-	return &validator.AuditResponse{
-		Violations: []*validator.Violation{
-			&validator.Violation{
-				Constraint: "GCPAlwaysViolatesConstraintV1.always_violates_project_match_target",
-				Resource:   "//bigtable.googleapis.com/projects/my-project/instances/tf-instance",
-				Message:    "Constraint GCPAlwaysViolatesConstraintV1.always_violates_project_match_target on resource //bigtable.googleapis.com/projects/my-project/instances/tf-instance",
-				Severity:   "high",
-			},
+func testWithViolations() []*validator.Violation {
+	return []*validator.Violation{
+		&validator.Violation{
+			Constraint: "GCPAlwaysViolatesConstraintV1.always_violates_project_match_target",
+			Resource:   "//bigtable.googleapis.com/projects/my-project/instances/tf-instance",
+			Message:    "Constraint GCPAlwaysViolatesConstraintV1.always_violates_project_match_target on resource //bigtable.googleapis.com/projects/my-project/instances/tf-instance",
+			Severity:   "high",
 		},
 	}
 }
 
-func MockValidateAssetsWithViolations(ctx context.Context, assets []google.Asset, policyRootPath string) (*validator.AuditResponse, error) {
-	return testAuditResponseWithViolations(), nil
+func MockValidateAssetsWithViolations(ctx context.Context, assets []google.Asset, policyRootPath string) ([]*validator.Violation, error) {
+	return testWithViolations(), nil
 }
 
 func TestValidateRun(t *testing.T) {
@@ -76,10 +72,10 @@ func TestValidateRun(t *testing.T) {
 	a.Contains(output, "resource_body")
 	a.Len(output["resource_body"], 1)
 
-	var expectedAuditResponse map[string]interface{}
-	expectedAuditResponseJSON, _ := json.Marshal(testAuditResponseWithViolations())
-	json.Unmarshal(expectedAuditResponseJSON, &expectedAuditResponse)
-	a.Equal(expectedAuditResponse["violations"], output["resource_body"])
+	var expectedViolations []interface{}
+	expectedViolationsJSON, _ := json.Marshal(testWithViolations())
+	json.Unmarshal(expectedViolationsJSON, &expectedViolations)
+	a.Equal(expectedViolations, output["resource_body"])
 }
 
 func TestValidateRunNoViolations(t *testing.T) {
diff --git a/tfgcv/proto_test.go b/tfgcv/proto_test.go
index 8d42b92d..40c44d4a 100644
--- a/tfgcv/proto_test.go
+++ b/tfgcv/proto_test.go
@@ -18,8 +18,8 @@ import (
 	"testing"
 
 	"github.com/forseti-security/config-validator/pkg/api/validator"
-	structpb "github.com/golang/protobuf/ptypes/struct"
 	"github.com/golang/protobuf/proto"
+	structpb "github.com/golang/protobuf/ptypes/struct"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/genproto/googleapis/cloud/asset/v1"
 )