From 7bd89d500b6b8b11b7fe7fd54d6178ddaf096641 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Wed, 19 Nov 2025 16:51:38 -0300 Subject: [PATCH 01/46] add new feature to add the node auth in the generated inventory --- .../ansible/ansible/AnsibleDescribable.java | 8 ++ .../ansible/ansible/AnsibleRunner.java | 56 +++++++- .../ansible/AnsibleRunnerContextBuilder.java | 124 ++++++++++++++++-- .../AnsiblePlaybookInlineWorkflowStep.java | 2 + .../plugin/AnsiblePlaybookWorkflowStep.java | 4 +- .../plugins/ansible/util/AnsibleUtil.java | 10 ++ 6 files changed, 189 insertions(+), 15 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java index 0919418f..5b8696b1 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java @@ -100,6 +100,7 @@ public static String[] getValues() { public static final String ANSIBLE_INVENTORY_INLINE = "ansible-inventory-inline"; public static final String ANSIBLE_INVENTORY = "ansible-inventory"; public static final String ANSIBLE_GENERATE_INVENTORY = "ansible-generate-inventory"; + public static final String ANSIBLE_GENERATE_INVENTORY_NODES_AUTH = "ansible-generate-inventory-nodes-auth"; public static final String ANSIBLE_MODULE = "ansible-module"; public static final String ANSIBLE_MODULE_ARGS = "ansible-module-args"; public static final String ANSIBLE_DEBUG = "ansible-debug"; @@ -233,6 +234,13 @@ public static String[] getValues() { .description("Generate Ansible inventory from Rundeck nodes.") .build(); + static final Property GENERATE_INVENTORY_NODES_AUTH = PropertyBuilder.builder() + .booleanType(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH) + .required(false) + .title("Generate inventory, pass node authentication from rundeck nodes") + .description("Pass authentication from rundeck nodes.") + .build(); + public static Property EXECUTABLE_PROP = PropertyUtil.freeSelect( ANSIBLE_EXECUTABLE, "Executable", diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index ae653aea..2072f1a4 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -127,6 +127,15 @@ public static AnsibleRunner buildAnsibleRunner(AnsibleRunnerContextBuilder conte ansibleRunnerBuilder.inventory(inventory); } + Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); + if(generateInventoryNodeAuth){ + Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); + if (nodesAuth != null && !nodesAuth.isEmpty()) { + ansibleRunnerBuilder.addNodeAuthToInventory(true); + ansibleRunnerBuilder.nodesAuthentication(nodesAuth); + } + } + String limit = contextBuilder.getLimit(); if (limit != null) { ansibleRunnerBuilder.limits(limit); @@ -291,9 +300,13 @@ public static AnsibleRunner buildAnsibleRunner(AnsibleRunnerContextBuilder conte File tempSshVarsFile ; File tempBecameVarsFile ; File vaultPromptFile; + File tempNodeAuthFile = null; String customTmpDirPath; + Boolean addNodeAuthToInventory; + Map> nodesAuthentication; + public void deleteTempDirectory(Path tempDirectory) throws IOException { Files.walkFileTree(tempDirectory, new SimpleFileVisitor() { @Override @@ -396,6 +409,40 @@ public int run() throws Exception { if (inventory != null && !inventory.isEmpty()) { procArgs.add("-i"); procArgs.add(inventory); + + if(addNodeAuthToInventory) { + Map hostUsers = new LinkedHashMap<>(); + Map hostPasswords = new LinkedHashMap<>(); + nodesAuthentication.forEach((nodeName, authValues) -> { + String user = authValues.get("ansible_user"); + String password = authValues.get("ansible_password"); + if (user != null) { + hostUsers.put(nodeName, user); + } + if (password != null) { + String encryptedPassword = password; + if (useAnsibleVault) { + try { + encryptedPassword = encryptExtraVarsKey(password); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + hostPasswords.put(nodeName, encryptedPassword); + } + }); + // Build YAML structure + Map yamlData = new LinkedHashMap<>(); + yamlData.put("host_passwords", hostPasswords); + yamlData.put("host_users", hostUsers); + try { + String yamlContent = mapperYaml.writeValueAsString(yamlData); + tempNodeAuthFile = AnsibleUtil.createTemporaryFile("", "all.yaml", yamlContent, customTmpDirPath); + } catch (IOException e) { + throw new RuntimeException("Failed to write all.yaml for node auth", e); + } + } + } if (limits != null && limits.size() == 1) { @@ -513,8 +560,6 @@ public int run() throws Exception { //SET env variables Map processEnvironment = new HashMap<>(); - - if (configFile != null && !configFile.isEmpty()) { if (debug) { System.out.println(" ANSIBLE_CONFIG: " + configFile); @@ -663,6 +708,10 @@ public int run() throws Exception { vaultPromptFile.deleteOnExit(); } + if (tempNodeAuthFile != null && !tempNodeAuthFile.delete()) { + tempNodeAuthFile.deleteOnExit(); + } + if (usingTempDirectory && !retainTempDirectory) { deleteTempDirectory(baseDirectory); } @@ -824,4 +873,5 @@ public String encryptExtraVarsKey(String extraVars) throws Exception { } -} \ No newline at end of file +} + diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index ef74733b..449ec7dd 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -193,17 +193,7 @@ public String getSshPassword() throws ConfigurationException { if (null != storagePath) { //look up storage value - Path path = PathUtil.asPath(storagePath); - try { - ResourceMeta contents = context.getStorageTree().getResource(path) - .getContents(); - ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); - contents.writeContent(byteArrayOutputStream); - return byteArrayOutputStream.toString(); - } catch (StorageException | IOException e) { - throw new ConfigurationException("Failed to read the ssh password for " + - "storage path: " + storagePath + ": " + e.getMessage()); - } + return getPasswordFromPath(storagePath); } else { return null; @@ -211,6 +201,21 @@ public String getSshPassword() throws ConfigurationException { } } + public String getPasswordFromPath(String storagePath) throws ConfigurationException { + //look up storage value + Path path = PathUtil.asPath(storagePath); + try { + ResourceMeta contents = context.getStorageTree().getResource(path) + .getContents(); + ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); + contents.writeContent(byteArrayOutputStream); + return byteArrayOutputStream.toString(); + } catch (StorageException | IOException e) { + throw new ConfigurationException("Failed to read the ssh password for " + + "storage path: " + storagePath + ": " + e.getMessage()); + } + } + public Integer getSSHTimeout() throws ConfigurationException { Integer timeout = null; final String stimeout = PropertyResolver.resolveProperty( @@ -249,6 +254,23 @@ public String getSshUser() { return user; } + public String getSshNodeUser(INodeEntry node) { + final String user; + user = PropertyResolver.resolveProperty( + AnsibleDescribable.ANSIBLE_SSH_USER, + null, + getFrameworkProject(), + getFramework(), + node, + getJobConf() + ); + + if (null != user && user.contains("${")) { + return DataContextUtils.replaceDataReferencesInString(user, getContext().getDataContext()); + } + return user; + } + public AuthenticationType getSshAuthenticationType() { String authType = PropertyResolver.resolveProperty( @@ -880,4 +902,84 @@ public Map getListOptions(){ } return options; } + + public Map> getNodesAuthenticationMap(){ + + Map> authenticationNodesMap = new HashMap<>(); + + this.context.getNodes().forEach((node) -> { + String keyPath = PropertyResolver.resolveProperty( + AnsibleDescribable.ANSIBLE_SSH_PASSWORD_STORAGE_PATH, + null, + getFrameworkProject(), + getFramework(), + node, + getJobConf() + ); + + Map auth = new HashMap<>(); + + if(null!=keyPath){ + try { + auth.put("ansible_password",getPasswordFromPath(keyPath) ); + } catch (ConfigurationException e) { + throw new RuntimeException(e); + } + + } + String userName = getSshNodeUser(node); + + if(null!=userName){ + auth.put("ansible_user",userName ); + } + + authenticationNodesMap.put(node.getNodename(), auth); + }); + + return authenticationNodesMap; + } + + + public List getListNodesKeyPath(){ + + List secretPaths = new ArrayList<>(); + + this.context.getNodes().forEach((node) -> { + String keyPath = PropertyResolver.resolveProperty( + AnsibleDescribable.ANSIBLE_SSH_KEYPATH, + null, + getFrameworkProject(), + getFramework(), + node, + getJobConf() + ); + + if(null!=keyPath){ + if(!secretPaths.contains(keyPath)){ + secretPaths.add(keyPath); + } + + } + }); + + return secretPaths; + } + + + public Boolean generateInventoryNodesAuth() { + Boolean generateInventoryNodesAuth = null; + String sgenerateInventoryNodesAuth = PropertyResolver.resolveProperty( + AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, + null, + getFrameworkProject(), + getFramework(), + getNode(), + getJobConf() + ); + + if (null != sgenerateInventoryNodesAuth) { + generateInventoryNodesAuth = Boolean.parseBoolean(sgenerateInventoryNodesAuth); + } + return generateInventoryNodesAuth; + } } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index 8a0a09ad..acf2f979 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -43,6 +43,8 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes builder.property(PLAYBOOK_INLINE_PROP); builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); + builder.property(GENERATE_INVENTORY_PROP); + builder.property(GENERATE_INVENTORY_NODES_AUTH); builder.property(INVENTORY_INLINE_PROP); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 1f5ba521..0fef6462 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -43,6 +43,8 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); builder.property(INVENTORY_INLINE_PROP); + builder.property(GENERATE_INVENTORY_PROP); + builder.property(GENERATE_INVENTORY_NODES_AUTH); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); @@ -127,7 +129,7 @@ public Description getDescription() { @Override public List listSecretsPathWorkflowStep(ExecutionContext context, Map configuration) { AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(context, context.getFramework(), context.getNodes(), configuration); - return AnsibleUtil.getSecretsPath(builder); + return AnsibleUtil.getSecretsPathWorkflowSteps(builder); } @Override public Map getRuntimeProperties(ExecutionContext context) { diff --git a/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java b/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java index 126a36f7..9be0accd 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java @@ -86,6 +86,16 @@ public static List getSecretsPath(AnsibleRunnerContextBuilder builder){ } + public static List getSecretsPathWorkflowSteps(AnsibleRunnerContextBuilder builder){ + List secretPaths = getSecretsPath(builder); + List secretPathsNodes =builder.getListNodesKeyPath(); + + if(secretPathsNodes!=null && !secretPathsNodes.isEmpty()){ + secretPaths.addAll(secretPathsNodes); + } + return secretPaths; + } + public static Map getRuntimeProperties(ExecutionContext context, String propertyPrefix) { Map properties = null; From de654c6953feceb348e8fc953af61d4c0bb1aad3 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Thu, 11 Dec 2025 16:05:55 -0300 Subject: [PATCH 02/46] fix getting ssh-password from nodes --- .../plugins/ansible/ansible/AnsibleRunnerContextBuilder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 449ec7dd..3fa916bb 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -946,7 +946,7 @@ public List getListNodesKeyPath(){ this.context.getNodes().forEach((node) -> { String keyPath = PropertyResolver.resolveProperty( - AnsibleDescribable.ANSIBLE_SSH_KEYPATH, + AnsibleDescribable.ANSIBLE_SSH_PASSWORD_STORAGE_PATH, null, getFrameworkProject(), getFramework(), @@ -958,7 +958,6 @@ public List getListNodesKeyPath(){ if(!secretPaths.contains(keyPath)){ secretPaths.add(keyPath); } - } }); From 1fa16df32c71429dbc5b841070f3b7e3213790a0 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 11 Dec 2025 20:06:20 -0300 Subject: [PATCH 03/46] group_vars and properties in nodeexecutor --- .../ansible/ansible/AnsibleRunner.java | 41 ++++++++++++++++--- .../ansible/plugin/AnsibleNodeExecutor.java | 11 ++++- .../AnsiblePlaybookInlineWorkflowStep.java | 2 - .../plugin/AnsiblePlaybookWorkflowStep.java | 2 - 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 2072f1a4..8947afb6 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -128,7 +128,7 @@ public static AnsibleRunner buildAnsibleRunner(AnsibleRunnerContextBuilder conte } Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); - if(generateInventoryNodeAuth){ + if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); if (nodesAuth != null && !nodesAuth.isEmpty()) { ansibleRunnerBuilder.addNodeAuthToInventory(true); @@ -300,11 +300,13 @@ public static AnsibleRunner buildAnsibleRunner(AnsibleRunnerContextBuilder conte File tempSshVarsFile ; File tempBecameVarsFile ; File vaultPromptFile; - File tempNodeAuthFile = null; + File tempNodeAuthFile; + File groupVarsDir; String customTmpDirPath; - Boolean addNodeAuthToInventory; + @Builder.Default + Boolean addNodeAuthToInventory = false; Map> nodesAuthentication; public void deleteTempDirectory(Path tempDirectory) throws IOException { @@ -410,7 +412,7 @@ public int run() throws Exception { procArgs.add("-i"); procArgs.add(inventory); - if(addNodeAuthToInventory) { + if(addNodeAuthToInventory != null && addNodeAuthToInventory && nodesAuthentication != null && !nodesAuthentication.isEmpty()) { Map hostUsers = new LinkedHashMap<>(); Map hostPasswords = new LinkedHashMap<>(); nodesAuthentication.forEach((nodeName, authValues) -> { @@ -437,7 +439,30 @@ public int run() throws Exception { yamlData.put("host_users", hostUsers); try { String yamlContent = mapperYaml.writeValueAsString(yamlData); - tempNodeAuthFile = AnsibleUtil.createTemporaryFile("", "all.yaml", yamlContent, customTmpDirPath); + + // Create group_vars directory structure + File inventoryFile = new File(inventory); + File inventoryParentDir = inventoryFile.getParentFile(); + + if (inventoryParentDir != null) { + groupVarsDir = new File(inventoryParentDir, "group_vars"); + + if (!groupVarsDir.exists()) { + if (!groupVarsDir.mkdirs()) { + throw new RuntimeException("Failed to create group_vars directory at: " + groupVarsDir.getAbsolutePath()); + } + } + + // Create all.yaml in group_vars directory + tempNodeAuthFile = new File(groupVarsDir, "all.yaml"); + java.nio.file.Files.writeString(tempNodeAuthFile.toPath(), yamlContent); + tempNodeAuthFile.deleteOnExit(); + groupVarsDir.deleteOnExit(); + } else { + // Fallback to temp file if inventory has no parent directory + tempNodeAuthFile = AnsibleUtil.createTemporaryFile("group_vars", "all.yaml", yamlContent, customTmpDirPath); + } + } catch (IOException e) { throw new RuntimeException("Failed to write all.yaml for node auth", e); } @@ -712,6 +737,12 @@ public int run() throws Exception { tempNodeAuthFile.deleteOnExit(); } + if (groupVarsDir != null && groupVarsDir.exists()) { + if (!groupVarsDir.delete()) { + groupVarsDir.deleteOnExit(); + } + } + if (usingTempDirectory && !retainTempDirectory) { deleteTempDirectory(baseDirectory); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java index f7421c3b..e75ae84e 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java @@ -36,6 +36,7 @@ public class AnsibleNodeExecutor implements NodeExecutor, AnsibleDescribable, Pr builder.property(WINDOWS_EXECUTABLE_PROP); builder.property(CONFIG_FILE_PATH); builder.property(GENERATE_INVENTORY_PROP); + builder.property(GENERATE_INVENTORY_NODES_AUTH); builder.property(SSH_AUTH_TYPE_PROP); builder.property(SSH_USER_PROP); builder.property(SSH_PASSWORD_STORAGE_PROP); @@ -63,6 +64,8 @@ public class AnsibleNodeExecutor implements NodeExecutor, AnsibleDescribable, Pr builder.frameworkMapping(ANSIBLE_CONFIG_FILE_PATH,FWK_PROP_PREFIX + ANSIBLE_CONFIG_FILE_PATH); builder.mapping(ANSIBLE_GENERATE_INVENTORY,PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY,FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); + builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH,PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH,FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); builder.mapping(ANSIBLE_SSH_AUTH_TYPE,PROJ_PROP_PREFIX + ANSIBLE_SSH_AUTH_TYPE); builder.frameworkMapping(ANSIBLE_SSH_AUTH_TYPE,FWK_PROP_PREFIX + ANSIBLE_SSH_AUTH_TYPE); builder.mapping(ANSIBLE_SSH_USER,PROJ_PROP_PREFIX + ANSIBLE_SSH_USER); @@ -197,7 +200,13 @@ public List listSecretsPath(ExecutionContext context, INodeEntry node) { jobConf.put(AnsibleDescribable.ANSIBLE_LIMIT,node.getNodename()); AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(node, context, context.getFramework(), jobConf); - return AnsibleUtil.getSecretsPath(builder); + List secretPaths = AnsibleUtil.getSecretsPath(builder); + List secretPathsNodes = builder.getListNodesKeyPath(); + + if(secretPathsNodes != null && !secretPathsNodes.isEmpty()){ + secretPaths.addAll(secretPathsNodes); + } + return secretPaths; } } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index acf2f979..8a0a09ad 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -43,8 +43,6 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes builder.property(PLAYBOOK_INLINE_PROP); builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); - builder.property(GENERATE_INVENTORY_PROP); - builder.property(GENERATE_INVENTORY_NODES_AUTH); builder.property(INVENTORY_INLINE_PROP); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 0fef6462..762ad794 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -43,8 +43,6 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); builder.property(INVENTORY_INLINE_PROP); - builder.property(GENERATE_INVENTORY_PROP); - builder.property(GENERATE_INVENTORY_NODES_AUTH); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); From 71b4af994b7b0e80cdab5bb5471e51a3e187589d Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 18 Dec 2025 22:03:05 -0300 Subject: [PATCH 04/46] Added Inline Playbook option for testing --- .../plugin/AnsiblePlaybookInlineWorkflowNodeStep.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java index a9a1d40c..5827ccc6 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java @@ -43,6 +43,8 @@ public class AnsiblePlaybookInlineWorkflowNodeStep implements NodeStepPlugin, An builder.property(PLAYBOOK_INLINE_PROP); builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); + builder.property(GENERATE_INVENTORY_PROP); + builder.property(GENERATE_INVENTORY_NODES_AUTH); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); @@ -121,7 +123,13 @@ public void executeNodeStep( @Override public List listSecretsPathWorkflowNodeStep(ExecutionContext context, INodeEntry node, Map configuration) { AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(node, context, context.getFramework(), configuration); - return AnsibleUtil.getSecretsPath(builder); + List secretPaths = AnsibleUtil.getSecretsPath(builder); + List secretPathsNodes = builder.getListNodesKeyPath(); + + if(secretPathsNodes != null && !secretPathsNodes.isEmpty()){ + secretPaths.addAll(secretPathsNodes); + } + return secretPaths; } @Override From 70d64d2f7ec9fa65499e6b3c6233a39702070c9d Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Tue, 23 Dec 2025 22:09:55 -0300 Subject: [PATCH 05/46] authentitacion debug logs --- .../ansible/AnsibleRunnerContextBuilder.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 3fa916bb..128b5046 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -209,7 +209,7 @@ public String getPasswordFromPath(String storagePath) throws ConfigurationExcept .getContents(); ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); contents.writeContent(byteArrayOutputStream); - return byteArrayOutputStream.toString(); + return byteArrayOutputStream.toString("UTF-8"); } catch (StorageException | IOException e) { throw new ConfigurationException("Failed to read the ssh password for " + "storage path: " + storagePath + ": " + e.getMessage()); @@ -907,6 +907,8 @@ public Map> getNodesAuthenticationMap(){ Map> authenticationNodesMap = new HashMap<>(); + System.err.println("DEBUG: getNodesAuthenticationMap called"); + this.context.getNodes().forEach((node) -> { String keyPath = PropertyResolver.resolveProperty( AnsibleDescribable.ANSIBLE_SSH_PASSWORD_STORAGE_PATH, @@ -917,17 +919,28 @@ public Map> getNodesAuthenticationMap(){ getJobConf() ); + System.err.println("DEBUG: Node " + node.getNodename() + " keyPath: " + keyPath); + Map auth = new HashMap<>(); if(null!=keyPath){ try { - auth.put("ansible_password",getPasswordFromPath(keyPath) ); + String password = getPasswordFromPath(keyPath); + System.err.println("DEBUG: Retrieved password for " + node.getNodename() + ": " + (password != null ? password.substring(0, Math.min(3, password.length())) + "..." : "null")); + System.err.println("DEBUG: Password length: " + (password != null ? password.length() : 0)); + System.err.println("DEBUG: Password bytes: " + (password != null ? java.util.Arrays.toString(password.getBytes("UTF-8")) : "null")); + auth.put("ansible_password", password); } catch (ConfigurationException e) { + System.err.println("DEBUG: Error retrieving password for " + node.getNodename() + ": " + e.getMessage()); throw new RuntimeException(e); + } catch (Exception e2) { + System.err.println("DEBUG: Unexpected error: " + e2.getMessage()); + throw new RuntimeException(e2); } } String userName = getSshNodeUser(node); + System.err.println("DEBUG: Node " + node.getNodename() + " userName: " + userName); if(null!=userName){ auth.put("ansible_user",userName ); @@ -936,6 +949,7 @@ public Map> getNodesAuthenticationMap(){ authenticationNodesMap.put(node.getNodename(), auth); }); + System.err.println("DEBUG: authenticationNodesMap has " + authenticationNodesMap.size() + " entries"); return authenticationNodesMap; } From 1f7a7931b00b7aa1c9eccf9d4cbf39a7fea16da4 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Tue, 6 Jan 2026 11:10:05 -0300 Subject: [PATCH 06/46] fix encrypted group vars for generated inventory enable the node auth just for workflow steps --- .../ansible/ansible/AnsibleRunner.java | 89 ++++++++++++++----- .../ansible/AnsibleRunnerContextBuilder.java | 10 +-- .../AnsiblePlaybookInlineWorkflowStep.java | 17 +++- .../plugin/AnsiblePlaybookWorkflowStep.java | 15 +++- 4 files changed, 100 insertions(+), 31 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 8947afb6..341c6415 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -6,6 +6,7 @@ import com.dtolabs.rundeck.core.utils.SSHAgentProcess; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; import lombok.Builder; import lombok.Data; @@ -127,15 +128,6 @@ public static AnsibleRunner buildAnsibleRunner(AnsibleRunnerContextBuilder conte ansibleRunnerBuilder.inventory(inventory); } - Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); - if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ - Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); - if (nodesAuth != null && !nodesAuth.isEmpty()) { - ansibleRunnerBuilder.addNodeAuthToInventory(true); - ansibleRunnerBuilder.nodesAuthentication(nodesAuth); - } - } - String limit = contextBuilder.getLimit(); if (limit != null) { ansibleRunnerBuilder.limits(limit); @@ -284,7 +276,12 @@ public static AnsibleRunner buildAnsibleRunner(AnsibleRunnerContextBuilder conte @Builder.Default private boolean useAnsibleVault = false; - static ObjectMapper mapperYaml = new ObjectMapper(new YAMLFactory()); + static ObjectMapper mapperYaml = new ObjectMapper( + new YAMLFactory() + .enable(YAMLGenerator.Feature.LITERAL_BLOCK_STYLE) + .disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER) + .enable(YAMLGenerator.Feature.MINIMIZE_QUOTES) + ); static ObjectMapper mapperJson = new ObjectMapper(); private ProcessExecutor.ProcessExecutorBuilder processExecutorBuilder; @@ -399,8 +396,8 @@ public int run() throws Exception { // 3) become-password is used for node authentication if (encryptExtraVars && extraVars != null && !extraVars.isEmpty() || sshUsePassword || - (become && becomePassword != null && !becomePassword.isEmpty())) { - + (become && becomePassword != null && !becomePassword.isEmpty()) || + (addNodeAuthToInventory != null && addNodeAuthToInventory)){ useAnsibleVault = ansibleVault.checkAnsibleVault(); if(!useAnsibleVault) { @@ -413,6 +410,7 @@ public int run() throws Exception { procArgs.add(inventory); if(addNodeAuthToInventory != null && addNodeAuthToInventory && nodesAuthentication != null && !nodesAuthentication.isEmpty()) { + Map hostUsers = new LinkedHashMap<>(); Map hostPasswords = new LinkedHashMap<>(); nodesAuthentication.forEach((nodeName, authValues) -> { @@ -425,7 +423,7 @@ public int run() throws Exception { String encryptedPassword = password; if (useAnsibleVault) { try { - encryptedPassword = encryptExtraVarsKey(password); + encryptedPassword = ansibleVault.encryptVariable("ansible_password", password); } catch (Exception e) { throw new RuntimeException(e); } @@ -433,15 +431,17 @@ public int run() throws Exception { hostPasswords.put(nodeName, encryptedPassword); } }); - // Build YAML structure - Map yamlData = new LinkedHashMap<>(); - yamlData.put("host_passwords", hostPasswords); - yamlData.put("host_users", hostUsers); + + // Build YAML content using helper method + String yamlContent = buildGroupVarsYaml(hostPasswords, hostUsers); + try { - String yamlContent = mapperYaml.writeValueAsString(yamlData); // Create group_vars directory structure File inventoryFile = new File(inventory); + + System.err.println("DEBUG: inventoryFile: " + inventoryFile.getAbsolutePath()); + File inventoryParentDir = inventoryFile.getParentFile(); if (inventoryParentDir != null) { @@ -456,13 +456,17 @@ public int run() throws Exception { // Create all.yaml in group_vars directory tempNodeAuthFile = new File(groupVarsDir, "all.yaml"); java.nio.file.Files.writeString(tempNodeAuthFile.toPath(), yamlContent); - tempNodeAuthFile.deleteOnExit(); - groupVarsDir.deleteOnExit(); } else { // Fallback to temp file if inventory has no parent directory tempNodeAuthFile = AnsibleUtil.createTemporaryFile("group_vars", "all.yaml", yamlContent, customTmpDirPath); } + System.err.println("DEBUG: tempNodeAuthFile: " + tempNodeAuthFile.getAbsolutePath()); + + //set extra vars to resolve the host specific authentication + procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] }}\""); + procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] }}\""); + } catch (IOException e) { throw new RuntimeException("Failed to write all.yaml for node auth", e); } @@ -844,6 +848,51 @@ public boolean registerKeySshAgent(String keyPath) throws Exception { } + /** + * Builds YAML content for Ansible group_vars with vault-encrypted passwords and host users. + * This method manually constructs the YAML because Jackson cannot properly handle Ansible's + * custom !vault tag, which would result in an extra | character. + * + * @param hostPasswords Map of host names to vault-encrypted password strings + * @param hostUsers Map of host names to usernames + * @return YAML content string ready to be written to all.yaml + */ + private String buildGroupVarsYaml(Map hostPasswords, Map hostUsers) { + StringBuilder yamlContent = new StringBuilder(); + yamlContent.append("host_passwords:\n"); + + for (Map.Entry entry : hostPasswords.entrySet()) { + yamlContent.append(" ").append(entry.getKey()).append(": "); + String vaultValue = entry.getValue(); + + // The vault value already contains "!vault |\n" followed by the encrypted content + // We just need to split and indent properly (4 spaces for vault content lines) + if (vaultValue.contains("\n")) { + String[] lines = vaultValue.split("\n", -1); + for (int i = 0; i < lines.length; i++) { + if (i == 0) { + // First line: "!vault |" + yamlContent.append(lines[i]).append("\n"); + } else if (i < lines.length - 1 || !lines[i].isEmpty()) { + // Vault content: indent with 4 spaces + yamlContent.append(" ").append(lines[i]).append("\n"); + } + } + } else { + // Single line value (shouldn't happen with vault, but handle it) + yamlContent.append(vaultValue).append("\n"); + } + } + + yamlContent.append("\nhost_users:\n"); + for (Map.Entry entry : hostUsers.entrySet()) { + yamlContent.append(" ").append(entry.getKey()).append(": ") + .append(entry.getValue()).append("\n"); + } + + return yamlContent.toString(); + } + public String encryptExtraVarsKey(String extraVars) throws Exception { Map extraVarsMap = new HashMap<>(); Map encryptedExtraVarsMap = new HashMap<>(); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 128b5046..b821bfdb 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -256,15 +256,7 @@ public String getSshUser() { public String getSshNodeUser(INodeEntry node) { final String user; - user = PropertyResolver.resolveProperty( - AnsibleDescribable.ANSIBLE_SSH_USER, - null, - getFrameworkProject(), - getFramework(), - node, - getJobConf() - ); - + user = node.getUsername(); if (null != user && user.contains("${")) { return DataContextUtils.replaceDataReferencesInString(user, getContext().getDataContext()); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index 8a0a09ad..479eaf01 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -37,6 +37,10 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes builder.name(SERVICE_PROVIDER_NAME); builder.title("Ansible Playbook Inline"); builder.description("Runs an Inline Ansible Playbook."); + builder.metadata( + "automaticWorkflowStepRunnerAssociation", + "true" + ); builder.property(BINARIES_DIR_PATH_PROP); builder.property(BASE_DIR_PROP); @@ -93,8 +97,19 @@ public void executeStep(PluginStepContext context, Map configura configuration, pluginGroup); + try { runner = AnsibleRunner.buildAnsibleRunner(contextBuilder); + + Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); + if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ + Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); + if (nodesAuth != null && !nodesAuth.isEmpty()) { + runner.setAddNodeAuthToInventory(true); + runner.setNodesAuthentication(nodesAuth); + } + } + runner.setCustomTmpDirPath(AnsibleUtil.getCustomTmpPathDir(contextBuilder.getFramework())); } catch (ConfigurationException e) { throw new StepException("Error configuring Ansible runner: " + e.getMessage(), e, AnsibleException.AnsibleFailureReason.ParseArgumentsError); @@ -132,7 +147,7 @@ public Description getDescription() { @Override public List listSecretsPathWorkflowStep(ExecutionContext context, Map configuration) { AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(context, context.getFramework(), context.getNodes(), configuration); - return AnsibleUtil.getSecretsPath(builder); + return AnsibleUtil.getSecretsPathWorkflowSteps(builder); } @Override diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 762ad794..1f4fb319 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -36,7 +36,10 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.name(SERVICE_PROVIDER_NAME); builder.title("Ansible Playbook"); builder.description("Runs an Ansible Playbook."); - + builder.metadata( + "automaticWorkflowStepRunnerAssociation", + "true" + ); builder.property(BINARIES_DIR_PATH_PROP); builder.property(BASE_DIR_PROP); builder.property(PLAYBOOK_PATH_PROP); @@ -94,6 +97,16 @@ public void executeStep(PluginStepContext context, Map configura try { runner = AnsibleRunner.buildAnsibleRunner(contextBuilder); + + Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); + if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ + Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); + if (nodesAuth != null && !nodesAuth.isEmpty()) { + runner.setAddNodeAuthToInventory(true); + runner.setNodesAuthentication(nodesAuth); + } + } + runner.setCustomTmpDirPath(AnsibleUtil.getCustomTmpPathDir(contextBuilder.getFramework())); } catch (ConfigurationException e) { throw new StepException("Error configuring Ansible runner: " + e.getMessage(), e, AnsibleException.AnsibleFailureReason.ParseArgumentsError); From 4a4c3c29e429ba4b18d540258ef9418c3799e35c Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Tue, 6 Jan 2026 12:15:19 -0300 Subject: [PATCH 07/46] Added checkboxes at project level, added execution-tmp folder creation and cleanup --- .../ansible/AnsibleRunnerContextBuilder.java | 112 +++++++++++++++++- .../AnsiblePlaybookInlineWorkflowStep.java | 7 ++ .../plugin/AnsiblePlaybookWorkflowStep.java | 7 ++ .../ansible/plugin/AnsiblePluginGroup.java | 28 +++++ 4 files changed, 152 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index b821bfdb..b646e94b 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -40,6 +40,7 @@ public class AnsibleRunnerContextBuilder { private final Map jobConf; private final Collection nodes; private final Collection tempFiles; + private File executionSpecificDir; private AnsiblePluginGroup pluginGroup; @@ -610,6 +611,17 @@ public Boolean generateInventory() { if (null != sgenerateInventory) { generateInventory = Boolean.parseBoolean(sgenerateInventory); } + + // Fallback to plugin group configuration if not set + if (generateInventory == null || !generateInventory) { + if (this.pluginGroup != null && this.pluginGroup.getGenerateInventory() != null && this.pluginGroup.getGenerateInventory()) { + this.context.getExecutionLogger().log( + 4, "plugin group set getGenerateInventory: " + this.pluginGroup.getGenerateInventory() + ); + generateInventory = this.pluginGroup.getGenerateInventory(); + } + } + return generateInventory; } @@ -620,7 +632,8 @@ public String getInventory() throws ConfigurationException { if (isGenerated != null && isGenerated) { - File tempInventory = new AnsibleInventoryBuilder(this.nodes, AnsibleUtil.getCustomTmpPathDir(framework)).buildInventory(); + String executionSpecificDir = getExecutionSpecificTmpDir(); + File tempInventory = new AnsibleInventoryBuilder(this.nodes, executionSpecificDir).buildInventory(); tempFiles.add(tempInventory); inventory = tempInventory.getAbsolutePath(); return inventory; @@ -640,7 +653,8 @@ public String getInventory() throws ConfigurationException { the builder gets the nodes from rundeck in rundeck node format and converts to ansible inventory we don't want that, we simply want the list we provided in ansible format */ - File tempInventory = new AnsibleInlineInventoryBuilder(inline_inventory,AnsibleUtil.getCustomTmpPathDir(framework)).buildInventory(); + String executionSpecificDir = getExecutionSpecificTmpDir(); + File tempInventory = new AnsibleInlineInventoryBuilder(inline_inventory, executionSpecificDir).buildInventory(); tempFiles.add(tempInventory); inventory = tempInventory.getAbsolutePath(); return inventory; @@ -769,12 +783,53 @@ public INodeEntry getNode() { public void cleanupTempFiles() { + System.err.println("DEBUG: cleanupTempFiles called"); + + // Clean up individual temp files for (File temp : tempFiles) { if (!getDebug()) { + System.err.println("DEBUG: Deleting temp file: " + temp.getAbsolutePath()); temp.delete(); } } tempFiles.clear(); + + // Clean up execution-specific directory (including group_vars) + if (executionSpecificDir != null && executionSpecificDir.exists()) { + System.err.println("DEBUG: Execution-specific directory exists: " + executionSpecificDir.getAbsolutePath()); + if (!getDebug()) { + System.err.println("DEBUG: Deleting execution-specific directory recursively"); + deleteDirectoryRecursively(executionSpecificDir); + System.err.println("DEBUG: Execution-specific directory deleted"); + } else { + System.err.println("DEBUG: Skipping cleanup due to debug mode"); + } + } else { + System.err.println("DEBUG: No execution-specific directory to cleanup"); + } + } + + /** + * Recursively deletes a directory and all its contents. + * + * @param directory The directory to delete + */ + private void deleteDirectoryRecursively(File directory) { + if (directory == null || !directory.exists()) { + return; + } + + File[] files = directory.listFiles(); + if (files != null) { + for (File file : files) { + if (file.isDirectory()) { + deleteDirectoryRecursively(file); + } else { + file.delete(); + } + } + } + directory.delete(); } public Boolean getUseSshAgent() { @@ -985,6 +1040,59 @@ public Boolean generateInventoryNodesAuth() { if (null != sgenerateInventoryNodesAuth) { generateInventoryNodesAuth = Boolean.parseBoolean(sgenerateInventoryNodesAuth); } + + // Fallback to plugin group configuration if not set + if (generateInventoryNodesAuth == null || !generateInventoryNodesAuth) { + if (this.pluginGroup != null && this.pluginGroup.getGenerateInventoryNodesAuth() != null && this.pluginGroup.getGenerateInventoryNodesAuth()) { + this.context.getExecutionLogger().log( + 4, "plugin group set getGenerateInventoryNodesAuth: " + this.pluginGroup.getGenerateInventoryNodesAuth() + ); + generateInventoryNodesAuth = this.pluginGroup.getGenerateInventoryNodesAuth(); + } + } + return generateInventoryNodesAuth; } + + /** + * Creates and returns an execution-specific temporary directory path. + * This ensures that each execution has its own isolated directory for inventory and group_vars, + * preventing conflicts when multiple workflow step executions run in parallel. + * + * @return The path to the execution-specific directory + */ + private String getExecutionSpecificTmpDir() { + // Return cached directory if already created + if (executionSpecificDir != null) { + System.err.println("DEBUG: Using cached execution-specific directory: " + executionSpecificDir.getAbsolutePath()); + return executionSpecificDir.getAbsolutePath(); + } + + String executionId = null; + + // Get execution ID from data context + if (context.getDataContext() != null && context.getDataContext().get("job") != null) { + executionId = context.getDataContext().get("job").get("execid"); + System.err.println("DEBUG: Execution ID from context: " + executionId); + } + + // Get base tmp directory + String baseTmpDir = AnsibleUtil.getCustomTmpPathDir(framework); + + // Create execution-specific directory + if (executionId != null && !executionId.isEmpty()) { + executionSpecificDir = new File(baseTmpDir, "ansible-exec-" + executionId); + if (!executionSpecificDir.exists()) { + System.err.println("DEBUG: Creating execution-specific directory: " + executionSpecificDir.getAbsolutePath()); + executionSpecificDir.mkdirs(); + } else { + System.err.println("DEBUG: Execution-specific directory already exists: " + executionSpecificDir.getAbsolutePath()); + } + return executionSpecificDir.getAbsolutePath(); + } + + // Fallback to base tmp dir if execution ID is not available + System.err.println("DEBUG: No execution ID found, falling back to base tmp dir: " + baseTmpDir); + return baseTmpDir; + } } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index 479eaf01..3b12cabd 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -48,6 +48,7 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); builder.property(INVENTORY_INLINE_PROP); + builder.property(GENERATE_INVENTORY_PROP); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); @@ -66,6 +67,12 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes builder.property(BECOME_PASSWORD_STORAGE_PROP); builder.property(DISABLE_LIMIT_PROP); + // Project and framework level mappings for inventory generation + builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); + builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); + builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + DESC = builder.build(); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 1f4fb319..710d1377 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -46,6 +46,7 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); builder.property(INVENTORY_INLINE_PROP); + builder.property(GENERATE_INVENTORY_PROP); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); @@ -64,6 +65,12 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.property(BECOME_PASSWORD_STORAGE_PROP); builder.property(DISABLE_LIMIT_PROP); + // Project and framework level mappings for inventory generation + builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); + builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); + builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + DESC = builder.build(); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java index 743665f0..064092fa 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java @@ -39,6 +39,22 @@ public void setEncryptExtraVars(Boolean encryptExtraVars) { this.encryptExtraVars = encryptExtraVars; } + public Boolean getGenerateInventory() { + return generateInventory; + } + + public void setGenerateInventory(Boolean generateInventory) { + this.generateInventory = generateInventory; + } + + public Boolean getGenerateInventoryNodesAuth() { + return generateInventoryNodesAuth; + } + + public void setGenerateInventoryNodesAuth(Boolean generateInventoryNodesAuth) { + this.generateInventoryNodesAuth = generateInventoryNodesAuth; + } + @PluginProperty( title = "Ansible config file path", description = "Set ansible config file path." @@ -57,6 +73,18 @@ public void setEncryptExtraVars(Boolean encryptExtraVars) { ) Boolean encryptExtraVars; + @PluginProperty( + title = "Generate inventory", + description = "Generate an Ansible inventory from the targeted nodes." + ) + Boolean generateInventory; + + @PluginProperty( + title = "Generate inventory with node authentication", + description = "Add node-specific authentication credentials to the generated inventory via group_vars." + ) + Boolean generateInventoryNodesAuth; + @Override public Description getDescription() { DescriptionBuilder builder = DescriptionBuilder.builder(); From 1590942a90c046a2144de05d936797ef56e5f86d Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Tue, 6 Jan 2026 14:37:17 -0300 Subject: [PATCH 08/46] Added Vault validation and Name escaping, added debug logs to yaml file creation --- .../ansible/ansible/AnsibleRunner.java | 135 +++++++++++++++++- 1 file changed, 130 insertions(+), 5 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 341c6415..57476dd0 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -433,41 +433,58 @@ public int run() throws Exception { }); // Build YAML content using helper method + System.err.println("DEBUG: Building group_vars YAML with " + hostPasswords.size() + " passwords and " + hostUsers.size() + " users"); String yamlContent = buildGroupVarsYaml(hostPasswords, hostUsers); + System.err.println("DEBUG: YAML content built successfully, length: " + yamlContent.length()); try { // Create group_vars directory structure File inventoryFile = new File(inventory); - System.err.println("DEBUG: inventoryFile: " + inventoryFile.getAbsolutePath()); + System.err.println("DEBUG: inventory file exists: " + inventoryFile.exists()); File inventoryParentDir = inventoryFile.getParentFile(); + System.err.println("DEBUG: inventoryParentDir: " + (inventoryParentDir != null ? inventoryParentDir.getAbsolutePath() : "null")); if (inventoryParentDir != null) { groupVarsDir = new File(inventoryParentDir, "group_vars"); + System.err.println("DEBUG: group_vars directory path: " + groupVarsDir.getAbsolutePath()); if (!groupVarsDir.exists()) { + System.err.println("DEBUG: Creating group_vars directory"); if (!groupVarsDir.mkdirs()) { throw new RuntimeException("Failed to create group_vars directory at: " + groupVarsDir.getAbsolutePath()); } + System.err.println("DEBUG: group_vars directory created successfully"); + } else { + System.err.println("DEBUG: group_vars directory already exists"); } // Create all.yaml in group_vars directory tempNodeAuthFile = new File(groupVarsDir, "all.yaml"); + System.err.println("DEBUG: Writing all.yaml to: " + tempNodeAuthFile.getAbsolutePath()); java.nio.file.Files.writeString(tempNodeAuthFile.toPath(), yamlContent); + System.err.println("DEBUG: all.yaml written successfully, file size: " + tempNodeAuthFile.length() + " bytes"); } else { // Fallback to temp file if inventory has no parent directory + System.err.println("DEBUG: No parent directory, using temporary file"); tempNodeAuthFile = AnsibleUtil.createTemporaryFile("group_vars", "all.yaml", yamlContent, customTmpDirPath); + System.err.println("DEBUG: Temporary all.yaml created at: " + tempNodeAuthFile.getAbsolutePath()); } System.err.println("DEBUG: tempNodeAuthFile: " + tempNodeAuthFile.getAbsolutePath()); + System.err.println("DEBUG: tempNodeAuthFile exists: " + tempNodeAuthFile.exists()); + System.err.println("DEBUG: tempNodeAuthFile readable: " + tempNodeAuthFile.canRead()); //set extra vars to resolve the host specific authentication + System.err.println("DEBUG: Adding ansible extra vars for host-specific auth"); procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] }}\""); procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] }}\""); } catch (IOException e) { + System.err.println("ERROR: Failed to write all.yaml: " + e.getMessage()); + e.printStackTrace(); throw new RuntimeException("Failed to write all.yaml for node auth", e); } } @@ -858,17 +875,31 @@ public boolean registerKeySshAgent(String keyPath) throws Exception { * @return YAML content string ready to be written to all.yaml */ private String buildGroupVarsYaml(Map hostPasswords, Map hostUsers) { + System.err.println("DEBUG: buildGroupVarsYaml called with " + hostPasswords.size() + " hosts"); + StringBuilder yamlContent = new StringBuilder(); yamlContent.append("host_passwords:\n"); for (Map.Entry entry : hostPasswords.entrySet()) { - yamlContent.append(" ").append(entry.getKey()).append(": "); + String originalKey = entry.getKey(); + String escapedKey = escapeYamlKey(originalKey); + System.err.println("DEBUG: Processing host: " + originalKey + " -> escaped: " + escapedKey); + + yamlContent.append(" ").append(escapedKey).append(": "); String vaultValue = entry.getValue(); + // Validate vault format + if (!isValidVaultFormat(vaultValue)) { + System.err.println("ERROR: Invalid vault format for host: " + originalKey); + throw new RuntimeException("Invalid vault format for host: " + originalKey); + } + System.err.println("DEBUG: Vault format validated for host: " + originalKey); + // The vault value already contains "!vault |\n" followed by the encrypted content // We just need to split and indent properly (4 spaces for vault content lines) if (vaultValue.contains("\n")) { String[] lines = vaultValue.split("\n", -1); + System.err.println("DEBUG: Vault value has " + lines.length + " lines for host: " + originalKey); for (int i = 0; i < lines.length; i++) { if (i == 0) { // First line: "!vault |" @@ -880,17 +911,111 @@ private String buildGroupVarsYaml(Map hostPasswords, Map entry : hostUsers.entrySet()) { - yamlContent.append(" ").append(entry.getKey()).append(": ") - .append(entry.getValue()).append("\n"); + String originalKey = entry.getKey(); + String originalValue = entry.getValue(); + String escapedKey = escapeYamlKey(originalKey); + String escapedValue = escapeYamlValue(originalValue); + System.err.println("DEBUG: Adding user - host: " + originalKey + " -> " + escapedKey + ", user: " + originalValue + " -> " + escapedValue); + + yamlContent.append(" ").append(escapedKey).append(": ") + .append(escapedValue).append("\n"); } - return yamlContent.toString(); + String result = yamlContent.toString(); + System.err.println("DEBUG: Generated YAML content (" + result.length() + " bytes):"); + System.err.println("DEBUG: ========== YAML START =========="); + System.err.println(result); + System.err.println("DEBUG: ========== YAML END =========="); + + return result; + } + + /** + * Escapes YAML special characters in keys. + * If the key contains special characters, wraps it in quotes. + * + * @param key The key to escape + * @return Escaped key safe for YAML + */ + private String escapeYamlKey(String key) { + // YAML special characters that require quoting + if (key.matches(".*[:\\[\\]{}#&*!|>'\"%@`].*") || + key.startsWith("-") || + key.startsWith("?") || + key.matches("^[0-9].*")) { + // Escape any existing quotes and wrap in quotes + return "\"" + key.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; + } + return key; + } + + /** + * Escapes YAML special characters in values. + * If the value contains special characters, wraps it in quotes. + * + * @param value The value to escape + * @return Escaped value safe for YAML + */ + private String escapeYamlValue(String value) { + // Check if value needs quoting + if (value.matches(".*[:\\[\\]{}#&*!|>'\"%@`].*") || + value.startsWith("-") || + value.startsWith("?") || + value.trim().isEmpty()) { + // Escape any existing quotes and wrap in quotes + return "\"" + value.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; + } + return value; + } + + /** + * Validates that a string is in proper Ansible vault format. + * Expected format: "!vault |\n" followed by vault-encrypted content + * + * @param vaultValue The vault value to validate + * @return true if valid vault format, false otherwise + */ + private boolean isValidVaultFormat(String vaultValue) { + if (vaultValue == null || vaultValue.trim().isEmpty()) { + return false; + } + + // Must start with "!vault" (case insensitive) + String trimmed = vaultValue.trim(); + if (!trimmed.toLowerCase().startsWith("!vault")) { + return false; + } + + // For multiline vault values, should contain the pipe character and newline + if (vaultValue.contains("\n")) { + // Check if it follows the "!vault |" format + String firstLine = vaultValue.split("\n", 2)[0]; + if (!firstLine.trim().matches("!vault\\s*\\|?")) { + return false; + } + + // Should have encrypted content after the first line + String[] lines = vaultValue.split("\n"); + if (lines.length < 2) { + return false; + } + + // Vault content lines should not be empty (except maybe the last one) + for (int i = 1; i < lines.length - 1; i++) { + if (lines[i].trim().isEmpty()) { + return false; + } + } + } + + return true; } public String encryptExtraVarsKey(String extraVars) throws Exception { From e63534402baeae6b40ae1a96c9699cfe25b8be01 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Tue, 6 Jan 2026 16:26:40 -0300 Subject: [PATCH 09/46] clean debug messages --- .../ansible/ansible/AnsibleRunner.java | 66 ++++++++++--------- .../ansible/AnsibleRunnerContextBuilder.java | 27 +++----- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 57476dd0..0fb6bc43 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -433,52 +433,62 @@ public int run() throws Exception { }); // Build YAML content using helper method - System.err.println("DEBUG: Building group_vars YAML with " + hostPasswords.size() + " passwords and " + hostUsers.size() + " users"); String yamlContent = buildGroupVarsYaml(hostPasswords, hostUsers); - System.err.println("DEBUG: YAML content built successfully, length: " + yamlContent.length()); + + if(debug){ + System.err.println("DEBUG: Building group_vars YAML with " + hostPasswords.size() + " passwords and " + hostUsers.size() + " users"); + System.err.println("DEBUG: YAML content built successfully, length: " + yamlContent.length()); + } try { // Create group_vars directory structure File inventoryFile = new File(inventory); - System.err.println("DEBUG: inventoryFile: " + inventoryFile.getAbsolutePath()); - System.err.println("DEBUG: inventory file exists: " + inventoryFile.exists()); - File inventoryParentDir = inventoryFile.getParentFile(); - System.err.println("DEBUG: inventoryParentDir: " + (inventoryParentDir != null ? inventoryParentDir.getAbsolutePath() : "null")); + + if(debug) { + System.err.println("DEBUG: inventoryFile: " + inventoryFile.getAbsolutePath()); + System.err.println("DEBUG: inventory file exists: " + inventoryFile.exists()); + System.err.println("DEBUG: inventoryParentDir: " + (inventoryParentDir != null ? inventoryParentDir.getAbsolutePath() : "null")); + } if (inventoryParentDir != null) { groupVarsDir = new File(inventoryParentDir, "group_vars"); - System.err.println("DEBUG: group_vars directory path: " + groupVarsDir.getAbsolutePath()); + if(debug) { + System.err.println("DEBUG: group_vars directory path: " + groupVarsDir.getAbsolutePath()); + } if (!groupVarsDir.exists()) { - System.err.println("DEBUG: Creating group_vars directory"); if (!groupVarsDir.mkdirs()) { throw new RuntimeException("Failed to create group_vars directory at: " + groupVarsDir.getAbsolutePath()); } - System.err.println("DEBUG: group_vars directory created successfully"); - } else { - System.err.println("DEBUG: group_vars directory already exists"); } // Create all.yaml in group_vars directory tempNodeAuthFile = new File(groupVarsDir, "all.yaml"); - System.err.println("DEBUG: Writing all.yaml to: " + tempNodeAuthFile.getAbsolutePath()); java.nio.file.Files.writeString(tempNodeAuthFile.toPath(), yamlContent); - System.err.println("DEBUG: all.yaml written successfully, file size: " + tempNodeAuthFile.length() + " bytes"); + + if(debug) { + System.err.println("DEBUG: Writing all.yaml to: " + tempNodeAuthFile.getAbsolutePath()); + System.err.println("DEBUG: all.yaml written successfully, file size: " + tempNodeAuthFile.length() + " bytes"); + } } else { // Fallback to temp file if inventory has no parent directory - System.err.println("DEBUG: No parent directory, using temporary file"); tempNodeAuthFile = AnsibleUtil.createTemporaryFile("group_vars", "all.yaml", yamlContent, customTmpDirPath); - System.err.println("DEBUG: Temporary all.yaml created at: " + tempNodeAuthFile.getAbsolutePath()); + + if(debug) { + System.err.println("DEBUG: No parent directory, using temporary file"); + System.err.println("DEBUG: Temporary all.yaml created at: " + tempNodeAuthFile.getAbsolutePath()); + } } - System.err.println("DEBUG: tempNodeAuthFile: " + tempNodeAuthFile.getAbsolutePath()); - System.err.println("DEBUG: tempNodeAuthFile exists: " + tempNodeAuthFile.exists()); - System.err.println("DEBUG: tempNodeAuthFile readable: " + tempNodeAuthFile.canRead()); + if(debug) { + System.err.println("DEBUG: tempNodeAuthFile: " + tempNodeAuthFile.getAbsolutePath()); + System.err.println("DEBUG: tempNodeAuthFile exists: " + tempNodeAuthFile.exists()); + System.err.println("DEBUG: tempNodeAuthFile readable: " + tempNodeAuthFile.canRead()); + } //set extra vars to resolve the host specific authentication - System.err.println("DEBUG: Adding ansible extra vars for host-specific auth"); procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] }}\""); procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] }}\""); @@ -875,7 +885,6 @@ public boolean registerKeySshAgent(String keyPath) throws Exception { * @return YAML content string ready to be written to all.yaml */ private String buildGroupVarsYaml(Map hostPasswords, Map hostUsers) { - System.err.println("DEBUG: buildGroupVarsYaml called with " + hostPasswords.size() + " hosts"); StringBuilder yamlContent = new StringBuilder(); yamlContent.append("host_passwords:\n"); @@ -883,7 +892,6 @@ private String buildGroupVarsYaml(Map hostPasswords, Map entry : hostPasswords.entrySet()) { String originalKey = entry.getKey(); String escapedKey = escapeYamlKey(originalKey); - System.err.println("DEBUG: Processing host: " + originalKey + " -> escaped: " + escapedKey); yamlContent.append(" ").append(escapedKey).append(": "); String vaultValue = entry.getValue(); @@ -893,13 +901,10 @@ private String buildGroupVarsYaml(Map hostPasswords, Map hostPasswords, Map " + escapedKey + ", user: " + originalValue + " -> " + escapedValue); - yamlContent.append(" ").append(escapedKey).append(": ") .append(escapedValue).append("\n"); } String result = yamlContent.toString(); - System.err.println("DEBUG: Generated YAML content (" + result.length() + " bytes):"); - System.err.println("DEBUG: ========== YAML START =========="); - System.err.println(result); - System.err.println("DEBUG: ========== YAML END =========="); + + if(debug){ + System.err.println("DEBUG: Generated YAML content (" + result.length() + " bytes):"); + System.err.println("DEBUG: ========== YAML START =========="); + System.err.println(result); + System.err.println("DEBUG: ========== YAML END =========="); + } return result; } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index b646e94b..cc2ad0e2 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -783,8 +783,6 @@ public INodeEntry getNode() { public void cleanupTempFiles() { - System.err.println("DEBUG: cleanupTempFiles called"); - // Clean up individual temp files for (File temp : tempFiles) { if (!getDebug()) { @@ -796,7 +794,9 @@ public void cleanupTempFiles() { // Clean up execution-specific directory (including group_vars) if (executionSpecificDir != null && executionSpecificDir.exists()) { - System.err.println("DEBUG: Execution-specific directory exists: " + executionSpecificDir.getAbsolutePath()); + if (getDebug()) { + System.err.println("DEBUG: Execution-specific directory exists: " + executionSpecificDir.getAbsolutePath()); + } if (!getDebug()) { System.err.println("DEBUG: Deleting execution-specific directory recursively"); deleteDirectoryRecursively(executionSpecificDir); @@ -954,8 +954,6 @@ public Map> getNodesAuthenticationMap(){ Map> authenticationNodesMap = new HashMap<>(); - System.err.println("DEBUG: getNodesAuthenticationMap called"); - this.context.getNodes().forEach((node) -> { String keyPath = PropertyResolver.resolveProperty( AnsibleDescribable.ANSIBLE_SSH_PASSWORD_STORAGE_PATH, @@ -966,16 +964,11 @@ public Map> getNodesAuthenticationMap(){ getJobConf() ); - System.err.println("DEBUG: Node " + node.getNodename() + " keyPath: " + keyPath); - Map auth = new HashMap<>(); if(null!=keyPath){ try { String password = getPasswordFromPath(keyPath); - System.err.println("DEBUG: Retrieved password for " + node.getNodename() + ": " + (password != null ? password.substring(0, Math.min(3, password.length())) + "..." : "null")); - System.err.println("DEBUG: Password length: " + (password != null ? password.length() : 0)); - System.err.println("DEBUG: Password bytes: " + (password != null ? java.util.Arrays.toString(password.getBytes("UTF-8")) : "null")); auth.put("ansible_password", password); } catch (ConfigurationException e) { System.err.println("DEBUG: Error retrieving password for " + node.getNodename() + ": " + e.getMessage()); @@ -987,7 +980,6 @@ public Map> getNodesAuthenticationMap(){ } String userName = getSshNodeUser(node); - System.err.println("DEBUG: Node " + node.getNodename() + " userName: " + userName); if(null!=userName){ auth.put("ansible_user",userName ); @@ -996,7 +988,6 @@ public Map> getNodesAuthenticationMap(){ authenticationNodesMap.put(node.getNodename(), auth); }); - System.err.println("DEBUG: authenticationNodesMap has " + authenticationNodesMap.size() + " entries"); return authenticationNodesMap; } @@ -1073,7 +1064,10 @@ private String getExecutionSpecificTmpDir() { // Get execution ID from data context if (context.getDataContext() != null && context.getDataContext().get("job") != null) { executionId = context.getDataContext().get("job").get("execid"); - System.err.println("DEBUG: Execution ID from context: " + executionId); + + if (getDebug()) { + System.err.println("DEBUG: Execution ID from context: " + executionId); + } } // Get base tmp directory @@ -1083,16 +1077,15 @@ private String getExecutionSpecificTmpDir() { if (executionId != null && !executionId.isEmpty()) { executionSpecificDir = new File(baseTmpDir, "ansible-exec-" + executionId); if (!executionSpecificDir.exists()) { - System.err.println("DEBUG: Creating execution-specific directory: " + executionSpecificDir.getAbsolutePath()); + if (getDebug()) { + System.err.println("DEBUG: Creating execution-specific directory: " + executionSpecificDir.getAbsolutePath()); + } executionSpecificDir.mkdirs(); } else { System.err.println("DEBUG: Execution-specific directory already exists: " + executionSpecificDir.getAbsolutePath()); } return executionSpecificDir.getAbsolutePath(); } - - // Fallback to base tmp dir if execution ID is not available - System.err.println("DEBUG: No execution ID found, falling back to base tmp dir: " + baseTmpDir); return baseTmpDir; } } From d6635a35052640cbca5a80e0c26eab6fe78e0757 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Tue, 6 Jan 2026 16:35:00 -0300 Subject: [PATCH 10/46] removed ANSIBLE_GENERATE_INVENTORY_NODES_AUTH to only use plugin group configuration --- .../ansible/AnsibleRunnerContextBuilder.java | 32 ++++++------------- .../AnsiblePlaybookInlineWorkflowStep.java | 2 -- .../plugin/AnsiblePlaybookWorkflowStep.java | 2 -- 3 files changed, 9 insertions(+), 27 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index cc2ad0e2..48183627 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -1018,31 +1018,17 @@ public List getListNodesKeyPath(){ public Boolean generateInventoryNodesAuth() { - Boolean generateInventoryNodesAuth = null; - String sgenerateInventoryNodesAuth = PropertyResolver.resolveProperty( - AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, - null, - getFrameworkProject(), - getFramework(), - getNode(), - getJobConf() - ); - - if (null != sgenerateInventoryNodesAuth) { - generateInventoryNodesAuth = Boolean.parseBoolean(sgenerateInventoryNodesAuth); - } - - // Fallback to plugin group configuration if not set - if (generateInventoryNodesAuth == null || !generateInventoryNodesAuth) { - if (this.pluginGroup != null && this.pluginGroup.getGenerateInventoryNodesAuth() != null && this.pluginGroup.getGenerateInventoryNodesAuth()) { - this.context.getExecutionLogger().log( - 4, "plugin group set getGenerateInventoryNodesAuth: " + this.pluginGroup.getGenerateInventoryNodesAuth() - ); - generateInventoryNodesAuth = this.pluginGroup.getGenerateInventoryNodesAuth(); - } + // Only use plugin group configuration + if (this.pluginGroup != null && this.pluginGroup.getGenerateInventoryNodesAuth() != null) { + Boolean generateInventoryNodesAuth = this.pluginGroup.getGenerateInventoryNodesAuth(); + this.context.getExecutionLogger().log( + 4, "Using plugin group configuration for generateInventoryNodesAuth: " + generateInventoryNodesAuth + ); + return generateInventoryNodesAuth; } - return generateInventoryNodesAuth; + // Default to false if not configured + return false; } /** diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index 3b12cabd..eddce682 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -70,8 +70,6 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes // Project and framework level mappings for inventory generation builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); - builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); - builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); DESC = builder.build(); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 710d1377..796b95c9 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -68,8 +68,6 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab // Project and framework level mappings for inventory generation builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); - builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); - builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); DESC = builder.build(); } From 0b6c7dd4143d72a3f205f7954ae5e366106b0ac9 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Tue, 6 Jan 2026 17:08:25 -0300 Subject: [PATCH 11/46] reverted back to node exec and project config file --- .../ansible/AnsibleRunnerContextBuilder.java | 60 ++++++++++++------- .../AnsiblePlaybookInlineWorkflowStep.java | 22 +++++++ .../plugin/AnsiblePlaybookWorkflowStep.java | 22 +++++++ .../ansible/plugin/AnsiblePluginGroup.java | 28 --------- 4 files changed, 81 insertions(+), 51 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 48183627..4d5bca7b 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -612,16 +612,6 @@ public Boolean generateInventory() { generateInventory = Boolean.parseBoolean(sgenerateInventory); } - // Fallback to plugin group configuration if not set - if (generateInventory == null || !generateInventory) { - if (this.pluginGroup != null && this.pluginGroup.getGenerateInventory() != null && this.pluginGroup.getGenerateInventory()) { - this.context.getExecutionLogger().log( - 4, "plugin group set getGenerateInventory: " + this.pluginGroup.getGenerateInventory() - ); - generateInventory = this.pluginGroup.getGenerateInventory(); - } - } - return generateInventory; } @@ -798,14 +788,14 @@ public void cleanupTempFiles() { System.err.println("DEBUG: Execution-specific directory exists: " + executionSpecificDir.getAbsolutePath()); } if (!getDebug()) { - System.err.println("DEBUG: Deleting execution-specific directory recursively"); deleteDirectoryRecursively(executionSpecificDir); - System.err.println("DEBUG: Execution-specific directory deleted"); } else { System.err.println("DEBUG: Skipping cleanup due to debug mode"); } } else { - System.err.println("DEBUG: No execution-specific directory to cleanup"); + if (getDebug()) { + System.err.println("DEBUG: No execution-specific directory to cleanup"); + } } } @@ -1018,17 +1008,39 @@ public List getListNodesKeyPath(){ public Boolean generateInventoryNodesAuth() { - // Only use plugin group configuration - if (this.pluginGroup != null && this.pluginGroup.getGenerateInventoryNodesAuth() != null) { - Boolean generateInventoryNodesAuth = this.pluginGroup.getGenerateInventoryNodesAuth(); - this.context.getExecutionLogger().log( - 4, "Using plugin group configuration for generateInventoryNodesAuth: " + generateInventoryNodesAuth - ); - return generateInventoryNodesAuth; + Boolean generateInventoryNodesAuth = null; + + if(getDebug()) { + System.err.println("DEBUG: Resolving property ANSIBLE_GENERATE_INVENTORY_NODES_AUTH"); + System.err.println("DEBUG: Property key: " + AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + System.err.println("DEBUG: Framework project: " + getFrameworkProject()); } - // Default to false if not configured - return false; + String sgenerateInventoryNodesAuth = PropertyResolver.resolveProperty( + AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, + null, + getFrameworkProject(), + getFramework(), + getNode(), + getJobConf() + ); + + if(getDebug()) { + System.err.println("DEBUG: PropertyResolver returned: " + sgenerateInventoryNodesAuth); + } + + if (null != sgenerateInventoryNodesAuth) { + generateInventoryNodesAuth = Boolean.parseBoolean(sgenerateInventoryNodesAuth); + if(getDebug()) { + System.err.println("DEBUG: Parsed to boolean: " + generateInventoryNodesAuth); + } + } else { + if(getDebug()) { + System.err.println("DEBUG: Property not found, returning null"); + } + } + + return generateInventoryNodesAuth; } /** @@ -1068,7 +1080,9 @@ private String getExecutionSpecificTmpDir() { } executionSpecificDir.mkdirs(); } else { - System.err.println("DEBUG: Execution-specific directory already exists: " + executionSpecificDir.getAbsolutePath()); + if (getDebug()) { + System.err.println("DEBUG: Execution-specific directory already exists: " + executionSpecificDir.getAbsolutePath()); + } } return executionSpecificDir.getAbsolutePath(); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index eddce682..a5e64e19 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -70,6 +70,8 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes // Project and framework level mappings for inventory generation builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); + builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); DESC = builder.build(); } @@ -107,11 +109,31 @@ public void executeStep(PluginStepContext context, Map configura runner = AnsibleRunner.buildAnsibleRunner(contextBuilder); Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); + + boolean isDebug = context.getDataContext().get("job").get("loglevel").equals("DEBUG"); + + if(isDebug) { + System.err.println("DEBUG: generateInventoryNodesAuth returned: " + generateInventoryNodeAuth); + } + if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ + if(isDebug) { + System.err.println("DEBUG: Node auth is enabled, getting authentication map"); + } Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); + if(isDebug) { + System.err.println("DEBUG: Retrieved " + (nodesAuth != null ? nodesAuth.size() : 0) + " node authentications"); + } if (nodesAuth != null && !nodesAuth.isEmpty()) { runner.setAddNodeAuthToInventory(true); runner.setNodesAuthentication(nodesAuth); + if(isDebug) { + System.err.println("DEBUG: Set node authentication on runner"); + } + } + } else { + if(isDebug) { + System.err.println("DEBUG: Node auth is NOT enabled"); } } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 796b95c9..596c1734 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -68,6 +68,8 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab // Project and framework level mappings for inventory generation builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); + builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); DESC = builder.build(); } @@ -104,11 +106,31 @@ public void executeStep(PluginStepContext context, Map configura runner = AnsibleRunner.buildAnsibleRunner(contextBuilder); Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); + + boolean isDebug = context.getDataContext().get("job").get("loglevel").equals("DEBUG"); + + if(isDebug) { + System.err.println("DEBUG: generateInventoryNodesAuth returned: " + generateInventoryNodeAuth); + } + if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ + if(isDebug) { + System.err.println("DEBUG: Node auth is enabled, getting authentication map"); + } Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); + if(isDebug) { + System.err.println("DEBUG: Retrieved " + (nodesAuth != null ? nodesAuth.size() : 0) + " node authentications"); + } if (nodesAuth != null && !nodesAuth.isEmpty()) { runner.setAddNodeAuthToInventory(true); runner.setNodesAuthentication(nodesAuth); + if(isDebug) { + System.err.println("DEBUG: Set node authentication on runner"); + } + } + } else { + if(isDebug) { + System.err.println("DEBUG: Node auth is NOT enabled"); } } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java index 064092fa..743665f0 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePluginGroup.java @@ -39,22 +39,6 @@ public void setEncryptExtraVars(Boolean encryptExtraVars) { this.encryptExtraVars = encryptExtraVars; } - public Boolean getGenerateInventory() { - return generateInventory; - } - - public void setGenerateInventory(Boolean generateInventory) { - this.generateInventory = generateInventory; - } - - public Boolean getGenerateInventoryNodesAuth() { - return generateInventoryNodesAuth; - } - - public void setGenerateInventoryNodesAuth(Boolean generateInventoryNodesAuth) { - this.generateInventoryNodesAuth = generateInventoryNodesAuth; - } - @PluginProperty( title = "Ansible config file path", description = "Set ansible config file path." @@ -73,18 +57,6 @@ public void setGenerateInventoryNodesAuth(Boolean generateInventoryNodesAuth) { ) Boolean encryptExtraVars; - @PluginProperty( - title = "Generate inventory", - description = "Generate an Ansible inventory from the targeted nodes." - ) - Boolean generateInventory; - - @PluginProperty( - title = "Generate inventory with node authentication", - description = "Add node-specific authentication credentials to the generated inventory via group_vars." - ) - Boolean generateInventoryNodesAuth; - @Override public Description getDescription() { DescriptionBuilder builder = DescriptionBuilder.builder(); From 736f0d69a3ee267f1e9b806def8f94a4d3e0140d Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Wed, 7 Jan 2026 12:55:27 -0300 Subject: [PATCH 12/46] remove flag generate inventory at workflow step level --- .../ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java | 7 ------- .../ansible/plugin/AnsiblePlaybookWorkflowStep.java | 6 ------ 2 files changed, 13 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index a5e64e19..e5e626c0 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -48,7 +48,6 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); builder.property(INVENTORY_INLINE_PROP); - builder.property(GENERATE_INVENTORY_PROP); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); @@ -67,12 +66,6 @@ public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDes builder.property(BECOME_PASSWORD_STORAGE_PROP); builder.property(DISABLE_LIMIT_PROP); - // Project and framework level mappings for inventory generation - builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); - builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); - builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); - builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); - DESC = builder.build(); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 596c1734..061dff28 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -46,7 +46,6 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); builder.property(INVENTORY_INLINE_PROP); - builder.property(GENERATE_INVENTORY_PROP); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); @@ -65,11 +64,6 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.property(BECOME_PASSWORD_STORAGE_PROP); builder.property(DISABLE_LIMIT_PROP); - // Project and framework level mappings for inventory generation - builder.mapping(ANSIBLE_GENERATE_INVENTORY, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); - builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY); - builder.mapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, PROJ_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); - builder.frameworkMapping(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, FWK_PROP_PREFIX + ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); DESC = builder.build(); } From 5ab995440c7933a11b96db1e89794ae3c0f161c8 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Wed, 7 Jan 2026 12:57:53 -0300 Subject: [PATCH 13/46] clean messages --- .../ansible/ansible/AnsibleRunnerContextBuilder.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 4d5bca7b..2c6850f3 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -777,8 +777,8 @@ public void cleanupTempFiles() { for (File temp : tempFiles) { if (!getDebug()) { System.err.println("DEBUG: Deleting temp file: " + temp.getAbsolutePath()); - temp.delete(); } + temp.delete(); } tempFiles.clear(); @@ -787,15 +787,7 @@ public void cleanupTempFiles() { if (getDebug()) { System.err.println("DEBUG: Execution-specific directory exists: " + executionSpecificDir.getAbsolutePath()); } - if (!getDebug()) { - deleteDirectoryRecursively(executionSpecificDir); - } else { - System.err.println("DEBUG: Skipping cleanup due to debug mode"); - } - } else { - if (getDebug()) { - System.err.println("DEBUG: No execution-specific directory to cleanup"); - } + deleteDirectoryRecursively(executionSpecificDir); } } From 0a77dab9e53a29b809ee228dc982620e927c4d64 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Wed, 7 Jan 2026 19:01:16 -0300 Subject: [PATCH 14/46] fix debug message --- .../plugins/ansible/ansible/AnsibleRunnerContextBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 2c6850f3..83caae7f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -775,7 +775,7 @@ public INodeEntry getNode() { public void cleanupTempFiles() { // Clean up individual temp files for (File temp : tempFiles) { - if (!getDebug()) { + if (getDebug()) { System.err.println("DEBUG: Deleting temp file: " + temp.getAbsolutePath()); } temp.delete(); From 9fe662a33ab6201c8ffc42be733660239e0f148f Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 8 Jan 2026 10:57:44 -0300 Subject: [PATCH 15/46] Added unit tests --- .../ansible/ansible/AnsibleRunner.java | 20 +-- .../ansible/AnsibleRunnerContextBuilder.java | 2 +- .../ansible/ansible/AnsibleRunnerSpec.groovy | 165 ++++++++++++++++++ 3 files changed, 176 insertions(+), 11 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 0fb6bc43..16f7beee 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -884,7 +884,7 @@ public boolean registerKeySshAgent(String keyPath) throws Exception { * @param hostUsers Map of host names to usernames * @return YAML content string ready to be written to all.yaml */ - private String buildGroupVarsYaml(Map hostPasswords, Map hostUsers) { + String buildGroupVarsYaml(Map hostPasswords, Map hostUsers) { StringBuilder yamlContent = new StringBuilder(); yamlContent.append("host_passwords:\n"); @@ -950,13 +950,13 @@ private String buildGroupVarsYaml(Map hostPasswords, Map'\"%@`].*") || + String escapeYamlKey(String key) { + // YAML special characters that require quoting (including backslash) + if (key.matches(".*[:\\[\\]{}#&*!|>'\"%@`\\\\].*") || key.startsWith("-") || key.startsWith("?") || key.matches("^[0-9].*")) { - // Escape any existing quotes and wrap in quotes + // Escape any existing backslashes and quotes, then wrap in quotes return "\"" + key.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; } return key; @@ -969,13 +969,13 @@ private String escapeYamlKey(String key) { * @param value The value to escape * @return Escaped value safe for YAML */ - private String escapeYamlValue(String value) { - // Check if value needs quoting - if (value.matches(".*[:\\[\\]{}#&*!|>'\"%@`].*") || + String escapeYamlValue(String value) { + // Check if value needs quoting (including backslash) + if (value.matches(".*[:\\[\\]{}#&*!|>'\"%@`\\\\].*") || value.startsWith("-") || value.startsWith("?") || value.trim().isEmpty()) { - // Escape any existing quotes and wrap in quotes + // Escape any existing backslashes and quotes, then wrap in quotes return "\"" + value.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; } return value; @@ -988,7 +988,7 @@ private String escapeYamlValue(String value) { * @param vaultValue The vault value to validate * @return true if valid vault format, false otherwise */ - private boolean isValidVaultFormat(String vaultValue) { + boolean isValidVaultFormat(String vaultValue) { if (vaultValue == null || vaultValue.trim().isEmpty()) { return false; } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 83caae7f..d640b8ac 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -1042,7 +1042,7 @@ public Boolean generateInventoryNodesAuth() { * * @return The path to the execution-specific directory */ - private String getExecutionSpecificTmpDir() { + String getExecutionSpecificTmpDir() { // Return cached directory if already created if (executionSpecificDir != null) { System.err.println("DEBUG: Using cached execution-specific directory: " + executionSpecificDir.getAbsolutePath()); diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index fd25854d..9ffec5f1 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -597,4 +597,169 @@ class AnsibleRunnerSpec extends Specification{ } + def "escapeYamlKey: should quote keys with special characters"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + expect: + runner.escapeYamlKey(key) == expectedResult + + where: + key || expectedResult + "simple-key" || "simple-key" + "key:with:colons" || "\"key:with:colons\"" + "key[brackets]" || "\"key[brackets]\"" + "key{braces}" || "\"key{braces}\"" + "key#hash" || "\"key#hash\"" + "key&ersand" || "\"key&ersand\"" + "key*asterisk" || "\"key*asterisk\"" + "key!exclamation" || "\"key!exclamation\"" + "key|pipe" || "\"key|pipe\"" + "-starts-with-dash" || "\"-starts-with-dash\"" + "?starts-with-q" || "\"?starts-with-q\"" + "123numeric" || "\"123numeric\"" + "key with spaces" || "key with spaces" // spaces are handled differently + } + + def "escapeYamlValue: should quote values with special characters"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + expect: + runner.escapeYamlValue(value) == expectedResult + + where: + value || expectedResult + "simple-value" || "simple-value" + "value:colon" || "\"value:colon\"" + "value[bracket]" || "\"value[bracket]\"" + " " || "\" \"" // empty/whitespace should be quoted + "value@at" || "\"value@at\"" + "-starts-dash" || "\"-starts-dash\"" + } + + def "escapeYamlKey: should escape backslashes and quotes"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + expect: + runner.escapeYamlKey('key"with"quotes') == '"key\\"with\\"quotes"' + runner.escapeYamlKey('key\\backslash') == '"key\\\\backslash"' + } + + def "isValidVaultFormat: should validate vault format correctly"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + expect: + runner.isValidVaultFormat(vaultValue) == expectedResult + + where: + vaultValue || expectedResult + "!vault |\n encryptedcontent" || true + "!vault |\n line1\n line2" || true + "!vault\n content" || true // without pipe + "not a vault" || false + null || false + "" || false + "!vault" || true // minimal valid + "!vault |\n" || false // no content + } + + def "buildGroupVarsYaml: should create valid YAML with host passwords and users"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + def hostPasswords = [ + "web-server-1": "!vault |\n encrypted1", + "db-server-1": "!vault |\n encrypted2" + ] + def hostUsers = [ + "web-server-1": "webadmin", + "db-server-1": "dbadmin" + ] + + when: + def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers) + + then: + yaml.contains("host_passwords:") + yaml.contains("web-server-1: !vault |") + yaml.contains(" encrypted1") + yaml.contains("db-server-1: !vault |") + yaml.contains(" encrypted2") + yaml.contains("host_users:") + yaml.contains("web-server-1: webadmin") + yaml.contains("db-server-1: dbadmin") + } + + def "buildGroupVarsYaml: should escape special characters in node names"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + def hostPasswords = [ + "web:server:1": "!vault |\n encrypted" + ] + def hostUsers = [ + "web:server:1": "admin" + ] + + when: + def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers) + + then: + yaml.contains('"web:server:1": !vault |') + yaml.contains('"web:server:1": admin') + } + + def "buildGroupVarsYaml: should throw exception for invalid vault format"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + def hostPasswords = [ + "web-server-1": "not-a-vault-value" + ] + def hostUsers = [ + "web-server-1": "admin" + ] + + when: + runner.buildGroupVarsYaml(hostPasswords, hostUsers) + + then: + def e = thrown(RuntimeException) + e.message.contains("Invalid vault format for host: web-server-1") + } + + def "buildGroupVarsYaml: should handle empty host lists"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + def hostPasswords = [:] + def hostUsers = [:] + + when: + def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers) + + then: + yaml.contains("host_passwords:") + yaml.contains("host_users:") + } + } From 22d8b74ada07ae5d78b2b4dac4372df4a3ecec86 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Thu, 8 Jan 2026 12:14:45 -0300 Subject: [PATCH 16/46] fix issue with password with especial character --- .../ansible/ansible/AnsibleRunner.java | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 16f7beee..c9f068cf 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -552,7 +552,11 @@ public int run() throws Exception { } if (sshUsePassword) { - String extraVarsPassword = "ansible_password: " + sshPass; + // Escape special characters in the password to prevent YAML parsing errors + String escapedPassword = escapePasswordForYaml(sshPass); + + // Wrap in quotes to preserve special characters + String extraVarsPassword = "ansible_password: \"" + escapedPassword + "\""; String finalextraVarsPassword = extraVarsPassword; if(useAnsibleVault){ @@ -571,7 +575,11 @@ public int run() throws Exception { procArgs.add("--become"); if (becomePassword != null && !becomePassword.isEmpty()) { - String extraVarsPassword = "ansible_become_password: " + becomePassword; + // Escape special characters in the password to prevent YAML parsing errors + String escapedPassword = escapePasswordForYaml(becomePassword); + + // Wrap in quotes to preserve special characters + String extraVarsPassword = "ansible_become_password: \"" + escapedPassword + "\""; String finalextraVarsPassword = extraVarsPassword; if (useAnsibleVault) { @@ -981,6 +989,22 @@ String escapeYamlValue(String value) { return value; } + /** + * Escapes a password value for safe use in YAML. + * Always wraps the password in quotes and escapes special characters to prevent YAML parsing errors. + * + * @param password The password to escape + * @return Escaped and quoted password safe for YAML + */ + String escapePasswordForYaml(String password) { + // Escape special characters in the password to prevent YAML parsing errors + return password + .replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace("\n", "\\n") + .replace("\r", "\\r"); + } + /** * Validates that a string is in proper Ansible vault format. * Expected format: "!vault |\n" followed by vault-encrypted content From d6fd35a6f638e028cb6b014ddd57726d9ed253a6 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 8 Jan 2026 13:01:49 -0300 Subject: [PATCH 17/46] Added unit tests for passwords with special characters --- .../ansible/ansible/AnsibleRunnerSpec.groovy | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index 9ffec5f1..9214adec 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -762,4 +762,244 @@ class AnsibleRunnerSpec extends Specification{ yaml.contains("host_users:") } + def "escapePasswordForYaml: should escape special characters"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + expect: + runner.escapePasswordForYaml(password) == expectedResult + + where: + password || expectedResult + "simple" || "simple" + "p@ssword" || "p@ssword" + "pass:word" || "pass:word" + "pass#word" || "pass#word" + 'pass"word' || 'pass\\"word' + 'pass\\word' || 'pass\\\\word' + 'pass\nword' || 'pass\\nword' + 'pass\rword' || 'pass\\rword' + 'pass"\\word' || 'pass\\"\\\\word' + "p@ss:w#rd!" || "p@ss:w#rd!" + '"\\' || '\\"\\\\' + "" || "" + } + + def "escapePasswordForYaml: should handle complex passwords"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + when: + def result = runner.escapePasswordForYaml('My"P@ss\\word\n2024') + + then: + result == 'My\\"P@ss\\\\word\\n2024' + } + + def "ssh password with special characters should be escaped and quoted"() { + given: + String playbook = "test" + String password = 'p@ss"word:123' + + def runnerBuilder = AnsibleRunner.playbookInline(playbook) + runnerBuilder.sshPass(password) + runnerBuilder.sshUser("user") + runnerBuilder.sshUsePassword(true) + + def process = Mock(Process) { + waitFor() >> 0 + getInputStream() >> new ByteArrayInputStream("".getBytes()) + getOutputStream() >> new ByteArrayOutputStream() + getErrorStream() >> new ByteArrayInputStream("".getBytes()) + } + + def processExecutor = Mock(ProcessExecutor) { + run() >> process + } + + List capturedArgs = null + ProcessExecutor.ProcessExecutorBuilder processBuilder = Mock(ProcessExecutor.ProcessExecutorBuilder) + + // Configure fluent builder responses + processBuilder.build() >> processExecutor + processBuilder.procArgs(_ as List) >> { List args -> + capturedArgs = new ArrayList(args) + return processBuilder + } + processBuilder.environmentVariables(_ as Map) >> { Map e -> return processBuilder } + processBuilder.baseDirectory(_ as File) >> { File f -> return processBuilder } + processBuilder.stdinVariables(_ as List) >> { List v -> return processBuilder } + processBuilder.promptStdinLogFile(_ as File) >> { File f -> return processBuilder } + processBuilder.debug(_ as boolean) >> { boolean d -> return processBuilder } + + def ansibleVault = Mock(AnsibleVault) { + checkAnsibleVault() >> true + getVaultPasswordScriptFile() >> new File("vault-script-client.py") + } + + runnerBuilder.processExecutorBuilder(processBuilder) + runnerBuilder.ansibleVault(ansibleVault) + + when: + AnsibleRunner runner = runnerBuilder.build() + runner.setCustomTmpDirPath("/tmp") + def result = runner.run() + + then: + result == 0 + 1 * ansibleVault.encryptVariable(_, _) >> "!vault | value" + capturedArgs != null + // Flatten and verify --extra-vars argument exists + def flatArgs = capturedArgs.flatten().collect { it?.toString() } + flatArgs.any { it.contains("--extra-vars") } + } + + def "become password with special characters should be escaped and quoted"() { + given: + String playbook = "test" + String becomePass = 'admin"pass\\123' + + def runnerBuilder = AnsibleRunner.playbookInline(playbook) + runnerBuilder.become(true) + runnerBuilder.becomeUser("root") + runnerBuilder.becomePassword(becomePass) + runnerBuilder.becomeMethod("sudo") + + def process = Mock(Process) { + waitFor() >> 0 + getInputStream() >> new ByteArrayInputStream("".getBytes()) + getOutputStream() >> new ByteArrayOutputStream() + getErrorStream() >> new ByteArrayInputStream("".getBytes()) + } + + def processExecutor = Mock(ProcessExecutor) { + run() >> process + } + + List capturedArgs = null + ProcessExecutor.ProcessExecutorBuilder processBuilder = Mock(ProcessExecutor.ProcessExecutorBuilder) + + // Configure fluent builder responses + processBuilder.build() >> processExecutor + processBuilder.procArgs(_ as List) >> { List args -> + capturedArgs = new ArrayList(args) + return processBuilder + } + processBuilder.environmentVariables(_ as Map) >> { Map e -> return processBuilder } + processBuilder.baseDirectory(_ as File) >> { File f -> return processBuilder } + processBuilder.stdinVariables(_ as List) >> { List v -> return processBuilder } + processBuilder.promptStdinLogFile(_ as File) >> { File f -> return processBuilder } + processBuilder.debug(_ as boolean) >> { boolean d -> return processBuilder } + + def ansibleVault = Mock(AnsibleVault) { + checkAnsibleVault() >> true + getVaultPasswordScriptFile() >> new File("vault-script-client.py") + } + + runnerBuilder.processExecutorBuilder(processBuilder) + runnerBuilder.ansibleVault(ansibleVault) + + when: + AnsibleRunner runner = runnerBuilder.build() + runner.setCustomTmpDirPath("/tmp") + def result = runner.run() + + then: + result == 0 + 1 * ansibleVault.encryptVariable(_, _) >> "!vault | value" + capturedArgs != null + // Flatten and verify arguments + def flatArgs = capturedArgs.flatten().collect { it?.toString() } + flatArgs.contains("--become") + flatArgs.any { it.contains("--extra-vars") } + } + + def "escapePasswordForYaml: should preserve passwords without special chars"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + when: + def result = runner.escapePasswordForYaml("simplePassword123") + + then: + result == "simplePassword123" + } + + def "escapePasswordForYaml: should handle multiple escape sequences"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + when: + // Password with quote, backslash, and newline + def result = runner.escapePasswordForYaml('test"\\pass\n') + + then: + // Each special char should be escaped + result == 'test\\"\\\\pass\\n' + } + + def "both ssh and become passwords with special chars should work together"() { + given: + String playbook = "test" + String sshPass = 'ssh"pass:123' + String becomePass = 'sudo\\pass#456' + + def runnerBuilder = AnsibleRunner.playbookInline(playbook) + runnerBuilder.sshPass(sshPass) + runnerBuilder.sshUser("user") + runnerBuilder.sshUsePassword(true) + runnerBuilder.become(true) + runnerBuilder.becomeUser("root") + runnerBuilder.becomePassword(becomePass) + runnerBuilder.becomeMethod("sudo") + + def process = Mock(Process) { + waitFor() >> 0 + getInputStream() >> new ByteArrayInputStream("".getBytes()) + getOutputStream() >> new ByteArrayOutputStream() + getErrorStream() >> new ByteArrayInputStream("".getBytes()) + } + + def processExecutor = Mock(ProcessExecutor) { + run() >> process + } + + ProcessExecutor.ProcessExecutorBuilder processBuilder = Mock(ProcessExecutor.ProcessExecutorBuilder) + + // Configure fluent builder responses + processBuilder.build() >> processExecutor + processBuilder.procArgs(_ as List) >> { List args -> return processBuilder } + processBuilder.environmentVariables(_ as Map) >> { Map e -> return processBuilder } + processBuilder.baseDirectory(_ as File) >> { File f -> return processBuilder } + processBuilder.stdinVariables(_ as List) >> { List v -> return processBuilder } + processBuilder.promptStdinLogFile(_ as File) >> { File f -> return processBuilder } + processBuilder.debug(_ as boolean) >> { boolean d -> return processBuilder } + + def ansibleVault = Mock(AnsibleVault) { + checkAnsibleVault() >> true + getVaultPasswordScriptFile() >> new File("vault-script-client.py") + } + + runnerBuilder.processExecutorBuilder(processBuilder) + runnerBuilder.ansibleVault(ansibleVault) + + when: + AnsibleRunner runner = runnerBuilder.build() + runner.setCustomTmpDirPath("/tmp") + def result = runner.run() + + then: + result == 0 + // Both passwords should be encrypted + 2 * ansibleVault.encryptVariable(_, _) >> "!vault | value" + } + } From 3298d3bd4f7c4c4abfd00069dea9adda851bd9e4 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Thu, 8 Jan 2026 13:37:03 -0300 Subject: [PATCH 18/46] clean AnsiblePlaybookInlineWorkflowNodeStep --- .../plugin/AnsiblePlaybookInlineWorkflowNodeStep.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java index 5827ccc6..a9a1d40c 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java @@ -43,8 +43,6 @@ public class AnsiblePlaybookInlineWorkflowNodeStep implements NodeStepPlugin, An builder.property(PLAYBOOK_INLINE_PROP); builder.property(EXTRA_VARS_PROP); builder.property(CONFIG_ENCRYPT_EXTRA_VARS); - builder.property(GENERATE_INVENTORY_PROP); - builder.property(GENERATE_INVENTORY_NODES_AUTH); builder.property(VAULT_KEY_FILE_PROP); builder.property(VAULT_KEY_STORAGE_PROP); builder.property(EXTRA_ATTRS_PROP); @@ -123,13 +121,7 @@ public void executeNodeStep( @Override public List listSecretsPathWorkflowNodeStep(ExecutionContext context, INodeEntry node, Map configuration) { AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(node, context, context.getFramework(), configuration); - List secretPaths = AnsibleUtil.getSecretsPath(builder); - List secretPathsNodes = builder.getListNodesKeyPath(); - - if(secretPathsNodes != null && !secretPathsNodes.isEmpty()){ - secretPaths.addAll(secretPathsNodes); - } - return secretPaths; + return AnsibleUtil.getSecretsPath(builder); } @Override From b514ed2287c997e549de4c012764ec806195c18f Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 8 Jan 2026 23:13:48 -0300 Subject: [PATCH 19/46] Added Multiple node auth functional tests, added missing README files --- functional-test/README.md | 181 ++++++++++++++++++ functional-test/build.gradle | 9 +- .../functional/MultiNodeAuthSpec.groovy | 173 +++++++++++++++++ .../docker/ansible-multi-node-auth/README.md | 57 ++++++ .../ansible-multi-node-auth/ansible.cfg | 8 + .../ansible-multi-node-auth/inventory.ini | 7 + .../ansible-multi-node-auth/resources.xml | 41 ++++ .../ansible-multi-node-auth/test-playbook.yml | 39 ++++ .../test/resources/docker/docker-compose.yml | 25 +++ .../files/acls/node-acl.aclpolicy | 16 ++ .../files/etc/project.properties | 28 +++ .../jobs/job-ansible-playbook-test.xml | 35 ++++ .../jobs/job-multi-node-ping.xml | 36 ++++ 13 files changed, 654 insertions(+), 1 deletion(-) create mode 100644 functional-test/README.md create mode 100644 functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy create mode 100644 functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md create mode 100644 functional-test/src/test/resources/docker/ansible-multi-node-auth/ansible.cfg create mode 100644 functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini create mode 100644 functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml create mode 100644 functional-test/src/test/resources/docker/ansible-multi-node-auth/test-playbook.yml create mode 100644 functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/acls/node-acl.aclpolicy create mode 100644 functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/etc/project.properties create mode 100644 functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-ansible-playbook-test.xml create mode 100644 functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml diff --git a/functional-test/README.md b/functional-test/README.md new file mode 100644 index 00000000..8cdb771c --- /dev/null +++ b/functional-test/README.md @@ -0,0 +1,181 @@ +# Functional Tests for Ansible Plugin + +This directory contains functional tests for the Rundeck Ansible plugin using Testcontainers and Docker. + +## Prerequisites + +- Docker (Docker Desktop or Rancher Desktop) +- Java 11 or later +- Gradle 7.2 or later + +## Docker Configuration + +The functional tests use Testcontainers to spin up Docker containers for Rundeck and SSH nodes. Depending on your Docker setup, you may need to configure the Docker socket path. + +### Option 1: Using ~/.testcontainers.properties (Recommended) + +Create a file at `~/.testcontainers.properties` with the following content: + +**For Rancher Desktop on macOS:** +```properties +docker.host=unix:///Users//.rd/docker.sock +``` + +**For Docker Desktop on macOS/Linux:** +```properties +docker.host=unix:///var/run/docker.sock +``` + +**For Windows with Docker Desktop:** +```properties +docker.host=tcp://localhost:2375 +``` + +### Option 2: Modify build.gradle + +Edit `functional-test/build.gradle` and update the `docker.host` paths on lines 52-53: + +```gradle +systemProperty('docker.host', "unix:///path/to/your/docker.sock") +environment 'DOCKER_HOST', 'unix:///path/to/your/docker.sock' +``` + +## Running the Tests + +### Run All Functional Tests + +```bash +./gradlew :functional-test:functionalTest +``` + +### Run Specific Test Suite + +```bash +# Multi-node authentication tests +./gradlew :functional-test:functionalTest --tests "*MultiNodeAuthSpec*" + +# Basic integration tests +./gradlew :functional-test:functionalTest --tests "*BasicIntegrationSpec*" + +# Plugin group tests +./gradlew :functional-test:functionalTest --tests "*PluginGroupIntegrationSpec*" +``` + +## Test Suites + +### MultiNodeAuthSpec +Tests the multi-node authentication feature where each node can have its own password stored in Rundeck's key storage. + +**Tests:** +- `test ansible playbook with multi-node authentication` - Verifies Ansible playbooks execute across multiple nodes with per-node credentials +- `test multi-node authentication with different passwords` - Tests script execution on multiple nodes with different passwords +- `test nodes are accessible with different credentials` - Validates node registration and discovery +- `test passwords with special characters are properly escaped` - Verifies YAML escaping for special characters in passwords + +**Test Environment:** +- 3 SSH nodes (ssh-node, ssh-node-2, ssh-node-3) +- Each node has a different password, including special characters +- Tests both WorkflowStep (Ansible playbooks) and NodeStep (scripts) execution + +### BasicIntegrationSpec +Basic integration tests for the Ansible plugin functionality. + +### PluginGroupIntegrationSpec +Tests for plugin group configuration and execution. + +## Test Structure + +``` +functional-test/ +├── build.gradle # Test configuration +├── README.md # This file +└── src/ + └── test/ + ├── groovy/functional/ # Test specifications + │ ├── MultiNodeAuthSpec.groovy + │ ├── BasicIntegrationSpec.groovy + │ └── PluginGroupIntegrationSpec.groovy + └── resources/ + ├── docker/ # Docker compose and configs + │ ├── docker-compose.yml + │ ├── ansible-multi-node-auth/ # Multi-node test configs + │ ├── keys/ # SSH keys for tests + │ ├── node/ # SSH node Docker configs + │ └── rundeck/ # Rundeck Docker configs + └── project-import/ # Rundeck project definitions + └── ansible-multi-node-auth/ + └── rundeck-ansible-multi-node-auth/ + ├── files/etc/project.properties + └── jobs/*.xml +``` + +## Troubleshooting + +### Tests Fail with "Could not find Docker socket" + +**Problem:** Testcontainers cannot locate the Docker socket. + +**Solution:** +1. Verify Docker is running: `docker ps` +2. Check your Docker socket path: + - Rancher Desktop: `ls -la ~/.rd/docker.sock` + - Docker Desktop: `ls -la /var/run/docker.sock` +3. Update `~/.testcontainers.properties` or `build.gradle` with the correct path + +### Tests Timeout or Hang + +**Problem:** Tests take too long or appear to hang. + +**Solution:** +1. Check Docker container status: `docker ps -a` +2. Check Docker logs: `docker logs ` +3. Increase test timeout in build.gradle if needed +4. Ensure sufficient Docker resources (memory/CPU) + +### Port Conflicts + +**Problem:** Tests fail with "port already in use" errors. + +**Solution:** +1. Check for running containers: `docker ps` +2. Stop conflicting containers: `docker stop ` +3. Clean up: `docker-compose down` in the docker directory + +### Platform Mismatch Warnings (Apple Silicon) + +**Problem:** Warnings about platform mismatch (linux/amd64 vs linux/arm64). + +**Solution:** These warnings are expected on Apple Silicon Macs and can be safely ignored. Docker will use Rosetta 2 for emulation. + +## Test Reports + +After running tests, view the HTML report at: +``` +functional-test/build/reports/tests/functionalTest/index.html +``` + +## Adding New Tests + +1. Create a new Spock specification in `src/test/groovy/functional/` +2. Extend `BaseTestConfiguration` for common test utilities +3. Add any required Docker configs to `src/test/resources/docker/` +4. Add project imports to `src/test/resources/project-import/` +5. Follow existing test patterns for consistency + +## Multi-Node Authentication Feature + +The multi-node authentication feature allows running Ansible playbooks across multiple nodes where each node has its own password stored in Rundeck's key storage. + +**How it works:** +1. Enable at project level: `project.ansible-generate-inventory-nodes-auth=true` +2. Store per-node passwords in Key Storage with paths specified in node attributes +3. Plugin generates `group_vars/all.yaml` with vault-encrypted passwords +4. Ansible uses host-specific credentials from group_vars + +**Requirements:** +- Must use Ansible Playbook **Workflow Steps** (not Node Steps) +- Node attributes must include `ansible-ssh-password-storage-path` for each node +- Passwords are automatically encrypted using Ansible Vault +- Supports special characters with proper YAML escaping + +See `MultiNodeAuthSpec.groovy` for comprehensive test examples. diff --git a/functional-test/build.gradle b/functional-test/build.gradle index 681b7af5..188c4e88 100644 --- a/functional-test/build.gradle +++ b/functional-test/build.gradle @@ -42,7 +42,14 @@ dependencies { tasks.register('functionalTest', Test) { useJUnitPlatform() - systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.1.1") + systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.18.0") + + // Docker configuration for Testcontainers + // For Rancher Desktop on macOS: Use /Users//.rd/docker.sock + // For Docker Desktop on macOS: Use /var/run/docker.sock + // For Linux: Use /var/run/docker.sock + // You can also configure this via ~/.testcontainers.properties (see functional-test/README.md) + description = "Run Ansible integration tests" } diff --git a/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy b/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy new file mode 100644 index 00000000..94c0d181 --- /dev/null +++ b/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy @@ -0,0 +1,173 @@ +package functional + +import functional.base.BaseTestConfiguration +import functional.util.TestUtil +import org.rundeck.client.api.model.JobRun +import org.testcontainers.spock.Testcontainers + +@Testcontainers +class MultiNodeAuthSpec extends BaseTestConfiguration { + + static String PROJ_NAME = 'ansible-multi-node-auth' + static String NODE1_NAME = "ssh-node" + static String NODE2_NAME = "ssh-node-2" + static String NODE3_NAME = "ssh-node-3" + + // Test passwords for each node (matching docker-compose.yml) + static String NODE1_PASSWORD = "testpassword123" + static String NODE2_PASSWORD = "password2_special!@#" + static String NODE3_PASSWORD = 'password3"quote\'test' + + def setupSpec() { + startCompose() + configureMultiNodeAuthProject() + } + + def configureMultiNodeAuthProject() { + // Store passwords for each node in Rundeck key storage + okhttp3.RequestBody requestBody = okhttp3.RequestBody.create( + NODE1_PASSWORD.getBytes(), + org.rundeck.client.util.Client.MEDIA_TYPE_X_RUNDECK_PASSWORD + ) + client.apiCall { api -> + api.createKeyStorage("project/$PROJ_NAME/ssh-node.pass", requestBody) + } + + requestBody = okhttp3.RequestBody.create( + NODE2_PASSWORD.getBytes(), + org.rundeck.client.util.Client.MEDIA_TYPE_X_RUNDECK_PASSWORD + ) + client.apiCall { api -> + api.createKeyStorage("project/$PROJ_NAME/ssh-node-2.pass", requestBody) + } + + requestBody = okhttp3.RequestBody.create( + NODE3_PASSWORD.getBytes(), + org.rundeck.client.util.Client.MEDIA_TYPE_X_RUNDECK_PASSWORD + ) + client.apiCall { api -> + api.createKeyStorage("project/$PROJ_NAME/ssh-node-3.pass", requestBody) + } + + // Create project + def projList = client.apiCall { api -> api.listProjects() } + if (!projList*.name.contains(PROJ_NAME)) { + client.apiCall { api -> + api.createProject(new org.rundeck.client.api.model.ProjectItem(name: PROJ_NAME)) + } + } + + // Import project configuration + File projectFile = TestUtil.createArchiveJarFile( + PROJ_NAME, + new File("src/test/resources/project-import/$PROJ_NAME") + ) + okhttp3.RequestBody body = okhttp3.RequestBody.create( + projectFile, + org.rundeck.client.util.Client.MEDIA_TYPE_ZIP + ) + client.apiCall { api -> + api.importProjectArchive( + PROJ_NAME, + "preserve", + true, true, true, true, true, true, true, + [:], + body + ) + } + + // Wait for nodes to be available + waitForNodeAvailability(PROJ_NAME, NODE1_NAME) + waitForNodeAvailability(PROJ_NAME, NODE2_NAME) + waitForNodeAvailability(PROJ_NAME, NODE3_NAME) + } + + def "test multi-node authentication with different passwords"() { + when: + def jobId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" // multi-node-ping-test job + + JobRun request = new JobRun() + request.loglevel = 'INFO' + + def result = client.apiCall { api -> api.runJob(jobId, request) } + def executionId = result.id + + def executionState = waitForJob(executionId) + def logs = getLogs(executionId) + + then: + // Job succeeded - this verifies that multi-node authentication worked + executionState != null + executionState.getExecutionState() == "SUCCEEDED" + + // Verify all three nodes were in the execution context + executionState.targetNodes.contains("ssh-node") + executionState.targetNodes.contains("ssh-node-2") + executionState.targetNodes.contains("ssh-node-3") + } + + def "test ansible playbook with multi-node authentication"() { + when: + def jobId = "b2c3d4e5-f6a7-8901-bcde-f12345678901" // ansible-playbook-multi-node-test job + + JobRun request = new JobRun() + request.loglevel = 'DEBUG' + + def result = client.apiCall { api -> api.runJob(jobId, request) } + def executionId = result.id + + def executionState = waitForJob(executionId) + def logs = getLogs(executionId) + + then: + // Job succeeded - this verifies the playbook ran successfully with multi-node authentication + executionState != null + executionState.getExecutionState() == "SUCCEEDED" + + // Verify all three nodes were targeted by the playbook execution + executionState.targetNodes.contains("ssh-node") + executionState.targetNodes.contains("ssh-node-2") + executionState.targetNodes.contains("ssh-node-3") + } + + def "test nodes are accessible with different credentials"() { + when: + // List all nodes in the project + def nodes = client.apiCall { api -> api.listNodes(PROJ_NAME, ".*") } + + then: + // Verify all three nodes are registered + nodes != null + nodes.size() >= 3 + nodes.get(NODE1_NAME) != null + nodes.get(NODE2_NAME) != null + nodes.get(NODE3_NAME) != null + } + + def "test passwords with special characters are properly escaped"() { + when: + def jobId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" + + JobRun request = new JobRun() + request.loglevel = 'DEBUG' + request.filter = "name: ssh-node-2 || name: ssh-node-3" // Only test nodes with special chars in passwords + + def result = client.apiCall { api -> api.runJob(jobId, request) } + def executionId = result.id + + def executionState = waitForJob(executionId) + def logs = getLogs(executionId) + + then: + // Job succeeded even with special characters in passwords - verifies password escaping works + executionState != null + executionState.getExecutionState() == "SUCCEEDED" + + // Verify nodes with special character passwords were targeted + executionState.targetNodes.contains("ssh-node-2") + executionState.targetNodes.contains("ssh-node-3") + + // No YAML parsing errors - verifies special characters were properly escaped + !logs.any { it.log.toLowerCase().contains("yaml") && it.log.toLowerCase().contains("error") } + } +} diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md b/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md new file mode 100644 index 00000000..384af8bf --- /dev/null +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md @@ -0,0 +1,57 @@ +# Multi-Node Authentication Functional Test + +This test verifies the multi-node authentication feature where each node can have its own password stored in Rundeck's key storage. + +## Test Setup + +### Nodes +- **ssh-node**: Standard password (`testpassword123`) +- **ssh-node-2**: Password with special characters (`password2_special!@#`) +- **ssh-node-3**: Password with quotes (`password3"quote'test`) + +### Files +- `resources.xml`: Defines three nodes with node-specific password storage paths +- `inventory.ini`: Ansible inventory file for the test nodes +- `ansible.cfg`: Ansible configuration +- `test-playbook.yml`: Test playbook for verification + +## What's Being Tested + +1. **Multi-node authentication with different passwords per node** + - Each node has a different password stored in Rundeck key storage + - The plugin generates `group_vars/all.yaml` with vault-encrypted passwords + - Each node authenticates with its specific credentials + +2. **Password escaping for special characters** + - Node 2 has special characters: `!`, `@`, `#` + - Node 3 has quotes: `"` and `'` + - Tests verify YAML escaping works correctly + +3. **Project-level configuration** + - `project.ansible-generate-inventory-nodes-auth=true` enables the feature + - Configuration is at project level, not workflow step level + +## Jobs + +- **multi-node-ping-test**: Simple script that runs on all nodes +- **ansible-playbook-multi-node-test**: Ansible playbook that verifies authentication on each node + +## Expected Behavior + +1. Plugin reads node attributes to get password storage paths for each node +2. Plugin retrieves passwords from Rundeck key storage +3. Plugin escapes passwords to prevent YAML parsing errors +4. Plugin encrypts passwords using Ansible Vault +5. Plugin generates `group_vars/all.yaml` with: + ```yaml + host_passwords: + ssh-node: !vault | ... + ssh-node-2: !vault | ... + ssh-node-3: !vault | ... + host_users: + ssh-node: rundeck + ssh-node-2: rundeck + ssh-node-3: rundeck + ``` +6. Ansible uses host-specific credentials from group_vars +7. Each node authenticates successfully with its own password diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/ansible.cfg b/functional-test/src/test/resources/docker/ansible-multi-node-auth/ansible.cfg new file mode 100644 index 00000000..7d115df1 --- /dev/null +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/ansible.cfg @@ -0,0 +1,8 @@ +[defaults] +host_key_checking = False +timeout = 30 +gathering = smart +fact_caching = jsonfile +fact_caching_connection = /tmp/ansible_facts +fact_caching_timeout = 3600 +stdout_callback = yaml diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini b/functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini new file mode 100644 index 00000000..422d1a9a --- /dev/null +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini @@ -0,0 +1,7 @@ +[test_nodes] +ssh-node ansible_host=ssh-node ansible_port=22 +ssh-node-2 ansible_host=ssh-node-2 ansible_port=22 +ssh-node-3 ansible_host=ssh-node-3 ansible_port=22 + +[test_nodes:vars] +ansible_connection=ssh diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml b/functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml new file mode 100644 index 00000000..c38837c4 --- /dev/null +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml @@ -0,0 +1,41 @@ + + + + + + + + diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/test-playbook.yml b/functional-test/src/test/resources/docker/ansible-multi-node-auth/test-playbook.yml new file mode 100644 index 00000000..c8b26fc7 --- /dev/null +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/test-playbook.yml @@ -0,0 +1,39 @@ +--- +- name: Multi-Node Authentication Test Playbook + hosts: all + gather_facts: yes + tasks: + - name: Ping all nodes + ping: + + - name: Get hostname + command: hostname + register: node_hostname + + - name: Display connection info + debug: + msg: "Successfully connected to {{ node_hostname.stdout }} as {{ ansible_user_id }}" + + - name: Create test file + file: + path: "/tmp/multi_node_test_{{ inventory_hostname }}.txt" + state: touch + mode: '0644' + + - name: Write test content + copy: + content: "Multi-node authentication test passed on {{ inventory_hostname }}\n" + dest: "/tmp/multi_node_test_{{ inventory_hostname }}.txt" + + - name: Verify file exists + stat: + path: "/tmp/multi_node_test_{{ inventory_hostname }}.txt" + register: test_file_stat + + - name: Assert test file was created + assert: + that: + - test_file_stat.stat.exists + - test_file_stat.stat.isreg + success_msg: "Test file created successfully on {{ inventory_hostname }}" + fail_msg: "Failed to create test file on {{ inventory_hostname }}" diff --git a/functional-test/src/test/resources/docker/docker-compose.yml b/functional-test/src/test/resources/docker/docker-compose.yml index fedf4ce7..1ab6722f 100644 --- a/functional-test/src/test/resources/docker/docker-compose.yml +++ b/functional-test/src/test/resources/docker/docker-compose.yml @@ -13,6 +13,30 @@ services: volumes: - ./keys:/configuration:rw + ssh-node-2: + build: + context: node + environment: + NODE_USER_PASSWORD: password2_special!@# + networks: + - rundeck + ports: + - "2223:22" + volumes: + - ./keys:/configuration:rw + + ssh-node-3: + build: + context: node + environment: + NODE_USER_PASSWORD: password3"quote'test + networks: + - rundeck + ports: + - "2224:22" + volumes: + - ./keys:/configuration:rw + rundeck: build: context: rundeck @@ -37,6 +61,7 @@ services: - ./ansible-yaml-parsing:/home/rundeck/ansible-yaml-parsing:rw - ./ansible-child-groups:/home/rundeck/ansible-child-groups:rw - ./ansible-max-aliases:/home/rundeck/ansible-max-aliases:rw + - ./ansible-multi-node-auth:/home/rundeck/ansible-multi-node-auth:rw volumes: rundeck-data: diff --git a/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/acls/node-acl.aclpolicy b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/acls/node-acl.aclpolicy new file mode 100644 index 00000000..58f944e1 --- /dev/null +++ b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/acls/node-acl.aclpolicy @@ -0,0 +1,16 @@ +description: Node access for project +context: + project: '.*' +for: + resource: + - equals: + kind: node + allow: [read,create,update,refresh] + adhoc: + - allow: [read,run,runAs,kill,killAs] + job: + - allow: [create,read,update,delete,run,runAs,kill,killAs] + node: + - allow: [read,run] +by: + username: admin diff --git a/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/etc/project.properties b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/etc/project.properties new file mode 100644 index 00000000..95badb25 --- /dev/null +++ b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/files/etc/project.properties @@ -0,0 +1,28 @@ +#Multi-Node Authentication Test Project +project.name=ansible-multi-node-auth +project.description=Test project for multi-node authentication with per-node credentials +project.disable.executions=false +project.disable.schedule=false +project.execution.history.cleanup.enabled=false +project.jobs.gui.groupExpandLevel=1 +project.nodeCache.enabled=true +project.nodeCache.firstLoadSynch=true +project.output.allowUnsanitized=false + +# Enable multi-node authentication at project level +project.ansible-generate-inventory-nodes-auth=true + +# Ansible configuration +project.ansible-config-file-path=/home/rundeck/ansible-multi-node-auth/ansible.cfg +project.ansible-executable=/bin/bash + +# Resource source configuration - use file-based resources with node-specific attributes +resources.source.1.config.file=/home/rundeck/ansible-multi-node-auth/resources.xml +resources.source.1.config.generateFileAutomatically=false +resources.source.1.config.includeServerNode=false +resources.source.1.config.requireFileExists=true +resources.source.1.type=file + +# Default node executor and file copier +service.FileCopier.default.provider=com.batix.rundeck.plugins.AnsibleFileCopier +service.NodeExecutor.default.provider=com.batix.rundeck.plugins.AnsibleNodeExecutor diff --git a/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-ansible-playbook-test.xml b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-ansible-playbook-test.xml new file mode 100644 index 00000000..d065e6ac --- /dev/null +++ b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-ansible-playbook-test.xml @@ -0,0 +1,35 @@ + + + nodes + Test multi-node authentication using Ansible playbook + + true + false + ascending + false + 1 + + true + b2c3d4e5-f6a7-8901-bcde-f12345678901 + INFO + ansible-playbook-multi-node-test + false + + .* + + true + + true + + + + + + + + + + + b2c3d4e5-f6a7-8901-bcde-f12345678901 + + diff --git a/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml new file mode 100644 index 00000000..3a159b54 --- /dev/null +++ b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml @@ -0,0 +1,36 @@ + + + nodes + Test multi-node authentication with different passwords + + true + true + ascending + false + 3 + + true + a1b2c3d4-e5f6-7890-abcd-ef1234567890 + INFO + multi-node-ping-test + false + + name: ssh-node.* + + true + + true + + + + + + + a1b2c3d4-e5f6-7890-abcd-ef1234567890 + + From 24d84ccbf5049c2deea79eff3036370e4be1dba8 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 9 Jan 2026 18:06:06 -0300 Subject: [PATCH 20/46] remove unnecesary logs --- .../plugins/ansible/ansible/AnsibleRunner.java | 5 +---- .../ansible/ansible/AnsibleRunnerContextBuilder.java | 12 +++++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index c9f068cf..f395f67e 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -942,10 +942,7 @@ String buildGroupVarsYaml(Map hostPasswords, Map String result = yamlContent.toString(); if(debug){ - System.err.println("DEBUG: Generated YAML content (" + result.length() + " bytes):"); - System.err.println("DEBUG: ========== YAML START =========="); - System.err.println(result); - System.err.println("DEBUG: ========== YAML END =========="); + System.err.println("DEBUG: Generated YAML content (" + result.length() + " bytes)"); } return result; diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index d640b8ac..4978fbf8 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -953,10 +953,14 @@ public Map> getNodesAuthenticationMap(){ String password = getPasswordFromPath(keyPath); auth.put("ansible_password", password); } catch (ConfigurationException e) { - System.err.println("DEBUG: Error retrieving password for " + node.getNodename() + ": " + e.getMessage()); + if (getDebug()) { + System.err.println("DEBUG: Error retrieving password for " + node.getNodename() + ": " + e.getMessage()); + } throw new RuntimeException(e); } catch (Exception e2) { - System.err.println("DEBUG: Unexpected error: " + e2.getMessage()); + if (getDebug()) { + System.err.println("DEBUG: Unexpected error: " + e2.getMessage()); + } throw new RuntimeException(e2); } @@ -1045,7 +1049,9 @@ public Boolean generateInventoryNodesAuth() { String getExecutionSpecificTmpDir() { // Return cached directory if already created if (executionSpecificDir != null) { - System.err.println("DEBUG: Using cached execution-specific directory: " + executionSpecificDir.getAbsolutePath()); + if (getDebug()) { + System.err.println("DEBUG: Using cached execution-specific directory: " + executionSpecificDir.getAbsolutePath()); + } return executionSpecificDir.getAbsolutePath(); } From 35ffd42b111e396b1a7296ee18a533867b6198a6 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Tue, 13 Jan 2026 11:05:28 -0300 Subject: [PATCH 21/46] improve workflow step node authentication type --- .../ansible/ansible/AnsibleRunner.java | 46 ++++++++++++- .../ansible/AnsibleRunnerContextBuilder.java | 66 ++++++++++++------- 2 files changed, 86 insertions(+), 26 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index f395f67e..87bd2419 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -411,11 +411,32 @@ public int run() throws Exception { if(addNodeAuthToInventory != null && addNodeAuthToInventory && nodesAuthentication != null && !nodesAuthentication.isEmpty()) { + /* + Group vars file structure: + + host_users: + host1: user1 + host2: user2 + + host_passwords: + host1: enc(password1) + host2: enc(password1) + + host_private_keys: + host1: /path/to/private_key1 + host2: /path/to/private_key2 + */ + Map hostUsers = new LinkedHashMap<>(); Map hostPasswords = new LinkedHashMap<>(); + Map hostKeys = new LinkedHashMap<>(); + nodesAuthentication.forEach((nodeName, authValues) -> { String user = authValues.get("ansible_user"); String password = authValues.get("ansible_password"); + String privateKey = authValues.get("ansible_ssh_private_key"); + + if (user != null) { hostUsers.put(nodeName, user); } @@ -430,6 +451,26 @@ public int run() throws Exception { } hostPasswords.put(nodeName, encryptedPassword); } + + if(privateKey != null){ + //create temporary file for private key + File tempHostPkFile; + try { + tempHostPkFile = AnsibleUtil.createTemporaryFile("","id_rsa_node_"+nodeName, privateKey,customTmpDirPath); + + // Only the owner can read and write + Set perms = new HashSet(); + perms.add(PosixFilePermission.OWNER_READ); + perms.add(PosixFilePermission.OWNER_WRITE); + Files.setPosixFilePermissions(tempHostPkFile.toPath(), perms); + + hostKeys.put(nodeName, tempHostPkFile.getAbsolutePath()); + + tempHostPkFile.deleteOnExit(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } }); // Build YAML content using helper method @@ -490,7 +531,10 @@ public int run() throws Exception { //set extra vars to resolve the host specific authentication procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] }}\""); - procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] }}\""); + if(!hostPasswords.isEmpty()){ + procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] }}\""); + } + procArgs.add("-e ansible_ssh_private_key_file=\"{{ host_private_keys[inventory_hostname] }}\""); } catch (IOException e) { System.err.println("ERROR: Failed to write all.yaml: " + e.getMessage()); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 4978fbf8..1a03c6e6 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -101,12 +101,16 @@ public String getPrivateKeyfilePath() { } public String getPrivateKeyStoragePath() { + return getPrivateKeyStoragePath(getNode()); + } + + public String getPrivateKeyStoragePath(INodeEntry node) { String path = PropertyResolver.resolveProperty( AnsibleDescribable.ANSIBLE_SSH_KEYPATH_STORAGE_PATH, null, getFrameworkProject(), getFramework(), - getNode(), + node, getJobConf() ); //expand properties in path @@ -139,9 +143,9 @@ public String getPasswordStoragePath() { return path; } - public String getSshPrivateKey() throws ConfigurationException { + public String getSshPrivateKey(node) throws ConfigurationException { //look for storage option - String storagePath = getPrivateKeyStoragePath(); + String storagePath = getPrivateKeyStoragePath(node); if (null != storagePath) { Path path = PathUtil.asPath(storagePath); @@ -266,12 +270,17 @@ public String getSshNodeUser(INodeEntry node) { public AuthenticationType getSshAuthenticationType() { + return getSshAuthenticationType(getNode()); + } + + + public AuthenticationType getSshAuthenticationType(INodeEntry node) { String authType = PropertyResolver.resolveProperty( AnsibleDescribable.ANSIBLE_SSH_AUTH_TYPE, null, getFrameworkProject(), getFramework(), - getNode(), + node, getJobConf() ); @@ -932,39 +941,46 @@ public Map getListOptions(){ return options; } + /* + Returns a map of node names to their respective authentication details. + Each entry in the outer map corresponds to a node, with the key being the node name + and the value being another map containing authentication parameters such as + + "ansible_ssh_private_key" or "ansible_password". + [ "node1": { "ansible_ssh_private_key": "...", "ansible_user": "..." }, + "node2": { "ansible_password": "...", "ansible_user": "..." } + ] + + */ public Map> getNodesAuthenticationMap(){ Map> authenticationNodesMap = new HashMap<>(); this.context.getNodes().forEach((node) -> { - String keyPath = PropertyResolver.resolveProperty( - AnsibleDescribable.ANSIBLE_SSH_PASSWORD_STORAGE_PATH, - null, - getFrameworkProject(), - getFramework(), - node, - getJobConf() - ); - Map auth = new HashMap<>(); - - if(null!=keyPath){ + final AuthenticationType authType = getSshAuthenticationType(node); + if (AnsibleDescribable.AuthenticationType.privateKey == authType) { + final String privateKey; try { - String password = getPasswordFromPath(keyPath); - auth.put("ansible_password", password); + privateKey = getSshPrivateKey(node); } catch (ConfigurationException e) { - if (getDebug()) { - System.err.println("DEBUG: Error retrieving password for " + node.getNodename() + ": " + e.getMessage()); - } throw new RuntimeException(e); - } catch (Exception e2) { - if (getDebug()) { - System.err.println("DEBUG: Unexpected error: " + e2.getMessage()); + } + if (privateKey != null) { + auth.put("ansible_ssh_private_key", privateKey); + } + } else if (AnsibleDescribable.AuthenticationType.password == authType) { + try { + String password = getSshPassword(node); + if(null!=password){ + auth.put("ansible_password", password); } - throw new RuntimeException(e2); + } catch (ConfigurationException e) { + System.err.println("DEBUG: Error retrieving password for " + node.getNodename() + ": " + e.getMessage()); + throw new RuntimeException(e); } - } + String userName = getSshNodeUser(node); if(null!=userName){ From 80a329678c0681e4df40502c8c33963d25558399 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Tue, 13 Jan 2026 22:54:46 -0300 Subject: [PATCH 22/46] Refactored code, added private key and functoinal test --- functional-test/build.gradle | 2 +- .../functional/MultiNodeAuthSpec.groovy | 163 ++++++++++-------- .../ansible-multi-node-auth/inventory.ini | 1 + .../ansible-multi-node-auth/resources.xml | 13 ++ .../test/resources/docker/docker-compose.yml | 12 ++ .../ansible/ansible/AnsibleRunner.java | 56 ++++-- .../ansible/AnsibleRunnerContextBuilder.java | 73 ++++---- .../ansible/ansible/AnsibleRunnerSpec.groovy | 10 +- 8 files changed, 212 insertions(+), 118 deletions(-) diff --git a/functional-test/build.gradle b/functional-test/build.gradle index 188c4e88..46cd047c 100644 --- a/functional-test/build.gradle +++ b/functional-test/build.gradle @@ -42,7 +42,7 @@ dependencies { tasks.register('functionalTest', Test) { useJUnitPlatform() - systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.18.0") + systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.1.1") // Docker configuration for Testcontainers // For Rancher Desktop on macOS: Use /Users//.rd/docker.sock diff --git a/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy b/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy index 94c0d181..0b4b7b0f 100644 --- a/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy +++ b/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy @@ -12,12 +12,16 @@ class MultiNodeAuthSpec extends BaseTestConfiguration { static String NODE1_NAME = "ssh-node" static String NODE2_NAME = "ssh-node-2" static String NODE3_NAME = "ssh-node-3" + static String NODE4_NAME = "ssh-node-4" // Test passwords for each node (matching docker-compose.yml) static String NODE1_PASSWORD = "testpassword123" static String NODE2_PASSWORD = "password2_special!@#" static String NODE3_PASSWORD = 'password3"quote\'test' + // Private key for node 4 + static String NODE4_PRIVATE_KEY_PATH = "src/test/resources/docker/keys/id_rsa" + def setupSpec() { startCompose() configureMultiNodeAuthProject() @@ -25,29 +29,12 @@ class MultiNodeAuthSpec extends BaseTestConfiguration { def configureMultiNodeAuthProject() { // Store passwords for each node in Rundeck key storage - okhttp3.RequestBody requestBody = okhttp3.RequestBody.create( - NODE1_PASSWORD.getBytes(), - org.rundeck.client.util.Client.MEDIA_TYPE_X_RUNDECK_PASSWORD - ) - client.apiCall { api -> - api.createKeyStorage("project/$PROJ_NAME/ssh-node.pass", requestBody) - } + storePasswordInKeyStorage("ssh-node.pass", NODE1_PASSWORD) + storePasswordInKeyStorage("ssh-node-2.pass", NODE2_PASSWORD) + storePasswordInKeyStorage("ssh-node-3.pass", NODE3_PASSWORD) - requestBody = okhttp3.RequestBody.create( - NODE2_PASSWORD.getBytes(), - org.rundeck.client.util.Client.MEDIA_TYPE_X_RUNDECK_PASSWORD - ) - client.apiCall { api -> - api.createKeyStorage("project/$PROJ_NAME/ssh-node-2.pass", requestBody) - } - - requestBody = okhttp3.RequestBody.create( - NODE3_PASSWORD.getBytes(), - org.rundeck.client.util.Client.MEDIA_TYPE_X_RUNDECK_PASSWORD - ) - client.apiCall { api -> - api.createKeyStorage("project/$PROJ_NAME/ssh-node-3.pass", requestBody) - } + // Store private key for node 4 in Rundeck key storage + storePrivateKeyInKeyStorage("ssh-node-4.key", NODE4_PRIVATE_KEY_PATH) // Create project def projList = client.apiCall { api -> api.listProjects() } @@ -80,54 +67,82 @@ class MultiNodeAuthSpec extends BaseTestConfiguration { waitForNodeAvailability(PROJ_NAME, NODE1_NAME) waitForNodeAvailability(PROJ_NAME, NODE2_NAME) waitForNodeAvailability(PROJ_NAME, NODE3_NAME) + waitForNodeAvailability(PROJ_NAME, NODE4_NAME) } - def "test multi-node authentication with different passwords"() { - when: - def jobId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" // multi-node-ping-test job + // Helper method to store password in Rundeck key storage + private void storePasswordInKeyStorage(String keyPath, String password) { + okhttp3.RequestBody requestBody = okhttp3.RequestBody.create( + password.getBytes(), + org.rundeck.client.util.Client.MEDIA_TYPE_X_RUNDECK_PASSWORD + ) + client.apiCall { api -> + api.createKeyStorage("project/$PROJ_NAME/$keyPath", requestBody) + } + } + + // Helper method to store private key in Rundeck key storage + private void storePrivateKeyInKeyStorage(String keyPath, String privateKeyFilePath) { + File privateKeyFile = new File(privateKeyFilePath) + okhttp3.RequestBody requestBody = okhttp3.RequestBody.create( + privateKeyFile, + org.rundeck.client.util.Client.MEDIA_TYPE_OCTET_STREAM + ) + client.apiCall { api -> + api.createKeyStorage("project/$PROJ_NAME/$keyPath", requestBody) + } + } + // Helper method to execute job and get execution state + private Map executeJobAndWait(String jobId, String loglevel = 'INFO', String filter = null) { JobRun request = new JobRun() - request.loglevel = 'INFO' + request.loglevel = loglevel + if (filter) { + request.filter = filter + } def result = client.apiCall { api -> api.runJob(jobId, request) } def executionId = result.id - def executionState = waitForJob(executionId) def logs = getLogs(executionId) - then: - // Job succeeded - this verifies that multi-node authentication worked - executionState != null - executionState.getExecutionState() == "SUCCEEDED" - - // Verify all three nodes were in the execution context - executionState.targetNodes.contains("ssh-node") - executionState.targetNodes.contains("ssh-node-2") - executionState.targetNodes.contains("ssh-node-3") + return [executionState: executionState, logs: logs, executionId: executionId] } - def "test ansible playbook with multi-node authentication"() { + // Helper method to verify all four nodes are targeted + private void verifyAllNodesTargeted(def targetNodes) { + assert targetNodes.contains("ssh-node") + assert targetNodes.contains("ssh-node-2") + assert targetNodes.contains("ssh-node-3") + assert targetNodes.contains("ssh-node-4") + } + + def "test multi-node authentication with different passwords"() { when: - def jobId = "b2c3d4e5-f6a7-8901-bcde-f12345678901" // ansible-playbook-multi-node-test job + def jobId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" // multi-node-ping-test job + def result = executeJobAndWait(jobId, 'INFO') - JobRun request = new JobRun() - request.loglevel = 'DEBUG' + then: + // Job succeeded - this verifies that multi-node authentication worked (passwords + private keys) + result.executionState != null + result.executionState.getExecutionState() == "SUCCEEDED" - def result = client.apiCall { api -> api.runJob(jobId, request) } - def executionId = result.id + // Verify all four nodes were in the execution context (3 with passwords, 1 with private key) + verifyAllNodesTargeted(result.executionState.targetNodes) + } - def executionState = waitForJob(executionId) - def logs = getLogs(executionId) + def "test ansible playbook with multi-node authentication"() { + when: + def jobId = "b2c3d4e5-f6a7-8901-bcde-f12345678901" // ansible-playbook-multi-node-test job + def result = executeJobAndWait(jobId, 'DEBUG') then: - // Job succeeded - this verifies the playbook ran successfully with multi-node authentication - executionState != null - executionState.getExecutionState() == "SUCCEEDED" - - // Verify all three nodes were targeted by the playbook execution - executionState.targetNodes.contains("ssh-node") - executionState.targetNodes.contains("ssh-node-2") - executionState.targetNodes.contains("ssh-node-3") + // Job succeeded - this verifies the playbook ran successfully with multi-node authentication (mixed auth types) + result.executionState != null + result.executionState.getExecutionState() == "SUCCEEDED" + + // Verify all four nodes were targeted by the playbook execution (3 passwords + 1 private key) + verifyAllNodesTargeted(result.executionState.targetNodes) } def "test nodes are accessible with different credentials"() { @@ -136,38 +151,48 @@ class MultiNodeAuthSpec extends BaseTestConfiguration { def nodes = client.apiCall { api -> api.listNodes(PROJ_NAME, ".*") } then: - // Verify all three nodes are registered + // Verify all four nodes are registered (3 with password auth, 1 with private key auth) nodes != null - nodes.size() >= 3 + nodes.size() >= 4 nodes.get(NODE1_NAME) != null nodes.get(NODE2_NAME) != null nodes.get(NODE3_NAME) != null + nodes.get(NODE4_NAME) != null } def "test passwords with special characters are properly escaped"() { when: def jobId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" - - JobRun request = new JobRun() - request.loglevel = 'DEBUG' - request.filter = "name: ssh-node-2 || name: ssh-node-3" // Only test nodes with special chars in passwords - - def result = client.apiCall { api -> api.runJob(jobId, request) } - def executionId = result.id - - def executionState = waitForJob(executionId) - def logs = getLogs(executionId) + def result = executeJobAndWait(jobId, 'DEBUG', "name: ssh-node-2 || name: ssh-node-3") then: // Job succeeded even with special characters in passwords - verifies password escaping works - executionState != null - executionState.getExecutionState() == "SUCCEEDED" + result.executionState != null + result.executionState.getExecutionState() == "SUCCEEDED" // Verify nodes with special character passwords were targeted - executionState.targetNodes.contains("ssh-node-2") - executionState.targetNodes.contains("ssh-node-3") + result.executionState.targetNodes.contains("ssh-node-2") + result.executionState.targetNodes.contains("ssh-node-3") // No YAML parsing errors - verifies special characters were properly escaped - !logs.any { it.log.toLowerCase().contains("yaml") && it.log.toLowerCase().contains("error") } + !result.logs.any { it.log.toLowerCase().contains("yaml") && it.log.toLowerCase().contains("error") } + } + + def "test private key authentication works in isolation"() { + when: + def jobId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" // multi-node-ping-test job + def result = executeJobAndWait(jobId, 'DEBUG', "name: ssh-node-4") + + then: + // Job succeeded with ONLY private key authentication (no passwords) + result.executionState != null + result.executionState.getExecutionState() == "SUCCEEDED" + + // Verify only the private key node was targeted + result.executionState.targetNodes.contains("ssh-node-4") + result.executionState.targetNodes.size() == 1 + + // Verify no password-related errors in logs + !result.logs.any { it.log.toLowerCase().contains("password") && it.log.toLowerCase().contains("error") } } } diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini b/functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini index 422d1a9a..c305c424 100644 --- a/functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/inventory.ini @@ -2,6 +2,7 @@ ssh-node ansible_host=ssh-node ansible_port=22 ssh-node-2 ansible_host=ssh-node-2 ansible_port=22 ssh-node-3 ansible_host=ssh-node-3 ansible_port=22 +ssh-node-4 ansible_host=ssh-node-4 ansible_port=22 [test_nodes:vars] ansible_connection=ssh diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml b/functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml index c38837c4..974b60fe 100644 --- a/functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/resources.xml @@ -38,4 +38,17 @@ ansible-ssh-auth-type="password" ansible-ssh-password-storage-path="keys/project/ansible-multi-node-auth/ssh-node-3.pass" ansible-ssh-user="rundeck"/> + + diff --git a/functional-test/src/test/resources/docker/docker-compose.yml b/functional-test/src/test/resources/docker/docker-compose.yml index 1ab6722f..05bf8951 100644 --- a/functional-test/src/test/resources/docker/docker-compose.yml +++ b/functional-test/src/test/resources/docker/docker-compose.yml @@ -37,6 +37,18 @@ services: volumes: - ./keys:/configuration:rw + ssh-node-4: + build: + context: node + environment: + NODE_USER_PASSWORD: unused_password + networks: + - rundeck + ports: + - "2225:22" + volumes: + - ./keys:/configuration:rw + rundeck: build: context: rundeck diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 87bd2419..05ed1666 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -474,10 +474,10 @@ public int run() throws Exception { }); // Build YAML content using helper method - String yamlContent = buildGroupVarsYaml(hostPasswords, hostUsers); + String yamlContent = buildGroupVarsYaml(hostPasswords, hostUsers, hostKeys); if(debug){ - System.err.println("DEBUG: Building group_vars YAML with " + hostPasswords.size() + " passwords and " + hostUsers.size() + " users"); + System.err.println("DEBUG: Building group_vars YAML with " + hostPasswords.size() + " passwords, " + hostUsers.size() + " users, and " + hostKeys.size() + " private keys"); System.err.println("DEBUG: YAML content built successfully, length: " + yamlContent.length()); } @@ -928,20 +928,24 @@ public boolean registerKeySshAgent(String keyPath) throws Exception { /** - * Builds YAML content for Ansible group_vars with vault-encrypted passwords and host users. + * Builds YAML content for Ansible group_vars with vault-encrypted passwords, host users, and private keys. * This method manually constructs the YAML because Jackson cannot properly handle Ansible's * custom !vault tag, which would result in an extra | character. * * @param hostPasswords Map of host names to vault-encrypted password strings * @param hostUsers Map of host names to usernames + * @param hostKeys Map of host names to private key file paths * @return YAML content string ready to be written to all.yaml */ - String buildGroupVarsYaml(Map hostPasswords, Map hostUsers) { + String buildGroupVarsYaml(Map hostPasswords, Map hostUsers, Map hostKeys) { StringBuilder yamlContent = new StringBuilder(); - yamlContent.append("host_passwords:\n"); - for (Map.Entry entry : hostPasswords.entrySet()) { + // Only add host_passwords section if there are passwords + if (!hostPasswords.isEmpty()) { + yamlContent.append("host_passwords:\n"); + + for (Map.Entry entry : hostPasswords.entrySet()) { String originalKey = entry.getKey(); String escapedKey = escapeYamlKey(originalKey); @@ -971,18 +975,16 @@ String buildGroupVarsYaml(Map hostPasswords, Map System.err.println("DEBUG: Single line vault value for host: " + originalKey); yamlContent.append(vaultValue).append("\n"); } + } } - yamlContent.append("\nhost_users:\n"); - for (Map.Entry entry : hostUsers.entrySet()) { - String originalKey = entry.getKey(); - String originalValue = entry.getValue(); - String escapedKey = escapeYamlKey(originalKey); - String escapedValue = escapeYamlValue(originalValue); - yamlContent.append(" ").append(escapedKey).append(": ") - .append(escapedValue).append("\n"); + // Add host_private_keys section if there are private keys + if (!hostKeys.isEmpty()) { + appendYamlMapSection(yamlContent, "host_private_keys", hostKeys, true); } + appendYamlMapSection(yamlContent, "host_users", hostUsers, true); + String result = yamlContent.toString(); if(debug){ @@ -992,6 +994,32 @@ String buildGroupVarsYaml(Map hostPasswords, Map return result; } + /** + * Helper method to append a simple YAML map section with key-value pairs. + * Extracted to reduce duplication between host_private_keys and host_users sections. + * + * @param yamlContent The StringBuilder to append to + * @param sectionName The name of the YAML section (e.g., "host_users", "host_private_keys") + * @param entries The map of key-value pairs to add to the section + * @param addLeadingNewline Whether to add a newline before the section name + */ + private void appendYamlMapSection(StringBuilder yamlContent, String sectionName, + Map entries, boolean addLeadingNewline) { + if (addLeadingNewline) { + yamlContent.append("\n"); + } + yamlContent.append(sectionName).append(":\n"); + + for (Map.Entry entry : entries.entrySet()) { + String originalKey = entry.getKey(); + String originalValue = entry.getValue(); + String escapedKey = escapeYamlKey(originalKey); + String escapedValue = escapeYamlValue(originalValue); + yamlContent.append(" ").append(escapedKey).append(": ") + .append(escapedValue).append("\n"); + } + } + /** * Escapes YAML special characters in keys. * If the key contains special characters, wraps it in quotes. diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 1a03c6e6..675ed45c 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -105,19 +105,10 @@ public String getPrivateKeyStoragePath() { } public String getPrivateKeyStoragePath(INodeEntry node) { - String path = PropertyResolver.resolveProperty( + return resolveAndExpandStoragePath( AnsibleDescribable.ANSIBLE_SSH_KEYPATH_STORAGE_PATH, - null, - getFrameworkProject(), - getFramework(), - node, - getJobConf() + node ); - //expand properties in path - if (path != null && path.contains("${")) { - path = DataContextUtils.replaceDataReferencesInString(path, context.getDataContext()); - } - return path; } public byte[] getPrivateKeyStorageDataBytes() throws IOException { @@ -126,13 +117,27 @@ public byte[] getPrivateKeyStorageDataBytes() throws IOException { } public String getPasswordStoragePath() { + return getPasswordStoragePath(getNode()); + } - String path = PropertyResolver.resolveProperty( + public String getPasswordStoragePath(INodeEntry node) { + return resolveAndExpandStoragePath( AnsibleDescribable.ANSIBLE_SSH_PASSWORD_STORAGE_PATH, + node + ); + } + + /** + * Helper method to resolve a storage path property and expand any data references. + * Extracted to reduce duplication between password and private key storage path resolution. + */ + private String resolveAndExpandStoragePath(String propertyName, INodeEntry node) { + String path = PropertyResolver.resolveProperty( + propertyName, null, getFrameworkProject(), getFramework(), - getNode(), + node, getJobConf() ); @@ -143,22 +148,16 @@ public String getPasswordStoragePath() { return path; } - public String getSshPrivateKey(node) throws ConfigurationException { + public String getSshPrivateKey() throws ConfigurationException { + return getSshPrivateKey(getNode()); + } + + public String getSshPrivateKey(INodeEntry node) throws ConfigurationException { //look for storage option String storagePath = getPrivateKeyStoragePath(node); if (null != storagePath) { - Path path = PathUtil.asPath(storagePath); - try { - ResourceMeta contents = context.getStorageTree().getResource(path) - .getContents(); - ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); - contents.writeContent(byteArrayOutputStream); - return byteArrayOutputStream.toString(); - } catch (StorageException | IOException e) { - throw new ConfigurationException("Failed to read the ssh private key for " + - "storage path: " + storagePath + ": " + e.getMessage()); - } + return readFromStoragePath(storagePath, "ssh private key"); } else { //else look up option value final String path = getPrivateKeyfilePath(); @@ -176,6 +175,10 @@ public String getSshPrivateKey(node) throws ConfigurationException { } public String getSshPassword() throws ConfigurationException { + return getSshPassword(getNode()); + } + + public String getSshPassword(INodeEntry node) throws ConfigurationException { //look for option values first //typically jobs use secure options to dynamically setup the ssh password @@ -184,7 +187,7 @@ public String getSshPassword() throws ConfigurationException { AnsibleDescribable.DEFAULT_ANSIBLE_SSH_PASSWORD_OPTION, getFrameworkProject(), getFramework(), - getNode(), + node, getJobConf() ); String sshPassword = PropertyResolver.evaluateSecureOption(passwordOption, getContext()); @@ -194,7 +197,7 @@ public String getSshPassword() throws ConfigurationException { return sshPassword; } else { //look for storage option - String storagePath = getPasswordStoragePath(); + String storagePath = getPasswordStoragePath(node); if (null != storagePath) { //look up storage value @@ -207,7 +210,19 @@ public String getSshPassword() throws ConfigurationException { } public String getPasswordFromPath(String storagePath) throws ConfigurationException { - //look up storage value + return readFromStoragePath(storagePath, "ssh password"); + } + + /** + * Helper method to read content from Rundeck storage path. + * Extracted to reduce duplication between password and private key reading. + * + * @param storagePath The storage path to read from + * @param resourceType Description of resource type for error messages (e.g., "ssh password", "ssh private key") + * @return The content as a UTF-8 string + * @throws ConfigurationException if reading fails + */ + private String readFromStoragePath(String storagePath, String resourceType) throws ConfigurationException { Path path = PathUtil.asPath(storagePath); try { ResourceMeta contents = context.getStorageTree().getResource(path) @@ -216,7 +231,7 @@ public String getPasswordFromPath(String storagePath) throws ConfigurationExcept contents.writeContent(byteArrayOutputStream); return byteArrayOutputStream.toString("UTF-8"); } catch (StorageException | IOException e) { - throw new ConfigurationException("Failed to read the ssh password for " + + throw new ConfigurationException("Failed to read the " + resourceType + " for " + "storage path: " + storagePath + ": " + e.getMessage()); } } diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index 9214adec..b80e6cba 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -690,7 +690,7 @@ class AnsibleRunnerSpec extends Specification{ ] when: - def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers) + def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers, [:]) then: yaml.contains("host_passwords:") @@ -717,7 +717,7 @@ class AnsibleRunnerSpec extends Specification{ ] when: - def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers) + def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers, [:]) then: yaml.contains('"web:server:1": !vault |') @@ -738,7 +738,7 @@ class AnsibleRunnerSpec extends Specification{ ] when: - runner.buildGroupVarsYaml(hostPasswords, hostUsers) + runner.buildGroupVarsYaml(hostPasswords, hostUsers, [:]) then: def e = thrown(RuntimeException) @@ -755,10 +755,10 @@ class AnsibleRunnerSpec extends Specification{ def hostUsers = [:] when: - def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers) + def yaml = runner.buildGroupVarsYaml(hostPasswords, hostUsers, [:]) then: - yaml.contains("host_passwords:") + !yaml.contains("host_passwords:") // Should not appear when empty yaml.contains("host_users:") } From 84d816ac578973d029847616b2b369d796c041e3 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Wed, 14 Jan 2026 12:42:17 -0300 Subject: [PATCH 23/46] changed error messages, changed debug logs --- functional-test/build.gradle | 2 +- .../ansible/ansible/AnsibleRunner.java | 6 ++- .../ansible/AnsibleRunnerContextBuilder.java | 42 +++++++++++++++++-- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/functional-test/build.gradle b/functional-test/build.gradle index 46cd047c..188c4e88 100644 --- a/functional-test/build.gradle +++ b/functional-test/build.gradle @@ -42,7 +42,7 @@ dependencies { tasks.register('functionalTest', Test) { useJUnitPlatform() - systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.1.1") + systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.18.0") // Docker configuration for Testcontainers // For Rancher Desktop on macOS: Use /Users//.rd/docker.sock diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 05ed1666..d1f6f95f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -446,7 +446,8 @@ public int run() throws Exception { try { encryptedPassword = ansibleVault.encryptVariable("ansible_password", password); } catch (Exception e) { - throw new RuntimeException(e); + throw new RuntimeException("Failed to encrypt password for node '" + + nodeName + "' using Ansible Vault: " + e.getMessage(), e); } } hostPasswords.put(nodeName, encryptedPassword); @@ -468,7 +469,8 @@ public int run() throws Exception { tempHostPkFile.deleteOnExit(); } catch (IOException e) { - throw new RuntimeException(e); + throw new RuntimeException("Failed to create temporary private key file for node '" + + nodeName + "': " + e.getMessage(), e); } } }); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 675ed45c..bc8b7e4f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -974,12 +974,27 @@ public Map> getNodesAuthenticationMap(){ this.context.getNodes().forEach((node) -> { Map auth = new HashMap<>(); final AuthenticationType authType = getSshAuthenticationType(node); + + if (getDebug()) { + System.err.println("DEBUG: Processing authentication for node '" + node.getNodename() + + "' with auth type: " + authType); + } + if (AnsibleDescribable.AuthenticationType.privateKey == authType) { final String privateKey; try { privateKey = getSshPrivateKey(node); + if (getDebug()) { + System.err.println("DEBUG: Successfully retrieved private key for node '" + + node.getNodename() + "'"); + } } catch (ConfigurationException e) { - throw new RuntimeException(e); + if (getDebug()) { + System.err.println("DEBUG: Error retrieving private key for node '" + + node.getNodename() + "': " + e.getMessage()); + } + throw new RuntimeException("Failed to retrieve private key for node '" + + node.getNodename() + "': " + e.getMessage(), e); } if (privateKey != null) { auth.put("ansible_ssh_private_key", privateKey); @@ -989,10 +1004,18 @@ public Map> getNodesAuthenticationMap(){ String password = getSshPassword(node); if(null!=password){ auth.put("ansible_password", password); + if (getDebug()) { + System.err.println("DEBUG: Successfully retrieved password for node '" + + node.getNodename() + "'"); + } } } catch (ConfigurationException e) { - System.err.println("DEBUG: Error retrieving password for " + node.getNodename() + ": " + e.getMessage()); - throw new RuntimeException(e); + if (getDebug()) { + System.err.println("DEBUG: Error retrieving password for node '" + + node.getNodename() + "': " + e.getMessage()); + } + throw new RuntimeException("Failed to retrieve password for node '" + + node.getNodename() + "': " + e.getMessage(), e); } } @@ -1002,6 +1025,19 @@ public Map> getNodesAuthenticationMap(){ auth.put("ansible_user",userName ); } + // Validate that node has at least one authentication method configured + boolean hasPassword = auth.containsKey("ansible_password"); + boolean hasPrivateKey = auth.containsKey("ansible_ssh_private_key"); + + if (!hasPassword && !hasPrivateKey) { + context.getExecutionLogger().log(2, "WARNING: Node '" + node.getNodename() + + "' has no password or private key configured. Authentication may fail."); + if (getDebug()) { + System.err.println("DEBUG: Node '" + node.getNodename() + + "' has no credentials configured (only username: " + (auth.containsKey("ansible_user") ? "yes" : "no") + ")"); + } + } + authenticationNodesMap.put(node.getNodename(), auth); }); From 684aa512b42d970c969163c0bd1e9cb56a579777 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Wed, 14 Jan 2026 13:14:03 -0300 Subject: [PATCH 24/46] added node name sanitization and unit test --- .../ansible/ansible/AnsibleRunner.java | 7 ++- .../ansible/ansible/AnsibleRunnerSpec.groovy | 57 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index d1f6f95f..c8250574 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -457,7 +457,12 @@ public int run() throws Exception { //create temporary file for private key File tempHostPkFile; try { - tempHostPkFile = AnsibleUtil.createTemporaryFile("","id_rsa_node_"+nodeName, privateKey,customTmpDirPath); + // Sanitize node name for filesystem use (replace unsafe characters with underscores) + String safeNodeName = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_"); + if(debug && !nodeName.equals(safeNodeName)) { + System.err.println("DEBUG: Sanitized node name '" + nodeName + "' to '" + safeNodeName + "' for temp file"); + } + tempHostPkFile = AnsibleUtil.createTemporaryFile("","id_rsa_node_"+safeNodeName, privateKey,customTmpDirPath); // Only the owner can read and write Set perms = new HashSet(); diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index b80e6cba..06fd77c4 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -1002,4 +1002,61 @@ class AnsibleRunnerSpec extends Specification{ 2 * ansibleVault.encryptVariable(_, _) >> "!vault | value" } + def "node name sanitization: should sanitize node names with forward slashes for temp files"() { + given: + // Test case from real issue: node name with forward slashes + String nodeName = "/docker-runner-ansible-ssh-node-b-3" + String sanitized = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") + + expect: + sanitized == "_docker-runner-ansible-ssh-node-b-3" + // Verify sanitized name is filesystem-safe + !sanitized.contains("/") + } + + def "node name sanitization: should sanitize various special characters"() { + expect: + nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") == expected + + where: + nodeName || expected + "simple-node" || "simple-node" + "node.with.dots" || "node.with.dots" + "node_with_underscores" || "node_with_underscores" + "/docker-runner-ansible" || "_docker-runner-ansible" + "node:with:colons" || "node_with_colons" + "node with spaces" || "node_with_spaces" + "node\\with\\backslashes" || "node_with_backslashes" + "node*with?wildcards" || "node_with_wildcards" + "node|with|pipes" || "node_with_pipes" + "nodebrackets" || "node_with_brackets" + "node\"with'quotes" || "node_with_quotes" + 'node@with#special$chars%' || "node_with_special_chars_" + "/path/to/node-123" || "_path_to_node-123" + } + + def "node name sanitization: should preserve safe alphanumeric and allowed characters"() { + given: + String safeName = "my-node_123.server-A" + String sanitized = safeName.replaceAll("[^a-zA-Z0-9._-]", "_") + + expect: + sanitized == safeName // Should not be changed + } + + def "node name sanitization: should handle empty and edge case node names"() { + expect: + nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") == expected + + where: + nodeName || expected + "" || "" + "123" || "123" + "..." || "..." + "---" || "---" + "___" || "___" + "a" || "a" + "/////" || "_____" + } + } From 6a46aed89ca46905f120c87d830ba44ac2d459cd Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Wed, 14 Jan 2026 15:43:22 -0300 Subject: [PATCH 25/46] send private keys to runner --- .../ansible/ansible/AnsibleRunner.java | 5 ++++- .../ansible/AnsibleRunnerContextBuilder.java | 19 +++++++++++++++++++ .../ansible/plugin/AnsibleNodeExecutor.java | 8 +------- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index c8250574..6119922d 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -541,7 +541,10 @@ public int run() throws Exception { if(!hostPasswords.isEmpty()){ procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] }}\""); } - procArgs.add("-e ansible_ssh_private_key_file=\"{{ host_private_keys[inventory_hostname] }}\""); + + if(!hostKeys.isEmpty()){ + procArgs.add("-e ansible_ssh_private_key_file=\"{{ host_private_keys[inventory_hostname] }}\""); + } } catch (IOException e) { System.err.println("ERROR: Failed to write all.yaml: " + e.getMessage()); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index bc8b7e4f..c5d832d9 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -1047,6 +1047,10 @@ public Map> getNodesAuthenticationMap(){ public List getListNodesKeyPath(){ + if(!generateInventoryNodesAuth()) { + return new ArrayList<>(); + } + List secretPaths = new ArrayList<>(); this.context.getNodes().forEach((node) -> { @@ -1064,6 +1068,21 @@ public List getListNodesKeyPath(){ secretPaths.add(keyPath); } } + + String privateKeyPath = PropertyResolver.resolveProperty( + AnsibleDescribable.ANSIBLE_SSH_KEYPATH_STORAGE_PATH, + null, + getFrameworkProject(), + getFramework(), + node, + getJobConf() + ); + + if(null!=privateKeyPath){ + if(!secretPaths.contains(privateKeyPath)){ + secretPaths.add(privateKeyPath); + } + } }); return secretPaths; diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java index e75ae84e..3b1b7165 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java @@ -200,13 +200,7 @@ public List listSecretsPath(ExecutionContext context, INodeEntry node) { jobConf.put(AnsibleDescribable.ANSIBLE_LIMIT,node.getNodename()); AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(node, context, context.getFramework(), jobConf); - List secretPaths = AnsibleUtil.getSecretsPath(builder); - List secretPathsNodes = builder.getListNodesKeyPath(); - - if(secretPathsNodes != null && !secretPathsNodes.isEmpty()){ - secretPaths.addAll(secretPathsNodes); - } - return secretPaths; + return AnsibleUtil.getSecretsPath(builder); } } From 85a41366da413e2e7a4b746d14fe12c91192c0b8 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 15 Jan 2026 08:13:38 -0300 Subject: [PATCH 26/46] addressed copilot comments --- .../ansible/ansible/AnsibleDescribable.java | 4 +- .../ansible/ansible/AnsibleRunner.java | 128 +++++++++++------- .../ansible/AnsibleRunnerContextBuilder.java | 66 ++++++--- .../plugin/AnsibleModuleWorkflowStep.java | 8 +- ...AnsiblePlaybookInlineWorkflowNodeStep.java | 8 +- .../AnsiblePlaybookInlineWorkflowStep.java | 32 ++--- .../AnsiblePlaybookWorflowNodeStep.java | 8 +- .../plugin/AnsiblePlaybookWorkflowStep.java | 32 ++--- .../ansible/ansible/AnsibleRunnerSpec.groovy | 6 +- 9 files changed, 185 insertions(+), 107 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java index 5b8696b1..4b268e5c 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java @@ -237,8 +237,8 @@ public static String[] getValues() { static final Property GENERATE_INVENTORY_NODES_AUTH = PropertyBuilder.builder() .booleanType(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH) .required(false) - .title("Generate inventory, pass node authentication from rundeck nodes") - .description("Pass authentication from rundeck nodes.") + .title("Workflow Step: Generate inventory, pass node authentication from rundeck nodes") + .description("Pass authentication from rundeck nodes. Only applies to workflow steps.") .build(); public static Property EXECUTABLE_PROP = PropertyUtil.freeSelect( diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 6119922d..0e70334e 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -9,6 +9,7 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; import lombok.Builder; import lombok.Data; +import lombok.extern.slf4j.Slf4j; import java.io.*; @@ -17,6 +18,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.util.*; +@Slf4j @Builder @Data public class AnsibleRunner { @@ -831,7 +833,13 @@ public int run() throws Exception { } if (groupVarsDir != null && groupVarsDir.exists()) { - if (!groupVarsDir.delete()) { + log.debug("Attempting to delete group_vars directory: {}", groupVarsDir.getAbsolutePath()); + try { + deleteTempDirectory(groupVarsDir.toPath()); + log.debug("Successfully deleted group_vars directory: {}", groupVarsDir.getAbsolutePath()); + } catch (IOException e) { + log.warn("Failed to delete group_vars directory: {}. Marking for deletion on JVM exit. Error: {}", + groupVarsDir.getAbsolutePath(), e.getMessage()); groupVarsDir.deleteOnExit(); } } @@ -956,35 +964,35 @@ String buildGroupVarsYaml(Map hostPasswords, Map yamlContent.append("host_passwords:\n"); for (Map.Entry entry : hostPasswords.entrySet()) { - String originalKey = entry.getKey(); - String escapedKey = escapeYamlKey(originalKey); + String originalKey = entry.getKey(); + String escapedKey = escapeYamlKey(originalKey); - yamlContent.append(" ").append(escapedKey).append(": "); - String vaultValue = entry.getValue(); + yamlContent.append(" ").append(escapedKey).append(": "); + String vaultValue = entry.getValue(); - // Validate vault format - if (!isValidVaultFormat(vaultValue)) { - System.err.println("ERROR: Invalid vault format for host: " + originalKey); - throw new RuntimeException("Invalid vault format for host: " + originalKey); - } - // The vault value already contains "!vault |\n" followed by the encrypted content - // We just need to split and indent properly (4 spaces for vault content lines) - if (vaultValue.contains("\n")) { - String[] lines = vaultValue.split("\n", -1); - for (int i = 0; i < lines.length; i++) { - if (i == 0) { - // First line: "!vault |" - yamlContent.append(lines[i]).append("\n"); - } else if (i < lines.length - 1 || !lines[i].isEmpty()) { - // Vault content: indent with 4 spaces - yamlContent.append(" ").append(lines[i]).append("\n"); + // Validate vault format + if (!isValidVaultFormat(vaultValue)) { + System.err.println("ERROR: Invalid vault format for host: " + originalKey); + throw new RuntimeException("Invalid vault format for host: " + originalKey); + } + // The vault value already contains "!vault |\n" followed by the encrypted content + // We just need to split and indent properly (4 spaces for vault content lines) + if (vaultValue.contains("\n")) { + String[] lines = vaultValue.split("\n", -1); + for (int i = 0; i < lines.length; i++) { + if (i == 0) { + // First line: "!vault |" + yamlContent.append(lines[i]).append("\n"); + } else if (i < lines.length - 1 || !lines[i].isEmpty()) { + // Vault content: indent with 4 spaces + yamlContent.append(" ").append(lines[i]).append("\n"); + } } + } else { + // Single line value (shouldn't happen with vault, but handle it) + System.err.println("DEBUG: Single line vault value for host: " + originalKey); + yamlContent.append(vaultValue).append("\n"); } - } else { - // Single line value (shouldn't happen with vault, but handle it) - System.err.println("DEBUG: Single line vault value for host: " + originalKey); - yamlContent.append(vaultValue).append("\n"); - } } } @@ -993,7 +1001,10 @@ String buildGroupVarsYaml(Map hostPasswords, Map appendYamlMapSection(yamlContent, "host_private_keys", hostKeys, true); } - appendYamlMapSection(yamlContent, "host_users", hostUsers, true); + // Add host_users section if there are users + if (!hostUsers.isEmpty()) { + appendYamlMapSection(yamlContent, "host_users", hostUsers, true); + } String result = yamlContent.toString(); @@ -1043,7 +1054,10 @@ String escapeYamlKey(String key) { key.startsWith("-") || key.startsWith("?") || key.matches("^[0-9].*")) { - // Escape any existing backslashes and quotes, then wrap in quotes + // IMPORTANT: Order of replacements matters! Must escape backslashes FIRST, then quotes. + // If we escaped quotes first, the backslash replacement would double-escape them. + // Example: value="a\"b" -> replace quotes: "a\\"b" -> replace backslash: "a\\\\"b" (WRONG!) + // Correct: value="a\"b" -> replace backslash: "a\\"b" -> replace quotes: "a\\\"b" (RIGHT) return "\"" + key.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; } return key; @@ -1062,7 +1076,10 @@ String escapeYamlValue(String value) { value.startsWith("-") || value.startsWith("?") || value.trim().isEmpty()) { - // Escape any existing backslashes and quotes, then wrap in quotes + // IMPORTANT: Order of replacements matters! Must escape backslashes FIRST, then quotes. + // If we escaped quotes first, the backslash replacement would double-escape them. + // Example: value="a\"b" -> replace quotes: "a\\"b" -> replace backslash: "a\\\\"b" (WRONG!) + // Correct: value="a\"b" -> replace backslash: "a\\"b" -> replace quotes: "a\\\"b" (RIGHT) return "\"" + value.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; } return value; @@ -1088,8 +1105,12 @@ String escapePasswordForYaml(String password) { * Validates that a string is in proper Ansible vault format. * Expected format: "!vault |\n" followed by vault-encrypted content * + * Real Ansible Vault values are ALWAYS multiline with encrypted content. + * This validation rejects single-line vault values and vault values without content + * to prevent authentication failures. + * * @param vaultValue The vault value to validate - * @return true if valid vault format, false otherwise + * @return true if valid vault format with content, false otherwise */ boolean isValidVaultFormat(String vaultValue) { if (vaultValue == null || vaultValue.trim().isEmpty()) { @@ -1102,25 +1123,40 @@ boolean isValidVaultFormat(String vaultValue) { return false; } - // For multiline vault values, should contain the pipe character and newline - if (vaultValue.contains("\n")) { - // Check if it follows the "!vault |" format - String firstLine = vaultValue.split("\n", 2)[0]; - if (!firstLine.trim().matches("!vault\\s*\\|?")) { - return false; - } + // Ansible Vault values must be multiline - reject single-line values + if (!vaultValue.contains("\n")) { + return false; + } - // Should have encrypted content after the first line - String[] lines = vaultValue.split("\n"); - if (lines.length < 2) { - return false; + // Check if it follows the "!vault |" or "!vault" format + String firstLine = vaultValue.split("\n", 2)[0]; + if (!firstLine.trim().matches("!vault\\s*\\|?")) { + return false; + } + + // Must have encrypted content after the first line + String[] lines = vaultValue.split("\n"); + if (lines.length < 2) { + return false; + } + + // At least one content line must be non-empty (excluding the last line which may be empty) + boolean hasContent = false; + for (int i = 1; i < lines.length; i++) { + if (!lines[i].trim().isEmpty()) { + hasContent = true; + break; } + } - // Vault content lines should not be empty (except maybe the last one) - for (int i = 1; i < lines.length - 1; i++) { - if (lines[i].trim().isEmpty()) { - return false; - } + if (!hasContent) { + return false; + } + + // Vault content lines should not be empty (except the last one) + for (int i = 1; i < lines.length - 1; i++) { + if (lines[i].trim().isEmpty()) { + return false; } } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index c5d832d9..3faf2304 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -24,6 +24,7 @@ import com.rundeck.plugins.ansible.plugin.AnsiblePluginGroup; import com.rundeck.plugins.ansible.util.AnsibleUtil; import lombok.Getter; +import lombok.extern.slf4j.Slf4j; import org.rundeck.storage.api.Path; import java.util.*; @@ -31,6 +32,7 @@ import org.rundeck.storage.api.PathUtil; import org.rundeck.storage.api.StorageException; +@Slf4j @Getter public class AnsibleRunnerContextBuilder { @@ -219,10 +221,14 @@ public String getPasswordFromPath(String storagePath) throws ConfigurationExcept * * @param storagePath The storage path to read from * @param resourceType Description of resource type for error messages (e.g., "ssh password", "ssh private key") - * @return The content as a UTF-8 string + * @return The content as a UTF-8 string, or null if storagePath is null * @throws ConfigurationException if reading fails */ private String readFromStoragePath(String storagePath, String resourceType) throws ConfigurationException { + if (storagePath == null) { + return null; + } + Path path = PathUtil.asPath(storagePath); try { ResourceMeta contents = context.getStorageTree().getResource(path) @@ -799,19 +805,25 @@ public INodeEntry getNode() { public void cleanupTempFiles() { // Clean up individual temp files for (File temp : tempFiles) { - if (getDebug()) { - System.err.println("DEBUG: Deleting temp file: " + temp.getAbsolutePath()); + log.debug("Attempting to delete temp file: {}", temp.getAbsolutePath()); + if (!temp.delete()) { + log.warn("Failed to delete temp file: {}. Marking for deletion on JVM exit.", temp.getAbsolutePath()); + // Fallback: schedule for deletion on JVM exit + temp.deleteOnExit(); + } else { + log.debug("Successfully deleted temp file: {}", temp.getAbsolutePath()); } - temp.delete(); } tempFiles.clear(); // Clean up execution-specific directory (including group_vars) if (executionSpecificDir != null && executionSpecificDir.exists()) { - if (getDebug()) { - System.err.println("DEBUG: Execution-specific directory exists: " + executionSpecificDir.getAbsolutePath()); + log.debug("Cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + if (!deleteDirectoryRecursively(executionSpecificDir)) { + log.warn("Failed to completely delete execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + } else { + log.debug("Successfully deleted execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); } - deleteDirectoryRecursively(executionSpecificDir); } } @@ -819,23 +831,37 @@ public void cleanupTempFiles() { * Recursively deletes a directory and all its contents. * * @param directory The directory to delete + * @return true if the directory and all its contents were successfully deleted, false otherwise */ - private void deleteDirectoryRecursively(File directory) { + private boolean deleteDirectoryRecursively(File directory) { if (directory == null || !directory.exists()) { - return; + return true; } + boolean success = true; File[] files = directory.listFiles(); if (files != null) { for (File file : files) { if (file.isDirectory()) { - deleteDirectoryRecursively(file); + if (!deleteDirectoryRecursively(file)) { + success = false; + log.warn("Failed to delete subdirectory: {}", file.getAbsolutePath()); + } } else { - file.delete(); + if (!file.delete()) { + success = false; + log.warn("Failed to delete file: {}", file.getAbsolutePath()); + } } } } - directory.delete(); + + if (!directory.delete()) { + success = false; + log.warn("Failed to delete directory: {}", directory.getAbsolutePath()); + } + + return success; } public Boolean getUseSshAgent() { @@ -1159,14 +1185,18 @@ String getExecutionSpecificTmpDir() { if (executionId != null && !executionId.isEmpty()) { executionSpecificDir = new File(baseTmpDir, "ansible-exec-" + executionId); if (!executionSpecificDir.exists()) { - if (getDebug()) { - System.err.println("DEBUG: Creating execution-specific directory: " + executionSpecificDir.getAbsolutePath()); + log.debug("Creating execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + // Check return value to handle race conditions and creation failures + boolean created = executionSpecificDir.mkdirs(); + if (!created && !executionSpecificDir.exists()) { + // Failed to create and directory still doesn't exist - this is an error + String errorMsg = "Failed to create execution-specific directory: " + executionSpecificDir.getAbsolutePath(); + log.error(errorMsg); + throw new IllegalStateException(errorMsg); } - executionSpecificDir.mkdirs(); + log.debug("Successfully created execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); } else { - if (getDebug()) { - System.err.println("DEBUG: Execution-specific directory already exists: " + executionSpecificDir.getAbsolutePath()); - } + log.debug("Execution-specific directory already exists: {}", executionSpecificDir.getAbsolutePath()); } return executionSpecificDir.getAbsolutePath(); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java index 247b6b3c..706f4369 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java @@ -72,7 +72,13 @@ public void executeStep(PluginStepContext context, Map configura } // set log level - if (context.getDataContext().get("job").get("loglevel").equals("DEBUG")) { + String loglevel = null; + if (context.getDataContext() != null && + context.getDataContext().get("job") != null) { + loglevel = context.getDataContext().get("job").get("loglevel"); + } + + if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "True"); } else { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "False"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java index a9a1d40c..c95a6e2f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java @@ -79,7 +79,13 @@ public void executeNodeStep( configuration.put(AnsibleDescribable.ANSIBLE_LIMIT,entry.getNodename()); // set log level - if (context.getDataContext().get("job").get("loglevel").equals("DEBUG")) { + String loglevel = null; + if (context.getDataContext() != null && + context.getDataContext().get("job") != null) { + loglevel = context.getDataContext().get("job").get("loglevel"); + } + + if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG,"True"); } else { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG,"False"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index e5e626c0..63bc9dc8 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -16,12 +16,14 @@ import com.dtolabs.rundeck.plugins.step.StepPlugin; import com.dtolabs.rundeck.plugins.util.DescriptionBuilder; import com.rundeck.plugins.ansible.util.AnsibleUtil; +import lombok.extern.slf4j.Slf4j; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +@Slf4j @Plugin(name = AnsiblePlaybookInlineWorkflowStep.SERVICE_PROVIDER_NAME, service = ServiceNameConstants.WorkflowStep) public class AnsiblePlaybookInlineWorkflowStep implements StepPlugin, AnsibleDescribable, ProxyRunnerPlugin, ConfiguredBy { @@ -85,7 +87,13 @@ public void executeStep(PluginStepContext context, Map configura configuration.put(AnsibleDescribable.ANSIBLE_LIMIT, limit); } // set log level - if (context.getDataContext().get("job").get("loglevel").equals("DEBUG")) { + String loglevel = null; + if (context.getDataContext() != null && + context.getDataContext().get("job") != null) { + loglevel = context.getDataContext().get("job").get("loglevel"); + } + + if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "True"); } else { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "False"); @@ -103,31 +111,19 @@ public void executeStep(PluginStepContext context, Map configura Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); - boolean isDebug = context.getDataContext().get("job").get("loglevel").equals("DEBUG"); - - if(isDebug) { - System.err.println("DEBUG: generateInventoryNodesAuth returned: " + generateInventoryNodeAuth); - } + log.debug("generateInventoryNodesAuth returned: {}", generateInventoryNodeAuth); if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ - if(isDebug) { - System.err.println("DEBUG: Node auth is enabled, getting authentication map"); - } + log.debug("Node auth is enabled, getting authentication map"); Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); - if(isDebug) { - System.err.println("DEBUG: Retrieved " + (nodesAuth != null ? nodesAuth.size() : 0) + " node authentications"); - } + log.debug("Retrieved {} node authentications", (nodesAuth != null ? nodesAuth.size() : 0)); if (nodesAuth != null && !nodesAuth.isEmpty()) { runner.setAddNodeAuthToInventory(true); runner.setNodesAuthentication(nodesAuth); - if(isDebug) { - System.err.println("DEBUG: Set node authentication on runner"); - } + log.debug("Set node authentication on runner"); } } else { - if(isDebug) { - System.err.println("DEBUG: Node auth is NOT enabled"); - } + log.debug("Node auth is NOT enabled"); } runner.setCustomTmpDirPath(AnsibleUtil.getCustomTmpPathDir(contextBuilder.getFramework())); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java index 0a604110..f44fa2ef 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java @@ -76,7 +76,13 @@ public void executeNodeStep( configuration.put(AnsibleDescribable.ANSIBLE_LIMIT,entry.getNodename()); // set log level - if (context.getDataContext().get("job").get("loglevel").equals("DEBUG")) { + String loglevel = null; + if (context.getDataContext() != null && + context.getDataContext().get("job") != null) { + loglevel = context.getDataContext().get("job").get("loglevel"); + } + + if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG,"True"); } else { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG,"False"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index 061dff28..c0d294ba 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -16,11 +16,13 @@ import com.dtolabs.rundeck.plugins.step.StepPlugin; import com.dtolabs.rundeck.plugins.util.DescriptionBuilder; import com.rundeck.plugins.ansible.util.AnsibleUtil; +import lombok.extern.slf4j.Slf4j; import java.util.HashMap; import java.util.List; import java.util.Map; +@Slf4j @Plugin(name = AnsiblePlaybookWorkflowStep.SERVICE_PROVIDER_NAME, service = ServiceNameConstants.WorkflowStep) public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribable, ProxyRunnerPlugin, ConfiguredBy { @@ -84,7 +86,13 @@ public void executeStep(PluginStepContext context, Map configura configuration.put(AnsibleDescribable.ANSIBLE_LIMIT, limit); } // set log level - if (context.getDataContext().get("job").get("loglevel").equals("DEBUG")) { + String loglevel = null; + if (context.getDataContext() != null && + context.getDataContext().get("job") != null) { + loglevel = context.getDataContext().get("job").get("loglevel"); + } + + if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "True"); } else { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "False"); @@ -101,31 +109,19 @@ public void executeStep(PluginStepContext context, Map configura Boolean generateInventoryNodeAuth = contextBuilder.generateInventoryNodesAuth(); - boolean isDebug = context.getDataContext().get("job").get("loglevel").equals("DEBUG"); - - if(isDebug) { - System.err.println("DEBUG: generateInventoryNodesAuth returned: " + generateInventoryNodeAuth); - } + log.debug("generateInventoryNodesAuth returned: {}", generateInventoryNodeAuth); if(generateInventoryNodeAuth != null && generateInventoryNodeAuth){ - if(isDebug) { - System.err.println("DEBUG: Node auth is enabled, getting authentication map"); - } + log.debug("Node auth is enabled, getting authentication map"); Map> nodesAuth = contextBuilder.getNodesAuthenticationMap(); - if(isDebug) { - System.err.println("DEBUG: Retrieved " + (nodesAuth != null ? nodesAuth.size() : 0) + " node authentications"); - } + log.debug("Retrieved {} node authentications", (nodesAuth != null ? nodesAuth.size() : 0)); if (nodesAuth != null && !nodesAuth.isEmpty()) { runner.setAddNodeAuthToInventory(true); runner.setNodesAuthentication(nodesAuth); - if(isDebug) { - System.err.println("DEBUG: Set node authentication on runner"); - } + log.debug("Set node authentication on runner"); } } else { - if(isDebug) { - System.err.println("DEBUG: Node auth is NOT enabled"); - } + log.debug("Node auth is NOT enabled"); } runner.setCustomTmpDirPath(AnsibleUtil.getCustomTmpPathDir(contextBuilder.getFramework())); diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index 06fd77c4..6581448b 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -670,7 +670,7 @@ class AnsibleRunnerSpec extends Specification{ "not a vault" || false null || false "" || false - "!vault" || true // minimal valid + "!vault" || false // single-line without content - invalid "!vault |\n" || false // no content } @@ -759,7 +759,9 @@ class AnsibleRunnerSpec extends Specification{ then: !yaml.contains("host_passwords:") // Should not appear when empty - yaml.contains("host_users:") + !yaml.contains("host_users:") // Should not appear when empty + !yaml.contains("host_private_keys:") // Should not appear when empty + yaml.isEmpty() || yaml.trim().isEmpty() // Should produce empty or whitespace-only output } def "escapePasswordForYaml: should escape special characters"() { From 6d8c5486bf5433f173b514463e314f6c7569e674 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 15 Jan 2026 11:02:13 -0300 Subject: [PATCH 27/46] Copilot comments --- functional-test/README.md | 2 +- .../ansible/ansible/AnsibleRunner.java | 16 +++++++--- .../ansible/AnsibleRunnerContextBuilder.java | 31 ++++++++++++------- .../ansible/ansible/AnsibleRunnerSpec.groovy | 2 +- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/functional-test/README.md b/functional-test/README.md index 8cdb771c..bedac1f1 100644 --- a/functional-test/README.md +++ b/functional-test/README.md @@ -173,7 +173,7 @@ The multi-node authentication feature allows running Ansible playbooks across mu 4. Ansible uses host-specific credentials from group_vars **Requirements:** -- Must use Ansible Playbook **Workflow Steps** (not Node Steps) +- Must use Ansible Playbook **Workflow Steps** (not Node Steps); the multi-node authentication logic and inventory generation are only implemented for the Workflow Step variant of the plugin and are not executed for Node Steps, so enabling `project.ansible-generate-inventory-nodes-auth` while using Node Steps will not apply per-node credentials. This limitation exists because Node Steps run independently on each target node and do not share the global inventory context that the multi-node authentication feature relies on. - Node attributes must include `ansible-ssh-password-storage-path` for each node - Passwords are automatically encrypted using Ansible Vault - Supports special characters with proper YAML escaping diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 0e70334e..f27f72b3 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -516,7 +516,11 @@ public int run() throws Exception { // Create all.yaml in group_vars directory tempNodeAuthFile = new File(groupVarsDir, "all.yaml"); - java.nio.file.Files.writeString(tempNodeAuthFile.toPath(), yamlContent); + java.nio.file.Files.writeString( + tempNodeAuthFile.toPath(), + yamlContent, + java.nio.charset.StandardCharsets.UTF_8 + ); if(debug) { System.err.println("DEBUG: Writing all.yaml to: " + tempNodeAuthFile.getAbsolutePath()); @@ -539,13 +543,15 @@ public int run() throws Exception { } //set extra vars to resolve the host specific authentication - procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] }}\""); + if (!hostUsers.isEmpty()) { + procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] }}\""); + } if(!hostPasswords.isEmpty()){ - procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] }}\""); + procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] | default(omit) }}\""); } if(!hostKeys.isEmpty()){ - procArgs.add("-e ansible_ssh_private_key_file=\"{{ host_private_keys[inventory_hostname] }}\""); + procArgs.add("-e ansible_ssh_private_key_file=\"{{ host_private_keys[inventory_hostname] | default(omit) }}\""); } } catch (IOException e) { @@ -990,7 +996,7 @@ String buildGroupVarsYaml(Map hostPasswords, Map } } else { // Single line value (shouldn't happen with vault, but handle it) - System.err.println("DEBUG: Single line vault value for host: " + originalKey); + log.debug("Single line vault value for host: {}", originalKey); yamlContent.append(vaultValue).append("\n"); } } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 3faf2304..f1f4803b 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -235,7 +235,7 @@ private String readFromStoragePath(String storagePath, String resourceType) thro .getContents(); ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); contents.writeContent(byteArrayOutputStream); - return byteArrayOutputStream.toString("UTF-8"); + return byteArrayOutputStream.toString(java.nio.charset.StandardCharsets.UTF_8); } catch (StorageException | IOException e) { throw new ConfigurationException("Failed to read the " + resourceType + " for " + "storage path: " + storagePath + ": " + e.getMessage()); @@ -805,19 +805,21 @@ public INodeEntry getNode() { public void cleanupTempFiles() { // Clean up individual temp files for (File temp : tempFiles) { - log.debug("Attempting to delete temp file: {}", temp.getAbsolutePath()); - if (!temp.delete()) { - log.warn("Failed to delete temp file: {}. Marking for deletion on JVM exit.", temp.getAbsolutePath()); - // Fallback: schedule for deletion on JVM exit - temp.deleteOnExit(); - } else { - log.debug("Successfully deleted temp file: {}", temp.getAbsolutePath()); + if (!getDebug()) { + log.debug("Attempting to delete temp file: {}", temp.getAbsolutePath()); + if (!temp.delete()) { + log.warn("Failed to delete temp file: {}. Marking for deletion on JVM exit.", temp.getAbsolutePath()); + // Fallback: schedule for deletion on JVM exit + temp.deleteOnExit(); + } else { + log.debug("Successfully deleted temp file: {}", temp.getAbsolutePath()); + } } } tempFiles.clear(); // Clean up execution-specific directory (including group_vars) - if (executionSpecificDir != null && executionSpecificDir.exists()) { + if (!getDebug() && executionSpecificDir != null && executionSpecificDir.exists()) { log.debug("Cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); if (!deleteDirectoryRecursively(executionSpecificDir)) { log.warn("Failed to completely delete execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); @@ -1011,8 +1013,8 @@ public Map> getNodesAuthenticationMap(){ try { privateKey = getSshPrivateKey(node); if (getDebug()) { - System.err.println("DEBUG: Successfully retrieved private key for node '" + - node.getNodename() + "'"); + System.err.println("DEBUG: Retrieved private key for node '" + + node.getNodename() + "': " + (privateKey != null ? ("yes, length=" + privateKey.length()) : "null")); } } catch (ConfigurationException e) { if (getDebug()) { @@ -1024,6 +1026,13 @@ public Map> getNodesAuthenticationMap(){ } if (privateKey != null) { auth.put("ansible_ssh_private_key", privateKey); + if (getDebug()) { + System.err.println("DEBUG: Added private key to auth map for node '" + node.getNodename() + "'"); + } + } else { + if (getDebug()) { + System.err.println("DEBUG: Private key is null for node '" + node.getNodename() + "', not adding to auth map"); + } } } else if (AnsibleDescribable.AuthenticationType.password == authType) { try { diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index 6581448b..c66a84e7 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -620,7 +620,7 @@ class AnsibleRunnerSpec extends Specification{ "-starts-with-dash" || "\"-starts-with-dash\"" "?starts-with-q" || "\"?starts-with-q\"" "123numeric" || "\"123numeric\"" - "key with spaces" || "key with spaces" // spaces are handled differently + "key with spaces" || "key with spaces" // spaces are valid in unquoted YAML keys, so we intentionally do not quote them } def "escapeYamlValue: should quote values with special characters"() { From 65ab770f600f04a7c5e26d04c2ff25c980f4b037 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Thu, 15 Jan 2026 19:18:07 -0300 Subject: [PATCH 28/46] enable again the send node keys to runner in node-executor --- .../plugins/ansible/plugin/AnsibleNodeExecutor.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java index 3b1b7165..e75ae84e 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleNodeExecutor.java @@ -200,7 +200,13 @@ public List listSecretsPath(ExecutionContext context, INodeEntry node) { jobConf.put(AnsibleDescribable.ANSIBLE_LIMIT,node.getNodename()); AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(node, context, context.getFramework(), jobConf); - return AnsibleUtil.getSecretsPath(builder); + List secretPaths = AnsibleUtil.getSecretsPath(builder); + List secretPathsNodes = builder.getListNodesKeyPath(); + + if(secretPathsNodes != null && !secretPathsNodes.isEmpty()){ + secretPaths.addAll(secretPathsNodes); + } + return secretPaths; } } From f224449f818d5f600f75f5f9b87f7502d9b3e17c Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 15 Jan 2026 19:20:12 -0300 Subject: [PATCH 29/46] copilot comments --- .../test/resources/docker/docker-compose.yml | 2 +- .../ansible/ansible/AnsibleDescribable.java | 4 +- .../ansible/ansible/AnsibleRunner.java | 15 +++---- .../ansible/AnsibleRunnerContextBuilder.java | 39 ++++++++++++------- .../AnsiblePlaybookInlineWorkflowStep.java | 3 +- .../plugin/AnsiblePlaybookWorkflowStep.java | 3 +- .../ansible/ansible/AnsibleRunnerSpec.groovy | 2 +- 7 files changed, 40 insertions(+), 28 deletions(-) diff --git a/functional-test/src/test/resources/docker/docker-compose.yml b/functional-test/src/test/resources/docker/docker-compose.yml index 05bf8951..f8d16f37 100644 --- a/functional-test/src/test/resources/docker/docker-compose.yml +++ b/functional-test/src/test/resources/docker/docker-compose.yml @@ -29,7 +29,7 @@ services: build: context: node environment: - NODE_USER_PASSWORD: password3"quote'test + NODE_USER_PASSWORD: 'password3"quote''test' networks: - rundeck ports: diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java index 4b268e5c..646fb7be 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java @@ -237,8 +237,8 @@ public static String[] getValues() { static final Property GENERATE_INVENTORY_NODES_AUTH = PropertyBuilder.builder() .booleanType(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH) .required(false) - .title("Workflow Step: Generate inventory, pass node authentication from rundeck nodes") - .description("Pass authentication from rundeck nodes. Only applies to workflow steps.") + .title("Workflow Step: Generate Inventory and Pass Node Authentication from Rundeck Nodes") + .description("Pass authentication credentials from Rundeck nodes. IMPORTANT: Only applies to Ansible Playbook Workflow Steps. This feature does NOT work with Node Steps because they run independently per node and do not share the global inventory context.") .build(); public static Property EXECUTABLE_PROP = PropertyUtil.freeSelect( diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index f27f72b3..5fa0ccff 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -466,10 +466,9 @@ public int run() throws Exception { } tempHostPkFile = AnsibleUtil.createTemporaryFile("","id_rsa_node_"+safeNodeName, privateKey,customTmpDirPath); - // Only the owner can read and write + // Only the owner can read (SSH private key best practice: 0400) Set perms = new HashSet(); perms.add(PosixFilePermission.OWNER_READ); - perms.add(PosixFilePermission.OWNER_WRITE); Files.setPosixFilePermissions(tempHostPkFile.toPath(), perms); hostKeys.put(nodeName, tempHostPkFile.getAbsolutePath()); @@ -544,7 +543,7 @@ public int run() throws Exception { //set extra vars to resolve the host specific authentication if (!hostUsers.isEmpty()) { - procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] }}\""); + procArgs.add("-e ansible_user=\"{{ host_users[inventory_hostname] | default(omit) }}\""); } if(!hostPasswords.isEmpty()){ procArgs.add("-e ansible_password=\"{{ host_passwords[inventory_hostname] | default(omit) }}\""); @@ -597,10 +596,9 @@ public int run() throws Exception { String privateKeyData = sshPrivateKey.replaceAll("\r\n", "\n"); tempPkFile = AnsibleUtil.createTemporaryFile("","id_rsa", privateKeyData,customTmpDirPath); - // Only the owner can read and write + // Only the owner can read (SSH private key best practice: 0400) Set perms = new HashSet(); perms.add(PosixFilePermission.OWNER_READ); - perms.add(PosixFilePermission.OWNER_WRITE); Files.setPosixFilePermissions(tempPkFile.toPath(), perms); if (sshUseAgent) { @@ -838,6 +836,9 @@ public int run() throws Exception { tempNodeAuthFile.deleteOnExit(); } + // Clean up group_vars directory if it was created alongside user-provided inventory + // Note: When inventory is generated/inline, group_vars is inside executionSpecificDir + // and is cleaned up by AnsibleRunnerContextBuilder.cleanupTempFiles() if (groupVarsDir != null && groupVarsDir.exists()) { log.debug("Attempting to delete group_vars directory: {}", groupVarsDir.getAbsolutePath()); try { @@ -1134,9 +1135,9 @@ boolean isValidVaultFormat(String vaultValue) { return false; } - // Check if it follows the "!vault |" or "!vault" format + // Check if it follows the "!vault |" format (pipe is required by Ansible) String firstLine = vaultValue.split("\n", 2)[0]; - if (!firstLine.trim().matches("!vault\\s*\\|?")) { + if (!firstLine.trim().matches("!vault\\s*\\|")) { return false; } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index f1f4803b..33ffd494 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -216,9 +216,14 @@ public String getPasswordFromPath(String storagePath) throws ConfigurationExcept } /** - * Helper method to read content from Rundeck storage path. + * Helper method to read text-based content from Rundeck storage path. * Extracted to reduce duplication between password and private key reading. * + * IMPORTANT: This method converts storage content to UTF-8 string and is intended + * ONLY for text-based secrets (passwords, SSH private keys in PEM format, etc.). + * Do NOT use this method for binary data as UTF-8 conversion will cause data corruption. + * For binary data, use loadStoragePathData() which returns byte[] instead. + * * @param storagePath The storage path to read from * @param resourceType Description of resource type for error messages (e.g., "ssh password", "ssh private key") * @return The content as a UTF-8 string, or null if storagePath is null @@ -818,7 +823,8 @@ public void cleanupTempFiles() { } tempFiles.clear(); - // Clean up execution-specific directory (including group_vars) + // Clean up execution-specific directory (including group_vars when inventory was generated/inline) + // Note: group_vars created alongside user-provided inventory files is cleaned up by AnsibleRunner if (!getDebug() && executionSpecificDir != null && executionSpecificDir.exists()) { log.debug("Cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); if (!deleteDirectoryRecursively(executionSpecificDir)) { @@ -984,16 +990,19 @@ public Map getListOptions(){ return options; } - /* - Returns a map of node names to their respective authentication details. - Each entry in the outer map corresponds to a node, with the key being the node name - and the value being another map containing authentication parameters such as - - "ansible_ssh_private_key" or "ansible_password". - [ "node1": { "ansible_ssh_private_key": "...", "ansible_user": "..." }, - "node2": { "ansible_password": "...", "ansible_user": "..." } - ] - + /** + * Returns a map of node names to their respective authentication details. + * Each entry in the outer map corresponds to a node, with the key being the node name + * and the value being another map containing authentication parameters such as + * "ansible_ssh_private_key" or "ansible_password". + * + * @return Map of node names to authentication details. Example: + * 
+     * {
+     *   "node1": { "ansible_ssh_private_key": "...", "ansible_user": "..." },
+     *   "node2": { "ansible_password": "...", "ansible_user": "..." }
+     * }
+     *
*/ public Map> getNodesAuthenticationMap(){ @@ -1195,7 +1204,11 @@ String getExecutionSpecificTmpDir() { executionSpecificDir = new File(baseTmpDir, "ansible-exec-" + executionId); if (!executionSpecificDir.exists()) { log.debug("Creating execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); - // Check return value to handle race conditions and creation failures + // Race condition handling: mkdirs() returns false if the directory already exists + // or if creation failed. If another thread/process created the directory between + // our exists() check and mkdirs() call, mkdirs() returns false but the directory + // now exists. We use double-check pattern: only throw if mkdirs() failed AND + // directory still doesn't exist (true creation failure). boolean created = executionSpecificDir.mkdirs(); if (!created && !executionSpecificDir.exists()) { // Failed to create and directory still doesn't exist - this is an error diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index 63bc9dc8..01cc5fc9 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -105,7 +105,6 @@ public void executeStep(PluginStepContext context, Map configura configuration, pluginGroup); - try { runner = AnsibleRunner.buildAnsibleRunner(contextBuilder); @@ -162,7 +161,7 @@ public Description getDescription() { @Override public List listSecretsPathWorkflowStep(ExecutionContext context, Map configuration) { - AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(context, context.getFramework(), context.getNodes(), configuration); + AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(context, context.getFramework(), context.getNodes(), configuration, pluginGroup); return AnsibleUtil.getSecretsPathWorkflowSteps(builder); } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index c0d294ba..f193d869 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -66,7 +66,6 @@ public class AnsiblePlaybookWorkflowStep implements StepPlugin, AnsibleDescribab builder.property(BECOME_PASSWORD_STORAGE_PROP); builder.property(DISABLE_LIMIT_PROP); - DESC = builder.build(); } @@ -156,7 +155,7 @@ public Description getDescription() { @Override public List listSecretsPathWorkflowStep(ExecutionContext context, Map configuration) { - AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(context, context.getFramework(), context.getNodes(), configuration); + AnsibleRunnerContextBuilder builder = new AnsibleRunnerContextBuilder(context, context.getFramework(), context.getNodes(), configuration, pluginGroup); return AnsibleUtil.getSecretsPathWorkflowSteps(builder); } @Override diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index c66a84e7..554a0cd1 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -666,7 +666,7 @@ class AnsibleRunnerSpec extends Specification{ vaultValue || expectedResult "!vault |\n encryptedcontent" || true "!vault |\n line1\n line2" || true - "!vault\n content" || true // without pipe + "!vault\n content" || false // missing pipe - Ansible Vault always requires "|" for literal block style "not a vault" || false null || false "" || false From 19ad6624a48b35ed9c3f9e4a0fa1f878599f861d Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Thu, 15 Jan 2026 22:29:26 -0300 Subject: [PATCH 30/46] copilot comments --- functional-test/build.gradle | 7 +++ .../functional/MultiNodeAuthSpec.groovy | 2 +- .../ansible/ansible/AnsibleDescribable.java | 6 +++ .../ansible/ansible/AnsibleRunner.java | 35 ++++++++++---- .../ansible/AnsibleRunnerContextBuilder.java | 47 ++++++++++--------- .../ansible/ansible/AnsibleRunnerSpec.groovy | 36 +++++++++++--- 6 files changed, 95 insertions(+), 38 deletions(-) diff --git a/functional-test/build.gradle b/functional-test/build.gradle index 188c4e88..3cba6635 100644 --- a/functional-test/build.gradle +++ b/functional-test/build.gradle @@ -42,6 +42,13 @@ dependencies { tasks.register('functionalTest', Test) { useJUnitPlatform() + + // Rundeck test image version + // NOTE: Using Rundeck 5.18.0 for functional testing. This version upgrade from the + // previously used 5.1.1 ensures compatibility with newer Rundeck releases and + // validates that the multi-node authentication feature works correctly with + // current Rundeck APIs and behavior. If the plugin's minimum supported Rundeck + // version changes, update both this test image and the plugin documentation. systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.18.0") // Docker configuration for Testcontainers diff --git a/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy b/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy index 0b4b7b0f..a9f4e787 100644 --- a/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy +++ b/functional-test/src/test/groovy/functional/MultiNodeAuthSpec.groovy @@ -17,7 +17,7 @@ class MultiNodeAuthSpec extends BaseTestConfiguration { // Test passwords for each node (matching docker-compose.yml) static String NODE1_PASSWORD = "testpassword123" static String NODE2_PASSWORD = "password2_special!@#" - static String NODE3_PASSWORD = 'password3"quote\'test' + static String NODE3_PASSWORD = 'password3"quote\'test' // Expected password value: password3"quote'test // Private key for node 4 static String NODE4_PRIVATE_KEY_PATH = "src/test/resources/docker/keys/id_rsa" diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java index 646fb7be..26e5e1ae 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java @@ -100,6 +100,12 @@ public static String[] getValues() { public static final String ANSIBLE_INVENTORY_INLINE = "ansible-inventory-inline"; public static final String ANSIBLE_INVENTORY = "ansible-inventory"; public static final String ANSIBLE_GENERATE_INVENTORY = "ansible-generate-inventory"; + /** + * Controls whether node authentication data is included when generating the inventory. + * This is an enhancement of {@link #ANSIBLE_GENERATE_INVENTORY} and only has effect + * when inventory generation is enabled. It is exposed as a separate top-level + * property for configuration/UI clarity. + */ public static final String ANSIBLE_GENERATE_INVENTORY_NODES_AUTH = "ansible-generate-inventory-nodes-auth"; public static final String ANSIBLE_MODULE = "ansible-module"; public static final String ANSIBLE_MODULE_ARGS = "ansible-module-args"; diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 5fa0ccff..4afef2f5 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -461,6 +461,10 @@ public int run() throws Exception { try { // Sanitize node name for filesystem use (replace unsafe characters with underscores) String safeNodeName = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_"); + // Prevent hidden files (starting with .) and problematic names (empty or all dots) + if (safeNodeName.isEmpty() || safeNodeName.startsWith(".") || safeNodeName.matches("^\\.+$")) { + safeNodeName = "_" + safeNodeName; + } if(debug && !nodeName.equals(safeNodeName)) { System.err.println("DEBUG: Sanitized node name '" + nodeName + "' to '" + safeNodeName + "' for temp file"); } @@ -492,6 +496,11 @@ public int run() throws Exception { try { // Create group_vars directory structure + // IMPORTANT: Following Ansible convention, group_vars must be created in the same + // directory as the inventory file for Ansible to find it. While this means writing + // to user-specified locations, the passwords are vault-encrypted making them safe + // for storage. Administrators should ensure inventory directories have appropriate + // filesystem permissions to prevent unauthorized access. File inventoryFile = new File(inventory); File inventoryParentDir = inventoryFile.getParentFile(); @@ -507,10 +516,10 @@ public int run() throws Exception { System.err.println("DEBUG: group_vars directory path: " + groupVarsDir.getAbsolutePath()); } - if (!groupVarsDir.exists()) { - if (!groupVarsDir.mkdirs()) { - throw new RuntimeException("Failed to create group_vars directory at: " + groupVarsDir.getAbsolutePath()); - } + try { + Files.createDirectories(groupVarsDir.toPath()); + } catch (IOException e) { + throw new RuntimeException("Failed to create group_vars directory at: " + groupVarsDir.getAbsolutePath(), e); } // Create all.yaml in group_vars directory @@ -596,7 +605,10 @@ public int run() throws Exception { String privateKeyData = sshPrivateKey.replaceAll("\r\n", "\n"); tempPkFile = AnsibleUtil.createTemporaryFile("","id_rsa", privateKeyData,customTmpDirPath); - // Only the owner can read (SSH private key best practice: 0400) + // Set SSH private key permissions to 0400 (owner read-only). + // This is a security best practice: private keys should never be writable after creation. + // SSH itself will warn or refuse to use keys with overly permissive permissions (e.g., 0600). + // This temporary file is immutable and will be deleted after use. Set perms = new HashSet(); perms.add(PosixFilePermission.OWNER_READ); Files.setPosixFilePermissions(tempPkFile.toPath(), perms); @@ -1093,14 +1105,19 @@ String escapeYamlValue(String value) { } /** - * Escapes a password value for safe use in YAML. - * Always wraps the password in quotes and escapes special characters to prevent YAML parsing errors. + * Escapes a password value for safe use inside double-quoted YAML strings. + * Note: This method does NOT add quotes - the caller must wrap the result in double quotes. + * This differs from escapeYamlKey() and escapeYamlValue() which conditionally add quotes based on content. + * + * Why the different approach: + * - Passwords are passed as Ansible extra vars (e.g., -e 'ansible_password: "..."') and are ALWAYS quoted + * - YAML keys/values in group_vars files should only be quoted when necessary for cleaner output * * @param password The password to escape - * @return Escaped and quoted password safe for YAML + * @return Escaped password safe for use inside double-quoted YAML strings (quotes not included) */ String escapePasswordForYaml(String password) { - // Escape special characters in the password to prevent YAML parsing errors + // Escape special characters for use inside double-quoted strings return password .replace("\\", "\\\\") .replace("\"", "\\\"") diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 33ffd494..36b698a8 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -819,18 +819,27 @@ public void cleanupTempFiles() { } else { log.debug("Successfully deleted temp file: {}", temp.getAbsolutePath()); } + } else { + // In debug mode, temp files are intentionally preserved for inspection + log.debug("Debug mode enabled; not deleting temp file: {}", temp.getAbsolutePath()); } } tempFiles.clear(); // Clean up execution-specific directory (including group_vars when inventory was generated/inline) // Note: group_vars created alongside user-provided inventory files is cleaned up by AnsibleRunner - if (!getDebug() && executionSpecificDir != null && executionSpecificDir.exists()) { - log.debug("Cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); - if (!deleteDirectoryRecursively(executionSpecificDir)) { - log.warn("Failed to completely delete execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + if (executionSpecificDir != null && executionSpecificDir.exists()) { + if (!getDebug()) { + log.debug("Cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + if (!deleteDirectoryRecursively(executionSpecificDir)) { + log.warn("Failed to completely delete execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + } else { + log.debug("Successfully deleted execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + } } else { - log.debug("Successfully deleted execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + // In debug mode, the execution-specific directory is intentionally preserved for troubleshooting. + // Note: These directories will accumulate over time and may require periodic manual cleanup. + log.debug("Debug mode enabled; not cleaning up execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); } } } @@ -1202,23 +1211,17 @@ String getExecutionSpecificTmpDir() { // Create execution-specific directory if (executionId != null && !executionId.isEmpty()) { executionSpecificDir = new File(baseTmpDir, "ansible-exec-" + executionId); - if (!executionSpecificDir.exists()) { - log.debug("Creating execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); - // Race condition handling: mkdirs() returns false if the directory already exists - // or if creation failed. If another thread/process created the directory between - // our exists() check and mkdirs() call, mkdirs() returns false but the directory - // now exists. We use double-check pattern: only throw if mkdirs() failed AND - // directory still doesn't exist (true creation failure). - boolean created = executionSpecificDir.mkdirs(); - if (!created && !executionSpecificDir.exists()) { - // Failed to create and directory still doesn't exist - this is an error - String errorMsg = "Failed to create execution-specific directory: " + executionSpecificDir.getAbsolutePath(); - log.error(errorMsg); - throw new IllegalStateException(errorMsg); - } - log.debug("Successfully created execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); - } else { - log.debug("Execution-specific directory already exists: {}", executionSpecificDir.getAbsolutePath()); + log.debug("Creating execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); + try { + // Use Files.createDirectories() which is idempotent (safe to call if directory exists) + // and handles race conditions properly. Unlike mkdirs(), it doesn't return false + // when the directory already exists - it only throws IOException on actual failure. + Files.createDirectories(executionSpecificDir.toPath()); + log.debug("Successfully ensured execution-specific directory exists: {}", executionSpecificDir.getAbsolutePath()); + } catch (IOException e) { + String errorMsg = "Failed to create execution-specific directory: " + executionSpecificDir.getAbsolutePath(); + log.error(errorMsg, e); + throw new IllegalStateException(errorMsg, e); } return executionSpecificDir.getAbsolutePath(); } diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index 554a0cd1..d3b95bf4 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -620,7 +620,7 @@ class AnsibleRunnerSpec extends Specification{ "-starts-with-dash" || "\"-starts-with-dash\"" "?starts-with-q" || "\"?starts-with-q\"" "123numeric" || "\"123numeric\"" - "key with spaces" || "key with spaces" // spaces are valid in unquoted YAML keys, so we intentionally do not quote them + "key with spaces" || "key with spaces" // internal spaces are valid in unquoted YAML keys; node names with leading/trailing spaces are not supported } def "escapeYamlValue: should quote values with special characters"() { @@ -761,7 +761,7 @@ class AnsibleRunnerSpec extends Specification{ !yaml.contains("host_passwords:") // Should not appear when empty !yaml.contains("host_users:") // Should not appear when empty !yaml.contains("host_private_keys:") // Should not appear when empty - yaml.isEmpty() || yaml.trim().isEmpty() // Should produce empty or whitespace-only output + yaml.isEmpty() // Should produce empty output when all host lists are empty } def "escapePasswordForYaml: should escape special characters"() { @@ -1009,6 +1009,10 @@ class AnsibleRunnerSpec extends Specification{ // Test case from real issue: node name with forward slashes String nodeName = "/docker-runner-ansible-ssh-node-b-3" String sanitized = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") + // Prevent hidden files (starting with .) and problematic names (empty or all dots) + if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { + sanitized = "_" + sanitized + } expect: sanitized == "_docker-runner-ansible-ssh-node-b-3" @@ -1017,8 +1021,15 @@ class AnsibleRunnerSpec extends Specification{ } def "node name sanitization: should sanitize various special characters"() { + given: + String sanitized = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") + // Prevent hidden files (starting with .) and problematic names (empty or all dots) + if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { + sanitized = "_" + sanitized + } + expect: - nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") == expected + sanitized == expected where: nodeName || expected @@ -1041,24 +1052,37 @@ class AnsibleRunnerSpec extends Specification{ given: String safeName = "my-node_123.server-A" String sanitized = safeName.replaceAll("[^a-zA-Z0-9._-]", "_") + // Prevent hidden files (starting with .) and problematic names (empty or all dots) + if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { + sanitized = "_" + sanitized + } expect: sanitized == safeName // Should not be changed } def "node name sanitization: should handle empty and edge case node names"() { + given: + String sanitized = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") + // Prevent hidden files (starting with .) and problematic names (empty or all dots) + if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { + sanitized = "_" + sanitized + } + expect: - nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") == expected + sanitized == expected where: nodeName || expected - "" || "" + "" || "_" // empty -> prepend underscore "123" || "123" - "..." || "..." + "..." || "_..." // all dots -> prepend underscore to avoid filesystem issues "---" || "---" "___" || "___" "a" || "a" "/////" || "_____" + ".hidden" || "_.hidden" // starts with dot -> prepend underscore to prevent hidden files + ".config" || "_.config" // starts with dot -> prepend underscore to prevent hidden files } } From 636cede3f06d11d3d0c049c26ffecc3d270ead19 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 01:05:33 -0300 Subject: [PATCH 31/46] copilot comments --- functional-test/build.gradle | 11 ++-- .../ansible/ansible/AnsibleDescribable.java | 2 +- .../ansible/ansible/AnsibleRunner.java | 56 ++++++++++++++++--- .../ansible/AnsibleRunnerContextBuilder.java | 2 +- .../plugin/AnsibleModuleWorkflowStep.java | 6 +- ...AnsiblePlaybookInlineWorkflowNodeStep.java | 6 +- .../AnsiblePlaybookInlineWorkflowStep.java | 6 +- .../AnsiblePlaybookWorflowNodeStep.java | 6 +- .../plugin/AnsiblePlaybookWorkflowStep.java | 6 +- .../plugins/ansible/util/AnsibleUtil.java | 26 +++++++++ .../ansible/ansible/AnsibleRunnerSpec.groovy | 28 +++------- 11 files changed, 95 insertions(+), 60 deletions(-) diff --git a/functional-test/build.gradle b/functional-test/build.gradle index 3cba6635..419b4f6b 100644 --- a/functional-test/build.gradle +++ b/functional-test/build.gradle @@ -44,11 +44,12 @@ tasks.register('functionalTest', Test) { useJUnitPlatform() // Rundeck test image version - // NOTE: Using Rundeck 5.18.0 for functional testing. This version upgrade from the - // previously used 5.1.1 ensures compatibility with newer Rundeck releases and - // validates that the multi-node authentication feature works correctly with - // current Rundeck APIs and behavior. If the plugin's minimum supported Rundeck - // version changes, update both this test image and the plugin documentation. + // NOTE: Functional tests run against Rundeck 5.18.0. This is an upgrade from the + // previously used 5.1.1 to validate compatibility with newer Rundeck releases and + // ensure the multi-node authentication feature works correctly with current + // Rundeck APIs and behavior. The plugin's minimum supported Rundeck version is + // defined in the main README/changelog; updating that minimum version should be + // accompanied by updating this test image tag accordingly. systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.18.0") // Docker configuration for Testcontainers diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java index 26e5e1ae..7fff5b83 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleDescribable.java @@ -244,7 +244,7 @@ public static String[] getValues() { .booleanType(ANSIBLE_GENERATE_INVENTORY_NODES_AUTH) .required(false) .title("Workflow Step: Generate Inventory and Pass Node Authentication from Rundeck Nodes") - .description("Pass authentication credentials from Rundeck nodes. IMPORTANT: Only applies to Ansible Playbook Workflow Steps. This feature does NOT work with Node Steps because they run independently per node and do not share the global inventory context.") + .description("Pass authentication credentials from Rundeck nodes for Ansible Playbook Workflow Steps only (not supported for Node Steps). See the Ansible plugin documentation for details.") .build(); public static Property EXECUTABLE_PROP = PropertyUtil.freeSelect( diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 4afef2f5..14ec8a0f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -301,6 +301,7 @@ public static AnsibleRunner buildAnsibleRunner(AnsibleRunnerContextBuilder conte File vaultPromptFile; File tempNodeAuthFile; File groupVarsDir; + List tempNodePrivateKeyFiles; String customTmpDirPath; @@ -433,6 +434,9 @@ public int run() throws Exception { Map hostPasswords = new LinkedHashMap<>(); Map hostKeys = new LinkedHashMap<>(); + // Track node-specific private key files for explicit cleanup + tempNodePrivateKeyFiles = new ArrayList<>(); + nodesAuthentication.forEach((nodeName, authValues) -> { String user = authValues.get("ansible_user"); String password = authValues.get("ansible_password"); @@ -459,12 +463,8 @@ public int run() throws Exception { //create temporary file for private key File tempHostPkFile; try { - // Sanitize node name for filesystem use (replace unsafe characters with underscores) - String safeNodeName = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_"); - // Prevent hidden files (starting with .) and problematic names (empty or all dots) - if (safeNodeName.isEmpty() || safeNodeName.startsWith(".") || safeNodeName.matches("^\\.+$")) { - safeNodeName = "_" + safeNodeName; - } + // Sanitize node name for filesystem use + String safeNodeName = sanitizeNodeNameForFilesystem(nodeName); if(debug && !nodeName.equals(safeNodeName)) { System.err.println("DEBUG: Sanitized node name '" + nodeName + "' to '" + safeNodeName + "' for temp file"); } @@ -477,7 +477,8 @@ public int run() throws Exception { hostKeys.put(nodeName, tempHostPkFile.getAbsolutePath()); - tempHostPkFile.deleteOnExit(); + // Track for explicit cleanup to minimize exposure time of sensitive credentials + tempNodePrivateKeyFiles.add(tempHostPkFile); } catch (IOException e) { throw new RuntimeException("Failed to create temporary private key file for node '" + nodeName + "': " + e.getMessage(), e); @@ -517,6 +518,9 @@ public int run() throws Exception { } try { + // Use Files.createDirectories() which is idempotent (safe to call if directory exists) + // and handles race conditions properly. Unlike mkdirs(), it doesn't return false + // when the directory already exists - it only throws IOException on actual failure. Files.createDirectories(groupVarsDir.toPath()); } catch (IOException e) { throw new RuntimeException("Failed to create group_vars directory at: " + groupVarsDir.getAbsolutePath(), e); @@ -530,6 +534,12 @@ public int run() throws Exception { java.nio.charset.StandardCharsets.UTF_8 ); + // Alert administrators that sensitive files are being created in their inventory directory + log.warn("Writing vault-encrypted authentication data to user-provided inventory location: {}. " + + "Administrators should ensure this directory has appropriate filesystem permissions. " + + "This file will be cleaned up after execution, but may persist if cleanup fails.", + tempNodeAuthFile.getAbsolutePath()); + if(debug) { System.err.println("DEBUG: Writing all.yaml to: " + tempNodeAuthFile.getAbsolutePath()); System.err.println("DEBUG: all.yaml written successfully, file size: " + tempNodeAuthFile.length() + " bytes"); @@ -608,7 +618,9 @@ public int run() throws Exception { // Set SSH private key permissions to 0400 (owner read-only). // This is a security best practice: private keys should never be writable after creation. // SSH itself will warn or refuse to use keys with overly permissive permissions (e.g., 0600). - // This temporary file is immutable and will be deleted after use. + // Write permission is unnecessary since this temporary file is created once, read by SSH, + // and never modified. The file will be deleted after use. This matches the permissions + // used for node-specific private keys (lines 469-472). Set perms = new HashSet(); perms.add(PosixFilePermission.OWNER_READ); Files.setPosixFilePermissions(tempPkFile.toPath(), perms); @@ -848,6 +860,15 @@ public int run() throws Exception { tempNodeAuthFile.deleteOnExit(); } + // Clean up node-specific private key files to minimize exposure time of sensitive credentials + if (tempNodePrivateKeyFiles != null) { + for (File nodeKeyFile : tempNodePrivateKeyFiles) { + if (nodeKeyFile != null && !nodeKeyFile.delete()) { + nodeKeyFile.deleteOnExit(); + } + } + } + // Clean up group_vars directory if it was created alongside user-provided inventory // Note: When inventory is generated/inline, group_vars is inside executionSpecificDir // and is cleaned up by AnsibleRunnerContextBuilder.cleanupTempFiles() @@ -1246,6 +1267,25 @@ public String encryptExtraVarsKey(String extraVars) throws Exception { return stringBuilder.toString(); } + /** + * Sanitizes a node name to be safe for use in filesystem paths. + * Replaces unsafe characters with underscores and prevents hidden files and problematic names. + * + * @param nodeName The original node name + * @return A sanitized node name safe for use in file paths + */ + String sanitizeNodeNameForFilesystem(String nodeName) { + // Replace unsafe characters with underscores + String safeNodeName = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_"); + + // Prevent hidden files (starting with .) and problematic names (empty or all dots) + if (safeNodeName.isEmpty() || safeNodeName.startsWith(".") || safeNodeName.matches("^\\.+$")) { + safeNodeName = "_" + safeNodeName; + } + + return safeNodeName; + } + } diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 36b698a8..f39c335f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -165,7 +165,7 @@ public String getSshPrivateKey(INodeEntry node) throws ConfigurationException { final String path = getPrivateKeyfilePath(); if (path != null) { try { - return new String(Files.readAllBytes(Paths.get(path))); + return new String(Files.readAllBytes(Paths.get(path)), java.nio.charset.StandardCharsets.UTF_8); } catch (IOException e) { throw new ConfigurationException("Failed to read the ssh private key from path " + path + ": " + e.getMessage()); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java index 706f4369..18acc085 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsibleModuleWorkflowStep.java @@ -72,11 +72,7 @@ public void executeStep(PluginStepContext context, Map configura } // set log level - String loglevel = null; - if (context.getDataContext() != null && - context.getDataContext().get("job") != null) { - loglevel = context.getDataContext().get("job").get("loglevel"); - } + String loglevel = AnsibleUtil.getJobLogLevel(context); if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "True"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java index c95a6e2f..b633cf91 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowNodeStep.java @@ -79,11 +79,7 @@ public void executeNodeStep( configuration.put(AnsibleDescribable.ANSIBLE_LIMIT,entry.getNodename()); // set log level - String loglevel = null; - if (context.getDataContext() != null && - context.getDataContext().get("job") != null) { - loglevel = context.getDataContext().get("job").get("loglevel"); - } + String loglevel = AnsibleUtil.getJobLogLevel(context); if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG,"True"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java index 01cc5fc9..96c1ab9f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookInlineWorkflowStep.java @@ -87,11 +87,7 @@ public void executeStep(PluginStepContext context, Map configura configuration.put(AnsibleDescribable.ANSIBLE_LIMIT, limit); } // set log level - String loglevel = null; - if (context.getDataContext() != null && - context.getDataContext().get("job") != null) { - loglevel = context.getDataContext().get("job").get("loglevel"); - } + String loglevel = AnsibleUtil.getJobLogLevel(context); if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "True"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java index f44fa2ef..6a5dea27 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorflowNodeStep.java @@ -76,11 +76,7 @@ public void executeNodeStep( configuration.put(AnsibleDescribable.ANSIBLE_LIMIT,entry.getNodename()); // set log level - String loglevel = null; - if (context.getDataContext() != null && - context.getDataContext().get("job") != null) { - loglevel = context.getDataContext().get("job").get("loglevel"); - } + String loglevel = AnsibleUtil.getJobLogLevel(context); if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG,"True"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java index f193d869..4495c9e6 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/plugin/AnsiblePlaybookWorkflowStep.java @@ -85,11 +85,7 @@ public void executeStep(PluginStepContext context, Map configura configuration.put(AnsibleDescribable.ANSIBLE_LIMIT, limit); } // set log level - String loglevel = null; - if (context.getDataContext() != null && - context.getDataContext().get("job") != null) { - loglevel = context.getDataContext().get("job").get("loglevel"); - } + String loglevel = AnsibleUtil.getJobLogLevel(context); if ("DEBUG".equals(loglevel)) { configuration.put(AnsibleDescribable.ANSIBLE_DEBUG, "True"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java b/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java index 9be0accd..60f4baf7 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/util/AnsibleUtil.java @@ -140,5 +140,31 @@ public static String getCustomTmpPathDir(Framework framework){ return customTmpDir; } + /** + * Safely retrieves the loglevel from the execution context's data context. + * Returns null if the loglevel cannot be retrieved (e.g., if data context or job is null). + * + * @param context The execution context + * @return The loglevel string, or null if not available + */ + public static String getJobLogLevel(ExecutionContext context) { + if (context.getDataContext() != null && + context.getDataContext().get("job") != null) { + return context.getDataContext().get("job").get("loglevel"); + } + return null; + } + + /** + * Safely retrieves the loglevel from the plugin step context's data context. + * Returns null if the loglevel cannot be retrieved (e.g., if data context or job is null). + * + * @param context The plugin step context + * @return The loglevel string, or null if not available + */ + public static String getJobLogLevel(PluginStepContext context) { + return getJobLogLevel(context.getExecutionContext()); + } + } diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index d3b95bf4..df6575af 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -1007,12 +1007,9 @@ class AnsibleRunnerSpec extends Specification{ def "node name sanitization: should sanitize node names with forward slashes for temp files"() { given: // Test case from real issue: node name with forward slashes + def runner = AnsibleRunner.playbookInline("test").build() String nodeName = "/docker-runner-ansible-ssh-node-b-3" - String sanitized = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") - // Prevent hidden files (starting with .) and problematic names (empty or all dots) - if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { - sanitized = "_" + sanitized - } + String sanitized = runner.sanitizeNodeNameForFilesystem(nodeName) expect: sanitized == "_docker-runner-ansible-ssh-node-b-3" @@ -1022,11 +1019,8 @@ class AnsibleRunnerSpec extends Specification{ def "node name sanitization: should sanitize various special characters"() { given: - String sanitized = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") - // Prevent hidden files (starting with .) and problematic names (empty or all dots) - if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { - sanitized = "_" + sanitized - } + def runner = AnsibleRunner.playbookInline("test").build() + String sanitized = runner.sanitizeNodeNameForFilesystem(nodeName) expect: sanitized == expected @@ -1050,12 +1044,9 @@ class AnsibleRunnerSpec extends Specification{ def "node name sanitization: should preserve safe alphanumeric and allowed characters"() { given: + def runner = AnsibleRunner.playbookInline("test").build() String safeName = "my-node_123.server-A" - String sanitized = safeName.replaceAll("[^a-zA-Z0-9._-]", "_") - // Prevent hidden files (starting with .) and problematic names (empty or all dots) - if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { - sanitized = "_" + sanitized - } + String sanitized = runner.sanitizeNodeNameForFilesystem(safeName) expect: sanitized == safeName // Should not be changed @@ -1063,11 +1054,8 @@ class AnsibleRunnerSpec extends Specification{ def "node name sanitization: should handle empty and edge case node names"() { given: - String sanitized = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_") - // Prevent hidden files (starting with .) and problematic names (empty or all dots) - if (sanitized.isEmpty() || sanitized.startsWith(".") || sanitized.matches(/^\.\+$/)) { - sanitized = "_" + sanitized - } + def runner = AnsibleRunner.playbookInline("test").build() + String sanitized = runner.sanitizeNodeNameForFilesystem(nodeName) expect: sanitized == expected From 69f0b9dedf3f9af4dca6531ad1a2eeb8f249c9f0 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 09:22:44 -0300 Subject: [PATCH 32/46] Copilot comments --- .../docker/ansible-multi-node-auth/README.md | 17 +++++++++++------ .../plugins/ansible/ansible/AnsibleRunner.java | 4 ++-- .../ansible/AnsibleRunnerContextBuilder.java | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md b/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md index 384af8bf..05d5f88e 100644 --- a/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md @@ -1,6 +1,6 @@ # Multi-Node Authentication Functional Test -This test verifies the multi-node authentication feature where each node can have its own password stored in Rundeck's key storage. +This test verifies the multi-node authentication feature where each node can have its own credentials (password or private key) stored in Rundeck's key storage. ## Test Setup @@ -8,18 +8,20 @@ This test verifies the multi-node authentication feature where each node can hav - **ssh-node**: Standard password (`testpassword123`) - **ssh-node-2**: Password with special characters (`password2_special!@#`) - **ssh-node-3**: Password with quotes (`password3"quote'test`) +- **ssh-node-4**: Private key authentication (uses SSH key from key storage) ### Files -- `resources.xml`: Defines three nodes with node-specific password storage paths +- `resources.xml`: Defines four nodes with node-specific authentication (three with password storage paths, one with private key storage path) - `inventory.ini`: Ansible inventory file for the test nodes - `ansible.cfg`: Ansible configuration - `test-playbook.yml`: Test playbook for verification ## What's Being Tested -1. **Multi-node authentication with different passwords per node** - - Each node has a different password stored in Rundeck key storage - - The plugin generates `group_vars/all.yaml` with vault-encrypted passwords +1. **Multi-node authentication with different credentials per node** + - Three nodes have different passwords stored in Rundeck key storage + - One node uses private key authentication stored in Rundeck key storage + - The plugin generates `group_vars/all.yaml` with vault-encrypted passwords and private key paths - Each node authenticates with its specific credentials 2. **Password escaping for special characters** @@ -52,6 +54,9 @@ This test verifies the multi-node authentication feature where each node can hav ssh-node: rundeck ssh-node-2: rundeck ssh-node-3: rundeck + ssh-node-4: rundeck + host_private_keys: + ssh-node-4: /path/to/temporary/key ``` 6. Ansible uses host-specific credentials from group_vars -7. Each node authenticates successfully with its own password +7. Each node authenticates successfully with its own credentials (password or private key) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 14ec8a0f..fa48a965 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -423,7 +423,7 @@ public int run() throws Exception { host_passwords: host1: enc(password1) - host2: enc(password1) + host2: enc(password2) host_private_keys: host1: /path/to/private_key1 @@ -620,7 +620,7 @@ public int run() throws Exception { // SSH itself will warn or refuse to use keys with overly permissive permissions (e.g., 0600). // Write permission is unnecessary since this temporary file is created once, read by SSH, // and never modified. The file will be deleted after use. This matches the permissions - // used for node-specific private keys (lines 469-472). + // used for node-specific private keys (lines 474-476). Set perms = new HashSet(); perms.add(PosixFilePermission.OWNER_READ); Files.setPosixFilePermissions(tempPkFile.toPath(), perms); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index f39c335f..895ac862 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -1075,7 +1075,7 @@ public Map> getNodesAuthenticationMap(){ String userName = getSshNodeUser(node); if(null!=userName){ - auth.put("ansible_user",userName ); + auth.put("ansible_user", userName); } // Validate that node has at least one authentication method configured From f40645c62e154f7827763c2a4a064dd8d83cb779 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 09:39:57 -0300 Subject: [PATCH 33/46] Update src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../com/rundeck/plugins/ansible/ansible/AnsibleRunner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index fa48a965..908dffac 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -1030,7 +1030,7 @@ String buildGroupVarsYaml(Map hostPasswords, Map } } else { // Single line value (shouldn't happen with vault, but handle it) - log.debug("Single line vault value for host: {}", originalKey); + log.warn("Single line vault value for host: {}", originalKey); yamlContent.append(vaultValue).append("\n"); } } From 46e56dbf5438ca3d583e9cfe42bcaec9ef4e82cf Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 09:40:41 -0300 Subject: [PATCH 34/46] Update functional-test/build.gradle Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- functional-test/build.gradle | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/functional-test/build.gradle b/functional-test/build.gradle index 419b4f6b..4ae22bf2 100644 --- a/functional-test/build.gradle +++ b/functional-test/build.gradle @@ -44,12 +44,10 @@ tasks.register('functionalTest', Test) { useJUnitPlatform() // Rundeck test image version - // NOTE: Functional tests run against Rundeck 5.18.0. This is an upgrade from the - // previously used 5.1.1 to validate compatibility with newer Rundeck releases and - // ensure the multi-node authentication feature works correctly with current - // Rundeck APIs and behavior. The plugin's minimum supported Rundeck version is - // defined in the main README/changelog; updating that minimum version should be - // accompanied by updating this test image tag accordingly. + // Minimum supported Rundeck version for this plugin: 5.1.1 (see main README/changelog). + // Functional tests intentionally run against a newer version, Rundeck 5.18.0, to validate + // compatibility with current releases. When raising the minimum supported version, update + // this test image tag in tandem. systemProperty('RUNDECK_TEST_IMAGE', "rundeck/rundeck:5.18.0") // Docker configuration for Testcontainers From 27b2e6bb96f05ffe0a677a085d7320cf89927f21 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 09:40:59 -0300 Subject: [PATCH 35/46] Update src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../com/rundeck/plugins/ansible/ansible/AnsibleRunner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 908dffac..16f782d4 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -535,7 +535,7 @@ public int run() throws Exception { ); // Alert administrators that sensitive files are being created in their inventory directory - log.warn("Writing vault-encrypted authentication data to user-provided inventory location: {}. " + + log.info("Writing vault-encrypted authentication data to user-provided inventory location: {}. " + "Administrators should ensure this directory has appropriate filesystem permissions. " + "This file will be cleaned up after execution, but may persist if cleanup fails.", tempNodeAuthFile.getAbsolutePath()); From 958f407b023d76a6c1a110e4ce366fc24533f083 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 09:41:57 -0300 Subject: [PATCH 36/46] Update src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../com/rundeck/plugins/ansible/ansible/AnsibleRunner.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 16f782d4..c6b77910 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -619,8 +619,8 @@ public int run() throws Exception { // This is a security best practice: private keys should never be writable after creation. // SSH itself will warn or refuse to use keys with overly permissive permissions (e.g., 0600). // Write permission is unnecessary since this temporary file is created once, read by SSH, - // and never modified. The file will be deleted after use. This matches the permissions - // used for node-specific private keys (lines 474-476). + // and never modified. The file will be deleted after use. + // Node-specific private keys created elsewhere in this class should use the same 0400 permissions for consistency. Set perms = new HashSet(); perms.add(PosixFilePermission.OWNER_READ); Files.setPosixFilePermissions(tempPkFile.toPath(), perms); From 19045a87f70170572c2d67d87e1c8274b5b0bbb3 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 12:10:26 -0300 Subject: [PATCH 37/46] copilot comments --- .../ansible/ansible/AnsibleRunner.java | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index c6b77910..aefed428 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -1081,6 +1081,29 @@ private void appendYamlMapSection(StringBuilder yamlContent, String sectionName, } } + /** + * Pattern for detecting YAML special characters that require quoting. + * Includes: : [ ] { } # & * ! | > ' " % @ ` \ + */ + private static final java.util.regex.Pattern YAML_SPECIAL_CHARS_PATTERN = + java.util.regex.Pattern.compile(".*[:\\[\\]{}#&*!|>'\"%@`\\\\].*"); + + /** + * Checks if a string needs YAML quoting based on special characters and prefixes. + * + * @param value The string to check + * @param checkNumericPrefix If true, requires quoting if value starts with a digit + * @param checkEmpty If true, requires quoting if value is empty after trimming + * @return true if the string needs to be wrapped in quotes + */ + private boolean needsYamlQuoting(String value, boolean checkNumericPrefix, boolean checkEmpty) { + return YAML_SPECIAL_CHARS_PATTERN.matcher(value).matches() || + value.startsWith("-") || + value.startsWith("?") || + (checkNumericPrefix && value.matches("^[0-9].*")) || + (checkEmpty && value.trim().isEmpty()); + } + /** * Escapes YAML special characters in keys. * If the key contains special characters, wraps it in quotes. @@ -1089,11 +1112,7 @@ private void appendYamlMapSection(StringBuilder yamlContent, String sectionName, * @return Escaped key safe for YAML */ String escapeYamlKey(String key) { - // YAML special characters that require quoting (including backslash) - if (key.matches(".*[:\\[\\]{}#&*!|>'\"%@`\\\\].*") || - key.startsWith("-") || - key.startsWith("?") || - key.matches("^[0-9].*")) { + if (needsYamlQuoting(key, true, false)) { // IMPORTANT: Order of replacements matters! Must escape backslashes FIRST, then quotes. // If we escaped quotes first, the backslash replacement would double-escape them. // Example: value="a\"b" -> replace quotes: "a\\"b" -> replace backslash: "a\\\\"b" (WRONG!) @@ -1111,11 +1130,7 @@ String escapeYamlKey(String key) { * @return Escaped value safe for YAML */ String escapeYamlValue(String value) { - // Check if value needs quoting (including backslash) - if (value.matches(".*[:\\[\\]{}#&*!|>'\"%@`\\\\].*") || - value.startsWith("-") || - value.startsWith("?") || - value.trim().isEmpty()) { + if (needsYamlQuoting(value, false, true)) { // IMPORTANT: Order of replacements matters! Must escape backslashes FIRST, then quotes. // If we escaped quotes first, the backslash replacement would double-escape them. // Example: value="a\"b" -> replace quotes: "a\\"b" -> replace backslash: "a\\\\"b" (WRONG!) From 93a0f7459441b801d1333262a9bf68f4cc8189a3 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 12:15:27 -0300 Subject: [PATCH 38/46] Update src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../com/rundeck/plugins/ansible/ansible/AnsibleRunner.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index aefed428..163b428a 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -1293,8 +1293,8 @@ String sanitizeNodeNameForFilesystem(String nodeName) { // Replace unsafe characters with underscores String safeNodeName = nodeName.replaceAll("[^a-zA-Z0-9._-]", "_"); - // Prevent hidden files (starting with .) and problematic names (empty or all dots) - if (safeNodeName.isEmpty() || safeNodeName.startsWith(".") || safeNodeName.matches("^\\.+$")) { + // Prevent hidden files (starting with .) and problematic names (empty) + if (safeNodeName.isEmpty() || safeNodeName.startsWith(".")) { safeNodeName = "_" + safeNodeName; } From 01e918fd08ec747750738c72fd8a3c5553dcd6cb Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 12:40:15 -0300 Subject: [PATCH 39/46] Update functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml index 3a159b54..7fd7aefb 100644 --- a/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml +++ b/functional-test/src/test/resources/project-import/ansible-multi-node-auth/rundeck-ansible-multi-node-auth/jobs/job-multi-node-ping.xml @@ -10,6 +10,7 @@ 3 true + a1b2c3d4-e5f6-7890-abcd-ef1234567890 INFO multi-node-ping-test From bc32a3bb8515568c2e73fd1ad0577ada9dbd63f9 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 12:41:07 -0300 Subject: [PATCH 40/46] Update src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../com/rundeck/plugins/ansible/ansible/AnsibleRunner.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 163b428a..07749ae0 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -573,8 +573,7 @@ public int run() throws Exception { } } catch (IOException e) { - System.err.println("ERROR: Failed to write all.yaml: " + e.getMessage()); - e.printStackTrace(); + log.error("ERROR: Failed to write all.yaml for node auth", e); throw new RuntimeException("Failed to write all.yaml for node auth", e); } } From 6b4e92111f3ca5064e8fa1e80adf1c5171f64244 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 12:42:36 -0300 Subject: [PATCH 41/46] Update src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../plugins/ansible/ansible/AnsibleRunner.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 07749ae0..81201214 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -614,14 +614,14 @@ public int run() throws Exception { String privateKeyData = sshPrivateKey.replaceAll("\r\n", "\n"); tempPkFile = AnsibleUtil.createTemporaryFile("","id_rsa", privateKeyData,customTmpDirPath); - // Set SSH private key permissions to 0400 (owner read-only). - // This is a security best practice: private keys should never be writable after creation. - // SSH itself will warn or refuse to use keys with overly permissive permissions (e.g., 0600). - // Write permission is unnecessary since this temporary file is created once, read by SSH, - // and never modified. The file will be deleted after use. - // Node-specific private keys created elsewhere in this class should use the same 0400 permissions for consistency. + // Set SSH private key permissions to 0600 (owner read/write). + // This keeps the key private (not accessible by group/world) while preserving backward + // compatibility for workflows that may need to modify or reuse this temporary file. + // SSH itself will warn or refuse to use keys with overly permissive permissions, so we + // restrict access to the file owner only. Set perms = new HashSet(); perms.add(PosixFilePermission.OWNER_READ); + perms.add(PosixFilePermission.OWNER_WRITE); Files.setPosixFilePermissions(tempPkFile.toPath(), perms); if (sshUseAgent) { From 2784ac963f2d3d1af9f73eb8a3295a3e55684175 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 12:59:03 -0300 Subject: [PATCH 42/46] Copilot comments --- .../ansible/ansible/AnsibleRunner.java | 19 ++- .../ansible/AnsibleRunnerContextBuilder.java | 110 ++++++------------ 2 files changed, 53 insertions(+), 76 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 81201214..335d15e5 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -1082,7 +1082,24 @@ private void appendYamlMapSection(StringBuilder yamlContent, String sectionName, /** * Pattern for detecting YAML special characters that require quoting. - * Includes: : [ ] { } # & * ! | > ' " % @ ` \ + * Matches any string containing one or more of these characters: + *
    + *
  • : - colon (key-value separator)
    • + *
  • [ ] - square brackets (array notation)
    • + *
  • { } - curly braces (object notation)
    • + *
  • # - hash (comment marker)
    • + *
  • & - ampersand (anchor reference)
    • + *
  • * - asterisk (alias reference)
    • + *
  • ! - exclamation (tag indicator)
    • + *
  • | - pipe (literal block scalar)
    • + *
  • > - greater-than (folded block scalar)
    • + *
  • ' " - quotes (string delimiters)
    • + *
  • % - percent (directive indicator)
    • + *
  • @ - at-sign (reserved for future use)
    • + *
  • ` - backtick (reserved for future use)
    • + *
  • \ - backslash (escape character)
    • + *
+ * Regex pattern: .*[:\\[\\]{}#&*!|>'\"%@`\\\\].* */ private static final java.util.regex.Pattern YAML_SPECIAL_CHARS_PATTERN = java.util.regex.Pattern.compile(".*[:\\[\\]{}#&*!|>'\"%@`\\\\].*"); diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 895ac862..40c6e5f5 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -845,7 +845,8 @@ public void cleanupTempFiles() { } /** - * Recursively deletes a directory and all its contents. + * Recursively deletes a directory and all its contents using Java NIO. + * Uses Files.walk() for robust directory traversal and handles errors gracefully. * * @param directory The directory to delete * @return true if the directory and all its contents were successfully deleted, false otherwise @@ -855,30 +856,22 @@ private boolean deleteDirectoryRecursively(File directory) { return true; } - boolean success = true; - File[] files = directory.listFiles(); - if (files != null) { - for (File file : files) { - if (file.isDirectory()) { - if (!deleteDirectoryRecursively(file)) { - success = false; - log.warn("Failed to delete subdirectory: {}", file.getAbsolutePath()); - } - } else { - if (!file.delete()) { - success = false; - log.warn("Failed to delete file: {}", file.getAbsolutePath()); - } - } - } - } - - if (!directory.delete()) { - success = false; - log.warn("Failed to delete directory: {}", directory.getAbsolutePath()); + try { + // Walk the file tree in depth-first order (reverse) to delete files before their parent directories + Files.walk(directory.toPath()) + .sorted(java.util.Comparator.reverseOrder()) + .forEach(path -> { + try { + Files.delete(path); + } catch (java.io.IOException e) { + log.warn("Failed to delete: {}", path.toAbsolutePath(), e); + } + }); + return true; + } catch (java.io.IOException e) { + log.warn("Failed to walk directory tree for deletion: {}", directory.getAbsolutePath(), e); + return false; } - - return success; } public Boolean getUseSshAgent() { @@ -1021,52 +1014,34 @@ public Map> getNodesAuthenticationMap(){ Map auth = new HashMap<>(); final AuthenticationType authType = getSshAuthenticationType(node); - if (getDebug()) { - System.err.println("DEBUG: Processing authentication for node '" + node.getNodename() + - "' with auth type: " + authType); - } + log.debug("Processing authentication for node '{}' with auth type: {}", node.getNodename(), authType); if (AnsibleDescribable.AuthenticationType.privateKey == authType) { final String privateKey; try { privateKey = getSshPrivateKey(node); - if (getDebug()) { - System.err.println("DEBUG: Retrieved private key for node '" + - node.getNodename() + "': " + (privateKey != null ? ("yes, length=" + privateKey.length()) : "null")); - } + log.debug("Retrieved private key for node '{}': {}", node.getNodename(), + (privateKey != null ? ("yes, length=" + privateKey.length()) : "null")); } catch (ConfigurationException e) { - if (getDebug()) { - System.err.println("DEBUG: Error retrieving private key for node '" + - node.getNodename() + "': " + e.getMessage()); - } + log.debug("Error retrieving private key for node '{}': {}", node.getNodename(), e.getMessage()); throw new RuntimeException("Failed to retrieve private key for node '" + node.getNodename() + "': " + e.getMessage(), e); } if (privateKey != null) { auth.put("ansible_ssh_private_key", privateKey); - if (getDebug()) { - System.err.println("DEBUG: Added private key to auth map for node '" + node.getNodename() + "'"); - } + log.debug("Added private key to auth map for node '{}'", node.getNodename()); } else { - if (getDebug()) { - System.err.println("DEBUG: Private key is null for node '" + node.getNodename() + "', not adding to auth map"); - } + log.debug("Private key is null for node '{}', not adding to auth map", node.getNodename()); } } else if (AnsibleDescribable.AuthenticationType.password == authType) { try { String password = getSshPassword(node); if(null!=password){ auth.put("ansible_password", password); - if (getDebug()) { - System.err.println("DEBUG: Successfully retrieved password for node '" + - node.getNodename() + "'"); - } + log.debug("Successfully retrieved password for node '{}'", node.getNodename()); } } catch (ConfigurationException e) { - if (getDebug()) { - System.err.println("DEBUG: Error retrieving password for node '" + - node.getNodename() + "': " + e.getMessage()); - } + log.debug("Error retrieving password for node '{}': {}", node.getNodename(), e.getMessage()); throw new RuntimeException("Failed to retrieve password for node '" + node.getNodename() + "': " + e.getMessage(), e); } @@ -1085,10 +1060,8 @@ public Map> getNodesAuthenticationMap(){ if (!hasPassword && !hasPrivateKey) { context.getExecutionLogger().log(2, "WARNING: Node '" + node.getNodename() + "' has no password or private key configured. Authentication may fail."); - if (getDebug()) { - System.err.println("DEBUG: Node '" + node.getNodename() + - "' has no credentials configured (only username: " + (auth.containsKey("ansible_user") ? "yes" : "no") + ")"); - } + log.debug("Node '{}' has no credentials configured (only username: {})", + node.getNodename(), (auth.containsKey("ansible_user") ? "yes" : "no")); } authenticationNodesMap.put(node.getNodename(), auth); @@ -1145,11 +1118,9 @@ public List getListNodesKeyPath(){ public Boolean generateInventoryNodesAuth() { Boolean generateInventoryNodesAuth = null; - if(getDebug()) { - System.err.println("DEBUG: Resolving property ANSIBLE_GENERATE_INVENTORY_NODES_AUTH"); - System.err.println("DEBUG: Property key: " + AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); - System.err.println("DEBUG: Framework project: " + getFrameworkProject()); - } + log.debug("Resolving property ANSIBLE_GENERATE_INVENTORY_NODES_AUTH"); + log.debug("Property key: {}", AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH); + log.debug("Framework project: {}", getFrameworkProject()); String sgenerateInventoryNodesAuth = PropertyResolver.resolveProperty( AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH, @@ -1160,19 +1131,13 @@ public Boolean generateInventoryNodesAuth() { getJobConf() ); - if(getDebug()) { - System.err.println("DEBUG: PropertyResolver returned: " + sgenerateInventoryNodesAuth); - } + log.debug("PropertyResolver returned: {}", sgenerateInventoryNodesAuth); if (null != sgenerateInventoryNodesAuth) { generateInventoryNodesAuth = Boolean.parseBoolean(sgenerateInventoryNodesAuth); - if(getDebug()) { - System.err.println("DEBUG: Parsed to boolean: " + generateInventoryNodesAuth); - } + log.debug("Parsed to boolean: {}", generateInventoryNodesAuth); } else { - if(getDebug()) { - System.err.println("DEBUG: Property not found, returning null"); - } + log.debug("Property not found, returning null"); } return generateInventoryNodesAuth; @@ -1188,9 +1153,7 @@ public Boolean generateInventoryNodesAuth() { String getExecutionSpecificTmpDir() { // Return cached directory if already created if (executionSpecificDir != null) { - if (getDebug()) { - System.err.println("DEBUG: Using cached execution-specific directory: " + executionSpecificDir.getAbsolutePath()); - } + log.debug("Using cached execution-specific directory: {}", executionSpecificDir.getAbsolutePath()); return executionSpecificDir.getAbsolutePath(); } @@ -1199,10 +1162,7 @@ String getExecutionSpecificTmpDir() { // Get execution ID from data context if (context.getDataContext() != null && context.getDataContext().get("job") != null) { executionId = context.getDataContext().get("job").get("execid"); - - if (getDebug()) { - System.err.println("DEBUG: Execution ID from context: " + executionId); - } + log.debug("Execution ID from context: {}", executionId); } // Get base tmp directory From a4356009a33417c5b2956700a4f7569e2379bd67 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 13:56:29 -0300 Subject: [PATCH 43/46] Copilot comments debug --- .../ansible/ansible/AnsibleRunner.java | 48 +++++++------------ 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 335d15e5..31f0356a 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -465,8 +465,8 @@ public int run() throws Exception { try { // Sanitize node name for filesystem use String safeNodeName = sanitizeNodeNameForFilesystem(nodeName); - if(debug && !nodeName.equals(safeNodeName)) { - System.err.println("DEBUG: Sanitized node name '" + nodeName + "' to '" + safeNodeName + "' for temp file"); + if(!nodeName.equals(safeNodeName)) { + log.debug("Sanitized node name '{}' to '{}' for temp file", nodeName, safeNodeName); } tempHostPkFile = AnsibleUtil.createTemporaryFile("","id_rsa_node_"+safeNodeName, privateKey,customTmpDirPath); @@ -489,10 +489,8 @@ public int run() throws Exception { // Build YAML content using helper method String yamlContent = buildGroupVarsYaml(hostPasswords, hostUsers, hostKeys); - if(debug){ - System.err.println("DEBUG: Building group_vars YAML with " + hostPasswords.size() + " passwords, " + hostUsers.size() + " users, and " + hostKeys.size() + " private keys"); - System.err.println("DEBUG: YAML content built successfully, length: " + yamlContent.length()); - } + log.debug("Building group_vars YAML with {} passwords, {} users, and {} private keys", hostPasswords.size(), hostUsers.size(), hostKeys.size()); + log.debug("YAML content built successfully, length: {}", yamlContent.length()); try { @@ -505,17 +503,13 @@ public int run() throws Exception { File inventoryFile = new File(inventory); File inventoryParentDir = inventoryFile.getParentFile(); - if(debug) { - System.err.println("DEBUG: inventoryFile: " + inventoryFile.getAbsolutePath()); - System.err.println("DEBUG: inventory file exists: " + inventoryFile.exists()); - System.err.println("DEBUG: inventoryParentDir: " + (inventoryParentDir != null ? inventoryParentDir.getAbsolutePath() : "null")); - } + log.debug("inventoryFile: {}", inventoryFile.getAbsolutePath()); + log.debug("inventory file exists: {}", inventoryFile.exists()); + log.debug("inventoryParentDir: {}", (inventoryParentDir != null ? inventoryParentDir.getAbsolutePath() : "null")); if (inventoryParentDir != null) { groupVarsDir = new File(inventoryParentDir, "group_vars"); - if(debug) { - System.err.println("DEBUG: group_vars directory path: " + groupVarsDir.getAbsolutePath()); - } + log.debug("group_vars directory path: {}", groupVarsDir.getAbsolutePath()); try { // Use Files.createDirectories() which is idempotent (safe to call if directory exists) @@ -540,25 +534,19 @@ public int run() throws Exception { "This file will be cleaned up after execution, but may persist if cleanup fails.", tempNodeAuthFile.getAbsolutePath()); - if(debug) { - System.err.println("DEBUG: Writing all.yaml to: " + tempNodeAuthFile.getAbsolutePath()); - System.err.println("DEBUG: all.yaml written successfully, file size: " + tempNodeAuthFile.length() + " bytes"); - } + log.debug("Writing all.yaml to: {}", tempNodeAuthFile.getAbsolutePath()); + log.debug("all.yaml written successfully, file size: {} bytes", tempNodeAuthFile.length()); } else { // Fallback to temp file if inventory has no parent directory tempNodeAuthFile = AnsibleUtil.createTemporaryFile("group_vars", "all.yaml", yamlContent, customTmpDirPath); - if(debug) { - System.err.println("DEBUG: No parent directory, using temporary file"); - System.err.println("DEBUG: Temporary all.yaml created at: " + tempNodeAuthFile.getAbsolutePath()); - } + log.debug("No parent directory, using temporary file"); + log.debug("Temporary all.yaml created at: {}", tempNodeAuthFile.getAbsolutePath()); } - if(debug) { - System.err.println("DEBUG: tempNodeAuthFile: " + tempNodeAuthFile.getAbsolutePath()); - System.err.println("DEBUG: tempNodeAuthFile exists: " + tempNodeAuthFile.exists()); - System.err.println("DEBUG: tempNodeAuthFile readable: " + tempNodeAuthFile.canRead()); - } + log.debug("tempNodeAuthFile: {}", tempNodeAuthFile.getAbsolutePath()); + log.debug("tempNodeAuthFile exists: {}", tempNodeAuthFile.exists()); + log.debug("tempNodeAuthFile readable: {}", tempNodeAuthFile.canRead()); //set extra vars to resolve the host specific authentication if (!hostUsers.isEmpty()) { @@ -1011,7 +999,7 @@ String buildGroupVarsYaml(Map hostPasswords, Map // Validate vault format if (!isValidVaultFormat(vaultValue)) { - System.err.println("ERROR: Invalid vault format for host: " + originalKey); + log.error("Invalid vault format for host: {}", originalKey); throw new RuntimeException("Invalid vault format for host: " + originalKey); } // The vault value already contains "!vault |\n" followed by the encrypted content @@ -1047,9 +1035,7 @@ String buildGroupVarsYaml(Map hostPasswords, Map String result = yamlContent.toString(); - if(debug){ - System.err.println("DEBUG: Generated YAML content (" + result.length() + " bytes)"); - } + log.debug("Generated YAML content ({} bytes)", result.length()); return result; } From 6bf39e972fbcdf0cd8814e6b3493423eff57a66c Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 14:44:56 -0300 Subject: [PATCH 44/46] Update functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../src/test/resources/docker/ansible-multi-node-auth/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md b/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md index 05d5f88e..3cff29da 100644 --- a/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md +++ b/functional-test/src/test/resources/docker/ansible-multi-node-auth/README.md @@ -7,7 +7,7 @@ This test verifies the multi-node authentication feature where each node can hav ### Nodes - **ssh-node**: Standard password (`testpassword123`) - **ssh-node-2**: Password with special characters (`password2_special!@#`) -- **ssh-node-3**: Password with quotes (`password3"quote'test`) +- **ssh-node-3**: Password containing both a double quote (") and a single quote ('): password3"quote'test - **ssh-node-4**: Private key authentication (uses SSH key from key storage) ### Files From 65ae4447c59d8cefc9ce2aa40b3088d86eb256a2 Mon Sep 17 00:00:00 2001 From: Eduardo Baltra Date: Fri, 16 Jan 2026 16:15:10 -0300 Subject: [PATCH 45/46] Final Copilot comments --- .../ansible/ansible/AnsibleRunner.java | 22 ++++++++++++++++--- .../ansible/AnsibleRunnerContextBuilder.java | 5 +++-- .../ansible/ansible/AnsibleRunnerSpec.groovy | 18 +++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java index 31f0356a..a74e018f 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java @@ -1286,10 +1286,26 @@ public String encryptExtraVarsKey(String extraVars) throws Exception { /** * Sanitizes a node name to be safe for use in filesystem paths. - * Replaces unsafe characters with underscores and prevents hidden files and problematic names. + *

+ * This method performs the following transformations: + *

    + *
  • Replaces all characters except alphanumeric, dot, underscore, and hyphen with underscores
    • + *
  • Prevents hidden files by prepending underscore if name starts with dot
    • + *
  • Handles empty strings by prepending underscore
    • + *
+ *

+ *

+ * Examples: + *

    + *
  • {@code "node-1"} → {@code "node-1"} (no change)
    • + *
  • {@code "node@host"} → {@code "node_host"} (@ replaced)
    • + *
  • {@code ".hidden"} → {@code "_.hidden"} (prevents hidden file)
    • + *
  • {@code ""} → {@code "_"} (handles empty)
    • + *
+ *

* - * @param nodeName The original node name - * @return A sanitized node name safe for use in file paths + * @param nodeName The original node name to sanitize + * @return A sanitized node name safe for use in file paths, never null */ String sanitizeNodeNameForFilesystem(String nodeName) { // Replace unsafe characters with underscores diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 40c6e5f5..83efa3bc 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -226,8 +226,9 @@ public String getPasswordFromPath(String storagePath) throws ConfigurationExcept * * @param storagePath The storage path to read from * @param resourceType Description of resource type for error messages (e.g., "ssh password", "ssh private key") - * @return The content as a UTF-8 string, or null if storagePath is null - * @throws ConfigurationException if reading fails + * @return The content as a UTF-8 string. Returns null ONLY if storagePath parameter is null (early return). + * After attempting to read content, this method either returns a non-null string or throws an exception. + * @throws ConfigurationException if the storage path cannot be read or does not exist */ private String readFromStoragePath(String storagePath, String resourceType) throws ConfigurationException { if (storagePath == null) { diff --git a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy index df6575af..41d5757c 100644 --- a/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy +++ b/src/test/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerSpec.groovy @@ -623,6 +623,24 @@ class AnsibleRunnerSpec extends Specification{ "key with spaces" || "key with spaces" // internal spaces are valid in unquoted YAML keys; node names with leading/trailing spaces are not supported } + def "escapeYamlKey: node names with leading/trailing spaces are not modified"() { + given: + def builder = AnsibleRunner.playbookInline("test") + builder.customTmpDirPath("/tmp") + def runner = builder.build() + + expect: + // This documents that escapeYamlKey does not normalize leading/trailing spaces. + // Node names with such spaces are considered unsupported at a higher level. + runner.escapeYamlKey(key) == expectedResult + + where: + key || expectedResult + " leading-space" || " leading-space" + "trailing-space " || "trailing-space " + " both-sides " || " both-sides " + } + def "escapeYamlValue: should quote values with special characters"() { given: def builder = AnsibleRunner.playbookInline("test") From 7db271f89a7d5b6f6eb09149690332a4b6853747 Mon Sep 17 00:00:00 2001 From: Luis Toledo Date: Fri, 16 Jan 2026 19:21:01 -0300 Subject: [PATCH 46/46] fix NPE --- .../plugins/ansible/ansible/AnsibleRunnerContextBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java index 83efa3bc..c9f3598c 100644 --- a/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java +++ b/src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java @@ -1117,7 +1117,7 @@ public List getListNodesKeyPath(){ public Boolean generateInventoryNodesAuth() { - Boolean generateInventoryNodesAuth = null; + boolean generateInventoryNodesAuth = false; log.debug("Resolving property ANSIBLE_GENERATE_INVENTORY_NODES_AUTH"); log.debug("Property key: {}", AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH);