Merge pull request #420 from rundeck-plugins/RUN-3327-issues-with-ansible-inline-workflow-executions-where-it-shows-an-unwanted-output

RUN-3327:  Issues with ansible inline workflow executions where it shows an unwanted output

Author: edbaltra

src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleInventory.java

@@ -59,18 +59,27 @@ public AnsibleInventory addHost(String nodeName, String host, Map<String, String
     for (String r: reserved){
       attributes.remove(r);
     }
-    all.addHost(nodeName, host, attributes);
+
     // Create Ansible groups by attribute
     // Group by osFamily is needed for windows hosts setup
     String[] attributeGroups = { "osFamily", "tags" };
     for (String g: attributeGroups) {
       if (attributes.containsKey(g)) {
         String[] groupNames = attributes.get(g).toLowerCase().split(",");
         for (String groupName: groupNames) {
-          all.getOrAddChildHostGroup(groupName.trim()).addHost(nodeName);
+          // Sanitize group name: Ansible only allows letters, numbers, underscores, and hyphens
+          // Note: group names are already lowercased above, so we only need to match lowercase
+          // Replace invalid characters (like colons, spaces, special chars) with underscores
+          String sanitizedGroupName = groupName.trim().replaceAll("[^a-z0-9_\\-]", "_");
+          all.getOrAddChildHostGroup(sanitizedGroupName).addHost(nodeName);
         }
       }
     }
+
+    //remove "tags" since it's reserved in Ansible and we've already used it for grouping
+    attributes.remove("tags");
+
+    all.addHost(nodeName, host, attributes);
     return this;
   }
 }

src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleResourceModelSource.java

@@ -786,6 +786,30 @@ public void processHosts(Map<String, Object> hostNames, HashSet<String> tags) th
     }
 
     for (Map.Entry<String, Object> hostNode : hosts.entrySet()) {
+      // Filter out invalid host keys that aren't Strings (can occur with YAML anchors/aliases)
+      Object hostKeyObj = hostNode.getKey();
+      if (hostKeyObj == null) {
+        log.warn("Skipping host entry with null key");
+        continue;
+      }
+      if (!(hostKeyObj instanceof String)) {
+        log.warn("Skipping invalid host entry with non-String key: {}", hostKeyObj.getClass().getName());
+        continue;
+      }
+
+      // Additional validation: skip keys that are JSON objects (likely serialized data structures)
+      String hostKey = (String) hostKeyObj;
+      try {
+        JsonElement jsonElement = JsonParser.parseString(hostKey);
+        if (jsonElement.isJsonObject()) {
+          log.warn("Skipping host entry with key that is a JSON object (likely serialized data): {}",
+                   hostKey.length() > 100 ? hostKey.substring(0, 100) + "..." : hostKey);
+          continue;
+        }
+      } catch (Exception e) {
+        // Not valid JSON - treat as a legitimate host key and continue processing
+      }
+
       NodeEntryImpl node = createNodeEntry(hostNode);
       addNode(node, tags);
     }

src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleInventorySpec.groovy

