Skip to content

Commit 2784ac9

Browse files
edbaltraedbaltra
committed
Copilot comments
1 parent 6b4e921 commit 2784ac9

2 files changed

Lines changed: 53 additions & 76 deletions

File tree

‎src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunner.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1082,7 +1082,24 @@ private void appendYamlMapSection(StringBuilder yamlContent, String sectionName,
10821082

10831083
/**
10841084
* Pattern for detecting YAML special characters that require quoting.
1085-
* Includes: : [ ] { } # & * ! | > ' " % @ ` \
1085+
* Matches any string containing one or more of these characters:
1086+
* <ul>
1087+
* <li><code>:</code> - colon (key-value separator)</li>
1088+
* <li><code>[ ]</code> - square brackets (array notation)</li>
1089+
* <li><code>{ }</code> - curly braces (object notation)</li>
1090+
* <li><code>#</code> - hash (comment marker)</li>
1091+
* <li><code>&</code> - ampersand (anchor reference)</li>
1092+
* <li><code>*</code> - asterisk (alias reference)</li>
1093+
* <li><code>!</code> - exclamation (tag indicator)</li>
1094+
* <li><code>|</code> - pipe (literal block scalar)</li>
1095+
* <li><code>&gt;</code> - greater-than (folded block scalar)</li>
1096+
* <li><code>' "</code> - quotes (string delimiters)</li>
1097+
* <li><code>%</code> - percent (directive indicator)</li>
1098+
* <li><code>@</code> - at-sign (reserved for future use)</li>
1099+
* <li><code>`</code> - backtick (reserved for future use)</li>
1100+
* <li><code>\</code> - backslash (escape character)</li>
1101+
* </ul>
1102+
* Regex pattern: <code>.*[:\\[\\]{}#&amp;*!|&gt;'\"%@`\\\\].*</code>
10861103
*/
10871104
private static final java.util.regex.Pattern YAML_SPECIAL_CHARS_PATTERN =
10881105
java.util.regex.Pattern.compile(".*[:\\[\\]{}#&*!|>'\"%@`\\\\].*");

‎src/main/groovy/com/rundeck/plugins/ansible/ansible/AnsibleRunnerContextBuilder.java‎

Lines changed: 35 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,8 @@ public void cleanupTempFiles() {
845845
}
846846

847847
/**
848-
* Recursively deletes a directory and all its contents.
848+
* Recursively deletes a directory and all its contents using Java NIO.
849+
* Uses Files.walk() for robust directory traversal and handles errors gracefully.
849850
*
850851
* @param directory The directory to delete
851852
* @return true if the directory and all its contents were successfully deleted, false otherwise
@@ -855,30 +856,22 @@ private boolean deleteDirectoryRecursively(File directory) {
855856
return true;
856857
}
857858

858-
boolean success = true;
859-
File[] files = directory.listFiles();
860-
if (files != null) {
861-
for (File file : files) {
862-
if (file.isDirectory()) {
863-
if (!deleteDirectoryRecursively(file)) {
864-
success = false;
865-
log.warn("Failed to delete subdirectory: {}", file.getAbsolutePath());
866-
}
867-
} else {
868-
if (!file.delete()) {
869-
success = false;
870-
log.warn("Failed to delete file: {}", file.getAbsolutePath());
871-
}
872-
}
873-
}
874-
}
875-
876-
if (!directory.delete()) {
877-
success = false;
878-
log.warn("Failed to delete directory: {}", directory.getAbsolutePath());
859+
try {
860+
// Walk the file tree in depth-first order (reverse) to delete files before their parent directories
861+
Files.walk(directory.toPath())
862+
.sorted(java.util.Comparator.reverseOrder())
863+
.forEach(path -> {
864+
try {
865+
Files.delete(path);
866+
} catch (java.io.IOException e) {
867+
log.warn("Failed to delete: {}", path.toAbsolutePath(), e);
868+
}
869+
});
870+
return true;
871+
} catch (java.io.IOException e) {
872+
log.warn("Failed to walk directory tree for deletion: {}", directory.getAbsolutePath(), e);
873+
return false;
879874
}
880-
881-
return success;
882875
}
883876

884877
public Boolean getUseSshAgent() {
@@ -1021,52 +1014,34 @@ public Map<String, Map<String, String>> getNodesAuthenticationMap(){
10211014
Map<String, String> auth = new HashMap<>();
10221015
final AuthenticationType authType = getSshAuthenticationType(node);
10231016

1024-
if (getDebug()) {
1025-
System.err.println("DEBUG: Processing authentication for node '" + node.getNodename() +
1026-
"' with auth type: " + authType);
1027-
}
1017+
log.debug("Processing authentication for node '{}' with auth type: {}", node.getNodename(), authType);
10281018

10291019
if (AnsibleDescribable.AuthenticationType.privateKey == authType) {
10301020
final String privateKey;
10311021
try {
10321022
privateKey = getSshPrivateKey(node);
1033-
if (getDebug()) {
1034-
System.err.println("DEBUG: Retrieved private key for node '" +
1035-
node.getNodename() + "': " + (privateKey != null ? ("yes, length=" + privateKey.length()) : "null"));
1036-
}
1023+
log.debug("Retrieved private key for node '{}': {}", node.getNodename(),
1024+
(privateKey != null ? ("yes, length=" + privateKey.length()) : "null"));
10371025
} catch (ConfigurationException e) {
1038-
if (getDebug()) {
1039-
System.err.println("DEBUG: Error retrieving private key for node '" +
1040-
node.getNodename() + "': " + e.getMessage());
1041-
}
1026+
log.debug("Error retrieving private key for node '{}': {}", node.getNodename(), e.getMessage());
10421027
throw new RuntimeException("Failed to retrieve private key for node '" +
10431028
node.getNodename() + "': " + e.getMessage(), e);
10441029
}
10451030
if (privateKey != null) {
10461031
auth.put("ansible_ssh_private_key", privateKey);
1047-
if (getDebug()) {
1048-
System.err.println("DEBUG: Added private key to auth map for node '" + node.getNodename() + "'");
1049-
}
1032+
log.debug("Added private key to auth map for node '{}'", node.getNodename());
10501033
} else {
1051-
if (getDebug()) {
1052-
System.err.println("DEBUG: Private key is null for node '" + node.getNodename() + "', not adding to auth map");
1053-
}
1034+
log.debug("Private key is null for node '{}', not adding to auth map", node.getNodename());
10541035
}
10551036
} else if (AnsibleDescribable.AuthenticationType.password == authType) {
10561037
try {
10571038
String password = getSshPassword(node);
10581039
if(null!=password){
10591040
auth.put("ansible_password", password);
1060-
if (getDebug()) {
1061-
System.err.println("DEBUG: Successfully retrieved password for node '" +
1062-
node.getNodename() + "'");
1063-
}
1041+
log.debug("Successfully retrieved password for node '{}'", node.getNodename());
10641042
}
10651043
} catch (ConfigurationException e) {
1066-
if (getDebug()) {
1067-
System.err.println("DEBUG: Error retrieving password for node '" +
1068-
node.getNodename() + "': " + e.getMessage());
1069-
}
1044+
log.debug("Error retrieving password for node '{}': {}", node.getNodename(), e.getMessage());
10701045
throw new RuntimeException("Failed to retrieve password for node '" +
10711046
node.getNodename() + "': " + e.getMessage(), e);
10721047
}
@@ -1085,10 +1060,8 @@ public Map<String, Map<String, String>> getNodesAuthenticationMap(){
10851060
if (!hasPassword && !hasPrivateKey) {
10861061
context.getExecutionLogger().log(2, "WARNING: Node '" + node.getNodename() +
10871062
"' has no password or private key configured. Authentication may fail.");
1088-
if (getDebug()) {
1089-
System.err.println("DEBUG: Node '" + node.getNodename() +
1090-
"' has no credentials configured (only username: " + (auth.containsKey("ansible_user") ? "yes" : "no") + ")");
1091-
}
1063+
log.debug("Node '{}' has no credentials configured (only username: {})",
1064+
node.getNodename(), (auth.containsKey("ansible_user") ? "yes" : "no"));
10921065
}
10931066

10941067
authenticationNodesMap.put(node.getNodename(), auth);
@@ -1145,11 +1118,9 @@ public List<String> getListNodesKeyPath(){
11451118
public Boolean generateInventoryNodesAuth() {
11461119
Boolean generateInventoryNodesAuth = null;
11471120

1148-
if(getDebug()) {
1149-
System.err.println("DEBUG: Resolving property ANSIBLE_GENERATE_INVENTORY_NODES_AUTH");
1150-
System.err.println("DEBUG: Property key: " + AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH);
1151-
System.err.println("DEBUG: Framework project: " + getFrameworkProject());
1152-
}
1121+
log.debug("Resolving property ANSIBLE_GENERATE_INVENTORY_NODES_AUTH");
1122+
log.debug("Property key: {}", AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH);
1123+
log.debug("Framework project: {}", getFrameworkProject());
11531124

11541125
String sgenerateInventoryNodesAuth = PropertyResolver.resolveProperty(
11551126
AnsibleDescribable.ANSIBLE_GENERATE_INVENTORY_NODES_AUTH,
@@ -1160,19 +1131,13 @@ public Boolean generateInventoryNodesAuth() {
11601131
getJobConf()
11611132
);
11621133

1163-
if(getDebug()) {
1164-
System.err.println("DEBUG: PropertyResolver returned: " + sgenerateInventoryNodesAuth);
1165-
}
1134+
log.debug("PropertyResolver returned: {}", sgenerateInventoryNodesAuth);
11661135

11671136
if (null != sgenerateInventoryNodesAuth) {
11681137
generateInventoryNodesAuth = Boolean.parseBoolean(sgenerateInventoryNodesAuth);
1169-
if(getDebug()) {
1170-
System.err.println("DEBUG: Parsed to boolean: " + generateInventoryNodesAuth);
1171-
}
1138+
log.debug("Parsed to boolean: {}", generateInventoryNodesAuth);
11721139
} else {
1173-
if(getDebug()) {
1174-
System.err.println("DEBUG: Property not found, returning null");
1175-
}
1140+
log.debug("Property not found, returning null");
11761141
}
11771142

11781143
return generateInventoryNodesAuth;
@@ -1188,9 +1153,7 @@ public Boolean generateInventoryNodesAuth() {
11881153
String getExecutionSpecificTmpDir() {
11891154
// Return cached directory if already created
11901155
if (executionSpecificDir != null) {
1191-
if (getDebug()) {
1192-
System.err.println("DEBUG: Using cached execution-specific directory: " + executionSpecificDir.getAbsolutePath());
1193-
}
1156+
log.debug("Using cached execution-specific directory: {}", executionSpecificDir.getAbsolutePath());
11941157
return executionSpecificDir.getAbsolutePath();
11951158
}
11961159

@@ -1199,10 +1162,7 @@ String getExecutionSpecificTmpDir() {
11991162
// Get execution ID from data context
12001163
if (context.getDataContext() != null && context.getDataContext().get("job") != null) {
12011164
executionId = context.getDataContext().get("job").get("execid");
1202-
1203-
if (getDebug()) {
1204-
System.err.println("DEBUG: Execution ID from context: " + executionId);
1205-
}
1165+
log.debug("Execution ID from context: {}", executionId);
12061166
}
12071167

12081168
// Get base tmp directory

0 commit comments

Comments
 (0)