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

Fix UnauthorizedError on GitHub Enterprise publish #740

Merged
merged 7 commits into from
Feb 24, 2023

Conversation

necosta
Copy link
Contributor

@necosta necosta commented Feb 10, 2023

When gitHostingURL is set, we see the following error:

Error making request to GitHub
 | => bat microsites.github.GitHubOps.$anonfun$run$1(GitHubOps.scala:208)
	at uncancelable @ org.http4s.client.ConnectionManager$.pool(ConnectionManager.scala:83)
	at uncancelable @ org.http4s.client.ConnectionManager$.pool(ConnectionManager.scala:83)
	at flatMap @ org.http4s.client.PoolManager.releaseRecyclable(PoolManager.scala:225)
	at uncancelable @ org.http4s.client.ConnectionManager$.pool(ConnectionManager.scala:83)
	at unsafeRunSync @ microsites.MicrositeAutoImportSettings.$anonfun$micrositeTasksSettings$14(MicrositeKeys.scala:496)
	at withPermit @ org.http4s.client.PoolManager.shutdown(PoolManager.scala:370)
	at uncancelable @ org.http4s.client.ConnectionManager$.pool(ConnectionManager.scala:83)
	at withPermit @ org.http4s.client.PoolManager.shutdown(PoolManager.scala:370)
	at guarantee$extension @ org.http4s.client.blaze.Http1Connection.$anonfun$executeRequest$2(Http1Connection.scala:193)
	at map @ fs2.internal.CompileScope.$anonfun$close$9(CompileScope.scala:246)
Caused by: UnauthorizedError(Bad credentials, https://docs.github.com/rest)

We need to adjust GithubConfig base/authorize/accessToken URLs to contain the gitHostingURL host.


@TomyMeren , can you:

  1. Clone my repo (branch fix/github_enterprise)
  2. Run publishLocal (haven't checked but should be configured)
  3. On your repo, change resolver to include resolvers += Resolver.mavenLocal, adjust github4s version to this published version
  4. Compile, run and inform if it's now working

@necosta necosta changed the title WIP: Fix UnauthorizedError on GitHub Enterprise publish Fix UnauthorizedError on GitHub Enterprise publish Feb 13, 2023
fedefernandez
fedefernandez previously approved these changes Feb 13, 2023
Copy link
Contributor

@fedefernandez fedefernandez left a comment

Choose a reason for hiding this comment

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

Thanks @necosta. Please, @TomyMeren, let me know if this fixes the issue.

src/main/scala/microsites/MicrositeKeys.scala Outdated Show resolved Hide resolved
@necosta
Copy link
Contributor Author

necosta commented Feb 13, 2023

After reading google/go-github#958, adjusted baseUrl logic.

fedefernandez
fedefernandez previously approved these changes Feb 13, 2023
@necosta
Copy link
Contributor Author

necosta commented Feb 17, 2023

Added edge-case scenario for when micrositeGitHostingUrl is still pointing to public GitHub:

case Right(url) if url.getHost == "github.com" => GithubConfig.default

fedefernandez
fedefernandez previously approved these changes Feb 20, 2023
@TomyMeren
Copy link
Contributor

Good progress! @necosta.

The current error is this: NotFoundError(Not Found, https://docs.github.com/enterprise/3.4/rest)
After Debugging I've seem that it is throwing here

gh.gitData.getReference(owner, repo, s"heads/$branch", pagination = None, headers = headers)

It could be a problem with the method updateReference in the github4s repo but I could't find the https://docs.github.com/enterprise/3.4/rest value in the project

TomyMeren
TomyMeren previously approved these changes Feb 21, 2023
Copy link
Contributor

@TomyMeren TomyMeren left a comment

Choose a reason for hiding this comment

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

LGTM!

src/main/scala/microsites/MicrositeKeys.scala Outdated Show resolved Hide resolved
@necosta
Copy link
Contributor Author

necosta commented Feb 22, 2023

Thanks @TomyMeren for testing. I did mess up the PR when I last updated, will get the changes from the "patched" branch, adjust the tests, then I think we are good to merge. Will do it now.

@TomyMeren
Copy link
Contributor

TomyMeren commented Feb 22, 2023

When I publish the sbt-microsite project locally, for use in the project, version 1.4.1+1.....-SNAPSHOT is created. The latest release version is 1.4.2. Do you know why this is?

@necosta
Copy link
Contributor Author

necosta commented Feb 22, 2023

When I publish the sbt-microsite project locally, for use in the project, version 1.4.1+1.....-SNAPSHOT is created. The latest release version is 1.4.2. Do you know why this is?

Yes, publishLocal is configured to build a "SNAPSHOT" version and not a "release" version. It should work this way to avoid any version conflict.

@necosta
Copy link
Contributor Author

necosta commented Feb 22, 2023

Green CI. It's ready for a final review @fedefernandez , feel free to merge after you approve the PR. Thank you @TomyMeren for testing it.

@TomyMeren
Copy link
Contributor

When I publish the sbt-microsite project locally, for use in the project, version 1.4.1+1.....-SNAPSHOT is created. The latest release version is 1.4.2. Do you know why this is?

Yes, publishLocal is configured to build a "SNAPSHOT" version and not a "release" version. It should work this way to avoid any version conflict.

Sorry for my unclear comment.
I was talking about the difference between 1.4.1 and 1.4.2

@necosta
Copy link
Contributor Author

necosta commented Feb 22, 2023

When I publish the sbt-microsite project locally, for use in the project, version 1.4.1+1.....-SNAPSHOT is created. The latest release version is 1.4.2. Do you know why this is?

Yes, publishLocal is configured to build a "SNAPSHOT" version and not a "release" version. It should work this way to avoid any version conflict.

Sorry for my unclear comment. I was talking about the difference between 1.4.1 and 1.4.2

Ah I see. I tried now, I do get 1.4.2-SNAPSHOT when publishing locally. Perhaps you are missing the git tag?

Copy link
Contributor

@TomyMeren TomyMeren left a comment

Choose a reason for hiding this comment

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

Yes It was a missing tag
LGTM!

@necosta
Copy link
Contributor Author

necosta commented Feb 24, 2023

Green CI and all comments resolved. Is it ok to merge to master @fedefernandez ? thanks

@fedefernandez fedefernandez merged commit 02e9681 into 47degrees:main Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants