terraform-provider-github icon indicating copy to clipboard operation
terraform-provider-github copied to clipboard

Auth changes for v5

Open kfcampbell opened this issue 4 years ago • 1 comments

Part of #980.

TODO:

  • [ ] Can we do a lookup and obviate the need for GITHUB_TEST_ORGANIZATION?
  • [ ] Ensure skipUnlessMode does the right thing with organization, user, and anonymous tests
    • I don't think this is the case now since I'm seeing a bunch of requests to GET /orgs/kfcampbell HTTP/1.1 in the tests
  • [ ] Perform same array of testing matrix as I did with the main branch earlier

This WIP PR aims to:

  • Deprecate GITHUB_ORGANIZATION environment variable (and organization provider block configuration) in favor of GITHUB_OWNER
  • If no token is present, still process owner variables so that unauthenticated requests may be made against public org-owned repos
  • Improve error handling and messaging around missing tokens/owner configuration (currently it's pretty difficult to figure out this is a problem and act appropriately
  • Tweaks to avoid pointless requests (for example, when no token or owner is configured, we try a pointless request to e.g. api.github.com//example-repo, which obviously won't
  • Docs
    • Clarify how provider authentication works in the case where both the integrations and hashicorp providers are used (I keep returning to @tibbes's excellent comment here about it)
    • Specify in our docs exactly how the decision tree works for each form of authentication (app, provider PAT config, environment variable PAT config) and when/how they can be combined
    • Return an error in some cases where our requests fail but the provider doesn't return an error code

Note that there is another small behavior change: I doubt anyone is using this, and I don't think it's particularly good practice, but I believe it used to be possible to run actions on behalf of both an organization and an individual account, using the same token, in the same terraform file. After this v5.0.0 change is released, this will no longer be true, and setting different owners for a token will require different provider configuration. This change is intentional and (I think) a good thing.

kfcampbell avatar Nov 26 '21 19:11 kfcampbell

Will this address #977?

jcogilvie avatar Jan 05 '22 16:01 jcogilvie

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

nickfloyd avatar Nov 30 '22 16:11 nickfloyd

@kfcampbell if this is still valid then can you update the title to reference vNEXT to align with your new major version strategy? I understand that the branch name would look awkward but the title is misleading as there is already a v5.

arledesma avatar Jan 22 '23 03:01 arledesma

I hope that this feature will be available in 2023. 🥲

posquit0 avatar Jan 24 '23 14:01 posquit0

if this is still valid then can you update the title to reference vNEXT to align with your new major version strategy?

Done!

I hope that this feature will be available in 2023. :smiling_face_with_tear:

I apologize for the slow rate of our feature development here. If you'd like to take this branch and run with it, please feel free!

kfcampbell avatar Feb 03 '23 23:02 kfcampbell

👋 Hey Friends, this pull request has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

github-actions[bot] avatar Nov 01 '23 01:11 github-actions[bot]