feat(appset): add SCM-Manager to Pull Request and SCM Provider Generators #14332

eheimbuch · 2023-07-04T10:44:06Z

This PR implements the functionality of Pull Request Generator for SCM-Manager Repos. Also we added SCM-Manager to be available as SCM Provider.

Checklist:

Please see Contribution FAQs if you have questions about your pull-request.

codecov · 2023-07-05T09:54:51Z

Codecov Report

Attention: Patch coverage is 56.80000% with 108 lines in your changes missing coverage. Please review.

Project coverage is 55.19%. Comparing base (c739478) to head (207ce7b).

Files with missing lines	Patch %	Lines
applicationset/webhook/webhook.go	44.23%	22 Missing and 7 partials ⚠️
...pplicationset/services/scm_provider/scm-manager.go	76.63%	18 Missing and 7 partials ⚠️
applicationset/generators/scm_provider.go	0.00%	17 Missing ⚠️
applicationset/generators/pull_request.go	0.00%	14 Missing ⚠️
...pplicationset/services/pull_request/scm-manager.go	70.73%	8 Missing and 4 partials ⚠️
util/webhook/webhook.go	57.14%	4 Missing and 2 partials ⚠️
.../apis/application/v1alpha1/applicationset_types.go	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #14332      +/-   ##
==========================================
+ Coverage   55.16%   55.19%   +0.03%     
==========================================
  Files         337      339       +2     
  Lines       57039    57289     +250     
==========================================
+ Hits        31464    31620     +156     
- Misses      22874    22946      +72     
- Partials     2701     2723      +22

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

ishitasequeira

Thanks @eheimbuch for the PR!! A couple nits and a general question.

Should we also provide the ability for the user to configure a cert bundle on the appset controller so that users don't have to disable cert validation similar to Gitlab SCMProvider?

docs/operator-manual/applicationset/Generators-Pull-Request.md

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

pfeuffer · 2023-09-14T18:36:13Z

Hi @ishitasequeira !

Should we also provide the ability for the user to configure a cert bundle on the appset controller so that users don't have to disable cert validation similar to Gitlab SCMProvider?

We would skip this feature from our side for now and would be very greatful to see this first step going live. Thanks for your review, we highly appreciate this.

eheimbuch · 2023-10-23T07:20:38Z

Hi @ishitasequeira,

we fixed your review some time ago and still waiting for your feedback / pr merge. We have more integrations for SCM Manager with ArgoCD in our pipeline, but are hesitant due to the status of this pull request.

eheimbuch · 2023-11-07T07:52:45Z

Bump @ishitasequeira

ishitasequeira

@eheimbuch @pfeuffer Apologies for lack of response on this PR and getting back to you so late.

IMO, disabling the cert validation is a security risk and we should support that feature from the start. It would be highly appreciated if you could get the support added for cert validation. We do have a reference available for similar use-case. Ref PR: #14348

pfeuffer · 2023-12-18T08:29:06Z

Hi @ishitasequeira , thanks for the feedback. We will add this and report back when we're finished.

pfeuffer · 2024-08-21T09:50:15Z

Hey @ishitasequeira , thanks for the patience! We've added the cert validation and it would be great if you could take a look again.

Signed-off-by: Till-André Diegeler <[email protected]>

Signed-off-by: René Pfeuffer <[email protected]>

…_support

Signed-off-by: René Pfeuffer <[email protected]>

…_support

Signed-off-by: René Pfeuffer <[email protected]>

tidiegeler

Add some minor questions / points

applicationset/services/scm_provider/scm-manager.go

+func (g *ScmManagerProvider) ListRepos(ctx context.Context, cloneProtocol string) ([]*Repository, error) {
+	repos := []*Repository{}
+	filter := g.client.NewRepoListFilter()
+	filter.Limit = 9999


applicationset/webhook/webhook.go

 	"git.pullrequest.updated",
 }

