Add support for Bitbucket Self-Hosted URL

panoramix360 commented 1 year ago

Summary

This PR plans to create a new field on the Plugin Configurations to let the user set a self-hosted Bitbucket URL instead of only having the option to use the fixed one: https://bitbucket.org/.

Ticket Link

Fixes https://github.com/mattermost/mattermost/issues/24188

panoramix360 commented 1 year ago

hey @mickmister and @jprusch!

Just to let you guys know that I created the draft.

Things missing on this PR that I plan to implement:

[x] Create the docker-compose so it's easier to test the Self-Hosted Bitbucket URL, similar to this one.
[x] Add a new config to the Bitbucket Self-Hosted API URL
[x] Continue looking and analyzing if there is a place missing to change the BaseURL.

@mickmister Since you are probably more familiar with the code, I saw that on utils.go there are some utility methods to get an API baseURL that it's using this one https://api.bitbucket.org/2.0. I was thinking that maybe we will need to have this on configuration too? or am I wrong?

panoramix360 commented 1 year ago

hey @mickmister! thank you for your comments, fixed all of them :D

A question about the API Url that I mentioned, do you think we will need to create a second property on the Plugin Configuration to have the API Url configured?

I don't know if it's possible to infer these from the Self-Hosted URL only.

Please, let me know your thoughts.

mickmister commented 1 year ago

A question about the API Url that I mentioned, do you think we will need to create a second property on the Plugin Configuration to have the API Url configured?

@panoramix360 Yes this makes sense. We've done the same pattern for other plugins like the zoom plugin https://github.com/mattermost/mattermost-plugin-zoom/blob/de8de086a243bef494a2df4116c7e2a4e5ef155a/plugin.json#L37

panoramix360 commented 1 year ago

Thanks @mickmister! I'll add it!

I created the docker-compose.yml and I'm setting bitbucket up with it :D

mickmister commented 1 year ago

Awesome work with the docker setup @panoramix360 :tada:

panoramix360 commented 1 year ago

hey @mickmister!

I'm trying to test with the bitbucket locally, but I'm having some trouble in configuring it.

I followed the tutorial on README.md but it was not quite the same paths hehe so maybe we need to update it.

I think I configured everything:

Created an application link on Bitbucket using the OAuth URL
Configured the webhook secret on a repo

But it seems that the plugin is not being able to start it.

When I save a new config on the Mattermost Bitbucket page, I receive this log on the server:

{"timestamp":"2023-08-23 10:03:15.226 -03:00","level":"error","msg":"Unable to activate plugin","caller":"app/plugin.go:166","plugin_id":"bitbucket","error":"unable to start plugin: bitbucket: unable to generate plugin checksum: read plugins/bitbucket: is a directory"}
{"timestamp":"2023-08-23 10:03:15.228 -03:00","level":"debug","msg":"Advanced logging config not provided for notification logging","caller":"platform/config.go:152"}
{"timestamp":"2023-08-23 10:03:15.228 -03:00","level":"debug","msg":"Workers received config change.","caller":"jobs/workers.go:63"}
{"timestamp":"2023-08-23 10:03:15.237 -03:00","level":"error","msg":"Unable to activate plugin","caller":"app/plugin.go:166","plugin_id":"bitbucket","error":"unable to start plugin: bitbucket: unable to generate plugin checksum: read plugins/bitbucket: is a directory"}
{"timestamp":"2023-08-23 10:03:15.241 -03:00","level":"debug","msg":"Received HTTP request","caller":"web/handlers.go:163","method":"POST","url":"/api/v4/plugins/bitbucket/enable","request_id":"8n1h6kj81pfkjdxbozebj6gf8w","status_code":"200"}

Maybe I'm missing something.

I noticed that when I try to type /bitbucket on the chat, it's not appearing, so it's possible that the bitbucket plugin instance is not being able to run.

mickmister commented 1 year ago

@panoramix360 Do you happen to be using an M1 Mac or possibly linux ARM? This project is currently not updated to support M1 Mac. In order to fix this, we'll need to copy some lines over from the plugin starter template:

https://github.com/mattermost/mattermost-plugin-starter-template/blob/eef27c854b10ff55f9f4defaeac1329efdab94dd/Makefile#L79

https://github.com/mattermost/mattermost-plugin-starter-template/blob/eef27c854b10ff55f9f4defaeac1329efdab94dd/plugin.json#L16

