[FIXED JENKINS-46065] Add extension point for contributing validation

jenkinsci · Aug 8, 2017 · b496df9 · b496df9
1 parent 103bec1
commit b496df9
Show file tree

Hide file tree

Showing 4 changed files with 326 additions and 24 deletions.
diff --git a/...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy b/...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy
@@ -75,12 +75,12 @@ class ModelValidatorImpl implements ModelValidator {
 
     public boolean validateElement(@Nonnull ModelASTPostBuild postBuild) {
         // post specific validation
-        true
+        return validateFromContributors(postBuild, true)
     }
 
     public boolean validateElement(@Nonnull ModelASTPostStage post) {
         // post stage specific validation
-        true
+        return validateFromContributors(post, true)
     }
 
     public boolean validateElement(@Nonnull ModelASTBuildConditionsContainer post) {
@@ -100,7 +100,7 @@ class ModelValidatorImpl implements ModelValidator {
             valid = false
         }
 
-        return valid
+        return validateFromContributors(post, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTBuildCondition buildCondition) {
@@ -116,7 +116,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(buildCondition, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTEnvironment env) {
@@ -133,12 +133,12 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(env, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTInternalFunctionCall call) {
         // TODO: Make this real validation when JENKINS-41759 lands
-        return true
+        return validateFromContributors(call, true)
     }
 
     public boolean validateElement(@Nonnull ModelASTTools t) {
@@ -170,7 +170,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(t, valid)
     }
 
     public boolean validateElement(ModelASTWhen when) {
@@ -180,7 +180,7 @@ class ModelValidatorImpl implements ModelValidator {
             valid = false
         }
 
-        return valid
+        return validateFromContributors(when, valid)
     }
 
     public boolean validateElement(ModelASTLibraries libraries) {
@@ -195,7 +195,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(libraries, valid)
     }
 
     public boolean validateElement(ModelASTWhenCondition condition) {
@@ -240,7 +240,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(condition, valid)
     }
 
     private boolean isValidStepParameter(DescribableModel<? extends Describable> model,
@@ -384,7 +384,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(step, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTMethodCall meth) {
@@ -453,7 +453,7 @@ class ModelValidatorImpl implements ModelValidator {
                 }
             }
         }
-        return valid
+        return validateFromContributors(meth, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTOptions opts) {
@@ -469,7 +469,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(opts, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTTrigger trig) {
@@ -487,7 +487,7 @@ class ModelValidatorImpl implements ModelValidator {
             errorCollector.error(trig, Messages.ModelValidatorImpl_MixedNamedAndUnnamedParameters())
             valid = false
         }
-        return valid
+        return validateFromContributors(trig, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTTriggers triggers) {
@@ -503,7 +503,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(triggers, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTBuildParameter param) {
@@ -522,16 +522,17 @@ class ModelValidatorImpl implements ModelValidator {
             errorCollector.error(param, Messages.ModelValidatorImpl_MixedNamedAndUnnamedParameters())
             valid = false
         }
-        return valid
+        return validateFromContributors(param, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTBuildParameters params) {
+        boolean valid = true
         if (params.parameters.isEmpty()) {
             errorCollector.error(params, Messages.ModelValidatorImpl_EmptySection("parameters"))
-            return false
+            valid = false
         }
 
-        return true
+        return validateFromContributors(params, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTOption opt) {
@@ -550,7 +551,7 @@ class ModelValidatorImpl implements ModelValidator {
             errorCollector.error(opt, Messages.ModelValidatorImpl_MixedNamedAndUnnamedParameters())
             valid = false
         }
-        return valid
+        return validateFromContributors(opt, valid)
     }
 
     private boolean validateParameterType(ModelASTValue v, Class erasedType, ModelASTKey k = null) {
@@ -584,7 +585,7 @@ class ModelValidatorImpl implements ModelValidator {
             valid = false
         }
 
-        return valid
+        return validateFromContributors(branch, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTPipelineDef pipelineDef) {
@@ -598,7 +599,7 @@ class ModelValidatorImpl implements ModelValidator {
         if (pipelineDef.agent == null) {
             errorCollector.error(pipelineDef, Messages.ModelValidatorImpl_RequiredSection("agent"))
         }
-        return valid
+        return validateFromContributors(pipelineDef, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTStage stage, boolean isNested) {
@@ -638,7 +639,7 @@ class ModelValidatorImpl implements ModelValidator {
             }
         }
 
-        return valid
+        return validateFromContributors(stage, valid, isNested)
     }
 
     public boolean validateElement(@Nonnull ModelASTStages stages) {
@@ -658,7 +659,7 @@ class ModelValidatorImpl implements ModelValidator {
             valid = false
         }
 
-        return valid
+        return validateFromContributors(stages, valid)
     }
 
     public boolean validateElement(@Nonnull ModelASTAgent agent) {
@@ -705,6 +706,26 @@ class ModelValidatorImpl implements ModelValidator {
                 }
             }
         }
-        return valid
+        return validateFromContributors(agent, valid)
+    }
+
+    private boolean validateFromContributors(ModelASTElement element, boolean isValid, boolean isNested = false) {
+        boolean contributorsValid = DeclarativeValidatorContributor.all().every { contributor ->
+            String error = null
+            if (!(element instanceof ModelASTStage)) {
+                error = contributor.validateElement(element)
+            } else {
+                error = contributor.validateElement((ModelASTStage)element, isNested)
+            }
+            if (error != null) {
+                errorCollector.error(element, error)
+                return false
+            } else {
+                return true
+            }
+        }
+        if (isValid) {
+            return contributorsValid
+        }
     }
 }
diff --git a/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java b/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
@@ -24,17 +24,26 @@
 package org.jenkinsci.plugins.pipeline.modeldefinition;
 
 import hudson.slaves.DumbSlave;
+import jenkins.model.OptionalJobProperty;
 import org.apache.commons.lang.StringUtils;
+import org.jenkinsci.Symbol;
+import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTOption;
 import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTStep;
 import org.jenkinsci.plugins.pipeline.modeldefinition.model.BuildCondition;
 import org.jenkinsci.plugins.pipeline.modeldefinition.model.Options;
 import org.jenkinsci.plugins.pipeline.modeldefinition.model.Parameters;
 import org.jenkinsci.plugins.pipeline.modeldefinition.model.Tools;
 import org.jenkinsci.plugins.pipeline.modeldefinition.model.Triggers;
+import org.jenkinsci.plugins.pipeline.modeldefinition.validator.DeclarativeValidatorContributor;
 import org.jenkinsci.plugins.pipeline.modeldefinition.when.DeclarativeStageConditionalDescriptor;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.DataBoundConstructor;
+
+import javax.annotation.Nonnull;
 
 /**
  * @author Andrew Bayer
@@ -615,4 +624,40 @@ public void parallelStagesDeepNesting() throws Exception {
                 .logContains(Messages.ModelValidatorImpl_NoNestedWithinNestedStages())
                 .go();
     }
+
+    @Issue("JENKINS-46065")
+    @Test
+    public void validatorContributor() throws Exception {
+        expectError("validatorContributor")
+                .logContains("testProperty is rejected")
+                .go();
+    }
+
+    @TestExtension
+    public static class RejectTestProperty extends DeclarativeValidatorContributor {
+        @Override
+        public String validateElement(@Nonnull ModelASTOption option) {
+            if (option.getName() != null && option.getName().equals("testProperty")) {
+                return "testProperty is rejected";
+            } else {
+                return null;
+            }
+        }
+    }
+
+    public static class TestProperty extends OptionalJobProperty<WorkflowJob> {
+        @DataBoundConstructor
+        public TestProperty() {
+
+        }
+
+        @TestExtension
+        @Symbol("testProperty")
+        public static class DescriptorImpl extends OptionalJobPropertyDescriptor {
+            @Override
+            public String getDisplayName() {
+                return "Test job property to be rejected by a validator contributor.";
+            }
+        }
+    }
 }
diff --git a/pipeline-model-definition/src/test/resources/errors/validatorContributor.groovy b/pipeline-model-definition/src/test/resources/errors/validatorContributor.groovy
@@ -0,0 +1,40 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+pipeline {
+    agent none
+    options {
+        testProperty()
+    }
+    stages {
+        stage("foo") {
+            steps {
+                echo "hello"
+            }
+        }
+    }
+}
+
+
+