diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 75601792c7..e2e7e7ce69 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -25,6 +25,7 @@ */ package hudson; +import hudson.model.Slave; import jenkins.util.SystemProperties; import hudson.cli.CLICommand; import hudson.console.ConsoleAnnotationDescriptor; @@ -158,6 +159,7 @@ import java.util.concurrent.atomic.AtomicLong; import javax.annotation.CheckForNull; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; +import org.kohsuke.accmod.restrictions.DoNotUse; /** * Utility functions used in views. @@ -889,10 +891,24 @@ public class Functions { return SCM._for(project); } + /** + * @since 2.12 + * @deprecated replaced by {@link Slave.SlaveDescriptor#computerLauncherDescriptors(Slave)} + */ + @Deprecated + @Restricted(DoNotUse.class) + @RestrictedSince("2.12") public static List> getComputerLauncherDescriptors() { return Jenkins.getInstance().>getDescriptorList(ComputerLauncher.class); } + /** + * @since 2.12 + * @deprecated replaced by {@link Slave.SlaveDescriptor#retentionStrategyDescriptors(Slave)} + */ + @Deprecated + @Restricted(DoNotUse.class) + @RestrictedSince("2.12") public static List>> getRetentionStrategyDescriptors() { return RetentionStrategy.all(); } @@ -913,6 +929,13 @@ public class Functions { return MyViewsTabBar.all(); } + /** + * @deprecated replaced by {@link Slave.SlaveDescriptor#nodePropertyDescriptors(Slave)} + * @since 2.12 + */ + @Deprecated + @Restricted(DoNotUse.class) + @RestrictedSince("2.12") public static List getNodePropertyDescriptors(Class clazz) { List result = new ArrayList(); Collection list = (Collection) Jenkins.getInstance().getDescriptorList(NodeProperty.class); diff --git a/core/src/main/java/hudson/model/DescriptorVisibilityFilter.java b/core/src/main/java/hudson/model/DescriptorVisibilityFilter.java index bd5cbfd36e..838e3a0caf 100644 --- a/core/src/main/java/hudson/model/DescriptorVisibilityFilter.java +++ b/core/src/main/java/hudson/model/DescriptorVisibilityFilter.java @@ -3,13 +3,16 @@ package hudson.model; import hudson.ExtensionList; import hudson.ExtensionPoint; import hudson.scm.SCMDescriptor; -import jenkins.ExtensionFilter; - import java.util.ArrayList; import java.util.List; - -import java.util.logging.Logger; +import java.util.WeakHashMap; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import jenkins.ExtensionFilter; +import jenkins.util.SystemProperties; /** * Hides {@link Descriptor}s from users. @@ -22,6 +25,22 @@ public abstract class DescriptorVisibilityFilter implements ExtensionPoint { private static final Logger LOGGER = Logger.getLogger(DescriptorVisibilityFilter.class.getName()); + /** + * Decides if the given descriptor should be visible to the user. + * + * @param contextClass The class of object that indicates where the visibility of a descriptor is evaluated. + * For example, if Jenkins is deciding whether a {@link FreeStyleProject} should gets a + * {@link SCMDescriptor}, the context class will be {@link FreeStyleProject}. + * @param descriptor Descriptor whose visibility is evaluated. Never null. + * @return true to allow the descriptor to be visible. false to hide it. + * If any of the installed {@link DescriptorVisibilityFilter} returns false, + * the descriptor is not shown. + * @since FIXME + */ + public boolean filterType(@Nonnull Class contextClass, @Nonnull Descriptor descriptor) { + return true; + } + /** * Decides if the given descriptor should be visible to the user. * @@ -38,7 +57,7 @@ public abstract class DescriptorVisibilityFilter implements ExtensionPoint { * If any of the installed {@link DescriptorVisibilityFilter} returns false, * the descriptor is not shown. */ - public abstract boolean filter(Object context, Descriptor descriptor); + public abstract boolean filter(@CheckForNull Object context, @Nonnull Descriptor descriptor); public static ExtensionList all() { return ExtensionList.lookup(DescriptorVisibilityFilter.class); @@ -47,7 +66,8 @@ public abstract class DescriptorVisibilityFilter implements ExtensionPoint { public static List apply(Object context, Iterable source) { ExtensionList filters = all(); List r = new ArrayList(); - + Class contextClass = context == null ? null : context.getClass(); + OUTER: for (T d : source) { if (LOGGER.isLoggable(Level.FINE)) { @@ -57,10 +77,24 @@ public abstract class DescriptorVisibilityFilter implements ExtensionPoint { if (LOGGER.isLoggable(Level.FINER)) { LOGGER.finer("Querying " + f + " for visibility of " + d + " in " + context); } - if (!f.filter(context,d)) { - if (LOGGER.isLoggable(Level.CONFIG)) { - LOGGER.config("Filter " + f + " hides " + d + " in context " + context); + try { + if (contextClass != null && !f.filterType(contextClass, d)) { + if (LOGGER.isLoggable(Level.CONFIG)) { + LOGGER.config("Filter " + f + " hides " + d + " in contexts of type " + contextClass); + } + continue OUTER; // veto-ed. not shown } + if (!f.filter(context, d)) { + if (LOGGER.isLoggable(Level.CONFIG)) { + LOGGER.config("Filter " + f + " hides " + d + " in context " + context); + } + continue OUTER; // veto-ed. not shown + } + } catch (Error e) { + LOGGER.log(Level.WARNING, "Encountered error while processing filter " + f + " for context " + context, e); + throw e; + } catch (Throwable e) { + LOGGER.log(logLevelFor(f), "Uncaught exception from filter " + f + " for context " + context, e); continue OUTER; // veto-ed. not shown } } @@ -68,4 +102,72 @@ public abstract class DescriptorVisibilityFilter implements ExtensionPoint { } return r; } + + public static List applyType(Class contextClass, Iterable source) { + ExtensionList filters = all(); + List r = new ArrayList(); + + OUTER: + for (T d : source) { + if (LOGGER.isLoggable(Level.FINE)) { + LOGGER.fine("Determining visibility of " + d + " in contexts of type " + contextClass); + } + for (DescriptorVisibilityFilter f : filters) { + if (LOGGER.isLoggable(Level.FINER)) { + LOGGER.finer("Querying " + f + " for visibility of " + d + " in type " + contextClass); + } + try { + if (contextClass != null && !f.filterType(contextClass, d)) { + if (LOGGER.isLoggable(Level.CONFIG)) { + LOGGER.config("Filter " + f + " hides " + d + " in contexts of type " + contextClass); + } + continue OUTER; // veto-ed. not shown + } + } catch (Error e) { + LOGGER.log(Level.WARNING, + "Encountered error while processing filter " + f + " for contexts of type " + contextClass, e); + throw e; + } catch (Throwable e) { + LOGGER.log(logLevelFor(f), "Uncaught exception from filter " + f + " for context of type " + contextClass, e); + continue OUTER; // veto-ed. not shown + } + } + r.add(d); + } + return r; + } + + /** + * Returns the {@link Level} to log an uncaught exception from a {@link DescriptorVisibilityFilter}. We + * need to suppress repeated exceptions as there can be many invocations of the {@link DescriptorVisibilityFilter} + * triggered by the UI and spamming the logs would be bad. + * + * @param f the {@link DescriptorVisibilityFilter}. + * @return the level to report uncaught exceptions at. + */ + private static Level logLevelFor(DescriptorVisibilityFilter f) { + Long interval = SystemProperties.getLong( + DescriptorVisibilityFilter.class.getName() + ".badFilterLogWarningIntervalMinutes", + 60L); + // the healthy path will never see this synchronized block + synchronized (ResourceHolder.BAD_FILTERS) { + Long lastTime = ResourceHolder.BAD_FILTERS.get(f); + if (lastTime == null || lastTime + TimeUnit.MINUTES.toMillis(interval) < System.currentTimeMillis()) { + ResourceHolder.BAD_FILTERS.put(f, System.currentTimeMillis()); + return Level.WARNING; + } else { + return Level.FINE; + } + } + } + + /** + * Lazy initialization singleton for the map of bad filters. Should never be instantiated in a healthy instance. + */ + private static final class ResourceHolder { + /** + * The last time we complained in the logs about specific filters. + */ + private static final WeakHashMap BAD_FILTERS = new WeakHashMap<>(); + } } diff --git a/core/src/main/java/hudson/model/Slave.java b/core/src/main/java/hudson/model/Slave.java index 36d38fdc6f..76fefa1fe1 100644 --- a/core/src/main/java/hudson/model/Slave.java +++ b/core/src/main/java/hudson/model/Slave.java @@ -25,11 +25,11 @@ package hudson.model; import com.google.common.collect.ImmutableSet; +import hudson.DescriptorExtensionList; import hudson.FilePath; import hudson.Launcher; -import hudson.Util; import hudson.Launcher.RemoteLauncher; -import jenkins.util.SystemProperties; +import hudson.Util; import hudson.model.Descriptor.FormException; import hudson.remoting.Callable; import hudson.slaves.CommandLauncher; @@ -44,7 +44,6 @@ import hudson.slaves.SlaveComputer; import hudson.util.ClockDifference; import hudson.util.DescribableList; import hudson.util.FormValidation; - import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -53,21 +52,20 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLConnection; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Set; -import java.util.TreeSet; - -import javax.servlet.ServletException; - import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import javax.servlet.ServletException; import jenkins.model.Jenkins; import jenkins.security.MasterToSlaveCallable; import jenkins.slaves.WorkspaceLocator; - +import jenkins.util.SystemProperties; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.QueryParameter; @@ -488,6 +486,63 @@ public abstract class Slave extends Node implements Serializable { return FormValidation.ok(); } + + /** + * Returns the list of {@link ComputerLauncher} descriptors appropriate to the supplied {@link Slave}. + * + * @param it the {@link Slave} or {@code null} to assume the slave is of type {@link #clazz}. + * @return the filtered list + * @since 2.12 + */ + @Nonnull + @Restricted(NoExternalUse.class) // intedned for use by Jelly EL only (plus hack in DelegatingComputerLauncher) + public final List> computerLauncherDescriptors(@CheckForNull Slave it) { + DescriptorExtensionList> all = + Jenkins.getInstance().>getDescriptorList( + ComputerLauncher.class); + return it == null ? DescriptorVisibilityFilter.applyType(clazz, all) + : DescriptorVisibilityFilter.apply(it, all); + } + + /** + * Returns the list of {@link RetentionStrategy} descriptors appropriate to the supplied {@link Slave}. + * + * @param it the {@link Slave} or {@code null} to assume the slave is of type {@link #clazz}. + * @return the filtered list + * @since 2.12 + */ + @Nonnull + @SuppressWarnings("unchecked") // used by Jelly EL only + @Restricted(NoExternalUse.class) // used by Jelly EL only + public final List>> retentionStrategyDescriptors(@CheckForNull Slave it) { + return it == null ? DescriptorVisibilityFilter.applyType(clazz, RetentionStrategy.all()) + : DescriptorVisibilityFilter.apply(it, RetentionStrategy.all()); + } + + /** + * Returns the list of {@link NodePropertyDescriptor} appropriate to the supplied {@link Slave}. + * + * @param it the {@link Slave} or {@code null} to assume the slave is of type {@link #clazz}. + * @return the filtered list + * @since 2.12 + */ + @Nonnull + @SuppressWarnings("unchecked") // used by Jelly EL only + @Restricted(NoExternalUse.class) // used by Jelly EL only + public final List nodePropertyDescriptors(@CheckForNull Slave it) { + List result = new ArrayList(); + Collection list = + (Collection) Jenkins.getInstance().getDescriptorList(NodeProperty.class); + for (NodePropertyDescriptor npd : it == null + ? DescriptorVisibilityFilter.applyType(clazz, list) + : DescriptorVisibilityFilter.apply(it, list)) { + if (npd.isApplicable(clazz)) { + result.add(npd); + } + } + return result; + } + } diff --git a/core/src/main/java/hudson/slaves/DelegatingComputerLauncher.java b/core/src/main/java/hudson/slaves/DelegatingComputerLauncher.java index 47a9b0815b..510b0db957 100644 --- a/core/src/main/java/hudson/slaves/DelegatingComputerLauncher.java +++ b/core/src/main/java/hudson/slaves/DelegatingComputerLauncher.java @@ -23,13 +23,19 @@ */ package hudson.slaves; -import hudson.Functions; +import hudson.RestrictedSince; import hudson.model.Descriptor; +import hudson.model.Slave; import hudson.model.TaskListener; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import jenkins.model.Jenkins; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.DoNotUse; /** * Convenient base implementation of {@link ComputerLauncher} that allows @@ -70,10 +76,29 @@ public abstract class DelegatingComputerLauncher extends ComputerLauncher { /** * Returns the applicable nested computer launcher types. * The default implementation avoids all delegating descriptors, as that creates infinite recursion. + * @since 2.12 */ + public List> applicableDescriptors(@CheckForNull Slave it, + @Nonnull Slave.SlaveDescriptor itDescriptor) { + List> r = new ArrayList>(); + for (Descriptor d : itDescriptor.computerLauncherDescriptors(it)) { + if (DelegatingComputerLauncher.class.isAssignableFrom(d.getKlass().toJavaClass())) continue; + r.add(d); + } + return r; + } + /** + * Returns the applicable nested computer launcher types. + * The default implementation avoids all delegating descriptors, as that creates infinite recursion. + * @deprecated use {@link #applicableDescriptors(Slave, Slave.SlaveDescriptor)} + */ + @Deprecated + @Restricted(DoNotUse.class) + @RestrictedSince("2.12") public List> getApplicableDescriptors() { List> r = new ArrayList>(); - for (Descriptor d : Functions.getComputerLauncherDescriptors()) { + for (Descriptor d : + Jenkins.getInstance().>getDescriptorList(ComputerLauncher.class)) { if (DelegatingComputerLauncher.class.isAssignableFrom(d.getKlass().toJavaClass())) continue; r.add(d); } diff --git a/core/src/main/resources/hudson/slaves/DelegatingComputerLauncher/config.jelly b/core/src/main/resources/hudson/slaves/DelegatingComputerLauncher/config.jelly index 843cb9d3c9..798f805eed 100644 --- a/core/src/main/resources/hudson/slaves/DelegatingComputerLauncher/config.jelly +++ b/core/src/main/resources/hudson/slaves/DelegatingComputerLauncher/config.jelly @@ -25,5 +25,5 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly b/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly index 667e123f29..c06a9e1d35 100644 --- a/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly +++ b/core/src/main/resources/hudson/slaves/DumbSlave/configure-entries.jelly @@ -47,9 +47,10 @@ THE SOFTWARE. + - + @@ -63,10 +64,11 @@ THE SOFTWARE. - + + - + - + diff --git a/core/src/test/java/hudson/slaves/DelegatingComputerLauncherTest.java b/core/src/test/java/hudson/slaves/DelegatingComputerLauncherTest.java index 3b76651562..5d47c1a2af 100644 --- a/core/src/test/java/hudson/slaves/DelegatingComputerLauncherTest.java +++ b/core/src/test/java/hudson/slaves/DelegatingComputerLauncherTest.java @@ -65,7 +65,7 @@ public class DelegatingComputerLauncherTest { assertTrue("DelegatingComputerLauncher should filter out other DelegatingComputerLauncher instances " + "from its descriptor's getApplicableDescriptors() method", - new DummyTwo.DummyTwoDescriptor().getApplicableDescriptors().isEmpty()); + new DummyTwo.DummyTwoDescriptor().applicableDescriptors(null, new DumbSlave.DescriptorImpl()).isEmpty()); } } diff --git a/test/src/test/java/hudson/model/Slave2Test.java b/test/src/test/java/hudson/model/Slave2Test.java index 2179adf9ad..89e4ba4661 100644 --- a/test/src/test/java/hudson/model/Slave2Test.java +++ b/test/src/test/java/hudson/model/Slave2Test.java @@ -23,15 +23,35 @@ */ package hudson.model; +import hudson.DescriptorExtensionList; +import hudson.ExtensionList; +import hudson.slaves.ComputerLauncher; +import hudson.slaves.DumbSlave; +import hudson.slaves.NodeProperty; +import hudson.slaves.NodePropertyDescriptor; +import hudson.slaves.RetentionStrategy; import java.net.MalformedURLException; import java.net.URLEncoder; +import java.util.HashSet; +import java.util.Set; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.*; +import static org.junit.Assume.assumeThat; + import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.TestExtension; /** * Tests for the {@link Slave} class. @@ -96,4 +116,78 @@ public class Slave2Test { assertEquals(200, client.getPage(client.getContextPath() + "jnlpJars/" + URLEncoder.encode(url, "UTF-8")).getWebResponse().getStatusCode()); assertEquals(200, client.getPage(jnlpJar.getURL()).getWebResponse().getStatusCode()); } + + @Test + @Issue("JENKINS-36280") + public void launcherFiltering() throws Exception { + DumbSlave.DescriptorImpl descriptor = + rule.getInstance().getDescriptorByType(DumbSlave.DescriptorImpl.class); + DescriptorExtensionList> descriptors = + rule.getInstance().getDescriptorList(ComputerLauncher.class); + assumeThat("we need at least two launchers to test this", descriptors.size(), not(anyOf(is(0), is(1)))); + assertThat(descriptor.computerLauncherDescriptors(null), containsInAnyOrder(descriptors.toArray(new Descriptor[descriptors.size()]))); + + Descriptor victim = descriptors.iterator().next(); + assertThat(descriptor.computerLauncherDescriptors(null), hasItem(victim)); + DynamicFilter.descriptors().add(victim); + assertThat(descriptor.computerLauncherDescriptors(null), not(hasItem(victim))); + DynamicFilter.descriptors().remove(victim); + assertThat(descriptor.computerLauncherDescriptors(null), hasItem(victim)); + } + + @Test + @Issue("JENKINS-36280") + public void retentionFiltering() throws Exception { + DumbSlave.DescriptorImpl descriptor = + rule.getInstance().getDescriptorByType(DumbSlave.DescriptorImpl.class); + DescriptorExtensionList, Descriptor>> descriptors = RetentionStrategy.all(); + assumeThat("we need at least two retention strategies to test this", descriptors.size(), not(anyOf(is(0), is(1)))); + assertThat(descriptor.retentionStrategyDescriptors(null), containsInAnyOrder(descriptors.toArray(new Descriptor[descriptors.size()]))); + + Descriptor> victim = descriptors.iterator().next(); + assertThat(descriptor.retentionStrategyDescriptors(null), hasItem(victim)); + DynamicFilter.descriptors().add(victim); + assertThat(descriptor.retentionStrategyDescriptors(null), not(hasItem(victim))); + DynamicFilter.descriptors().remove(victim); + assertThat(descriptor.retentionStrategyDescriptors(null), hasItem(victim)); + } + + @Test + @Issue("JENKINS-36280") + public void propertyFiltering() throws Exception { + DumbSlave.DescriptorImpl descriptor = + rule.getInstance().getDescriptorByType(DumbSlave.DescriptorImpl.class); + DescriptorExtensionList, NodePropertyDescriptor> descriptors = NodeProperty.all(); + assumeThat("we need at least two node properties to test this", descriptors.size(), not(anyOf(is(0), is(1)))); + assertThat(descriptor.nodePropertyDescriptors(null), containsInAnyOrder(descriptors.toArray(new Descriptor[descriptors.size()]))); + + NodePropertyDescriptor victim = descriptors.iterator().next(); + assertThat(descriptor.nodePropertyDescriptors(null), hasItem(victim)); + DynamicFilter.descriptors().add(victim); + assertThat(descriptor.nodePropertyDescriptors(null), not(hasItem(victim))); + DynamicFilter.descriptors().remove(victim); + assertThat(descriptor.nodePropertyDescriptors(null), hasItem(victim)); + } + + + + @TestExtension + public static class DynamicFilter extends DescriptorVisibilityFilter { + + private final Set descriptors = new HashSet<>(); + + public static Set descriptors() { + return ExtensionList.lookup(DescriptorVisibilityFilter.class).get(DynamicFilter.class).descriptors; + } + + @Override + public boolean filterType(@Nonnull Class contextClass, @Nonnull Descriptor descriptor) { + return !descriptors.contains(descriptor); + } + + @Override + public boolean filter(@CheckForNull Object context, @Nonnull Descriptor descriptor) { + return !descriptors.contains(descriptor); + } + } }