Merge pull request #88 from matthewlowry/bugfix/number-value-extra-var

Handle Number-typed extra var values.

Author: jonesbusy

README.md

@@ -487,8 +487,10 @@ node {
 
 ### Extra Variables
 
-Extra variables can be passed to ansible by using a map in the pipeline script. Use the `hidden` parameter
-to keep the variable secret in the build log.
+Extra variables can be passed to ansible by using a map in the pipeline script.
+Supported value types are: `String`, `Boolean`, `Number`.
+By default the value will be considered potentially sensitive and masked in the logs.
+To override this give a map with keys `value` and `hidden`.
 
 ```groovy
 node {
@@ -497,7 +499,9 @@ node {
         playbook: 'cloud_playbooks/create-aws.yml',
         extraVars: [
             login: 'mylogin',
-            secret_key: [value: 'g4dfKWENpeF6pY05', hidden: true]
+            toggle: true,
+            forks: 8,
+            not_secret: [value: 'I want to see this in the logs', hidden: false]
         ])
 }
 ```

src/main/java/org/jenkinsci/plugins/ansible/AbstractAnsibleInvocation.java

@@ -120,6 +120,10 @@ public T setExtraVars(List<ExtraVar> extraVars) {
     public ArgumentListBuilder appendExtraVars(ArgumentListBuilder args) {
         if (extraVars != null && ! extraVars.isEmpty()) {
             for (ExtraVar var : extraVars) {
+                if (var.getSecretValue() == null) {
+                    listener.getLogger().println("[WARN] Omitting extra var " + var.getKey() + ": check value is a supported type.");
+                    continue;
+                }
                 args.add("-e");
                 String value = envVars.expand(var.getSecretValue().getPlainText());
                 if (Pattern.compile("\\s").matcher(value).find()) {

src/main/java/org/jenkinsci/plugins/ansible/workflow/AnsiblePlaybookStep.java

@@ -374,7 +374,7 @@ public static final class AnsiblePlaybookExecution extends AbstractSynchronousNo
         @StepContextParameter
         private transient Computer computer;
 
-        private List<ExtraVar> convertExtraVars(Map<String, Object> extraVars) {
+        private static List<ExtraVar> convertExtraVars(Map<String, Object> extraVars) {
             if (extraVars == null) {
                 return null;
             }
@@ -384,25 +384,42 @@ private List<ExtraVar> convertExtraVars(Map<String, Object> extraVars) {
                 var.setKey(entry.getKey());
                 Object o = entry.getValue();
                 if (o instanceof Map) {
-                    var.setSecretValue(Secret.fromString((String)((Map)o).get("value")));
-                    var.setHidden((Boolean)((Map)o).get("hidden"));
-                }
-                else if (o instanceof String) {
-                    var.setSecretValue(Secret.fromString((String)o));
-                    var.setHidden(true);
-                }
-                else if (o instanceof Boolean) {
-                    var.setSecretValue(Secret.fromString(o.toString()));
-                    var.setHidden(true);
-                }
-                else if (o instanceof Secret) {
-                    var.setSecretValue((Secret)o);
+                    var.setSecretValue(getSecretFromScalarValue(((Map<?,?>)o).get("value")));
+                    Object hidden = ((Map<?,?>)o).get("hidden");
+                    // If we are given a Boolean value for hidden, respect that.
+                    // Otherwise if omitted or explictly null or any other type adopt the safe default of hidden=true.
+                    if (hidden instanceof Boolean) {
+                        var.setHidden((Boolean)hidden);
+                    } else {
+                        var.setHidden(true);
+                    }
+                } else {
+                    var.setSecretValue(getSecretFromScalarValue(o));
+                    // Consistent with above: for a scalar value effectively hidden is omitted so adopt the safe default of hidden=true.
                     var.setHidden(true);
                 }
                 extraVarList.add(var);
             }
             return extraVarList;
         }
+        
+        private static Secret getSecretFromScalarValue(Object o) {
+            if (o instanceof String) {
+                return Secret.fromString((String)o);
+            }
+            else if (o instanceof Boolean) {
+                return Secret.fromString(o.toString());
+            }
+            else if (o instanceof Number) {
+                return Secret.fromString(o.toString());
+            }
+            else if (o instanceof Secret) {
+                return (Secret)o;
+            } 
+            else {
+                return null;
+            }
+        }
 
         @Override
         protected Void run() throws Exception {

src/test/java/org/jenkinsci/plugins/ansible/PipelineTest.java

@@ -87,6 +87,18 @@ public void testExtraVarsBoolean() throws Exception {
         ));
     }
 
+    @Test
+    public void testExtraVarsNumeric() throws Exception {
+        String pipeline = IOUtils.toString(PipelineTest.class.getResourceAsStream("/pipelines/extraVarsNumeric.groovy"), StandardCharsets.UTF_8);
+        WorkflowJob workflowJob = jenkins.createProject(WorkflowJob.class);
+        workflowJob.setDefinition(new CpsFlowDefinition(pipeline, true));
+        WorkflowRun run1 = workflowJob.scheduleBuild2(0).waitForStart();
+        jenkins.waitForCompletion(run1);
+        assertThat(run1.getLog(), allOf(
+                containsString("ansible-playbook playbook.yml -e ********")
+        ));
+    }
+
     @Test
     public void testAnsiblePlaybookSshPass() throws Exception {
 

src/test/resources/pipelines/extraVarsNumeric.groovy

@@ -0,0 +1,27 @@
+pipeline {
+    agent {
+        label('test-agent')
+    }
+    stages {
+        stage('Create playbook') {
+            steps {
+                writeFile(encoding: 'UTF-8', file: 'playbook.yml', text: '''- hosts: localhost
+  connection: local
+  gather_facts: no
+  tasks:
+    - debug: msg=test
+                ''')
+            }
+        }
+        stage('Ansible playbook') {
+            steps {
+                warnError(message: 'ansible command not found?') {
+                    ansiblePlaybook(
+                            playbook: 'playbook.yml',
+                            extraVars: [foo1: 8],
+                    )
+                }
+            }
+        }
+    }
+}