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

Bridge should handle SchemaConfigModeAttr block attributes #1762

Closed
VenelinMartinov opened this issue Mar 18, 2024 · 4 comments · Fixed by #1971
Closed

Bridge should handle SchemaConfigModeAttr block attributes #1762

VenelinMartinov opened this issue Mar 18, 2024 · 4 comments · Fixed by #1971
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@VenelinMartinov
Copy link
Contributor

What happened?

During the work on #1725 I stumbled upon a panic in pulumi-azure: #1725 (comment)

TF SDK providers seem to have a special backwards-compatibility mode which allows specific resources to opt-in to a behaviour where they can distinguish between nil and empty collections - https://developer.hashicorp.com/terraform/plugin/sdkv2/guides/terraform-0.12-compatibility#computed-resource-attributes

This is not handled anywhere in the bridge and I suspect is the root cause for pulumi/pulumi-azure#1881 and the panic in #1725 (comment)

Example

const resourceGroup = new azure.core.ResourceGroup("test", {location: "westus2"});

const namespace = new eventhub.EventHubNamespace("test", {
    resourceGroupName: resourceGroup.name,
    sku: "Standard",
    networkRulesets: {
         defaultAction: "Allow",
         ipRules: [{
             action: "Allow",
             ipMask: "0.0.0.0/0",
         }],
     },
});

Output of pulumi about

tested with azure 5.69 which uses 3.77 of the bridge

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
Copy link
Contributor Author

VenelinMartinov commented Mar 22, 2024

Really hard to make progress here without sufficient tests. Very large room for regressions around this.

This would be easier with #1766 but it'd still need some work as TF JSON syntax might have limitations related to this issue: https://developer.hashicorp.com/terraform/language/attr-as-blocks#in-json-syntax

@t0yv0
Copy link
Member

t0yv0 commented Mar 22, 2024

Can you try to specify a test a long the lines of

#1736

This should be doable but needs a little bit of thinking.

We can maybe check that ConfigureFunc receives the exact same data under Pulumi and TF over a range of auto-generated schemes with SchemaConfigModeAttr variability, and that it emits similar warnings - though comparing emitted warnings can get a bit gnarly and need some massaging.

@VenelinMartinov
Copy link
Contributor Author

I had a quick go at it here #1784 and the create method seems to not get the same value between TF and pulumi but it might also be #1767.

I failed to repro #1767 in #1782, so not 100% sure what is happening. I'll have another go at all of this to sort it out next week.

Seems promising though

@t0yv0
Copy link
Member

t0yv0 commented Mar 23, 2024

I'd like to pair early next week to have a look together. This is going to be interesting.

@iwahbe iwahbe removed the needs-triage Needs attention from the triage team label Mar 25, 2024
VenelinMartinov added a commit that referenced this issue May 2, 2024
Related to #1762

Adds support for config mode in the rapid-generated schemas.

Stacked on #1908
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jun 5, 2024
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/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants