diff --git a/hack/testdata/pod-changed.yaml b/hack/testdata/pod-changed.yaml new file mode 100644 index 0000000000..69476100d3 --- /dev/null +++ b/hack/testdata/pod-changed.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Pod +metadata: + name: test-pod + labels: + name: test-pod-label +spec: + containers: + - name: kubernetes-pause + image: k8s.gcr.io/pause:3.0 diff --git a/pkg/kubectl/cmd/diff.go b/pkg/kubectl/cmd/diff.go index 33a5dc64f8..37094ab560 100644 --- a/pkg/kubectl/cmd/diff.go +++ b/pkg/kubectl/cmd/diff.go @@ -44,11 +44,8 @@ import ( var ( diffLong = templates.LongDesc(i18n.T(` - Diff configurations specified by filename or stdin between their local, - last-applied, live and/or "merged" versions. - - LOCAL and LIVE versions are diffed by default. Other available keywords - are MERGED and LAST. + Diff configurations specified by filename or stdin between the current online + configuration, and the configuration as it would be if applied. Output is always YAML. @@ -56,52 +53,22 @@ var ( diff command. By default, the "diff" command available in your path will be run with "-u" (unicode) and "-N" (treat new files as empty) options.`)) diffExample = templates.Examples(i18n.T(` - # Diff resources included in pod.json. By default, it will diff LOCAL and LIVE versions + # Diff resources included in pod.json. kubectl alpha diff -f pod.json - # When one version is specified, diff that version against LIVE - cat service.yaml | kubectl alpha diff -f - MERGED - - # Or specify both versions - kubectl alpha diff -f pod.json -f service.yaml LAST LOCAL`)) + # Diff file read from stdin + cat service.yaml | kubectl alpha diff -f -`)) ) type DiffOptions struct { FilenameOptions resource.FilenameOptions } -func isValidArgument(arg string) error { - switch arg { - case "LOCAL", "LIVE", "LAST", "MERGED": - return nil - default: - return fmt.Errorf(`Invalid parameter %q, must be either "LOCAL", "LIVE", "LAST" or "MERGED"`, arg) +func checkDiffArgs(cmd *cobra.Command, args []string) error { + if len(args) != 0 { + return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args) } - -} - -func parseDiffArguments(args []string) (string, string, error) { - if len(args) > 2 { - return "", "", fmt.Errorf("Invalid number of arguments: expected at most 2.") - } - // Default values - from := "LOCAL" - to := "LIVE" - if len(args) > 0 { - from = args[0] - } - if len(args) > 1 { - to = args[1] - } - - if err := isValidArgument(to); err != nil { - return "", "", err - } - if err := isValidArgument(from); err != nil { - return "", "", err - } - - return from, to, nil + return nil } func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command { @@ -113,13 +80,12 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C cmd := &cobra.Command{ Use: "diff -f FILENAME", DisableFlagsInUseLine: true, - Short: i18n.T("Diff different versions of configurations"), + Short: i18n.T("Diff live version against would-be applied version"), Long: diffLong, Example: diffExample, Run: func(cmd *cobra.Command, args []string) { - from, to, err := parseDiffArguments(args) - cmdutil.CheckErr(err) - cmdutil.CheckErr(RunDiff(f, &diff, &options, from, to)) + cmdutil.CheckErr(checkDiffArgs(cmd, args)) + cmdutil.CheckErr(RunDiff(f, &diff, &options)) }, } @@ -201,10 +167,6 @@ func (v *DiffVersion) getObject(obj Object) (map[string]interface{}, error) { return obj.Live() case "MERGED": return obj.Merged() - case "LOCAL": - return obj.Local() - case "LAST": - return obj.Last() } return nil, fmt.Errorf("Unknown version: %v", v.Name) } @@ -254,9 +216,7 @@ func (d *Directory) Delete() error { // Object is an interface that let's you retrieve multiple version of // it. type Object interface { - Local() (map[string]interface{}, error) Live() (map[string]interface{}, error) - Last() (map[string]interface{}, error) Merged() (map[string]interface{}, error) Name() string @@ -282,14 +242,6 @@ func (obj InfoObject) toMap(data []byte) (map[string]interface{}, error) { return m, err } -func (obj InfoObject) Local() (map[string]interface{}, error) { - data, err := runtime.Encode(obj.Encoder, obj.Info.Object) - if err != nil { - return nil, err - } - return obj.toMap(data) -} - func (obj InfoObject) Live() (map[string]interface{}, error) { if obj.Remote == nil { return nil, nil // Object doesn't exist on cluster. @@ -298,10 +250,11 @@ func (obj InfoObject) Live() (map[string]interface{}, error) { } func (obj InfoObject) Merged() (map[string]interface{}, error) { - local, err := obj.Local() + data, err := runtime.Encode(obj.Encoder, obj.Info.Object) if err != nil { return nil, err } + local, err := obj.toMap(data) live, err := obj.Live() if err != nil { @@ -436,14 +389,14 @@ func (d *Downloader) Download(info *resource.Info) (*unstructured.Unstructured, // RunDiff uses the factory to parse file arguments, find the version to // diff, and find each Info object for each files, and runs against the // differ. -func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions, from, to string) error { +func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions) error { openapi, err := f.OpenAPISchema() if err != nil { return err } parser := &parse.Factory{Resources: openapi} - differ, err := NewDiffer(from, to) + differ, err := NewDiffer("LIVE", "MERGED") if err != nil { return err } diff --git a/pkg/kubectl/cmd/diff_test.go b/pkg/kubectl/cmd/diff_test.go index af4b56e0fb..9597a1353a 100644 --- a/pkg/kubectl/cmd/diff_test.go +++ b/pkg/kubectl/cmd/diff_test.go @@ -31,10 +31,8 @@ import ( type FakeObject struct { name string - local map[string]interface{} merged map[string]interface{} live map[string]interface{} - last map[string]interface{} } var _ Object = &FakeObject{} @@ -43,10 +41,6 @@ func (f *FakeObject) Name() string { return f.name } -func (f *FakeObject) Local() (map[string]interface{}, error) { - return f.local, nil -} - func (f *FakeObject) Merged() (map[string]interface{}, error) { return f.merged, nil } @@ -55,87 +49,6 @@ func (f *FakeObject) Live() (map[string]interface{}, error) { return f.live, nil } -func (f *FakeObject) Last() (map[string]interface{}, error) { - return f.last, nil -} - -func TestArguments(t *testing.T) { - tests := []struct { - // Input - args []string - - // Outputs - from string - to string - err string - }{ - // Defaults - { - args: []string{}, - from: "LOCAL", - to: "LIVE", - err: "", - }, - // One valid argument - { - args: []string{"MERGED"}, - from: "MERGED", - to: "LIVE", - err: "", - }, - // One invalid argument - { - args: []string{"WRONG"}, - from: "", - to: "", - err: `Invalid parameter "WRONG", must be either "LOCAL", "LIVE", "LAST" or "MERGED"`, - }, - // Two valid arguments - { - args: []string{"MERGED", "LAST"}, - from: "MERGED", - to: "LAST", - err: "", - }, - // Two same arguments is fine - { - args: []string{"MERGED", "MERGED"}, - from: "MERGED", - to: "MERGED", - err: "", - }, - // Second argument is invalid - { - args: []string{"MERGED", "WRONG"}, - from: "", - to: "", - err: `Invalid parameter "WRONG", must be either "LOCAL", "LIVE", "LAST" or "MERGED"`, - }, - // Three arguments - { - args: []string{"MERGED", "LIVE", "LAST"}, - from: "", - to: "", - err: `Invalid number of arguments: expected at most 2.`, - }, - } - - for _, test := range tests { - from, to, e := parseDiffArguments(test.args) - err := "" - if e != nil { - err = e.Error() - } - if from != test.from || to != test.to || err != test.err { - t.Errorf("parseDiffArguments(%v) = (%v, %v, %v), expected (%v, %v, %v)", - test.args, - from, to, err, - test.from, test.to, test.err, - ) - } - } -} - func TestDiffProgram(t *testing.T) { os.Setenv("KUBERNETES_EXTERNAL_DIFF", "echo") streams, _, stdout, _ := genericclioptions.NewTestIOStreams() @@ -175,7 +88,7 @@ string: string } func TestDiffVersion(t *testing.T) { - diff, err := NewDiffVersion("LOCAL") + diff, err := NewDiffVersion("MERGED") if err != nil { t.Fatal(err) } @@ -183,8 +96,6 @@ func TestDiffVersion(t *testing.T) { obj := FakeObject{ name: "bla", - local: map[string]interface{}{"local": true}, - last: map[string]interface{}{"last": true}, live: map[string]interface{}{"live": true}, merged: map[string]interface{}{"merged": true}, } @@ -196,7 +107,7 @@ func TestDiffVersion(t *testing.T) { if err != nil { t.Fatal(err) } - econtent := "local: true\n" + econtent := "merged: true\n" if string(fcontent) != econtent { t.Fatalf("File has %q, expected %q", string(fcontent), econtent) } @@ -248,7 +159,7 @@ func TestDirectory(t *testing.T) { } func TestDiffer(t *testing.T) { - diff, err := NewDiffer("LOCAL", "LIVE") + diff, err := NewDiffer("LIVE", "MERGED") if err != nil { t.Fatal(err) } @@ -256,8 +167,6 @@ func TestDiffer(t *testing.T) { obj := FakeObject{ name: "bla", - local: map[string]interface{}{"local": true}, - last: map[string]interface{}{"last": true}, live: map[string]interface{}{"live": true}, merged: map[string]interface{}{"merged": true}, } @@ -269,7 +178,7 @@ func TestDiffer(t *testing.T) { if err != nil { t.Fatal(err) } - econtent := "local: true\n" + econtent := "live: true\n" if string(fcontent) != econtent { t.Fatalf("File has %q, expected %q", string(fcontent), econtent) } @@ -278,7 +187,7 @@ func TestDiffer(t *testing.T) { if err != nil { t.Fatal(err) } - econtent = "live: true\n" + econtent = "merged: true\n" if string(fcontent) != econtent { t.Fatalf("File has %q, expected %q", string(fcontent), econtent) } diff --git a/test/cmd/diff.sh b/test/cmd/diff.sh index 4edd8d8121..fb9d298b54 100755 --- a/test/cmd/diff.sh +++ b/test/cmd/diff.sh @@ -27,27 +27,13 @@ run_kubectl_diff_tests() { kube::log::status "Testing kubectl alpha diff" # Test that it works when the live object doesn't exist - output_message=$(kubectl alpha diff LOCAL LIVE -f hack/testdata/pod.yaml) + output_message=$(kubectl alpha diff -f hack/testdata/pod.yaml) kube::test::if_has_string "${output_message}" 'test-pod' kubectl apply -f hack/testdata/pod.yaml - # Ensure that selfLink has been added, and shown in the diff - output_message=$(kubectl alpha diff -f hack/testdata/pod.yaml) - kube::test::if_has_string "${output_message}" 'selfLink' - output_message=$(kubectl alpha diff LOCAL LIVE -f hack/testdata/pod.yaml) - kube::test::if_has_string "${output_message}" 'selfLink' - output_message=$(kubectl alpha diff LOCAL MERGED -f hack/testdata/pod.yaml) - kube::test::if_has_string "${output_message}" 'selfLink' - - output_message=$(kubectl alpha diff MERGED MERGED -f hack/testdata/pod.yaml) - kube::test::if_empty_string "${output_message}" - output_message=$(kubectl alpha diff LIVE LIVE -f hack/testdata/pod.yaml) - kube::test::if_empty_string "${output_message}" - output_message=$(kubectl alpha diff LAST LAST -f hack/testdata/pod.yaml) - kube::test::if_empty_string "${output_message}" - output_message=$(kubectl alpha diff LOCAL LOCAL -f hack/testdata/pod.yaml) - kube::test::if_empty_string "${output_message}" + output_message=$(kubectl alpha diff -f hack/testdata/pod-changed.yaml) + kube::test::if_has_string "${output_message}" 'k8s.gcr.io/pause:3.0' kubectl delete -f hack/testdata/pod.yaml