review feedback 2

Signed-off-by: Alex Pana <[email protected]>

cmd/gator/expand/expand.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"sort"
 
+	"github.com/open-policy-agent/gatekeeper/cmd/gator/util"
 	"github.com/open-policy-agent/gatekeeper/pkg/gator/expand"
 	"github.com/open-policy-agent/gatekeeper/pkg/gator/reader"
 	"github.com/spf13/cobra"
@@ -68,15 +69,15 @@ func init() {
 func run(cmd *cobra.Command, args []string) {
 	unstrucs, err := reader.ReadSources(flagFilenames, flagImages, flagTempDir)
 	if err != nil {
-		errFatalf("reading: %v\n", err)
+		util.ErrFatalf("reading: %v\n", err)
 	}
 	if len(unstrucs) == 0 {
-		errFatalf("no input data identified\n")
+		util.ErrFatalf("no input data identified\n")
 	}
 
 	resultants, err := expand.Expand(unstrucs)
 	if err != nil {
-		errFatalf("error expanding resources: %v", err)
+		util.ErrFatalf("error expanding resources: %v", err)
 	}
 	// Sort resultants for deterministic output
 	sortUnstructs(resultants)
@@ -86,7 +87,7 @@ func run(cmd *cobra.Command, args []string) {
 		fmt.Println(output)
 	} else {
 		fmt.Printf("Writing output to file: %s\n", flagOutput)
-		stringToFile(output, flagOutput)
+		util.WriteToFile(output, flagOutput)
 	}
 
 	os.Exit(0)
@@ -95,27 +96,27 @@ func run(cmd *cobra.Command, args []string) {
 func resourcetoYAMLString(resource *unstructured.Unstructured) string {
 	jsonb, err := json.Marshal(resource)
 	if err != nil {
-		errFatalf("pre-marshaling results to json: %v", err)
+		util.ErrFatalf("pre-marshaling results to json: %v", err)
 	}
 
 	unmarshalled := map[string]interface{}{}
 	err = json.Unmarshal(jsonb, &unmarshalled)
 	if err != nil {
-		errFatalf("pre-unmarshaling results from json: %v", err)
+		util.ErrFatalf("pre-unmarshaling results from json: %v", err)
 	}
 
 	var b bytes.Buffer
 	yamlEncoder := yaml.NewEncoder(&b)
 	if err := yamlEncoder.Encode(unmarshalled); err != nil {
-		errFatalf("marshaling validation yaml results: %v", err)
+		util.ErrFatalf("marshaling validation yaml results: %v", err)
 	}
 	return b.String()
 }
 
 func resourceToJSONString(resource *unstructured.Unstructured) string {
 	b, err := json.MarshalIndent(resource, "", "    ")
 	if err != nil {
-		errFatalf("marshaling validation json results: %v", err)
+		util.ErrFatalf("marshaling validation json results: %v", err)
 	}
 	return string(b)
 }
@@ -128,7 +129,7 @@ func resourcesToString(resources []*unstructured.Unstructured, format string) st
 	case stringJSON:
 		conversionFunc = resourceToJSONString
 	default:
-		errFatalf("unrecognized value for %s flag: %s", flagNameFormat, format)
+		util.ErrFatalf("unrecognized value for %s flag: %s", flagNameFormat, format)
 	}
 
 	output := ""
@@ -141,17 +142,6 @@ func resourcesToString(resources []*unstructured.Unstructured, format string) st
 	return output
 }
 
-func stringToFile(s string, path string) {
-	file, err := os.Create(path)
-	if err != nil {
-		errFatalf("error creating file at path %s: %v", path, err)
-	}
-
-	if _, err = fmt.Fprint(file, s); err != nil {
-		errFatalf("error writing to file at path %s: %s", path, err)
-	}
-}
-
 func sortUnstructs(objs []*unstructured.Unstructured) {
 	sortKey := func(o *unstructured.Unstructured) string {
 		return o.GetName() + o.GetAPIVersion() + o.GetKind()
@@ -160,8 +150,3 @@ func sortUnstructs(objs []*unstructured.Unstructured) {
 		return sortKey(objs[i]) > sortKey(objs[j])
 	})
 }
-
-func errFatalf(format string, a ...interface{}) {
-	fmt.Fprintf(os.Stderr, format+"\n", a...)
-	os.Exit(1)
-}

cmd/gator/test/test.go

@@ -8,7 +8,7 @@ import (
 	"strings"
 
 	"github.com/open-policy-agent/frameworks/constraint/pkg/instrumentation"
-	"github.com/open-policy-agent/gatekeeper/cmd/gator/commons"
+	cmdutils "github.com/open-policy-agent/gatekeeper/cmd/gator/util"
 	"github.com/open-policy-agent/gatekeeper/pkg/gator/reader"
 	"github.com/open-policy-agent/gatekeeper/pkg/gator/test"
 	"github.com/open-policy-agent/gatekeeper/pkg/util"
@@ -55,15 +55,16 @@ var (
 )
 
 const (
-	flagNameFilename        = "filename"
-	flagNameOutput          = "output"
-	flagNameImage           = "image"
-	flagNameTempDir         = "tempdir"
-	flagStatsOutputFileName = "stats-ofile"
+	flagNameFilename = "filename"
+	flagNameOutput   = "output"
+	flagNameImage    = "image"
+	flagNameTempDir  = "tempdir"
 
 	stringJSON          = "json"
 	stringYAML          = "yaml"
 	stringHumanFriendly = "default"
+
+	fourSpaceTab = "    "
 )
 
 func init() {
@@ -78,15 +79,15 @@ func init() {
 func run(cmd *cobra.Command, args []string) {
 	unstrucs, err := reader.ReadSources(flagFilenames, flagImages, flagTempDir)
 	if err != nil {
-		commons.ErrFatalf("reading: %v", err)
+		cmdutils.ErrFatalf("reading: %v", err)
 	}
 	if len(unstrucs) == 0 {
-		commons.ErrFatalf("no input data identified")
+		cmdutils.ErrFatalf("no input data identified")
 	}
 
-	responses, err := test.Test(unstrucs, test.TOpts{IncludeTrace: flagIncludeTrace, GatherStats: flagGatherStats})
+	responses, err := test.Test(unstrucs, test.Opts{IncludeTrace: flagIncludeTrace, GatherStats: flagGatherStats})
 	if err != nil {
-		commons.ErrFatalf("auditing objects: %v\n", err)
+		cmdutils.ErrFatalf("auditing objects: %v", err)
 	}
 	results := responses.Results()
 
@@ -107,15 +108,15 @@ func formatOutput(flagOutput string, results []*test.GatorResult, stats []*instr
 		var jsonB []byte
 		var err error
 		if stats != nil {
-			statsAndResuluts := map[string]interface{}{"results": results, "stats": stats}
-			jsonB, err = json.MarshalIndent(statsAndResuluts, "", "    ")
+			statsAndResults := map[string]interface{}{"results": results, "stats": stats}
+			jsonB, err = json.MarshalIndent(statsAndResults, "", fourSpaceTab)
 			if err != nil {
-				commons.ErrFatalf("marshaling validation json results and stats: %v", err)
+				cmdutils.ErrFatalf("marshaling validation json results and stats: %v", err)
 			}
 		} else {
-			jsonB, err = json.MarshalIndent(results, "", "    ")
+			jsonB, err = json.MarshalIndent(results, "", fourSpaceTab)
 			if err != nil {
-				commons.ErrFatalf("marshaling validation json results: %v", err)
+				cmdutils.ErrFatalf("marshaling validation json results: %v", err)
 			}
 		}
 
@@ -124,38 +125,38 @@ func formatOutput(flagOutput string, results []*test.GatorResult, stats []*instr
 		yamlResults := test.GetYamlFriendlyResults(results)
 		jsonb, err := json.Marshal(yamlResults)
 		if err != nil {
-			commons.ErrFatalf("pre-marshaling results to json: %v", err)
+			cmdutils.ErrFatalf("pre-marshaling results to json: %v", err)
 		}
 
 		unmarshalled := []*test.YamlGatorResult{}
 		err = json.Unmarshal(jsonb, &unmarshalled)
 		if err != nil {
-			commons.ErrFatalf("pre-unmarshaling results from json: %v", err)
+			cmdutils.ErrFatalf("pre-unmarshaling results from json: %v", err)
 		}
 
 		var yamlb []byte
 		if stats != nil {
-			statsAndResuluts := map[string]interface{}{"results": yamlResults, "stats": stats}
+			statsAndResults := map[string]interface{}{"results": yamlResults, "stats": stats}
 
 			statsJSONB, err := json.Marshal(stats)
 			if err != nil {
-				commons.ErrFatalf("pre-marshaling stats to json: %v", err)
+				cmdutils.ErrFatalf("pre-marshaling stats to json: %v", err)
 			}
 
 			unmarshalledStats := []*instrumentation.StatsEntry{}
 			err = json.Unmarshal(statsJSONB, &unmarshalledStats)
 			if err != nil {
-				commons.ErrFatalf("pre-unmarshaling stats from json: %v", err)
+				cmdutils.ErrFatalf("pre-unmarshaling stats from json: %v", err)
 			}
 
-			yamlb, err = yaml.Marshal(statsAndResuluts)
+			yamlb, err = yaml.Marshal(statsAndResults)
 			if err != nil {
-				commons.ErrFatalf("marshaling validation yaml results and stats: %v", err)
+				cmdutils.ErrFatalf("marshaling validation yaml results and stats: %v", err)
 			}
 		} else {
 			yamlb, err = yaml.Marshal(unmarshalled)
 			if err != nil {
-				commons.ErrFatalf("marshaling validation yaml results: %v", err)
+				cmdutils.ErrFatalf("marshaling validation yaml results: %v", err)
 			}
 		}
 

cmd/gator/commons/commons.go → cmd/gator/util/util.go

@@ -1,4 +1,4 @@
-package commons
+package util
 
 import (
 	"fmt"
@@ -10,7 +10,7 @@ func ErrFatalf(format string, a ...interface{}) {
 	os.Exit(1)
 }
 
-func StringToFile(s, path string) {
+func WriteToFile(s string, path string) {
 	file, err := os.Create(path)
 	if err != nil {
 		ErrFatalf("error creating file at path %s: %v", path, err)

pkg/expansion/aggregate.go

@@ -10,7 +10,7 @@ import (
 const (
 	childMsgPrefix = "[Implied by %s]"
 
-	ChildStatLabel = "EphemeralReviewFor"
+	ChildStatLabel = "Implied by"
 )
 
 // AggregateResponses aggregates all responses from children into the parent.

pkg/expansion/aggregate_test.go

@@ -146,10 +146,6 @@ func Test_AggregateStats(t *testing.T) {
 			child := tc.child
 			AggregateStats(tc.templateName, parent, child)
 
-			// if len(tc.parent.StatsEntries) != tc.expectedStats {
-			// 	t.Errorf("expected %d stats, got %d", tc.expectedStats, len(tc.parent.StatsEntries))
-			// }
-
 			require.Len(t, parent.StatsEntries, tc.expectedStats)
 
 			for _, se := range tc.parent.StatsEntries {

pkg/gator/test/test.go

@@ -28,13 +28,13 @@ func init() {
 }
 
 // options for the Test func.
-type TOpts struct {
+type Opts struct {
 	// Driver specific options
 	IncludeTrace bool
 	GatherStats  bool
 }
 
-func Test(objs []*unstructured.Unstructured, tOpts TOpts) (*GatorResponses, error) {
+func Test(objs []*unstructured.Unstructured, tOpts Opts) (*GatorResponses, error) {
 	// create the client
 	driver, err := makeRegoDriver(tOpts)
 	if err != nil {
@@ -186,7 +186,7 @@ func isConstraint(u *unstructured.Unstructured) bool {
 	return gvk.Group == "constraints.gatekeeper.sh"
 }
 
-func makeRegoDriver(tOpts TOpts) (*rego.Driver, error) {
+func makeRegoDriver(tOpts Opts) (*rego.Driver, error) {
 	var args []rego.Arg
 	if tOpts.GatherStats {
 		args = append(args, rego.GatherStats())

pkg/gator/test/test_test.go

@@ -178,7 +178,7 @@ func TestTest(t *testing.T) {
 				objs = append(objs, u)
 			}
 
-			resps, err := Test(objs, TOpts{})
+			resps, err := Test(objs, Opts{})
 			if tc.err != nil {
 				require.ErrorIs(t, err, tc.err)
 			} else if err != nil {
@@ -218,7 +218,7 @@ func Test_Test_withTrace(t *testing.T) {
 		objs = append(objs, u)
 	}
 
-	resps, err := Test(objs, TOpts{IncludeTrace: true})
+	resps, err := Test(objs, Opts{IncludeTrace: true})
 	if err != nil {
 		t.Errorf("got err '%v', want nil", err)
 	}
@@ -265,7 +265,7 @@ func Test_Test_withStats(t *testing.T) {
 		objs = append(objs, u)
 	}
 
-	resps, err := Test(objs, TOpts{GatherStats: true})
+	resps, err := Test(objs, Opts{GatherStats: true})
 	assert.NoError(t, err)
 
 	actualStats := resps.StatsEntries

pkg/logging/logging_test.go

@@ -2,6 +2,7 @@ package logging
 
 import (
 	"bytes"
+	"fmt"
 	"testing"
 
 	constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client"
@@ -45,8 +46,8 @@ func Test_LogStatsEntries(t *testing.T) {
 		"test message",
 	)
 
-	require.Contains(t, testBuf.String(), "\"test message\" someLabel=\"someLabelValue\" "+
+	expectedLogLine := fmt.Sprintf("\"test message\" someLabel=\"someLabelValue\" "+
 		"scope=\"someScope\" statsFor=\"someConstranint\" source_type=\"someType\" "+
-		"source_value=\"someValue\" name=\"someStat\" value=\"someValue\" description=\"unknown description\"",
-	)
+		"source_value=\"someValue\" name=\"someStat\" value=\"someValue\" description=\"%s\"", instrumentation.UnknownDescription)
+	require.Contains(t, testBuf.String(), expectedLogLine)
 }