Check if required setters are set

commands/applycmd.go

@@ -6,6 +6,7 @@ package commands
 import (
 	"os"
 
+	"github.com/GoogleContainerTools/kpt/internal/util/setters"
 	"github.com/GoogleContainerTools/kpt/pkg/live"
 	"github.com/spf13/cobra"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -47,6 +48,11 @@ func (w *ApplyRunnerWrapper) Command() *cobra.Command {
 // invoked. Returns an error if one happened. Swallows the
 // "AlreadyExists" error for CRD installation.
 func (w *ApplyRunnerWrapper) RunE(cmd *cobra.Command, args []string) error {
+	if len(args) > 0 {
+		if err := setters.CheckForRequiredSetters(args[0]); err != nil {
+			return err
+		}
+	}
 	if _, exists := os.LookupEnv(resourceGroupEnv); exists {
 		klog.V(4).Infoln("wrapper applyRunner detected environment variable")
 		err := live.ApplyResourceGroupCRD(w.factory)

commands/livecmd.go

@@ -27,7 +27,6 @@ import (
 	"sigs.k8s.io/cli-utils/cmd/destroy"
 	"sigs.k8s.io/cli-utils/cmd/diff"
 	"sigs.k8s.io/cli-utils/cmd/initcmd"
-	"sigs.k8s.io/cli-utils/cmd/preview"
 	"sigs.k8s.io/cli-utils/cmd/status"
 	"sigs.k8s.io/cli-utils/pkg/manifestreader"
 	"sigs.k8s.io/cli-utils/pkg/provider"
@@ -90,7 +89,7 @@ func GetLiveCommand(name string, f util.Factory) *cobra.Command {
 	applyCmd.Long = livedocs.ApplyShort + "\n" + livedocs.ApplyLong
 	applyCmd.Example = livedocs.ApplyExamples
 
-	previewCmd := preview.GetPreviewRunner(p, l, ioStreams).Command
+	previewCmd := GetPreviewRunner(p, l, ioStreams).Command()
 	previewCmd.Short = livedocs.PreviewShort
 	previewCmd.Long = livedocs.PreviewShort + "\n" + livedocs.PreviewLong
 	previewCmd.Example = livedocs.PreviewExamples

commands/previewcmd.go

@@ -0,0 +1,48 @@
+package commands
+
+import (
+	"github.com/GoogleContainerTools/kpt/internal/util/setters"
+	"github.com/spf13/cobra"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	cmdutil "k8s.io/kubectl/pkg/cmd/util"
+	"sigs.k8s.io/cli-utils/cmd/preview"
+	"sigs.k8s.io/cli-utils/pkg/manifestreader"
+	"sigs.k8s.io/cli-utils/pkg/provider"
+)
+
+// Get PreviewRunner returns a wrapper around the cli-utils preview command PreviewRunner. Sets
+// up the Run on this wrapped runner to be the PreviewRunnerWrapper run.
+func GetPreviewRunner(provider provider.Provider, loader manifestreader.ManifestLoader, ioStreams genericclioptions.IOStreams) *PreviewRunnerWrapper {
+	previewRunner := preview.GetPreviewRunner(provider, loader, ioStreams)
+	w := &PreviewRunnerWrapper{
+		previewRunner: previewRunner,
+		factory:       provider.Factory(),
+	}
+	// Set the wrapper run to be the RunE function for the wrapped command.
+	previewRunner.Command.RunE = w.RunE
+	return w
+}
+
+// PreviewRunnerWrapper encapsulates the cli-utils preview command PreviewRunner as well
+// as structures necessary to run.
+type PreviewRunnerWrapper struct {
+	previewRunner *preview.PreviewRunner
+	factory       cmdutil.Factory
+}
+
+// Command returns the wrapped PreviewRunner cobraCommand structure.
+func (w *PreviewRunnerWrapper) Command() *cobra.Command {
+	return w.previewRunner.Command
+}
+
+// RunE checks if required setters are set as a pre-step if Kptfile
+// exists in the package path. Then the wrapped PreviewRunner is
+// invoked. Returns an error if one happened.
+func (w *PreviewRunnerWrapper) RunE(cmd *cobra.Command, args []string) error {
+	if len(args) > 0 {
+		if err := setters.CheckForRequiredSetters(args[0]); err != nil {
+			return err
+		}
+	}
+	return w.previewRunner.RunE(cmd, args)
+}

e2e/e2e_test.go

@@ -471,3 +471,133 @@ spec:
 		})
 	}
 }
+
+func TestLiveCommands(t *testing.T) {
+	var tests = []struct {
+		name         string
+		inputOpenAPI string
+		command      string
+		args         []string
+		out          string
+		errMsg       string
+	}{
+		{
+			name:    "test preview command setters pre-check",
+			command: "preview",
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: OpenAPIfile
+openAPI:
+  definitions:
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+          required: true
+          isSet: false
+ `,
+			errMsg: `setter replicas is required but not set, please set it to new value and try again`,
+		},
+		{
+			name:    "test apply command setters pre-check",
+			command: "apply",
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: OpenAPIfile
+openAPI:
+  definitions:
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+          required: true
+          isSet: false
+ `,
+			errMsg: `setter replicas is required but not set, please set it to new value and try again`,
+		},
+		{
+			name:    "preview command setters pre-check pass, fail with no cluster",
+			command: "preview",
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: OpenAPIfile
+openAPI:
+  definitions:
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+          required: true
+          isSet: true
+ `,
+			errMsg: `Package uninitialized`,
+		},
+		{
+			name:    "apply command setters pre-check pass, fail with no cluster",
+			command: "apply",
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: OpenAPIfile
+openAPI:
+  definitions:
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+          required: true
+          isSet: true
+ `,
+			errMsg: `Package uninitialized`,
+		},
+	}
+	for i := range tests {
+		test := tests[i]
+		t.Run(test.name, func(t *testing.T) {
+			// reset the openAPI afterward
+			openapi.ResetOpenAPI()
+			defer openapi.ResetOpenAPI()
+
+			dir, err := ioutil.TempDir("", "")
+			if err != nil {
+				t.FailNow()
+			}
+			defer os.RemoveAll(dir)
+
+			err = ioutil.WriteFile(dir+"/Kptfile", []byte(test.inputOpenAPI), 0600)
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+
+			cmd := run.GetMain()
+			args := []string{"live", test.command, dir}
+			args = append(args, test.args...)
+			cmd.SetArgs(args)
+			err = cmd.Execute()
+
+			if test.errMsg != "" {
+				if !assert.NotNil(t, err) {
+					t.FailNow()
+				}
+				if !assert.Contains(t, err.Error(), test.errMsg) {
+					t.FailNow()
+				}
+			}
+
+			if test.errMsg == "" && !assert.NoError(t, err) {
+				t.FailNow()
+			}
+		})
+	}
+}

internal/util/setters/setters.go

@@ -364,3 +364,24 @@ func DefExists(resourcePath, setterName string) bool {
 	setter, _ := openapi.Resolve(&ref, sc)
 	return setter != nil
 }
+
+// CheckForRequiredSetters takes the package path, checks if there is a KrmFile
+// and checks if all the required setters are set
+func CheckForRequiredSetters(path string) error {
+	kptFilePath := filepath.Join(path, kptfile.KptFileName)
+	_, err := os.Stat(kptFilePath)
+	if err != nil {
+		// if file is not readable or doesn't exist, exit without error
+		// as there might be packages without KrmFile
+		return nil
+	}
+	settersSchema, err := openapi.SchemaFromFile(kptFilePath)
+	if err != nil {
+		return err
+	}
+	if settersSchema == nil {
+		// this happens when there is Kptfile but no setter definitions
+		return nil
+	}
+	return setters2.CheckRequiredSettersSet(settersSchema)
+}

internal/util/setters/setters_test.go

@@ -663,3 +663,70 @@ metadata:
 		})
 	}
 }
