Merge pull request #61227 from apelisse/fix-diff

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

diff: Fix broken `Local()` logic

Local and Live functions where doing and returning the same thing,
giving empty results by default. Fix the local function by copying the
objects before fetching the live version.



**What this PR does / why we need it**: Diff prints empty output by default. Fixes it.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes https://github.com/kubernetes/kubernetes/issues/61145

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

hack/make-rules/test-cmd-util.sh

@@ -1185,6 +1185,39 @@ run_kubectl_apply_deployments_tests() {
   set +o errexit
 }
 
+# Runs tests for kubectl alpha diff
+run_kubectl_diff_tests() {
+    set -o nounset
+    set -o errexit
+
+    create_and_use_new_namespace
+    kube::log::status "Testing kubectl alpha diff"
+
+    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}"
+
+    kubectl delete -f  hack/testdata/pod.yaml
+
+    set +o nounset
+    set +o errexit
+}
+
 # Runs tests for --save-config tests.
 run_save_config_tests() {
   set -o nounset
@@ -4983,6 +5016,11 @@ runTests() {
     record_command run_kubectl_apply_deployments_tests
   fi
 
+  ################
+  # Kubectl diff #
+  ################
+  record_command run_kubectl_diff_tests
+
   ###############
   # Kubectl get #
   ###############

pkg/kubectl/cmd/diff.go

@@ -26,9 +26,11 @@ import (
 
 	"github.com/ghodss/yaml"
 	"github.com/spf13/cobra"
-	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/dynamic"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/kubectl/apply/parse"
 	"k8s.io/kubernetes/pkg/kubectl/apply/strategy"
@@ -263,6 +265,7 @@ type Object interface {
 // InfoObject is an implementation of the Object interface. It gets all
 // the information from the Info object.
 type InfoObject struct {
+	Remote  runtime.Unstructured
 	Info    *resource.Info
 	Encoder runtime.Encoder
 	Parser  *parse.Factory
@@ -288,14 +291,10 @@ func (obj InfoObject) Local() (map[string]interface{}, error) {
 }
 
 func (obj InfoObject) Live() (map[string]interface{}, error) {
-	if obj.Info.Object == nil {
+	if obj.Remote == nil {
 		return nil, nil // Object doesn't exist on cluster.
 	}
-	data, err := runtime.Encode(obj.Encoder, obj.Info.Object)
+	return obj.Remote.UnstructuredContent(), nil
-	if err != nil {
-		return nil, err
-	}
-	return obj.toMap(data)
 }
 
 func (obj InfoObject) Merged() (map[string]interface{}, error) {
@@ -327,10 +326,10 @@ func (obj InfoObject) Merged() (map[string]interface{}, error) {
 }
 
 func (obj InfoObject) Last() (map[string]interface{}, error) {
-	if obj.Info.Object == nil {
+	if obj.Remote == nil {
 		return nil, nil // No object is live, return empty
 	}
-	accessor, err := meta.Accessor(obj.Info.Object)
+	accessor, err := meta.Accessor(obj.Remote)
 	if err != nil {
 		return nil, err
 	}
@@ -390,6 +389,50 @@ func (d *Differ) TearDown() {
 	d.To.Dir.Delete()   // Ignore error
 }
 
+type Downloader struct {
+	mapper  meta.RESTMapper
+	dclient dynamic.Interface
+	ns      string
+}
+
+func NewDownloader(f cmdutil.Factory) (*Downloader, error) {
+	var err error
+	var d Downloader
+
+	d.mapper, err = f.RESTMapper()
+	if err != nil {
+		return nil, err
+	}
+	d.dclient, err = f.DynamicClient()
+	if err != nil {
+		return nil, err
+	}
+	d.ns, _, _ = f.DefaultNamespace()
+
+	return &d, nil
+}
+
+func (d *Downloader) Download(info *resource.Info) (*unstructured.Unstructured, error) {
+	gvk := info.Object.GetObjectKind().GroupVersionKind()
+	mapping, err := d.mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
+	if err != nil {
+		return nil, err
+	}
+
+	var resource dynamic.ResourceInterface
+	switch mapping.Scope.Name() {
+	case meta.RESTScopeNameNamespace:
+		if info.Namespace == "" {
+			info.Namespace = d.ns
+		}
+		resource = d.dclient.Resource(mapping.Resource).Namespace(info.Namespace)
+	case meta.RESTScopeNameRoot:
+		resource = d.dclient.Resource(mapping.Resource)
+	}
+
+	return resource.Get(info.Name, metav1.GetOptions{})
+}
+
 // 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.
@@ -417,25 +460,26 @@ func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions, from, t
 		Unstructured().
 		NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, &options.FilenameOptions).
+		Local().
 		Flatten().
 		Do()
 	if err := r.Err(); err != nil {
 		return err
 	}
 
+	dl, err := NewDownloader(f)
+	if err != nil {
+		return err
+	}
+
 	err = r.Visit(func(info *resource.Info, err error) error {
 		if err != nil {
 			return err
 		}
 
-		if err := info.Get(); err != nil {
+		remote, _ := dl.Download(info)
-			if !errors.IsNotFound(err) {
-				return cmdutil.AddSourceToErr(fmt.Sprintf("retrieving current configuration of:\n%s\nfrom server for:", info.String()), info.Source, err)
-			}
-			info.Object = nil
-		}
-
 		obj := InfoObject{
+			Remote:  remote,
 			Info:    info,
 			Parser:  parser,
 			Encoder: cmdutil.InternalVersionJSONEncoder(),