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

PF investigate support for PlanModifiers #2171

Closed
VenelinMartinov opened this issue Jul 10, 2024 · 6 comments
Closed

PF investigate support for PlanModifiers #2171

VenelinMartinov opened this issue Jul 10, 2024 · 6 comments
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/no-repro This issue wasn't able to be reproduced
Milestone

Comments

@VenelinMartinov
Copy link
Contributor

What happened?

Looking at the code around PF Diff it doesn't look like PlanModifiers are used in ProposedNew anywhere - does that mean we ignore these?

Example

"vlan_id": schema.StringAttribute{
	MarkdownDescription: `VLAN ID`,
	Computed:            true,
	Optional:            true,
	PlanModifiers: []planmodifier.String{
		stringplanmodifier.UseStateForUnknown(),
	},
},

This should yield a different plan then when the PlanModifier is missing.

Output of pulumi about

.

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

@VenelinMartinov VenelinMartinov added the kind/engineering Work that is not visible to an external user label Jul 10, 2024
@t0yv0
Copy link
Member

t0yv0 commented Jul 16, 2024

I expect plan modifiers to fully work, curious if you find a counter-example.

@corymhall
Copy link
Contributor

I think that pulumi/pulumi-aws#4273 is an example of this in pulumi-aws

@VenelinMartinov
Copy link
Contributor Author

I think I have a repro: #2217

I'm assuming this is connected to #2218 and will investigate as part of that.

@VenelinMartinov VenelinMartinov self-assigned this Jul 24, 2024
@VenelinMartinov VenelinMartinov added kind/bug Some behavior is incorrect or out of spec and removed kind/engineering Work that is not visible to an external user labels Jul 24, 2024
@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented Jul 24, 2024

Not 100% sure about this anymore - tried to repro in pulumi-random:

"""A Python Pulumi program"""

import pulumi
import pulumi_random as random


resNumeric = random.RandomString(
    "str1",
    length=10,
    numeric=False,
)

resNumber = random.RandomString(
    "str2",
    length=10,
    number=False,
)

pulumi.export("number val 1", resNumeric.number)
pulumi.export("numeric val 1", resNumeric.numeric)

pulumi.export("number val 2", resNumber.number)
pulumi.export("numeric val 2", resNumber.numeric)

These properties have plan modifiers and they seem to be working: https://github.com/hashicorp/terraform-provider-random/blame/455294a2038f3cf39a31f8eda231b95f190df250/internal/provider/resource_string.go#L453

The above program outputs False, False, False, False

@corymhall
Copy link
Contributor

corymhall commented Jul 24, 2024

I can't reproduce the AWS issue anymore either if I use the latest bridge@master so something must have been fixed (I can reproduce on latest aws which is on [email protected])

@VenelinMartinov VenelinMartinov added the resolution/no-repro This issue wasn't able to be reproduced label Jul 26, 2024
@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented Jul 26, 2024

This turned out to be a red herring - the test was set up wrong - plan modifiers are applied fine as long as the provider is implemented correctly to apply the plan, not the config:

#2217 will add a regression test here.

@mjeffryes mjeffryes added this to the 0.108 milestone Aug 16, 2024
VenelinMartinov added a commit that referenced this issue Aug 21, 2024
Adds PF integration tests for defaults and plan modifiers.

Regression tests for
#2171 and
#2218

stacked on #2254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec resolution/no-repro This issue wasn't able to be reproduced
Projects
None yet
Development

No branches or pull requests

4 participants