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

@@ -323,6 +357,13 @@ var azuredevopsAllowedPullRequestActions = []string{
 	"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

@@ -234,6 +244,7 @@ func TestWebhookHandler(t *testing.T) {
 			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.

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

tidiegeler added 3 commits November 21, 2024 13:33

another fix

30977b0

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

Merge branch 'argoproj:master' into feature/scm_manager_support

486a82c

apply test fixes for SCM-Manager merge

1dca723

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

tidiegeler force-pushed the feature/scm_manager_support branch from 3c5a762 to 1dca723 Compare November 22, 2024 05:44

tidiegeler and others added 9 commits November 25, 2024 14:32

Merge branch 'argoproj:master' into feature/scm_manager_support

0a2beda

Merge branch 'argoproj:master' into feature/scm_manager_support

fd52f69

Replace HTMLUrl in SCMM with SourceUrl

e4a2fad

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

Add screenshot for webhook configuration in SCM-Manager

642d642

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

Implement pull request hooks

5522238

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

eheimbuch commented Jul 4, 2023 •

edited

Loading

codecov bot commented Jul 5, 2023 •

edited

Loading

ishitasequeira left a comment

pfeuffer commented Sep 14, 2023

eheimbuch commented Oct 23, 2023

eheimbuch commented Nov 7, 2023

ishitasequeira left a comment

pfeuffer commented Dec 18, 2023

pfeuffer commented Aug 21, 2024

tidiegeler left a comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

tidiegeler commented Jan 7, 2025

tidiegeler commented Jan 22, 2025

tidiegeler commented Feb 6, 2025 •

edited

Loading

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

Conversation

eheimbuch commented Jul 4, 2023 • edited Loading

codecov bot commented Jul 5, 2023 • edited Loading

Codecov Report

ishitasequeira left a comment

Choose a reason for hiding this comment

pfeuffer commented Sep 14, 2023

eheimbuch commented Oct 23, 2023

eheimbuch commented Nov 7, 2023

ishitasequeira left a comment

Choose a reason for hiding this comment

pfeuffer commented Dec 18, 2023

pfeuffer commented Aug 21, 2024

tidiegeler left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

tidiegeler commented Jan 7, 2025

tidiegeler commented Jan 22, 2025

tidiegeler commented Feb 6, 2025 • edited Loading

eheimbuch commented Jul 4, 2023 •

edited

Loading

codecov bot commented Jul 5, 2023 •

edited

Loading

tidiegeler commented Feb 6, 2025 •

edited

Loading