+var scmManagerAllowedPullRequestActions = []string{


applicationset/webhook/webhook.go

+		}
+		infoUrl, err := url.Parse(info.SCMManager.Host)
+		if err != nil {
+			log.Errorf("Failed to parse SCM-Manager URL '%s'", info.SCMManager.Host)


docs/assets/scm-webhook-config.png

applicationset/webhook/webhook_test.go

 			assert.Equal(t, test.expectedStatusCode, w.Code)

 			list := &v1alpha1.ApplicationSetList{}
+			fmt.Printf("List: %v\n", list)


applicationset/services/scm_provider/scm-manager_test.go

+		assert.True(t, ok)
+	})
+
+	t.Run("does not exists", func(t *testing.T) {


Signed-off-by: Till-André Diegeler <[email protected]>

…_support

tidiegeler · 2025-01-07T12:28:07Z

Hi @ishitasequeira , thank you for having taken a look at our PR over a year ago.

It has been a long time and the matter was postponed on our side until a few months ago. We were putting a lot of effort into reaching a stable state for our solution and were also tackling the remaining steps on the native Argo-CD side. The solution is now able to be merged with the most recent states; if a merge conflict still occurs, it may be due to a recent (minor) divergence.

Is this state acceptable, so that it can be officially taken as an addition to the Argo-CD project?

Thank you!

Signed-off-by: René Pfeuffer <[email protected]>

tidiegeler · 2025-01-22T09:58:48Z

Bump @ishitasequeira

tidiegeler · 2025-02-06T12:26:12Z

Bump @ishitasequeira

👋 This is from an argoproj PR.

tidiegeler · 2025-03-13T08:25:41Z

Hi @crenshaw-dev, we've been waiting for feedback for a change from our side. Would you be the right person to take a look over it? Sadly, we couldn't reach out someone else before.

crenshaw-dev · 2025-03-13T12:47:56Z

Hey @tidiegeler I'm not going to have time to do an in depth review before Mondays 3.0 RC release. But I'll try to give it a quick first pass so we can be moving. This might be the oldest open PR that's actively being worked on. 😅

tidiegeler · 2025-05-05T06:04:31Z

Hello @crenshaw-dev, thanks for your comment from about two months ago :) Is there still any interest/capacity for looking into this PR?

tidiegeler · 2025-05-20T13:52:22Z

Hello @crenshaw-dev, thanks for your comment from about two months ago :) Is there still any interest/capacity for looking into this PR?

Bump @crenshaw-dev

tidiegeler · 2025-06-02T05:25:10Z

Bump @crenshaw-dev

tidiegeler · 2025-08-29T11:15:03Z

Hello @ishitasequeira @crenshaw-dev, is there any chance for a review availability within the next months? Despite the strikingly high age of this PR; we are still interested in it :)

tidiegeler · 2025-09-01T12:08:56Z

@nitishfy @agaudreault Sorry for the disturbance! 😅 I hope you can help us out with this topic. It's gaining traction lately.

# Conflicts: # pkg/apis/application/v1alpha1/generated.pb.go

eheimbuch changed the title ~~Initial implementation of SCM-Manager Support~~ feat: Initial implementation of SCM-Manager Support Jul 4, 2023

eheimbuch changed the title ~~feat: Initial implementation of SCM-Manager Support~~ feat(appset): add Pull Request Generator for SCM-Manager Jul 4, 2023

eheimbuch force-pushed the feature/scm_manager_support branch 3 times, most recently from 6910010 to 77aa1ff Compare July 5, 2023 09:40

eheimbuch marked this pull request as ready for review July 6, 2023 12:43

ishitasequeira changed the title ~~feat(appset): add Pull Request Generator for SCM-Manager~~ feat(appset): add SCM-Manager to Pull Request and SCM Provider Generators Aug 29, 2023

ishitasequeira requested changes Sep 6, 2023

View reviewed changes

docs/operator-manual/applicationset/Generators-Pull-Request.md Outdated Show resolved Hide resolved

docs/operator-manual/applicationset/Generators-SCM-Provider.md Outdated Show resolved Hide resolved

eheimbuch requested review from a team as code owners November 7, 2023 07:06

eheimbuch force-pushed the feature/scm_manager_support branch from 8d381ad to 2ea0341 Compare November 7, 2023 07:40

ishitasequeira requested changes Dec 14, 2023

View reviewed changes

sdorra force-pushed the feature/scm_manager_support branch from 9d576ed to 01b684f Compare August 8, 2024 14:23

karat-1 force-pushed the feature/scm_manager_support branch 4 times, most recently from 6f047d3 to a182c30 Compare August 20, 2024 09:23

sdorra force-pushed the feature/scm_manager_support branch 2 times, most recently from 14e8829 to 59bd2e6 Compare August 21, 2024 07:26

tidiegeler force-pushed the feature/scm_manager_support branch 3 times, most recently from 88df022 to f194b97 Compare November 21, 2024 07:53

feat(appset): add SCM-Manager support

2941102

Signed-off-by: Till-André Diegeler <[email protected]>

tidiegeler force-pushed the feature/scm_manager_support branch from f194b97 to 2941102 Compare November 21, 2024 08:23

pfeuffer added 4 commits December 5, 2024 08:46

Fix tests for util/webhook

84ddbed

Signed-off-by: René Pfeuffer <[email protected]>

Fix tests for util/webhook

45426dc

Signed-off-by: René Pfeuffer <[email protected]>

Add test for applicationset/webhook

25eba06

Signed-off-by: René Pfeuffer <[email protected]>

Remove debug log

ad51074

Signed-off-by: René Pfeuffer <[email protected]>

pfeuffer force-pushed the feature/scm_manager_support branch from 0eadd5d to ad51074 Compare December 5, 2024 07:46

pfeuffer added 4 commits December 5, 2024 09:08

Bump goscm version

ec74aed

Signed-off-by: René Pfeuffer <[email protected]>

Merge remote-tracking branch 'origin/master' into feature/scm_manager…

d483457

…_support

Fix import order

00202c5

Signed-off-by: René Pfeuffer <[email protected]>

Merge remote-tracking branch 'origin/master' into feature/scm_manager…

2dd4620

…_support

pfeuffer force-pushed the feature/scm_manager_support branch from cc6e582 to 2dd4620 Compare December 5, 2024 15:06

Add documentation for SCM-Manager webhook configuration

b003349

Signed-off-by: René Pfeuffer <[email protected]>

pfeuffer force-pushed the feature/scm_manager_support branch from 164b98d to b003349 Compare December 6, 2024 13:44

tidiegeler approved these changes Dec 9, 2024

View reviewed changes

tidiegeler and others added 3 commits December 9, 2024 10:01

implement minor fixes

ab23c4e

Signed-off-by: Till-André Diegeler <[email protected]>

Merge remote-tracking branch 'origin/master' into feature/scm_manager…

f4eb396

…_support

Merge branch 'master' into feature/scm_manager_support

b3de71b

Fix linting errors

2915145

Signed-off-by: René Pfeuffer <[email protected]>

pfeuffer force-pushed the feature/scm_manager_support branch from 207ce7b to 2915145 Compare January 7, 2025 14:59

Merge branch master into feature/scm_manager_support

0444e4b

# Conflicts: # pkg/apis/application/v1alpha1/generated.pb.go

agaudreault added this to the v3.3 milestone Sep 3, 2025

feat(appset): add SCM-Manager to Pull Request and SCM Provider Generators #14332

Are you sure you want to change the base?

feat(appset): add SCM-Manager to Pull Request and SCM Provider Generators #14332

Uh oh!

Conversation

eheimbuch commented Jul 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ishitasequeira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pfeuffer commented Sep 14, 2023

Uh oh!

eheimbuch commented Oct 23, 2023

Uh oh!

eheimbuch commented Nov 7, 2023

Uh oh!

ishitasequeira left a comment

Choose a reason for hiding this comment

Uh oh!

pfeuffer commented Dec 18, 2023

Uh oh!

pfeuffer commented Aug 21, 2024

Uh oh!

tidiegeler left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

tidiegeler commented Jan 7, 2025

Uh oh!

tidiegeler commented Jan 22, 2025

Uh oh!

tidiegeler commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tidiegeler commented Mar 13, 2025

Uh oh!

crenshaw-dev commented Mar 13, 2025

Uh oh!

tidiegeler commented May 5, 2025

Uh oh!

tidiegeler commented May 20, 2025

Uh oh!

tidiegeler commented Jun 2, 2025

Uh oh!

tidiegeler commented Aug 29, 2025

Uh oh!

tidiegeler commented Sep 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eheimbuch commented Jul 4, 2023 •

edited

Loading

codecov bot commented Jul 5, 2023 •

edited

Loading

tidiegeler commented Feb 6, 2025 •

edited

Loading