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

Confusing diffs when unknowns are passed into set or list-nested blocks #1885

Closed
t0yv0 opened this issue Apr 18, 2024 · 4 comments · Fixed by #2061
Closed

Confusing diffs when unknowns are passed into set or list-nested blocks #1885

t0yv0 opened this issue Apr 18, 2024 · 4 comments · Fixed by #2061
Assignees
Labels
bug/diff Bugs in computing Diffs and planning resource changes kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@t0yv0
Copy link
Member

t0yv0 commented Apr 18, 2024

What happened?

There is machinery in MakeTerraformInputs specifically makeTerraformUnknown that tries to substitute reasonably shaped empty values for unknowns when the schema indicates lists or sets. Unfortunately this is involved in Check turnaround, so that Pulumi providers start swallowing these unknowns even at Check level, and definitely continue swallowing them during diffs.

Primarily this leads to surprising and unreliable Diff behavior where unknowns are replaced by empties.

But occasionally it also leads to substantial problems; in case of pulumi/pulumi-aws#3835 this uncovered a latent bug making the provider panic on this empty input that would not have otherwise occur in a normal program.

See also:

Example

Customer repro:

package main

import (
	"github.com/pulumi/pulumi-aws/sdk/v6/go/aws/ec2"
	"github.com/pulumi/pulumi-aws/sdk/v6/go/aws/iam"
	"github.com/pulumi/pulumi-aws/sdk/v6/go/aws/rds"
	"github.com/pulumi/pulumi-aws/sdk/v6/go/aws/secretsmanager"
	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
)

func main() {
	pulumi.Run(func(ctx *pulumi.Context) error {

		assumeRolePolicy, _ := iam.GetPolicyDocument(ctx, &iam.GetPolicyDocumentArgs{
			Statements: []iam.GetPolicyDocumentStatement{
				{
					Actions:    []string{"sts:AssumeRole"},
					Principals: []iam.GetPolicyDocumentStatementPrincipal{{Type: "Service", Identifiers: []string{"lambda.amazonaws.com"}}},
				},
			},
		})
		role, _ := iam.NewRole(ctx, "myrole", &iam.RoleArgs{
			AssumeRolePolicy: pulumi.String(assumeRolePolicy.Json),
		})

		vpc, _ := ec2.NewVpc(ctx, "Vpc", &ec2.VpcArgs{
			CidrBlock: pulumi.String("10.0.0.0/16"),
		})
		subnet, _ := ec2.NewSubnet(ctx, "subnet", &ec2.SubnetArgs{
			VpcId:            vpc.ID(),
			CidrBlock:        pulumi.String("10.0.2.0/24"),
			AvailabilityZone: pulumi.String("us-east-2a"),
		})
		anotherSubnet, _ := ec2.NewSubnet(ctx, "another-subnet", &ec2.SubnetArgs{
			VpcId:            vpc.ID(),
			CidrBlock:        pulumi.String("10.0.1.0/24"),
			AvailabilityZone: pulumi.String("us-east-1a"),
		})

		secret, err := secretsmanager.NewSecret(ctx, "secret", &secretsmanager.SecretArgs{})
		if err != nil {
			panic(err)
		}

		if 1+2 == 3 {
			rds.NewProxy(ctx, "proxy", &rds.ProxyArgs{
				VpcSubnetIds: pulumi.StringArray{
					subnet.ID(),
					anotherSubnet.ID(),
				},
				Auths: secret.Arn.ApplyT(func(arn string) []rds.ProxyAuth {
					return []rds.ProxyAuth{
						{
							AuthScheme: pulumi.StringRef("SECRETS"),
							IamAuth:    pulumi.StringRef("DISABLED"),
							SecretArn:  &arn,
						},
					}
				}).(rds.ProxyAuthArrayInput),
				RoleArn:      role.Arn,
				EngineFamily: pulumi.String("MYSQL"),
			})
		}
		return nil
	})
}

Auths receives a secret.Arn which is unknown. However Check translates it to a known value.

Output of pulumi about

N/A

Additional context

No response

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

@t0yv0 t0yv0 added needs-triage Needs attention from the triage team kind/bug Some behavior is incorrect or out of spec labels Apr 18, 2024
@VenelinMartinov VenelinMartinov added bug/diff Bugs in computing Diffs and planning resource changes 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 here!

@iwahbe iwahbe self-assigned this Apr 25, 2024
@VenelinMartinov
Copy link
Contributor

@iwahbe I've done some investigation on this around #2032 which is likely a duplicated/very related.

I'll take over if that's ok

@VenelinMartinov
Copy link
Contributor

#2061 fixes this

@pulumi-bot pulumi-bot added resolution/fixed This issue was fixed labels Jul 17, 2024
@mjeffryes mjeffryes added this to the 0.107 milestone Jul 24, 2024
@pulumi-bot
Copy link
Contributor

This issue has been addressed in PR #2061 and shipped in release v3.88.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/diff Bugs in computing Diffs and planning resource changes kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants