pulumi / pulumi-gcp

A Google Cloud Platform (GCP) Pulumi resource package, providing multi-language access to GCP
Apache License 2.0
183 stars 53 forks source link

unknown StringOutput Project on NewProvider() causes potential unwanted deletion/modification of many resources #2539

Closed pdemagny closed 2 weeks ago

pdemagny commented 3 weeks ago

Describe what happened

After an upgrade of this provider and without any changes in my codebase (can't remember exactly but it was around may I think), I started having very weird previews:

I would get a lot of warnings:

warning: The provider for this resource has inputs that are not known during preview.
This preview may not correctly represent the changes that will be applied during an update.

And lots of values would end up unknown again:

"pdemagny-postgres-blobr-ai" => output<string>

And lots of occurences in the diff of:

                    - __defaults: []

And other randomly removed properties on resources ...

Sample program

I have this package that is overly complex for what it does, but it looks like this to maintain consistency with all our other packages in our codebase that all implements the Golang Functional Options Pattern.

import (
    "http://github.com/kr/pretty"
    googlecloud "http://github.com/pulumi/pulumi-gcp/sdk/v7/go/gcp"
    "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
)

type (
    ResourceArgs struct {
        Name      string
        ProjectId pulumi.StringOutput
        Region    string
    }
    GcpProviderOptions func(*GcpProviderArgs)
    GcpProviderArgs    struct {
        Context         *pulumi.Context
        ResourceOptions []pulumi.ResourceOption
        Debug           bool
        ResourceArgs    ResourceArgs
    }
)

func WithContext(ctx *pulumi.Context) GcpProviderOptions {
    return func(p *GcpProviderArgs) { p.Context = ctx }
}

func WithResourceOptions(resourceOptions []pulumi.ResourceOption) GcpProviderOptions {
    return func(p *GcpProviderArgs) { p.ResourceOptions = resourceOptions }
}

func WithDebug(debug bool) GcpProviderOptions {
    return func(p *GcpProviderArgs) { p.Debug = debug }
}

func WithName(name string) GcpProviderOptions {
    return func(p *GcpProviderArgs) { p.ResourceArgs.Name = name }
}

func WithRegion(region string) GcpProviderOptions {
    return func(p *GcpProviderArgs) { p.ResourceArgs.Region = region }
}

func WithProjectId(projectId pulumi.StringOutput) GcpProviderOptions {
    return func(i *GcpProviderArgs) { i.ResourceArgs.ProjectId = projectId }
}

func NewProvider(opts ...GcpProviderOptions) *GcpProviderArgs {
    // Default values
    obj := GcpProviderArgs{
        Debug: false,
    }
    for _, opt := range opts {
        opt(&obj)
    }
    if obj.Debug {
        ctx := obj.Context
        ctx.Log.Info("gcp/provider.NewProvider:", nil)
        ctx.Log.Info(pretty.Sprint(&obj.ResourceArgs), nil)
    }
    return &obj
}

func (p *GcpProviderArgs) Create() (*googlecloud.Provider, error) {
    return googlecloud.NewProvider(p.Context, p.ResourceArgs.Name, &googlecloud.ProviderArgs{
        Region:  pulumi.String(p.ResourceArgs.Region),
        Project: p.ResourceArgs.ProjectId,
    })
}

As i said I can't find out what version it was, but a version of the Pulumi GCP Provider required me to add the Project option to the NewProvider() method. But it doesn't seem to be a hard requirement, so when I create the provider like this:

gcpEuropeWest1Provider, err := gcpprovider.NewProvider(
    gcpprovider.WithContext(ctx),
    gcpprovider.WithDebug(true),
    gcpprovider.WithName("gcp-europewest1-provider"),
    gcpprovider.WithRegion(gcpRegion),
).Create()
if err != nil {
    return err
}

It causes warnings and ultimately errors and panic (same result with both pulumi cli & automation api):

Diagnostics:
  gcp:organizations:Folder (test-engineering):
    warning: The provider for this resource has inputs that are not known during preview.
    This preview may not correctly represent the changes that will be applied during an update.

  gcp:organizations:Project (test-prod-org1):
    warning: The provider for this resource has inputs that are not known during preview.
    This preview may not correctly represent the changes that will be applied during an update.

And it goes like this for all resources of this provider.

If I try to apply anyway:

panic: fatal: An assertion has failed: Update cannot be called if the configuration is unknown

When I remove only the Region instead of the Project, i get the expected:

error: rpc error: code = Unknown desc = expected a non-empty string: region was set to ``

Finally When I create the provider like this:

gcpEuropeWest1Provider, err := gcpprovider.NewProvider(
    gcpprovider.WithContext(ctx),
    gcpprovider.WithDebug(true),
    gcpprovider.WithName("gcp-europewest1-provider"),
    gcpprovider.WithRegion("europe-west1"),
    gcpprovider.WithProjectId(u.Ps("myproject").ToStringOutput()),
).Create()
if err != nil {
    return err
}

Everything works as expected !

I suspect that somehow with my code the Project ends up nil and it makes the provider loose his mind instead of just saying that it can't be empty (as it does when I don't provide the Region).

Log output

No response

Affected Resource(s)

Output of pulumi about

❯ pulumi about CLI
Version 3.137.0 Go Version go1.23.2 Go Compiler gc

Host
OS ubuntu Version 22.04 Arch x86_64

Backend
Name pulumi.com URL https://app.pulumi.com/pdemagny-blobr User pdemagny-blobr Organizations pdemagny-blobr, blobr-io, blobr-ai Token type personal

Pulumi locates its logs in /tmp by default

Additional context

This is related to support ticket n°5654.

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

VenelinMartinov commented 3 weeks ago

Hey @pdemagny. Sorry you've had trouble here. Looking at your code it looks like you are typing the ProjectId input to the Provider as pulumi.StringOutput. I don't believe that providers support output-type inputs, which is what the error message is trying to tell you.

Can you try changing that to:

type (
    ResourceArgs struct {
        Name      string
        ProjectId string
        Region    string
    }

That should work much better as that promises pulumi that the project id configuration will always be available - it can not be unknown.

pdemagny commented 2 weeks ago

When I modify my code with your suggestion, it demonstrates the expected behavior:

error: rpc error: code = Unknown desc = expected a non-empty string: project was set to ``

Thanks for the fix. But it's still possible to pass a StringOutput that ends up potentialy unknown and risk breaking stuff, isn't there a way to prevent this ?

VenelinMartinov commented 2 weeks ago

Hey @pdemagny. Do you think the warning is insufficient here? Would you prefer it if we provided a flag for this to become a hard error? Unfortunately we can not do this by default as some user programs do depend on this behaviour and we'd like to avoid breaking them.

pdemagny commented 2 weeks ago

Hi @VenelinMartinov. I think it would definitely be best if was an error instead of a warning, but not at the cost of a breaking change, and anyway that's not my place to tell 😅 In this case I think the warning could be enough, since it's gonna be a rare bug, but the warning could maybe be clearer somehow.

VenelinMartinov commented 2 weeks ago

Glad you managed to fix the issue on your end. Let me know if you run into any more troubles here!