Made structured output of validate command always set resource_body t…

…o a valid type (#324)

Simplified return value for ValidateAssetsFunc to be a slice of violations instead of an AuditResponse. This guarantees that the violations won't end up being a nil slice, which would be serialized to null instead of []

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,

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) {

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

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"
 )

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