Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BucketIAMBinding unexpected diff re-order of members #1935

Closed
zbuchheit opened this issue Apr 17, 2024 · 5 comments
Closed

BucketIAMBinding unexpected diff re-order of members #1935

zbuchheit opened this issue Apr 17, 2024 · 5 comments
Assignees
Labels
area/import An issue related to `pulumi import` or the import resource option. bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@zbuchheit
Copy link

What happened?

While attempting to import a BucketIAMBinding I encountered a re-order diff of the members. It resulted in no functional changes but could cause some diff anxiety to a user importing a resource.

Example

Code example

import * as pulumi from "@pulumi/pulumi";
import * as gcp from "@pulumi/gcp";
import { Console } from "console";

// Create a GCP resource (Storage Bucket)
const bucket = new gcp.storage.Bucket("zbucket", {
    location: "US"
});

const objectAdminBinding = new gcp.storage.BucketIAMBinding("objectAdminBinding", {
    bucket: bucket.name,
    role: "roles/storage.objectAdmin",
    members: [
        "user:zbuchheit@pulumi.com",
        "user:phillip@pulumi.com"
    ],
}, {protect: false});

export const urn = objectAdminBinding.urn;

Repro shell script

#!/bin/bash

pulumi up --yes

urn=$(pulumi stack output urn)
bucketName=$(pulumi stack output bucketName)

pulumi state delete $urn --yes

pulumi import gcp:storage/bucketIAMBinding:BucketIAMBinding objectAdminBinding "b/$bucketName roles/storage.objectAdmin" --yes

pulumi preview --diff

Example output


      ~ bucket : "b/zbucket-7c2523e" => "zbucket-7c2523e"
      ~ members: [
          ~ [0]: "user:phillip@pulumi.com" => "user:zbuchheit@pulumi.com"
          ~ [1]: "user:zbuchheit@pulumi.com" => "user:phillip@pulumi.com"
        ]

Output of pulumi about

CLI          
Version      3.113.0
Go Version   go1.22.2
Go Compiler  gc

Plugins
NAME    VERSION
gcp     7.9.0
nodejs  unknown

Host     
OS       darwin
Version  14.2.1
Arch     arm64

This project is written in nodejs: executable='/Users/zbuchheit/.nvm/versions/node/v18.17.1/bin/node' version='v18.17.1'

Current Stack: zbuchheit-pulumi-corp/gcp-ts/test1

TYPE                                           URN
pulumi:pulumi:Stack                            urn:pulumi:test1::gcp-ts::pulumi:pulumi:Stack::gcp-ts-test1
pulumi:providers:gcp                           urn:pulumi:test1::gcp-ts::pulumi:providers:gcp::default_7_9_0
gcp:storage/bucket:Bucket                      urn:pulumi:test1::gcp-ts::gcp:storage/bucket:Bucket::zbucket
gcp:storage/bucketIAMBinding:BucketIAMBinding  urn:pulumi:test1::gcp-ts::gcp:storage/bucketIAMBinding:BucketIAMBinding::objectAdminBinding


Found no pending operations associated with test1

Backend        
Name           pulumi.com
URL            https://app.pulumi.com/zbuchheit-pulumi-corp
User           zbuchheit-pulumi-corp
Organizations  zbuchheit-pulumi-corp
Token type     personal

Dependencies:
NAME            VERSION
@pulumi/gcp     7.9.0
@pulumi/pulumi  3.112.0
@types/node     18.19.31

Pulumi locates its logs in /var/folders/lh/l71cdh810xb33t0jc7qmt5_80000gn/T/ by default

Additional context

Encountered while looking at #1900

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

@zbuchheit zbuchheit added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Apr 17, 2024
@VenelinMartinov VenelinMartinov added area/import An issue related to `pulumi import` or the import resource option. bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. and removed needs-triage Needs attention from the triage team labels Apr 18, 2024
@VenelinMartinov
Copy link
Contributor

Thanks for the report and the repro!

Looks like upstream defines the members parameter as a Set: https://github.com/hashicorp/terraform-provider-google-beta/blob/a01d7e06b11a9b17e13b29477a6f85d6a0e825f5/google-beta/tpgiamresource/resource_iam_binding.go#L26

This means that we should not be producing a diff here as the order of elements in the set is meaningless, so this must be a bridge issue.

@t0yv0
Copy link
Member

t0yv0 commented Apr 24, 2024

This would be interesting to chase down and perhaps I prematurely closed pulumi/pulumi-terraform-bridge#1417

There is a canonical ordering of set elements defined by their key (TF calls "hash"). AFAIK when bridge handles diffing, I got the impression that states before and after that differ only on the order are suppressed as no-diff. However importing and refreshing is special and I've not done due diligence there.

@mikhailshilkov
Copy link
Member

@t0yv0 Did you have a chance to take a look? Should we reopen pulumi/pulumi-terraform-bridge#1417?

@t0yv0
Copy link
Member

t0yv0 commented May 10, 2024

I've not recently looked at this but I think this is indeed still a problem with both refresh and import that utilize the Read function. Today we have this issue tracking the refresh problem specifically:

pulumi/pulumi-terraform-bridge#1904

It is a little more complicated for imports as per:

https://pulumi-developer-docs.readthedocs.io/en/latest/architecture/import.html

In the case of pulumi import we have Read, Check, Diff which has more chances to fixup the order..

But perhaps we can fix this with canonicalizing the ordering in Read (in both import and refresh case).

@guineveresaenger
Copy link
Contributor

I believe this was fixed in pulumi/pulumi via pulumi/pulumi#16024.

I reran your steps on pulumi 3.116.1 and did not see a diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/import An issue related to `pulumi import` or the import resource option. bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

5 participants