feat: masked secrets in kubectl diff output (#96084)

* fix tests

* add changes

staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go

@@ -84,6 +84,13 @@ var (
 // Number of times we try to diff before giving-up
 const maxRetries = 4
 
+// Constants for masking sensitive values
+const (
+	sensitiveMaskDefault = "***"
+	sensitiveMaskBefore  = "*** (before)"
+	sensitiveMaskAfter   = "*** (after)"
+)
+
 // diffError returns the ExitError if the status code is less than 1,
 // nil otherwise.
 func diffError(err error) exec.ExitError {
@@ -257,17 +264,13 @@ func (v *DiffVersion) getObject(obj Object) (runtime.Object, error) {
 }
 
 // Print prints the object using the printer into a new file in the directory.
-func (v *DiffVersion) Print(obj Object, printer Printer) error {
-	vobj, err := v.getObject(obj)
-	if err != nil {
-		return err
-	}
-	f, err := v.Dir.NewFile(obj.Name())
+func (v *DiffVersion) Print(name string, obj runtime.Object, printer Printer) error {
+	f, err := v.Dir.NewFile(name)
 	if err != nil {
 		return err
 	}
 	defer f.Close()
-	return printer.Print(vobj, f)
+	return printer.Print(obj, f)
 }
 
 // Directory creates a new temp directory, and allows to easily create new files.
@@ -407,6 +410,125 @@ func (obj InfoObject) Name() string {
 	)
 }
 
+// toUnstructured converts a runtime.Object into an unstructured.Unstructured object.
+func toUnstructured(obj runtime.Object) (*unstructured.Unstructured, error) {
+	if obj == nil {
+		return nil, nil
+	}
+	c, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.DeepCopyObject())
+	if err != nil {
+		return nil, fmt.Errorf("convert to unstructured: %w", err)
+	}
+	u := &unstructured.Unstructured{}
+	u.SetUnstructuredContent(c)
+	return u, nil
+}
+
+// Masker masks sensitive values in an object while preserving diff-able
+// changes.
+//
+// All sensitive values in the object will be masked with a fixed-length
+// asterisk mask. If two values are different, an additional suffix will
+// be added so they can be diff-ed.
+type Masker struct {
+	from *unstructured.Unstructured
+	to   *unstructured.Unstructured
+}
+
+func NewMasker(from, to runtime.Object) (*Masker, error) {
+	// Convert objects to unstructured
+	f, err := toUnstructured(from)
+	if err != nil {
+		return nil, fmt.Errorf("convert to unstructured: %w", err)
+	}
+	t, err := toUnstructured(to)
+	if err != nil {
+		return nil, fmt.Errorf("convert to unstructured: %w", err)
+	}
+
+	// Run masker
+	m := &Masker{
+		from: f,
+		to:   t,
+	}
+	if err := m.run(); err != nil {
+		return nil, fmt.Errorf("run masker: %w", err)
+	}
+	return m, nil
+}
+
+// dataFromUnstructured returns the underlying nested map in the data key.
+func (m Masker) dataFromUnstructured(u *unstructured.Unstructured) (map[string]interface{}, error) {
+	if u == nil {
+		return nil, nil
+	}
+	data, found, err := unstructured.NestedMap(u.UnstructuredContent(), "data")
+	if err != nil {
+		return nil, fmt.Errorf("get nested map: %w", err)
+	}
+	if !found {
+		return nil, nil
+	}
+	return data, nil
+}
+
+// run compares and patches sensitive values.
+func (m *Masker) run() error {
+	// Extract nested map object
+	from, err := m.dataFromUnstructured(m.from)
+	if err != nil {
+		return fmt.Errorf("extract 'data' field: %w", err)
+	}
+	to, err := m.dataFromUnstructured(m.to)
+	if err != nil {
+		return fmt.Errorf("extract 'data' field: %w", err)
+	}
+
+	for k := range from {
+		// Add before/after suffix when key exists on both
+		// objects and are not equal, so that it will be
+		// visible in diffs.
+		if _, ok := to[k]; ok {
+			if from[k] != to[k] {
+				from[k] = sensitiveMaskBefore
+				to[k] = sensitiveMaskAfter
+				continue
+			}
+			to[k] = sensitiveMaskDefault
+		}
+		from[k] = sensitiveMaskDefault
+	}
+	for k := range to {
+		// Mask remaining keys that were not in 'from'
+		if _, ok := from[k]; !ok {
+			to[k] = sensitiveMaskDefault
+		}
+	}
+
+	// Patch objects with masked data
+	if m.from != nil && from != nil {
+		if err := unstructured.SetNestedMap(m.from.UnstructuredContent(), from, "data"); err != nil {
+			return fmt.Errorf("patch masked data: %w", err)
+		}
+	}
+	if m.to != nil && to != nil {
+		if err := unstructured.SetNestedMap(m.to.UnstructuredContent(), to, "data"); err != nil {
+			return fmt.Errorf("patch masked data: %w", err)
+		}
+	}
+	return nil
+}
+
+// From returns the masked version of the 'from' object.
+func (m *Masker) From() runtime.Object {
+	return m.from
+}
+
+// To returns the masked version of the 'to' object.
+func (m *Masker) To() runtime.Object {
+	return m.to
+}
+
 // Differ creates two DiffVersion and diffs them.
 type Differ struct {
 	From *DiffVersion
@@ -431,10 +553,28 @@ func NewDiffer(from, to string) (*Differ, error) {
 
 // Diff diffs to versions of a specific object, and print both versions to directories.
 func (d *Differ) Diff(obj Object, printer Printer) error {
-	if err := d.From.Print(obj, printer); err != nil {
+	from, err := d.From.getObject(obj)
+	if err != nil {
 		return err
 	}
-	if err := d.To.Print(obj, printer); err != nil {
+	to, err := d.To.getObject(obj)
+	if err != nil {
+		return err
+	}
+
+	// Mask secret values if object is V1Secret
+	if gvk := to.GetObjectKind().GroupVersionKind(); gvk.Version == "v1" && gvk.Kind == "Secret" {
+		m, err := NewMasker(from, to)
+		if err != nil {
+			return err
+		}
+		from, to = m.From(), m.To()
+	}
+
+	if err := d.From.Print(obj.Name(), from, printer); err != nil {
+		return err
+	}
+	if err := d.To.Print(obj.Name(), to, printer); err != nil {
 		return err
 	}
 	return nil

staging/src/k8s.io/kubectl/pkg/cmd/diff/diff_test.go

@@ -25,6 +25,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -44,10 +46,18 @@ func (f *FakeObject) Name() string {
 }
 
 func (f *FakeObject) Merged() (runtime.Object, error) {
+	// Return nil if merged object does not exist
+	if f.merged == nil {
+		return nil, nil
+	}
 	return &unstructured.Unstructured{Object: f.merged}, nil
 }
 
 func (f *FakeObject) Live() runtime.Object {
+	// Return nil if live object does not exist
+	if f.live == nil {
+		return nil
+	}
 	return &unstructured.Unstructured{Object: f.live}
 }
 
@@ -115,7 +125,11 @@ func TestDiffVersion(t *testing.T) {
 		live:   map[string]interface{}{"live": true},
 		merged: map[string]interface{}{"merged": true},
 	}
-	err = diff.Print(&obj, Printer{})
+	rObj, err := obj.Merged()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = diff.Print(obj.Name(), rObj, Printer{})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -208,3 +222,338 @@ func TestDiffer(t *testing.T) {
 		t.Fatalf("File has %q, expected %q", string(fcontent), econtent)
 	}
 }
+
+func TestMasker(t *testing.T) {
+	type diff struct {
+		from runtime.Object
+		to   runtime.Object
+	}
+	cases := []struct {
+		name  string
+		input diff
+		want  diff
+	}{
+		{
+			name: "no_changes",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***", // still masked
+							"password": "***", // still masked
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***", // still masked
+							"password": "***", // still masked
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "object_created",
+			input: diff{
+				from: nil, // does not exist yet
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+			},
+			want: diff{
+				from: nil, // does not exist yet
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***", // no suffix needed
+							"password": "***", // no suffix needed
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "object_removed",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+				to: nil, // removed
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***", // no suffix needed
+							"password": "***", // no suffix needed
+						},
+					},
+				},
+				to: nil, // removed
+			},
+		},
+		{
+			name: "data_key_added",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123", // added
+						},
+					},
+				},
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+							"password": "***", // no suffix needed
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "data_key_changed",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "456", // changed
+						},
+					},
+				},
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+							"password": "*** (before)", // added suffix for diff
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+							"password": "*** (after)", // added suffix for diff
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "data_key_removed",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							// "password": "123", // removed
+						},
+					},
+				},
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+							"password": "***", // no suffix needed
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+							// "password": "***",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "empty_secret_from",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{}, // no data key
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{}, // no data key
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+							"password": "***",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "empty_secret_to",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{}, // no data key
+				},
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"data": map[string]interface{}{
+							"username": "***",
+							"password": "***",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{}, // no data key
+				},
+			},
+		},
+		{
+			name: "invalid_data_key",
+			input: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"some_other_key": map[string]interface{}{ // invalid key
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"some_other_key": map[string]interface{}{ // invalid key
+							"username": "abc",
+							"password": "123",
+						},
+					},
+				},
+			},
+			want: diff{
+				from: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"some_other_key": map[string]interface{}{
+							"username": "abc", // skipped
+							"password": "123", // skipped
+						},
+					},
+				},
+				to: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"some_other_key": map[string]interface{}{
+							"username": "abc", // skipped
+							"password": "123", // skipped
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range cases {
+		tc := tc // capture range variable
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			m, err := NewMasker(tc.input.from, tc.input.to)
+			if err != nil {
+				t.Fatal(err)
+			}
+			from, to := m.From(), m.To()
+			if from != nil && tc.want.from != nil {
+				if diff := cmp.Diff(from, tc.want.from); diff != "" {
+					t.Errorf("from: (-want +got):\n%s", diff)
+				}
+			}
+			if to != nil && tc.want.to != nil {
+				if diff := cmp.Diff(to, tc.want.to); diff != "" {
+					t.Errorf("to: (-want +got):\n%s", diff)
+				}
+			}
+		})
+	}
+}