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(gitprovider): add Azure DevOps support and update provider confi… #3128

Merged
merged 24 commits into from
Dec 19, 2024

Conversation

diegocaspi
Copy link
Contributor

@diegocaspi diegocaspi commented Dec 12, 2024

…gurations

  • Introduced Azure DevOps as a supported Git provider in the application.
  • Updated configuration schemas to include 'azure' in the list of supported providers.
  • Added Azure-specific provider implementation with functionality for creating and managing pull requests.
  • Included tests for Azure repository URL parsing.

This enhancement allows users to interact with Azure DevOps repositories alongside existing GitHub and GitLab support.

Closes #3120

@diegocaspi diegocaspi requested a review from a team as a code owner December 12, 2024 16:03
Copy link

netlify bot commented Dec 12, 2024

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit 0a0cba8
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-io/deploys/67633595caee5e000819a966
😎 Deploy Preview https://deploy-preview-3128.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

go.mod Outdated
@@ -112,6 +112,7 @@ require (
github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect
github.com/lib/pq v1.10.9 // indirect
github.com/mattn/go-runewidth v0.0.9 // indirect
github.com/microsoft/azure-devops-go-api/azuredevops/v7 v7.1.0 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Something looks off here. This looks as if it should be a direct dependency. You may need to run go mod tidy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved. The only weird thing is that it also removed github.com/xanzy/go-gitlab

Copy link
Member

Choose a reason for hiding this comment

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

Ya... I see and that is strange. I'll take a look.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like main stopped using github.com/xanzy/go-gitlab just a few days ago and go mod tidy should have been run at that time and seems it wasn't.

#3146

So when you ran it, it fixed this.

All is well.

diegocaspi and others added 17 commits December 18, 2024 19:02
…gurations

- Introduced Azure DevOps as a supported Git provider in the application.
- Updated configuration schemas to include 'azure' in the list of supported providers.
- Added Azure-specific provider implementation with functionality for creating and managing pull requests.
- Included tests for Azure repository URL parsing.

This enhancement allows users to interact with Azure DevOps repositories alongside existing GitHub and GitLab support.

Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: Mayursinh Sarvaiya <[email protected]>
Signed-off-by: Diego Caspi <[email protected]>
…kuity#3130)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
…/hack/tools (akuity#3134)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: Justin Marquis <[email protected]>
Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Faeka Ansari <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Faeka Ansari <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
…n /hack/tools in the go-minor group (akuity#3145)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Diego Caspi <[email protected]>
)

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Mayursinh Sarvaiya <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Mayursinh Sarvaiya <[email protected]>
Signed-off-by: Diego Caspi <[email protected]>
@diegocaspi diegocaspi force-pushed the ado-promotion-steps-support branch from 089db6e to 8e3af16 Compare December 18, 2024 18:07
@diegocaspi diegocaspi requested a review from a team as a code owner December 18, 2024 18:07
internal/gitprovider/azure/azure.go Outdated Show resolved Hide resolved
internal/gitprovider/azure/azure.go Outdated Show resolved Hide resolved
internal/gitprovider/azure/azure.go Outdated Show resolved Hide resolved
@krancour
Copy link
Member

Whoa... you may want to roll back to ccc05b5 and rebase on main instead of merging main into your branch.

It's tough to zero in on the changes here.

@krancour
Copy link
Member

This looks pretty good overall. I don't see any major problems. Once this is all set, I'll give it another once over.

…e error messages

- Removed dependency on slices package and replaced it with string comparisons for host suffixes.
- Updated error messages for better clarity.
- Refactored URL parsing logic to clearly distinguish between modern and legacy Azure DevOps URLs.
- Removed unused dependency from go.mod and updated go.sum accordingly.

Signed-off-by: Diego Caspi <[email protected]>
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 15.43624% with 126 lines in your changes missing coverage. Please review.

Project coverage is 51.07%. Comparing base (b75ef48) to head (0a0cba8).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
internal/gitprovider/azure/azure.go 15.43% 125 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3128      +/-   ##
==========================================
- Coverage   51.27%   51.07%   -0.21%     
==========================================
  Files         285      286       +1     
  Lines       25706    25855     +149     
==========================================
+ Hits        13182    13205      +23     
- Misses      11824    11949     +125     
- Partials      700      701       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Diego Caspi <[email protected]>
@diegocaspi
Copy link
Contributor Author

I feel like rebasing to sign off that particular commit is not going to be easy, there are conflicts in a lot of steps of the rebase. Tomorrow I'll do it carefully :)

@krancour
Copy link
Member

@diegocaspi don't bother... you're ok now. It's no longer showing every diff from the 20 odd commits that you merged into your branch from main. All good.

@diegocaspi diegocaspi requested a review from krancour December 19, 2024 06:35
@krancour
Copy link
Member

@diegocaspi code looks solid. I'd normally test drive this as well, but I don't have a repo in ADO readily available to me at the moment. I assume you've see this work end-to-end?

@diegocaspi
Copy link
Contributor Author

diegocaspi commented Dec 19, 2024

yes, I ran an end-to-end test with both git-open-pr and git-wait-for-pr steps and it was working as expected. Haven't tested with the latest changes yet (pr comments fixes), I'll do it in some hours :)

@krancour
Copy link
Member

Much appreciated @diegocaspi!

@diegocaspi
Copy link
Contributor Author

shoud I write some more unit-tests? I see that the code coverage is under the threshold

@krancour
Copy link
Member

I see that the code coverage is under the threshold

That's mainly for informational purposes. We don't require it to pass.

Normally, I would look for more thorough test coverage, but unit testing this can't easily be done without mocking out several ADO API endpoints. A mistake in the mocked out endpoints can result in tests that pass, but shouldn't.

This is probably better suited to integration tests, but we'd either need a dedicated ADO repo perpetually available to test against or a way to provision one just-in-time and clean up afterwards. It's something we need to tackle eventually, but we've got to do it for GitHub, GitLab, and a few other external systems as well. I don't want this PR held up while work that out.

@fykaa
Copy link
Contributor

fykaa commented Dec 19, 2024

@diegocaspi you may need to sign your commits for this PR

@krancour
Copy link
Member

you may need to sign your commits for this PR

@fykaa he did. DCO failed because some unsigned commits that were already in main got merged into his branch.

We can overlook the DCO check for such a case.

@diegocaspi
Copy link
Contributor Author

just tested and everything looks fine

@krancour
Copy link
Member

No outstanding issues. LGTM.

Thanks @diegocaspi!

@krancour krancour merged commit d551ab8 into akuity:main Dec 19, 2024
14 of 17 checks passed
fykaa added a commit to fykaa/kargo that referenced this pull request Dec 20, 2024
akuity#3128)

Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: Diego Caspi <[email protected]>
Signed-off-by: Mayursinh Sarvaiya <[email protected]>
Signed-off-by: Hidde Beydals <[email protected]>
Signed-off-by: Faeka Ansari <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Justin Marquis <[email protected]>
Signed-off-by: Kent Rancourt <[email protected]>
Co-authored-by: Mayursinh Sarvaiya <[email protected]>
Co-authored-by: Hidde Beydals <[email protected]>
Co-authored-by: Faeka Ansari <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Justin Marquis <[email protected]>
Co-authored-by: Kent Rancourt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure DevOps integration for git related promotion steps
6 participants