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

Bug: tfgen does not include descriptions for nested config fields #1045

Closed
guineveresaenger opened this issue Apr 27, 2023 · 1 comment · Fixed by #1650
Closed

Bug: tfgen does not include descriptions for nested config fields #1045

guineveresaenger opened this issue Apr 27, 2023 · 1 comment · Fixed by #1650
Assignees
Labels
area/docs Improvements or additions to docs for this repo area/docsgen Issues with docs capture or example rendering, historically part of pkg/tfgen area/tfgen Issues in pkg/tgen, excluding docs generation - use area/docsgen for those customer/feedback Feedback from customers kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@guineveresaenger
Copy link
Contributor

What happened?

A user reported missing examples for the Docker provider configuration and I noticed that even the descriptions were missing.

After failing to create an upstream patch against an index.md file, I realized that unlike for Resources/Data Sources, for Provider.Config, we read descriptions (and only descriptions) from the TF schema directly.

The Description fields exist for the nested resource; we are just not consuming them.

Expected Behavior

I expected nested schema descriptions to be read into the provider schema correctly.

Steps to reproduce

  1. Patch upstream/internal/provider/provider.go on the Docker provider - edit the description for ca_material, and edit the one for registry_auth:address
  2. With the new patch in place, run make tfgen
  3. Observe caMaterial having an updated description, while registryAuth.Adddress has no description at all.

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

@guineveresaenger guineveresaenger added needs-triage Needs attention from the triage team kind/bug Some behavior is incorrect or out of spec labels Apr 27, 2023
@guineveresaenger guineveresaenger added area/docs Improvements or additions to docs for this repo area/tfgen Issues in pkg/tgen, excluding docs generation - use area/docsgen for those area/docsgen Issues with docs capture or example rendering, historically part of pkg/tfgen labels Apr 27, 2023
@jazzyfresh jazzyfresh removed the needs-triage Needs attention from the triage team label Apr 28, 2023
@jazzyfresh
Copy link
Contributor

Thanks for noticing this! Adding it to the broken docs epic https://github.com/pulumi/home/issues/2738

@t0yv0 t0yv0 added the customer/feedback Feedback from customers label Jan 4, 2024
@guineveresaenger guineveresaenger self-assigned this Jan 18, 2024
guineveresaenger added a commit that referenced this issue Jan 31, 2024
This pull request aims to improve accuracy for nested property docs
descriptions.

It uses the SchemaMap to look up and read `Description` fields [such as
this
one](https://github.com/kreuzwerker/terraform-provider-docker/blob/master/internal/provider/provider.go#L111)
in the case where trying to obtain documentation form `entityDocs` does
not yield any results.

The original impetus for this change was to address #1045, which is very
specific to Config() fields. The Provider configuration docs are
special-cased in the bridge and we do not have access to any
`entityDocs`. Instead, we were reading in the TF schema's Description()
field into `rawdocs` when generating a top-level config variable. This
did not persist into any nested fields on Configuration variables, hence
this fix. But it turns out that we can catch quite a few more missing
descriptions if we extend implementation across all nested fields (see
stats below).

As a side note - the Terraform Plugin Framework implements
`Description()` in a way that respects and prioritizes descriptions that
come from Markdown docs; for `sdkv2` resources we still need to use, and
prefer, our parsed-from-markdown `entityDocs` descriptions. It would be
interesting to see if we get improved docs for TFPF resources by not
reading descriptions from `entityDocs`.
It was also necessary to return an empty string from TFPF `typeSchema`
so as not to cause a panic during schemagen on TFPF using providers.
Open to improvements/suggestions here!

This PR aims to be fully additive - we are not changing the way in which
docs are generated; we only fill in missing docs.
Nevertheless, this PR does also seem to improve the correctness of
nested docs where some description fields have been matched to the wrong
input field.

Please see a [new Docker schema generated against these
changes](https://github.com/pulumi/pulumi-docker/blob/nested-configs-sample-schema/provider/cmd/pulumi-resource-docker/schema.json)
Quick stats:
```
pulumi/pulumi-docker🦉 git diff master --stat -- provider/cmd
 provider/cmd/pulumi-resource-docker/schema.json | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------
 1 file changed, 68 insertions(+), 43 deletions(-)
 ```

Here is [an AWS schema](https://github.com/pulumi/pulumi-aws/blob/testing-docsgen/provider/cmd/pulumi-resource-aws/schema.json) - you will want to pull that branch and look at the git diff to `master` locally.
Quick stats:
```
pulumi/pulumi-aws🦉 git diff master --stat -- provider/cmd 
provider/cmd/pulumi-resource-aws/schema.json | 1810
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 1206 insertions(+), 604 deletions(-)
 ```

This PR fixes #1045.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Improvements or additions to docs for this repo area/docsgen Issues with docs capture or example rendering, historically part of pkg/tfgen area/tfgen Issues in pkg/tgen, excluding docs generation - use area/docsgen for those customer/feedback Feedback from customers 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