diff --git a/cdq-analysis/pkg/util.go b/cdq-analysis/pkg/util.go index 1a7c695e6..e17ce8acd 100644 --- 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 index e9f0d3b6b..42a967fb7 100644 --- 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 index 71c49c336..7122363ea 100644 --- 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 index c792b84ba..45f921229 100644 --- 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 index 8f8cb0d71..b5348f014 100644 --- 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 index 41d5f674f..3f17d10cd 100644 --- 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) - }) - }) })