diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index b1310b04f4d..44b55da23cd 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/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 diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff_test.go index 9c68fb38775..d88f7ec378e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff_test.go +++ b/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) + } + } + }) + } +}