Tracks Secrets in `__inputs` and `lastAppliedConfig` #741

ellismg · 2019-08-23T17:51:50Z

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:

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.
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

ellismg · 2019-08-23T22:21:52Z

@pgavlin @lblackstone Worthwhile I think to get your eyes on this now, as I continue my validation. At a high level, this does the following:

Opts the provider into accepting RPCs where the payloads from the engine have secrets annotated with correct metadata.
Follows the same strategy of marking each "output" value as a secret if the same "input" value was a secret.
Does the same sort of marking for the data on the __inputs field of the state.
Marks the last applied config annotation as a secret if any of the inputs are secret (right now this is just a stringified JSON object that could contain secret data, because it copies in the all the inputs).

@lblackstone Interested in understanding if there are other places where we logically duplicate inputs somewhere else in the state.

We drop the secretness when we convert these PropertyMaps to Unstructure'd, so the core of the provider doesn't change much.

ellismg · 2019-08-23T22:22:22Z

@lblackstone thinks that the travis failure here is due to a race since the helm chart does not namespace the config maps. I'll be adding some directed testing before merging.

lblackstone · 2019-08-23T23:17:40Z

Looks like you're on the right track, but I'm getting an error trying to add a secret to a ConfigMap:

import * as k8s from "@pulumi/kubernetes";

const pw = pulumi.secret("supers3kr3t");
const cm = new k8s.core.v1.ConfigMap("cm", {binaryData: {password: pw}});

ly failed: resource cm-l8kxa7qn/%!s(MISSING) was not successfully created by the Kubernetes API server : ConfigMap in version "v1" cannot be handled as a ConfigMap: v1.ConfigMap.ObjectMeta: v1.ObjectMeta.TypeMeta: Kind: BinaryData: decode base64: illegal base64 data at input byte 8, error found in #10 byte of ...|pers3kr3t"},"kind":"|..., bigger context ...|rsion":"v1","binaryData":{"password":"supers3kr3t"},"kind":"ConfigMap","metadata":{"annotations":{"k|...

import * as k8s from "@pulumi/kubernetes";

const pw = pulumi.secret("supers3kr3t");
const cm = new k8s.core.v1.ConfigMap("cm", {data: {password: pw}});

works because k8s base64 encodes it server-side when using data vs binaryData.

Here's what the ConfigMap looks like in k8s:

apiVersion: v1
data:
  password: supers3kr3t
kind: ConfigMap
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"v1","data":{"password":"supers3kr3t"},"kind":"ConfigMap","metadata":{"annotations":{"pulumi.com/autonamed":"true"},"labels":{"app.kubernetes.io/managed-by":"pulumi"},"name":"cm-wuhxd54q"}}
    pulumi.com/autonamed: "true"
  creationTimestamp: "2019-08-23T23:18:20Z"
  labels:
    app.kubernetes.io/managed-by: pulumi
  name: cm-wuhxd54q
  namespace: default
  resourceVersion: "813035"
  selfLink: /api/v1/namespaces/default/configmaps/cm-wuhxd54q
  uid: 4ba3bb71-c5fc-11e9-b2bd-025000000001

and here's what the ConfigMap looks like in the Pulumi SaaS Console:

{
  "__inputs": {
    "apiVersion": "v1",
    "data": {
      "password": {
        "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
        "ciphertext": "AAABANZuLjRjIT4Ky6L9ETGKR1JG7G3zp/aH1QZFZqXuWfH2KSN8cXtKaWRK"
      }
    },
    "kind": "ConfigMap",
    "metadata": {
      "annotations": {
        "pulumi.com/autonamed": "true"
      },
      "labels": {
        "app.kubernetes.io/managed-by": "pulumi"
      },
      "name": "cm-wuhxd54q"
    }
  },
  "apiVersion": "v1",
  "data": {
    "password": {
      "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
      "ciphertext": "AAABAJDrt163iRrnQoG1MPQLb4Coyj22h9QDqEGoheIP+DiQULKpPq+dEPE2"
    }
  },
  "kind": "ConfigMap",
  "metadata": {
    "annotations": {
      "kubectl.kubernetes.io/last-applied-configuration": {
        "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
        "ciphertext": "AAABAAqYN+3FS9t51ROk1sz18AEP07eJIv3XjYZQ124TU67AzIw0wIHlxfN2fAuV8QOneh86J/mrw0Zylpj+0O8xy0Qlrk3CV9pGl8WvZH/7g3C5vLNvnwb0kawk3nMeWqjaPltve9rkk6jcnWFpNuZBVN+cxa1cYqeZU4z5uf+w++3le7baoBYTtmzfNn+a9PWd/BlYJ0J6JlCUaY1PS+xTyc6k6wh3fTyubUXjxFGMLvXNvYuCTgS3QzMZ5znIcf3kACvD1C/s/0OHh5pH1QcLuogtLgWQVkV88UpeGGfFVKyQ77WIcRPr+yShqKXBEJPcE/ErNW+97UjKPt1kfJRVEdcf++ta39PhyLqttw=="
      },
      "pulumi.com/autonamed": "true"
    },
    "creationTimestamp": "2019-08-23T23:18:20Z",
    "labels": {
      "app.kubernetes.io/managed-by": "pulumi"
    },
    "name": "cm-wuhxd54q",
    "namespace": "default",
    "resourceVersion": "813035",
    "selfLink": "/api/v1/namespaces/default/configmaps/cm-wuhxd54q",
    "uid": "4ba3bb71-c5fc-11e9-b2bd-025000000001"
  }
}

lblackstone · 2019-08-23T23:49:48Z

Actually, that may be correct behavior. I think you have to base64 encode the data yourself if you use the binaryData parameter. Only possible concern there would be leaking the secret through the error message.

lukehoban · 2019-08-25T22:07:27Z

Actually, that may be correct behavior.

To first approximation yes - I think that is correct behaviour. In general, it is possible for providers which have these values in plain text to leak them through error messages via errors coming from the cloud provider. That is generally tied to places where the cloud provider does not properly treat these as secret (which Kubernetes does not in this case).

To solve this, we likely need to expand our masking technology in the CLI layer from masking on the values of secret config to being able to mask the values of any secrets that flow through the program. I've opened pulumi/pulumi#3138 to track that.

ellismg · 2019-08-26T22:25:25Z

Okay, @lukehoban @pgavlin @lblackstone this should now be ready for review, I wrote a small test that ensures the state file does not contain any plaintext copies of a secret, vs just checking the things we cared about being marked as secrets so if we start copying stuff to other places we'll see a test failure.

lblackstone

Change LGTM. Should add one more test for completeness.

lblackstone · 2019-08-26T22:42:24Z

tests/integration/secrets/step1/index.ts

