From c155b7bb87c4a94d3abc35cc4e3ae92d7dabe789 Mon Sep 17 00:00:00 2001 From: ZhengYuan Loo Date: Thu, 6 May 2021 12:51:15 +0800 Subject: [PATCH] feat: masked secrets in kubectl diff output (#96084) * fix tests * add changes Kubernetes-commit: 0cbf00ad018d5104e438b5693fde435fa06e30c9 --- Godeps/Godeps.json | 8 +- go.mod | 16 +- go.sum | 16 +- pkg/cmd/diff/diff.go | 158 ++++++++++++++++- pkg/cmd/diff/diff_test.go | 351 +++++++++++++++++++++++++++++++++++++- 5 files changed, 519 insertions(+), 30 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 24181f1a..849b9458 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1040,11 +1040,11 @@ }, { "ImportPath": "k8s.io/api", - "Rev": "73cb810742ad" + "Rev": "0097618c6e4c" }, { "ImportPath": "k8s.io/apimachinery", - "Rev": "83e6b5ff9c68" + "Rev": "fda713515979" }, { "ImportPath": "k8s.io/cli-runtime", @@ -1052,7 +1052,7 @@ }, { "ImportPath": "k8s.io/client-go", - "Rev": "db078d2f1bfd" + "Rev": "2ed8b3081056" }, { "ImportPath": "k8s.io/code-generator", @@ -1064,7 +1064,7 @@ }, { "ImportPath": "k8s.io/component-helpers", - "Rev": "090244e35f78" + "Rev": "d2cb76f2050a" }, { "ImportPath": "k8s.io/gengo", diff --git a/go.mod b/go.mod index db38f9da..18f71f6e 100644 --- a/go.mod +++ b/go.mod @@ -32,12 +32,12 @@ require ( github.com/stretchr/testify v1.6.1 golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073 gopkg.in/yaml.v2 v2.4.0 - k8s.io/api v0.0.0-20210518101604-73cb810742ad - k8s.io/apimachinery v0.0.0-20210518100455-83e6b5ff9c68 + k8s.io/api v0.0.0-20210518101607-13f29098a509 + k8s.io/apimachinery v0.0.0-20210518100456-fda713515979 k8s.io/cli-runtime v0.0.0-20210518124922-8a120cfd4c50 - k8s.io/client-go v0.0.0-20210518102921-db078d2f1bfd + k8s.io/client-go v0.0.0-20210518102924-2ed8b3081056 k8s.io/component-base v0.0.0-20210518111232-44faecbf614e - k8s.io/component-helpers v0.0.0-20210518112012-090244e35f78 + k8s.io/component-helpers v0.0.0-20210518112016-d2cb76f2050a k8s.io/klog/v2 v2.8.0 k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e k8s.io/metrics v0.0.0-20210518124132-b6e3c12b3e8a @@ -48,12 +48,12 @@ require ( ) replace ( - k8s.io/api => k8s.io/api v0.0.0-20210518101604-73cb810742ad - k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210518100455-83e6b5ff9c68 + k8s.io/api => k8s.io/api v0.0.0-20210505141115-0097618c6e4c + k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210518100456-fda713515979 k8s.io/cli-runtime => k8s.io/cli-runtime v0.0.0-20210518124922-8a120cfd4c50 - k8s.io/client-go => k8s.io/client-go v0.0.0-20210518102921-db078d2f1bfd + k8s.io/client-go => k8s.io/client-go v0.0.0-20210518102924-2ed8b3081056 k8s.io/code-generator => k8s.io/code-generator v0.0.0-20210518095634-b9c3fe925685 k8s.io/component-base => k8s.io/component-base v0.0.0-20210518111232-44faecbf614e - k8s.io/component-helpers => k8s.io/component-helpers v0.0.0-20210518112012-090244e35f78 + k8s.io/component-helpers => k8s.io/component-helpers v0.0.0-20210518112016-d2cb76f2050a k8s.io/metrics => k8s.io/metrics v0.0.0-20210518124132-b6e3c12b3e8a ) diff --git a/go.sum b/go.sum index f48e30af..be2ba916 100644 --- a/go.sum +++ b/go.sum @@ -731,19 +731,19 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= -k8s.io/api v0.0.0-20210518101604-73cb810742ad h1:Y3Y8dvutfX0ctUvW8lOIRWtGQ78do2iq0kyzOgP7Kzs= -k8s.io/api v0.0.0-20210518101604-73cb810742ad/go.mod h1:oMHvpJd6cjQV4EGOZctXILY3h91ViVRHybL+6tnKvPA= -k8s.io/apimachinery v0.0.0-20210518100455-83e6b5ff9c68 h1:TOBIgeJvdBu5pIW9s0eqs8ZlkY0Ld5oP84shav2e7Co= -k8s.io/apimachinery v0.0.0-20210518100455-83e6b5ff9c68/go.mod h1:fBRSkoylGO2QUTae8Wb2wac6pZ83/r+tL6HFSXGbzfs= +k8s.io/api v0.0.0-20210505141115-0097618c6e4c h1:QmszvJU0uDvIUanyZ0QhOYfZaILVvbkLJbRM7Ve3nts= +k8s.io/api v0.0.0-20210505141115-0097618c6e4c/go.mod h1:IgKLK42qxUHnMxdcdbEBFOo+TfTANemQTLg3gcGe8/M= +k8s.io/apimachinery v0.0.0-20210518100456-fda713515979 h1:qjGp8QxGJZSGSMQw31AFuI2za6IEfDmdJ5/cY+Eeas8= +k8s.io/apimachinery v0.0.0-20210518100456-fda713515979/go.mod h1:fBRSkoylGO2QUTae8Wb2wac6pZ83/r+tL6HFSXGbzfs= k8s.io/cli-runtime v0.0.0-20210518124922-8a120cfd4c50 h1:f0rVBw9d3DbPN6QLGO9++zqdlBgEW4PJYCM8MTFu1/Y= k8s.io/cli-runtime v0.0.0-20210518124922-8a120cfd4c50/go.mod h1:Wa1KFjbnEkfhs9pgjrdinm7T/xK0X8LNGM+/mF60PJY= -k8s.io/client-go v0.0.0-20210518102921-db078d2f1bfd h1:PQSnP399+SZ8ece3BUUV1Nn1CbujBfe5tdo8hfGMSU8= -k8s.io/client-go v0.0.0-20210518102921-db078d2f1bfd/go.mod h1:LMNQnUB2TU/kU7eqIJw+VESS9SZ7Q7TrXSk84icDAVo= +k8s.io/client-go v0.0.0-20210518102924-2ed8b3081056 h1:t4nxl6RpDRcvk1H0shNODINaiDEcBvGozB+ufxikbiA= +k8s.io/client-go v0.0.0-20210518102924-2ed8b3081056/go.mod h1:qBjkJPJp2FjoQFwy9eLFYI4JdHeFm0KH+j9/XQhCMi8= k8s.io/code-generator v0.0.0-20210518095634-b9c3fe925685/go.mod h1:tHNeGA58jE3nJvZLDN0c/5k7P3NlYdjbWuJYK9QiU3s= k8s.io/component-base v0.0.0-20210518111232-44faecbf614e h1:+LvsEqL+o2/s3SNrTdZm22miglZOm3TPof8oO5pa2F8= k8s.io/component-base v0.0.0-20210518111232-44faecbf614e/go.mod h1:VYU6k+gXEg7lXm+XCubCKvSlHHI9l+JHRJ0Hw4nrjug= -k8s.io/component-helpers v0.0.0-20210518112012-090244e35f78 h1:eab5RQFjErbldBe6CfHsgcBxrVAfmrI0vKakmZ+zQnw= -k8s.io/component-helpers v0.0.0-20210518112012-090244e35f78/go.mod h1:9f/0aloaLdeiOTnDlpZvMt+ZXGP38b4ygEDTPR3FEf4= +k8s.io/component-helpers v0.0.0-20210518112016-d2cb76f2050a h1:8/j0qwWx0p2tkFBK+URPu6ANIqp1MrZumZvlvfM7FwA= +k8s.io/component-helpers v0.0.0-20210518112016-d2cb76f2050a/go.mod h1:h0T3Vxhshgu1I5OuwwqkDdxOM7E3qsIDOA+hnvgxI7M= k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20201214224949-b6c5ce23f027/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= diff --git a/pkg/cmd/diff/diff.go b/pkg/cmd/diff/diff.go index b1310b04..44b55da2 100644 --- a/pkg/cmd/diff/diff.go +++ b/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/pkg/cmd/diff/diff_test.go b/pkg/cmd/diff/diff_test.go index 9c68fb38..d88f7ec3 100644 --- a/pkg/cmd/diff/diff_test.go +++ b/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) + } + } + }) + } +}