docs: update GitHub App env-var auth guidance for v6.13.0 blockless behavior#3551
docs: update GitHub App env-var auth guidance for v6.13.0 blockless behavior#3551SebastianBienert wants to merge 4 commits into
Conversation
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
deiga
left a comment
There was a problem hiding this comment.
Please note that docs files are generated. You need to update the template files as well
docs/index.md is generated by tfplugindocs. Apply the same change to its
sources so generation stays idempotent:
- templates/index.md.tmpl: the environment-variable note
- examples/provider/app_auth_env/main.tf: inlined by {{ tffile }}
Addresses review feedback on integrations#3551.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
stevehipwell
left a comment
There was a problem hiding this comment.
I'm not sure I fully agree with these changes. The docs do need to be updated to show that all of the app auth config can be driven from environment variables and the app_auth block is not required. However setting the app_auth block if you explicitly want to use app auth is the recommended pattern as it errors if the env variables haven't been set, as is using the block for static configuration (e.g. app ID).
|
Thanks @deiga, updated. Pushed the same change to the sources: |
|
@stevehipwell reworked to address this. The note now states the |
stevehipwell
left a comment
There was a problem hiding this comment.
Thanks for the quick response @SebastianBienert.
LGTM
|
@SebastianBienert could you please rebase this PR? |
docs/index.md is generated by tfplugindocs. Apply the same change to its
sources so generation stays idempotent:
- templates/index.md.tmpl: the environment-variable note
- examples/provider/app_auth_env/main.tf: inlined by {{ tffile }}
Addresses review feedback on integrations#3551.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…it auth Address review feedback on the app_auth env-var guidance. Rather than framing the block as removable, present it as the recommended way to opt into GitHub App auth: its arguments are schema-Required, so terraform validate fails fast when a value is missing, and it accepts static non-secret values (e.g. app id). - templates/index.md.tmpl: concise note plus three subsections - examples/provider/app_auth_mixed/main.tf: static non-secret IDs, PEM from env - docs/index.md: regenerated via `go generate ./...` Verified in provider.go: getAppAuth reads GITHUB_APP_* directly and block values override them; nested id/installation_id/pem_file are Required. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7cafbb1 to
9e44b37
Compare
|
@stevehipwell done |
Resolves #3550
Before the change?
app_auth {}block is required when authenticating through theGITHUB_APP_XXXenvironment variables, and themain.tfexample shows that pattern.getAppAuth()reads the environment variables directly, before consulting the block.terraform validate/tofu validatein any environment where the variables are not set (CI linting, local pre-commit hooks), because theapp_autharguments are schema-required withEnvDefaultFunc. Details and reproduction in [DOCS]: Empty app_auth {} recommendation fails validate without credentials #3550.After the change?
app_authblock is needed for environment-variable auth, and that the previously documented empty block should be removed because it failsvalidatewithout the variables set.main.tfexample no longer shows an emptyapp_auth {}block.Pull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!