Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history 
map[] in error output exposes secret data in last-applied-annotation
& patch error

Invalid secrets with stringData exposes the secret values in diff. Attempt a
normalization to prevent it.

Refactor stringData to data conversion to eliminate code duplication

Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
  • Loading branch information
@svghadi
svghadi authored Jan 29, 2025
1 parent bd7681a commit 4c6e03c
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 9 deletions.
36 changes: 27 additions & 9 deletions pkg/diff/diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package diff
import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -864,6 +865,32 @@ func NormalizeSecret(un *unstructured.Unstructured, opts ...Option) {
if gvk.Group != "" || gvk.Kind != "Secret" {
return
}

// move stringData to data section
if stringData, found, err := unstructured.NestedMap(un.Object, "stringData"); found && err == nil {
var data map[string]interface{}
data, found, _ = unstructured.NestedMap(un.Object, "data")
if !found {
data = make(map[string]interface{})
}

// base64 encode string values and add non-string values as is.
// This ensures that the apply fails if the secret is invalid.
for k, v := range stringData {
strVal, ok := v.(string)
if ok {
data[k] = base64.StdEncoding.EncodeToString([]byte(strVal))
} else {
data[k] = v
}
}

err := unstructured.SetNestedField(un.Object, data, "data")
if err == nil {
delete(un.Object, "stringData")
}
}

o := applyOptions(opts)
var secret corev1.Secret
err := runtime.DefaultUnstructuredConverter.FromUnstructured(un.Object, &secret)
Expand All @@ -877,15 +904,6 @@ func NormalizeSecret(un *unstructured.Unstructured, opts ...Option) {
secret.Data[k] = []byte("")
}
}
if len(secret.StringData) > 0 {
if secret.Data == nil {
secret.Data = make(map[string][]byte)
}
for k, v := range secret.StringData {
secret.Data[k] = []byte(v)
}
delete(un.Object, "stringData")
}
newObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&secret)
if err != nil {
o.log.Error(err, "object unable to convert from secret")
Expand Down
120 changes: 120 additions & 0 deletions pkg/diff/diff_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,126 @@ func TestHideSecretDataDifferentKeysDifferentValues(t *testing.T) {
assert.Equal(t, map[string]interface{}{"key2": replacement2, "key3": replacement1}, secretData(live))
}

func TestHideStringDataInInvalidSecret(t *testing.T) {
liveUn := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
"kind": "Secret",
"metadata": map[string]interface{}{
"name": "test-secret",
},
"type": "Opaque",
"data": map[string]interface{}{
"key1": "a2V5MQ==",
"key2": "a2V5MQ==",
},
},
}
targetUn := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
"kind": "Secret",
"metadata": map[string]interface{}{
"name": "test-secret",
},
"type": "Opaque",
"data": map[string]interface{}{
"key1": "a2V5MQ==",
"key2": "a2V5Mg==",
"key3": false,
},
"stringData": map[string]interface{}{
"key4": "key4",
"key5": 5,
},
},
}

liveUn = remarshal(liveUn, applyOptions(diffOptionsForTest()))
targetUn = remarshal(targetUn, applyOptions(diffOptionsForTest()))

target, live, err := HideSecretData(targetUn, liveUn)
require.NoError(t, err)

assert.Equal(t, map[string]interface{}{"key1": replacement1, "key2": replacement2}, secretData(live))
assert.Equal(t, map[string]interface{}{"key1": replacement1, "key2": replacement1, "key3": replacement1, "key4": replacement1, "key5": replacement1}, secretData(target))
}

// stringData in secrets should be normalized even if it is invalid
func TestNormalizeSecret(t *testing.T) {
var tests = []struct {
testname string
data map[string]interface{}
stringData map[string]interface{}
}{
{
testname: "Valid secret",
data: map[string]interface{}{
"key1": "key1",
},
stringData: map[string]interface{}{
"key2": "a2V5Mg==",
},
},
{
testname: "Invalid secret",
data: map[string]interface{}{
"key1": "key1",
"key2": 2,
},
stringData: map[string]interface{}{
"key3": "key3",
"key4": nil,
},
},
{
testname: "Invalid secret with stringData only",
data: nil,
stringData: map[string]interface{}{
"key3": "key3",
"key4": true,
},
},
}

for _, tt := range tests {
t.Run(tt.testname, func(t *testing.T) {
un := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
"kind": "Secret",
"metadata": map[string]interface{}{
"name": "test-secret",
},
"type": "Opaque",
"data": tt.data,
"stringData": tt.stringData,
},
}
un = remarshal(un, applyOptions(diffOptionsForTest()))

NormalizeSecret(un)

_, found, _ := unstructured.NestedMap(un.Object, "stringData")
assert.False(t, found)

data, found, _ := unstructured.NestedMap(un.Object, "data")
assert.True(t, found)

// check all secret keys are found under data in normalized secret
for _, obj := range []map[string]interface{}{tt.data, tt.stringData} {
if obj == nil {
continue
}
for k := range obj {
_, ok := data[k]
assert.True(t, ok)
}
}
})
}
}

func getTargetSecretJsonBytes() []byte {
return []byte(`
{
Expand Down
3 changes: 3 additions & 0 deletions pkg/utils/kube/kube.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,15 @@ var (
// See ApplyOpts::Run()
// cmdutil.AddSourceToErr(fmt.Sprintf("applying patch:\n%s\nto:\n%v\nfor:", patchBytes, info), info.Source, err)
kubectlApplyPatchErrOutRegexp = regexp.MustCompile(`(?s)^error when applying patch:.*\nfor: "\S+": `)

kubectlErrOutMapRegexp = regexp.MustCompile(`map\[.*\]`)
)

// cleanKubectlOutput makes the error output of kubectl a little better to read
func cleanKubectlOutput(s string) string {
s = strings.TrimSpace(s)
s = kubectlErrOutRegexp.ReplaceAllString(s, "")
s = kubectlErrOutMapRegexp.ReplaceAllString(s, "")
s = kubectlApplyPatchErrOutRegexp.ReplaceAllString(s, "")
s = strings.Replace(s, "; if you choose to ignore these errors, turn validation off with --validate=false", "", -1)
return s
Expand Down
12 changes: 12 additions & 0 deletions pkg/utils/kube/kube_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,18 @@ Object: &{map["apiVersion":"v1" "kind":"Service" "metadata":map["annotations":ma
for: "/var/folders/_m/991sn1ds7g39lnbhp6wvqp9d_j5476/T/224503547": Service "my-service" is invalid: spec.clusterIP: Invalid value: "10.96.0.44": field is immutable`
assert.Equal(t, cleanKubectlOutput(s), `Service "my-service" is invalid: spec.clusterIP: Invalid value: "10.96.0.44": field is immutable`)
}
{
s := `error when patching "/var/folders/mj/c96jcs7j2cq7xcjfhqjq3m2w0000gn/T/745145319": "" is invalid: patch: Invalid value: "map[data:map[email:aaaaa password:<nil> username:<nil>] metadata:map[annotations:map[kubectl.kubernetes.io/last-applied-configuration:{\"apiVersion\":\"v1\",\"data\":{\"email\":\"aaaaa\"},\"kind\":\"Secret\",\"metadata\":{\"annotations\":{},\"labels\":{\"app.kubernetes.io/instance\":\"test\"},\"name\":\"my-secret\",\"namespace\":\"default\"},\"stringData\":{\"id\":1,\"password\":0,\"username\":\"username\"},\"type\":\"Opaque\"}\n]] stringData:map[id:1 password:0 username:username]]": error decoding from json: illegal base64 data at input byte 4`
assert.Equal(t, cleanKubectlOutput(s), `error when patching "/var/folders/mj/c96jcs7j2cq7xcjfhqjq3m2w0000gn/T/745145319": "" is invalid: patch: Invalid value: "": error decoding from json: illegal base64 data at input byte 4`)
}
{
s := `error when patching "/var/folders/mj/c96jcs7j2cq7xcjfhqjq3m2w0000gn/T/2250018703": "" is invalid: patch: Invalid value: "map[data:<nil> metadata:map[annotations:map[kubectl.kubernetes.io/last-applied-configuration:{\"apiVersion\":\"v1\",\"kind\":\"Secret\",\"metadata\":{\"annotations\":{},\"labels\":{\"app.kubernetes.io/instance\":\"test\"},\"name\":\"my-secret\",\"namespace\":\"default\"},\"stringData\":{\"id\":1,\"password\":0,\"username\":\"username\"},\"type\":\"Opaque\"}\n]]]": cannot convert int64 to string`
assert.Equal(t, cleanKubectlOutput(s), `error when patching "/var/folders/mj/c96jcs7j2cq7xcjfhqjq3m2w0000gn/T/2250018703": "" is invalid: patch: Invalid value: "": cannot convert int64 to string`)
}
{
s := `Secret in version "v1" cannot be handled as a Secret: json: cannot unmarshal bool into Go struct field Secret.data of type []uint8`
assert.Equal(t, cleanKubectlOutput(s), `Secret in version "v1" cannot be handled as a Secret: json: cannot unmarshal bool into Go struct field Secret.data of type []uint8`)
}
}

func TestInClusterKubeConfig(t *testing.T) {
Expand Down

0 comments on commit 4c6e03c

Please sign in to comment.