@@ -0,0 +1,121 @@
+package com.rundeck.plugins.ansible.ansible
+
+import com.google.gson.Gson
+import spock.lang.Specification
+
+/**
+ * AnsibleInventory test
+ */
+class AnsibleInventorySpec extends Specification {
+
+    void "addHost should sanitize group names with invalid characters"() {
+        given: "an AnsibleInventory instance"
+        AnsibleInventory inventory = new AnsibleInventory()
+        Gson gson = new Gson()
+
+        when: "adding a host with tags containing colons"
+        Map<String, String> attributes = new HashMap<>()
+        attributes.put("tags", "runner:tag:ansible,testnodes")
+        inventory.addHost("test-node", "127.0.0.1", attributes)
+
+        then: "group names should have colons replaced with underscores"
+        String json = gson.toJson(inventory)
+        json.contains("runner_tag_ansible")
+        json.contains("testnodes")
+        !json.contains("runner:tag:ansible")
+    }
+
+    void "addHost should sanitize group names with special characters"() {
+        given: "an AnsibleInventory instance"
+        AnsibleInventory inventory = new AnsibleInventory()
+        Gson gson = new Gson()
+
+        when: "adding a host with tags containing various special characters"
+        Map<String, String> attributes = new HashMap<>()
+        attributes.put("tags", "tag@with#special!chars,normal-tag_ok")
+        inventory.addHost("test-node", "127.0.0.1", attributes)
+
+        then: "special characters should be replaced with underscores"
+        String json = gson.toJson(inventory)
+        json.contains("tag_with_special_chars")
+        json.contains("normal-tag_ok")
+    }
+
+    void "addHost should remove reserved tags attribute from host variables"() {
+        given: "an AnsibleInventory instance"
+        AnsibleInventory inventory = new AnsibleInventory()
+        Gson gson = new Gson()
+
+        when: "adding a host with tags attribute"
+        Map<String, String> attributes = new HashMap<>()
+        attributes.put("tags", "tag1,tag2")
+        attributes.put("custom_attr", "value")
+        inventory.addHost("test-node", "127.0.0.1", attributes)
+
+        then: "tags should not appear in host variables but custom attributes should"
+        String json = gson.toJson(inventory)
+        json.contains("custom_attr")
+        json.contains("value")
+        // Verify tags is not in the host's attributes (not in quoted key context)
+        !json.contains('"tags"')
+    }
+
+    void "addHost should remove reserved Ansible variables"() {
+        given: "an AnsibleInventory instance"
+        AnsibleInventory inventory = new AnsibleInventory()
+        Gson gson = new Gson()
+
+        when: "adding a host with reserved Ansible variables"
+        Map<String, String> attributes = new HashMap<>()
+        attributes.put("hostvars", "should_be_removed")
+        attributes.put("group_names", "should_be_removed")
+        attributes.put("groups", "should_be_removed")
+        attributes.put("environment", "should_be_removed")
+        attributes.put("custom_var", "should_remain")
+        inventory.addHost("test-node", "127.0.0.1", attributes)
+
+        then: "reserved variables should be removed"
+        String json = gson.toJson(inventory)
+        json.contains("custom_var")
+        json.contains("should_remain")
+        !json.contains("hostvars")
+        !json.contains("group_names")
+        !json.contains("should_be_removed")
+    }
+
+    void "addHost should create groups from osFamily attribute and retain it in host variables"() {
+        given: "an AnsibleInventory instance"
+        AnsibleInventory inventory = new AnsibleInventory()
+        Gson gson = new Gson()
+
+        when: "adding a host with osFamily attribute"
+        Map<String, String> attributes = new HashMap<>()
+        attributes.put("osFamily", "unix")
+        attributes.put("custom_attr", "test_value")
+        inventory.addHost("test-node", "127.0.0.1", attributes)
+
+        then: "unix group should be created AND osFamily should remain in host variables"
+        String json = gson.toJson(inventory)
+        json.contains("unix")
+        // osFamily should NOT be removed (unlike tags) - it's needed by other components
+        json.contains('"osFamily":"unix"')
+        json.contains("custom_attr")
+    }
+
+    void "addHost should handle multiple tags and create multiple groups"() {
+        given: "an AnsibleInventory instance"
+        AnsibleInventory inventory = new AnsibleInventory()
+        Gson gson = new Gson()
+
+        when: "adding a host with multiple tags"
+        Map<String, String> attributes = new HashMap<>()
+        attributes.put("tags", "linux,web-server,production")
+        inventory.addHost("test-node", "127.0.0.1", attributes)
+
+        then: "all groups should be created"
+        String json = gson.toJson(inventory)
+        json.contains("linux")
+        json.contains("web-server")
+        json.contains("production")
+    }
+}

src/test/groovy/com/rundeck/plugins/ansible/plugin/AnsibleResourceModelSourceSpec.groovy

@@ -278,4 +278,140 @@ class AnsibleResourceModelSourceSpec extends Specification {
         return yaml.dump(all)
     }
 
