lastAppliedConfig is tainting entire metadata field

[lblackstone]

Problem description

• I saw an issue in the following Pulumi Program:

let googleMapsKey = config.requireSecret('googleMapsKey');
const googleMapsKeySecret = new k8s.core.v1.Secret(`google-maps-key-${stackName}`, {
    metadata: { namespace: namespace },
    stringData: {GOOGLE_MAPS_KEY: googleMapsKey},
}, {provider: provider});

const deployment = new ServiceDeployment(`deployment-${stackName}`, namespace, {
    // ...
    envFrom: [{secretRef: {name: googleMapsKeySecret.metadata.name}}],
}, {provider: provider});

export class ServiceDeployment extends pulumi.ComponentResource {
    constructor(name: string, namespace: pulumi.Output<string>, args: ServiceDeploymentArgs, opts?: pulumi.ComponentResourceOptions) {
        super("vantrix:pulumi-library:ServiceDeployment", name, {}, opts);

        const container: k8stypes.core.v1.Container = {
            // ...
            envFrom: args.envFrom,
        };
        this.deployment = new k8s.apps.v1.Deployment(name, {
            // ...
            spec: {
                template: {
                    spec: {
                        containers: [ container ],
                        imagePullSecrets: args.imagePullSecrets,
                    },
                },
            },
        }, { parent: this });
    }
}

export interface ServiceDeploymentArgs {
    // ...
    imagePullSecrets?: pulumi.Input<k8stypes.core.v1.LocalObjectReference>[],
    envFrom?: pulumi.Input<k8stypes.core.v1.EnvFromSource>[];
}

When the Secret is created, Pulumi is setting .metadata.annotations[kubectl.kubernetes.io/last-applied-configuration] and (correctly) marking it as secret. Unfortunately, this is also marking all of the other .metadata fields as secret, even though they do not contain secret values.

In this program, the Deployment's containers field is being tainted as secret by transitively referring to the Secret's .metadata.name.

This behavior was introduced by #741

Errors & Logs

Affected product version(s)

Reproducing the issue

Suggestions for a fix

We should narrow the scope of the "secretness" so that it doesn't include all of the .metadata fields.

[ellismg]

We should narrow the scope of the "secretness" so that it doesn't include all of the .metadata fields.

Note that the problem here is sort of orthogonal to the kubernetes provider. The tainting of the top level Output<T> happens inside @pulumi/pulumi when we are deserializing the result of the RPC to register resource. The logic here is in deserializeProperty inside the runtime portion of the SDK.

Indeed - the provider itself is doing the right thing, it's being very explicit about what parts of the overall object are secret, but we just don't have a great way to capture that in the language, since we track everything at the output of T level.

I could imagine ways that this could be made to work (Output<T> could hold on to more information for rich objects about what properties were secret or not) and that would allow us to perhaps make things like foo.bar (where foo is an object with a secret property named baz but bar itself is not secret) by playing games with our proxies.

It may be a little weird because it would mean that foo.bar would be different than foo.apply(f => f.bar) (in this case, we'd have to taint the entire result of the apply since we have no idea of some secret fields have been twiddled with).

As a first step, we should probably just do something like pulumi.runtime.removeSecretness(Output<T>) that lets you remove the secretness bit from an given Output<T>

/cc @pgavlin

[ellismg]

FWIW - This problem is a more generalized case of what we faced with StackOutput.

[squarebracket]

FWIW, I was able to work around this like so:

googleMapsKeySecret.metadata.name.apply(n => {
    const deployment = new ServiceDeployment(`deployment-${stackName}`, namespace, {
        // ...
        envFrom: [{secretRef: {name: n}}],
    }, {provider: provider});
});

Doing this showed me the changes for the container. Not pretty, but it works.

[lblackstone]

Fixed by #2445