Ideally we would synchronize the starter template project with this project, to make it more in sync than just these blocks here. But I'm fine making just this change for M1 with this for the purpose of this PR. Please let me know what you think :+1:

If this is indeed the issue for you, I'll create a ticket to update the wording of that error to be more obvious about the M1 issue.

panoramix360 commented 1 year ago

Nice catch!! yes, I'm using an M1 Mac! Thank you for that, I'd eventually waste a lot of time until I figured it out it was this haha

I'll update the files then in this PR, thanks!

panoramix360 commented 1 year ago

hey @mickmister!

I was able to make the plugin work and be enabled now, thanks for the tip!

I added the arm64 configuration and the new plugin property for the API Self-Hosted URL.

I already configured everything (or so I think haha), but it seems that the Webhook is not working correctly.

Can you try to test on your side as well?

I'm having the following error, I'm not sure is related but it seems so:

{"timestamp":"2023-08-28 20:40:44.766 -03:00","level":"debug","msg":"Failed to upgrade websocket connection.","caller":"web/context.go:113","path":"/api/v4/websocket","request_id":"eycszgz6eidzjebjmtrbc8fxnc","ip_addr":"127.0.0.1","user_id":"3y38567dk7yidf88hb6p9pma9w","method":"GET","err_where":"connect","http_code":400,"error":"connect: Failed to upgrade websocket connection., websocket: request origin not allowed by Upgrader.CheckOrigin"}
{"timestamp":"2023-08-28 20:40:44.766 -03:00","level":"debug","msg":"Received HTTP request","caller":"web/handlers.go:163","method":"GET","url":"/api/v4/websocket","request_id":"eycszgz6eidzjebjmtrbc8fxnc","status_code":"400"}

I didn't configure the Bitbucket API Self-Hosted URL yet on the Plugin Configurations, because I wasn't able to discover what is this URL for the Bitbucket locally. Since this property is probably just used to add links to the messages I believe it's not the problem.

I'll investigate more to try to tackle the error.

I think that the current code solution is already sufficient to solve the issue :D

It's just missing these improvements:

[x] Add a validation to the Self-Hosted URL inside the IsValid function, to check for a valid URL.
[x] Update tests that use the configurations
[x] Maybe create small tests for the added func
[ ] Maybe moving the getSelfHostedURL method to the configuration.go for better organization (Can you share your thoughts about this one?)

mickmister commented 1 year ago

Hi @panoramix360, I should have some time to test this tomorrow. Here are some thoughts on your last comment:

I'm having the following error, I'm not sure is related but it seems so:

I think those errors are unrelated to the plugin. Nothing to worry about.

I already configured everything (or so I think haha), but it seems that the Webhook is not working correctly.

Your Mattermost server will need to be reachable from the BitBucket server for webhook requests sent to Mattermost. In order to do this, you can use a tool like https://ngrok.com. Running ngrok http 8065 will give a URL that forwards to port 8065. Then you should set the Mattermost Site URL to that value, and use that as part of the webhook URL put into BitBucket. This also allows you to view webhook traffic at http://localhost:4040

I didn't configure the Bitbucket API Self-Hosted URL yet on the Plugin Configurations, because I wasn't able to discover what is this URL for the Bitbucket locally.

This article shows this as an example API URL:

https://bitbucket.example.com/rest/api/1.0/projects/JIRA/repos/jira/commits

So maybe it would be http://localhost:7990/rest/api/1.0 in the case of the local docker environment?

Since this property is probably just used to add links to the messages I believe it's not the problem.

I'm not sure what this means. If I understand correctly, this needs to be set for all API requests right?

Thanks so much for your efforts on this @panoramix360. Please let me know your thoughts on the above :+1:

mickmister commented 1 year ago

Also note that I submitted a PR to your branch to add some files related to development with Gitpod https://github.com/panoramix360/mattermost-plugin-bitbucket/pull/1

panoramix360 commented 1 year ago

hey @mickmister! Thank you for the PR and feedback!

I think those errors are unrelated to the plugin. Nothing to worry about.

Ok! Thank you!

Your Mattermost server will need to be reachable from the BitBucket server for webhook requests sent to Mattermost. In order to do this, you can use a tool like https://ngrok.com/. Running ngrok http 8065 will give a URL that forwards to port 8065. Then you should set the Mattermost Site URL to that value, and use that as part of the webhook URL put into BitBucket. This also allows you to view webhook traffic at http://localhost:4040/

