From 8e3d49401f0a8d24690cd29522eca1414f659263 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Tue, 30 May 2023 16:31:05 +0200 Subject: Create singleton CloudNames for known clouds --- .../com/yahoo/config/provision/CloudNameTest.java | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 config-provisioning/src/test/java/com/yahoo/config/provision/CloudNameTest.java (limited to 'config-provisioning/src/test/java/com/yahoo/config/provision') diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/CloudNameTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/CloudNameTest.java new file mode 100644 index 00000000000..b030233d459 --- /dev/null +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/CloudNameTest.java @@ -0,0 +1,22 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.config.provision; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * @author freva + */ +class CloudNameTest { + + @Test + void returns_same_instance_for_known_clouds() { + assertSame(CloudName.from("aws"), CloudName.AWS); + assertSame(CloudName.from("gcp"), CloudName.GCP); + assertSame(CloudName.from("default"), CloudName.DEFAULT); + assertSame(CloudName.from("yahoo"), CloudName.YAHOO); + assertThrows(IllegalArgumentException.class, () -> CloudName.from("aWs")); // Must be lower case + } +} -- cgit v1.2.3 From fbdc11ed2c64aa29d7c6b567bc6aad5ff86d8135 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Tue, 30 May 2023 16:31:26 +0200 Subject: Add CloudName to CloudAccount --- .../change/CloudAccountChangeValidatorTest.java | 6 +- .../com/yahoo/config/provision/CloudAccount.java | 96 ++++++++++++++-------- .../yahoo/config/provision/CloudAccountTest.java | 75 +++++++++++++++++ .../maintenance/EnclaveAccessMaintainerTest.java | 2 +- .../hosted/provision/restapi/ArchiveResponse.java | 2 +- 5 files changed, 143 insertions(+), 38 deletions(-) create mode 100644 config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java (limited to 'config-provisioning/src/test/java/com/yahoo/config/provision') diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CloudAccountChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CloudAccountChangeValidatorTest.java index a8a063cb5fb..77704817045 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CloudAccountChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CloudAccountChangeValidatorTest.java @@ -1,14 +1,14 @@ package com.yahoo.vespa.model.application.validation.change; -import com.yahoo.config.provision.ClusterInfo; -import com.yahoo.config.provision.IntRange; import com.yahoo.config.model.api.Provisioned; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.ClusterInfo; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.IntRange; import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; @@ -36,7 +36,7 @@ class CloudAccountChangeValidatorTest { fail("Expected exception"); } catch (IllegalArgumentException e) { assertEquals(e.getMessage(), "Cannot change cloud account from unspecified account to " + - "account '000000000000'. The existing deployment must be removed before " + + "account '000000000000' in aws. The existing deployment must be removed before " + "changing accounts"); } assertEquals(List.of(), validator.validate(model0, model0, new DeployState.Builder().build())); diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java b/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java index 215afbca255..465e4573d60 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java @@ -1,40 +1,41 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision; -import ai.vespa.validation.PatternedStringWrapper; -import ai.vespa.validation.Validation; - +import java.util.Map; +import java.util.Objects; import java.util.regex.Pattern; +import java.util.stream.Collectors; /** * Identifies an account in a public cloud, such as {@link CloudName#AWS} or {@link CloudName#GCP}. * * @author mpolden */ -public class CloudAccount extends PatternedStringWrapper { +public class CloudAccount implements Comparable { - private static final String EMPTY = ""; - private static final String AWS_ACCOUNT_ID = "[0-9]{12}"; - private static final Pattern AWS_ACCOUNT_ID_PATTERN = Pattern.compile(AWS_ACCOUNT_ID); - private static final String GCP_PROJECT_ID = "[a-z][a-z0-9-]{4,28}[a-z0-9]"; - private static final Pattern GCP_PROJECT_ID_PATTERN = Pattern.compile(GCP_PROJECT_ID); + private record CloudMeta(String accountType, Pattern pattern) { + private boolean matches(String account) { return pattern.matcher(account).matches(); } + } + private static final Map META_BY_CLOUD = Map.of( + "aws", new CloudMeta("Account ID", Pattern.compile("[0-9]{12}")), + "gcp", new CloudMeta("Project name", Pattern.compile("[a-z][a-z0-9-]{4,28}[a-z0-9]"))); /** Empty value. When this is used, either implicitly or explicitly, the zone will use its default account */ - public static final CloudAccount empty = new CloudAccount("", EMPTY, "cloud account"); + public static final CloudAccount empty = new CloudAccount("", CloudName.DEFAULT); - /** Verifies accountId is a valid AWS account ID, or throw an IllegalArgumentException. */ - public static void requireAwsAccountId(String accountId) { - Validation.requireMatch(accountId, "AWS account ID", AWS_ACCOUNT_ID_PATTERN); - } + private final String account; + private final CloudName cloudName; - /** Verifies accountId is a valid GCP project ID, or throw an IllegalArgumentException. */ - public static void requireGcpProjectId(String projectId) { - Validation.requireMatch(projectId, "GCP project ID", GCP_PROJECT_ID_PATTERN); + private CloudAccount(String account, CloudName cloudName) { + this.account = account; + this.cloudName = cloudName; } - private CloudAccount(String value, String regex, String description) { - super(value, Pattern.compile("^(" + regex + ")$"), description); - } + public String account() { return account; } + public CloudName cloudName() { return cloudName; } + + /** Returns the serialized value of this account that can be deserialized with {@link CloudAccount#from} */ + public final String value() { return account; } // TODO (freva): Change to cloudName:account public boolean isUnspecified() { return this.equals(empty); @@ -47,27 +48,56 @@ public class CloudAccount extends PatternedStringWrapper { !equals(zone.cloud().account()); } - /** Verifies this account is a valid AWS account ID, or throw an IllegalArgumentException. */ - public void requireAwsAccountId() { - requireAwsAccountId(value()); + @Override + public String toString() { + return isUnspecified() ? "unspecified account" : "account '" + account + "' in " + cloudName; } - /** Verifies this account is a valid GCP project ID, or throw an IllegalArgumentException. */ - public void requireGcpProjectId() { - requireGcpProjectId(value()); + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CloudAccount that = (CloudAccount) o; + return account.equals(that.account) && cloudName.equals(that.cloudName); } + @Override + public int hashCode() { + return Objects.hash(account, cloudName); + } + + @Override + public int compareTo(CloudAccount o) { + return this.value().compareTo(o.value()); + } + + public static CloudAccount from(String cloudAccount) { - return switch (cloudAccount) { + int index = cloudAccount.indexOf(':'); + if (index < 0) { // Tenants are allowed to specify "default" in services.xml. - case "", "default" -> empty; - default -> new CloudAccount(cloudAccount, AWS_ACCOUNT_ID + "|" + GCP_PROJECT_ID, "cloud account"); - }; + if (cloudAccount.isEmpty() || cloudAccount.equals("default")) + return empty; + if (META_BY_CLOUD.get("aws").matches(cloudAccount)) + return new CloudAccount(cloudAccount, CloudName.AWS); + if (META_BY_CLOUD.get("gcp").matches(cloudAccount)) // TODO (freva): Remove July 2023 + return new CloudAccount(cloudAccount, CloudName.GCP); + throw illegal(cloudAccount, "Must be on format ':' or 'default'"); + } + + String cloud = cloudAccount.substring(0, index); + String account = cloudAccount.substring(index + 1); + CloudMeta cloudMeta = META_BY_CLOUD.get(cloud); + if (cloudMeta == null) + throw illegal(cloudAccount, "Cloud name must be one of: " + META_BY_CLOUD.keySet().stream().sorted().collect(Collectors.joining(", "))); + + if (!cloudMeta.matches(account)) + throw illegal(cloudAccount, cloudMeta.accountType + " must match '" + cloudMeta.pattern.pattern() + "'"); + return new CloudAccount(account, CloudName.from(cloud)); } - @Override - public String toString() { - return isUnspecified() ? "unspecified account" : "account '" + value() + "'"; + private static IllegalArgumentException illegal(String cloudAccount, String details) { + return new IllegalArgumentException("Invalid cloud account '" + cloudAccount + "': " + details); } } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java new file mode 100644 index 00000000000..f2ae9f8eab5 --- /dev/null +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java @@ -0,0 +1,75 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.config.provision; + +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * @author freva + */ +class CloudAccountTest { + + @Test + void aws_accounts() { + CloudAccount oldFormat = CloudAccount.from("123456789012"); + CloudAccount newFormat = CloudAccount.from("aws:123456789012"); + assertEquals(oldFormat, newFormat); + + for (CloudAccount account : List.of(oldFormat, newFormat)) { + assertFalse(account.isUnspecified()); + assertEquals(account, CloudAccount.from(account.value())); + assertEquals("123456789012", account.account()); + assertEquals(CloudName.AWS, account.cloudName()); + assertEquals("123456789012", account.value()); + } + } + + @Test + void gcp_accounts() { + CloudAccount oldFormat = CloudAccount.from("my-project"); + CloudAccount newFormat = CloudAccount.from("gcp:my-project"); + assertEquals(oldFormat, newFormat); + + for (CloudAccount account : List.of(oldFormat, newFormat)) { + assertFalse(account.isUnspecified()); + assertEquals(account, CloudAccount.from(account.value())); + assertEquals("my-project", account.account()); + assertEquals(CloudName.GCP, account.cloudName()); + assertEquals("my-project", account.value()); + } + } + + @Test + void default_accounts() { + CloudAccount variant1 = CloudAccount.from(""); + CloudAccount variant2 = CloudAccount.from("default"); + assertEquals(variant1, variant2); + + for (CloudAccount account : List.of(variant1, variant2)) { + assertTrue(account.isUnspecified()); + assertEquals(account, CloudAccount.from(account.value())); + assertEquals("", account.account()); + assertEquals(CloudName.DEFAULT, account.cloudName()); + assertEquals("", account.value()); + } + } + + @Test + void invalid_accounts() { + assertInvalidAccount("aws:123", "Invalid cloud account 'aws:123': Account ID must match '[0-9]{12}'"); + assertInvalidAccount("gcp:123", "Invalid cloud account 'gcp:123': Project name must match '[a-z][a-z0-9-]{4,28}[a-z0-9]'"); + assertInvalidAccount("$something", "Invalid cloud account '$something': Must be on format ':' or 'default'"); + assertInvalidAccount("unknown:account", "Invalid cloud account 'unknown:account': Cloud name must be one of: aws, gcp"); + } + + private static void assertInvalidAccount(String account, String message) { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> CloudAccount.from(account)); + assertEquals(message, exception.getMessage()); + } +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EnclaveAccessMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EnclaveAccessMaintainerTest.java index f5188d52db6..5bfac2866ce 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EnclaveAccessMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EnclaveAccessMaintainerTest.java @@ -33,7 +33,7 @@ class EnclaveAccessMaintainerTest { tester.flagSource().withListFlag(PermanentFlags.CLOUD_ACCOUNTS.id(), List.of("123123123123", "321321321321"), String.class); assertEquals(1, sharer.maintain()); - assertEquals(Set.of(CloudAccount.from("123123123123"), CloudAccount.from("321321321321")), amis.currentAccounts()); + assertEquals(Set.of(CloudAccount.from("aws:123123123123"), CloudAccount.from("aws:321321321321")), amis.currentAccounts()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ArchiveResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ArchiveResponse.java index 84c82d314c9..6370b01af23 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ArchiveResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ArchiveResponse.java @@ -27,7 +27,7 @@ public class ArchiveResponse extends SlimeJsonResponse { archiveObject.setString("uri", entry.getValue()); }); archiveUris.accountArchiveUris().entrySet().stream() - .sorted(Map.Entry.comparingByKey()) + .sorted() .forEach(entry -> { Cursor archiveObject = archivesArray.addObject(); archiveObject.setString("account", entry.getKey().value()); -- cgit v1.2.3 From e3f3e2723285a98ed330d231b60eb9ace632a87a Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Tue, 30 May 2023 17:57:14 +0200 Subject: Project name -> Project ID --- .../src/main/java/com/yahoo/config/provision/CloudAccount.java | 2 +- .../src/test/java/com/yahoo/config/provision/CloudAccountTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'config-provisioning/src/test/java/com/yahoo/config/provision') diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java b/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java index 465e4573d60..8026e4c5205 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/CloudAccount.java @@ -18,7 +18,7 @@ public class CloudAccount implements Comparable { } private static final Map META_BY_CLOUD = Map.of( "aws", new CloudMeta("Account ID", Pattern.compile("[0-9]{12}")), - "gcp", new CloudMeta("Project name", Pattern.compile("[a-z][a-z0-9-]{4,28}[a-z0-9]"))); + "gcp", new CloudMeta("Project ID", Pattern.compile("[a-z][a-z0-9-]{4,28}[a-z0-9]"))); /** Empty value. When this is used, either implicitly or explicitly, the zone will use its default account */ public static final CloudAccount empty = new CloudAccount("", CloudName.DEFAULT); diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java index f2ae9f8eab5..4eee52def6c 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/CloudAccountTest.java @@ -63,7 +63,7 @@ class CloudAccountTest { @Test void invalid_accounts() { assertInvalidAccount("aws:123", "Invalid cloud account 'aws:123': Account ID must match '[0-9]{12}'"); - assertInvalidAccount("gcp:123", "Invalid cloud account 'gcp:123': Project name must match '[a-z][a-z0-9-]{4,28}[a-z0-9]'"); + assertInvalidAccount("gcp:123", "Invalid cloud account 'gcp:123': Project ID must match '[a-z][a-z0-9-]{4,28}[a-z0-9]'"); assertInvalidAccount("$something", "Invalid cloud account '$something': Must be on format ':' or 'default'"); assertInvalidAccount("unknown:account", "Invalid cloud account 'unknown:account': Cloud name must be one of: aws, gcp"); } -- cgit v1.2.3