feat: Add Azure DevOps SCM Provider Generator; add branchNormalized to SCM Generator template fields. #9283

brinchm · 2022-05-03T11:34:37Z

Adds Azure DevOps SCM provider generator. For organizations that use Azure DevOps as their SCM, this will enable ApplicationSets to deploy applications based on repos and branches while in the development phase.

The pull request also contains an enhancement to SCMProviderGenerator.GenerateParams: branchNormalized. This aids in generating applications with unique names:

template:
    metadata: 
      name: "{{repository}}-{{branchNormalized}}{{sha}}"

Closes #9280
Signed-off-by: brinchm christer.brinchmann@gmail.com

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

codecov · 2022-05-03T11:46:42Z

Codecov Report

Merging #9283 (5a407bf) into master (a66046e) will increase coverage by 0.08%.
The diff coverage is 68.96%.

@@            Coverage Diff             @@
##           master    #9283      +/-   ##
==========================================
+ Coverage   45.76%   45.85%   +0.08%     
==========================================
  Files         225      226       +1     
  Lines       26565    26675     +110     
==========================================
+ Hits        12157    12231      +74     
- Misses      12751    12783      +32     
- Partials     1657     1661       +4

Impacted Files	Coverage Δ
...is/applicationset/v1alpha1/applicationset_types.go	`34.69% <ø> (ø)`
applicationset/generators/scm_provider.go	`37.64% <50.00%> (-2.62%)`	⬇️
...plicationset/services/scm_provider/azure_devops.go	`71.56% <71.56%> (ø)`
util/settings/settings.go	`48.16% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a66046e...5a407bf. Read the comment docs.

crenshaw-dev · 2022-05-03T17:41:42Z

Thanks for the PR @brinchm! Will review ASAP.

rishabh625 · 2022-05-03T18:27:44Z

applicationset/services/scm_provider/azure_devops_test.go

+			hasPath, err := provider.RepoHasPath(ctx, repo, path)
+
+			if testCase.returnError {
+				assert.Error(t, err)


curious! why we have this as a check? both the scenario error or noerror as passing test case?

I tried covering the cases where azure devops returns an error signaling that the requested path doesn't exist (which is fine; maybe the path isn't there in the specified branch, so the return is false, nil), and also where azure devops returns other errors. In that latter case, I expect RepoHasPath to actually return an error.

But if the intent of the test cases is not clear I can try rewriting the test cases or possibly doing a new "Azure Devops Failed" test.

Probably we should segregate it with two names, one positive and other negative

crenshaw-dev

This is awesome! Most of the comments are style nits. There are a couple comments that are more impactful.

applicationset/services/scm_provider/azure_devops.go

applicationset/services/scm_provider/azure_devops_test.go

docs/operator-manual/applicationset/Generators-SCM-Provider.md

applicationset/services/scm_provider/azure_devops.go

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

…getting default branch. Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

crenshaw-dev

Just a few more tiny nitpicks. @rishabh625 can you give this one a final look?

pkg/apis/applicationset/v1alpha1/applicationset_types.go

crenshaw-dev · 2022-06-02T21:01:38Z

applicationset/services/scm_provider/azure_devops.go

+		if err != nil {
+			if wrappedError, isWrappedError := err.(azuredevops.WrappedError); isWrappedError && wrappedError.TypeKey != nil {
+				if *wrappedError.TypeKey == AzureDevOpsErrorsTypeKeyValues.GitRepositoryNotFound {
+					return repos, nil


I wonder if this error should be bubbled up. Is this ignored because it applies to disabled repos? If it applies to more than than, should we log a warning?

The GitRepository returned from the Azure DevOps Golang library doesn't include info indicating if a repo is disabled, but the REST API it calls, does.

When retrieving branches for a disabled repo, the WrappedError contains the GitRepositoryNotFoundException text. For this situation, I chose to ignore the error and just return an empty slice.

I'll need to give this some thought; The GetRepositories call is quite simple, so a GET could be issued directly to the REST API... Might work, but it seems like a suboptimal solution.

Yeah, reinventing the wheel there would stink. I'm okay merging as-is. I don't think this will catch transient errors and accidentally delete Apps or anything nefarious like that.

Thanks :)
The feature branch has one conflict in go.mod and some 129 commits behind master. I'll do a merge and push asap.

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

crenshaw-dev

Thanks for seeing this through!

brinchm · 2022-06-15T15:27:11Z

Thank you, likewise! Happy to contribute :)

brinchm added 4 commits May 3, 2022 13:00

feat: add Azure DevOps SCM provider

cdf0503

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

feat: add branchNormalized scm provider parameter.

9fa5206

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

add Azure DevOps SCM provider docs.

e4826b2

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

add branchNormalized docs.

c9ae79e

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

crenshaw-dev self-requested a review May 3, 2022 17:41

rishabh625 reviewed May 3, 2022

View reviewed changes

crenshaw-dev requested changes May 10, 2022

View reviewed changes

crenshaw-dev reviewed May 10, 2022

View reviewed changes

applicationset/services/scm_provider/azure_devops.go Outdated Show resolved Hide resolved

brinchm added 4 commits May 24, 2022 13:42

Changes to Azure DevOps SCM generator from PR feeback.

ccf23eb

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

fix linting error and failing tests.

7eda041

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

remove NoError assertion

2e47bba

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

handle errors when repos are disabled, fix branch name in query when …

a16529a

…getting default branch. Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

brinchm requested a review from crenshaw-dev May 27, 2022 07:54

crenshaw-dev requested changes Jun 2, 2022

View reviewed changes

brinchm added 3 commits June 10, 2022 14:00

set proper example values for SCMProviderGeneratorAzureDevOps fields.

1097eae

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

Merge remote origin branch 'master' into feat/scmprovider-azuredevops

b6bd6c3

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

sort dependencies properly by name

5a407bf

Signed-off-by: Christer Brinchmann <christer.brinchmann@gmail.com>

crenshaw-dev approved these changes Jun 15, 2022

View reviewed changes

crenshaw-dev merged commit 56eb095 into argoproj:master Jun 15, 2022

brinchm deleted the feat/scmprovider-azuredevops branch June 15, 2022 15:27

brinchm mentioned this pull request Jun 16, 2022

chore: docs correction on how to override azure devops URL. #9699

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Azure DevOps SCM Provider Generator; add branchNormalized to SCM Generator template fields. #9283

feat: Add Azure DevOps SCM Provider Generator; add branchNormalized to SCM Generator template fields. #9283

brinchm commented May 3, 2022

codecov bot commented May 3, 2022 •

edited

Loading

crenshaw-dev commented May 3, 2022

rishabh625 May 3, 2022

brinchm May 3, 2022

rishabh625 May 10, 2022 •

edited

Loading

crenshaw-dev left a comment

crenshaw-dev left a comment

crenshaw-dev Jun 2, 2022

brinchm Jun 10, 2022

crenshaw-dev Jun 13, 2022

brinchm Jun 14, 2022

crenshaw-dev left a comment

brinchm commented Jun 15, 2022

feat: Add Azure DevOps SCM Provider Generator; add branchNormalized to SCM Generator template fields. #9283

feat: Add Azure DevOps SCM Provider Generator; add branchNormalized to SCM Generator template fields. #9283

Conversation

brinchm commented May 3, 2022

codecov bot commented May 3, 2022 • edited Loading

Codecov Report

crenshaw-dev commented May 3, 2022

rishabh625 May 3, 2022

Choose a reason for hiding this comment

brinchm May 3, 2022

Choose a reason for hiding this comment

rishabh625 May 10, 2022 • edited Loading

Choose a reason for hiding this comment

crenshaw-dev left a comment

Choose a reason for hiding this comment

crenshaw-dev left a comment

Choose a reason for hiding this comment

crenshaw-dev Jun 2, 2022

Choose a reason for hiding this comment

brinchm Jun 10, 2022

Choose a reason for hiding this comment

crenshaw-dev Jun 13, 2022

Choose a reason for hiding this comment

brinchm Jun 14, 2022

Choose a reason for hiding this comment

crenshaw-dev left a comment

Choose a reason for hiding this comment

brinchm commented Jun 15, 2022

codecov bot commented May 3, 2022 •

edited

Loading

rishabh625 May 10, 2022 •

edited

Loading