feat: Add support for language specific settings for resources #14308

tmeckel · 2023-10-22T19:31:13Z

Description

This PR contains changes to support language specific settings for resources. This PR is a prerequisite to resolve a corresponding bug in the Terraform Bridge.

Fixes pulumi/pulumi-terraform-bridge#1460

Checklist

I have run make tidy to update any new dependencies
I have run make lint to verify my code passes the lint check
I have formatted my code using gofumpt

I have added tests that prove my fix is effective or that my feature works

I have run make changelog and committed the changelog/pending/<file> documenting my change

Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

github-actions · 2023-10-22T19:31:32Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

pulumi-bot · 2023-10-22T19:32:11Z

Changelog

[uncommitted] (2023-11-29)

Features

[sdkgen/dotnet] Added support for language specific settings for resources and support for overriding resource name in dotnet codegen

github-actions · 2023-10-23T09:33:24Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

tmeckel · 2023-10-23T09:43:44Z

The docs generator has an issue where the resourceName method is not language specific and thus does not honor any language specific settings for a resource. Eventually the following panic is raised when trying to create SDK docs via registrygen

panic: fatal: An assertion has failed: duplicate file: router/bgp/_index.md

goroutine 1 [running]:
github.com/pulumi/pulumi/sdk/v3/go/common/util/contract.failfast(...)
        /home/vagrant/code/pulumi/sdk/go/common/util/contract/failfast.go:23
github.com/pulumi/pulumi/sdk/v3/go/common/util/contract.Assertf(0x30?, {0x1ae9498?, 0xc00d629530?}, {0xc000bcf040?, 0x48797e?, 0x1a24940?})
        /home/vagrant/code/pulumi/sdk/go/common/util/contract/assert.go:35 +0xed
github.com/pulumi/pulumi/pkg/v3/codegen.Fs.Add(...)
        /home/vagrant/code/pulumi/pkg/codegen/utilities.go:180
github.com/pulumi/pulumi/pkg/v3/codegen/docs.(*modContext).gen.func1({0x0, 0x0}, {0x1adb5af, 0xa}, {0x1a24940, 0xc00e3303c0})
        /home/vagrant/code/pulumi/pkg/codegen/docs/gen.go:1854 +0x225
github.com/pulumi/pulumi/pkg/v3/codegen/docs.(*modContext).gen(0xc000bf2aa0, 0xc000bede00)
        /home/vagrant/code/pulumi/pkg/codegen/docs/gen.go:1962 +0x11db
github.com/pulumi/pulumi/pkg/v3/codegen/docs.(*docGenContext).generatePackage(0xc0000c1400, {0x0?, 0x0?}, 0x0?)
        /home/vagrant/code/pulumi/pkg/codegen/docs/gen.go:2201 +0x2e8
github.com/pulumi/pulumi/pkg/v3/codegen/docs.GeneratePackage(...)
        /home/vagrant/code/pulumi/pkg/codegen/docs/gen.go:2255
github.com/pulumi/registrygen/pkg.generateDocsFromSchema({0x7ffeb02ae50a, 0xf}, 0xc4a256?)
        /home/vagrant/code/pulumi-registrygen/pkg/docs.go:218 +0x54
github.com/pulumi/registrygen/pkg.GenerateDocs({0x7ffeb02ae493?, 0x0?}, {0x7ffeb02ae4b8, 0x6}, {0x7ffeb02ae4cc, 0x30}, {0x7ffeb02ae50a, 0xf}, {0x7ffeb02ae532, 0xb}, ...)
        /home/vagrant/code/pulumi-registrygen/pkg/docs.go:111 +0x896
github.com/pulumi/registrygen/cmd/docs.PackageDocsCmd.func1(0xc0007ff800?, {0x1ad0743?, 0x9?, 0x9?})
        /home/vagrant/code/pulumi-registrygen/cmd/docs/cli.go:89 +0x74
github.com/spf13/cobra.(*Command).execute(0xc0007ff800, {0xc0006943f0, 0x9, 0x9})
        /home/vagrant/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:940 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc0007fe600)
        /home/vagrant/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3bd
github.com/spf13/cobra.(*Command).Execute(0xc000124120?)
        /home/vagrant/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992 +0x19
main.main()
        /home/vagrant/code/pulumi-registrygen/main.go:30 +0x8b

I think the type DocLanguageHelper interface should be extended with a method GetResourceName(modName string, f *schema.Resource) string as with GetFunctionName or GetModName.

tmeckel · 2023-10-24T20:18:30Z

So I had some time to further analyze the above shown panic output. This exception occurs when a module has the same name as a resource or function within it.

Example:

Module: Router Resource: Bgp: path for resource.tmpl file = router/bgp/_index.md
Module: Router/Bgp: path for module _index.md file = router/bgp/_index.md

The moduleConflictResolver is no help here because the files are generated with different module paths and thus the conflict resolver cannot detect the name collision.

@t0yv0 @iwahbe

iwahbe · 2023-10-24T21:20:42Z

/run-acceptance-tests
Please view the results of the acceptance tests Here

t0yv0 · 2023-10-25T02:17:55Z

I'll let the platform team pipe in but the bar for making changes to Pulumi package schema is very high I think. Is this needed or useful for non-bridged providers? If not can we fix it in the bridge please? The tokens are free form already and the bridge can assign them. There are hidden costs to extra degrees of freedom here, such as breaking pulumi convert code generation.

tmeckel · 2023-10-25T18:48:57Z

@t0yv0 Since I'm really working on the core of Pulumi, the hurdles are probably so high that no one will be willing to accept the changes. Nevertheless, I think it's helpful, if not say required, to have the possibility to define the name of a class/object differently as defined by the default name if you have to deeply nest namespaces inside an SDK of a provider to make the SDK easy to use and approachable. Right now even huge Pulumi providers like AWS and Azure put resources into namespaces one level deep. Now I know why: because structuring namespaces with more levels, causes errors while creating the SDK items itself or the documentation. As you can see by the panic I posted and my respective analysis. To make it clear, the panic has nothing to do with the changes I introduced in the PR. It's another bug inside the codegen packge in Pulumi.

Regarding you question if this could be handled isolated in the Bridge: I don't think so, because as with the CSharpName for properties of objects/resources the field must be set in the Bridge (if configured) but because tfgen calls into the codegen package to create the SDK and registrygen is using the same package to create the SDK documentation it must be implemented in Pulumi itself.

Aside that I could reproduce the error that failed the acceptance tests locally. Reason is that I must implement a custom Marshaler/Unmarshaler for YAML and the ResourceSpec struct. I thought I could go the easy way and omit that, but that wasn't the case. I'm working on this right now.

cc: @iwahbe

Frassle · 2023-10-25T22:17:20Z

Since I'm really working on the core of Pulumi, the hurdles are probably so high that no one will be willing to accept the changes.

This is true, we're actually on an internal hold for all codegen related work right now so the chance of this getting accepted soon is virtually nil.

Nevertheless, I think it's helpful, if not say required, to have the possibility to define the name of a class/object differently as defined by the default name if you have to deeply nest namespaces inside an SDK of a provider to make the SDK easy to use and approachable.

Personally I'd like to remove all these features from the schema but we're not doing that anytime soon. Given we support overrides in most places for most languages its at least consistent to support this as well.

Now I know why: because structuring namespaces with more levels, causes errors while creating the SDK items itself or the documentation.

Yes, it's not a well tested path. I don't know if all our providers went with non-nested namespaces because it was too buggy to do nesting or if they had other reasons and so we never invested in supporting nested namespaces.

Regarding you question if this could be handled isolated in the Bridge:

Agreed. Being a csharp codegen change it won't be possible to fix bridge side.

Reason is that I must implement a custom Marshaler/Unmarshaler for YAML and the ResourceSpec struct.

That surprises me. The other schema objects with language fields don't need this?

github-actions · 2023-10-27T17:28:08Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

