Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for __inputs secrets #2300 #2301

Merged
merged 7 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
**/obj/
**/node_modules/
**/.vs
**/.vscode
**/.idea
**/.ionide
.pulumi
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## Unreleased

## 3.24.0 (February 6, 2023)

- Fix unencrypted secrets in the state `outputs` after `Secret.get` #2300
- Upgrade to latest helm and k8s client dependencies (https://github.com/pulumi/pulumi-kubernetes/pulls/2292)

## 3.23.1 (December 19, 2022)
Expand Down
24 changes: 21 additions & 3 deletions provider/pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ const (
lastAppliedConfigKey = "kubectl.kubernetes.io/last-applied-configuration"
initialAPIVersionKey = "__initialApiVersion"
fieldManagerKey = "__fieldManager"
secretKind = "Secret"
)

type cancellationContext struct {
Expand Down Expand Up @@ -2931,16 +2932,19 @@ func initialAPIVersion(state resource.PropertyMap, oldConfig *unstructured.Unstr

func checkpointObject(inputs, live *unstructured.Unstructured, fromInputs resource.PropertyMap,
initialAPIVersion, fieldManager string) resource.PropertyMap {

object := resource.NewPropertyMapFromMap(live.Object)
inputsPM := resource.NewPropertyMapFromMap(inputs.Object)

annotateSecrets(object, fromInputs)
annotateSecrets(inputsPM, fromInputs)

isSecretKind := live.GetKind() == secretKind

// For secrets, if `stringData` is present in the inputs, the API server will have filled in `data` based on it. By
// base64 encoding the secrets. We should mark any of the values which were secrets in the `stringData` object
// as secrets in the `data` field as well.
if live.GetAPIVersion() == "v1" && live.GetKind() == "Secret" {
if live.GetAPIVersion() == "v1" && isSecretKind {
stringData, hasStringData := fromInputs["stringData"]
data, hasData := object["data"]

Expand All @@ -2958,7 +2962,7 @@ func checkpointObject(inputs, live *unstructured.Unstructured, fromInputs resour
// Ensure that the annotation we add for lastAppliedConfig is treated as a secret if any of the inputs were secret
// (the value of this annotation is a string-ified JSON so marking the entire thing as a secret is really the best
// that we can do).
if fromInputs.ContainsSecrets() {
if fromInputs.ContainsSecrets() || isSecretKind {
if _, has := object["metadata"]; has && object["metadata"].IsObject() {
metadata := object["metadata"].ObjectValue()
if _, has := metadata["annotations"]; has && metadata["annotations"].IsObject() {
Expand Down Expand Up @@ -3340,7 +3344,21 @@ func (pc *patchConverter) addPatchArrayToDiff(
// and the order may not be preserved across an operation. This means we do end up encrypting the entire array
// but that's better than accidentally leaking a value which just moved to a different location.
func annotateSecrets(outs, ins resource.PropertyMap) {
if outs == nil || ins == nil {
if outs == nil {
return
}

if kind, ok := outs["kind"]; ok && kind.StringValue() == secretKind {
if data, hasData := outs["data"]; hasData {
outs["data"] = resource.MakeSecret(data)
}
thomas11 marked this conversation as resolved.
Show resolved Hide resolved
if stringData, hasStringData := outs["stringData"]; hasStringData {
outs["stringData"] = resource.MakeSecret(stringData)
}
return
}

if ins == nil {
return
}

Expand Down
33 changes: 33 additions & 0 deletions provider/pkg/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,39 @@ func TestCheckpointObject(t *testing.T) {
assert.Equal(t, objLive, obj.Mappable())
}

// #2300 - Read() on top-level k8s objects of kind "secret" led to unencrypted __input
func TestCheckpointSecretObject(t *testing.T) {
objInputSecret := map[string]interface{}{
"kind": "Secret",
"data": map[string]interface{}{
"password": "verysecret",
},
}
objSecretLive := map[string]interface{}{
initialAPIVersionKey: "",
fieldManagerKey: "",
"kind": "Secret",
"data": map[string]interface{}{
"password": "verysecret",
},
}

// Questionable but correct pinning test as of the time of writing
assert.False(t, resource.NewPropertyMapFromMap(objInputSecret).ContainsSecrets())
assert.False(t, resource.NewPropertyMapFromMap(objSecretLive).ContainsSecrets())

inputs := &unstructured.Unstructured{Object: objInputSecret}
live := &unstructured.Unstructured{Object: objSecretLive}

obj := checkpointObject(inputs, live, nil, "", "")
assert.NotNil(t, obj)

oldInputs := obj["__inputs"]
assert.True(t, oldInputs.IsObject())
oldInputsVal := oldInputs.ObjectValue()
assert.True(t, oldInputsVal["data"].ContainsSecrets())
}

func TestRoundtripCheckpointObject(t *testing.T) {
old := resource.NewPropertyMapFromMap(objLive)
old["__inputs"] = resource.NewObjectProperty(resource.NewPropertyMapFromMap(objInputs))
Expand Down