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

Support both Bitbucket Cloud and Server #3030

Merged

Conversation

goober
Copy link
Contributor

@goober goober commented Oct 21, 2020

Hey, I just made a Pull Request!

This pull request will add support for fetching files from Bitbucket Server.

Changes were required in an underlying library so merging this is still blocked by IonicaBizau/git-url-parse#113

Fixes #2735

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@goober goober force-pushed the feature/support-bitbucket-server branch from 1095868 to f95f5e6 Compare October 22, 2020 09:21
@goober goober marked this pull request as ready for review October 22, 2020 09:46
@goober goober requested a review from a team as a code owner October 22, 2020 09:46
@benjdlambert
Copy link
Member

Hey @goober! Thanks for the contribution and welcome to the community!

Ideally, it would be great if we could wait for your upstream changes to be merged first rather than using your own fork.

There's also some conflicts that could do with fixing too!

Thanks for your work! 🏅

Copy link
Member

@freben freben left a comment

Choose a reason for hiding this comment

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

cool stuff!

packages/backend-common/src/reading/BitbucketUrlReader.ts Outdated Show resolved Hide resolved
packages/backend-common/src/reading/BitbucketUrlReader.ts Outdated Show resolved Hide resolved
packages/backend-common/src/reading/BitbucketUrlReader.ts Outdated Show resolved Hide resolved
packages/backend-common/src/reading/BitbucketUrlReader.ts Outdated Show resolved Hide resolved
@goober
Copy link
Contributor Author

goober commented Oct 22, 2020

@benjdlambert Thanks. Its a fast moving project.. I will rebase against master once again.

It seems that the upstream changes are about to be merged so we can definitely wait with merging this until they have released the new version. I just wanted to make my changes available so people who wants to take it for a spin is able to, and to get some feedback

@goober
Copy link
Contributor Author

goober commented Oct 22, 2020

After some further investigation it seems like the Bitbucket cloud api requires the username/appPassword combo for authentication, but also support an oauth2 setup.

However, since the two apis are so different maybe it is better to split them into two readers? Keep the current one and add a new BitbucketServerUrlReader @freben @Rugvip what do you think?

@goober goober force-pushed the feature/support-bitbucket-server branch from f95f5e6 to 166b3c1 Compare October 22, 2020 21:55
@Rugvip
Copy link
Member

Rugvip commented Oct 22, 2020

@goober configuration-wise I'd like to keep them in one reader, but we could split logic into two helper classes or smth of that sort. I think the GitHub one uses a similar pattern.

Regarding OAuth that may not support our use-case, as this is service to service authentication. Depends on what they implement though.

@goober goober force-pushed the feature/support-bitbucket-server branch from 166b3c1 to 6579769 Compare October 23, 2020 08:10
@goober goober force-pushed the feature/support-bitbucket-server branch from e8f11ab to 15b57a1 Compare October 23, 2020 08:30
@goober goober requested review from freben and Rugvip October 23, 2020 18:28
@goober goober requested a review from a team as a code owner October 24, 2020 14:20
@bryce-larson
Copy link

bryce-larson commented Oct 26, 2020

@goober thank you for add support for BitBucket Server.

I've tested your branch using a TOKEN for auth and it is working as expected.
Finding the apiBaseUrl not well document, but mine the URL I needed was https://<host>/rest/api/latest

Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@benjdlambert
Copy link
Member

@spotify/techdocs-core, could we also get a +1 :)

@benjdlambert benjdlambert merged commit 04d6f63 into backstage:master Oct 26, 2020
@goober goober deleted the feature/support-bitbucket-server branch October 26, 2020 13:30
@JdataNich
Copy link

Hi, is there an example of the what the auth or app-config should look like for the bitbucket.org server? thanks

@iamEAP
Copy link
Member

iamEAP commented Nov 18, 2020

Hey @JdataNich, you might get more 👀  eyes on your question if you bring it up in the #support channel in the Backstage discord.

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.

Add support for Bitbucket Server as a ReaderProcessor
8 participants