Decouple manifest reader from cli

Author: phanimarupaka

cmd/apply/cmdapply.go

@@ -113,7 +113,7 @@ func (r *ApplyRunner) RunE(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), args)
+	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), flagutils.PathFromArgs(args))
 	if err != nil {
 		return err
 	}

cmd/destroy/cmddestroy.go

@@ -72,7 +72,7 @@ func (r *DestroyRunner) RunE(cmd *cobra.Command, args []string) error {
 	}
 
 	// Retrieve the inventory object.
-	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), args)
+	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), flagutils.PathFromArgs(args))
 	if err != nil {
 		return err
 	}

cmd/flagutils/utils.go

@@ -26,3 +26,12 @@ func ConvertInventoryPolicy(policy string) (inventory.InventoryPolicy, error) {
 			"inventory policy must be one of strict, adopt")
 	}
 }
+
+// PathFromArgs returns the path which is a positional arg from args list
+// returns "-" if there is length of args is 0, which implies no path is provided
+func PathFromArgs(args []string) string {
+	if len(args) == 0 {
+		return "-"
+	}
+	return args[0]
+}

cmd/preview/cmdpreview.go

@@ -103,7 +103,7 @@ func (r *PreviewRunner) RunE(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), args)
+	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), flagutils.PathFromArgs(args))
 	if err != nil {
 		return err
 	}

cmd/status/cmdstatus.go

@@ -12,6 +12,7 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	cmdutil "k8s.io/kubectl/pkg/cmd/util"
+	"sigs.k8s.io/cli-utils/cmd/flagutils"
 	"sigs.k8s.io/cli-utils/cmd/status/printers"
 	"sigs.k8s.io/cli-utils/pkg/apply/poller"
 	"sigs.k8s.io/cli-utils/pkg/common"
@@ -77,7 +78,7 @@ func (r *StatusRunner) runE(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), args)
+	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), flagutils.PathFromArgs(args))
 	if err != nil {
 		return err
 	}

pkg/manifestreader/fake-loader.go

@@ -27,7 +27,7 @@ func NewFakeLoader(f util.Factory, objs []object.ObjMetadata) *fakeLoader {
 	}
 }
 
-func (f *fakeLoader) ManifestReader(reader io.Reader, _ []string) (ManifestReader, error) {
+func (f *fakeLoader) ManifestReader(reader io.Reader, _ string) (ManifestReader, error) {
 	mapper, err := f.factory.ToRESTMapper()
 	if err != nil {
 		return nil, err

pkg/manifestreader/manifestloader.go

@@ -15,7 +15,7 @@ import (
 // and parsing the resources and inventory info.
 type ManifestLoader interface {
 	InventoryInfo([]*unstructured.Unstructured) (inventory.InventoryInfo, []*unstructured.Unstructured, error)
-	ManifestReader(reader io.Reader, args []string) (ManifestReader, error)
+	ManifestReader(reader io.Reader, path string) (ManifestReader, error)
 }
 
 // manifestLoader implements the ManifestLoader interface
@@ -37,7 +37,7 @@ func (f *manifestLoader) InventoryInfo(objs []*unstructured.Unstructured) (inven
 	return inventory.WrapInventoryInfoObj(invObj), objs, err
 }
 
-func (f *manifestLoader) ManifestReader(reader io.Reader, args []string) (ManifestReader, error) {
+func (f *manifestLoader) ManifestReader(reader io.Reader, path string) (ManifestReader, error) {
 	// Fetch the namespace from the configloader. The source of this
 	// either the namespace flag or the context. If the namespace is provided
 	// with the flag, enforceNamespace will be true. In this case, it is
@@ -59,18 +59,24 @@ func (f *manifestLoader) ManifestReader(reader io.Reader, args []string) (Manife
 		EnforceNamespace: enforceNamespace,
 	}
 
+	return mReader(path, reader, readerOptions), nil
+}
+
+// mReader returns the ManifestReader based in the input args
+func mReader(path string, reader io.Reader, readerOptions ReaderOptions) ManifestReader {
 	var mReader ManifestReader
-	if len(args) == 0 {
+	// Read from stdin if "-" is specified, similar to kubectl
+	if path == "-" {
 		mReader = &StreamManifestReader{
 			ReaderName:    "stdin",
 			Reader:        reader,
 			ReaderOptions: readerOptions,
 		}
 	} else {
 		mReader = &PathManifestReader{
-			Path:          args[0],
+			Path:          path,
 			ReaderOptions: readerOptions,
 		}
 	}
-	return mReader, nil
+	return mReader
 }

pkg/manifestreader/manifestloader_test.go

@@ -0,0 +1,79 @@
+// Copyright 2020 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package manifestreader
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
+)
+
+func TestMReader_Read(t *testing.T) {
+	testCases := map[string]struct {
+		manifests        map[string]string
+		namespace        string
+		enforceNamespace bool
+		validate         bool
+		path             string
+
+		infosCount int
+		namespaces []string
+	}{
+		"path mReader: namespace should be set if not already present": {
+			namespace:        "foo",
+			enforceNamespace: true,
+			path:             "${reader-test-dir}",
+			infosCount:       1,
+			namespaces:       []string{"foo"},
+		},
+		"stream mReader: namespace should be set if not already present": {
+			namespace:        "foo",
+			enforceNamespace: true,
+			path:             "-",
+			infosCount:       1,
+			namespaces:       []string{"foo"},
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			tf := cmdtesting.NewTestFactory().WithNamespace("test-ns")
+			defer tf.Cleanup()
+
+			mapper, err := tf.ToRESTMapper()
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+
+			dir, err := ioutil.TempDir("", "reader-test")
+			assert.NoError(t, err)
+			p := filepath.Join(dir, "dep.yaml")
+			err = ioutil.WriteFile(p, []byte(depManifest), 0600)
+			assert.NoError(t, err)
+			stringReader := strings.NewReader(depManifest)
+
+			if tc.path == "${reader-test-dir}" {
+				tc.path = dir
+			}
+
+			objs, err := mReader(tc.path, stringReader, ReaderOptions{
+				Mapper:           mapper,
+				Namespace:        tc.namespace,
+				EnforceNamespace: tc.enforceNamespace,
+				Validate:         tc.validate,
+			}).Read()
+
+			assert.NoError(t, err)
+			assert.Equal(t, len(objs), tc.infosCount)
+
+			for i, obj := range objs {
+				assert.Equal(t, tc.namespaces[i], obj.GetNamespace())
+			}
+		})
+	}
+}