From 00317b4c2e2f75af52c11fad4fca809bebd94a8b Mon Sep 17 00:00:00 2001 From: Ray Vincent Date: Sun, 7 Feb 2021 12:39:32 -0800 Subject: [PATCH 1/3] JENKINS-60630: Add GitSCMFileSystem support for EnvVar expansion --- .../jenkins/plugins/git/GitSCMFileSystem.java | 94 ++++++++---- .../plugins/git/GitSCMFileSystemTest.java | 140 ++++++++++++++++++ 2 files changed, 206 insertions(+), 28 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index c3c5a16e04..2474acfff5 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -35,6 +35,8 @@ import hudson.EnvVars; import hudson.Extension; import hudson.model.Item; +import hudson.model.Job; +import hudson.model.Run; import hudson.model.TaskListener; import hudson.plugins.git.BranchSpec; import hudson.plugins.git.GitException; @@ -252,19 +254,29 @@ public static class BuilderImpl extends SCMFileSystem.Builder { @Override public boolean supports(SCM source) { - return source instanceof GitSCM - && ((GitSCM) source).getUserRemoteConfigs().size() == 1 - && ((GitSCM) source).getBranches().size() == 1 - && !((GitSCM) source).getBranches().get(0).getName().equals("*") // JENKINS-57587 - && ( - ((GitSCM) source).getBranches().get(0).getName().matches( - "^((\\Q" + Constants.R_HEADS + "\\E.*)|([^/]+)|(\\*/[^/*]+(/[^/*]+)*))$" - ) - || ((GitSCM) source).getBranches().get(0).getName().matches( - "^((\\Q" + Constants.R_TAGS + "\\E.*)|([^/]+)|(\\*/[^/*]+(/[^/*]+)*))$" + if (source instanceof GitSCM && ((GitSCM) source).getBranches().size() > 0) { + for (BranchSpec branchSpec : ((GitSCM) source).getBranches()) { + String branch = branchSpec.getName(); + + // we only support where the branch spec is obvious and not a wildcard + if (!( + !branch.equals("*") // JENKINS-57587 + && ( + branch.matches( + "^((\\Q" + Constants.R_HEADS + "\\E.*)|([^/]+)|(\\*/[^/*]+(/[^/*]+)*))$" + ) + || branch.matches( + "^((\\Q" + Constants.R_TAGS + "\\E.*)|([^/]+)|(\\*/[^/*]+(/[^/*]+)*))$" + ) ) - ); - // we only support where the branch spec is obvious and not a wildcard + )) { + return false; + } + } + return true; + } else { + return false; + } } @Override @@ -293,7 +305,6 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull } GitSCM gitSCM = (GitSCM) scm; UserRemoteConfig config = gitSCM.getUserRemoteConfigs().get(0); - BranchSpec branchSpec = gitSCM.getBranches().get(0); String remote = config.getUrl(); TaskListener listener = new LogTaskListener(LOGGER, Level.FINE); if (remote == null) { @@ -343,26 +354,53 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull } catch (URISyntaxException ex) { listener.getLogger().println("URI syntax exception for '" + remoteName + "' " + ex); } - String prefix = Constants.R_HEADS; - if(branchSpec.getName().startsWith(Constants.R_TAGS)){ - prefix = Constants.R_TAGS; - } - String headName; + // Initialize to master before overriding + // It will always be overridden since at least once branch must be provided + String headName = "master"; + String prefix = Constants.R_HEADS; if (rev != null) { headName = rev.getHead().getName(); + client.fetch_().prune(true).from(remoteURI, Arrays.asList(new RefSpec( + "+" + prefix + headName + ":" + Constants.R_REMOTES + remoteName + "/" + + headName))).execute(); } else { - if (branchSpec.getName().startsWith(prefix)){ - headName = branchSpec.getName().substring(prefix.length()); - } else if (branchSpec.getName().startsWith("*/")) { - headName = branchSpec.getName().substring(2); - } else { - headName = branchSpec.getName(); + EnvVars env = new EnvVars(); + if (owner instanceof Job + && ((Job) owner).getLastBuild() != null + && ((Job) owner).getLastBuild().isBuilding() + ) { + Run run = ((Job) owner).getLastBuild(); + env = run.getEnvironment(listener); + } + + for (BranchSpec branchSpec : gitSCM.getBranches()) { + if (branchSpec.getName().startsWith(Constants.R_HEADS)){ + prefix = Constants.R_HEADS; + headName = branchSpec.getName().substring(prefix.length()); + } else if (branchSpec.getName().startsWith(Constants.R_TAGS)){ + prefix = Constants.R_TAGS; + headName = branchSpec.getName().substring(prefix.length()); + } else if (branchSpec.getName().startsWith("*/")) { + headName = branchSpec.getName().substring(2); + } else { + headName = branchSpec.getName(); + } + headName = env.expand(headName); + headName = headName.replaceAll("\\$\\{[^\\}]+\\}", ""); + + // If the EnvVar expaned branch name is not empty, then break + if (!headName.equals("")) { + try { + client.fetch_().prune(true).from(remoteURI, Arrays.asList(new RefSpec( + "+" + prefix + headName + ":" + Constants.R_REMOTES + remoteName + "/" + + headName))).execute(); + break; + } catch(Exception exc) { + continue; + } + } } } - client.fetch_().prune(true).from(remoteURI, Arrays - .asList(new RefSpec( - "+" + prefix + headName + ":" + Constants.R_REMOTES + remoteName + "/" - + headName))).execute(); listener.getLogger().println("Done."); return new GitSCMFileSystem(client, remote, Constants.R_REMOTES + remoteName + "/" +headName, (AbstractGitSCMSource.SCMRevisionImpl) rev); } finally { diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index b4dde88a66..beba81f809 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -26,6 +26,8 @@ package jenkins.plugins.git; import hudson.EnvVars; +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; import hudson.model.TaskListener; import hudson.plugins.git.BranchSpec; import hudson.plugins.git.GitSCM; @@ -33,8 +35,10 @@ import hudson.plugins.git.GitException; import java.io.ByteArrayOutputStream; import java.io.File; +import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.Set; import java.util.TreeSet; import jenkins.scm.api.SCMFile; @@ -52,6 +56,7 @@ import org.junit.Test; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.mockito.Mockito; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; @@ -65,6 +70,7 @@ import static org.junit.Assert.assertFalse; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; /** * Tests for {@link AbstractGitSCMSource} @@ -195,6 +201,140 @@ public void slashyBranches() throws Exception { assertThat(file.contentAsString(), is("modified")); } + @Issue("JENKINS-60630") + @Test + public void envVarBranchFromLastBuild() throws Exception { + EnvVars env = new EnvVars(); + env.put("BRANCH_SOURCE", "bug/envBranch"); + + FreeStyleProject project = r.createFreeStyleProject(); + FreeStyleProject projectSpy = Mockito.spy(project); + FreeStyleBuild buildSpy = Mockito.spy(new FreeStyleBuild(project)); + Mockito.when(buildSpy.isBuilding()).thenReturn(true); + Mockito.when(buildSpy.getEnvironment(any())).thenReturn(env); + Mockito.when(projectSpy.getLastBuild()).thenReturn(buildSpy); + + sampleRepo.init(); + sampleRepo.git("checkout", "-b", "bug/envBranch"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), Collections.singletonList(new BranchSpec("*/${BRANCH_SOURCE}")), false, Collections.emptyList(), null, null, Collections.emptyList())); + assertThat(fs, notNullValue()); + SCMFile root = fs.getRoot(); + assertThat(root, notNullValue()); + assertTrue(root.isRoot()); + Iterable children = root.children(); + Iterator iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile file = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(file.getName(), is("file")); + assertThat(file.contentAsString(), is("modified")); + } + + @Issue("JENKINS-60630") + @Test + public void firstEnvVarBranchEmpty() throws Exception { + EnvVars env = new EnvVars(); + env.put("BRANCH_TARGET", "bug/envBranch"); + + FreeStyleProject project = r.createFreeStyleProject(); + FreeStyleProject projectSpy = Mockito.spy(project); + FreeStyleBuild buildSpy = Mockito.spy(new FreeStyleBuild(project)); + Mockito.when(buildSpy.isBuilding()).thenReturn(true); + Mockito.when(buildSpy.getEnvironment(any())).thenReturn(env); + Mockito.when(projectSpy.getLastBuild()).thenReturn(buildSpy); + + sampleRepo.init(); + sampleRepo.git("checkout", "-b", "bug/envBranch"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + List branches = new ArrayList(); + branches.add(new BranchSpec("*/${BRANCH_SOURCE}")); + branches.add(new BranchSpec("*/${BRANCH_TARGET}")); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, false, Collections.emptyList(), null, null, Collections.emptyList())); + assertThat(fs, notNullValue()); + SCMFile root = fs.getRoot(); + assertThat(root, notNullValue()); + assertTrue(root.isRoot()); + Iterable children = root.children(); + Iterator iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile file = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(file.getName(), is("file")); + assertThat(file.contentAsString(), is("modified")); + } + + @Issue("JENKINS-60630") + @Test + public void firstEnvVarBranchValid() throws Exception { + EnvVars env = new EnvVars(); + env.put("BRANCH_SOURCE", "bug/envBranch"); + + FreeStyleProject project = r.createFreeStyleProject(); + FreeStyleProject projectSpy = Mockito.spy(project); + FreeStyleBuild buildSpy = Mockito.spy(new FreeStyleBuild(project)); + Mockito.when(buildSpy.isBuilding()).thenReturn(true); + Mockito.when(buildSpy.getEnvironment(any())).thenReturn(env); + Mockito.when(projectSpy.getLastBuild()).thenReturn(buildSpy); + + sampleRepo.init(); + sampleRepo.git("checkout", "-b", "bug/envBranch"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + List branches = new ArrayList(); + branches.add(new BranchSpec("*/${BRANCH_SOURCE}")); + branches.add(new BranchSpec("*/${BRANCH_TARGET}")); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, false, Collections.emptyList(), null, null, Collections.emptyList())); + assertThat(fs, notNullValue()); + SCMFile root = fs.getRoot(); + assertThat(root, notNullValue()); + assertTrue(root.isRoot()); + Iterable children = root.children(); + Iterator iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile file = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(file.getName(), is("file")); + assertThat(file.contentAsString(), is("modified")); + } + + @Issue("JENKINS-60630") + @Test + public void firstEnvVarBranchNotFoundSecondBranchValid() throws Exception { + EnvVars env = new EnvVars(); + env.put("BRANCH_SOURCE", "bug/deletedBranch"); + env.put("BRANCH_TARGET", "bug/envBranch"); + + FreeStyleProject project = r.createFreeStyleProject(); + FreeStyleProject projectSpy = Mockito.spy(project); + FreeStyleBuild buildSpy = Mockito.spy(new FreeStyleBuild(project)); + Mockito.when(buildSpy.isBuilding()).thenReturn(true); + Mockito.when(buildSpy.getEnvironment(any())).thenReturn(env); + Mockito.when(projectSpy.getLastBuild()).thenReturn(buildSpy); + + sampleRepo.init(); + sampleRepo.git("checkout", "-b", "bug/envBranch"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + List branches = new ArrayList(); + branches.add(new BranchSpec("*/${BRANCH_SOURCE}")); + branches.add(new BranchSpec("*/${BRANCH_TARGET}")); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, false, Collections.emptyList(), null, null, Collections.emptyList())); + assertThat(fs, notNullValue()); + SCMFile root = fs.getRoot(); + assertThat(root, notNullValue()); + assertTrue(root.isRoot()); + Iterable children = root.children(); + Iterator iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile file = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(file.getName(), is("file")); + assertThat(file.contentAsString(), is("modified")); + } + @Issue("JENKINS-57587") @Test public void wildcardBranchNameCausesNPE() throws Exception { From 07b777b13b773dbc6564458520d71328597586c8 Mon Sep 17 00:00:00 2001 From: Ray Vincent Date: Mon, 8 Feb 2021 16:23:43 -0800 Subject: [PATCH 2/3] JENKINS-60630: Small Code Coverage Improvement --- .../jenkins/plugins/git/GitSCMFileSystem.java | 2 +- .../plugins/git/GitSCMFileSystemTest.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java index 2474acfff5..80dc07059e 100644 --- a/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java +++ b/src/main/java/jenkins/plugins/git/GitSCMFileSystem.java @@ -254,7 +254,7 @@ public static class BuilderImpl extends SCMFileSystem.Builder { @Override public boolean supports(SCM source) { - if (source instanceof GitSCM && ((GitSCM) source).getBranches().size() > 0) { + if (source instanceof GitSCM) { for (BranchSpec branchSpec : ((GitSCM) source).getBranches()) { String branch = branchSpec.getName(); diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index beba81f809..591b256477 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -335,6 +335,26 @@ public void firstEnvVarBranchNotFoundSecondBranchValid() throws Exception { assertThat(file.contentAsString(), is("modified")); } + @Test + public void branchSpecRefHeads() throws Exception { + sampleRepo.init(); + sampleRepo.git("checkout", "-b", "dev"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), Collections.singletonList(new BranchSpec("refs/heads/dev")), false, Collections.emptyList(), null, null, Collections.emptyList())); + assertThat(fs, notNullValue()); + SCMFile root = fs.getRoot(); + assertThat(root, notNullValue()); + assertTrue(root.isRoot()); + Iterable children = root.children(); + Iterator iterator = children.iterator(); + assertThat(iterator.hasNext(), is(true)); + SCMFile file = iterator.next(); + assertThat(iterator.hasNext(), is(false)); + assertThat(file.getName(), is("file")); + assertThat(file.contentAsString(), is("modified")); + } + @Issue("JENKINS-57587") @Test public void wildcardBranchNameCausesNPE() throws Exception { From c68769e5cdd8f2520928e1709175a935116d09c4 Mon Sep 17 00:00:00 2001 From: Ray Vincent Date: Fri, 26 Feb 2021 14:29:39 -0800 Subject: [PATCH 3/3] Cleanup tests for deprecation of submodule config --- .../java/jenkins/plugins/git/GitSCMFileSystemTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java index 591b256477..094ba82f31 100644 --- a/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java +++ b/src/test/java/jenkins/plugins/git/GitSCMFileSystemTest.java @@ -218,7 +218,7 @@ public void envVarBranchFromLastBuild() throws Exception { sampleRepo.git("checkout", "-b", "bug/envBranch"); sampleRepo.write("file", "modified"); sampleRepo.git("commit", "--all", "--message=dev"); - SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), Collections.singletonList(new BranchSpec("*/${BRANCH_SOURCE}")), false, Collections.emptyList(), null, null, Collections.emptyList())); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), Collections.singletonList(new BranchSpec("*/${BRANCH_SOURCE}")), null, null, Collections.emptyList())); assertThat(fs, notNullValue()); SCMFile root = fs.getRoot(); assertThat(root, notNullValue()); @@ -252,7 +252,7 @@ public void firstEnvVarBranchEmpty() throws Exception { List branches = new ArrayList(); branches.add(new BranchSpec("*/${BRANCH_SOURCE}")); branches.add(new BranchSpec("*/${BRANCH_TARGET}")); - SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, false, Collections.emptyList(), null, null, Collections.emptyList())); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, null, null, Collections.emptyList())); assertThat(fs, notNullValue()); SCMFile root = fs.getRoot(); assertThat(root, notNullValue()); @@ -286,7 +286,7 @@ public void firstEnvVarBranchValid() throws Exception { List branches = new ArrayList(); branches.add(new BranchSpec("*/${BRANCH_SOURCE}")); branches.add(new BranchSpec("*/${BRANCH_TARGET}")); - SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, false, Collections.emptyList(), null, null, Collections.emptyList())); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, null, null, Collections.emptyList())); assertThat(fs, notNullValue()); SCMFile root = fs.getRoot(); assertThat(root, notNullValue()); @@ -321,7 +321,7 @@ public void firstEnvVarBranchNotFoundSecondBranchValid() throws Exception { List branches = new ArrayList(); branches.add(new BranchSpec("*/${BRANCH_SOURCE}")); branches.add(new BranchSpec("*/${BRANCH_TARGET}")); - SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, false, Collections.emptyList(), null, null, Collections.emptyList())); + SCMFileSystem fs = SCMFileSystem.of(projectSpy, new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), branches, null, null, Collections.emptyList())); assertThat(fs, notNullValue()); SCMFile root = fs.getRoot(); assertThat(root, notNullValue()); @@ -341,7 +341,7 @@ public void branchSpecRefHeads() throws Exception { sampleRepo.git("checkout", "-b", "dev"); sampleRepo.write("file", "modified"); sampleRepo.git("commit", "--all", "--message=dev"); - SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), Collections.singletonList(new BranchSpec("refs/heads/dev")), false, Collections.emptyList(), null, null, Collections.emptyList())); + SCMFileSystem fs = SCMFileSystem.of(r.createFreeStyleProject(), new GitSCM(GitSCM.createRepoList(sampleRepo.toString(), null), Collections.singletonList(new BranchSpec("refs/heads/dev")), null, null, Collections.emptyList())); assertThat(fs, notNullValue()); SCMFile root = fs.getRoot(); assertThat(root, notNullValue());