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

feat(scaffolder): add autocompletion for Bitbucket #25132

Merged

Conversation

benjidotsh
Copy link
Contributor

@benjidotsh benjidotsh commented Jun 11, 2024

This PR adds autocompletion to the scaffolder for Bitbucket to the workspace, project and repository fields. freeSolo is enabled on the repository field to allow entering a repository that doesn't exist yet.

This PR only makes sense whenever #24881 is merged, because autocompletion after filling in all the fields might not make a lot of sense 😅

I'm pretty sure there is some stuff that can still be improved, like the way the access token is passed to BitbucketRepoPicker and passing the config to BitbucketCloudClient using the integration API, but I was struggling to make it work, so any help is appreciated!

Scherm­afbeelding 2024-06-11 om 12 16 47

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@github-actions github-actions bot added the area:scaffolder Everything and all things related to the scaffolder project area label Jun 11, 2024
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Jun 11, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/integration packages/integration minor v1.12.0
@backstage/plugin-bitbucket-cloud-common plugins/bitbucket-cloud-common patch v0.2.20
@backstage/plugin-scaffolder-backend-module-bitbucket-cloud plugins/scaffolder-backend-module-bitbucket-cloud patch v0.1.9
@backstage/plugin-scaffolder-backend plugins/scaffolder-backend minor v1.22.9
@backstage/plugin-scaffolder-node plugins/scaffolder-node patch v0.4.5
@backstage/plugin-scaffolder-react plugins/scaffolder-react minor v1.9.0
@backstage/plugin-scaffolder plugins/scaffolder minor v1.21.0

@benjdlambert
Copy link
Member

Hey @benjidotsh thanks for this! Some nice steps in this PR.

Wanna run some thoughts by you though, and see what you think.

I was thinking about moving some of this logic to the backend, under maybe scaffolder-backend, which would allow re-use for all providers not just bitbucket, and abstract away some of the difficulties with populating without a usertoken too.

So for example you could do something like /scaffolder/v2/integrations/bitbucket/lookup with a postBody of the token or something, and if there isn't a user token provided it would use the token thats in the integrations config already server side to do the lookup. This would be good for example with GitHub apps or whatever, that are only installed in certain organizations and that's where they have permission to write.

#8047 and some of the linked issues is where this pops up a lot, so I'm thinking that if we want to solve this it might be worth the extra effort of making the implementation extenisble.

@benjidotsh
Copy link
Contributor Author

@benjdlambert I like the idea, but isn't this a security risk if an organization doesn't want you to see certain workspaces, projects or repositories?

Also, do you see this as a dynamic endpoint? Something like /scaffolder/v2/integrations/:name/:resource, name being bitbucket and resource being workspaces for example?

@benjdlambert
Copy link
Member

@benjidotsh yeah, for sure, but we also hook into permissions framework or something here to help us lock that down a little bit, and of course, totally opt out of using the sever side token entirely if you really wanna lock things down. Thinking that this is a little flexible.

And yeah, a little unclear of the API design at the minute. But something like that for sure might be useful, especially for being able to dynamically populating the second field based off the input from a previous one for example.

Might be worth thinking about how best to do it a bit, if you've got any ideas I'm all ears, but I'll also have a think.

@benjdlambert
Copy link
Member

benjdlambert commented Jun 11, 2024

@benjidotsh I was looking around at some old code I put together for doing something like this, and it initially started off as an integrations-backend plugin that you would install separately, but I think it's fine to assume that we can disregard that idea, and just keep this local to the scaffolder plugin for now, as the use cases don't creep outside the boundary of the scaffolder at this point at least.

master...blam/integrations-backend#diff-94e0a69b86f3657e380f24218183592af209dba534b9e7eeb8ea155279ba4403R1-R92

I went with something simple for github orgs in the first place, with /v1/github/orgs as the endpoint, which seems to line up with our thinking now of having /:provider/:resource, maybe we can start there and see where it get us.

You can also see that there was even config to disable the integrations.config lookup too, and just use the token from the user instead.

I'm thinking that it probably makes sense for us to have something similar, and for any endpoints which need filtering by a particular org, or project, we can pass that as a query string to the relevant resource path?

/github/orgs with my PAT would return a few, backstage being one of them.
/github/repos?org=backstage would return all repos under the backstage org with my PAT.

