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

Use correct base url for gh auth token --hostname #1898

Merged
merged 4 commits into from
Oct 4, 2023

Conversation

nfx
Copy link
Contributor

@nfx nfx commented Sep 15, 2023

This PR is a follow-up to #1854

Before the change?

GitHub CLI uses different base URLs in ~/.config/gh/hosts.yml, so when we're using the standard base path of this provider, it doesn't align with how gh CLI stores the credentials - I think some of the recent CLI changes made this effect (or gh was not updated for a while on my machine). The following doesn't work:

$ gh auth token --hostname api.github.com
> no oauth token

... but the following does work correctly

$ gh auth token --hostname github.com
> gh..<valid token>

attempted a workaround with

provider "github" {
  owner = "..."
  base_url = "github.com"
}

But it didn't work as expected.

After the change?

provider conf works:

provider "github" {
  owner = "..."
}

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

This PR is a follow-up to integrations#1854

GitHub CLI uses different base URLs in `~/.config/gh/hosts.yml`, so when we're using the standard base path of this provider, it doesn't align with how `gh` CLI stores the credentials. The following doesn't work:

```
$ gh auth token --hostname api.github.com
> no oauth token
```
... but the following does work correctly

```
$ gh auth token --hostname github.com
> gh..<valid token>
```

attempted a workaround with

```
provider "github" {
  owner = "..."
  base_url = "github.com"
}
```

But it didn't work as expected.
@nfx
Copy link
Contributor Author

nfx commented Sep 15, 2023

@nickfloyd @kfcampbell please review :)

@nickfloyd
Copy link
Contributor

Hey @nfx This was addressed here in this PR.

I'm going to close this now that the other has been merged in. Thank you for working to make our community better!

@nickfloyd nickfloyd closed this Sep 22, 2023
@nickfloyd nickfloyd added the Type: Bug Something isn't working as documented label Sep 22, 2023
@nickfloyd nickfloyd reopened this Sep 22, 2023
@nickfloyd
Copy link
Contributor

We're still working through the solution over here - join the conversation if you have time!

@nfx
Copy link
Contributor Author

nfx commented Sep 23, 2023

Let's see..,

@nfx
Copy link
Contributor Author

nfx commented Oct 4, 2023

@nickfloyd are we going to merge this with comments or close it? :) recent version picks up the token nicely, btw

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the additional effort here @nfx. I'm going to get this merged in and released but I think I am going to have another look at the logic here and see if I can simplify it and/or determine if there are any more edge cases that we could've missed.

Thanks again for everything here!

@nickfloyd nickfloyd merged commit 28c27a5 into integrations:main Oct 4, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants