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

Fix urn name clash for ReservedIp resource in DigitalOcean provider #515

Closed
wants to merge 2 commits into from

Conversation

lbialy
Copy link
Contributor

@lbialy lbialy commented Oct 26, 2023

I'm not sure if this solves the whole thing but it seems all the other DO resources that have an URN field use this trick to rename it in generated schema.

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@t0yv0 t0yv0 requested a review from a team October 26, 2023 14:59
@mjeffryes
Copy link
Member

/run-acceptance-tests

@pulumi-bot
Copy link
Contributor

@guineveresaenger
Copy link
Contributor

Hi @lbialy - this is the current correct approach for disambiguation, so thank you for sending this PR. We would like to have a bit more background into this change.

Can you add a more detailed PR description as to why this disambiguation is necessary; and how the resource behaves without it? Is there an issue describing this as a bug, with a repro? Please also add the name of the resource to the PR title. :)

Thank you so much!

@lbialy lbialy changed the title Fix urn name clash in DigitalOcean provider Fix urn name clash for ReservedIp resource in DigitalOcean provider Nov 5, 2023
@lbialy
Copy link
Contributor Author

lbialy commented Nov 5, 2023

Hi Guinevere, I've created #528 with rationale for this change and added resource name to the name of the PR. I have no idea how ReservedIp works right now with the clash in other language sdks and I believe the behaviour depends on the implementation of given SDK strongly. There's #352 in which ReservedIp seems to create a problem and it kinda makes sense that urn clash could lead to this for instance. For pulumi-scala this leads to a) duplicate urn fields generated which crash in Scala compilation (no duplicate fields allowed by compiler) and b) to codegen crash in newer versions that detect duplicates including reserved keywords like urn and id.

@mikhailshilkov
Copy link
Member

@lbialy Thank you for your PR!

You would need to run the full codegen and see the change percolate into all language SDKs. make build should get you there. Let me know if you need help.

@lbialy
Copy link
Contributor Author

lbialy commented Nov 6, 2023

Would make build generate the new schema.json file? That's the only thing we need.

@iwahbe
Copy link
Member

iwahbe commented Nov 6, 2023

Would make build generate the new schema.json file? That's the only thing we need.

To regenerate the schema, you should use make tfgen. To regenerate the sdks, you would use make build_sdks.

Copy link

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@iwahbe iwahbe mentioned this pull request Nov 21, 2023
iwahbe added a commit that referenced this pull request Nov 22, 2023
Rebased copy of #515.

I have checked with Platform, and they confirm that `urn` and `id` are
reserved for their use.

---------

Co-authored-by: Łukasz Biały <[email protected]>
@iwahbe
Copy link
Member

iwahbe commented Nov 22, 2023

This merged as part of #547.

@iwahbe iwahbe closed this Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants