Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-32526] - Handle null @AncestorInPath when selecting Jobs/Dow… #79

Merged
merged 5 commits into from Apr 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/java/hudson/plugins/copyartifact/CopyArtifact.java
Expand Up @@ -578,6 +578,8 @@ public static final class DescriptorImpl extends BuildStepDescriptor<Builder> {

public FormValidation doCheckProjectName(
@AncestorInPath Job<?,?> anc, @QueryParameter String value) {
// JENKINS-32526: Check that it behaves gracefully for an unknown context
if (anc == null) return FormValidation.ok(Messages.CopyArtifact_AncestorIsNull());
// Require CONFIGURE permission on this project
if (!anc.hasPermission(Item.CONFIGURE)) return FormValidation.ok();

Expand Down
Expand Up @@ -177,7 +177,7 @@ protected boolean containsVariable(String str) {
* @return
*/
public FormValidation doCheckUpstreamProjectName(
@AncestorInPath AbstractProject<?,?> project,
@AncestorInPath Job<?,?> project,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should skip the validation if project is null (that is, when configuring a template).

@QueryParameter String upstreamProjectName
) {
upstreamProjectName = StringUtils.trim(upstreamProjectName);
Expand All @@ -194,10 +194,20 @@ public FormValidation doCheckUpstreamProjectName(
// Jenkins is unavailable and validation is useless.
return FormValidation.ok();
}

AbstractProject<?,?> upstreamProject = jenkins.getItem(
upstreamProjectName, project.getRootProject(), AbstractProject.class

if (project == null) {
// Context is unknown and validation is useless.
return FormValidation.ok(Messages.CopyArtifact_AncestorIsNull());
}

Job<?,?> upstreamRoot = (project instanceof AbstractProject)
? ((AbstractProject<?,?>) project).getRootProject()
: project;

Job<?,?> upstreamProject = jenkins.getItem(
upstreamProjectName, upstreamRoot, Job.class
);

if (upstreamProject == null || !upstreamProject.hasPermission(Item.READ)) {
return FormValidation.error(Messages.DownstreamBuildSelector_UpstreamProjectName_NotFound());
}
Expand All @@ -210,7 +220,7 @@ public FormValidation doCheckUpstreamProjectName(
* @return
*/
public FormValidation doCheckUpstreamBuildNumber(
@AncestorInPath AbstractProject<?,?> project,
@AncestorInPath Job<?,?> project,
@QueryParameter String upstreamProjectName,
@QueryParameter String upstreamBuildNumber
) {
Expand All @@ -237,10 +247,20 @@ public FormValidation doCheckUpstreamBuildNumber(
// Jenkins is unavailable and validation is useless.
return FormValidation.ok();
}


if (project == null) {
// Context is unknown and validation is useless.
return FormValidation.ok(Messages.CopyArtifact_AncestorIsNull());
}

Job<?,?> upstreamRoot = (project instanceof AbstractProject)
? ((AbstractProject<?,?>) project).getRootProject()
: project;

AbstractProject<?,?> upstreamProject = jenkins.getItem(
upstreamProjectName, project.getRootProject(), AbstractProject.class
upstreamProjectName, upstreamRoot, AbstractProject.class
);

if (upstreamProject == null || !upstreamProject.hasPermission(Item.READ)) {
return FormValidation.ok();
}
Expand Down Expand Up @@ -289,10 +309,12 @@ public FormValidation doCheckUpstreamBuildNumber(
*/
public AutoCompletionCandidates doAutoCompleteUpstreamProjectName(
@QueryParameter String value,
@AncestorInPath AbstractProject<?,?> project
@AncestorInPath Job<?,?> project
) {
// Specified Item to allow to autocomplete folders (maybe confusing...).
return AutoCompletionCandidates.ofJobNames(Item.class, value, project, project.getParent());
return project == null
? new AutoCompletionCandidates()
: AutoCompletionCandidates.ofJobNames(Item.class, value, project, project.getParent());
}
}
}
@@ -1,3 +1,4 @@
CopyArtifact.AncestorIsNull=Context Unknown: the value specified cannot be validated
CopyArtifact.Copied=Copied {0} {0,choice,0#artifacts|1#artifact|1<artifacts} from "{1}" build number {2}
CopyArtifact.DisplayName=Copy artifacts from another project
CopyArtifact.FailedToCopy=Failed to copy artifacts from {0} with filter: {1}
Expand Down
Expand Up @@ -1155,10 +1155,16 @@ public void testFieldValidation() throws Exception {
assertSame(FormValidation.Kind.ERROR, descriptor.doCheckProjectName(p, "").kind);
// Parameterized value
assertSame(FormValidation.Kind.WARNING, descriptor.doCheckProjectName(p, "$FOO").kind);
//JENKINS-32526: Check that it behaves gracefully for an unknown context.
assertSame(FormValidation.Kind.OK, descriptor.doCheckProjectName(null, p.getFullName()).kind);
assertSame(FormValidation.Kind.OK, descriptor.doCheckProjectName(null, "").kind);
assertSame(FormValidation.Kind.OK, descriptor.doCheckProjectName(null, "$FOO").kind);

// Just returns OK if no permission
hudson.setAuthorizationStrategy(new GlobalMatrixAuthorizationStrategy());
SecurityContextHolder.clearContext();
assertSame(FormValidation.Kind.OK, descriptor.doCheckProjectName(p, "").kind);
assertSame(FormValidation.Kind.OK, descriptor.doCheckProjectName(null, "").kind);
// Other descriptor methods
assertTrue(descriptor.isApplicable(null));
assertTrue(descriptor.getDisplayName().length() > 0);
Expand Down
Expand Up @@ -30,10 +30,9 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;

import hudson.plugins.copyartifact.testutils.CopyArtifactUtil;
import jenkins.model.Jenkins;
import hudson.FilePath;
import hudson.model.AbstractProject;
import hudson.model.Cause;
import hudson.model.FreeStyleProject;
import hudson.model.FreeStyleBuild;
Expand All @@ -44,6 +43,9 @@
import hudson.model.StringParameterValue;
import hudson.model.User;
import hudson.model.Result;
import hudson.plugins.copyartifact.testutils.CopyArtifactUtil;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like wildcard import. (especially for classes)
Do you actually need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No sorry. That's the IDE that organized import automatically.

import jenkins.model.Jenkins;
import hudson.FilePath;
import hudson.plugins.copyartifact.testutils.FileWriteBuilder;
import hudson.security.Permission;
import hudson.security.AuthorizationMatrixProperty;
Expand Down Expand Up @@ -546,13 +548,28 @@ public void testCheckUpstreamProjectName() throws Exception {
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(project2, "../project1").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(project2, "project3").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(project2, "/folder1/project3").kind);

//JENKINS-32526: Check that it behaves gracefully for an unknown context
assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamProjectName(null, null).kind);
assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamProjectName(null, "").kind);
assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamProjectName(null, " ").kind);

//Ancestor null
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(null, "nosuchproject").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(null, "$VAR").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(null, "FOO${VAR}").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(null, "Project\\$").kind); // limitation
//Only relative path from Root works
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(null, "folder1/project2").kind);

// permission check
Authentication a = Jenkins.getAuthentication();
try {
SecurityContextHolder.getContext().setAuthentication(User.get("devel").impersonate());
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(project2, "../project1").kind);
assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamProjectName(project2, "project3").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(null, "/project1").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamProjectName(null, "project3").kind);
} finally {
SecurityContextHolder.getContext().setAuthentication(a);
}
Expand Down Expand Up @@ -605,14 +622,77 @@ public void testCheckUpstreamBuildNumber() throws Exception {

assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamBuildNumber(project1, "project2", "9999").kind);
assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamBuildNumber(project1, "project2", "NosuchBuild").kind);


//JENKINS-32526: Check that it behaves gracefully for an unknown context
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "", Integer.toString(build1.getNumber())).kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "$VAR", Integer.toString(build1.getNumber())).kind);

assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamBuildNumber(null, "project2", null).kind);
assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamBuildNumber(null, "project2", "").kind);
assertEquals(FormValidation.Kind.ERROR, d.doCheckUpstreamBuildNumber(null, "project2", " ").kind);

assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project2", "FOO${VAR}").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project2", "\\${VAR}").kind); // limitation

assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project2", Integer.toString(build1.getNumber())).kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project2", build1.getId()).kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project2", build1.getDisplayName()).kind);

assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project2", "9999").kind);
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project2", "NosuchBuild").kind);

// permission check
Authentication a = Jenkins.getAuthentication();
try {
SecurityContextHolder.getContext().setAuthentication(User.get("devel").impersonate());
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(project1, "project3", "nosuchbuild").kind); // limitation
assertEquals(FormValidation.Kind.OK, d.doCheckUpstreamBuildNumber(null, "project3", "nosuchbuild").kind);
} finally {
SecurityContextHolder.getContext().setAuthentication(a);
}
}

@Test
public void testAutoCompleteUpstreamProjectName() throws Exception {
DownstreamBuildSelector.DescriptorImpl d = (DownstreamBuildSelector.DescriptorImpl) j.jenkins.getDescriptorOrDie(DownstreamBuildSelector.class);

ProjectMatrixAuthorizationStrategy pmas = new ProjectMatrixAuthorizationStrategy();
pmas.add(Jenkins.READ, "devel");

j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
j.jenkins.setAuthorizationStrategy(pmas);

// project1
// project2 cannot read from devel
FreeStyleProject project1 = j.createFreeStyleProject("project1");
{
Map<Permission, Set<String>> map = new HashMap<Permission, Set<String>>();
map.put(Item.READ, Sets.newHashSet("devel"));
project1.addProperty(new AuthorizationMatrixProperty(map));
}

FreeStyleProject project2 = j.createFreeStyleProject("project2");

//Check Empty strings
testAutoCompleteUpstreamProjectName(new String [] {project1.getName(), project2.getName()}, "", project1, d);
//Check simple matching string
testAutoCompleteUpstreamProjectName(new String [] {project1.getName(), project2.getName()}, "proj", project1, d);
//Check non matching string
testAutoCompleteUpstreamProjectName(new String [] {}, "FOO", project1, d);
//Check matching string
testAutoCompleteUpstreamProjectName(new String [] {project1.getName()}, "project1", project2, d);
}

private void testAutoCompleteUpstreamProjectName(
String [] expectedValues,
String value,
AbstractProject project,
DownstreamBuildSelector.DescriptorImpl d) {

Set<String> actualValues = new TreeSet<String>(d.doAutoCompleteUpstreamProjectName(value, project).getValues());
assertArrayEquals(expectedValues, actualValues.toArray(new String [actualValues.size()]));
//JENKINS-32526: Auto-completion disabled if no context
actualValues = new TreeSet<String>(d.doAutoCompleteUpstreamProjectName(value, null).getValues());
assertArrayEquals(new String[] {}, actualValues.toArray(new String [actualValues.size()]));
}
}