From a08f069483a6604362396cd7d15bd2c5af174392 Mon Sep 17 00:00:00 2001 From: Wido den Hollander Date: Mon, 14 Sep 2026 12:39:58 +0000 Subject: [PATCH] ConfigDrive: write every SSH key as its own entry in the OpenStack metadata An Instance with more than one SSH keypair gets its keys as a single newline-joined string. The OpenStack meta_data.json builder passed that string through as one key, so "keys" held a single object and "public_keys" a single map entry whose value contained both keys with a newline in between. cloud-init treats every public_keys value as one key and never splits it, so only the first key worked at best. The name derived from the third whitespace-separated token could also end up containing a newline, and the replace("\\n", "") calls stripped a literal backslash-n rather than a newline and did nothing. The builder now splits the content on line breaks and emits one "keys" object and one "public_keys" entry per key. A key is named after its comment when it has one no earlier key used, otherwise "key" for a lone key and key0, key1, ... when there are several. --- .../configdrive/ConfigDriveBuilder.java | 52 +++++++---- .../configdrive/ConfigDriveBuilderTest.java | 90 +++++++++++++++++++ 2 files changed, 125 insertions(+), 17 deletions(-) diff --git a/engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java b/engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java index 15febbe972c4..333c6bf50b37 100644 --- a/engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java +++ b/engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java @@ -31,9 +31,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import com.cloud.network.Network; import com.cloud.vm.NicProfile; @@ -505,33 +507,49 @@ private static JsonArray arrayOf(JsonElement... elements) { return array; } - private static void buildOpenStackMetaData(JsonObject metaData, String dataType, String fileName, String content) { + protected static void buildOpenStackMetaData(JsonObject metaData, String dataType, String fileName, String content) { if (!NetworkModel.METATDATA_DIR.equals(dataType)) { return; } if (StringUtils.isEmpty(content)) { return; } - //keys are a special case in OpenStack format if (NetworkModel.PUBLIC_KEYS_FILE.equals(fileName)) { - String[] keyArray = content.replace("\\n", "").split(" "); - String keyName = "key"; - if (keyArray.length > 3 && StringUtils.isNotEmpty(keyArray[2])) { - keyName = keyArray[2]; - } - - JsonObject keyLegacy = new JsonObject(); - keyLegacy.addProperty("type", "ssh"); - keyLegacy.addProperty("data", content.replace("\\n", "")); - keyLegacy.addProperty("name", keyName); - metaData.add("keys", arrayOf(keyLegacy)); - - JsonObject key = new JsonObject(); - key.addProperty(keyName, content); - metaData.add("public_keys", key); + buildOpenStackPublicKeys(metaData, content); } else if (NetworkModel.openStackFileMapping.get(fileName) != null) { metaData.addProperty(NetworkModel.openStackFileMapping.get(fileName), content); } } + /** + * Keys are a special case in the OpenStack format. The public-keys file holds one key per line, + * while OpenStack wants one entry per key, both in the legacy "keys" list and in the "public_keys" + * map; cloud-init treats every map value as a single key. A key is named after its comment when + * it has one that no earlier key used, otherwise "key" for a lone key and key0, key1, ... when + * there are several, skipping names already used by earlier keys. + */ + private static void buildOpenStackPublicKeys(JsonObject metaData, String content) { + List publicKeys = Arrays.stream(content.split("\\R")).map(String::trim).filter(StringUtils::isNotEmpty).collect(Collectors.toList()); + JsonArray keys = new JsonArray(); + JsonObject keyMap = new JsonObject(); + for (int i = 0; i < publicKeys.size(); i++) { + String publicKey = publicKeys.get(i); + String[] parts = publicKey.split("\\s+", 3); + String name = parts.length == 3 && !keyMap.has(parts[2]) ? parts[2] : (publicKeys.size() == 1 ? "key" : "key" + i); + int nextKeyIndex = i; + while (keyMap.has(name)) { + name = "key" + ++nextKeyIndex; + } + + JsonObject key = new JsonObject(); + key.addProperty("type", "ssh"); + key.addProperty("data", publicKey); + key.addProperty("name", name); + keys.add(key); + keyMap.addProperty(name, publicKey); + } + metaData.add("keys", keys); + metaData.add("public_keys", keyMap); + } + } diff --git a/engine/storage/configdrive/src/test/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java b/engine/storage/configdrive/src/test/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java index 03ceac843997..0c963a650491 100644 --- a/engine/storage/configdrive/src/test/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java +++ b/engine/storage/configdrive/src/test/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java @@ -35,7 +35,9 @@ import java.util.Map; import com.cloud.network.Network; +import com.cloud.network.NetworkModel; import com.cloud.vm.NicProfile; +import com.google.gson.JsonElement; import com.google.gson.JsonParser; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; @@ -383,6 +385,94 @@ public void createJsonObjectWithVmDataTesT() { } } + private static final String KEY_ONE = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFeMnwadvS7Z/sN0yCVnfcMgvxrmlNr2zElAMlFvbsP2"; + private static final String KEY_TWO = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIEPnHzS1LN+3VoXrUBWhRlIbWlSiqVdPynRNDy4bnLOn"; + + private JsonObject buildOpenStackKeys(String content) { + JsonObject metadata = new JsonObject(); + ConfigDriveBuilder.buildOpenStackMetaData(metadata, NetworkModel.METATDATA_DIR, NetworkModel.PUBLIC_KEYS_FILE, content); + return metadata; + } + + private void assertOpenStackKey(JsonObject metadata, int index, String name, String data) { + JsonObject key = metadata.getAsJsonArray("keys").get(index).getAsJsonObject(); + Assert.assertEquals("ssh", key.get("type").getAsString()); + Assert.assertEquals(name, key.get("name").getAsString()); + Assert.assertEquals(data, key.get("data").getAsString()); + Assert.assertEquals(data, metadata.getAsJsonObject("public_keys").get(name).getAsString()); + } + + @Test + public void buildOpenStackMetaDataTestSingleKey() { + JsonObject metadata = buildOpenStackKeys(KEY_ONE); + + Assert.assertEquals(1, metadata.getAsJsonArray("keys").size()); + Assert.assertEquals(1, metadata.getAsJsonObject("public_keys").size()); + assertOpenStackKey(metadata, 0, "key", KEY_ONE); + } + + @Test + public void buildOpenStackMetaDataTestSingleKeyNamedAfterComment() { + JsonObject metadata = buildOpenStackKeys(KEY_ONE + " user@laptop\n"); + + Assert.assertEquals(1, metadata.getAsJsonArray("keys").size()); + assertOpenStackKey(metadata, 0, "user@laptop", KEY_ONE + " user@laptop"); + } + + @Test + public void buildOpenStackMetaDataTestMultipleKeysBecomeSeparateEntries() { + JsonObject metadata = buildOpenStackKeys(KEY_ONE + "\n" + KEY_TWO + " user@laptop"); + + Assert.assertEquals(2, metadata.getAsJsonArray("keys").size()); + Assert.assertEquals(2, metadata.getAsJsonObject("public_keys").size()); + assertOpenStackKey(metadata, 0, "key0", KEY_ONE); + assertOpenStackKey(metadata, 1, "user@laptop", KEY_TWO + " user@laptop"); + for (Map.Entry entry : metadata.getAsJsonObject("public_keys").entrySet()) { + Assert.assertFalse(entry.getValue().getAsString().contains("\n")); + } + } + + @Test + public void buildOpenStackMetaDataTestDuplicateCommentsAndBlankLines() { + JsonObject metadata = buildOpenStackKeys("\r\n" + KEY_ONE + " shared\n\n" + KEY_TWO + " shared\n"); + + Assert.assertEquals(2, metadata.getAsJsonArray("keys").size()); + assertOpenStackKey(metadata, 0, "shared", KEY_ONE + " shared"); + assertOpenStackKey(metadata, 1, "key1", KEY_TWO + " shared"); + } + + @Test + public void buildOpenStackMetaDataTestUnnamedKeyDoesNotOverwriteCommentName() { + JsonObject metadata = buildOpenStackKeys(KEY_ONE + " key1\n" + KEY_TWO); + + Assert.assertEquals(2, metadata.getAsJsonArray("keys").size()); + Assert.assertEquals(2, metadata.getAsJsonObject("public_keys").size()); + assertOpenStackKey(metadata, 0, "key1", KEY_ONE + " key1"); + assertOpenStackKey(metadata, 1, "key2", KEY_TWO); + } + + @Test + public void buildOpenStackMetaDataTestDuplicateCommentSkipsOccupiedFallbackNames() { + JsonObject metadata = buildOpenStackKeys(KEY_ONE + " key2\n" + KEY_TWO + " key3\n" + KEY_ONE + " key2"); + + Assert.assertEquals(3, metadata.getAsJsonArray("keys").size()); + Assert.assertEquals(3, metadata.getAsJsonObject("public_keys").size()); + assertOpenStackKey(metadata, 0, "key2", KEY_ONE + " key2"); + assertOpenStackKey(metadata, 1, "key3", KEY_TWO + " key3"); + assertOpenStackKey(metadata, 2, "key4", KEY_ONE + " key2"); + } + + @Test + public void buildOpenStackMetaDataTestOtherFilesAndDataTypesUntouched() { + JsonObject metadata = new JsonObject(); + ConfigDriveBuilder.buildOpenStackMetaData(metadata, NetworkModel.METATDATA_DIR, NetworkModel.PUBLIC_KEYS_FILE, ""); + ConfigDriveBuilder.buildOpenStackMetaData(metadata, NetworkModel.USERDATA_DIR, NetworkModel.PUBLIC_KEYS_FILE, KEY_ONE); + Assert.assertEquals(0, metadata.size()); + + ConfigDriveBuilder.buildOpenStackMetaData(metadata, NetworkModel.METATDATA_DIR, NetworkModel.LOCAL_HOSTNAME_FILE, "vm-1"); + Assert.assertEquals("vm-1", metadata.get(NetworkModel.openStackFileMapping.get(NetworkModel.LOCAL_HOSTNAME_FILE)).getAsString()); + } + @Test public void buildCustomUserdataParamsMetadataTestNullContent() { try (MockedStatic configDriveBuilderMocked = Mockito.mockStatic(ConfigDriveBuilder.class)) {