From 77a3f5bff3642cbedd0a181c963156bddcc8d90f Mon Sep 17 00:00:00 2001 From: John Collier Date: Tue, 25 Jul 2023 14:20:44 -0400 Subject: [PATCH] [DEVHAS-407] Detect branch from cloned repo (#368) * [DEVHAS-407] Detect branch from cloned repo Signed-off-by: John Collier * Fix tests Signed-off-by: John Collier --------- Signed-off-by: John Collier --- cdq-analysis/pkg/componentdetectionquery.go | 63 +++++++++++-------- .../pkg/componentdetectionquery_test.go | 10 ++- cdq-analysis/pkg/util.go | 18 ++++++ cdq-analysis/pkg/util_test.go | 53 ++++++++++++++++ .../componentdetectionquery_controller.go | 56 +++++++++-------- ...componentdetectionquery_controller_test.go | 4 +- 6 files changed, 147 insertions(+), 57 deletions(-) diff --git a/cdq-analysis/pkg/componentdetectionquery.go b/cdq-analysis/pkg/componentdetectionquery.go index f3bcd5bf..1fdd8b5b 100644 --- a/cdq-analysis/pkg/componentdetectionquery.go +++ b/cdq-analysis/pkg/componentdetectionquery.go @@ -38,7 +38,7 @@ type K8sInfoClient struct { // CDQ analyzer // return values are for testing purpose -func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfilePath, dockerfilePath, URL, Revision, DevfileRegistryURL string, isDevfilePresent, isDockerfilePresent bool) (map[string][]byte, map[string]string, map[string]string, map[string][]int, error) { +func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfilePath, dockerfilePath, URL, Revision, DevfileRegistryURL string, isDevfilePresent, isDockerfilePresent bool) (map[string][]byte, map[string]string, map[string]string, map[string][]int, string, error) { log := k.Log var clonePath, componentPath string alizerClient := AlizerClient{} @@ -54,18 +54,46 @@ func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfil context = "./" } + clonePath, err = CreateTempPath(name, Fs) + if err != nil { + log.Error(err, fmt.Sprintf("Unable to create a temp path %s for cloning %v", clonePath, namespace)) + k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) + return nil, nil, nil, nil, "", err + } + + err = CloneRepo(clonePath, URL, Revision, gitToken) + if err != nil { + log.Error(err, fmt.Sprintf("Unable to clone repo %s to path %s, exiting reconcile loop %v", URL, clonePath, namespace)) + k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) + return nil, nil, nil, nil, "", err + } + log.Info(fmt.Sprintf("cloned from %s to path %s... %v", URL, clonePath, namespace)) + componentPath = clonePath + if context != "" { + componentPath = path.Join(clonePath, context) + } + + if Revision == "" { + Revision, err = GetBranchFromRepo(componentPath) + if err != nil { + log.Error(err, fmt.Sprintf("Unable to clone repo %s to path %s, exiting reconcile loop %v", URL, clonePath, namespace)) + k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) + return nil, nil, nil, nil, "", err + } + } + isMultiComponent := false if isDevfilePresent { updatedLink, err := UpdateGitLink(URL, Revision, path.Join(context, devfilePath)) if err != nil { log.Error(err, fmt.Sprintf("Unable to update the devfile link for CDQ %v... %v", name, namespace)) k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err + return nil, nil, nil, nil, "", err } shouldIgnoreDevfile, devfileBytes, err := ValidateDevfile(log, updatedLink) if err != nil { k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err + return nil, nil, nil, nil, "", err } if shouldIgnoreDevfile { isDevfilePresent = false @@ -81,25 +109,6 @@ func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfil dockerfileContextMap[context] = dockerfilePath } - clonePath, err = CreateTempPath(name, Fs) - if err != nil { - log.Error(err, fmt.Sprintf("Unable to create a temp path %s for cloning %v", clonePath, namespace)) - k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err - } - - err = CloneRepo(clonePath, URL, Revision, gitToken) - if err != nil { - log.Error(err, fmt.Sprintf("Unable to clone repo %s to path %s, exiting reconcile loop %v", URL, clonePath, namespace)) - k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err - } - log.Info(fmt.Sprintf("cloned from %s to path %s... %v", URL, clonePath, namespace)) - componentPath = clonePath - if context != "" { - componentPath = path.Join(clonePath, context) - } - if !isDockerfilePresent { log.Info(fmt.Sprintf("Unable to find devfile, Dockerfile or Containerfile under root directory, run Alizer to detect components... %v", namespace)) @@ -108,7 +117,7 @@ func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfil if err != nil { log.Error(err, fmt.Sprintf("Unable to detect components using Alizer for repo %v, under path %v... %v ", URL, componentPath, namespace)) k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err + return nil, nil, nil, nil, "", err } log.Info(fmt.Sprintf("components detected %v... %v", components, namespace)) // If no devfile and no Dockerfile or Containerfile present in the root @@ -128,7 +137,7 @@ func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfil if _, ok := err.(*NoDevfileFound); !ok { log.Error(err, fmt.Sprintf("Unable to find devfile(s) in repo %s due to an error %s, exiting reconcile loop %v", URL, err.Error(), namespace)) k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err + return nil, nil, nil, nil, "", err } } } else { @@ -137,7 +146,7 @@ func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfil if err != nil { log.Error(err, fmt.Sprintf("Unable to analyze path %s for a devfile, Dockerfile or Containerfile %v", componentPath, namespace)) k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err + return nil, nil, nil, nil, "", err } } @@ -145,12 +154,12 @@ func CloneAndAnalyze(k K8sInfoClient, gitToken, namespace, name, context, devfil if err := Fs.RemoveAll(clonePath); err != nil { log.Error(err, fmt.Sprintf("Unable to remove the clonepath %s %v", clonePath, namespace)) k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, err) - return nil, nil, nil, nil, err + return nil, nil, nil, nil, "", err } } k.SendBackDetectionResult(devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, name, namespace, nil) - return devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, nil + return devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, Revision, nil } func (k K8sInfoClient) SendBackDetectionResult(devfilesMap map[string][]byte, devfilesURLMap map[string]string, dockerfileContextMap map[string]string, componentPortsMap map[string][]int, name, namespace string, completeError error) { diff --git a/cdq-analysis/pkg/componentdetectionquery_test.go b/cdq-analysis/pkg/componentdetectionquery_test.go index ee8a941c..96107d5e 100644 --- a/cdq-analysis/pkg/componentdetectionquery_test.go +++ b/cdq-analysis/pkg/componentdetectionquery_test.go @@ -96,6 +96,7 @@ language: JavaScript wantDevfilesURLMap map[string]string wantDockerfileContextMap map[string]string wantComponentsPortMap map[string][]int + wantBranch string }{ { testCase: "repo with devfile - should successfully detect spring component", @@ -109,6 +110,7 @@ language: JavaScript "./": "https://raw.githubusercontent.com/devfile-samples/devfile-sample-java-springboot-basic/main/docker/Dockerfile", }, wantComponentsPortMap: map[string][]int{}, + wantBranch: "main", }, { testCase: "repo without devfile and dockerfile - should successfully detect spring component", @@ -126,6 +128,7 @@ language: JavaScript "./": "https://raw.githubusercontent.com/devfile-samples/devfile-sample-java-springboot-basic/main/docker/Dockerfile", }, wantComponentsPortMap: map[string][]int{}, + wantBranch: "main", }, { testCase: "private repo - should error out with no token provided", @@ -177,6 +180,7 @@ language: JavaScript "python-src-none": "https://raw.githubusercontent.com/devfile-samples/devfile-sample-python-basic/main/docker/Dockerfile", }, wantComponentsPortMap: map[string][]int{}, + wantBranch: "main", }, { testCase: "should successfully detect single component when context is provided", @@ -191,12 +195,13 @@ language: JavaScript wantDockerfileContextMap: map[string]string{ "devfile-sample-nodejs-basic": "https://raw.githubusercontent.com/nodeshift-starters/devfile-sample/main/Dockerfile", }, + wantBranch: "main", }, } for _, tt := range tests { t.Run(tt.testCase, func(t *testing.T) { - devfilesMap, devfilesURLMap, dockerfileContextMap, componentsPortMap, err := CloneAndAnalyze(k8sClient, tt.gitToken, namespaceName, compName, tt.context, tt.devfilePath, "", tt.URL, tt.Revision, tt.DevfileRegistryURL, tt.isDevfilePresent, tt.isDockerfilePresent) + devfilesMap, devfilesURLMap, dockerfileContextMap, componentsPortMap, branch, err := CloneAndAnalyze(k8sClient, tt.gitToken, namespaceName, compName, tt.context, tt.devfilePath, "", tt.URL, tt.Revision, tt.DevfileRegistryURL, tt.isDevfilePresent, tt.isDockerfilePresent) if (err != nil) != (tt.wantErr != "") { t.Errorf("got unexpected error %v", err) } else if err == nil { @@ -223,6 +228,9 @@ language: JavaScript if !reflect.DeepEqual(componentsPortMap, tt.wantComponentsPortMap) { t.Errorf("Expected componentsPortMap: %+v, Got: %+v", tt.wantComponentsPortMap, componentsPortMap) } + if branch != tt.wantBranch { + t.Errorf("Expected branch: %+v, Got: %+v", tt.wantBranch, branch) + } } else if err != nil { assert.Regexp(t, tt.wantErr, err.Error(), "Error message should match") } diff --git a/cdq-analysis/pkg/util.go b/cdq-analysis/pkg/util.go index 4485e0cc..d1c43ae4 100644 --- a/cdq-analysis/pkg/util.go +++ b/cdq-analysis/pkg/util.go @@ -71,6 +71,24 @@ func CloneRepo(clonePath, repoURL string, revision string, token string) error { return nil } +// GetBranchFromRepo gets the current branch from the cloned repository +func GetBranchFromRepo(clonePath string) (string, error) { + // Command we want to run is: git rev-parse --abbrev-ref HEAD + c := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD") + c.Dir = clonePath + + // Get the output of the command + branchBytes, err := c.CombinedOutput() + if err != nil { + return "", fmt.Errorf("failed to get the branch from the repo: %v", err) + } + branch := string(branchBytes) + + // Remove newline characters potentially present + branch = strings.Split(branch, "\n")[0] + return branch, nil +} + // CurlEndpoint curls the endpoint and returns the response or an error if the response is a non-200 status func CurlEndpoint(endpoint string) ([]byte, error) { var respBytes []byte diff --git a/cdq-analysis/pkg/util_test.go b/cdq-analysis/pkg/util_test.go index 7c7b0900..ea8d809f 100644 --- a/cdq-analysis/pkg/util_test.go +++ b/cdq-analysis/pkg/util_test.go @@ -177,6 +177,59 @@ func TestCloneRepo(t *testing.T) { } } +func TestGetBranchFromRepo(t *testing.T) { + os.Mkdir("/tmp/alreadyexistingdir", 0755) + + tests := []struct { + name string + clonePath string + repo string + revision string + token string + wantErr bool + want string + }{ + { + name: "Detect Successfully", + clonePath: "/tmp/testspringbootclone", + repo: "https://github.com/devfile-samples/devfile-sample-java-springboot-basic", + want: "main", + }, + { + name: "Detect alternate branch Successfully", + clonePath: "/tmp/testspringbootclonealt", + repo: "https://github.com/devfile-samples/devfile-sample-java-springboot-basic", + revision: "testbranch", + want: "testbranch", + }, + { + name: "Repo not exist", + clonePath: "FDSFSDFSDFSDFjsdklfjsdklfjs", + repo: "https://github.com/devfile-samples/devfile-sample-java-springboot-basic", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.RemoveAll(tt.clonePath) + if tt.name != "Repo not exist" { + CloneRepo(tt.clonePath, tt.repo, tt.revision, tt.token) + } + + branch, err := GetBranchFromRepo(tt.clonePath) + if (err != nil) != tt.wantErr { + t.Errorf("TestGetBranchFromRepo() unexpected error: %v", err) + } + if err != nil { + if branch != tt.want { + t.Errorf("TestGetBranchFromRepo() unexpected branch, expected %v got %v", tt.want, branch) + } + } + }) + } +} + func TestConvertGitHubURL(t *testing.T) { tests := []struct { name string diff --git a/controllers/componentdetectionquery_controller.go b/controllers/componentdetectionquery_controller.go index 88a46ed0..b61e9065 100644 --- a/controllers/componentdetectionquery_controller.go +++ b/controllers/componentdetectionquery_controller.go @@ -159,32 +159,6 @@ func (r *ComponentDetectionQueryReconciler) Reconcile(ctx context.Context, req c return ctrl.Result{}, nil } - if source.Revision == "" { - log.Info(fmt.Sprintf("Look for default branch of repo %s... %v", source.URL, req.NamespacedName)) - metricsLabel := prometheus.Labels{"controller": cdqName, "tokenName": ghClient.TokenName, "operation": "GetDefaultBranchFromURL"} - metrics.ControllerGitRequest.With(metricsLabel).Inc() - source.Revision, err = ghClient.GetDefaultBranchFromURL(sourceURL, ctx) - metrics.HandleRateLimitMetrics(err, metricsLabel) - if err != nil { - log.Error(err, fmt.Sprintf("Unable to get default branch of Github Repo %v, try to fall back to main branch... %v", source.URL, req.NamespacedName)) - metricsLabel := prometheus.Labels{"controller": cdqName, "tokenName": ghClient.TokenName, "operation": "GetBranchFromURL"} - metrics.ControllerGitRequest.With(metricsLabel).Inc() - _, err := ghClient.GetBranchFromURL(sourceURL, ctx, "main") - if err != nil { - metrics.HandleRateLimitMetrics(err, metricsLabel) - log.Error(err, fmt.Sprintf("Unable to get main branch of Github Repo %v ... %v", source.URL, req.NamespacedName)) - retErr := fmt.Errorf("unable to get default branch of Github Repo %v, try to fall back to main branch, failed to get main branch... %v", source.URL, req.NamespacedName) - r.SetCompleteConditionAndUpdateCR(ctx, req, &componentDetectionQuery, copiedCDQ, retErr) - return ctrl.Result{}, nil - } else { - source.Revision = "main" - } - } - } - - // set in the CDQ spec - componentDetectionQuery.Spec.GitSource.Revision = source.Revision - if source.DevfileURL == "" { isDockerfilePresent := false isDevfilePresent := false @@ -215,7 +189,8 @@ func (r *ComponentDetectionQueryReconciler) Reconcile(ctx context.Context, req c CreateK8sJob: false, } - devfilesMapReturned, devfilesURLMapReturned, dockerfileContextMapReturned, componentPortsMapReturned, err := cdqanalysis.CloneAndAnalyze(k8sInfoClient, gitToken, req.Namespace, req.Name, context, devfilePath, dockerfilePath, source.URL, source.Revision, r.DevfileRegistryURL, isDevfilePresent, isDockerfilePresent) + devfilesMapReturned, devfilesURLMapReturned, dockerfileContextMapReturned, componentPortsMapReturned, branch, err := cdqanalysis.CloneAndAnalyze(k8sInfoClient, gitToken, req.Namespace, req.Name, context, devfilePath, dockerfilePath, source.URL, source.Revision, r.DevfileRegistryURL, isDevfilePresent, isDockerfilePresent) + componentDetectionQuery.Spec.GitSource.Revision = branch if err != nil { log.Error(err, fmt.Sprintf("Error running cdq analysis... %v", req.NamespacedName)) r.SetCompleteConditionAndUpdateCR(ctx, req, &componentDetectionQuery, copiedCDQ, err) @@ -229,6 +204,33 @@ func (r *ComponentDetectionQueryReconciler) Reconcile(ctx context.Context, req c } else { log.Info(fmt.Sprintf("devfile was explicitly specified at %s %v", source.DevfileURL, req.NamespacedName)) + // For scenarios where a devfile is passed in, we still need to use the GH API for branch detection as we do not clone. + if source.Revision == "" { + log.Info(fmt.Sprintf("Look for default branch of repo %s... %v", source.URL, req.NamespacedName)) + metricsLabel := prometheus.Labels{"controller": cdqName, "tokenName": ghClient.TokenName, "operation": "GetDefaultBranchFromURL"} + metrics.ControllerGitRequest.With(metricsLabel).Inc() + source.Revision, err = ghClient.GetDefaultBranchFromURL(sourceURL, ctx) + metrics.HandleRateLimitMetrics(err, metricsLabel) + if err != nil { + log.Error(err, fmt.Sprintf("Unable to get default branch of Github Repo %v, try to fall back to main branch... %v", source.URL, req.NamespacedName)) + metricsLabel := prometheus.Labels{"controller": cdqName, "tokenName": ghClient.TokenName, "operation": "GetBranchFromURL"} + metrics.ControllerGitRequest.With(metricsLabel).Inc() + _, err := ghClient.GetBranchFromURL(sourceURL, ctx, "main") + if err != nil { + metrics.HandleRateLimitMetrics(err, metricsLabel) + log.Error(err, fmt.Sprintf("Unable to get main branch of Github Repo %v ... %v", source.URL, req.NamespacedName)) + retErr := fmt.Errorf("unable to get default branch of Github Repo %v, try to fall back to main branch, failed to get main branch... %v", source.URL, req.NamespacedName) + r.SetCompleteConditionAndUpdateCR(ctx, req, &componentDetectionQuery, copiedCDQ, retErr) + return ctrl.Result{}, nil + } else { + source.Revision = "main" + } + } + } + + // set in the CDQ spec + componentDetectionQuery.Spec.GitSource.Revision = source.Revision + shouldIgnoreDevfile, devfileBytes, err := cdqanalysis.ValidateDevfile(log, source.DevfileURL) if err != nil { // if a direct devfileURL is provided and errors out, we dont do an alizer detection diff --git a/controllers/componentdetectionquery_controller_test.go b/controllers/componentdetectionquery_controller_test.go index 2655658d..965a3f47 100644 --- a/controllers/componentdetectionquery_controller_test.go +++ b/controllers/componentdetectionquery_controller_test.go @@ -569,7 +569,7 @@ var _ = Describe("Component Detection Query controller", func() { // index is 1 because of CDQ status condition Processing Expect(createdHasCompDetectionQuery.Status.Conditions[1].Status).Should(Equal(metav1.ConditionFalse)) - Expect(createdHasCompDetectionQuery.Status.Conditions[1].Message).Should(ContainSubstring("error getting devfile info from url: failed to retrieve")) + Expect(createdHasCompDetectionQuery.Status.Conditions[1].Message).Should(ContainSubstring("failed to clone the repo: exit status 128")) // Delete the specified Detection Query resource deleteCompDetQueryCR(hasCompDetQueryLookupKey) @@ -628,7 +628,7 @@ var _ = Describe("Component Detection Query controller", func() { // index is 1 because of CDQ status condition Processing Expect(createdHasCompDetectionQuery.Status.Conditions[1].Status).Should(Equal(metav1.ConditionFalse)) - Expect(createdHasCompDetectionQuery.Status.Conditions[1].Message).Should(ContainSubstring("error getting devfile info from url: failed to retrieve")) + Expect(createdHasCompDetectionQuery.Status.Conditions[1].Message).Should(ContainSubstring("failed to clone the repo: exit status 128")) // Delete the specified Detection Query resource deleteCompDetQueryCR(hasCompDetQueryLookupKey)