Merge pull request #175 from seans3/diff-stdin-fix

Expand stdin and filter inventory object for diff

cmd/diff/cmddiff.go

@@ -4,10 +4,12 @@
 package diff
 
 import (
-	"fmt"
+	"io/ioutil"
+	"os"
 
 	"github.com/spf13/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"k8s.io/klog"
 	"k8s.io/kubectl/pkg/cmd/apply"
 	"k8s.io/kubectl/pkg/cmd/diff"
 	"k8s.io/kubectl/pkg/cmd/util"
@@ -15,18 +17,22 @@ import (
 	"sigs.k8s.io/cli-utils/pkg/common"
 )
 
+const tmpDirPrefix = "diff-cmd"
+
 // NewCmdDiff returns cobra command to implement client-side diff of package
 // directory. For each local config file, get the resource in the cluster
 // and diff the local config resource against the resource in the cluster.
 func NewCmdDiff(f util.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
 	options := diff.NewDiffOptions(ioStreams)
 	cmd := &cobra.Command{
-		Use:                   "diff DIRECTORY",
+		Use:                   "diff (DIRECTORY | STDIN)",
 		DisableFlagsInUseLine: true,
 		Short:                 i18n.T("Diff local config against cluster applied version"),
 		Args:                  cobra.MaximumNArgs(1),
 		Run: func(cmd *cobra.Command, args []string) {
-			util.CheckErr(Initialize(options, f, args))
+			cleanupFunc, err := Initialize(options, f, args)
+			defer cleanupFunc()
+			util.CheckErr(err)
 			util.CheckErr(options.Run())
 		},
 	}
@@ -35,39 +41,53 @@ func NewCmdDiff(f util.Factory, ioStreams genericclioptions.IOStreams) *cobra.Co
 }
 
 // Initialize fills in the DiffOptions in preparation for DiffOptions.Run().
-// Returns error if there is an error filling in the options or if there
+// Returns a cleanup function for removing temp files after expanding stdin, or
+// error if there is an error filling in the options or if there
 // is not one argument that is a directory.
-func Initialize(o *diff.DiffOptions, f util.Factory, args []string) error {
+func Initialize(o *diff.DiffOptions, f util.Factory, args []string) (func(), error) {
-	// TODO(seans3): Accept no arguments, treating it as stdin like other commands.
+	cleanupFunc := func() {}
-	if len(args) == 0 {
-		return fmt.Errorf("missing argument; specify exactly one directory path argument")
-	}
 	// Validate the only argument is a (package) directory path.
 	filenameFlags, err := common.DemandOneDirectory(args)
 	if err != nil {
-		return err
+		return cleanupFunc, err
 	}
-	// We do not want to diff the inventory object. So we expand
+	// Process input from stdin
-	// the config file paths, excluding the inventory object.
+	if len(args) == 0 {
-	filenameFlags, err = common.ExpandPackageDir(filenameFlags)
+		tmpDir, err := createTempDir()
-	if err != nil {
+		if err != nil {
-		return err
+			return cleanupFunc, err
+		}
+		cleanupFunc = func() {
+			os.RemoveAll(tmpDir)
+		}
+		filenameFlags.Filenames = &[]string{tmpDir}
+		klog.V(6).Infof("stdin diff command temp dir: %s", tmpDir)
+		if err := common.FilterInputFile(os.Stdin, tmpDir); err != nil {
+			return cleanupFunc, err
+		}
+	} else {
+		// We do not want to diff the inventory object. So we expand
+		// the config file paths, excluding the inventory object.
+		filenameFlags, err = common.ExpandPackageDir(filenameFlags)
+		if err != nil {
+			return cleanupFunc, err
+		}
 	}
 	o.FilenameOptions = filenameFlags.ToOptions()
 
 	o.OpenAPISchema, err = f.OpenAPISchema()
 	if err != nil {
-		return err
+		return cleanupFunc, err
 	}
 
 	o.DiscoveryClient, err = f.ToDiscoveryClient()
 	if err != nil {
-		return err
+		return cleanupFunc, err
 	}
 
 	o.DynamicClient, err = f.DynamicClient()
 	if err != nil {
-		return err
+		return cleanupFunc, err
 	}
 
 	o.DryRunVerifier = &apply.DryRunVerifier{
@@ -77,7 +97,7 @@ func Initialize(o *diff.DiffOptions, f util.Factory, args []string) error {
 
 	o.CmdNamespace, o.EnforceNamespace, err = f.ToRawKubeConfigLoader().Namespace()
 	if err != nil {
-		return err
+		return cleanupFunc, err
 	}
 
 	o.Builder = f.NewBuilder()
@@ -86,5 +106,15 @@ func Initialize(o *diff.DiffOptions, f util.Factory, args []string) error {
 	o.ServerSideApply = false
 	o.ForceConflicts = false
 
-	return nil
+	return cleanupFunc, nil
+}
+
+func createTempDir() (string, error) {
+	// Create a temporary file with the passed prefix in
+	// the default temporary directory.
+	tmpDir, err := ioutil.TempDir("", tmpDirPrefix)
+	if err != nil {
+		return "", err
+	}
+	return tmpDir, nil
 }

pkg/common/path.go

@@ -5,15 +5,23 @@ package common
 
 import (
 	"fmt"
+	"io"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"k8s.io/klog"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
+	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
-const stdinDash = "-"
+const (
+	stdinDash    = "-"
+	tmpDirPrefix = "diff-cmd-config"
+	fileRegexp   = "config-*.yaml"
+)
 
 func processPaths(paths []string) genericclioptions.FileNameFlags {
 	// No arguments means we are reading from StdIn
@@ -80,6 +88,54 @@ func ExpandPackageDir(f genericclioptions.FileNameFlags) (genericclioptions.File
 	return f, nil
 }
 
+// FilterInputFile copies the resource config on stdin into a file
+// at the tmpDir, filtering the inventory object. It is the
+// responsibility of the caller to clean up the tmpDir. Returns
+// an error if one occurs.
+func FilterInputFile(in io.Reader, tmpDir string) error {
+	// Copy the config from "in" into a local temp file.
+	dir, err := ioutil.TempDir("", tmpDirPrefix)
+	if err != nil {
+		return err
+	}
+	tmpFile, err := ioutil.TempFile(dir, fileRegexp)
+	if err != nil {
+		return err
+	}
+	defer os.RemoveAll(dir)
+	klog.V(6).Infof("Temp File: %s", tmpFile.Name())
+	if _, err := io.Copy(tmpFile, in); err != nil {
+		return err
+	}
+	// Read the config stored locally, parsing into RNodes
+	r := kio.LocalPackageReader{PackagePath: dir}
+	nodes, err := r.Read()
+	if err != nil {
+		return err
+	}
+	klog.V(6).Infof("Num read configs: %d", len(nodes))
+	// Filter RNodes to remove the inventory object.
+	filteredNodes := []*yaml.RNode{}
+	for _, node := range nodes {
+		meta, err := node.GetMeta()
+		if err != nil {
+			continue
+		}
+		// If object has inventory label, skip it.
+		labels := meta.Labels
+		if _, exists := labels[InventoryLabel]; !exists {
+			filteredNodes = append(filteredNodes, node)
+		}
+	}
+	// Write the remaining configs into a file in the tmpDir
+	w := kio.LocalPackageWriter{
+		PackagePath:           tmpDir,
+		KeepReaderAnnotations: false,
+	}
+	klog.V(6).Infof("Writing %d configs", len(filteredNodes))
+	return w.Write(filteredNodes)
+}
+
 // expandDir takes a single package directory as a parameter, and returns
 // an array of config file paths excluding the inventory object. Returns
 // an error if one occurred while processing the paths.

pkg/common/path_test.go

@@ -4,6 +4,9 @@
 package common
 
 import (
+	"bytes"
+	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -18,6 +21,7 @@ const (
 	inventoryFilename = "inventory.yaml"
 	podAFilename      = "pod-a.yaml"
 	podBFilename      = "pod-b.yaml"
+	configSeparator   = "---"
 )
 
 var (
@@ -68,7 +72,7 @@ var podB = []byte(`
 apiVersion: v1
 kind: Pod
 metadata:
-  name: pod-a
+  name: pod-b
   namespace: test-namespace
   labels:
     name: test-pod-label
@@ -78,6 +82,17 @@ spec:
     image: k8s.gcr.io/pause:2.0
 `)
 
+func buildMultiResourceConfig(configs ...[]byte) []byte {
+	r := []byte{}
+	for i, config := range configs {
+		if i > 0 {
+			r = append(r, []byte(configSeparator)...)
+		}
+		r = append(r, config...)
+	}
+	return r
+}
+
 func TestProcessPaths(t *testing.T) {
 	tf := setupTestFilesystem(t)
 	defer tf.Clean()
@@ -130,6 +145,79 @@ func TestProcessPaths(t *testing.T) {
 	}
 }
 
+func TestFilterInputFile(t *testing.T) {
+	tf := testutil.Setup(t)
+	defer tf.Clean()
+
+	testCases := map[string]struct {
+		configObjects   [][]byte
+		expectedObjects [][]byte
+	}{
+		"Empty config objects writes empty file": {
+			configObjects:   [][]byte{},
+			expectedObjects: [][]byte{},
+		},
+		"Only inventory obj writes empty file": {
+			configObjects:   [][]byte{inventoryConfigMap},
+			expectedObjects: [][]byte{},
+		},
+		"Only pods writes both pods": {
+			configObjects:   [][]byte{podA, podB},
+			expectedObjects: [][]byte{podA, podB},
+		},
+		"Basic case of inventory obj and two pods": {
+			configObjects:   [][]byte{inventoryConfigMap, podA, podB},
+			expectedObjects: [][]byte{podA, podB},
+		},
+		"Basic case of inventory obj and two pods in different order": {
+			configObjects:   [][]byte{podB, inventoryConfigMap, podA},
+			expectedObjects: [][]byte{podB, podA},
+		},
+	}
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			// Build a single file of multiple resource configs, and
+			// call the tested function FilterInputFile. This writes
+			// the passed file to the test filesystem, filtering
+			// the inventory object if it exists in the passed file.
+			in := buildMultiResourceConfig(tc.configObjects...)
+			err := FilterInputFile(bytes.NewReader(in), tf.GetRootDir())
+			if err != nil {
+				t.Fatalf("Unexpected error in FilterInputFile: %s", err)
+			}
+			// Retrieve the files from the test filesystem.
+			actualFiles, err := ioutil.ReadDir(tf.GetRootDir())
+			if err != nil {
+				t.Fatalf("Error reading test filesystem directory: %s", err)
+			}
+			// Since we remove the generated file for each test, there should
+			// not be more than one file in the test filesystem.
+			if len(actualFiles) > 1 {
+				t.Fatalf("Wrong number of files (%d) in dir: %s", len(actualFiles), tf.GetRootDir())
+			}
+			// If there is a generated file, then read it into actualStr.
+			actualStr := ""
+			if len(actualFiles) != 0 {
+				actualFilename := (actualFiles[0]).Name()
+				defer os.Remove(actualFilename)
+				actual, err := ioutil.ReadFile(actualFilename)
+				if err != nil {
+					t.Fatalf("Error reading created file (%s): %s", actualFilename, err)
+				}
+				actualStr = strings.TrimSpace(string(actual))
+			}
+			// Build the expected string from the expectedObjects. This expected
+			// string should not have the inventory object config in it.
+			expected := buildMultiResourceConfig(tc.expectedObjects...)
+			expectedStr := strings.TrimSpace(string(expected))
+			if expectedStr != actualStr {
+				t.Errorf("Expected file contents (%s) not equal to actual file contents (%s)",
+					expectedStr, actualStr)
+			}
+		})
+	}
+}
+
 func TestExpandDirErrors(t *testing.T) {
 	tf := setupTestFilesystem(t)
 	defer tf.Clean()