+
+const pw = (new pulumi.Config()).requireSecret("message");
+const cm = new k8s.core.v1.ConfigMap("cm", {
+    binaryData: {


Could you also test with a ConfigMap like this?

const cm = new k8s.core.v1.ConfigMap("cm", { data: { password: pw, } })

k8s will base64 encode that input server-side, and it will be accessible at the same place (cm.binaryData).

Actually, @lblackstone I have a question about this. I'm not observing the behavior you describe. For me, if I create a ConfigMap passing a secret in as just "data", the binaryData property of the object is never filled in (I've confirmed this by looking at both pulum's state file via stack export as well as the underlying object with kubectl get cm <name> -o json. In both cases, the object just has a data property. Also note that calling pulumi refresh does not cause this to appear (sensible, given the above).

Assuming that the API server behaved as you said it would - this would actually expose a problem with this. While binaryData is logically a transformed value of data and hence should be kept as a secret, it's not clear to me what part of the system would make that decision and how. I think it would have to be logic specific to ConfigMap itself, and I think that would require deeper surgery than what I have here (since lower level portions of the code that actually create resources would need to be able to observe secretness, right now everything is stripped off when we create the Unstructured objects.

So I can add the test to ensure that data is handled, but I don't think we will see the behavior you expect WRT server side encoding of data?

Oh, whoops. I think I was mixing up Secret and ConfigMap. Let me check.

Ok, let's add the following cases:

const stringSecret = new k8s.core.v1.Secret("stringSecret", {stringData: {password: pw}}); const dataSecret = new k8s.core.v1.Secret("dataSecret", {data: {password: pw.apply(d => new Buffer(d).toString("base64"))}}) export const secretStringData = stringSecret.data; export const secretData = dataSecret.data;

(The .data field will be identical in these cases)

@lblackstone - So, as I expected, this does lead to an issue. Specifically, for the stringSecret resource, the data property is not masked. Here's the relevant part of the state file for the object:

{ "urn": "urn:pulumi:dev-2::provider::kubernetes:core/v1:Secret::stringsecret", "custom": true, "id": "default/stringsecret-2nsgegwh", "type": "kubernetes:core/v1:Secret", "inputs": { "apiVersion": "v1", "kind": "Secret", "metadata": { "annotations": { "pulumi.com/autonamed": "true" }, "labels": { "app.kubernetes.io/managed-by": "pulumi" }, "name": "stringsecret-2nsgegwh" }, "stringData": { "password": { "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270", "ciphertext": "AAABAA/YOyfH4Zu9xLkYJKJ/pzJhUqZHf9wxjiVdPZB36SzbqpReK7t0dCUQx+kzzrY=" } } }, "outputs": { "__inputs": { "apiVersion": "v1", "kind": "Secret", "metadata": { "annotations": { "pulumi.com/autonamed": "true" }, "labels": { "app.kubernetes.io/managed-by": "pulumi" }, "name": "stringsecret-2nsgegwh" }, "stringData": { "password": { "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270", "ciphertext": "AAABAO8ySfCDQbi+a52zXIQo9ptvx7RBeKU9anECcdlhihVkzsqI4ERi6CIyiDS47XA=" } } }, "apiVersion": "v1", "data": { "password": "YSBzZWNyZXQgbWVzc2FnZQ==" }, "kind": "Secret", "metadata": { "annotations": { "kubectl.kubernetes.io/last-applied-configuration": { "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270", "ciphertext": "AAABAHUOURpgdiT5dBi9g9Bdi4csXrxVjwMdU0x9XCCnYK7Jimq/2QzEZw+vwTfRqd++GqwU++OmlLqwjhb0AkvWQ26L0MIR7NfFCi57AQLVJ+/6kHSum1ZLsHHAb7gBPWYHvCuFZY1siWq0H/VlglY7taYC7Kg1LlRwBJgDJbEJC3n9Tm5+CQEN9lj5+u94Xq2cCKu3OebyRnStSOwUsyau4l5KDDfJLuOYz/53REhH+Olr2DlduVXTfz4JBNAcFYQezyC1Bvi+lbuTkJg2nm9A9LKzAfDo1Slpn8s4hu+X8kMa7bCQS2fzvxzlgwaV7DRfHexsP0kJhYjCmNtP0QDja/yWc7WXAre+GQXzRWgLGVvLdq7dO9zna+03NNGdFw==" }, "pulumi.com/autonamed": "true" }, "creationTimestamp": "2019-08-26T23:57:22Z", "labels": { "app.kubernetes.io/managed-by": "pulumi" }, "name": "stringsecret-2nsgegwh", "namespace": "default", "resourceVersion": "67529893", "selfLink": "/api/v1/namespaces/default/secrets/stringsecret-2nsgegwh", "uid": "3f06a660-c85d-11e9-9301-42010a8a0098" }, "type": "Opaque" }, "parent": "urn:pulumi:dev-2::provider::pulumi:pulumi:Stack::provider-dev-2", "provider": "urn:pulumi:dev-2::provider::pulumi:providers:kubernetes::default_1_0_1_alpha_1566858130_g25ffc20c_dirty::1b3d4bf7-1b7e-4d7b-9ff3-6d15e3ecf88a", "propertyDependencies": { "apiVersion": null, "kind": null, "stringData": null } },

Here we can see that the data property just has the b64 encoded secret value from the API server.

I'm not sure what we can do here. It feels like we would need the secret resource to understand that stringData is present and secret than data itself gets marked as a secret, but I think that will require much more work to support.

Are there other resources where the API server does something like this?

Indeed, just for posterity, the following program "leaks" the pulumi.secret at the Kubernetes Secret's field .data["foo"] into the Deployment:

const s = pulumi.secret("bar"); const ss = new k8s.core.v1.Secret("bar", { stringData: { foo: s } }); // nginx container, replicated 1 time. const appName = "nginx"; const appLabels = { app: appName }; const nginx = new k8s.apps.v1.Deployment(appName, { metadata: { name: "foo" }, spec: { selector: { matchLabels: appLabels }, replicas: 1, template: { metadata: { labels: appLabels, annotations: { pwd: ss.data.apply(data => (<any>data)["foo"]) }, }, spec: { containers: [ { name: appName, image: "nginx:1.15-alpine", ports: [{ containerPort: 80 }] }, ], }, }, }, });

This should be pretty uncommon in practice. Secrets in the Kubernetes API are generally kept inside Secret and referenced only indirectly using something like (e.g., in the case of environment variables in containers) { envFrom: { secretRef: "my-secret" } } . But we should ship v1 knowing this is a limitation.

pgavlin · 2019-08-26T23:33:21Z

pkg/provider/provider.go

+// 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


This is going to be pretty impactful for things like the containers property of a deployment spec: that property is an array consisting of rich container specs. If any of these specs contains a secret, the whole array will be secret. This might be okay for now, but I would not be surprised if the user experience is not great there.

@lblackstone @hausdorff thoughts?

FWIW - We already have the same problem today with the current implementation, since the engine does this same sort of secrets application when the provider does not understand secrets. So the containers property in the state is already going to experience this problem, even without these changes.

I actually can't think of a situation where the containers array should have a secret value directly embedded in it. Every case I know of where you'd want to embed a secret, you'd make a Secret resource, and then use the downward API to reference the relevant field in the Secret.

So if someone embedded a secret value in an envvar, would we just tell them "don't do that"?

Yeah, I think so. As long as we're not leaking the secret value, I'm fine with this workflow not being nice to work with. We should point users to the upstream docs if they run into this:

Kubernetes secret objects let you store and manage sensitive information, such as passwords, OAuth tokens, and ssh keys. Putting this information in a secret is safer and more flexible than putting it verbatim in a Pod definition or in a container image .

That also makes it work nicely with our stuff.

Perfect; thanks.

pgavlin

Changes look good overall, just one question about arrays.

ellismg · 2019-08-27T00:11:10Z

There's an issue with this patch (which was not caught by the test I wrote) which causes a no-op update to propose changes to parts of the object. Looking into this now.

lblackstone · 2019-08-27T00:17:06Z

@ellismg Actually, we could just special case the data field for Secrets to always be marked as secret. That’s the only case like that I’m aware of.

pkg/provider/provider.go

hausdorff

We probably can get away with not implementing the "transitive secret flow" thing I talk about here. I think we probably can't get away with not marking the auto-populated .data field in Secret, as secret when the user populates .stringData.

As was mentioned elsewhere, populating .stringData in Secret will cause the Kubernetes API server to remove that field, populating the contents of .data with b64-encoded version of whatever used to be in .stringData. This is a very common use case—right now in this case .data "leaks" this secret to the statefile.

I think it's enough to take a pretty naive approach to this—the provider should be able to just check if the resource is of the form { apiVersion: "v1", kind: "Secret", …}, and mark .data as secret as well? Could be wrong.

hausdorff · 2019-08-27T18:42:28Z

Weird, in following program, the secret value shows up as "partially leaked" and "partially null". Known issue? I would expect that all of .stringData is encrypted here.

const s = pulumi.secret({ bar: "bat" });
const ss = new k8s.core.v1.Secret("bar", { type: "Opaque", stringData: s });

    +-kubernetes:core/v1:Secret: (replace)
        [id=default/bar-5mlmrdc5]
        [urn=urn:pulumi:exp-dev::exposed-deployment::kubernetes:core/v1:Secret::bar]
        [provider=urn:pulumi:exp-dev::exposed-deployment::pulumi:providers:kubernetes::default_0_25_5::118b2eaf-a922-46a5-898e-bfa2be188f0e]
      ~ stringData: {
          + bar: <null>
          - foo: "[secret]"
        }
      + type      : "Opaque"
    --kubernetes:core/v1:Secret: (delete-replaced)
        [id=default/bar-5mlmrdc5]
        [urn=urn:pulumi:exp-dev::exposed-deployment::kubernetes:core/v1:Secret::bar]
        [provider=urn:pulumi:exp-dev::exposed-deployment::pulumi:providers:kubernetes::default_0_25_5::118b2eaf-a922-46a5-898e-bfa2be188f0e]

Yet, it is correctly reported in the statefile:

                "inputs": {
                    "apiVersion": "v1",
                    "kind": "Secret",
                    "metadata": {
                        "annotations": {
                            "pulumi.com/autonamed": "true"
                        },
                        "labels": {
                            "app.kubernetes.io/managed-by": "pulumi"
                        },
                        "name": "bar-vb4cluky"
                    },
                    "stringData": {
                        "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                        "ciphertext": "AAABAFCguCZ0vY68z/gCeS67b6O7sktog9md3vZ7LWqA5Wg2UGXL8Lc+ym5M"
                    },
                    "type": "Opaque"
                },

ellismg · 2019-08-27T18:47:05Z

Weird, in following program, the secret value shows up as "partially leaked" and "partially null". Known issue? I would expect that all of .stringData is encrypted here.

The output makes me believe that this is from doing an initial update, modifying the source and then doing another update with some changes. Could you please share the complete set of steps?

hausdorff · 2019-08-27T18:50:32Z

The output makes me believe that this is from doing an initial update, modifying the source and then doing another update with some changes. Could you please share the complete set of steps?

Indeed, this does not happen on a fresh create. The previous version of the program is below. Change that program to the program above and it should replicate.

const s = pulumi.secret("bat");
const ss = new k8s.core.v1.Secret("bar", { type: "Opaque", stringData: { foo: s } });

hausdorff · 2019-08-27T19:01:26Z

Diffing behavior is kind of confusing when you change secrets. Change the image field in the program below to a pulumi.secret and no diff is rendered. Yet, in the statefile, I can clearly see that containers goes from being not encrypted -> encrypted. I would have expected a diff indicating plaintext -> [secret].

// nginx container, replicated 1 time.
const appName = "nginx";
const appLabels = { app: appName };
const nginx = new k8s.apps.v1.Deployment(appName, {
    metadata: { name: "foo" },
    spec: {
        selector: { matchLabels: appLabels },
        replicas: 1,
        template: {
            metadata: { labels: appLabels },
            spec: {
                containers: [
                    { name: appName, image: "nginx:1.15-alpine", ports: [{ containerPort: 80 }] },
                ],
            },
        },
    },
});

Secondly, and related to the last diff weirdness I mentioned, starting with this program:

const s = pulumi.secret({ foo: "bat"});
const ss = new k8s.core.v1.Secret("bar", { type: "Opaque", stringData: s });

and changing it to this:

const s = pulumi.secret("bat");
const ss = new k8s.core.v1.Secret("bar", { type: "Opaque", stringData: { foo: s } });

Will render no diff—and the statefile seems to keep the entire .stringData field secret—see below. This isn't a huge deal, but it might be indicative of mechanical problems that might result in "real" bugs.

                "outputs": {
                    "__inputs": {
                        "apiVersion": "v1",
                        "kind": "Secret",
                        "metadata": {
                            "annotations": {
                                "pulumi.com/autonamed": "true"
                            },
                            "labels": {
                                "app.kubernetes.io/managed-by": "pulumi"
                            },
                            "name": "bar-h0h0tphh"
                        },
                        "stringData": {
                            "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                            "ciphertext": "AAABAJaIqOW1taCrE8B1qnSkkHCXUObE6bE9twNI95Etm2oF2BpgtCIzE6ZM"
                        },
                        "type": "Opaque"
                    },

Yet, when I change pulumi.secret("bat"); -> pulumi.secret("bar");, we can see that now only .stringData.foo is encrypted.

                "outputs": {
                    "__inputs": {
                        "apiVersion": "v1",
                        "kind": "Secret",
                        "metadata": {
                            "annotations": {
                                "pulumi.com/autonamed": "true"
                            },
                            "labels": {
                                "app.kubernetes.io/managed-by": "pulumi"
                            },
                            "name": "bar-gw54zj9e"
                        },
                        "stringData": {
                            "foo": {
                                "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                                "ciphertext": "AAABANbaiqgdYXQAzw2AKQUkCKk/z6p90nd+XaRRRhoENy/Dvw=="
                            }
                        },
                        "type": "Opaque"
                    },

hausdorff · 2019-08-27T20:04:15Z

FYI, I did try upgrading/downgrading kube providers, and that seems to work well. I think all my feedback is related to diff, encrypting .data, and this "transitive" secret flow that I don't think is critical to address in this review. I suspect the diff parts of this feedback are actually problems generally, though.

lblackstone · 2019-08-27T22:49:21Z

pkg/provider/provider.go

+
+		if hasStringData && hasData {
+			if stringData.IsSecret() && !data.IsSecret() {
+				object["data"] = resource.MakeSecret(data)


Is there a reason to include this check rather than always marking data as secret?

Yeah, I think we'll probably want to always mark .data as secret. Here's the current behavior:

const foo = new k8s.core.v1.Secret("foo", { stringData: { password: "foo", } }); export const fooval = foo.data;

fooval : { + password: "Zm9v" }

Obviously you wouldn't want to encode secrets directly in the program like that, but they could have been generated as part of a Helm chart or similar and not marked as secret.

Obviously you wouldn't want to encode secrets directly in the program like that, but they could have been generated as part of a Helm chart or similar and not marked as secret.

My goal here was the limit the blast radius of this change given how close we are to the end of the release. When we did stuff like this on the TF side, we ran into a bunch of issues and had to roll back the chage.

What you say would be correct, and is something we should consider doing as part of the next release, but I am concerned about making this change by default, since it will impact everyone who is using a Secret today.

If you want to opt into the behavior you describe, you can use additionalSecretOutputs to ensure the entire data field of the Secret is marked as something that should be persisted securely.

tests/integration/secrets/step1/index.ts

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). In addition, this adds code to mark `data` as secret on `k8s.core.v1.Secret` if `stringData` is a secret (the API Server base64 encodes the `stringData` bag into `data` and so we should logically flow the secretness). Fixes #734

ellismg · 2019-08-28T00:01:26Z

Okay, @lblackstone I have addressed all your feedback and CI should now be green.

lblackstone

LGTM, aside from one question: do you still need secrets.test? Not sure what that's for.

ellismg · 2019-08-28T17:03:43Z

LGTM, aside from one question: do you still need secrets.test? Not sure what that's for.

It was the test binary (generated by VSCode when trying to "debug" the test) which was incorrectly checked in because of an errant git add

ellismg force-pushed the ellismg/fix-734 branch from d09875b to e0e7890 Compare August 23, 2019 17:52

ellismg requested review from pgavlin, lblackstone and lukehoban August 23, 2019 22:17

lblackstone mentioned this pull request Aug 26, 2019

Mask values of any program secrets in CLI output, not just config secrets pulumi/pulumi#3138

Open

ellismg force-pushed the ellismg/fix-734 branch from e0e7890 to 25ffc20 Compare August 26, 2019 22:22

ellismg changed the title ~~WIP - Tracks Secrets in __inputs and lastAppliedConfig~~ Tracks Secrets in __inputs and lastAppliedConfig Aug 26, 2019

ellismg marked this pull request as ready for review August 26, 2019 22:23

lblackstone requested changes Aug 26, 2019

View reviewed changes

ellismg force-pushed the ellismg/fix-734 branch from 25ffc20 to f1a4ce6 Compare August 26, 2019 23:08

pgavlin reviewed Aug 26, 2019

View reviewed changes

ellismg force-pushed the ellismg/fix-734 branch from f1a4ce6 to 1c5d218 Compare August 26, 2019 23:42

ellismg force-pushed the ellismg/fix-734 branch from 1c5d218 to 07a913a Compare August 27, 2019 00:42

hausdorff reviewed Aug 27, 2019

View reviewed changes

pkg/provider/provider.go Outdated Show resolved Hide resolved

pkg/provider/provider.go Outdated Show resolved Hide resolved

hausdorff reviewed Aug 27, 2019

View reviewed changes

ellismg force-pushed the ellismg/fix-734 branch from 07a913a to 4fa8649 Compare August 27, 2019 22:29

lblackstone reviewed Aug 27, 2019

View reviewed changes

tests/integration/secrets/step1/index.ts Outdated Show resolved Hide resolved

lblackstone requested changes Aug 27, 2019

View reviewed changes

tests/integration/secrets/step1/index.ts Outdated Show resolved Hide resolved

ellismg force-pushed the ellismg/fix-734 branch from 4fa8649 to 9aa2546 Compare August 27, 2019 23:15

ellismg force-pushed the ellismg/fix-734 branch from 9aa2546 to 133c4e8 Compare August 28, 2019 00:01

lblackstone approved these changes Aug 28, 2019

View reviewed changes

Cleanup PR

0412236

lblackstone merged commit f86e632 into master Aug 28, 2019

pulumi-bot deleted the ellismg/fix-734 branch August 28, 2019 16:45

lblackstone mentioned this pull request Sep 6, 2019

lastAppliedConfig is tainting entire metadata field #787

Closed

Tracks Secrets in __inputs and lastAppliedConfig #741

Tracks Secrets in __inputs and lastAppliedConfig #741

Conversation

ellismg commented Aug 23, 2019 • edited Loading

ellismg commented Aug 23, 2019

ellismg commented Aug 23, 2019

lblackstone commented Aug 23, 2019 • edited Loading

lblackstone commented Aug 23, 2019

lukehoban commented Aug 25, 2019 • edited Loading

ellismg commented Aug 26, 2019

lblackstone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lblackstone Aug 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hausdorff Aug 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgavlin left a comment

Choose a reason for hiding this comment

ellismg commented Aug 27, 2019

lblackstone commented Aug 27, 2019

hausdorff left a comment • edited Loading

Choose a reason for hiding this comment

hausdorff commented Aug 27, 2019 • edited Loading

ellismg commented Aug 27, 2019

hausdorff commented Aug 27, 2019

hausdorff commented Aug 27, 2019 • edited Loading

hausdorff commented Aug 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellismg commented Aug 28, 2019 • edited Loading

lblackstone left a comment

Choose a reason for hiding this comment

ellismg commented Aug 28, 2019

Tracks Secrets in `__inputs` and `lastAppliedConfig` #741

Tracks Secrets in `__inputs` and `lastAppliedConfig` #741

ellismg commented Aug 23, 2019 •

edited

Loading

lblackstone commented Aug 23, 2019 •

edited

Loading

lukehoban commented Aug 25, 2019 •

edited

Loading

lblackstone Aug 26, 2019 •

edited

Loading

hausdorff Aug 27, 2019 •

edited

Loading

hausdorff left a comment •

edited

Loading

hausdorff commented Aug 27, 2019 •

edited

Loading

hausdorff commented Aug 27, 2019 •

edited

Loading

ellismg commented Aug 28, 2019 •

edited

Loading