iwahbe · 2023-10-27T19:29:48Z

/run-acceptance-tests
Please view the results of the acceptance tests Here

github-actions · 2023-10-28T08:55:42Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

iwahbe · 2023-11-02T00:33:29Z

/run-acceptance-tests
Please view the results of the acceptance tests Here

github-actions · 2023-11-27T22:51:14Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

Frassle

Having a careful look over this seems low risk and unblocks a tfbridge fix, so ✅

Frassle · 2023-11-28T08:46:51Z

Looks like a lint issue in one test file:
codegen/schema/schema_test.go:1379:8: syntax error: unexpected TestMarshalResourceWithLanguageSettings, expected ( (typecheck)

github-actions · 2023-11-29T13:09:45Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

…tings for ResourceSpec

…guageSettings

…pdated ImportResourceSpec to unmarshal raw data

…ceInfo in resourceName

…rpResourceInfo in resourceTypeName

…-for-language-specific-settings-for-resources-and-support-for-overriding-resource-name-in-dotnet-codegen.yaml

…kenToName to ensure type rename via CSharpResourceInfo is applied

…on struct ResourceSpec because it is already defined by the composite ObjectTypeSpec

…es because the messages are compared literally in acceptance tests.

github-actions · 2023-11-29T13:14:51Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

tmeckel · 2023-11-29T13:17:52Z

Looks like a lint issue in one test file: codegen/schema/schema_test.go:1379:8: syntax error: unexpected TestMarshalResourceWithLanguageSettings, expected ( (typecheck)

@Frassle I couldn't really make any sense of the error message, but a rebase to the master branch led to a collision. I resolved this first and updated the branch. Perhaps the error has now disappeared.

CC @iwahbe

Would be great if someone could approve the waiting workflows. 😀

Oh and of course another approval is required because I changed the branch by pushing.

This PR implements the missing part for PR pulumi/pulumi#14308 from the Pulumi SDK to have the ability to rename the generated C# (dotnet) class from it's provider resource name. This PR comes with an update of `github.com/pulumi/pulumi/pkg/v3` and `github.com/pulumi/pulumi/sdk/v3` to at v3.95.0 because this is the first version of Pulumi which implements the required code for the `dotnet` codegen to emit a renamed class using the `CSharpName` property. --------- Co-authored-by: Thomas Meckel <tmeckel@users.noreply.github.com> Co-authored-by: Anton Tayanovskyy <anton@pulumi.com>

tmeckel mentioned this pull request Oct 22, 2023

bug: CSharpName property on resource definition has no effect when building dotnet SDK pulumi/pulumi-terraform-bridge#1460

Closed

tmeckel changed the title ~~feat: Added support for language specific settings for resources~~ feat: Add support for language specific settings for resources Oct 22, 2023

t0yv0 requested a review from a team October 23, 2023 14:12

pulumi-bot added needs-triage Needs attention from the triage team labels Oct 25, 2023

Frassle approved these changes Nov 27, 2023

View reviewed changes

Frassle enabled auto-merge November 27, 2023 22:52

Frassle added this pull request to the merge queue Nov 27, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2023

tmeckel added 5 commits November 29, 2023 13:12

feat(codegen/schema/schema.go): add support for language specific set…

b4227d1

…tings for ResourceSpec

test(codegen/schema/schema_test.go): added TestMarshalResourceWithLan…

18323f1

…guageSettings

feat(codegen/dotnet/importer.go): added CSharpResourceInfo type and u…

f9bf731

…pdated ImportResourceSpec to unmarshal raw data

feat(codegen/dotnet/gen.go): override resource name from CSharpResour…

ed4a4ab

…ceInfo in resourceName

feat(codegen/dotnet/gen_program.go): override resource name from CSha…

39fa07e

…rpResourceInfo in resourceTypeName

tmeckel added 4 commits November 29, 2023 13:13

chore: added changelog/pending/20231022--sdkgen-dotnet--added-support…

ab029cb

…-for-language-specific-settings-for-resources-and-support-for-overriding-resource-name-in-dotnet-codegen.yaml

fix(codegen/dotnet/gen.go): using function resourceName instead of to…

c8489c2

…kenToName to ensure type rename via CSharpResourceInfo is applied

fix(codegen/schema/schema.go): removed double defined Language field …

b77227b

…on struct ResourceSpec because it is already defined by the composite ObjectTypeSpec

refactor(codegen/schema/schema.go): reverted changes to errors messag…

90c6e64

…es because the messages are compared literally in acceptance tests.

tmeckel force-pushed the feat/lang-settings-resource branch from acac2e3 to 90c6e64 Compare November 29, 2023 13:14

iwahbe enabled auto-merge November 29, 2023 16:30

iwahbe added this pull request to the merge queue Nov 29, 2023

Merged via the queue into pulumi:master with commit f83ee4a Nov 29, 2023
9 of 16 checks passed

tmeckel mentioned this pull request Dec 3, 2023

feat: Implement CSharpName property on provider resource pulumi/pulumi-terraform-bridge#1551

Merged

tmeckel mentioned this pull request Dec 11, 2023

bug(codegen): docs generator fails when nested module has same name as function or resource #14820

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for language specific settings for resources #14308

feat: Add support for language specific settings for resources #14308

tmeckel commented Oct 22, 2023 •

edited

github-actions bot commented Oct 22, 2023

pulumi-bot commented Oct 22, 2023 •

edited

github-actions bot commented Oct 23, 2023

tmeckel commented Oct 23, 2023 •

edited

tmeckel commented Oct 24, 2023

iwahbe commented Oct 24, 2023 •

edited by pulumi-bot

t0yv0 commented Oct 25, 2023

tmeckel commented Oct 25, 2023 •

edited

Frassle commented Oct 25, 2023

github-actions bot commented Oct 27, 2023

iwahbe commented Oct 27, 2023 •

edited by pulumi-bot

github-actions bot commented Oct 28, 2023

iwahbe commented Nov 2, 2023 •

edited by pulumi-bot

github-actions bot commented Nov 27, 2023

Frassle left a comment

Frassle commented Nov 28, 2023

github-actions bot commented Nov 29, 2023

github-actions bot commented Nov 29, 2023

tmeckel commented Nov 29, 2023 •

edited

feat: Add support for language specific settings for resources #14308

feat: Add support for language specific settings for resources #14308

Conversation

tmeckel commented Oct 22, 2023 • edited

Description

Checklist

github-actions bot commented Oct 22, 2023

pulumi-bot commented Oct 22, 2023 • edited

Changelog

[uncommitted] (2023-11-29)

Features

github-actions bot commented Oct 23, 2023

tmeckel commented Oct 23, 2023 • edited

tmeckel commented Oct 24, 2023

iwahbe commented Oct 24, 2023 • edited by pulumi-bot

t0yv0 commented Oct 25, 2023

tmeckel commented Oct 25, 2023 • edited

Frassle commented Oct 25, 2023

github-actions bot commented Oct 27, 2023

iwahbe commented Oct 27, 2023 • edited by pulumi-bot

github-actions bot commented Oct 28, 2023

iwahbe commented Nov 2, 2023 • edited by pulumi-bot

github-actions bot commented Nov 27, 2023

Frassle left a comment

Choose a reason for hiding this comment

Frassle commented Nov 28, 2023

github-actions bot commented Nov 29, 2023

github-actions bot commented Nov 29, 2023

tmeckel commented Nov 29, 2023 • edited

tmeckel commented Oct 22, 2023 •

edited

pulumi-bot commented Oct 22, 2023 •

edited

tmeckel commented Oct 23, 2023 •

edited

iwahbe commented Oct 24, 2023 •

edited by pulumi-bot

tmeckel commented Oct 25, 2023 •

edited

iwahbe commented Oct 27, 2023 •

edited by pulumi-bot

iwahbe commented Nov 2, 2023 •

edited by pulumi-bot

tmeckel commented Nov 29, 2023 •

edited