Skip to content

Commit

Permalink
Tracks Secrets in __inputs and lastAppliedConfig
Browse files Browse the repository at this point in the history
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

Fixes #734
  • Loading branch information
ellismg committed Aug 26, 2019
1 parent d6274b8 commit f1a4ce6
Show file tree
Hide file tree
Showing 9 changed files with 238 additions and 29 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

### Bug fixes

- Do not leak unencrypted secret values into the state file (fixes https://github.com/pulumi/pulumi-kubernetes/issues/734).

## 1.0.0-beta.2 (August 26,2019)

### Supported Kubernetes versions
Expand Down
135 changes: 108 additions & 27 deletions pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type kubeProvider struct {
opts kubeOpts
defaultNamespace string
enableDryRun bool
enableSecrets bool

clientSet *clients.DynamicClientSet
}
Expand Down Expand Up @@ -218,6 +219,7 @@ func (k *kubeProvider) Configure(_ context.Context, req *pulumirpc.ConfigureRequ
k.opts = kubeOpts{
rejectUnknownResources: vars["kubernetes:config:rejectUnknownResources"] == "true",
}
k.enableSecrets = req.GetAcceptSecrets()

//
// Configure client-go using provided or ambient kubeconfig file.
Expand Down Expand Up @@ -281,7 +283,9 @@ func (k *kubeProvider) Configure(_ context.Context, req *pulumirpc.ConfigureRequ
}
k.clientSet = cs

return &pulumirpc.ConfigureResponse{}, nil
return &pulumirpc.ConfigureResponse{
AcceptSecrets: true,
}, nil
}

// Invoke dynamically executes a built-in function in the provider.
Expand Down Expand Up @@ -344,7 +348,7 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
// an update.
oldResInputs := req.GetOlds()
olds, err := plugin.UnmarshalProperties(oldResInputs, plugin.MarshalOptions{
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand All @@ -355,7 +359,7 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
// an update.
newResInputs := req.GetNews()
news, err := plugin.UnmarshalProperties(newResInputs, plugin.MarshalOptions{
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -465,9 +469,14 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
}
}

autonamedInputs, err := plugin.MarshalProperties(
resource.NewPropertyMapFromMap(newInputs.Object), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.autonamedInputs", label), KeepUnknowns: true, SkipNulls: true,
checkedInputs := resource.NewPropertyMapFromMap(newInputs.Object);
annotateSecrets(checkedInputs, news);

autonamedInputs, err := plugin.MarshalProperties(checkedInputs, plugin.MarshalOptions{
Label: fmt.Sprintf("%s.autonamedInputs", label),
KeepUnknowns: true,
SkipNulls: true,
KeepSecrets: k.enableSecrets,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -503,7 +512,7 @@ func (k *kubeProvider) Diff(
// previous resource inputs supplied by the user, and `live` is the computed state of that inputs
// we received back from the API server.
oldState, err := plugin.UnmarshalProperties(req.GetOlds(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand All @@ -512,7 +521,7 @@ func (k *kubeProvider) Diff(

// Get new resource inputs. The user is submitting these as an update.
newResInputs, err := plugin.UnmarshalProperties(req.GetNews(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -688,11 +697,12 @@ func (k *kubeProvider) Create(

// Parse inputs
newResInputs, err := plugin.UnmarshalProperties(req.GetProperties(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.properties", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.properties", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
}

newInputs := propMapToUnstructured(newResInputs)

annotatedInputs, err := withLastAppliedConfig(newInputs)
Expand Down Expand Up @@ -739,8 +749,11 @@ func (k *kubeProvider) Create(
}

inputsAndComputed, err := plugin.MarshalProperties(
checkpointObject(newInputs, initialized), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.inputsAndComputed", label), KeepUnknowns: true, SkipNulls: true,
checkpointObject(newInputs, initialized, newResInputs), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.inputsAndComputed", label),
KeepUnknowns: true,
SkipNulls: true,
KeepSecrets: k.enableSecrets,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -795,7 +808,7 @@ func (k *kubeProvider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*p
// Obtain new properties, create a Kubernetes `unstructured.Unstructured` that we can pass to the
// validation routines.
oldState, err := plugin.UnmarshalProperties(req.GetProperties(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -882,16 +895,21 @@ func (k *kubeProvider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*p

// Return a new "checkpoint object".
state, err := plugin.MarshalProperties(
checkpointObject(liveInputs, liveObj), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.state", label), KeepUnknowns: true, SkipNulls: true,
checkpointObject(liveInputs, liveObj, oldState), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.state", label),
KeepUnknowns: true,
SkipNulls: true,
KeepSecrets: k.enableSecrets,
})
if err != nil {
return nil, err
}

inputs, err := plugin.MarshalProperties(
resource.NewPropertyMapFromMap(liveInputs.Object), plugin.MarshalOptions{
Label: label + ".inputs", KeepUnknowns: true, SkipNulls: true,
liveInputsPM := resource.NewPropertyMapFromMap(liveInputs.Object);
annotateSecrets(liveInputsPM, oldState)

inputs, err := plugin.MarshalProperties(liveInputsPM, plugin.MarshalOptions{
Label: label + ".inputs", KeepUnknowns: true, SkipNulls: true, KeepSecrets: k.enableSecrets,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -978,7 +996,7 @@ func (k *kubeProvider) Update(

// Obtain old properties, create a Kubernetes `unstructured.Unstructured`.
oldState, err := plugin.UnmarshalProperties(req.GetOlds(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand All @@ -988,7 +1006,7 @@ func (k *kubeProvider) Update(

// Obtain new properties, create a Kubernetes `unstructured.Unstructured`.
newResInputs, err := plugin.UnmarshalProperties(req.GetNews(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.news", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -1042,8 +1060,11 @@ func (k *kubeProvider) Update(

// Return a new "checkpoint object".
inputsAndComputed, err := plugin.MarshalProperties(
checkpointObject(newInputs, initialized), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.inputsAndComputed", label), KeepUnknowns: true, SkipNulls: true,
checkpointObject(newInputs, initialized, newResInputs), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.inputsAndComputed", label),
KeepUnknowns: true,
SkipNulls: true,
KeepSecrets: k.enableSecrets,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -1071,7 +1092,7 @@ func (k *kubeProvider) Delete(

// Obtain new properties, create a Kubernetes `unstructured.Unstructured`.
oldState, err := plugin.UnmarshalProperties(req.GetProperties(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true,
Label: fmt.Sprintf("%s.olds", label), KeepUnknowns: true, SkipNulls: true, KeepSecrets: true,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -1109,8 +1130,11 @@ func (k *kubeProvider) Delete(
lastKnownState := partialErr.Object()

inputsAndComputed, err := plugin.MarshalProperties(
checkpointObject(current, lastKnownState), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.inputsAndComputed", label), KeepUnknowns: true, SkipNulls: true,
checkpointObject(current, lastKnownState, oldState), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.inputsAndComputed", label),
KeepUnknowns: true,
SkipNulls: true,
KeepSecrets: k.enableSecrets,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -1255,10 +1279,19 @@ func (k *kubeProvider) inputPatch(
return jsonpatch.CreateMergePatch(oldInputsJSON, newInputsJSON)
}

func mapReplStripSecrets(v resource.PropertyValue) (interface{}, bool) {
if v.IsSecret() {
return v.SecretValue().Element.MapRepl(nil, mapReplStripSecrets), true
}

return nil, false;
}

func propMapToUnstructured(pm resource.PropertyMap) *unstructured.Unstructured {
return &unstructured.Unstructured{Object: pm.Mappable()}
return &unstructured.Unstructured{Object: pm.MapRepl(nil, mapReplStripSecrets)}
}


func withLastAppliedConfig(config *unstructured.Unstructured) (*unstructured.Unstructured, error) {
// Serialize the inputs and add the last-applied-configuration annotation.
marshaled, err := config.MarshalJSON()
Expand All @@ -1279,9 +1312,27 @@ func withLastAppliedConfig(config *unstructured.Unstructured) (*unstructured.Uns
return config, nil
}

func checkpointObject(inputs, live *unstructured.Unstructured) resource.PropertyMap {
func checkpointObject(inputs, live *unstructured.Unstructured, fromInputs resource.PropertyMap) resource.PropertyMap {
object := resource.NewPropertyMapFromMap(live.Object)
object["__inputs"] = resource.NewObjectProperty(resource.NewPropertyMapFromMap(inputs.Object))
inputsPM := resource.NewPropertyMapFromMap(inputs.Object)

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

if fromInputs.ContainsSecrets() {
if _, has := object["metadata"]; has && object["metadata"].IsObject() {
metadata := object["metadata"].ObjectValue()
if _, has := metadata["annotations"]; has && metadata["annotations"].IsObject() {
annotations := metadata["annotations"].ObjectValue();
if lastAppliedConfig, has := annotations[lastAppliedConfigKey]; has && !lastAppliedConfig.IsSecret() {
annotations[lastAppliedConfigKey] = resource.MakeSecret(lastAppliedConfig)
}
}
}
}

object["__inputs"] = resource.NewObjectProperty(inputsPM)

return object
}

Expand Down Expand Up @@ -1624,3 +1675,33 @@ func (pc *patchConverter) addPatchArrayToDiff(
}
return nil
}
// annotateSecrets copies the "secretness" from the ins to the outs. If there are values with the same keys for the
// outs and the ins, if they are both objects, they are transformed recursively. Otherwise, if the value in the ins
// contains a secret, the entire out value is marked as a secret. This is very close to how we project secrets
// in the programming model, with one small difference, which is how we treat the case where both are objects. In the
// programming model, we would say the entire output object is a secret. Here, we actually recur in. We do this because
// we don't want a single secret value in a rich structure to taint the entire object. Doing so would mean things like
// the entire value in the deployment would be encrypted instead of a small chunk. It also means the entire property
// would be displayed as `[secret]` in the CLI instead of a small part.
//
// NOTE: This means that for an array, if any value in the input version is a secret, the entire output array is
// marked as a secret. This is actually a very nice result, because often arrays are treated like sets by providers
// 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 {
return
}

for key, inValue := range ins {
outValue, has := outs[key]
if !has {
continue
}
if outValue.IsObject() && inValue.IsObject() {
annotateSecrets(outValue.ObjectValue(), inValue.ObjectValue())
} else if !outValue.IsSecret() && inValue.ContainsSecrets() {
outs[key] = resource.MakeSecret(outValue)
}
}
}
4 changes: 2 additions & 2 deletions pkg/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestCheckpointObject(t *testing.T) {
inputs := &unstructured.Unstructured{Object: objInputs}
live := &unstructured.Unstructured{Object: objLive}

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

oldInputs := obj["__inputs"]
Expand All @@ -81,7 +81,7 @@ func TestRoundtripCheckpointObject(t *testing.T) {
assert.Equal(t, objInputs, oldInputs.Object)
assert.Equal(t, objLive, oldLive.Object)

obj := checkpointObject(oldInputs, oldLive)
obj := checkpointObject(oldInputs, oldLive, nil)
assert.Equal(t, old, obj)

newInputs, newLive := parseCheckpointObject(obj)
Expand Down
Binary file added tests/integration/secrets/secrets.test
Binary file not shown.
55 changes: 55 additions & 0 deletions tests/integration/secrets/secrets_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2016-2019, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package ints

import (
b64 "encoding/base64"
json "encoding/json"
"os"
"testing"

"github.com/pulumi/pulumi/pkg/testing/integration"
"github.com/stretchr/testify/assert"
)

func TestSecrets(t *testing.T) {
kubectx := os.Getenv("KUBERNETES_CONTEXT")

if kubectx == "" {
t.Skipf("Skipping test due to missing KUBERNETES_CONTEXT variable")
}

secretMessage := "secret message for testing"

integration.ProgramTest(t, &integration.ProgramTestOptions{
Dir: "step1",
Dependencies: []string{"@pulumi/kubernetes"},
Quick: true,
Config: map[string]string{
"message": secretMessage,
},
ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
assert.NotNil(t, stackInfo.Deployment)
state, err := json.Marshal(stackInfo.Deployment)
assert.NoError(t, err)

assert.NotContains(t, string(state), secretMessage)

// The program converts the secret message to base64, to make a ConfigMap from it, so the state
// should also not contain the base64 encoding of secret message.
assert.NotContains(t, string(state), b64.StdEncoding.EncodeToString([]byte(secretMessage)))
},
})
}
3 changes: 3 additions & 0 deletions tests/integration/secrets/step1/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: provider
description: Tests first-class provider support.
runtime: nodejs
Loading

0 comments on commit f1a4ce6

Please sign in to comment.