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

Modifying an IAMMember deletes the resource and doesn't recreate it. #2463

Open stooj opened 1 month ago

stooj commented 1 month ago

Describe what happened

When modifying the name of a gcp.projects.IAMMember, the resource is deleted on changes.

The same thing happens with gcp.serviceaccount.IAMMember.

Pulumi deletes the old resource, and the create action "succeeds", but the resource is not actually created.

To reproduce:

  1. pulumi up the code below
  2. Check the users associated with the project with gcloud projects get-iam-policy ticket-5866 --flatten="bindings[].members" --format="table(bindings.members)"
  3. Make a change to projectMember name (say, ticket-5866 -> ticket-5866foo)
  4. Run another pulumi up
  5. Check the users again using the same gcloud command.

Sample program

import * as gcp from "@pulumi/gcp";

// Project to house all this stuff
const ticketProject = new gcp.organizations.Project(
  "ticketProject",
  {
    name: "Ticket 5866",
    // deletionPolicy: "DELETE",
    projectId: "ticket-5866",
    orgId: "<REPLACE_WITH_ORG_ID>",
  },
  {
    retainOnDelete: true,
  },
);

// IAMMember
const projectMember = new gcp.projects.IAMMember("ticket-5866", {
  project: ticketProject.id,
  role: "roles/editor",
  member: "user:josse@pulumi.com",
});

Log output

change-iam-member.txt

Affected Resource(s)

Output of pulumi about

Tested with pulumi v3.113.0 and v3.134.1, and with pulumi/gcp 8.3.1.

CLI          
Version      3.134.1
Go Version   go1.23.1
Go Compiler  gc

Plugins
KIND      NAME    VERSION
resource  gcp     8.3.1
language  nodejs  unknown

Host     
OS       nixos
Version  24.05 (Uakari)
Arch     x86_64

This project is written in nodejs: executable='/nix/store/ilkfhnqz4xczrliqjva8770x2svbfznd-nodejs-20.14.0/bin/node' version='v20.14.0'

Current Stack: stooj/5866/dev

TYPE                               URN
pulumi:pulumi:Stack                urn:pulumi:dev::5866::pulumi:pulumi:Stack::5866-dev
pulumi:providers:gcp               urn:pulumi:dev::5866::pulumi:providers:gcp::default_8_3_1
gcp:organizations/project:Project  urn:pulumi:dev::5866::gcp:organizations/project:Project::ticketProject
gcp:projects/iAMMember:IAMMember   urn:pulumi:dev::5866::gcp:projects/iAMMember:IAMMember::ticket-5866foo

Found no pending operations associated with dev

Backend        
Name           pulumi.com
URL            https://app.pulumi.com/stooj
User           stooj
Organizations  stooj, team-ce
Token type     personal

Dependencies:
NAME            VERSION
typescript      5.6.2
@pulumi/gcp     8.3.1
@pulumi/pulumi  3.134.1
@types/node     18.19.53

Pulumi locates its logs in /tmp by default

Additional context

A pulumi up --refresh will fix this, but since pulumi thinks the recreation succeeds the user isn't given any warning that things didn't actually work.

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

guineveresaenger commented 1 month ago

Hi @stooj - thank you for filing this issue. It sounds like there's a workaround for in the meantime but we'll take a look.

VenelinMartinov commented 1 month ago

This sounds like a structural resource which needs DeleteBeforeReplace in the schema. For example: https://github.com/pulumi/pulumi-aws/blob/3ff4fa4ddc54bcef6d043e7c62e0e881c6ecdaa5/provider/resources.go#L2435

stooj commented 1 week ago

Thanks for looking at this @rshade

I tested my repro with v8.8.0 and it still exhibits the same behaviour as before, so this isn't fixed 🙁

stooj commented 20 hours ago

Changing the name of the pulumi resource changes the URN, so pulumi removes the old resource and creates a new one.

But because nothing on the GCP side is changing, GCP doesn't see any differences.

And pulumi creates first, then deletes. So what I think is happening is:

  1. Pulumi sees a new URN and marks the old one for deletion, the new one for creation
  2. Pulumi asks GCP to create a new object: {"a" = 1, "b" = 2}
  3. GCP already has that object so returns a success
  4. Pulumi then asks GCP to delete the old object: {"a" = 1, "b" = 2}
  5. GCP deletes the only object it has, which is the original

This issue wouldn't happen if any other properties were changed on the resource.

But Pulumi deleting resources without alerting the user is a footgun that we should warn the user about. Can the engine detect a URN change when nothing else is changed?

Do we support changing the names of resources?

rshade commented 17 hours ago

Aliases is your answer here. We will be adding DeleteBeforeReplace back once we get conditional DBR, which will solve it getting deleted in the second step, however, renaming of resource is always handled via Aliases, that way its a NoOp.

stooj commented 17 hours ago

Sure, I agree that using aliases are the fix.

But we need to protect users from this if they don't know they need to use aliases, because at the moment pulumi deletes resources without telling you about it.

VenelinMartinov commented 17 hours ago

We reverted DeleteBeforeReplace by default for these resources: https://github.com/pulumi/pulumi-gcp/pull/2595

However, it looks like the IAMMember can have an unconditional DeleteBeforeReplace - it seems unlikely that it ever works without DeleteBeforeReplace

We could add it back for IAMMember, IAMPolicy and any other resources which never work without DeleteBeforeReplace - these do not need a conditional DeleteBeforeReplace

rshade commented 14 hours ago

@VenelinMartinov yep, I can do that later.

@stooj I would say after that you need a pulumi/pulumi ticket because this is about comparing selfLinks to determine replacement strategy versus the urn.

rshade commented 14 hours ago

https://github.com/pulumi/pulumi-gcp/pull/2650

VenelinMartinov commented 13 hours ago

@stooj I think https://github.com/pulumi/pulumi/issues/15982 captures the problem you described