Merge pull request #382 from rcerven/offboardwebhook

when offbording don't remove webhook from gitlab if other component is

controllers/component_build_controller_pac.go

@@ -33,6 +33,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/konflux-ci/build-service/pkg/boerrors"
@@ -534,8 +535,27 @@ func (r *ComponentBuildReconciler) UnconfigureRepositoryForPaC(ctx context.Conte
 	}
 
 	isAppUsed := IsPaCApplicationConfigured(gitProvider, pacConfig)
-	if !isAppUsed {
-		if webhookTargetUrl != "" {
+	if !isAppUsed && webhookTargetUrl != "" {
+		gitUrl := strings.TrimSuffix(strings.TrimSuffix(component.Spec.Source.GitSource.URL, ".git"), "/")
+		componentList := &appstudiov1alpha1.ComponentList{}
+		if err := r.Client.List(ctx, componentList, &client.ListOptions{Namespace: component.Namespace}); err != nil {
+			log.Error(err, "failed to list components")
+			return "", "", "", err
+		}
+
+		sameRepoUsed := false
+		for _, comp := range componentList.Items {
+			if comp.Name == component.Name {
+				continue
+			}
+			componentUrl := strings.TrimSuffix(strings.TrimSuffix(comp.Spec.Source.GitSource.URL, ".git"), "/")
+			if componentUrl == gitUrl {
+				sameRepoUsed = true
+				break
+			}
+		}
+
+		if !sameRepoUsed {
 			err = gitClient.DeletePaCWebhook(repoUrl, webhookTargetUrl)
 			if err != nil {
 				// Just log the error and continue with merge request creation

controllers/component_build_controller_test.go

@@ -1000,7 +1000,7 @@ var _ = Describe("Component initial build controller", func() {
 			isRemovePaCPullRequestInvoked := false
 			UndoPaCMergeRequestFunc = func(repoUrl string, d *gp.MergeRequestData) (webUrl string, err error) {
 				isRemovePaCPullRequestInvoked = true
-				Expect(repoUrl).To(Equal(SampleRepoLink + "-" + resourceCleanupKey.Name))
+				Expect(repoUrl).To(Equal(SampleRepoLink + "-samerepo"))
 				Expect(len(d.Files)).To(Equal(2))
 				for _, file := range d.Files {
 					Expect(strings.HasPrefix(file.FullPath, ".tekton/")).To(BeTrue())
@@ -1014,11 +1014,9 @@ var _ = Describe("Component initial build controller", func() {
 				Expect(d.AuthorEmail).ToNot(BeEmpty())
 				return mergeUrl, nil
 			}
-			isDeletePaCWebhookInvoked := false
 			DeletePaCWebhookFunc = func(repoUrl string, webhookUrl string) error {
-				isDeletePaCWebhookInvoked = true
-				Expect(repoUrl).To(Equal(SampleRepoLink + "-" + resourceCleanupKey.Name))
-				Expect(webhookUrl).To(Equal(pacWebhookUrl))
+				defer GinkgoRecover()
+				Fail("Should not remove webhook because another component is using the same repository")
 				return nil
 			}
 
@@ -1028,19 +1026,50 @@ var _ = Describe("Component initial build controller", func() {
 			createComponentAndProcessBuildRequest(componentConfig{
 				componentKey: resourceCleanupKey,
 				annotations:  defaultPipelineAnnotations,
+				gitURL:       SampleRepoLink + "-samerepo",
 			}, BuildRequestConfigurePaCAnnotationValue)
 			waitPaCFinalizerOnComponent(resourceCleanupKey)
 
+			// create second component which uses the same url
+			secondComponentKey := types.NamespacedName{Name: HASCompName + "-cleanup-second", Namespace: HASAppNamespace}
+			createComponentAndProcessBuildRequest(componentConfig{
+				componentKey: secondComponentKey,
+				annotations:  defaultPipelineAnnotations,
+				gitURL:       SampleRepoLink + "-samerepo",
+			}, BuildRequestConfigurePaCAnnotationValue)
+			waitPaCFinalizerOnComponent(secondComponentKey)
+
+			// shouldn't remove webhook because another component exists for the same repo
 			setComponentBuildRequest(resourceCleanupKey, BuildRequestUnconfigurePaCAnnotationValue)
 
+			Eventually(func() bool {
+				return isRemovePaCPullRequestInvoked
+			}, timeout, interval).Should(BeTrue())
+
+			expectPacBuildStatus(resourceCleanupKey, "disabled", 0, "", mergeUrl)
+			// delete component so there will be just 1 component using the repository
+			deleteComponent(resourceCleanupKey)
+
+			isRemovePaCPullRequestInvoked = false
+			isDeletePaCWebhookInvoked := false
+			DeletePaCWebhookFunc = func(repoUrl string, webhookUrl string) error {
+				isDeletePaCWebhookInvoked = true
+				Expect(repoUrl).To(Equal(SampleRepoLink + "-samerepo"))
+				Expect(webhookUrl).To(Equal(pacWebhookUrl))
+				return nil
+			}
+
+			// should remove webhook because there isn't another component which uses the same repo
+			setComponentBuildRequest(secondComponentKey, BuildRequestUnconfigurePaCAnnotationValue)
+
 			Eventually(func() bool {
 				return isRemovePaCPullRequestInvoked
 			}, timeout, interval).Should(BeTrue())
 			Eventually(func() bool {
 				return isDeletePaCWebhookInvoked
 			}, timeout, interval).Should(BeTrue())
 
-			expectPacBuildStatus(resourceCleanupKey, "disabled", 0, "", mergeUrl)
+			expectPacBuildStatus(secondComponentKey, "disabled", 0, "", mergeUrl)
 		})
 
 		It("should not block component deletion if PaC definitions removal failed", func() {

controllers/renovate_util.go

@@ -273,9 +273,9 @@ func (u ComponentDependenciesUpdater) GetUpdateTargetsGithubApp(ctx context.Cont
 		}
 
 		targetsToUpdate = append(targetsToUpdate, updateTarget{
-			ComponentName:           component.Name,
-			GitProvider:             gitProvider,
-			Username:                fmt.Sprintf("%s[bot]", slug),
+			ComponentName: component.Name,
+			GitProvider:   gitProvider,
+			Username:      fmt.Sprintf("%s[bot]", slug),
 			// hardcoding the number because mintmaker has it hardcoded as well, so that way mintmaker will recognize the same author
 			GitAuthor:               fmt.Sprintf("%s <126015336+%s[bot]@users.noreply.github.com>", slug, slug),
 			Token:                   githubAppInstallation.Token,