Merge pull request #457 from johnmcollier/revert454

Revert "Return error for non supported development platform urls"
redhat-appstudio · Mar 27, 2024 · 6956cf6 · 6956cf6
2 parents f00d9c3 + 6671c59
commit 6956cf6
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 185 deletions.
diff --git a/cdq-analysis/pkg/util.go b/cdq-analysis/pkg/util.go
@@ -279,17 +279,3 @@ func UpdateGitLink(repo, revision, context string) (string, error) {
 	}
 	return rawGitURL, nil
 }
-
-// ValidateGithubURL checks if the given url includes github in hostname
-// In case of invalid url (not able to parse / not github) returns an error.
-func ValidateGithubURL(URL string) error {
-	parsedURL, err := url.Parse(URL)
-	if err != nil {
-		return err
-	}
-
-	if strings.Contains(parsedURL.Host, "github") {
-		return nil
-	}
-	return fmt.Errorf("source git url %v is not from github", URL)
-}
diff --git a/cdq-analysis/pkg/util_test.go b/cdq-analysis/pkg/util_test.go
@@ -317,11 +317,6 @@ func TestConvertGitHubURL(t *testing.T) {
 			url:     "https://github.com/devfile/api/tree/2.1.x",
 			wantUrl: "https://raw.githubusercontent.com/devfile/api/2.1.x",
 		},
-		{
-			name:    "A non github url",
-			url:     "https://gitlab.com/",
-			wantUrl: "https://gitlab.com",
-		},
 		{
 			name:    "A non url",
 			url:     "\000x",
@@ -640,36 +635,3 @@ func TestUpdateGitLink(t *testing.T) {
 		})
 	}
 }
-
-func TestValidateGithubURL(t *testing.T) {
-	tests := []struct {
-		name         string
-		sourceGitURL string
-		wantErr      bool
-	}{
-		{
-			name:         "Valid github url",
-			sourceGitURL: "https://github.com/devfile-samples",
-			wantErr:      false,
-		},
-		{
-			name:         "Invalid url",
-			sourceGitURL: "afgae devfile",
-			wantErr:      true,
-		},
-		{
-			name:         "Not github url",
-			sourceGitURL: "https://gitlab.com/devfile-samples",
-			wantErr:      true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			err := ValidateGithubURL(tt.sourceGitURL)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("TestValidateGithubURL() unexpected error: %v", err)
-			}
-		})
-	}
-}
diff --git a/controllers/component_controller.go b/controllers/component_controller.go
@@ -301,13 +301,6 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		var devfileBytes []byte
 
 		if source.GitSource != nil && source.GitSource.URL != "" {
-			if err := cdqanalysis.ValidateGithubURL(source.GitSource.URL); err != nil {
-				// User error - the git url provided is not from github
-				log.Error(err, "unable to validate github url")
-				metrics.IncrementComponentCreationSucceeded("", "")
-				_ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err)
-				return ctrl.Result{}, nil
-			}
 			context := source.GitSource.Context
 			// If a Git secret was passed in, retrieve it for use in our Git operations
 			// The secret needs to be in the same namespace as the Component

diff --git a/controllers/component_controller_test.go b/controllers/component_controller_test.go
@@ -52,15 +52,14 @@ var _ = Describe("Component controller", func() {
 
 	// Define utility constants for object names and testing timeouts/durations and intervals.
 	const (
-		HASAppName           = "test-application"
-		HASCompName          = "test-component"
-		HASAppNamespace      = "default"
-		DisplayName          = "petclinic"
-		Description          = "Simple petclinic app"
-		ComponentName        = "backend"
-		SampleRepoLink       = "https://github.com/devfile-samples/devfile-sample-java-springboot-basic"
-		SampleGitlabRepoLink = "https://gitlab.com/devfile-samples/devfile-sample-java-springboot-basic"
-		gitToken             = "" //empty for public repo test
+		HASAppName      = "test-application"
+		HASCompName     = "test-component"
+		HASAppNamespace = "default"
+		DisplayName     = "petclinic"
+		Description     = "Simple petclinic app"
+		ComponentName   = "backend"
+		SampleRepoLink  = "https://github.com/devfile-samples/devfile-sample-java-springboot-basic"
+		gitToken        = "" //empty for public repo test
 	)
 
 	prometheus.MustRegister(metrics.GetComponentCreationTotalReqs(), metrics.GetComponentCreationFailed(), metrics.GetComponentCreationSucceeded())
@@ -1105,11 +1104,11 @@ var _ = Describe("Component controller", func() {
 				return len(createdHasComp.Status.Conditions) > 0
 			}, timeout, interval).Should(BeTrue())
 
-			// Make sure the err was set
-			Expect(createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-1].Reason).Should(Equal("Error"))
-			Expect(strings.ToLower(createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-1].Message)).Should(ContainSubstring("component create failed:"))
+			// Make sure the devfile model was properly set in Component
+			errCondition := createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-1]
+			Expect(errCondition.Status).Should(Equal(metav1.ConditionFalse))
+			Expect(errCondition.Message).Should(ContainSubstring("Component create failed: unable to get default branch of Github Repo"))
 
-			// Confirm user error hasn't increase the failure metrics
 			hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace}
 
 			Expect(testutil.ToFloat64(metrics.GetComponentCreationTotalReqs()) > beforeCreateTotalReqs).To(BeTrue())
@@ -2960,6 +2959,7 @@ var _ = Describe("Component controller", func() {
 			deleteHASAppCR(hasAppLookupKey)
 		})
 	})
+
 	Context("Component with application marked to be deleted", func() {
 		It("Should not increase the deletion metrics", func() {
 			beforeDeleteFailedReqs := testutil.ToFloat64(metrics.GetComponentDeletionFailed())
@@ -3037,67 +3037,6 @@ var _ = Describe("Component controller", func() {
 			Expect(testutil.ToFloat64(metrics.GetComponentDeletionTotalReqs()) == beforeDeleteTotalReqs).To(BeTrue())
 		})
 	})
-	Context("Create component having git source from gitlab", func() {
-		It("Should not increase the component failure metrics", func() {
-			beforeCreateTotalReqs := testutil.ToFloat64(metrics.GetComponentCreationTotalReqs())
-			beforeCreateSucceedReqs := testutil.ToFloat64(metrics.GetComponentCreationSucceeded())
-			beforeCreateFailedReqs := testutil.ToFloat64(metrics.GetComponentCreationFailed())
-
-			ctx := context.Background()
-
-			applicationName := HASAppName + "30"
-			componentName := HASCompName + "30"
-
-			createAndFetchSimpleApp(applicationName, HASAppNamespace, DisplayName, Description)
-
-			hasComp := &appstudiov1alpha1.Component{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: "appstudio.redhat.com/v1alpha1",
-					Kind:       "Component",
-				},
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      componentName,
-					Namespace: HASAppNamespace,
-				},
-				Spec: appstudiov1alpha1.ComponentSpec{
-					ComponentName: ComponentName,
-					Application:   applicationName,
-					Source: appstudiov1alpha1.ComponentSource{
-						ComponentSourceUnion: appstudiov1alpha1.ComponentSourceUnion{
-							GitSource: &appstudiov1alpha1.GitSource{
-								URL: SampleGitlabRepoLink,
-							},
-						},
-					},
-				},
-			}
-			Expect(k8sClient.Create(ctx, hasComp)).Should(Succeed())
-
-			// Look up the has app resource that was created.
-			// num(conditions) may still be < 1 on the first try, so retry until at least _some_ condition is set
-			hasCompLookupKey := types.NamespacedName{Name: componentName, Namespace: HASAppNamespace}
-			createdHasComp := &appstudiov1alpha1.Component{}
-			Eventually(func() bool {
-				k8sClient.Get(ctx, hasCompLookupKey, createdHasComp)
-				return len(createdHasComp.Status.Conditions) > 0
-			}, timeout, interval).Should(BeTrue())
-
-			// Make sure the err was set
-			Expect(createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-1].Reason).Should(Equal("Error"))
-			Expect(strings.ToLower(createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-1].Message)).Should(ContainSubstring("is not from github"))
-			hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace}
-
-			Expect(testutil.ToFloat64(metrics.GetComponentCreationTotalReqs()) > beforeCreateTotalReqs).To(BeTrue())
-			Expect(testutil.ToFloat64(metrics.GetComponentCreationSucceeded()) > beforeCreateSucceedReqs).To(BeTrue())
-			Expect(testutil.ToFloat64(metrics.GetComponentCreationFailed()) == beforeCreateFailedReqs).To(BeTrue())
-
-			// Delete the specified HASComp resource
-			deleteHASCompCR(hasCompLookupKey)
-
-			// Delete the specified HASApp resource
-			deleteHASAppCR(hasAppLookupKey)
-		})
-	})
 })
 
 type updateChecklist struct {

diff --git a/controllers/componentdetectionquery_controller.go b/controllers/componentdetectionquery_controller.go
@@ -170,14 +170,6 @@ func (r *ComponentDetectionQueryReconciler) Reconcile(ctx context.Context, req c
 			return ctrl.Result{}, nil
 		}
 
-		// check if the given url is from github
-		if err := cdqanalysis.ValidateGithubURL(sourceURL); err != nil {
-			// User error - the git url provided is not from github
-			log.Error(err, "unable to validate github url")
-			r.SetCompleteConditionAndUpdateCR(ctx, req, &componentDetectionQuery, copiedCDQ, err)
-			return ctrl.Result{}, nil
-		}
-
 		cdqInfo := &cdqanalysis.CDQInfo{
 			DevfileRegistryURL: r.DevfileRegistryURL,
 			GitURL:             cdqanalysis.GitURL{RepoURL: source.URL, Revision: source.Revision, Token: gitToken},

diff --git a/controllers/componentdetectionquery_controller_test.go b/controllers/componentdetectionquery_controller_test.go
@@ -3337,50 +3337,6 @@ metadata:
 		})
 
 	})
-	Context("Create Component Detection Query with non Github URL", func() {
-		It("Should err out", func() {
-			ctx := context.Background()
-
-			queryName := HASCompDetQuery + "21"
-
-			hasCompDetectionQuery := &appstudiov1alpha1.ComponentDetectionQuery{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: "appstudio.redhat.com/v1alpha1",
-					Kind:       "ComponentDetectionQuery",
-				},
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      queryName,
-					Namespace: HASNamespace,
-					Annotations: map[string]string{
-						"runCDQAnalysisLocal": "true",
-					},
-				},
-				Spec: appstudiov1alpha1.ComponentDetectionQuerySpec{
-					GitSource: appstudiov1alpha1.GitSource{
-						URL:      "https://gitlab.com/redhat-appstudio-appdata/sample",
-						Revision: "main",
-					},
-				},
-			}
-
-			Expect(k8sClient.Create(ctx, hasCompDetectionQuery)).Should(Succeed())
-
-			// Look up the has app resource that was created.
-			// num(conditions) may still be < 1 on the first try, so retry until at least _some_ condition is set
-			hasCompDetQueryLookupKey := types.NamespacedName{Name: queryName, Namespace: HASNamespace}
-			createdHasCompDetectionQuery := &appstudiov1alpha1.ComponentDetectionQuery{}
-			Eventually(func() bool {
-				k8sClient.Get(context.Background(), hasCompDetQueryLookupKey, createdHasCompDetectionQuery)
-				return len(createdHasCompDetectionQuery.Status.Conditions) > 1
-			}, timeout, interval).Should(BeTrue())
-
-			// Make sure the right err is set
-			Expect(createdHasCompDetectionQuery.Status.Conditions[1].Message).Should(ContainSubstring("is not from github"))
-
-			// Delete the specified Detection Query resource
-			deleteCompDetQueryCR(hasCompDetQueryLookupKey)
-		})
-	})
 
 })