I read something about Ngrok on the Mattermost Docs, I figured it could be something related to that. I already used Ngrok in the past, I'll install it and try to make the port forwarding, let's see. I think that's what I need to finally finish the testing.

This article shows this as an example API URL:

Oh nice find! I'll test this URL to see if it's correct and I'll get back to you.

I'm not sure what this means. If I understand correctly, this needs to be set for all API requests right?

Yep, you are right, I thought that the Bitbucket API was used on links inside plugin messages on the chat, but I'm wrong. Just looked through the code and actually, it uses the API URL to compose the URLs for pulling PRs and other data.

Just approved your PR :D! I'll wait for your tests as well then, ty!

panoramix360 commented 1 year ago

Just added the URL validation on the configuration.go file :D

With that, I believe it's not more a Draft 🕺

mickmister commented 1 year ago

Awesome thanks @panoramix360! From your testing, can you please describe what you've determined to be working, and what seems to not be working?

codecov-commenter commented 1 year ago

Codecov Report

Attention: 68 lines in your changes are missing coverage. Please review.

Comparison is base (a2702ef) 15.03% compared to head (e29edea) 15.17%. Report is 1 commits behind head on bitbucket-server.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## bitbucket-server #118 +/- ## ==================================================== + Coverage 15.03% 15.17% +0.13% ==================================================== Files 13 13 Lines 2301 2346 +45 ==================================================== + Hits 346 356 +10 - Misses 1936 1969 +33 - Partials 19 21 +2 ``` | [Files](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) | Coverage Δ | | |---|---|---| | [server/manifest.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL21hbmlmZXN0Lmdv) | `100.00% <ø> (ø)` | | | [server/api.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL2FwaS5nbw==) | `6.78% <50.00%> (ø)` | | | [server/subscriptions.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3N1YnNjcmlwdGlvbnMuZ28=) | `13.12% <0.00%> (ø)` | | | [server/webhook.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3dlYmhvb2suZ28=) | `0.00% <0.00%> (ø)` | | | [server/command.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL2NvbW1hbmQuZ28=) | `6.34% <0.00%> (-0.03%)` | :arrow_down: | | [server/utils.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3V0aWxzLmdv) | `16.66% <16.66%> (-1.37%)` | :arrow_down: | | [server/configuration.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL2NvbmZpZ3VyYXRpb24uZ28=) | `17.91% <0.00%> (-6.58%)` | :arrow_down: | | [server/plugin.go](https://app.codecov.io/gh/mattermost/mattermost-plugin-bitbucket/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3BsdWdpbi5nbw==) | `5.47% <26.66%> (+2.34%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

panoramix360 commented 1 year ago

hey @mickmister!

I'm going to test the bitbucket connection and try to test many hooks, especially the ones that touch the code that I changed.

Let me know if you think we should test other things.

panoramix360 commented 1 year ago

hey @mickmister

Committed some code to update the code that we managed to find out about the Bitbucket Server on our talk!

Can you take a look?

I tried to do some testing today and tried other OAuth Scopes but I didn't manage to make it work, unfortunately.

I found this doc, but it's not with the Bitbucket Server scopes reference.

If you have any tips, just let me know!

mattermost-build commented 1 year ago

This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!

mickmister commented 1 year ago

@panoramix360 It looks like there are a few linting errors. Can you please take a look at the errors below? We can merge once these are solved 🚀

Error: server/api.go:158:28: unused-parameter: parameter 'c' seems to be unused, consider removing or renaming it as _ (revive)
func (p *Plugin) ServeHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Request) {
                           ^
Error: server/command.go:323:82: unused-parameter: parameter 'userInfo' seems to be unused, consider removing or renaming it as _ (revive)
func (p *Plugin) handleHelp(_ *plugin.Context, _ *model.CommandArgs, _ []string, userInfo *BitbucketUserInfo) string {
                                                                                 ^
make: *** [Makefile:53: check-style] Error 1
Error: Process completed with exit code 2.

panoramix360 commented 1 year ago

@mickmister The first lint problem I fixed.

The second one is trickier because it's added on a map with a type that expects this third argument, what we can do in this case?

EDIT: Fixed with an underscore haha sorry I'm relatively new with GoLang Lint

mattermost-build commented 12 months ago

Will try to auto merge this PR once all tests and checks are passing. This might take up to an hour.

mickmister commented 12 months ago

Thanks @panoramix360! We've merged the PR into the bitbucket-server branch to continue work there

mattermost-community / mattermost-plugin-bitbucket