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

Ensure # Configuration section of README.md is up to date #1585

Closed
iwahbe opened this issue Dec 19, 2023 · 11 comments · Fixed by pulumi/ci-mgmt#862
Closed

Ensure # Configuration section of README.md is up to date #1585

iwahbe opened this issue Dec 19, 2023 · 11 comments · Fixed by pulumi/ci-mgmt#862
Assignees
Labels
area/docsgen Issues with docs capture or example rendering, historically part of pkg/tfgen kind/enhancement Improvements or new features resolution/fixed This issue was fixed

Comments

@iwahbe
Copy link
Member

iwahbe commented Dec 19, 2023

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Each bridged provider has a configuration section for setting up the provider. This is manually copied from TF, and not automatically updated. It looks like this (for auth0):

Screenshot 2023-12-19 at 9 00 00 AM

As part of an update, the bridge should re-derive the configuration section from schema.json and update README.md. This is necessary to keep our provider documentation up to date in a world of automatic updates.

Affected area/feature

@iwahbe iwahbe added needs-triage Needs attention from the triage team kind/enhancement Improvements or new features labels Dec 19, 2023
@iwahbe iwahbe added area/docsgen Issues with docs capture or example rendering, historically part of pkg/tfgen and removed needs-triage Needs attention from the triage team labels Dec 20, 2023
@t0yv0
Copy link
Member

t0yv0 commented Dec 21, 2023

Could we just direct to registry where this information is already up-to-date by construction? And delete from README.

@iwahbe
Copy link
Member Author

iwahbe commented Dec 21, 2023

Could we just direct to registry where this information is already up-to-date by construction? And delete from README.

I don't see where this information is in the registry, but I agree that the registry is the natural place for it.

@t0yv0
Copy link
Member

t0yv0 commented Dec 21, 2023

For GCP it's linked here:
https://www.pulumi.com/registry/packages/gcp/installation-configuration/

For Auth0 it's harder to find it seems but it's found here:
https://www.pulumi.com/registry/packages/auth0/api-docs/provider/

@iwahbe
Copy link
Member Author

iwahbe commented Dec 21, 2023

I think it needs to be easy to find and speak directly to the Pulumi..yaml configuration before we can swap out. For GCP, that is hand written (and probably out of date).

@t0yv0
Copy link
Member

t0yv0 commented Dec 21, 2023

Sadness.. you're probably right.

@iwahbe
Copy link
Member Author

iwahbe commented Dec 27, 2023

Related to this, it would be nice to surface warnings across all providers such as pulumi/pulumi-hcloud#368. Right now this is impossible. I think it is generically worth it to let the bridge manage some section of the README.md. To allow a normal README.md to exist, we can surround the bridge section with some commented out fence.

<!-- pulumi-terraform-bridge: start -->
## Configuration

...


## Caveats

### Python

Right now, pulumi_${PROVIDER} does not support python 3.12. See ${ISSUE} for details.
<!-- pulumi-terraform-bridge: end -->

@t0yv0
Copy link
Member

t0yv0 commented Dec 27, 2023

Right now this is impossible

That's a nice use case indeed, good thinking there. Are you sure there's no GitHub mechanisms to communicate instead like pinned issues? Having files that combine auto generated sections with source-of-truth content is always a little tricky to explain, and since the bridge is used outside of our org there's always that, though of course we can keep such features flagged and internal.

@iwahbe
Copy link
Member Author

iwahbe commented Dec 27, 2023

Are you sure there's no GitHub mechanisms to communicate instead like pinned issues?

None that are great. We could write a script that would create an issue in each provider, then pin it, but that's not a great solution. More importantly, it would be a worse solution than sticking the warning in the README.md.

There are other places where we want to make bulk readme changes. For example, pulumi-hcloud mentions how to install in multiple languages (but not java). It doesn't link to it's registry entry.

@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Mar 26, 2024

This will not work for all provider. Ex. in pulumi-github:
https://github.com/pulumi/pulumi-github?tab=readme-ov-file#configuration

The configuration section has the env var names which can be used instead of the provider config but the schema does not: https://github.com/pulumi/pulumi-github/blob/632c46d96b5ca28d396de50d66beb44ba25b9bd3/provider/cmd/pulumi-resource-github/schema.json#L51

This means that if we auto-generate the Configuration section from the schema, we'd regress on this.

We might be able to fix this with a check on PRs, similar to what we have for breaking changes - all configuration options should have an entry in the README.

@VenelinMartinov VenelinMartinov self-assigned this Mar 26, 2024
@VenelinMartinov VenelinMartinov changed the title Automatically update the # Configuration section of README.md Ensure # Configuration section of README.md is up to date Mar 26, 2024
@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Mar 26, 2024

I got a CI check working in pulumi/pulumi-github#615 - it just checks that each config option is mentioned in the Configuration section.

I think it's simple and should help us keep these sections up to date. I don't see an automatic solution working without pulumi/registry#4749 since it is the upstream docs which have the detailed explanations of each option, which is not always in the schema.

I'll lift the check into ci-mgmt.

@iwahbe
Copy link
Member Author

iwahbe commented Mar 26, 2024

I got a CI check working in pulumi/pulumi-github#615 - it just checks that each config option is mentioned in the Configuration section.

I think it's simple and should help us keep these sections up to date. I don't see an automatic solution working without pulumi/registry#4749 since it is the upstream docs which have the detailed explanations of each option, which is not always in the schema.

I'll lift the check into ci-mgmt.

A CI check is a good place to start, though I continue to believe we will need to find a way to automate this in the future. This will help us calibrate how badly we are behind here.

VenelinMartinov added a commit to pulumi/ci-mgmt that referenced this issue Mar 26, 2024
fixes pulumi/pulumi-terraform-bridge#1585

This PR adds a check in bridged provider CI for configuration options in
README.md.
CI will now comment on PRs if any configuration options are missing in
the README.md

Tested in pulumi/pulumi-github#615 and
pulumi/pulumi-xyz#31
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docsgen Issues with docs capture or example rendering, historically part of pkg/tfgen kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants