Allow empty valued variables without ignoring enviroment vars substitution (#2665)

* Using configurator for global props

* Accept EnvVars wihtout value

* Testing additional global props

* Removing not used describe

* Switching to use DataBoundConstructor

* Adding variable resolution

* Added export with vars test

* spotless

* Exporting empty env vars

* fixed test

---------

Co-authored-by: Francisco Javier Fernandez Gonzalez <[email protected]>

Author: PereBueno

integrations/src/test/java/io/jenkins/plugins/casc/GlobalNodePropertiesTest.java

@@ -7,15 +7,19 @@
 import static org.hamcrest.core.Is.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import hudson.node_monitors.DiskSpaceMonitorNodeProperty;
 import hudson.slaves.EnvironmentVariablesNodeProperty;
 import hudson.slaves.NodeProperty;
 import hudson.slaves.NodePropertyDescriptor;
+import hudson.tools.ToolLocationNodeProperty;
 import hudson.util.DescribableList;
 import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
 import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
 import io.jenkins.plugins.casc.misc.junit.jupiter.WithJenkinsConfiguredWithCode;
 import io.jenkins.plugins.casc.model.CNode;
+import java.util.Iterator;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import jenkins.model.Jenkins;
 import org.junit.jupiter.api.Test;
@@ -30,14 +34,33 @@ void configure(JenkinsConfiguredWithCodeRule j) {
 
         DescribableList<NodeProperty<?>, NodePropertyDescriptor> nodeProperties = jenkins.getGlobalNodeProperties();
 
-        Set<Map.Entry<String, String>> entries = ((EnvironmentVariablesNodeProperty) nodeProperties.get(0))
+        assertEquals(3, nodeProperties.size());
+
+        Set<Map.Entry<String, String>> envVars = ((EnvironmentVariablesNodeProperty)
+                        nodeProperties.get(EnvironmentVariablesNodeProperty.class))
                 .getEnvVars()
                 .entrySet();
-        assertEquals(1, entries.size());
+        assertEquals(2, envVars.size());
 
-        Map.Entry<String, String> envVar = entries.iterator().next();
+        Iterator<Entry<String, String>> iterator = envVars.iterator();
+        Map.Entry<String, String> envVar = iterator.next();
         assertEquals("FOO", envVar.getKey());
         assertEquals("BAR", envVar.getValue());
+
+        envVar = iterator.next();
+        assertEquals("FOO2", envVar.getKey());
+        assertEquals("", envVar.getValue());
+
+        DiskSpaceMonitorNodeProperty diskSpace = nodeProperties.get(DiskSpaceMonitorNodeProperty.class);
+        assertEquals("1GiB", diskSpace.getFreeDiskSpaceThreshold());
+        assertEquals("2GiB", diskSpace.getFreeDiskSpaceWarningThreshold());
+        assertEquals("1GiB", diskSpace.getFreeTempSpaceThreshold());
+        assertEquals("2GiB", diskSpace.getFreeTempSpaceWarningThreshold());
+
+        ToolLocationNodeProperty toolLocations = nodeProperties.get(ToolLocationNodeProperty.class);
+        assertEquals(1, toolLocations.getLocations().size());
+        assertEquals("Default", toolLocations.getLocations().get(0).getName());
+        assertEquals("/home/user/bin/git", toolLocations.getLocations().get(0).getHome());
     }
 
     @Test

integrations/src/test/java/io/jenkins/plugins/casc/GlobalNodePropertiesWithEnvVarsTest.java

@@ -0,0 +1,64 @@
+package io.jenkins.plugins.casc;
+
+import static io.jenkins.plugins.casc.misc.Util.getJenkinsRoot;
+import static io.jenkins.plugins.casc.misc.Util.toStringFromYamlFile;
+import static io.jenkins.plugins.casc.misc.Util.toYamlString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+
+import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
+import hudson.tools.ToolLocationNodeProperty;
+import hudson.util.DescribableList;
+import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
+import io.jenkins.plugins.casc.misc.Env;
+import io.jenkins.plugins.casc.misc.EnvVarsRule;
+import io.jenkins.plugins.casc.misc.Envs;
+import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
+import io.jenkins.plugins.casc.model.CNode;
+import java.util.Map;
+import org.hamcrest.core.Is;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+
+public class GlobalNodePropertiesWithEnvVarsTest {
+
+    private JenkinsConfiguredWithCodeRule j = new JenkinsConfiguredWithCodeRule();
+
+    @Rule
+    public RuleChain chain = RuleChain.outerRule(new EnvVarsRule()).around(j);
+
+    @Test
+    @ConfiguredWithCode("GlobalNodePropertiesWithEnvVarsTest.yml")
+    @Envs({@Env(name = "VALUE_1", value = "BAR"), @Env(name = "TEST_GIT_HOME", value = "git-home")})
+    public void configureWithEnvVarsTest() {
+        DescribableList<NodeProperty<?>, NodePropertyDescriptor> nodeProperties = j.jenkins.getGlobalNodeProperties();
+        Map<String, String> envVars = ((EnvironmentVariablesNodeProperty)
+                        nodeProperties.get(EnvironmentVariablesNodeProperty.class))
+                .getEnvVars();
+
+        assertThat(envVars.size(), is(2));
+        assertThat(envVars.get("FOO"), is("BAR"));
+        assertThat(envVars.get("FOO2"), is(""));
+
+        ToolLocationNodeProperty toolLocations = nodeProperties.get(ToolLocationNodeProperty.class);
+        assertThat(toolLocations.getLocations(), hasSize(1));
+        assertThat(toolLocations.getLocations().get(0).getHome(), is("git-home"));
+    }
+
+    @Test
+    @ConfiguredWithCode("GlobalNodePropertiesWithEnvVarsTest.yml")
+    @Envs({@Env(name = "VALUE_1", value = "BAR"), @Env(name = "TEST_GIT_HOME", value = "git-home")})
+    public void export() throws Exception {
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        ConfigurationContext context = new ConfigurationContext(registry);
+        CNode yourAttribute = getJenkinsRoot(context).get("globalNodeProperties");
+
+        String exported = toYamlString(yourAttribute);
+        String expected = toStringFromYamlFile(this, "GlobalNodePropertiesWithEnvVarsTestExpected.yml");
+        assertThat(exported, Is.is(expected));
+    }
+}

integrations/src/test/resources/io/jenkins/plugins/casc/GlobalNodePropertiesTest.yml

@@ -1,6 +1,17 @@
 jenkins:
   globalNodeProperties:
+    - diskSpaceMonitor:
+        freeDiskSpaceThreshold: "1GiB"
+        freeDiskSpaceWarningThreshold: "2GiB"
+        freeTempSpaceThreshold: "1GiB"
+        freeTempSpaceWarningThreshold: "2GiB"
     - envVars:
         env:
           - key: FOO
             value: BAR
+          - key: FOO2
+            value: ""
+    - toolLocation:
+        locations:
+          - home: "/home/user/bin/git"
+            key: "hudson.plugins.git.GitTool$DescriptorImpl@Default"

integrations/src/test/resources/io/jenkins/plugins/casc/GlobalNodePropertiesTestExpected.yml

@@ -1,4 +1,15 @@
+- diskSpaceMonitor:
+    freeDiskSpaceThreshold: "1GiB"
+    freeDiskSpaceWarningThreshold: "2GiB"
+    freeTempSpaceThreshold: "1GiB"
+    freeTempSpaceWarningThreshold: "2GiB"
 - envVars:
     env:
     - key: "FOO"
       value: "BAR"
+    - key: "FOO2"
+      value: ""
+- toolLocation:
+    locations:
+    - home: "/home/user/bin/git"
+      key: "hudson.plugins.git.GitTool$DescriptorImpl@Default"

integrations/src/test/resources/io/jenkins/plugins/casc/GlobalNodePropertiesWithEnvVarsTest.yml

@@ -0,0 +1,12 @@
+jenkins:
+  globalNodeProperties:
+    - envVars:
+        env:
+          - key: FOO
+            value: ${VALUE_1}
+          - key: FOO2
+            value: ${VALUE_2}
+    - toolLocation:
+        locations:
+          - home: "${TEST_GIT_HOME}"
+            key: "hudson.plugins.git.GitTool$DescriptorImpl@Default"

integrations/src/test/resources/io/jenkins/plugins/casc/GlobalNodePropertiesWithEnvVarsTestExpected.yml

@@ -0,0 +1,10 @@
+- envVars:
+    env:
+    - key: "FOO"
+      value: "BAR"
+    - key: "FOO2"
+      value: ""
+- toolLocation:
+    locations:
+    - home: "git-home"
+      key: "hudson.plugins.git.GitTool$DescriptorImpl@Default"

plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java

@@ -657,7 +657,7 @@ public Node toYaml(CNode config) throws ConfiguratorException {
             default:
                 final Scalar scalar = config.asScalar();
                 final String value = scalar.getValue();
-                if (value == null || value.length() == 0) {
+                if (StringUtils.isBlank(value) && !scalar.isPrintableWhenEmpty()) {
                     return null;
                 }
 

plugin/src/main/java/io/jenkins/plugins/casc/core/GlobalNodePropertiesConfigurator.java

@@ -0,0 +1,57 @@
+package io.jenkins.plugins.casc.core;
+
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.Extension;
+import hudson.slaves.EnvironmentVariablesNodeProperty;
+import io.jenkins.plugins.casc.Attribute;
+import io.jenkins.plugins.casc.ConfigurationContext;
+import io.jenkins.plugins.casc.ConfiguratorException;
+import io.jenkins.plugins.casc.impl.configurators.DataBoundConfigurator;
+import io.jenkins.plugins.casc.model.CNode;
+import io.jenkins.plugins.casc.model.Mapping;
+
+@Extension
+public class GlobalNodePropertiesConfigurator extends DataBoundConfigurator<EnvironmentVariablesNodeProperty> {
+
+    public GlobalNodePropertiesConfigurator() {
+        this(EnvironmentVariablesNodeProperty.class);
+    }
+
+    public GlobalNodePropertiesConfigurator(Class<?> clazz) {
+        super(EnvironmentVariablesNodeProperty.class);
+    }
+
+    @NonNull
+    @Override
+    public String getName() {
+        return "globalNodeProperties";
+    }
+
+    @NonNull
+    @Override
+    public EnvironmentVariablesNodeProperty configure(CNode c, ConfigurationContext context)
+            throws ConfiguratorException {
+        return super.configure(c, context);
+    }
+
+    @Override
+    @CheckForNull
+    public CNode describe(EnvironmentVariablesNodeProperty instance, ConfigurationContext context) throws Exception {
+        Mapping mapping = new Mapping();
+        for (Attribute attribute : getAttributes()) {
+            CNode value = attribute.describe(instance, context);
+            if (value != null) {
+                // Making sure empty variables are part of the export
+                value.asSequence().forEach(entry -> {
+                    if (entry.asMapping().get("key") != null
+                            && entry.asMapping().get("value") != null) {
+                        entry.asMapping().get("value").asScalar().setPrintableWhenEmpty(true);
+                    }
+                });
+                mapping.put(attribute.getName(), value);
+            }
+        }
+        return mapping;
+    }
+}

plugin/src/main/java/io/jenkins/plugins/casc/model/CNode.java

@@ -35,6 +35,14 @@ default boolean isSensitiveData() {
         return false;
     }
 
+    /**
+     * Indicates if the field should be included when describing even if empty
+     * @return false by default
+     */
+    default boolean isPrintableWhenEmpty() {
+        return false;
+    }
+
     /**
      * Indicate the source (file, line number) this specific configuration node comes from.
      * This is used to offer relevant diagnostic messages

plugin/src/main/java/io/jenkins/plugins/casc/model/Scalar.java

@@ -16,6 +16,7 @@ public final class Scalar implements CNode, CharSequence {
     private Source source;
     private boolean sensitive;
     private boolean encrypted;
+    private boolean printableWhenEmpty = false;
 
     public enum Format {
         STRING,
@@ -163,6 +164,15 @@ public Source getSource() {
         return source;
     }
 
+    @Override
+    public boolean isPrintableWhenEmpty() {
+        return printableWhenEmpty;
+    }
+
+    public void setPrintableWhenEmpty(boolean print) {
+        this.printableWhenEmpty = print;
+    }
+
     @Override
     public CNode clone() {
         return new Scalar(this);
@@ -175,5 +185,6 @@ private Scalar(Scalar it) {
         this.source = it.source;
         this.sensitive = it.sensitive;
         this.encrypted = it.encrypted;
+        this.printableWhenEmpty = it.printableWhenEmpty;
     }
 }