From 0c01c4238364fee54061bceb09edb96c63dcc50b Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Thu, 7 Nov 2024 14:52:23 -0500 Subject: [PATCH 01/18] allow using a file based ssh credential via system property --- README.md | 21 +++++-- .../java/hudson/plugins/ec2/EC2Cloud.java | 59 +++++++++++++++---- .../hudson/plugins/ec2/SlaveTemplate.java | 1 + 3 files changed, 64 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 3aeaff214..b25986cb5 100644 --- a/README.md +++ b/README.md @@ -52,14 +52,25 @@ the main "Manage Jenkins" \> "Configure System" page, and scroll down near the bottom to the "Cloud" section. There, you click the "Add a new cloud" button, and select the "Amazon EC2" option. This will display the UI for configuring the EC2 plugin.  Then enter the Access Key and Secret -Access Key which act like a username/password (see IAM section). Because -of the way EC2 works, you also need to have an RSA private key that the +Access Key which act like a username/password (see IAM section). + +Because of the way EC2 works, you also need to have an RSA private key that the cloud has the other half for, to permit sshing into the instances that are started. Please use the AWS console or any other tool of your choice -to generate the private key to interactively log in to EC2 instances. +to generate the private key to interactively log in to EC2 instances. + +Once you have generated the needed private key you must either store it as +a Jenkins `SSH Private Key` credential (and select that credential in your cloud +config). + +If you do not want to create a new Jenkins credential you may alterantively store it +in plain text on disk, indicating its file path via the Jenkins system property +`SSH_KEY_PAIR_PRIVATE_KEY_FILE`. If this system property has a non-empty value then +it will override the ssh credential specified in the cloud configuration page. This +approach works well for `k8s` secrets that are mounted in a jenkins container for example. -Once you have put in your Access Key and Secret Access Key, select a -region for the cloud (not shown in screenshot). You may define only one +Once you have put in your Access Key, Secret Access Key, and configured an ssh private key +select a region for the cloud (not shown in screenshot). You may define only one cloud for each region, and the regions offered in the UI will show only the regions that you don't already have clouds defined for them. diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 76f8d112c..d1b6382ca 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -87,6 +87,9 @@ import java.net.MalformedURLException; import java.net.Proxy; import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.*; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @@ -127,6 +130,10 @@ public abstract class EC2Cloud extends Cloud { private static final SimpleFormatter sf = new SimpleFormatter(); + // if this system property is defined and its value points to a valid ssh private key on disk + // then this will be used instead of any configured ssh credential + private static final String SSH_KEY_PAIR_PRIVATE_KEY_FILE = "SSH_KEY_PAIR_PRIVATE_KEY_FILE"; + private transient ReentrantLock slaveCountingLock = new ReentrantLock(); private final boolean useInstanceProfileForCredentials; @@ -195,7 +202,11 @@ protected EC2Cloud(String id, boolean useInstanceProfileForCredentials, String c @CheckForNull public EC2PrivateKey resolvePrivateKey(){ - if (sshKeysCredentialsId != null) { + if (!System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, "").isEmpty()) { + LOGGER.fine(() -> "(resolvePrivateKey) secret key file configured, will load from disk"); + return fetchPrivateKeyFromDisk(); + } else if (sshKeysCredentialsId != null) { + LOGGER.fine(() -> "(resolvePrivateKey) Using jenkins ssh credential"); SSHUserPrivateKey privateKeyCredential = getSshCredential(sshKeysCredentialsId, Jenkins.get()); if (privateKeyCredential != null) { return new EC2PrivateKey(privateKeyCredential.getPrivateKey()); @@ -204,6 +215,19 @@ public EC2PrivateKey resolvePrivateKey(){ return null; } + private static EC2PrivateKey fetchPrivateKeyFromDisk() { + String filename = System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, ""); + if (!filename.isEmpty()) { + try { + return new EC2PrivateKey(new String(Files.readAllBytes(Paths.get(filename)), StandardCharsets.UTF_8)); + } catch (IOException e) { + LOGGER.warning(() -> "unable to read private key from file " + filename); + return null; + } + } + return null; + } + public abstract URL getEc2EndpointUrl() throws IOException; public abstract URL getS3EndpointUrl() throws IOException; @@ -1122,6 +1146,7 @@ public ListBoxModel doFillSshKeysCredentialsIdItems(@AncestorInPath ItemGroup co AbstractIdCredentialsListBoxModel result = new StandardListBoxModel(); if (Jenkins.get().hasPermission(Jenkins.ADMINISTER)) { result = result + .includeEmptyValue() .includeMatchingAs(Jenkins.getAuthentication(), context, SSHUserPrivateKey.class, Collections.emptyList(), CredentialsMatchers.always()) .includeMatchingAs(ACL.SYSTEM, context, SSHUserPrivateKey.class, Collections.emptyList(), CredentialsMatchers.always()) .includeCurrentValue(sshKeysCredentialsId); @@ -1135,16 +1160,22 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont // Don't do anything if the user is only reading the configuration return FormValidation.ok(); } - if (value == null || value.isEmpty()){ - return FormValidation.error("No ssh credentials selected"); - } - SSHUserPrivateKey sshCredential = getSshCredential(value, context); String privateKey = ""; - if (sshCredential != null) { - privateKey = sshCredential.getPrivateKey(); + + if (System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, "").isEmpty()) { + if (value == null || value.isEmpty()) { + return FormValidation.error("No ssh credentials selected"); + } + + SSHUserPrivateKey sshCredential = getSshCredential(value, context); + if (sshCredential != null) { + privateKey = sshCredential.getPrivateKey(); + } else { + return FormValidation.error("Failed to find credential \"" + value + "\" in store."); + } } else { - return FormValidation.error("Failed to find credential \"" + value + "\" in store."); + privateKey = fetchPrivateKeyFromDisk().getPrivateKey(); } boolean hasStart = false, hasEnd = false; @@ -1188,12 +1219,16 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL return FormValidation.ok(); } try { - SSHUserPrivateKey sshCredential = getSshCredential(sshKeysCredentialsId, context); String privateKey = ""; - if (sshCredential != null) { - privateKey = sshCredential.getPrivateKey(); + if (System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, "").isEmpty()) { + SSHUserPrivateKey sshCredential = getSshCredential(sshKeysCredentialsId, context); + if (sshCredential != null) { + privateKey = sshCredential.getPrivateKey(); + } else { + return FormValidation.error("Failed to find credential \"" + sshKeysCredentialsId + "\" in store."); + } } else { - return FormValidation.error("Failed to find credential \"" + sshKeysCredentialsId + "\" in store."); + privateKey = fetchPrivateKeyFromDisk().getPrivateKey() ; } AWSCredentialsProvider credentialsProvider = createCredentialsProvider(useInstanceProfileForCredentials, credentialsId, roleArn, roleSessionName, region); diff --git a/src/main/java/hudson/plugins/ec2/SlaveTemplate.java b/src/main/java/hudson/plugins/ec2/SlaveTemplate.java index 337b2b78d..ce741d0cc 100644 --- a/src/main/java/hudson/plugins/ec2/SlaveTemplate.java +++ b/src/main/java/hudson/plugins/ec2/SlaveTemplate.java @@ -1655,6 +1655,7 @@ private KeyPair getKeyPair(AmazonEC2 ec2) throws IOException, AmazonClientExcept if (keyPair == null) { throw new AmazonClientException("No matching keypair found on EC2. Is the EC2 private key a valid one?"); } + LOGGER.fine("found matching keypair"); return keyPair; } From c51223bef3e7cc01952019e2acc00eb4fea7d421 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Thu, 7 Nov 2024 17:01:13 -0500 Subject: [PATCH 02/18] follow convention --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index d1b6382ca..c1a216323 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -132,7 +132,7 @@ public abstract class EC2Cloud extends Cloud { // if this system property is defined and its value points to a valid ssh private key on disk // then this will be used instead of any configured ssh credential - private static final String SSH_KEY_PAIR_PRIVATE_KEY_FILE = "SSH_KEY_PAIR_PRIVATE_KEY_FILE"; + private static final String sshPrivateKeyFilePath =EC2Cloud.class.getName() + "sshPrivateKeyFilePath"; private transient ReentrantLock slaveCountingLock = new ReentrantLock(); @@ -202,7 +202,7 @@ protected EC2Cloud(String id, boolean useInstanceProfileForCredentials, String c @CheckForNull public EC2PrivateKey resolvePrivateKey(){ - if (!System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, "").isEmpty()) { + if (!System.getProperty(sshPrivateKeyFilePath, "").isEmpty()) { LOGGER.fine(() -> "(resolvePrivateKey) secret key file configured, will load from disk"); return fetchPrivateKeyFromDisk(); } else if (sshKeysCredentialsId != null) { @@ -216,7 +216,7 @@ public EC2PrivateKey resolvePrivateKey(){ } private static EC2PrivateKey fetchPrivateKeyFromDisk() { - String filename = System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, ""); + String filename = System.getProperty(sshPrivateKeyFilePath, ""); if (!filename.isEmpty()) { try { return new EC2PrivateKey(new String(Files.readAllBytes(Paths.get(filename)), StandardCharsets.UTF_8)); @@ -1163,7 +1163,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont String privateKey = ""; - if (System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, "").isEmpty()) { + if (System.getProperty(sshPrivateKeyFilePath, "").isEmpty()) { if (value == null || value.isEmpty()) { return FormValidation.error("No ssh credentials selected"); } @@ -1220,7 +1220,7 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL } try { String privateKey = ""; - if (System.getProperty(SSH_KEY_PAIR_PRIVATE_KEY_FILE, "").isEmpty()) { + if (System.getProperty(sshPrivateKeyFilePath, "").isEmpty()) { SSHUserPrivateKey sshCredential = getSshCredential(sshKeysCredentialsId, context); if (sshCredential != null) { privateKey = sshCredential.getPrivateKey(); From 9d321ab4cd63626dac667d75fc8893c458623d70 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Thu, 7 Nov 2024 17:01:38 -0500 Subject: [PATCH 03/18] simplify --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index c1a216323..114ec485c 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -219,9 +219,9 @@ private static EC2PrivateKey fetchPrivateKeyFromDisk() { String filename = System.getProperty(sshPrivateKeyFilePath, ""); if (!filename.isEmpty()) { try { - return new EC2PrivateKey(new String(Files.readAllBytes(Paths.get(filename)), StandardCharsets.UTF_8)); + return new EC2PrivateKey(Files.readString(Paths.get(filename), StandardCharsets.UTF_8)); } catch (IOException e) { - LOGGER.warning(() -> "unable to read private key from file " + filename); + LOGGER.warning(() -> "unable to read private key from file " + filename + ": " + e); return null; } } @@ -1161,7 +1161,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont return FormValidation.ok(); } - String privateKey = ""; + String privateKey; if (System.getProperty(sshPrivateKeyFilePath, "").isEmpty()) { if (value == null || value.isEmpty()) { From e5c8ac8860cf51c6f3000af857c070c1af487a74 Mon Sep 17 00:00:00 2001 From: michael cirioli Date: Thu, 7 Nov 2024 17:55:01 -0500 Subject: [PATCH 04/18] better logging Co-authored-by: Jesse Glick --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 114ec485c..2ee776c95 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -221,7 +221,7 @@ private static EC2PrivateKey fetchPrivateKeyFromDisk() { try { return new EC2PrivateKey(Files.readString(Paths.get(filename), StandardCharsets.UTF_8)); } catch (IOException e) { - LOGGER.warning(() -> "unable to read private key from file " + filename + ": " + e); + LOGGER.log(Level.WARNING, "unable to read private key from file " + filename, e); return null; } } From 33d198fdec8ddadf4bf626d2620323f56dd796fd Mon Sep 17 00:00:00 2001 From: michael cirioli Date: Fri, 8 Nov 2024 03:54:25 -0500 Subject: [PATCH 05/18] Apply suggestions from code review pr feedback Co-authored-by: Antonio Muniz --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 2ee776c95..964b356c2 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -132,7 +132,7 @@ public abstract class EC2Cloud extends Cloud { // if this system property is defined and its value points to a valid ssh private key on disk // then this will be used instead of any configured ssh credential - private static final String sshPrivateKeyFilePath =EC2Cloud.class.getName() + "sshPrivateKeyFilePath"; + public static String SSH_PRIVATE_KEY_FILEPATH = EC2Cloud.class.getName() + ".sshPrivateKeyFilePath"; private transient ReentrantLock slaveCountingLock = new ReentrantLock(); @@ -202,7 +202,7 @@ protected EC2Cloud(String id, boolean useInstanceProfileForCredentials, String c @CheckForNull public EC2PrivateKey resolvePrivateKey(){ - if (!System.getProperty(sshPrivateKeyFilePath, "").isEmpty()) { + if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { LOGGER.fine(() -> "(resolvePrivateKey) secret key file configured, will load from disk"); return fetchPrivateKeyFromDisk(); } else if (sshKeysCredentialsId != null) { @@ -215,8 +215,9 @@ public EC2PrivateKey resolvePrivateKey(){ return null; } + @CheckForNull private static EC2PrivateKey fetchPrivateKeyFromDisk() { - String filename = System.getProperty(sshPrivateKeyFilePath, ""); + String filename = System.getProperty(SSH_PRIVATE_KEY_FILEPATH, ""); if (!filename.isEmpty()) { try { return new EC2PrivateKey(Files.readString(Paths.get(filename), StandardCharsets.UTF_8)); @@ -1163,7 +1164,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont String privateKey; - if (System.getProperty(sshPrivateKeyFilePath, "").isEmpty()) { + if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { if (value == null || value.isEmpty()) { return FormValidation.error("No ssh credentials selected"); } @@ -1220,7 +1221,7 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL } try { String privateKey = ""; - if (System.getProperty(sshPrivateKeyFilePath, "").isEmpty()) { + if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { SSHUserPrivateKey sshCredential = getSshCredential(sshKeysCredentialsId, context); if (sshCredential != null) { privateKey = sshCredential.getPrivateKey(); From 8c68a511f0f846b6c1bda61d9cc1844bf2c7ead7 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 04:35:25 -0500 Subject: [PATCH 06/18] add simple test for new functionality --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 3 ++- src/test/java/hudson/plugins/ec2/EC2CloudTest.java | 7 +++++++ src/test/resources/hudson/plugins/ec2/test.pem | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 src/test/resources/hudson/plugins/ec2/test.pem diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 964b356c2..afecc2f8d 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -215,8 +215,9 @@ public EC2PrivateKey resolvePrivateKey(){ return null; } + /* visible for testing */ @CheckForNull - private static EC2PrivateKey fetchPrivateKeyFromDisk() { + public static EC2PrivateKey fetchPrivateKeyFromDisk() { String filename = System.getProperty(SSH_PRIVATE_KEY_FILEPATH, ""); if (!filename.isEmpty()) { try { diff --git a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java index eb0461a21..05616a1a2 100644 --- a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java @@ -27,6 +27,13 @@ @RunWith(MockitoJUnitRunner.class) public class EC2CloudTest { + @Test + public void testFileBasedSShKey() { + System.setProperty("hudson.plugins.ec2.EC2Cloud.sshPrivateKeyFilePath", getClass().getClassLoader().getResource("hudson/plugins/ec2/test.pem").getPath()); + assertTrue("file content should not have been empty", EC2Cloud.fetchPrivateKeyFromDisk() != null); + System.setProperty("hudson.plugins.ec2.EC2Cloud.sshPrivateKeyFilePath",null); + } + @Test public void testSlaveTemplateAddition() throws Exception { AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, diff --git a/src/test/resources/hudson/plugins/ec2/test.pem b/src/test/resources/hudson/plugins/ec2/test.pem new file mode 100644 index 000000000..30f51a3fb --- /dev/null +++ b/src/test/resources/hudson/plugins/ec2/test.pem @@ -0,0 +1 @@ +hello, world! \ No newline at end of file From 6fbefc7e9732ed0eea438633e8a9ef764907a15e Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 05:07:43 -0500 Subject: [PATCH 07/18] thank you spotbugs --- .../java/hudson/plugins/ec2/EC2Cloud.java | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index afecc2f8d..49ef02db0 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -132,7 +132,7 @@ public abstract class EC2Cloud extends Cloud { // if this system property is defined and its value points to a valid ssh private key on disk // then this will be used instead of any configured ssh credential - public static String SSH_PRIVATE_KEY_FILEPATH = EC2Cloud.class.getName() + ".sshPrivateKeyFilePath"; + public static final String SSH_PRIVATE_KEY_FILEPATH = EC2Cloud.class.getName() + ".sshPrivateKeyFilePath"; private transient ReentrantLock slaveCountingLock = new ReentrantLock(); @@ -217,13 +217,17 @@ public EC2PrivateKey resolvePrivateKey(){ /* visible for testing */ @CheckForNull - public static EC2PrivateKey fetchPrivateKeyFromDisk() { - String filename = System.getProperty(SSH_PRIVATE_KEY_FILEPATH, ""); - if (!filename.isEmpty()) { + public static EC2PrivateKey fetchPrivateKeyFromDisk() { + return fetchPrivateKeyFromDisk(System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "")); + } + + @CheckForNull + public static EC2PrivateKey fetchPrivateKeyFromDisk(String filepath) { + if (!(filepath == null) && !(filepath.isEmpty())) { try { - return new EC2PrivateKey(Files.readString(Paths.get(filename), StandardCharsets.UTF_8)); + return new EC2PrivateKey(Files.readString(Paths.get(filepath), StandardCharsets.UTF_8)); } catch (IOException e) { - LOGGER.log(Level.WARNING, "unable to read private key from file " + filename, e); + LOGGER.log(Level.WARNING, "unable to read private key from file " + filepath, e); return null; } } @@ -1177,7 +1181,11 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont return FormValidation.error("Failed to find credential \"" + value + "\" in store."); } } else { - privateKey = fetchPrivateKeyFromDisk().getPrivateKey(); + EC2PrivateKey k = fetchPrivateKeyFromDisk(); + if (k == null) { + return FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH)); + } + privateKey = k.getPrivateKey(); } boolean hasStart = false, hasEnd = false; @@ -1230,7 +1238,11 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL return FormValidation.error("Failed to find credential \"" + sshKeysCredentialsId + "\" in store."); } } else { - privateKey = fetchPrivateKeyFromDisk().getPrivateKey() ; + EC2PrivateKey k = fetchPrivateKeyFromDisk(); + if (k == null) { + return FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH)); + } + privateKey = k.getPrivateKey(); } AWSCredentialsProvider credentialsProvider = createCredentialsProvider(useInstanceProfileForCredentials, credentialsId, roleArn, roleSessionName, region); From 536de5ffae538ce3d0a133e894a436b50c0c215e Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 05:07:59 -0500 Subject: [PATCH 08/18] fixup test --- src/test/java/hudson/plugins/ec2/EC2CloudTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java index 05616a1a2..34317b716 100644 --- a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java @@ -29,9 +29,7 @@ public class EC2CloudTest { @Test public void testFileBasedSShKey() { - System.setProperty("hudson.plugins.ec2.EC2Cloud.sshPrivateKeyFilePath", getClass().getClassLoader().getResource("hudson/plugins/ec2/test.pem").getPath()); - assertTrue("file content should not have been empty", EC2Cloud.fetchPrivateKeyFromDisk() != null); - System.setProperty("hudson.plugins.ec2.EC2Cloud.sshPrivateKeyFilePath",null); + assertTrue("file content should not have been empty", EC2Cloud.fetchPrivateKeyFromDisk(getClass().getClassLoader().getResource("hudson/plugins/ec2/test.pem").getPath()) != null); } @Test From 8687121cee46b6cc7c5b8aa064199eafacc1b4a5 Mon Sep 17 00:00:00 2001 From: michael cirioli Date: Fri, 8 Nov 2024 08:42:27 -0500 Subject: [PATCH 09/18] cleaner code Co-authored-by: Francisco Javier Fernandez <31063239+fcojfernandez@users.noreply.github.com> --- README.md | 2 +- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b25986cb5..a9492c135 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ config). If you do not want to create a new Jenkins credential you may alterantively store it in plain text on disk, indicating its file path via the Jenkins system property -`SSH_KEY_PAIR_PRIVATE_KEY_FILE`. If this system property has a non-empty value then +`hudson.plugins.ec2.EC2Cloud.sshPrivateKeyFilePath`. If this system property has a non-empty value then it will override the ssh credential specified in the cloud configuration page. This approach works well for `k8s` secrets that are mounted in a jenkins container for example. diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 49ef02db0..ca7995cb8 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -223,7 +223,7 @@ public static EC2PrivateKey fetchPrivateKeyFromDisk() { @CheckForNull public static EC2PrivateKey fetchPrivateKeyFromDisk(String filepath) { - if (!(filepath == null) && !(filepath.isEmpty())) { + if (StringUtils.isNotEmpty(filepath)) { try { return new EC2PrivateKey(Files.readString(Paths.get(filepath), StandardCharsets.UTF_8)); } catch (IOException e) { From 74c74f5b448ea6c2bd3f08eddb0ad36f12113ef6 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 08:58:24 -0500 Subject: [PATCH 10/18] refactor --- .../java/hudson/plugins/ec2/EC2Cloud.java | 28 ++---------------- .../hudson/plugins/ec2/EC2PrivateKey.java | 29 +++++++++++++++++++ .../java/hudson/plugins/ec2/EC2CloudTest.java | 2 +- 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index ca7995cb8..a8a79059f 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -87,9 +87,6 @@ import java.net.MalformedURLException; import java.net.Proxy; import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Paths; import java.util.*; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @@ -204,7 +201,7 @@ protected EC2Cloud(String id, boolean useInstanceProfileForCredentials, String c public EC2PrivateKey resolvePrivateKey(){ if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { LOGGER.fine(() -> "(resolvePrivateKey) secret key file configured, will load from disk"); - return fetchPrivateKeyFromDisk(); + return EC2PrivateKey.fetchFromDisk(); } else if (sshKeysCredentialsId != null) { LOGGER.fine(() -> "(resolvePrivateKey) Using jenkins ssh credential"); SSHUserPrivateKey privateKeyCredential = getSshCredential(sshKeysCredentialsId, Jenkins.get()); @@ -215,25 +212,6 @@ public EC2PrivateKey resolvePrivateKey(){ return null; } - /* visible for testing */ - @CheckForNull - public static EC2PrivateKey fetchPrivateKeyFromDisk() { - return fetchPrivateKeyFromDisk(System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "")); - } - - @CheckForNull - public static EC2PrivateKey fetchPrivateKeyFromDisk(String filepath) { - if (StringUtils.isNotEmpty(filepath)) { - try { - return new EC2PrivateKey(Files.readString(Paths.get(filepath), StandardCharsets.UTF_8)); - } catch (IOException e) { - LOGGER.log(Level.WARNING, "unable to read private key from file " + filepath, e); - return null; - } - } - return null; - } - public abstract URL getEc2EndpointUrl() throws IOException; public abstract URL getS3EndpointUrl() throws IOException; @@ -1181,7 +1159,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont return FormValidation.error("Failed to find credential \"" + value + "\" in store."); } } else { - EC2PrivateKey k = fetchPrivateKeyFromDisk(); + EC2PrivateKey k = EC2PrivateKey.fetchFromDisk(); if (k == null) { return FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH)); } @@ -1238,7 +1216,7 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL return FormValidation.error("Failed to find credential \"" + sshKeysCredentialsId + "\" in store."); } } else { - EC2PrivateKey k = fetchPrivateKeyFromDisk(); + EC2PrivateKey k = EC2PrivateKey.fetchFromDisk(); if (k == null) { return FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH)); } diff --git a/src/main/java/hudson/plugins/ec2/EC2PrivateKey.java b/src/main/java/hudson/plugins/ec2/EC2PrivateKey.java index a15728cac..bc5bba2ea 100644 --- a/src/main/java/hudson/plugins/ec2/EC2PrivateKey.java +++ b/src/main/java/hudson/plugins/ec2/EC2PrivateKey.java @@ -26,6 +26,9 @@ import java.io.BufferedReader; import java.io.IOException; import java.io.StringReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.security.UnrecoverableKeyException; import java.util.Base64; @@ -33,15 +36,20 @@ import com.amazonaws.services.ec2.AmazonEC2; import com.amazonaws.services.ec2.model.KeyPairInfo; +import edu.umd.cs.findbugs.annotations.CheckForNull; import hudson.util.Secret; import jenkins.bouncycastle.api.PEMEncodable; import javax.crypto.Cipher; import java.nio.charset.Charset; +import java.util.logging.Level; +import java.util.logging.Logger; import org.apache.commons.lang.StringUtils; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; +import static hudson.plugins.ec2.EC2Cloud.SSH_PRIVATE_KEY_FILEPATH; + /** * RSA private key (the one that you generate with ec2-add-keypair.) * @@ -51,6 +59,8 @@ */ public class EC2PrivateKey { + private static final Logger LOGGER = Logger.getLogger(EC2PrivateKey.class.getName()); + private final Secret privateKey; EC2PrivateKey(String privateKey) { @@ -143,6 +153,25 @@ public String decryptWindowsPassword(String encodedPassword) throws AmazonClient } } + /* visible for testing */ + @CheckForNull + public static EC2PrivateKey fetchFromDisk() { + return fetchFromDisk(System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "")); + } + + @CheckForNull + public static EC2PrivateKey fetchFromDisk(String filepath) { + if (StringUtils.isNotEmpty(filepath)) { + try { + return new EC2PrivateKey(Files.readString(Paths.get(filepath), StandardCharsets.UTF_8)); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "unable to read private key from file " + filepath, e); + return null; + } + } + return null; + } + @Override public int hashCode() { return privateKey.hashCode(); diff --git a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java index 34317b716..2f6cb71ee 100644 --- a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java @@ -29,7 +29,7 @@ public class EC2CloudTest { @Test public void testFileBasedSShKey() { - assertTrue("file content should not have been empty", EC2Cloud.fetchPrivateKeyFromDisk(getClass().getClassLoader().getResource("hudson/plugins/ec2/test.pem").getPath()) != null); + assertNotNull("file content should not have been empty", EC2PrivateKey.fetchFromDisk(getClass().getClassLoader().getResource("hudson/plugins/ec2/test.pem").getPath())); } @Test From 83f632b5f94bed88eb7390f68f44222852fff0c6 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 08:59:01 -0500 Subject: [PATCH 11/18] more user friendly --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index a8a79059f..2bd4ede27 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -1148,6 +1148,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont String privateKey; if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { + // not using a static ssh key file if (value == null || value.isEmpty()) { return FormValidation.error("No ssh credentials selected"); } @@ -1182,6 +1183,14 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont if (!hasEnd) return FormValidation .error("The private key is missing the trailing 'END RSA PRIVATE KEY' marker. Copy&paste error?"); + + if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { + if (!StringUtils.isEmpty(value)) { + return FormValidation.ok("Using private key file instead of selected credential"); + } else { + return FormValidation.ok("Using private key file"); + } + } return FormValidation.ok(); } @@ -1236,6 +1245,14 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL + pk.getFingerprint() + ")"); } + if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { + if (!StringUtils.isEmpty(sshKeysCredentialsId)) { + return FormValidation.ok("Using private key file instead of selected credential"); + } else { + return FormValidation.ok("Using private key file"); + } + } + return FormValidation.ok(Messages.EC2Cloud_Success()); } catch (AmazonClientException e) { LOGGER.log(Level.WARNING, "Failed to check EC2 credential", e); From d9094f65ba8bb91e857a57543ee61422b1f8da70 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 09:58:44 -0500 Subject: [PATCH 12/18] somewhat better test --- .../java/hudson/plugins/ec2/EC2CloudTest.java | 6 ----- .../plugins/ec2/FileBasedSSHKeyTest.java | 26 +++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java diff --git a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java index 2f6cb71ee..5ec6a49ae 100644 --- a/src/test/java/hudson/plugins/ec2/EC2CloudTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2CloudTest.java @@ -26,12 +26,6 @@ @RunWith(MockitoJUnitRunner.class) public class EC2CloudTest { - - @Test - public void testFileBasedSShKey() { - assertNotNull("file content should not have been empty", EC2PrivateKey.fetchFromDisk(getClass().getClassLoader().getResource("hudson/plugins/ec2/test.pem").getPath())); - } - @Test public void testSlaveTemplateAddition() throws Exception { AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, diff --git a/src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java b/src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java new file mode 100644 index 000000000..25ec0d1c6 --- /dev/null +++ b/src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java @@ -0,0 +1,26 @@ +package hudson.plugins.ec2; + +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.RealJenkinsRule; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +public class FileBasedSSHKeyTest { + @Rule + public RealJenkinsRule r = new RealJenkinsRule().javaOptions("-D" + EC2Cloud.class.getName() + ".sshPrivateKeyFilePath=" + + getClass().getClassLoader().getResource("hudson/plugins/ec2/test.pem").getPath()); + + @Test + public void testFileBasedSShKey() throws Throwable { + r.startJenkins(); + r.runRemotely(FileBasedSSHKeyTest::verifyKeyFile); + } + + private static void verifyKeyFile(JenkinsRule r) throws Throwable { + assertNotNull("file content should not have been empty", EC2PrivateKey.fetchFromDisk()); + assertEquals("file content did not match", EC2PrivateKey.fetchFromDisk().getPrivateKey(),"hello, world!"); + } +} From d25dc754619f9d1d0cfec2102b49e99247c62547 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 10:41:28 -0500 Subject: [PATCH 13/18] doh! --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 2bd4ede27..e94e15895 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -1184,7 +1184,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont return FormValidation .error("The private key is missing the trailing 'END RSA PRIVATE KEY' marker. Copy&paste error?"); - if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { + if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { if (!StringUtils.isEmpty(value)) { return FormValidation.ok("Using private key file instead of selected credential"); } else { @@ -1245,7 +1245,7 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL + pk.getFingerprint() + ")"); } - if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { + if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { if (!StringUtils.isEmpty(sshKeysCredentialsId)) { return FormValidation.ok("Using private key file instead of selected credential"); } else { From dffd59972fbdd60cfcafd1ffa337aa40d3046fde Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 10:41:50 -0500 Subject: [PATCH 14/18] better logs and validations --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index e94e15895..a5230332a 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -1186,7 +1186,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { if (!StringUtils.isEmpty(value)) { - return FormValidation.ok("Using private key file instead of selected credential"); + return FormValidation.warning("Using private key file instead of selected credential"); } else { return FormValidation.ok("Using private key file"); } @@ -1216,8 +1216,10 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL return FormValidation.ok(); } try { + LOGGER.fine(() -> "begin doTestConnection()"); String privateKey = ""; if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { + LOGGER.fine(() -> "static credential is in use"); SSHUserPrivateKey sshCredential = getSshCredential(sshKeysCredentialsId, context); if (sshCredential != null) { privateKey = sshCredential.getPrivateKey(); @@ -1225,12 +1227,14 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL return FormValidation.error("Failed to find credential \"" + sshKeysCredentialsId + "\" in store."); } } else { + LOGGER.fine(() -> "using static ssh keyfile"); EC2PrivateKey k = EC2PrivateKey.fetchFromDisk(); if (k == null) { return FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH)); } privateKey = k.getPrivateKey(); } + LOGGER.fine(() -> "private key found ok"); AWSCredentialsProvider credentialsProvider = createCredentialsProvider(useInstanceProfileForCredentials, credentialsId, roleArn, roleSessionName, region); AmazonEC2 ec2 = AmazonEC2Factory.getInstance().connect(credentialsProvider, ec2endpoint); @@ -1247,7 +1251,7 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { if (!StringUtils.isEmpty(sshKeysCredentialsId)) { - return FormValidation.ok("Using private key file instead of selected credential"); + return FormValidation.warning("Using private key file instead of selected credential"); } else { return FormValidation.ok("Using private key file"); } From 5cbf9500f078ae1176548d7124479c0dca660b06 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 10:55:12 -0500 Subject: [PATCH 15/18] better validation code --- .../java/hudson/plugins/ec2/EC2Cloud.java | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index a5230332a..68eca639c 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -1150,7 +1150,7 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { // not using a static ssh key file if (value == null || value.isEmpty()) { - return FormValidation.error("No ssh credentials selected"); + return FormValidation.error("No ssh credentials selected and no static key file defined"); } SSHUserPrivateKey sshCredential = getSshCredential(value, context); @@ -1167,6 +1167,8 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont privateKey = k.getPrivateKey(); } + List validations = new ArrayList<>(); + boolean hasStart = false, hasEnd = false; BufferedReader br = new BufferedReader(new StringReader(privateKey)); String line; @@ -1179,19 +1181,18 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont hasEnd = true; } if (!hasStart) - return FormValidation.error("This doesn't look like a private key at all"); + validations.add(FormValidation.error("This doesn't look like a private key at all")); if (!hasEnd) - return FormValidation - .error("The private key is missing the trailing 'END RSA PRIVATE KEY' marker. Copy&paste error?"); + validations.add(FormValidation.error("The private key is missing the trailing 'END RSA PRIVATE KEY' marker. Copy&paste error?")); if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { if (!StringUtils.isEmpty(value)) { - return FormValidation.warning("Using private key file instead of selected credential"); + validations.add(FormValidation.warning("Using private key file instead of selected credential")); } else { - return FormValidation.ok("Using private key file"); + validations.add(FormValidation.ok("Using private key file")); } } - return FormValidation.ok(); + return validations.isEmpty() ? FormValidation.ok() : FormValidation.aggregate(validations); } /** @@ -1240,24 +1241,26 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL AmazonEC2 ec2 = AmazonEC2Factory.getInstance().connect(credentialsProvider, ec2endpoint); ec2.describeInstances(); + List validations = new ArrayList<>(); + if (privateKey.trim().length() > 0) { // check if this key exists EC2PrivateKey pk = new EC2PrivateKey(privateKey); if (pk.find(ec2) == null) - return FormValidation + validations.add(FormValidation .error("The EC2 key pair private key isn't registered to this EC2 region (fingerprint is " - + pk.getFingerprint() + ")"); + + pk.getFingerprint() + ")")); } if (!System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { if (!StringUtils.isEmpty(sshKeysCredentialsId)) { - return FormValidation.warning("Using private key file instead of selected credential"); + validations.add(FormValidation.warning("Using private key file instead of selected credential")); } else { - return FormValidation.ok("Using private key file"); + validations.add(FormValidation.ok("Using private key file")); } } - return FormValidation.ok(Messages.EC2Cloud_Success()); + return validations.isEmpty() ? FormValidation.ok(Messages.EC2Cloud_Success()) : FormValidation.aggregate(validations); } catch (AmazonClientException e) { LOGGER.log(Level.WARNING, "Failed to check EC2 credential", e); return FormValidation.error(e.getMessage()); From 4c493cc04e1a34b2366da46c3018624b767f7aaf Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 11:03:13 -0500 Subject: [PATCH 16/18] aggregate sensibly --- src/main/java/hudson/plugins/ec2/EC2Cloud.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 68eca639c..2b6889239 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -1192,7 +1192,9 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont validations.add(FormValidation.ok("Using private key file")); } } - return validations.isEmpty() ? FormValidation.ok() : FormValidation.aggregate(validations); + + validations.add(FormValidation.ok("SSH key validation successful")); + return FormValidation.aggregate(validations); } /** @@ -1259,8 +1261,8 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL validations.add(FormValidation.ok("Using private key file")); } } - - return validations.isEmpty() ? FormValidation.ok(Messages.EC2Cloud_Success()) : FormValidation.aggregate(validations); + validations.add(FormValidation.ok(Messages.EC2Cloud_Success())); + return FormValidation.aggregate(validations); } catch (AmazonClientException e) { LOGGER.log(Level.WARNING, "Failed to check EC2 credential", e); return FormValidation.error(e.getMessage()); From 7e4f0fa2b8f3ca8ceb079a3a28052b0171760c25 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Fri, 8 Nov 2024 12:15:57 -0500 Subject: [PATCH 17/18] better validation --- .../java/hudson/plugins/ec2/EC2Cloud.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java index 2b6889239..138a94179 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Cloud.java +++ b/src/main/java/hudson/plugins/ec2/EC2Cloud.java @@ -1146,11 +1146,12 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont } String privateKey; + List validations = new ArrayList<>(); if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { // not using a static ssh key file if (value == null || value.isEmpty()) { - return FormValidation.error("No ssh credentials selected and no static key file defined"); + return FormValidation.error("No ssh credentials selected and no private key file defined"); } SSHUserPrivateKey sshCredential = getSshCredential(value, context); @@ -1162,12 +1163,15 @@ public FormValidation doCheckSshKeysCredentialsId(@AncestorInPath ItemGroup cont } else { EC2PrivateKey k = EC2PrivateKey.fetchFromDisk(); if (k == null) { - return FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH)); + validations.add(FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH))); + if (!StringUtils.isEmpty(value)) { + validations.add(FormValidation.warning("Private key file path defined, selected credential will be ignored")); + } + return FormValidation.aggregate(validations); } privateKey = k.getPrivateKey(); } - List validations = new ArrayList<>(); boolean hasStart = false, hasEnd = false; BufferedReader br = new BufferedReader(new StringReader(privateKey)); @@ -1219,6 +1223,8 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL return FormValidation.ok(); } try { + List validations = new ArrayList<>(); + LOGGER.fine(() -> "begin doTestConnection()"); String privateKey = ""; if (System.getProperty(SSH_PRIVATE_KEY_FILEPATH, "").isEmpty()) { @@ -1230,10 +1236,13 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL return FormValidation.error("Failed to find credential \"" + sshKeysCredentialsId + "\" in store."); } } else { - LOGGER.fine(() -> "using static ssh keyfile"); EC2PrivateKey k = EC2PrivateKey.fetchFromDisk(); if (k == null) { - return FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH)); + validations.add(FormValidation.error("Failed to find private key file " + System.getProperty(SSH_PRIVATE_KEY_FILEPATH))); + if (!StringUtils.isEmpty(sshKeysCredentialsId)) { + validations.add(FormValidation.warning("Private key file path defined, selected credential will be ignored")); + } + return FormValidation.aggregate(validations); } privateKey = k.getPrivateKey(); } @@ -1243,7 +1252,6 @@ protected FormValidation doTestConnection(@AncestorInPath ItemGroup context, URL AmazonEC2 ec2 = AmazonEC2Factory.getInstance().connect(credentialsProvider, ec2endpoint); ec2.describeInstances(); - List validations = new ArrayList<>(); if (privateKey.trim().length() > 0) { // check if this key exists From 5ec3dde0729c12ca2abfa36fe37f2c63f7ec0016 Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Mon, 11 Nov 2024 09:25:13 -0500 Subject: [PATCH 18/18] even better test --- .../java/hudson/plugins/ec2/FileBasedSSHKeyTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java b/src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java index 25ec0d1c6..37ac265ae 100644 --- a/src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java +++ b/src/test/java/hudson/plugins/ec2/FileBasedSSHKeyTest.java @@ -5,6 +5,8 @@ import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RealJenkinsRule; +import java.util.Collections; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -17,10 +19,18 @@ public class FileBasedSSHKeyTest { public void testFileBasedSShKey() throws Throwable { r.startJenkins(); r.runRemotely(FileBasedSSHKeyTest::verifyKeyFile); + r.runRemotely(FileBasedSSHKeyTest::verifyCorrectKeyIsResolved); } private static void verifyKeyFile(JenkinsRule r) throws Throwable { assertNotNull("file content should not have been empty", EC2PrivateKey.fetchFromDisk()); assertEquals("file content did not match", EC2PrivateKey.fetchFromDisk().getPrivateKey(),"hello, world!"); } + + private static void verifyCorrectKeyIsResolved(JenkinsRule r) throws Throwable { + AmazonEC2Cloud cloud = new AmazonEC2Cloud("us-east-1", true, "abc", "us-east-1", null, "ghi", "3", Collections.emptyList(), "roleArn", "roleSessionName"); + r.jenkins.clouds.add(cloud); + AmazonEC2Cloud c = r.jenkins.clouds.get(AmazonEC2Cloud.class); + assertEquals("An unexpected key was returned!", c.resolvePrivateKey().getPrivateKey(),"hello, world!"); + } }