Skip to content

Commit

Permalink
Strip secret values from Configure
Browse files Browse the repository at this point in the history
  • Loading branch information
iwahbe committed Sep 24, 2024
1 parent 22452e6 commit 4acb843
Show file tree
Hide file tree
Showing 5 changed files with 260 additions and 9 deletions.
1 change: 1 addition & 0 deletions pf/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/pulumi/pulumi-terraform-bridge/v3 v3.91.0
github.com/pulumi/pulumi-terraform-bridge/x/muxer v0.0.8
github.com/stretchr/testify v1.9.0
pgregory.net/rapid v0.6.1
)

require (
Expand Down
37 changes: 37 additions & 0 deletions pf/internal/plugin/provider_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package plugin
import (
"context"

"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
Expand Down Expand Up @@ -80,5 +81,41 @@ func (p providerThunk) Configure(
if ctx.Value(setupConfigureKey) != nil {
return plugin.ConfigureResponse{}, nil
}
req.Inputs = removeSecrets(req.Inputs)
contract.Assertf(!req.Inputs.ContainsSecrets(),
"Inputs to configure should not contain secrets")
return p.GrpcProvider.Configure(ctx, req)
}

func removeSecrets(pMap resource.PropertyMap) resource.PropertyMap {
var remove func(resource.PropertyValue) resource.PropertyValue
remove = func(v resource.PropertyValue) resource.PropertyValue {
switch {
case v.IsArray():
arr := make([]resource.PropertyValue, 0, len(v.ArrayValue()))
for _, v := range v.ArrayValue() {
arr = append(arr, remove(v))
}
return resource.NewProperty(arr)
case v.IsObject():
obj := make(resource.PropertyMap, len(v.ObjectValue()))
for k, v := range v.ObjectValue() {
obj[k] = remove(v)
}
return resource.NewProperty(obj)
case v.IsComputed():
return resource.MakeComputed(remove(v.Input().Element))
case v.IsOutput():
o := v.OutputValue()
o.Secret = false
o.Element = remove(o.Element)
return resource.NewProperty(o)
case v.IsSecret():
return remove(v.SecretValue().Element)
default:
return v
}
}

return remove(resource.NewProperty(pMap)).ObjectValue()
}
110 changes: 110 additions & 0 deletions pf/internal/plugin/provider_server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright 2016-2024, 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 plugin

import (
"testing"

"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
prapid "github.com/pulumi/pulumi/sdk/v3/go/property/testing"
"github.com/stretchr/testify/assert"
"pgregory.net/rapid"
)

// TestRemoveSecrets checks that removeSecrets removes [resource.Secret] values and unsets
// [resource.Output.Secret] fields without making any other changes.
func TestRemoveSecrets(t *testing.T) {

// These functions validate that a diff does not contain any non-secret changes.
var (
validateObjectDiff func(assert.TestingT, resource.ObjectDiff)
validateArrayDiff func(assert.TestingT, resource.ArrayDiff)
validateValueDiff func(assert.TestingT, resource.ValueDiff)
)

validateValueDiff = func(t assert.TestingT, v resource.ValueDiff) {
switch {
case v.Old.IsOutput():
oOld := v.Old.OutputValue()
oOld.Secret = false
if d := resource.NewProperty(oOld).DiffIncludeUnknowns(v.New); d != nil {
validateValueDiff(t, *d)
}
case v.Old.IsSecret():
if d := v.Old.SecretValue().Element.DiffIncludeUnknowns(v.New); d != nil {
validateValueDiff(t, *d)
}
case v.Old.IsObject():
validateObjectDiff(t, *v.Object)
case v.Old.IsArray():
validateArrayDiff(t, *v.Array)
default:
assert.Failf(t, "", "unexpected Update.Old type %q", v.Old.TypeString())
}
}

validateArrayDiff = func(t assert.TestingT, diff resource.ArrayDiff) {
assert.Empty(t, diff.Adds)
assert.Empty(t, diff.Deletes)

for _, v := range diff.Updates {
validateValueDiff(t, v)
}
}

validateObjectDiff = func(t assert.TestingT, diff resource.ObjectDiff) {
assert.Empty(t, diff.Adds)

// Diff does not distinguish from a missing key and a null property, so
// when we go from a map{k: secret(null)} to a map{k: null}, the diff
// machinery shows a delete.
//
// We have an explicit test for this behavior.
for _, v := range diff.Deletes {
assert.Equal(t, resource.MakeSecret(resource.NewNullProperty()), v)
}

for _, v := range diff.Updates {
validateValueDiff(t, v)
}
}

t.Run("rapid", rapid.MakeCheck(func(t *rapid.T) {
m := resource.ToResourcePropertyValue(prapid.Map(5).Draw(t, "top-level")).ObjectValue()
if m.ContainsSecrets() {
unsecreted := removeSecrets(m)
assert.False(t, unsecreted.ContainsSecrets())

// We need to assert that the only change between m and unsecreted
// is that secret values went to their element values.
if d := m.DiffIncludeUnknowns(unsecreted); d != nil {
validateObjectDiff(t, *d)
}
} else {
assert.Equal(t, m, removeSecrets(m))
}
}))

t.Run("map-null-secrets", func(t *testing.T) {
assert.Equal(t,
resource.PropertyMap{
"null": resource.NewNullProperty(),
},
removeSecrets(resource.PropertyMap{
"null": resource.MakeSecret(resource.NewNullProperty()),
}),
)
})
}
87 changes: 87 additions & 0 deletions pf/tests/provider_configure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,90 @@ func TestJSONNestedConfigure(t *testing.T) {
}
}`)
}

func TestJSONNestedConfigureWithSecrets(t *testing.T) {
server, err := newProviderServer(t, testprovider.SyntheticTestBridgeProvider())
require.NoError(t, err)
replay.ReplaySequence(t, server, `
[
{
"method": "/pulumirpc.ResourceProvider/Configure",
"request": {
"args": {
"stringConfigProp": "{\"4dabf18193072939515e22adb298388d\":\"1b47061264138c4ac30d75fd1eb44270\",\"value\":\"secret-example\"}",
"mapNestedProp": "{\"k1\":{\"4dabf18193072939515e22adb298388d\":\"1b47061264138c4ac30d75fd1eb44270\",\"value\":1},\"k2\":2}",
"listNestedProps": "[{\"4dabf18193072939515e22adb298388d\":\"1b47061264138c4ac30d75fd1eb44270\",\"value\":true},false]"
}
},
"response": {
"supportsPreview": true,
"acceptResources": true
}
},
{
"method": "/pulumirpc.ResourceProvider/Create",
"request": {
"urn": "urn:pulumi:test-stack::basicprogram::testbridge:index/testres:TestConfigRes::r1",
"preview": false
},
"response": {
"id": "id-1",
"properties": {
"configCopy": "secret-example",
"id": "id-1"
}
}
}
]`)
}

func TestConfigureWithSecrets(t *testing.T) {
server, err := newProviderServer(t, testprovider.SyntheticTestBridgeProvider())
require.NoError(t, err)
replay.ReplaySequence(t, server, `
[
{
"method": "/pulumirpc.ResourceProvider/Configure",
"request": {
"args": {
"stringConfigProp": {
"4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
"value": "secret-example"
},
"mapNestedProp": {
"k1": {
"4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
"value": 1
},
"k2": 2
},
"listNestedProps": [
{
"4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
"value": true
},
false
]
}
},
"response": {
"supportsPreview": true,
"acceptResources": true
}
},
{
"method": "/pulumirpc.ResourceProvider/Create",
"request": {
"urn": "urn:pulumi:test-stack::basicprogram::testbridge:index/testres:TestConfigRes::r1",
"preview": false
},
"response": {
"id": "id-1",
"properties": {
"configCopy": "secret-example",
"id": "id-1"
}
}
}
]`)
}
34 changes: 25 additions & 9 deletions pkg/tfbridge/config_encoding.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2016-2023, Pulumi Corporation.
// Copyright 2016-2024, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -68,19 +68,33 @@ func (*ConfigEncoding) tryUnwrapSecret(encoded any) (any, bool) {
}

func (enc *ConfigEncoding) convertStringToPropertyValue(s string, typ shim.ValueType) (resource.PropertyValue, error) {
// If the schema expects a string, we can just return this as-is.
if typ == shim.TypeString {
return resource.NewStringProperty(s), nil
}

// Otherwise, we will attempt to deserialize the input string as JSON and convert the result into a Pulumi
// We attempt to deserialize the input string as JSON and convert the result into a Pulumi
// property. If the input string is empty, we will return an appropriate zero value.
//
// We need to attempt JSON de-serialization for all incoming strings, even if the target type is also
// a string. This allows us to correctly handle secret or computed string values. For example, the
// provider might be sent an input like this:
//
// resource.PropertyMap{
// "k": resource.NewProperty(`{"4dabf18193072939515e22adb298388d":"1b47061264138c4ac30d75fd1eb44270","value":"secret-value"}`),
// }
//
// Even if `k` is a property of type string and it's value is a string, it should be decoded as:
//
// resource.PropertyMap{
// "k": resource.MakeSecret(resource.NewProperty("secret-value")),
// }
//
//nolint:lll
if s == "" {
return enc.zeroValue(typ), nil
}

var jsonValue interface{}
if err := json.Unmarshal([]byte(s), &jsonValue); err != nil {
if typ == shim.TypeString {
return resource.NewProperty(s), nil
}
return resource.PropertyValue{}, err
}

Expand All @@ -102,10 +116,12 @@ func (enc *ConfigEncoding) convertStringToPropertyValue(s string, typ shim.Value

func (*ConfigEncoding) zeroValue(typ shim.ValueType) resource.PropertyValue {
switch typ {
case shim.TypeString:
return resource.NewProperty("")
case shim.TypeBool:
return resource.NewPropertyValue(false)
return resource.NewProperty(false)
case shim.TypeInt, shim.TypeFloat:
return resource.NewPropertyValue(0)
return resource.NewProperty[float64](0)
case shim.TypeList, shim.TypeSet:
return resource.NewPropertyValue([]interface{}{})
default:
Expand Down

0 comments on commit 4acb843

Please sign in to comment.