+    void "processHosts should process valid String host keys without errors"() {
+        given: "a plugin with standard inventory"
+        Framework framework = Mock(Framework) {
+            getPropertyLookup() >> Mock(IPropertyLookup){
+                getProperty("framework.tmp.dir") >> '/tmp'
+            }
+            getBaseDir() >> Mock(File) {
+                getAbsolutePath() >> '/tmp'
+            }
+        }
+        AnsibleResourceModelSource plugin = new AnsibleResourceModelSource(framework)
+        Properties config = new Properties()
+        config.put('project', 'project_1')
+        config.put(AnsibleDescribable.ANSIBLE_GATHER_FACTS, 'false')
+        plugin.configure(config)
+        Services services = Mock(Services) {
+            getService(KeyStorageTree.class) >> Mock(KeyStorageTree)
+        }
+        plugin.setServices(services)
+
+        when: "YAML contains valid String keys"
+        // Note: Testing non-String keys directly is difficult because Java's type system
+        // prevents creating Map<String, Object> with non-String keys. The defensive code
+        // handles the rare edge case where complex YAML anchor/alias patterns produce non-String keys during parsing.
+        Yaml yaml = new Yaml()
+        def validHost = ['node-1' : ['hostname' : 'host-1']]
+        def hosts = ['hosts' : validHost]
+        def groups = ['ungrouped' : hosts]
+        def children = ['children' : groups]
+        def all = ['all' : children]
+        String result = yaml.dump(all)
+
+        AnsibleInventoryListBuilder inventoryListBuilder = Mock(AnsibleInventoryListBuilder) {
+            build() >> Mock(AnsibleInventoryList) {
+                getNodeList() >> result
+            }
+        }
+        plugin.ansibleInventoryListBuilder = inventoryListBuilder
+        INodeSet nodes = plugin.getNodes()
+
+        then: "valid String keys are processed successfully"
+        nodes.size() == 1
+        nodes.getNodeNames().contains('node-1')
+    }
+
+    void "processHosts should filter out JSON object keys"() {
+        given: "a plugin configured"
+        Framework framework = Mock(Framework) {
+            getPropertyLookup() >> Mock(IPropertyLookup){
+                getProperty("framework.tmp.dir") >> '/tmp'
+            }
+            getBaseDir() >> Mock(File) {
+                getAbsolutePath() >> '/tmp'
+            }
+        }
+        AnsibleResourceModelSource plugin = new AnsibleResourceModelSource(framework)
+        Properties config = new Properties()
+        config.put('project', 'project_1')
+        config.put(AnsibleDescribable.ANSIBLE_GATHER_FACTS, 'false')
+        plugin.configure(config)
+        Services services = Mock(Services) {
+            getService(KeyStorageTree.class) >> Mock(KeyStorageTree)
+        }
+        plugin.setServices(services)
+
+        when: "YAML contains a key that is a valid JSON object (serialized data)"
+        Yaml yaml = new Yaml()
+        def hosts = [
+            'valid-node': ['hostname': 'valid-host'],
+            '{"inventory":"data","nested":{"key":"value"}}': ['hostname': 'json-object-key'],
+            '{simple-curly-name}': ['hostname': 'not-json-host']  // Not valid JSON, should be kept
+        ]
+        def hostsMap = ['hosts': hosts]
+        def groups = ['ungrouped': hostsMap]
+        def children = ['children': groups]
+        def all = ['all': children]
+        String result = yaml.dump(all)
+
+        AnsibleInventoryListBuilder inventoryListBuilder = Mock(AnsibleInventoryListBuilder) {
+            build() >> Mock(AnsibleInventoryList) {
+                getNodeList() >> result
+            }
+        }
+        plugin.ansibleInventoryListBuilder = inventoryListBuilder
+        INodeSet nodes = plugin.getNodes()
+
+        then: "JSON object key is filtered out but valid nodes remain"
+        nodes.size() == 2
+        nodes.getNodeNames().contains('valid-node')
+        nodes.getNodeNames().contains('{simple-curly-name}')  // Not valid JSON, so kept
+        !nodes.getNodeNames().contains('{"inventory":"data","nested":{"key":"value"}}')
+    }
+
+    void "processHosts should handle valid nodes without exception"() {
+        given: "a plugin configured"
+        Framework framework = Mock(Framework) {
+            getPropertyLookup() >> Mock(IPropertyLookup){
+                getProperty("framework.tmp.dir") >> '/tmp'
+            }
+            getBaseDir() >> Mock(File) {
+                getAbsolutePath() >> '/tmp'
+            }
+        }
+        AnsibleResourceModelSource plugin = new AnsibleResourceModelSource(framework)
+        Properties config = new Properties()
+        config.put('project', 'project_1')
+        config.put(AnsibleDescribable.ANSIBLE_GATHER_FACTS, 'false')
+        plugin.configure(config)
+        Services services = Mock(Services) {
+            getService(KeyStorageTree.class) >> Mock(KeyStorageTree)
+        }
+        plugin.setServices(services)
+
+        when: "processing hosts through the normal flow"
+        Yaml yaml = new Yaml()
+        def hosts = ['valid-node': ['hostname': 'valid-host']]
+        def hostsMap = ['hosts': hosts]
+        def groups = ['ungrouped': hostsMap]
+        def children = ['children': groups]
+        def all = ['all': children]
+        String result = yaml.dump(all)
+
+        AnsibleInventoryListBuilder inventoryListBuilder = Mock(AnsibleInventoryListBuilder) {
+            build() >> Mock(AnsibleInventoryList) {
+                getNodeList() >> result
+            }
+        }
+        plugin.ansibleInventoryListBuilder = inventoryListBuilder
+        INodeSet nodes = plugin.getNodes()
+
+        then: "no exception thrown and node is processed correctly"
+        notThrown(Exception)
+        nodes.size() == 1
+        nodes.getNodeNames().contains('valid-node')
+    }
+
 }