Merge pull request #4148 from n2ygk/git_ref

pass in the specified git branch ref when cloning a single branch

.changeset/breezy-meals-lie.md

@@ -0,0 +1,20 @@
+---
+'@backstage/backend-common': patch
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Honor the branch ref in the url when cloning.
+
+This fixes a bug in the scaffolder prepare stage where a non-default branch
+was specified in the scaffolder URL but the default branch was cloned.
+For example, even though the `other` branch is specified in this example, the
+`master` branch was actually cloned:
+
+```yaml
+catalog:
+  locations:
+    - type: url
+      target: https://github.com/backstage/backstage/blob/other/plugins/scaffolder-backend/sample-templates/docs-template/template.yaml
+```
+
+This also fixes a 404 in the prepare stage for GitLab URLs.

packages/backend-common/src/scm/git.ts

@@ -86,13 +86,22 @@ export class Git {
     return git.commit({ fs, dir, message, author, committer });
   }
 
-  async clone({ url, dir }: { url: string; dir: string }): Promise<void> {
+  async clone({
+    url,
+    dir,
+    ref,
+  }: {
+    url: string;
+    dir: string;
+    ref?: string;
+  }): Promise<void> {
     this.config.logger?.info(`Cloning repo {dir=${dir},url=${url}}`);
     return git.clone({
       fs,
       http,
       url,
       dir,
+      ref,
       singleBranch: true,
       depth: 1,
       onProgress: this.onProgressHandler(),

plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.test.ts

@@ -45,7 +45,7 @@ describe('AzurePreparer', () => {
       metadata: {
         annotations: {
           [LOCATION_ANNOTATION]:
-            'url:https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=%2Ftemplate.yaml',
+            'url:https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=%2Ftemplate.yaml&version=GBmaster',
         },
         name: 'graphql-starter',
         title: 'GraphQL Service',
@@ -112,6 +112,7 @@ describe('AzurePreparer', () => {
       url:
         'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo',
       dir: expect.any(String),
+      ref: 'master',
     });
   });
 
@@ -124,6 +125,7 @@ describe('AzurePreparer', () => {
       url:
         'https://dev.azure.com/backstage-org/backstage-project/_git/template-repo',
       dir: expect.any(String),
+      ref: 'master',
     });
   });
 

plugins/scaffolder-backend/src/scaffolder/stages/prepare/azure.ts

@@ -42,6 +42,7 @@ export class AzurePreparer implements PreparerBase {
 
     const parsedGitLocation = parseGitUrl(location);
     const repositoryCheckoutUrl = parsedGitLocation.toString('https');
+    const ref = parsedGitLocation.ref;
     const tempDir = await fs.promises.mkdtemp(
       path.join(workingDirectory, templateId),
     );
@@ -63,6 +64,7 @@ export class AzurePreparer implements PreparerBase {
 
     await git.clone({
       url: repositoryCheckoutUrl,
+      ref: ref,
       dir: tempDir,
     });
 

plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.test.ts

@@ -89,6 +89,7 @@ describe('BitbucketPreparer', () => {
     expect(mockGitClient.clone).toHaveBeenCalledWith({
       url: 'https://bitbucket.org/backstage-project/backstage-repo',
       dir: expect.any(String),
+      ref: expect.any(String),
     });
   });
 
@@ -113,6 +114,7 @@ describe('BitbucketPreparer', () => {
     expect(mockGitClient.clone).toHaveBeenCalledWith({
       url: 'https://bitbucket.org/backstage-project/backstage-repo',
       dir: expect.any(String),
+      ref: expect.any(String),
     });
   });
 

plugins/scaffolder-backend/src/scaffolder/stages/prepare/bitbucket.ts

@@ -49,15 +49,15 @@ export class BitbucketPreparer implements PreparerBase {
     const logger = opts.logger;
     const templateId = template.metadata.name;
 
-    const repo = parseGitUrl(location);
-    const repositoryCheckoutUrl = repo.toString('https');
-
+    const parsedGitLocation = parseGitUrl(location);
+    const repositoryCheckoutUrl = parsedGitLocation.toString('https');
+    const ref = parsedGitLocation.ref;
     const tempDir = await fs.promises.mkdtemp(
       path.join(workingDirectory, templateId),
     );
 
     const templateDirectory = path.join(
-      `${path.dirname(repo.filepath)}`,
+      `${path.dirname(parsedGitLocation.filepath)}`,
       template.spec.path ?? '.',
     );
 
@@ -73,6 +73,7 @@ export class BitbucketPreparer implements PreparerBase {
 
     await git.clone({
       url: repositoryCheckoutUrl,
+      ref: ref,
       dir: tempDir,
     });
 

plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts

@@ -89,6 +89,7 @@ describe('GitHubPreparer', () => {
     expect(mockGitClient.clone).toHaveBeenCalledWith({
       url: 'https://github.com/benjdlambert/backstage-graphql-template',
       dir: expect.any(String),
+      ref: expect.any(String),
     });
   });
 
@@ -100,6 +101,7 @@ describe('GitHubPreparer', () => {
     expect(mockGitClient.clone).toHaveBeenCalledWith({
       url: 'https://github.com/benjdlambert/backstage-graphql-template',
       dir: expect.any(String),
+      ref: expect.any(String),
     });
   });
 

plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts

@@ -42,6 +42,7 @@ export class GithubPreparer implements PreparerBase {
 
     const parsedGitLocation = parseGitUrl(location);
     const repositoryCheckoutUrl = parsedGitLocation.toString('https');
+    const ref = parsedGitLocation.ref;
     const tempDir = await fs.promises.mkdtemp(
       path.join(workingDirectory, templateId),
     );
@@ -63,6 +64,7 @@ export class GithubPreparer implements PreparerBase {
 
     await git.clone({
       url: repositoryCheckoutUrl,
+      ref: ref,
       dir: tempDir,
     });
 

plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.test.ts

@@ -87,8 +87,9 @@ describe('GitLabPreparer', () => {
     await preparer.prepare(mockEntity, { logger: getVoidLogger() });
 
     expect(mockGitClient.clone).toHaveBeenCalledWith({
-      url: 'https://gitlab.com/benjdlambert/backstage-graphql-template',
+      url: 'https://gitlab.com/benjdlambert/backstage-graphql-template.git',
       dir: expect.any(String),
+      ref: expect.any(String),
     });
   });
 
@@ -111,8 +112,9 @@ describe('GitLabPreparer', () => {
     await preparer.prepare(mockEntity, { logger: getVoidLogger() });
 
     expect(mockGitClient.clone).toHaveBeenCalledWith({
-      url: 'https://gitlab.com/benjdlambert/backstage-graphql-template',
+      url: 'https://gitlab.com/benjdlambert/backstage-graphql-template.git',
       dir: expect.any(String),
+      ref: expect.any(String),
     });
   });
 

plugins/scaffolder-backend/src/scaffolder/stages/prepare/gitlab.ts

@@ -41,7 +41,9 @@ export class GitlabPreparer implements PreparerBase {
     const templateId = template.metadata.name;
 
     const parsedGitLocation = parseGitUrl(location);
+    parsedGitLocation.git_suffix = true;
     const repositoryCheckoutUrl = parsedGitLocation.toString('https');
+    const ref = parsedGitLocation.ref;
     const tempDir = await fs.promises.mkdtemp(
       path.join(workingDirectory, templateId),
     );
@@ -61,6 +63,7 @@ export class GitlabPreparer implements PreparerBase {
 
     await git.clone({
       url: repositoryCheckoutUrl,
+      ref: ref,
       dir: tempDir,
     });
 

yarn.lock

@@ -14187,6 +14187,13 @@ git-url-parse@^11.4.4:
   dependencies:
     git-up "^4.0.0"
 
+git-url-parse@^11.4.4:
+  version "11.4.4"
+  resolved "https://registry.npmjs.org/git-url-parse/-/git-url-parse-11.4.4.tgz#5d747debc2469c17bc385719f7d0427802d83d77"
+  integrity sha512-Y4o9o7vQngQDIU9IjyCmRJBin5iYjI5u9ZITnddRZpD7dcCFQj2sL2XuMNbLRE4b4B/4ENPsp2Q8P44fjAZ0Pw==
+  dependencies:
+    git-up "^4.0.0"
+
 gitconfiglocal@^1.0.0:
   version "1.0.0"
   resolved "https://registry.npmjs.org/gitconfiglocal/-/gitconfiglocal-1.0.0.tgz#41d045f3851a5ea88f03f24ca1c6178114464b9b"