Still pretty high level of course, and interested to hear your thoughts too 🙏

@benjidotsh
Copy link
Contributor Author

@benjdlambert I like that! I'll see if I can get some time to work on this later this week.

Working on Backstage is a part of my job and I already spent quite some time on this, so considering it already works for our use case, I'm not sure about the priority of this one. But I'll do my best! 😄

@benjidotsh
Copy link
Contributor Author

@benjdlambert I have created a /v2/autocomplete/:provider/:resource endpoint that gets used in the frontend now.

I looked into falling back to the integration token, but I couldn't figure out how without bypassing the client, like in the commit you mentioned.

@benjidotsh benjidotsh requested review from a team as code owners June 14, 2024 11:01
@github-actions github-actions bot added the area:techdocs Related to the TechDocs Project Area label Jun 14, 2024
@benjidotsh benjidotsh force-pushed the feat/scaffolder-bitbucket-autocomplete branch from 38aab63 to 1114749 Compare June 14, 2024 11:12
@github-actions github-actions bot removed the area:techdocs Related to the TechDocs Project Area label Jun 14, 2024
…rt for access tokens to BitbucketCloudClient

Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
…ackstage/plugin-scaffolder-react; update API reports; update tests of ActionsPage

Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
…lly work

Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
@benjidotsh benjidotsh force-pushed the feat/scaffolder-bitbucket-autocomplete branch from 1114749 to 4d88b29 Compare June 14, 2024 11:18
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
@benjdlambert
Copy link
Member

@benjidotsh thanks for the additional effort here! Much appreciated!

Do you think this is ready now? I have some little bits that I wanna change, but don't want to clash if you've got any more work to push 🙏

@benjidotsh
Copy link
Contributor Author

@benjdlambert go ahead! I might look a little more into the tests, but I think the gist of it should be ready!

Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
@benjidotsh
Copy link
Contributor Author

benjidotsh commented Jun 16, 2024

@benjdlambert I added some extra tests and slightly improved the existing ones. Should be done now!

Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
Signed-off-by: Benjamin Janssens <benji.janssens@gmail.com>
@benjdlambert
Copy link
Member

Nice - I'm hoping to get some time later this week to make some small little tweaks to this! Thanks for the work! 🙏

@benjdlambert
Copy link
Member

@benjidotsh there's a bit left to do in terms of tests that I need to add, but I think the majority of the refactor is done. Could you confirm that it's still working as expected 🤞 I can't test locally as I don't have bitbucket, but a summary of the changes is that we've flipped the control around, so that modules can provide additional autocomplete resolvers.

The logic for the resolver as now moved into the bitbucket-cloud module instead, and I cleaned up some of the types to come up with a pattern for extensibility. For instance you could even return a logo or something as well as just a title for some use cases like org selection etc.

Also moved the token and params (now context) to the POST body instead of a GET just to protect for access logs leaking peoples tokens out etc, think it's a bit more secure that way.

Let me know if I've broken something massively 😂

@benjidotsh
Copy link
Contributor Author

I like it!

Maybe I'm missing something, but the endpoint returns Unsupported provider: bitbucket-cloud now for me.

Signed-off-by: blam <ben@blam.sh>
Signed-off-by: blam <ben@blam.sh>
Signed-off-by: blam <ben@blam.sh>
@benjidotsh
Copy link
Contributor Author

benjidotsh commented Jun 24, 2024

Disregard my previous comment - @benjdlambert reached out on Discord and pointed out that I need to import @backstage/plugin-scaffolder-backend-module-bitbucket-cloud into my backend. 🥳

Seems to be working perfectly now!

Signed-off-by: blam <ben@blam.sh>
Signed-off-by: blam <ben@blam.sh>
@benjdlambert
Copy link
Member

@benjidotsh let's try and get this in the -next release today 🎉

@benjdlambert benjdlambert merged commit a2f12dc into backstage:master Jun 25, 2024
34 checks passed
Copy link
Contributor

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.29.0 release, scheduled for Tue, 16 Jul 2024.

@benjidotsh benjidotsh deleted the feat/scaffolder-bitbucket-autocomplete branch June 25, 2024 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:scaffolder Everything and all things related to the scaffolder project area
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants