Repository: datreeio/datree
Pull request #693 (2022-06-23 16:47:00)

refactor: unify duplicate logic between `test` & `kustomize`
From now on the code inside the cobra `Run` should be as lean as possible, and shared logic should be added to `TestWrapper` (or deeper).
This also fixes a bug, where `datree kustomize test` did not save the defaultRules.yaml file
If you have a better name, other then `TestWrapper` - please let me know 😊 

myishay: Approved. Can you please make sure the kustomize flow didn't break?

Patch:
diff --git a/cmd/kustomize/main.go b/cmd/kustomize/main.go
index 7e7bdc8..5405ae8 100644
--- a/cmd/kustomize/main.go
+++ b/cmd/kustomize/main.go
@@ -4,7 +4,6 @@ import (
 	"os"
 	"strings"
 
-	"github.com/datreeio/datree/pkg/ciContext"
 	"github.com/datreeio/datree/pkg/cliClient"
 
 	"github.com/datreeio/datree/cmd/test"
@@ -45,7 +44,11 @@ func New(testCtx *test.TestCommandContext, kustomizeCtx *KustomizeContext) *cobr
 		datree kustomize test https://github.com/kubernetes-sigs/kustomize.git/examples/helloWorld?ref=v1.0.6
 		`),
 		Args: func(cmd *cobra.Command, args []string) error {
-			return utils.ValidateStdinPathArgument(args)
+			err := utils.ValidateStdinPathArgument(args)
+			if err != nil {
+				return err
+			}
+			return testCommandFlags.Validate()
 		},
 		PreRunE: func(cmd *cobra.Command, args []string) error {
 			return test.LoadVersionMessages(testCtx, args, cmd)
@@ -59,28 +62,6 @@ func New(testCtx *test.TestCommandContext, kustomizeCtx *KustomizeContext) *cobr
 				}
 			}()
 
-			err = testCommandFlags.Validate()
-			if err != nil {
-				return err
-			}
-
-			localConfigContent, err := testCtx.LocalConfig.GetLocalConfiguration()
-			if err != nil {
-				return err
-			}
-
-			ciContext := ciContext.Extract()
-			testCtx.CliClient.AddFlags(testCommandFlags.ToMapping())
-			evaluationPrerunData, err := testCtx.CliClient.RequestEvaluationPrerunData(localConfigContent.Token, ciContext.IsCI)
-			if err != nil {
-				return err
-			}
-
-			testCommandOptions, err := test.GenerateTestCommandData(testCommandFlags, localConfigContent, evaluationPrerunData)
-			if err != nil {
-				return err
-			}
-
 			out, err := kustomizeCtx.CommandRunner.ExecuteKustomizeBin(args)
 			if err != nil {
 				return err
@@ -92,11 +73,7 @@ func New(testCtx *test.TestCommandContext, kustomizeCtx *KustomizeContext) *cobr
 			}
 			defer os.Remove(tempFilename)
 
-			err = test.Test(testCtx, []string{tempFilename}, testCommandOptions)
-			if err != nil {
-				return err
-			}
-			return nil
+			return test.TestWrapper(testCtx, []string{tempFilename}, testCommandFlags)
 		},
 	}
 	testCommandFlags.AddFlags(kustomizeTestCommand)


Test patch:
diff --git a/cmd/test/main.go b/cmd/test/main.go
index 9b3a9eb..194290d 100644
--- a/cmd/test/main.go
+++ b/cmd/test/main.go
@@ -195,29 +195,7 @@ func New(ctx *TestCommandContext) *cobra.Command {
 				}
 			}()
 
-			localConfigContent, err := ctx.LocalConfig.GetLocalConfiguration()
-			if err != nil {
-				return err
-			}
-
-			ctx.CliClient.AddFlags(testCommandFlags.ToMapping())
-			evaluationPrerunData, err := ctx.CliClient.RequestEvaluationPrerunData(localConfigContent.Token, ctx.CiContext.IsCI)
-			saveDefaultRulesAsFile(ctx, evaluationPrerunData.DefaultRulesYaml)
-
-			if err != nil {
-				return err
-			}
-
-			testCommandOptions, err := GenerateTestCommandData(testCommandFlags, localConfigContent, evaluationPrerunData)
-			if err != nil {
-				return err
-			}
-
-			err = Test(ctx, args, testCommandOptions)
-			if err != nil {
-				return err
-			}
-			return nil
+			return TestWrapper(ctx, args, testCommandFlags)
 		},
 	}
 	testCommandFlags.AddFlags(testCommand)
@@ -324,7 +302,27 @@ func validateK8sVersionFormatIfProvided(k8sVersion string) error {
 	}
 }
 
-func Test(ctx *TestCommandContext, paths []string, prerunData *TestCommandData) error {
+func TestWrapper(ctx *TestCommandContext, args []string, testCommandFlags *TestCommandFlags) error {
+	localConfigContent, err := ctx.LocalConfig.GetLocalConfiguration()
+	if err != nil {
+		return err
+	}
+
+	ctx.CliClient.AddFlags(testCommandFlags.ToMapping())
+	evaluationPrerunData, err := ctx.CliClient.RequestEvaluationPrerunData(localConfigContent.Token, ctx.CiContext.IsCI)
+	if err != nil {
+		return err
+	}
+
+	saveDefaultRulesAsFile(ctx, evaluationPrerunData.DefaultRulesYaml)
+	testCommandOptions, err := GenerateTestCommandData(testCommandFlags, localConfigContent, evaluationPrerunData)
+	if err != nil {
+		return err
+	}
+	return test(ctx, args, testCommandOptions)
+}
+
+func test(ctx *TestCommandContext, paths []string, prerunData *TestCommandData) error {
 	if paths[0] == "-" {
 		tempFile, err := os.CreateTemp("", "datree_temp_*.yaml")
 		if err != nil {
diff --git a/cmd/test/main_test.go b/cmd/test/main_test.go
index 46afb16..0389c02 100644
--- a/cmd/test/main_test.go
+++ b/cmd/test/main_test.go
@@ -266,7 +266,7 @@ func TestTestFlow(t *testing.T) {
 				CiContext:      ciContext,
 			}
 
-			err := Test(ctx, tt.args.path, &TestCommandData{K8sVersion: "1.18.0", Output: "", Policy: tt.expected.Evaluate.evaluationData.Policy, Token: "134kh"})
+			err := test(ctx, tt.args.path, &TestCommandData{K8sVersion: "1.18.0", Output: "", Policy: tt.expected.Evaluate.evaluationData.Policy, Token: "134kh"})
 			if tt.expected.err != nil {
 				assert.EqualError(t, err, tt.expected.err.Error())
 			} else {
@@ -731,13 +731,13 @@ func TestTestCommandEmptyDir(t *testing.T) {
 	emptyDirPaths := filepath.Join(emptyDir, "*.yaml")
 
 	readerMock.On("FilterFiles", []string{emptyDirPaths}).Return([]string{}, nil)
-	err := Test(ctx, []string{emptyDirPaths}, &TestCommandData{K8sVersion: "1.18.0", Output: "", Policy: testingPolicy, Token: "134kh"})
+	err := test(ctx, []string{emptyDirPaths}, &TestCommandData{K8sVersion: "1.18.0", Output: "", Policy: testingPolicy, Token: "134kh"})
 
 	assert.EqualError(t, err, "no files detected")
 }
 func TestTestCommandNoFlags(t *testing.T) {
 	setup()
-	_ = Test(ctx, []string{"8/*"}, &TestCommandData{K8sVersion: "1.18.0", Output: "", Policy: testingPolicy, Token: "134kh"})
+	_ = test(ctx, []string{"8/*"}, &TestCommandData{K8sVersion: "1.18.0", Output: "", Policy: testingPolicy, Token: "134kh"})
 
 	policyCheckData := evaluation.PolicyCheckData{
 		FilesConfigurations: filesConfigurations,
@@ -752,7 +752,7 @@ func TestTestCommandNoFlags(t *testing.T) {
 
 func TestTestCommandJsonOutput(t *testing.T) {
 	setup()
-	_ = Test(ctx, []string{"valid/path"}, &TestCommandData{Output: "json", Policy: testingPolicy})
+	_ = test(ctx, []string{"valid/path"}, &TestCommandData{Output: "json", Policy: testingPolicy})
 
 	policyCheckData := evaluation.PolicyCheckData{
 		FilesConfigurations: filesConfigurations,
@@ -767,7 +767,7 @@ func TestTestCommandJsonOutput(t *testing.T) {
 
 func TestTestCommandYamlOutput(t *testing.T) {
 	setup()
-	_ = Test(ctx, []string{"8/*"}, &TestCommandData{Output: "yaml", Policy: testingPolicy})
+	_ = test(ctx, []string{"8/*"}, &TestCommandData{Output: "yaml", Policy: testingPolicy})
 
 	policyCheckData := evaluation.PolicyCheckData{
 		FilesConfigurations: filesConfigurations,
@@ -782,7 +782,7 @@ func TestTestCommandYamlOutput(t *testing.T) {
 
 func TestTestCommandXmlOutput(t *testing.T) {
 	setup()
-	_ = Test(ctx, []string{"valid/path"}, &TestCommandData{Output: "xml", Policy: testingPolicy})
+	_ = test(ctx, []string{"valid/path"}, &TestCommandData{Output: "xml", Policy: testingPolicy})
 
 	policyCheckData := evaluation.PolicyCheckData{
 		FilesConfigurations: filesConfigurations,
@@ -797,7 +797,7 @@ func TestTestCommandXmlOutput(t *testing.T) {
 
 func TestTestCommandOnlyK8sFiles(t *testing.T) {
 	setup()
-	_ = Test(ctx, []string{"8/*"}, &TestCommandData{OnlyK8sFiles: true})
+	_ = test(ctx, []string{"8/*"}, &TestCommandData{OnlyK8sFiles: true})
 
 	k8sValidatorMock.AssertCalled(t, "ValidateResources", mock.Anything, 100)
 	k8sValidatorMock.AssertCalled(t, "GetK8sFiles", mock.Anything, 100)
@@ -805,7 +805,7 @@ func TestTestCommandOnlyK8sFiles(t *testing.T) {
 
 func TestTestCommandNoInternetConnection(t *testing.T) {
 	setup()
-	_ = Test(ctx, []string{"valid/path"}, &TestCommandData{Policy: testingPolicy})
+	_ = test(ctx, []string{"valid/path"}, &TestCommandData{Policy: testingPolicy})
 
 	policyCheckData := evaluation.PolicyCheckData{
 		FilesConfigurations: filesConfigurations,