Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

feat: add support for Bitbucket Server (SCM + PR) #506

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

feat: add support for Bitbucket Server (SCM + PR) #506

wants to merge 14 commits into from

Conversation

mlosicki
Copy link

Implementation details

Uses Basic auth instead of access tokens like the other SCMs. Note that Personal Access Tokens do work with this.

PRs do not support labels. Added a branchMatch field for PRs which allows you to filter based on the source branch name (e.g. only consider PRs that include -argocd)

Repos do indeed support labels, but the go-bitbucket-v1 library does not. It would have to be added upstream first.

Speaking of go-bitbucket-v1, version was increased as it has some bugfixes that were merged and are related to this PR (e.g. missing pagination options)

Closes #505

crenshaw-dev
crenshaw-dev suggested changes Feb 16, 2022
View reviewed changes
Copy link
Member

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

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

In the tests, can you replace assert.Nil(t, err) with assert.NoError(t, err) and context.TODO() with context.Background()? They're just semantic differences, but should read just a tiny bit nicer.

api/v1alpha1/applicationset_types.go Outdated Show resolved Hide resolved
api/v1alpha1/applicationset_types.go Outdated Show resolved Hide resolved
docs/Generators-Pull-Request.md Outdated Show resolved Hide resolved
docs/Generators-Pull-Request.md Outdated Show resolved Hide resolved
pkg/services/pull_request/bitbucket_server.go Outdated Show resolved Hide resolved
pkg/services/pull_request/bitbucket_server.go Outdated Show resolved Hide resolved
@mlosicki
feat: bitbucketServer for pull request generator
f9b76d7 
Discover pull requests from repositories in a Bitbucket Server (not the
same as Bitbucket Cloud). Private repos can be accessed via Basic
auth (password or personal access token).

Signed-off-by: mlosicki <[email protected]>
@mlosicki
feat: bitbucketServer for scmProvider
7f25b01 
Signed-off-by: mlosicki <[email protected]>
@mlosicki
fix: use 1.16 for deepcopy
3479c43 
Signed-off-by: mlosicki <[email protected]>
@mlosicki
docs: branchName for bitbucketServer PR + formatting
b2f0ccd 
Signed-off-by: mlosicki <[email protected]>
@mlosicki
docs: pr comments
51d3b6f 
Signed-off-by: mlosicki <[email protected]>
@mlosicki
fix: pr comments context + noError
f95bf8a 
Signed-off-by: mlosicki <[email protected]>
@mlosicki
fix: refactor for PR#472
4690633 
Signed-off-by: mlosicki <[email protected]>
@mlosicki
fix: pr comments, refactor bitbucket BasePath
216d0ee 
Signed-off-by: mlosicki <[email protected]>
@mlosicki
fix: pr comment, do not put full response in err
f198150 
Signed-off-by: mlosicki <[email protected]>
@mlosicki mlosicki force-pushed the feat/505-bitbucket-server-support branch from 64c0b59 to f198150 Compare February 28, 2022 15:33
crenshaw-dev
crenshaw-dev suggested changes Mar 2, 2022
View reviewed changes
api/v1alpha1/applicationset_types.go Outdated Show resolved Hide resolved
docs/Generators-SCM-Provider.md Outdated Show resolved Hide resolved
crenshaw-dev
crenshaw-dev reviewed Mar 11, 2022
View reviewed changes
Copy link
Member

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

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

Just clarifying questions at this point. Overall LGTM, will be happy to approve after clarifications!

pkg/services/scm_provider/bitbucket_server.go Outdated Show resolved Hide resolved
pkg/services/scm_provider/bitbucket_server.go Outdated Show resolved Hide resolved
pkg/services/scm_provider/bitbucket_server.go Show resolved Hide resolved
@crenshaw-dev
Copy link
Member

@mlosicki this morning got taken up by another review. But I should have a chance to do a last pass review no later than tomorrow morning.

crenshaw-dev
crenshaw-dev approved these changes Mar 17, 2022
View reviewed changes
Copy link
Member

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for your patience on the review. 🙂

@rishabh625
Copy link
Contributor

@mlosicki : Can u please move this PR into argocd as applicationset is moved. If u find it laborious, i can do it on your behalf with your consent

@rishabh625
Copy link
Contributor

This is merged we can close it

@simcax
Copy link

simcax commented May 6, 2022

@mlosicki - A question about this PR. I can see that Basic Auth is implemented. We are using ssh with privatekeys stored in secrets towards bitbucket server. Do you know if this would work, or would we still need to provide basic auth as well?

@mlosicki
Copy link
Author

mlosicki commented May 6, 2022

@mlosicki - A question about this PR. I can see that Basic Auth is implemented. We are using ssh with privatekeys stored in secrets towards bitbucket server. Do you know if this would work, or would we still need to provide basic auth as well?

@simcax No, it won't work. Bitbucket Server doesn't have an option to authenticate to the REST API via ssh keys. Those keys are only used for Git operations. So you do need Basic auth, preferably via personal access tokens.

@simcax
Copy link

simcax commented May 6, 2022

@mlosicki - A question about this PR. I can see that Basic Auth is implemented. We are using ssh with privatekeys stored in secrets towards bitbucket server. Do you know if this would work, or would we still need to provide basic auth as well?

@simcax No, it won't work. Bitbucket Server doesn't have an option to authenticate to the REST API via ssh keys. Those keys are only used for Git operations. So you do need Basic auth, preferably via personal access tokens.

Ah - of coures, that makes sense - although a bit of a nuisance having to create a service user of sorts. Thx!

@simcax
Copy link

simcax commented Jun 3, 2022

Has this PR been moved to applicationSet controller in ArgoCD, now that the applicationSet Controller is part of it?

@crenshaw-dev
Copy link
Member

Yep! argoproj/argo-cd#9049

@simcax
Copy link

simcax commented Jun 7, 2022

Yep! argoproj/argo-cd#9049

Cool!

@Lp-Francois Lp-Francois mentioned this pull request Jun 24, 2022
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@crenshaw-dev crenshaw-dev crenshaw-dev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Bitbucket Server
4 participants
@mlosicki @crenshaw-dev @rishabh625 @simcax