+
+func TestCheckRequiredSettersSet(t *testing.T) {
+	var tests = []struct {
+		name             string
+		inputOpenAPIfile string
+		expectedError    bool
+	}{
+		{
+			name: "required true, isSet false",
+			inputOpenAPIfile: `
+apiVersion: v1alpha1
+kind: OpenAPIfile
+openAPI:
+  definitions:
+    io.k8s.cli.setters.gcloud.project.projectNumber:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: gcloud.project.projectNumber
+          value: "123"
+          setBy: me
+    io.k8s.cli.setters.replicas:
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: replicas
+          value: "3"
+          setBy: me
+          required: true
+          isSet: false
+ `,
+			expectedError: true,
+		},
+		{
+			name:             "no file, no error",
+			inputOpenAPIfile: ``,
+			expectedError:    false,
+		},
+		{
+			name: "no setter defs, no error",
+			inputOpenAPIfile: `apiVersion: v1alpha1
+kind: OpenAPIfile`,
+			expectedError: false,
+		},
+	}
+	for i := range tests {
+		test := tests[i]
+		t.Run(test.name, func(t *testing.T) {
+			dir, err := ioutil.TempDir("", "")
+			assert.NoError(t, err)
+			defer os.RemoveAll(dir)
+			if test.inputOpenAPIfile != "" {
+				err = ioutil.WriteFile(filepath.Join(dir, "Kptfile"), []byte(test.inputOpenAPIfile), 0600)
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+			}
+			err = CheckForRequiredSetters(dir)
+			if test.expectedError && !assert.Error(t, err) {
+				t.FailNow()
+			}
+			if !test.expectedError && !assert.NoError(t, err) {
+				t.FailNow()
+			}
+		})
+	}
+}