diff --git a/docs/usage/properties/instance/cdk/common.md b/docs/usage/properties/instance/cdk/common.md index b9a108613b0..0d8b8883e22 100644 --- a/docs/usage/properties/instance/cdk/common.md +++ b/docs/usage/properties/instance/cdk/common.md @@ -7,6 +7,8 @@ The following instance properties are commonly used throughout Sleeper. | sleeper.version | The version of Sleeper that is being used. This property is used to identify the correct jars in the S3 jars bucket and to select the correct tag in the ECR repositories. | | sleeper.admin.role.arn | The ARN of a role that has permissions to administer the instance. | | sleeper.config.bucket | The S3 bucket name used to store configuration files. | +| sleeper.account | The AWS account number. This is the AWS account that the instance is deployed in. | +| sleeper.region | The AWS region the instance is deployed in. | | sleeper.data.bucket | The S3 bucket name used to store table data. | | sleeper.tables.name.index.dynamo.table | The name of the DynamoDB table indexing tables by their name. | | sleeper.tables.id.index.dynamo.table | The name of the DynamoDB table indexing tables by their ID. | diff --git a/docs/usage/properties/instance/user/common.md b/docs/usage/properties/instance/user/common.md index 1b9693eeee7..967cf4a69e0 100644 --- a/docs/usage/properties/instance/user/common.md +++ b/docs/usage/properties/instance/user/common.md @@ -15,8 +15,6 @@ The following instance properties are commonly used throughout Sleeper. | sleeper.retain.logs.after.destroy | Whether to keep the sleeper log groups when the instance is destroyed. | true | true | | sleeper.optional.stacks | The optional stacks to deploy. Not case sensitive.
Valid values: [IngestStack, IngestBatcherStack, EmrServerlessBulkImportStack, EmrBulkImportStack, PersistentEmrBulkImportStack, EksBulkImportStack, EmrStudioStack, BulkExportStack, QueryStack, WebSocketQueryStack, AthenaStack, KeepLambdaWarmStack, CompactionStack, GarbageCollectorStack, PartitionSplittingStack, DashboardStack, TableMetricsStack] | IngestStack,IngestBatcherStack,EmrServerlessBulkImportStack,EmrStudioStack,QueryStack,CompactionStack,GarbageCollectorStack,PartitionSplittingStack,DashboardStack,TableMetricsStack | true | | sleeper.lambda.deploy.type | The deployment type for AWS Lambda. Not case sensitive.
There are two types of Lambda deployments, jar and container.
If the size of the jar file is too large, it will always be deployed as a container.
Valid values: [jar, container] | jar | true | -| sleeper.account | The AWS account number. This is the AWS account that the instance will be deployed to. | | false | -| sleeper.region | The AWS region to deploy to. | | false | | sleeper.endpoint.url | The AWS endpoint URL. This should only be set for a non-standard service endpoint. Usually this is used to set the URL to LocalStack for a locally deployed instance. | | false | | sleeper.vpc | The id of the VPC to deploy to. | | false | | sleeper.vpc.endpoint.check | Whether to check that the VPC that the instance is deployed to has an S3 endpoint. If there is no S3 endpoint then the NAT costs can be very significant. | true | false | diff --git a/example/basic/instance.properties b/example/basic/instance.properties index c6e7e058071..5f860e86252 100644 --- a/example/basic/instance.properties +++ b/example/basic/instance.properties @@ -25,12 +25,6 @@ sleeper.retain.logs.after.destroy=true # PartitionSplittingStack, DashboardStack, TableMetricsStack] sleeper.optional.stacks=IngestStack,IngestBatcherStack,EmrServerlessBulkImportStack,EmrStudioStack,QueryStack,CompactionStack,GarbageCollectorStack,PartitionSplittingStack,DashboardStack,TableMetricsStack -# The AWS account number. This is the AWS account that the instance will be deployed to. -sleeper.account=1234567890 - -# The AWS region to deploy to. -sleeper.region=eu-west-2 - # The id of the VPC to deploy to. sleeper.vpc=1234567890 diff --git a/example/full/instance.properties b/example/full/instance.properties index 1284dd69b41..de317ffd0d2 100644 --- a/example/full/instance.properties +++ b/example/full/instance.properties @@ -51,12 +51,6 @@ sleeper.optional.stacks=IngestStack,IngestBatcherStack,EmrServerlessBulkImportSt # Valid values: [jar, container] sleeper.lambda.deploy.type=jar -# The AWS account number. This is the AWS account that the instance will be deployed to. -sleeper.account=1234567890 - -# The AWS region to deploy to. -sleeper.region=eu-west-2 - # The AWS endpoint URL. This should only be set for a non-standard service endpoint. Usually this is # used to set the URL to LocalStack for a locally deployed instance. # sleeper.endpoint.url= diff --git a/java/bulk-import/bulk-import-runner/src/test/java/sleeper/bulkimport/runner/dataframe/WriteParquetFilesIT.java b/java/bulk-import/bulk-import-runner/src/test/java/sleeper/bulkimport/runner/dataframe/WriteParquetFilesIT.java index 71a630871ae..ac5d9a9f442 100644 --- a/java/bulk-import/bulk-import-runner/src/test/java/sleeper/bulkimport/runner/dataframe/WriteParquetFilesIT.java +++ b/java/bulk-import/bulk-import-runner/src/test/java/sleeper/bulkimport/runner/dataframe/WriteParquetFilesIT.java @@ -42,14 +42,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.CONFIG_BUCKET; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.DATA_BUCKET; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.FILE_SYSTEM; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.table.TableProperty.TABLE_NAME; diff --git a/java/cdk-custom-resources/src/test/java/sleeper/cdk/custom/PropertiesWriterLambdaIT.java b/java/cdk-custom-resources/src/test/java/sleeper/cdk/custom/PropertiesWriterLambdaIT.java index 0bce0ade333..5e5d6ed6269 100644 --- a/java/cdk-custom-resources/src/test/java/sleeper/cdk/custom/PropertiesWriterLambdaIT.java +++ b/java/cdk-custom-resources/src/test/java/sleeper/cdk/custom/PropertiesWriterLambdaIT.java @@ -27,12 +27,12 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.CONFIG_BUCKET; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; diff --git a/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java b/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java index 20a9f43e832..073252ee1c5 100644 --- a/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java +++ b/java/cdk/src/main/java/sleeper/cdk/SleeperCdkApp.java @@ -25,9 +25,7 @@ import sleeper.core.properties.instance.InstanceProperties; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; /** * Deploys an instance of Sleeper, including any configured optional stacks. @@ -46,18 +44,18 @@ public static void main(String[] args) { DynamoDbClient dynamoClient = DynamoDbClient.create()) { SleeperInstanceProps props = SleeperInstanceProps.fromContext(app, s3Client, dynamoClient); InstanceProperties instanceProperties = props.getInstanceProperties(); - String id = instanceProperties.get(ID); + Environment environment = Environment.builder() - .account(instanceProperties.get(ACCOUNT)) - .region(instanceProperties.get(REGION)) + .account(System.getenv("CDK_DEFAULT_ACCOUNT")) + .region(System.getenv("CDK_DEFAULT_REGION")) .build(); SleeperInstance.createAsRootStack(app, id, StackProps.builder() .stackName(id) .env(environment) .build(), - SleeperInstanceProps.fromContext(app, s3Client, dynamoClient)); + props); instanceProperties.getTags() .forEach((key, value) -> Tags.of(app).add(key, value)); diff --git a/java/cdk/src/main/java/sleeper/cdk/SleeperInstance.java b/java/cdk/src/main/java/sleeper/cdk/SleeperInstance.java index 0f0980a3c08..925030216f6 100644 --- a/java/cdk/src/main/java/sleeper/cdk/SleeperInstance.java +++ b/java/cdk/src/main/java/sleeper/cdk/SleeperInstance.java @@ -27,6 +27,9 @@ import sleeper.cdk.stack.core.SleeperInstanceRoles; import sleeper.core.properties.instance.InstanceProperties; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; + /** * Deploys an instance of Sleeper, including any configured optional stacks. *

@@ -62,6 +65,8 @@ public SleeperInstanceRoles getRoles() { */ public static SleeperInstance createAsNestedStack(Construct scope, String id, NestedStackProps stackProps, SleeperInstanceProps sleeperProps) { NestedStack stack = new NestedStack(scope, id, stackProps); + sleeperProps.getInstanceProperties().set(ACCOUNT, stack.getAccount()); + sleeperProps.getInstanceProperties().set(REGION, stack.getRegion()); return addNestedStacks(stack, sleeperProps); } @@ -76,6 +81,8 @@ public static SleeperInstance createAsNestedStack(Construct scope, String id, Ne */ public static Stack createAsRootStack(Construct scope, String id, StackProps stackProps, SleeperInstanceProps sleeperProps) { Stack stack = new Stack(scope, id, stackProps); + sleeperProps.getInstanceProperties().set(ACCOUNT, stack.getAccount()); + sleeperProps.getInstanceProperties().set(REGION, stack.getRegion()); addNestedStacks(stack, sleeperProps); return stack; } diff --git a/java/cdk/src/main/java/sleeper/cdk/SleeperInstanceProps.java b/java/cdk/src/main/java/sleeper/cdk/SleeperInstanceProps.java index 4a372c62c21..b42e2f9de0c 100644 --- a/java/cdk/src/main/java/sleeper/cdk/SleeperInstanceProps.java +++ b/java/cdk/src/main/java/sleeper/cdk/SleeperInstanceProps.java @@ -33,6 +33,7 @@ import java.nio.file.Path; import java.util.List; import java.util.Objects; +import java.util.Properties; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; import static sleeper.core.properties.instance.CommonProperty.ID; @@ -260,6 +261,10 @@ public SleeperInstanceProps build() { Objects.requireNonNull(tableProperties, "tableProperties must not be null"); Objects.requireNonNull(jars, "jars must not be null"); + Properties tagsProperties = instanceProperties.getTagsProperties(); + tagsProperties.setProperty("InstanceID", instanceProperties.get(ID)); + instanceProperties.loadTags(tagsProperties); + if (validateProperties) { instanceProperties.validate(); try { diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/AthenaStack.java b/java/cdk/src/main/java/sleeper/cdk/stack/AthenaStack.java index ca4539e65b1..70503639a22 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/AthenaStack.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/AthenaStack.java @@ -50,8 +50,8 @@ import static sleeper.core.properties.instance.AthenaProperty.ATHENA_COMPOSITE_HANDLER_TIMEOUT_IN_SECONDS; import static sleeper.core.properties.instance.AthenaProperty.ATHENA_SPILL_MASTER_KEY_ARN; import static sleeper.core.properties.instance.AthenaProperty.SPILL_BUCKET_AGE_OFF_IN_DAYS; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; -import static sleeper.core.properties.instance.CommonProperty.REGION; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") public class AthenaStack extends NestedStack { diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/DashboardStack.java b/java/cdk/src/main/java/sleeper/cdk/stack/DashboardStack.java index 1a0eafeb17d..dce21743290 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/DashboardStack.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/DashboardStack.java @@ -43,8 +43,8 @@ import java.util.Map; import java.util.stream.IntStream; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.MetricsProperty.DASHBOARD_TIME_WINDOW_MINUTES; import static sleeper.core.properties.instance.MetricsProperty.METRICS_NAMESPACE; diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/bulkexport/BulkExportTaskResources.java b/java/cdk/src/main/java/sleeper/cdk/stack/bulkexport/BulkExportTaskResources.java index fe17190df59..73738a9d58e 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/bulkexport/BulkExportTaskResources.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/bulkexport/BulkExportTaskResources.java @@ -60,8 +60,8 @@ import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_EXPORT_CLUSTER; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_EXPORT_TASK_CREATION_CLOUDWATCH_RULE; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_EXPORT_TASK_CREATION_LAMBDA_FUNCTION; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_MEMORY_IN_MB; import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_TIMEOUT_IN_SECONDS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/CommonEmrBulkImportStack.java b/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/CommonEmrBulkImportStack.java index 4dbfc382c3c..fbdfe6b68a5 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/CommonEmrBulkImportStack.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/CommonEmrBulkImportStack.java @@ -52,11 +52,11 @@ import java.util.Map; import java.util.stream.Collectors; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_IMPORT_EMR_CLUSTER_ROLE_NAME; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_IMPORT_EMR_EC2_ROLE_NAME; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.instance.EMRProperty.BULK_IMPORT_EMR_EBS_ENCRYPTION_KEY_ARN; diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/EksBulkImportStack.java b/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/EksBulkImportStack.java index 463ad64d735..2c1c01c2bbf 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/EksBulkImportStack.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/bulkimport/EksBulkImportStack.java @@ -83,8 +83,8 @@ import static sleeper.core.properties.instance.BulkImportProperty.BULK_IMPORT_STARTER_LAMBDA_MEMORY; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_IMPORT_EKS_JOB_QUEUE_ARN; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_IMPORT_EKS_JOB_QUEUE_URL; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.instance.EKSProperty.EKS_CLUSTER_ADMIN_ROLES; diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionOnEc2Resources.java b/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionOnEc2Resources.java index a16b009075e..6eaaf1ae21c 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionOnEc2Resources.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionOnEc2Resources.java @@ -74,9 +74,9 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_AUTO_SCALING_GROUP; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_TASK_EC2_DEFINITION_FAMILY; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ECS_SECURITY_GROUPS; import static sleeper.core.properties.instance.CompactionProperty.COMPACTION_EC2_POOL_MAXIMUM; import static sleeper.core.properties.instance.CompactionProperty.COMPACTION_EC2_POOL_MINIMUM; diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionTaskResources.java b/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionTaskResources.java index 0440e999932..0ea19805a45 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionTaskResources.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/compaction/CompactionTaskResources.java @@ -61,8 +61,8 @@ import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_CLUSTER; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_TASK_CREATION_CLOUDWATCH_RULE; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_TASK_CREATION_LAMBDA_FUNCTION; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_MEMORY_IN_MB; import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_TIMEOUT_IN_SECONDS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; diff --git a/java/cdk/src/main/java/sleeper/cdk/stack/core/VpcCheckStack.java b/java/cdk/src/main/java/sleeper/cdk/stack/core/VpcCheckStack.java index 4a275454857..168c01dea32 100644 --- a/java/cdk/src/main/java/sleeper/cdk/stack/core/VpcCheckStack.java +++ b/java/cdk/src/main/java/sleeper/cdk/stack/core/VpcCheckStack.java @@ -38,7 +38,7 @@ import java.util.List; import java.util.Map; -import static sleeper.core.properties.instance.CommonProperty.REGION; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; public class VpcCheckStack extends NestedStack { diff --git a/java/cdk/src/test/java/sleeper/cdk/default-instance.approved.json b/java/cdk/src/test/java/sleeper/cdk/default-instance.approved.json index 0e42bc54862..1b630e08ded 100644 --- a/java/cdk/src/test/java/sleeper/cdk/default-instance.approved.json +++ b/java/cdk/src/test/java/sleeper/cdk/default-instance.approved.json @@ -12839,7 +12839,9 @@ { "Action": "elasticmapreduce:RunJobFlow", "Condition": { - "StringEquals": {} + "StringEquals": { + "elasticmapreduce:RequestTag/InstanceID": "test-instance" + } }, "Effect": "Allow", "Resource": "*" @@ -22099,7 +22101,7 @@ { "Ref": "referencetoTestInstanceCompactionNestedStackCompactionNestedStackResource654ED576OutputsTestInstanceCompactionPendingCompactionJobBatchDLQE42B71ECRef" }, - "\nsleeper.ingest.arrow.max.local.store.bytes\u003d536870912\nsleeper.query.leaf.partition.queue.arn\u003d", + "\nsleeper.tags\u003dInstanceID,test-instance\nsleeper.ingest.arrow.max.local.store.bytes\u003d536870912\nsleeper.query.leaf.partition.queue.arn\u003d", { "Ref": "referencetoTestInstanceQueryNestedStackQueryNestedStackResource79C6CBEEOutputsTestInstanceQueryLeafPartitionQueryQueue64615BA9Arn" }, diff --git a/java/cdk/src/test/java/sleeper/cdk/stack/SleeperInstanceStacksPropsIT.java b/java/cdk/src/test/java/sleeper/cdk/stack/SleeperInstanceStacksPropsIT.java index f8270f91214..feb655a29f5 100644 --- a/java/cdk/src/test/java/sleeper/cdk/stack/SleeperInstanceStacksPropsIT.java +++ b/java/cdk/src/test/java/sleeper/cdk/stack/SleeperInstanceStacksPropsIT.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.nio.file.Path; import java.util.Map; +import java.util.Properties; import java.util.UUID; import java.util.stream.Stream; @@ -38,10 +39,8 @@ import static sleeper.core.SleeperVersion.getVersion; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.BULK_IMPORT_BUCKET; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; @@ -60,10 +59,16 @@ void shouldLoadValidInstancePropertiesFromFile() throws IOException { InstanceProperties properties = createUserDefinedInstanceProperties(); SaveLocalProperties.saveToDirectory(tempDir, properties, Stream.empty()); - // When / Then + // When + InstanceProperties loaded = loadInstanceProperties(cdkContextWithPropertiesFile(tempDir)); + + // Then + Properties tagsProperties = properties.getTagsProperties(); + tagsProperties.setProperty("InstanceID", properties.get(ID)); + properties.loadTags(tagsProperties); properties.set(VERSION, getVersion()); - assertThat(loadInstanceProperties(cdkContextWithPropertiesFile(tempDir))) - .isEqualTo(properties); + + assertThat(loaded).isEqualTo(properties); } @Test @@ -93,6 +98,20 @@ void shouldSetVersionWhenInstancePropertiesAreLoaded() throws IOException { assertThat(loaded.get(VERSION)) .matches("\\d+\\.\\d+\\.\\d+(-SNAPSHOT)?"); } + + @Test + void shouldSetTagPropertiesWhenInstancePropertiesAreLoaded() throws IOException { + // Given + InstanceProperties properties = createUserDefinedInstanceProperties(); + SaveLocalProperties.saveToDirectory(tempDir, properties, Stream.empty()); + + // When + InstanceProperties loaded = loadInstanceProperties(cdkContextWithPropertiesFile(tempDir)); + + // Then + assertThat(loaded.getTags().get("InstanceID")) + .matches("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}$"); + } } @Nested @@ -197,10 +216,9 @@ private static InstanceProperties createUserDefinedInstanceProperties() { InstanceProperties instanceProperties = new InstanceProperties(); instanceProperties.set(ID, id); instanceProperties.set(JARS_BUCKET, "test-bucket"); - instanceProperties.set(ACCOUNT, "test-account"); - instanceProperties.set(REGION, "test-region"); instanceProperties.set(VPC_ID, "test-vpc"); instanceProperties.set(SUBNETS, "test-subnet"); + return instanceProperties; } } diff --git a/java/clients/src/main/java/sleeper/clients/admin/AdminClient.java b/java/clients/src/main/java/sleeper/clients/admin/AdminClient.java index 5cb70b7ee89..937b139b6ee 100644 --- a/java/clients/src/main/java/sleeper/clients/admin/AdminClient.java +++ b/java/clients/src/main/java/sleeper/clients/admin/AdminClient.java @@ -15,11 +15,14 @@ */ package sleeper.clients.admin; +import software.amazon.awssdk.regions.providers.AwsRegionProvider; +import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.ecr.EcrClient; import software.amazon.awssdk.services.emr.EmrClient; import software.amazon.awssdk.services.s3.S3Client; import software.amazon.awssdk.services.sqs.SqsClient; +import software.amazon.awssdk.services.sts.StsClient; import sleeper.clients.admin.properties.AdminClientPropertiesStore; import sleeper.clients.admin.properties.UpdatePropertiesWithTextEditor; @@ -94,10 +97,13 @@ public static void main(String[] args) throws IOException, InterruptedException DynamoDbClient dynamoClient = AwsV2ClientHelper.buildAwsV2Client(DynamoDbClient.builder()); SqsClient sqsClient = AwsV2ClientHelper.buildAwsV2Client(SqsClient.builder()); EcrClient ecrClient = AwsV2ClientHelper.buildAwsV2Client(EcrClient.builder()); - EmrClient emrClient = AwsV2ClientHelper.buildAwsV2Client(EmrClient.builder())) { + EmrClient emrClient = AwsV2ClientHelper.buildAwsV2Client(EmrClient.builder()); + StsClient stsClient = AwsV2ClientHelper.buildAwsV2Client(StsClient.builder())) { + AwsRegionProvider regionProvider = DefaultAwsRegionProviderChain.builder().build(); UploadDockerImagesToEcr uploadDockerImages = new UploadDockerImagesToEcr( UploadDockerImages.fromScriptsDirectory(scriptsDir), - CheckVersionExistsInEcr.withEcrClient(ecrClient)); + CheckVersionExistsInEcr.withEcrClient(ecrClient), + stsClient.getCallerIdentity().account(), regionProvider.getRegion().id()); errorCode = start(instanceId, s3Client, dynamoClient, cdk, generatedDir, uploadDockerImages, out, in, new UpdatePropertiesWithTextEditor(Path.of("/tmp")), diff --git a/java/clients/src/main/java/sleeper/clients/api/role/AssumeSleeperRole.java b/java/clients/src/main/java/sleeper/clients/api/role/AssumeSleeperRole.java index 8ed07cfc4d8..0fc3e336a13 100644 --- a/java/clients/src/main/java/sleeper/clients/api/role/AssumeSleeperRole.java +++ b/java/clients/src/main/java/sleeper/clients/api/role/AssumeSleeperRole.java @@ -29,8 +29,8 @@ import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ADMIN_ROLE_ARN; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.INGEST_BY_QUEUE_ROLE_ARN; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.INGEST_DIRECT_ROLE_ARN; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.ENDPOINT_URL; -import static sleeper.core.properties.instance.CommonProperty.REGION; public class AssumeSleeperRole { public static final Logger LOGGER = LoggerFactory.getLogger(AssumeSleeperRole.class); diff --git a/java/clients/src/main/java/sleeper/clients/deploy/DeployExistingInstance.java b/java/clients/src/main/java/sleeper/clients/deploy/DeployExistingInstance.java index f19bb2d4884..c51653fd865 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/DeployExistingInstance.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/DeployExistingInstance.java @@ -18,9 +18,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import software.amazon.awssdk.regions.providers.AwsRegionProvider; +import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.ecr.EcrClient; import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.sts.StsClient; import sleeper.clients.deploy.container.CheckVersionExistsInEcr; import sleeper.clients.deploy.container.UploadDockerImages; @@ -51,6 +54,8 @@ public class DeployExistingInstance { private final List tablePropertiesList; private final S3Client s3; private final EcrClient ecr; + private final StsClient sts; + private final AwsRegionProvider regionProvider; private final boolean deployPaused; private final CommandPipelineRunner runCommand; private final boolean createMultiPlatformBuilder; @@ -61,6 +66,8 @@ private DeployExistingInstance(Builder builder) { tablePropertiesList = builder.tablePropertiesList; s3 = builder.s3; ecr = builder.ecr; + sts = builder.sts; + regionProvider = builder.regionProvider; deployPaused = builder.deployPaused; runCommand = builder.runCommand; createMultiPlatformBuilder = builder.createMultiPlatformBuilder; @@ -81,8 +88,10 @@ public static void main(String[] args) throws IOException, InterruptedException try (S3Client s3Client = S3Client.create(); DynamoDbClient dynamoClient = DynamoDbClient.create(); - EcrClient ecrClient = EcrClient.create()) { - builder().clients(s3Client, ecrClient) + EcrClient ecrClient = EcrClient.create(); + StsClient stsClient = StsClient.create()) { + builder().clients(s3Client, ecrClient, stsClient) + .regionProvider(DefaultAwsRegionProviderChain.builder().build()) .scriptsDirectory(Path.of(args[0])) .instanceId(args[1]) .deployPaused(deployPaused) @@ -101,7 +110,7 @@ public void update() throws IOException, InterruptedException { .commandRunner(runCommand) .createMultiplatformBuilder(createMultiPlatformBuilder) .build(), - CheckVersionExistsInEcr.withEcrClient(ecr)), + CheckVersionExistsInEcr.withEcrClient(ecr), sts.getCallerIdentity().account(), regionProvider.getRegion().id()), DeployInstance.WriteLocalProperties.underScriptsDirectory(scriptsDirectory), InvokeCdk.builder().scriptsDirectory(scriptsDirectory).runCommand(runCommand).build()); @@ -121,6 +130,8 @@ public static final class Builder { private List tablePropertiesList; private S3Client s3; private EcrClient ecr; + private StsClient sts; + private AwsRegionProvider regionProvider; private boolean deployPaused; private CommandPipelineRunner runCommand = CommandUtils::runCommandInheritIO; private boolean createMultiPlatformBuilder = true; @@ -152,9 +163,15 @@ public Builder tablePropertiesList(List tablePropertiesList) { return this; } - public Builder clients(S3Client s3, EcrClient ecr) { + public Builder clients(S3Client s3, EcrClient ecr, StsClient sts) { this.s3 = s3; this.ecr = ecr; + this.sts = sts; + return this; + } + + public Builder regionProvider(AwsRegionProvider regionProvider) { + this.regionProvider = regionProvider; return this; } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/DeployInstance.java b/java/clients/src/main/java/sleeper/clients/deploy/DeployInstance.java index 20d88363d6f..8f90a20d860 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/DeployInstance.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/DeployInstance.java @@ -29,13 +29,12 @@ import sleeper.clients.deploy.container.UploadDockerImagesToEcrRequest; import sleeper.clients.deploy.jar.SyncJars; import sleeper.clients.deploy.jar.SyncJarsRequest; -import sleeper.clients.deploy.properties.PopulateInstancePropertiesAws; import sleeper.clients.util.ClientUtils; import sleeper.clients.util.cdk.CdkCommand; import sleeper.clients.util.cdk.InvokeCdk; -import sleeper.core.deploy.PopulateInstanceProperties; import sleeper.core.deploy.SleeperInstanceConfiguration; import sleeper.core.properties.instance.InstanceProperties; +import sleeper.core.properties.instance.UserDefinedInstanceProperty; import sleeper.core.properties.local.SaveLocalProperties; import java.io.IOException; @@ -85,19 +84,17 @@ public static void main(String[] args) throws IOException, InterruptedException StsClient stsClient = StsClient.create()) { SleeperInstanceConfiguration instanceConfiguration = SleeperInstanceConfiguration.fromLocalConfiguration(propertiesFile); - PopulateInstancePropertiesAws.builder(stsClient, regionProvider) - .instanceId(instanceId) - .vpcId(vpcId) - .subnetIds(subnetIds) - .build().populate(instanceConfiguration.getInstanceProperties()); - PopulateInstanceProperties.setFromEnvironmentVariables(instanceConfiguration.getInstanceProperties()); + instanceConfiguration.getInstanceProperties().set(ID, instanceId); + instanceConfiguration.getInstanceProperties().set(VPC_ID, vpcId); + instanceConfiguration.getInstanceProperties().set(SUBNETS, subnetIds); + setFromEnvironmentVariables(instanceConfiguration.getInstanceProperties()); instanceConfiguration.validate(); DeployInstance deployInstance = new DeployInstance( SyncJars.fromScriptsDirectory(s3Client, scriptsDirectory), new UploadDockerImagesToEcr( UploadDockerImages.fromScriptsDirectory(scriptsDirectory), - CheckVersionExistsInEcr.withEcrClient(ecrClient)), + CheckVersionExistsInEcr.withEcrClient(ecrClient), stsClient.getCallerIdentity().account(), regionProvider.getRegion().id()), WriteLocalProperties.underScriptsDirectory(scriptsDirectory), InvokeCdk.fromScriptsDirectory(scriptsDirectory)); @@ -155,4 +152,13 @@ static WriteLocalProperties toDirectory(Path directory) { }; } } + + private static void setFromEnvironmentVariables(InstanceProperties instanceProperties) { + for (UserDefinedInstanceProperty property : UserDefinedInstanceProperty.getAll()) { + String value = System.getenv(property.toEnvironmentVariable()); + if (value != null) { + instanceProperties.set(property, value); + } + } + } } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java b/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java index e435c377a39..80f1adf88be 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/DeployNewInstance.java @@ -29,7 +29,6 @@ import sleeper.clients.deploy.container.UploadDockerImages; import sleeper.clients.deploy.container.UploadDockerImagesToEcr; import sleeper.clients.deploy.jar.SyncJars; -import sleeper.clients.deploy.properties.PopulateInstancePropertiesAws; import sleeper.clients.table.AddTable; import sleeper.clients.util.cdk.CdkCommand; import sleeper.clients.util.cdk.InvokeCdk; @@ -37,7 +36,6 @@ import sleeper.clients.util.command.CommandUtils; import sleeper.configuration.properties.S3InstanceProperties; import sleeper.configuration.properties.S3TableProperties; -import sleeper.core.deploy.PopulateInstanceProperties; import sleeper.core.deploy.SleeperInstanceConfiguration; import sleeper.core.properties.instance.InstanceProperties; import sleeper.core.properties.table.TableProperties; @@ -48,6 +46,9 @@ import java.util.List; import static sleeper.clients.util.ClientUtils.optionalArgument; +import static sleeper.core.properties.instance.CommonProperty.ID; +import static sleeper.core.properties.instance.CommonProperty.SUBNETS; +import static sleeper.core.properties.instance.CommonProperty.VPC_ID; public class DeployNewInstance { private static final Logger LOGGER = LoggerFactory.getLogger(DeployNewInstance.class); @@ -55,6 +56,8 @@ public class DeployNewInstance { private final S3Client s3Client; private final DynamoDbClient dynamoClient; private final EcrClient ecrClient; + private final StsClient stsClient; + private final AwsRegionProvider regionProvider; private final Path scriptsDirectory; private final SleeperInstanceConfiguration deployInstanceConfiguration; private final List extraDockerImages; @@ -67,6 +70,8 @@ private DeployNewInstance(Builder builder) { s3Client = builder.s3Client; dynamoClient = builder.dynamoClient; ecrClient = builder.ecrClient; + stsClient = builder.stsClient; + regionProvider = builder.regionProvider; scriptsDirectory = builder.scriptsDirectory; deployInstanceConfiguration = builder.deployInstanceConfiguration; extraDockerImages = builder.extraDockerImages; @@ -85,23 +90,27 @@ public static void main(String[] args) throws IOException, InterruptedException throw new IllegalArgumentException("Usage: " + " "); } - AwsRegionProvider regionProvider = DefaultAwsRegionProviderChain.builder().build(); try (S3Client s3Client = S3Client.create(); DynamoDbClient dynamoClient = DynamoDbClient.create(); StsClient stsClient = StsClient.create(); EcrClient ecrClient = EcrClient.create()) { Path scriptsDirectory = Path.of(args[0]); - PopulateInstanceProperties populateInstanceProperties = PopulateInstancePropertiesAws.builder(stsClient, regionProvider) - .instanceId(args[1]).vpcId(args[2]).subnetIds(args[3]) - .build(); + Path instancePropertiesFile = optionalArgument(args, 4).map(Path::of).orElse(null); boolean deployPaused = "true".equalsIgnoreCase(optionalArgument(args, 5).orElse("false")); + + SleeperInstanceConfiguration config = SleeperInstanceConfiguration.forNewInstanceDefaultingInstance( + instancePropertiesFile, scriptsDirectory.resolve("templates")); + + config.getInstanceProperties().set(ID, args[1]); + config.getInstanceProperties().set(VPC_ID, args[2]); + config.getInstanceProperties().set(SUBNETS, args[3]); + builder().scriptsDirectory(scriptsDirectory) - .deployInstanceConfiguration(SleeperInstanceConfiguration.forNewInstanceDefaultingInstance( - instancePropertiesFile, populateInstanceProperties, scriptsDirectory.resolve("templates"))) + .deployInstanceConfiguration(config) .deployPaused(deployPaused) .instanceType(InvokeCdk.Type.STANDARD) - .deployWithClients(s3Client, dynamoClient, ecrClient); + .deployWithClients(s3Client, dynamoClient, ecrClient, stsClient, DefaultAwsRegionProviderChain.builder().build()); } } @@ -119,7 +128,7 @@ public void deploy() throws IOException, InterruptedException { .commandRunner(runCommand) .createMultiplatformBuilder(createMultiPlatformBuilder) .build(), - CheckVersionExistsInEcr.withEcrClient(ecrClient)), + CheckVersionExistsInEcr.withEcrClient(ecrClient), stsClient.getCallerIdentity().account(), regionProvider.getRegion().id()), DeployInstance.WriteLocalProperties.underScriptsDirectory(scriptsDirectory), InvokeCdk.builder().scriptsDirectory(scriptsDirectory).runCommand(runCommand).build()); @@ -143,8 +152,10 @@ public void deploy() throws IOException, InterruptedException { public static final class Builder { private S3Client s3Client; + private StsClient stsClient; private DynamoDbClient dynamoClient; private EcrClient ecrClient; + private AwsRegionProvider regionProvider; private Path scriptsDirectory; private SleeperInstanceConfiguration deployInstanceConfiguration; private List extraDockerImages = List.of(); @@ -161,6 +172,11 @@ public Builder s3Client(S3Client s3Client) { return this; } + public Builder stsClient(StsClient stsClient) { + this.stsClient = stsClient; + return this; + } + public Builder dynamoClient(DynamoDbClient dynamoClient) { this.dynamoClient = dynamoClient; return this; @@ -171,6 +187,11 @@ public Builder ecrClient(EcrClient ecrClient) { return this; } + public Builder regionProvider(AwsRegionProvider regionProvider) { + this.regionProvider = regionProvider; + return this; + } + public Builder scriptsDirectory(Path scriptsDirectory) { this.scriptsDirectory = scriptsDirectory; return this; @@ -211,10 +232,13 @@ public DeployNewInstance build() { } public void deployWithClients( - S3Client s3Client, DynamoDbClient dynamoClient, EcrClient ecrClient) throws IOException, InterruptedException { + S3Client s3Client, DynamoDbClient dynamoClient, EcrClient ecrClient, StsClient stsClient, + AwsRegionProvider regionProvider) throws IOException, InterruptedException { s3Client(s3Client) .dynamoClient(dynamoClient) .ecrClient(ecrClient) + .stsClient(stsClient) + .regionProvider(regionProvider) .build().deploy(); } } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/UploadArtefacts.java b/java/clients/src/main/java/sleeper/clients/deploy/UploadArtefacts.java index 2afa79e0672..1de67f25e5e 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/UploadArtefacts.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/UploadArtefacts.java @@ -129,7 +129,7 @@ public static void main(String[] rawArgs) throws IOException, InterruptedExcepti .deployConfig(DeployConfiguration.fromScriptsDirectory(args.scriptsDir())) .createMultiplatformBuilder(args.createMultiplatformBuilder()) .build(), - CheckVersionExistsInEcr.withEcrClient(ecrClient)); + CheckVersionExistsInEcr.withEcrClient(ecrClient), account, region); if (args.createDeployment()) { InvokeCdk.fromScriptsDirectory(args.scriptsDir()) @@ -137,11 +137,9 @@ public static void main(String[] rawArgs) throws IOException, InterruptedExcepti } syncJars.sync(SyncJarsRequest.builder() .bucketName(jarsBucket) - .region(region) .build()); uploadImages.upload(UploadDockerImagesToEcrRequest.builder() .ecrPrefix(ecrPrefix) - .account(account).region(region) .images(images) .build()); } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcr.java b/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcr.java index 8cfcd2a04de..90d9fe8e4b0 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcr.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcr.java @@ -34,10 +34,16 @@ public class UploadDockerImagesToEcr { private final UploadDockerImages uploader; private final CheckVersionExistsInEcr repositoryChecker; + private final String account; + private final String region; + private final String repositoryHost; - public UploadDockerImagesToEcr(UploadDockerImages uploader, CheckVersionExistsInEcr repositoryChecker) { + public UploadDockerImagesToEcr(UploadDockerImages uploader, CheckVersionExistsInEcr repositoryChecker, String account, String region) { this.uploader = uploader; this.repositoryChecker = repositoryChecker; + this.account = account; + this.region = region; + this.repositoryHost = String.format("%s.dkr.ecr.%s.amazonaws.com", this.account, this.region); } public void upload(UploadDockerImagesToEcrRequest request) throws IOException, InterruptedException { @@ -46,11 +52,11 @@ public void upload(UploadDockerImagesToEcrRequest request) throws IOException, I List imagesToUpload = requestedImages.stream() .filter(image -> imageDoesNotExistInRepositoryWithVersion(image, request)) .collect(Collectors.toUnmodifiableList()); - String repositoryPrefix = request.getRepositoryHost() + "/" + request.getEcrPrefix(); + String repositoryPrefix = repositoryHost + "/" + request.getEcrPrefix(); if (!imagesToUpload.isEmpty()) { uploader.getCommandRunner().runOrThrow(pipeline( - command("aws", "ecr", "get-login-password", "--region", request.getRegion()), - command("docker", "login", "--username", "AWS", "--password-stdin", request.getRepositoryHost()))); + command("aws", "ecr", "get-login-password", "--region", region), + command("docker", "login", "--username", "AWS", "--password-stdin", repositoryHost))); } uploader.upload(repositoryPrefix, imagesToUpload); } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrRequest.java b/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrRequest.java index 0c152d83faa..f4be0392575 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrRequest.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrRequest.java @@ -26,23 +26,15 @@ import static java.util.Objects.requireNonNull; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ECR_REPOSITORY_PREFIX; -import static sleeper.core.properties.instance.CommonProperty.REGION; public class UploadDockerImagesToEcrRequest { private final String ecrPrefix; - private final String account; - private final String region; private final List images; - private final String repositoryHost; private UploadDockerImagesToEcrRequest(Builder builder) { ecrPrefix = requireNonNull(builder.ecrPrefix, "ecrPrefix must not be null"); - account = requireNonNull(builder.account, "account must not be null"); - region = requireNonNull(builder.region, "region must not be null"); images = requireNonNull(builder.images, "images must not be null"); - repositoryHost = String.format("%s.dkr.ecr.%s.amazonaws.com", account, region); } public static Builder builder() { @@ -67,7 +59,7 @@ public static Optional forUpdateIfNeeded(Instanc } public Builder toBuilder() { - return builder().ecrPrefix(ecrPrefix).account(account).region(region).images(images); + return builder().ecrPrefix(ecrPrefix).images(images); } public UploadDockerImagesToEcrRequest withExtraImages(List extraImages) { @@ -84,22 +76,10 @@ public String getEcrPrefix() { return ecrPrefix; } - public String getAccount() { - return account; - } - - public String getRegion() { - return region; - } - public List getImages() { return images; } - public String getRepositoryHost() { - return repositoryHost; - } - @Override public boolean equals(Object obj) { if (this == obj) { @@ -109,23 +89,21 @@ public boolean equals(Object obj) { return false; } UploadDockerImagesToEcrRequest other = (UploadDockerImagesToEcrRequest) obj; - return Objects.equals(ecrPrefix, other.ecrPrefix) && Objects.equals(account, other.account) && Objects.equals(region, other.region) && Objects.equals(images, other.images); + return Objects.equals(ecrPrefix, other.ecrPrefix) && Objects.equals(images, other.images); } @Override public int hashCode() { - return Objects.hash(ecrPrefix, account, region, images); + return Objects.hash(ecrPrefix, images); } @Override public String toString() { - return "UploadDockerImagesToEcrRequest{ecrPrefix=" + ecrPrefix + ", account=" + account + ", region=" + region + ", images=" + images + "}"; + return "UploadDockerImagesToEcrRequest{ecrPrefix=" + ecrPrefix + ", images=" + images + "}"; } public static final class Builder { private String ecrPrefix; - private String account; - private String region; private List images; private Builder() { @@ -133,8 +111,6 @@ private Builder() { public Builder properties(InstanceProperties properties) { return ecrPrefix(properties.get(ECR_REPOSITORY_PREFIX)) - .account(properties.get(ACCOUNT)) - .region(properties.get(REGION)) .version(properties.get(VERSION)); } @@ -143,16 +119,6 @@ public Builder ecrPrefix(String ecrPrefix) { return this; } - public Builder account(String account) { - this.account = account; - return this; - } - - public Builder region(String region) { - this.region = region; - return this; - } - public Builder version(String version) { return this; } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/documentation/GeneratePropertiesTemplates.java b/java/clients/src/main/java/sleeper/clients/deploy/documentation/GeneratePropertiesTemplates.java index fc138b53bd5..6695577c5f1 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/documentation/GeneratePropertiesTemplates.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/documentation/GeneratePropertiesTemplates.java @@ -39,9 +39,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.instance.CompactionProperty.DEFAULT_SIZERATIO_COMPACTION_STRATEGY_MAX_CONCURRENT_JOBS_PER_PARTITION; @@ -68,8 +66,6 @@ public class GeneratePropertiesTemplates { private static final Map BASIC_INSTANCE_EXAMPLE_VALUES = Map.of( ID, "basic-example", - ACCOUNT, "1234567890", - REGION, "eu-west-2", VPC_ID, "1234567890", SUBNETS, "subnet-abcdefgh"); diff --git a/java/clients/src/main/java/sleeper/clients/deploy/jar/JarsBucketCreator.java b/java/clients/src/main/java/sleeper/clients/deploy/jar/JarsBucketCreator.java index f6a030aa267..5bf40a65688 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/jar/JarsBucketCreator.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/jar/JarsBucketCreator.java @@ -24,8 +24,8 @@ import sleeper.core.properties.instance.InstanceProperties; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.REGION; /** * A tool to create a jars bucket that matches how it is created by the CDK. Usually only used for testing. diff --git a/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJars.java b/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJars.java index aca6ca04b1f..fc9d6137e55 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJars.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJars.java @@ -46,20 +46,18 @@ public static SyncJars fromScriptsDirectory(S3Client s3, Path scriptsDirectory) } public static void main(String[] args) throws IOException { - if (args.length < 3 || args.length > 4) { - throw new IllegalArgumentException("Usage: "); + if (args.length < 2 || args.length > 3) { + throw new IllegalArgumentException("Usage: "); } Path jarsDirectory = Path.of(args[0]); String bucketName = args[1]; - String region = args[2]; - boolean deleteOldJars = optionalArgument(args, 3) + boolean deleteOldJars = optionalArgument(args, 2) .map(Boolean::parseBoolean) .orElse(false); try (S3Client s3 = S3Client.create()) { new SyncJars(s3, jarsDirectory) .sync(SyncJarsRequest.builder() .bucketName(bucketName) - .region(region) .deleteOldJars(deleteOldJars) .build()); } diff --git a/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJarsRequest.java b/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJarsRequest.java index 0cc0d1bd0a1..d61117c490c 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJarsRequest.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/jar/SyncJarsRequest.java @@ -22,18 +22,15 @@ import java.util.function.Predicate; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.REGION; public class SyncJarsRequest { private final String bucketName; - private final String region; private final Predicate uploadFilter; private final boolean deleteOldJars; private SyncJarsRequest(Builder builder) { bucketName = Objects.requireNonNull(builder.bucketName, "bucketName must not be null"); - region = Objects.requireNonNull(builder.region, "region must not be null"); uploadFilter = Objects.requireNonNull(builder.uploadFilter, "uploadFilter must not be null"); deleteOldJars = builder.deleteOldJars; } @@ -50,10 +47,6 @@ public String getBucketName() { return bucketName; } - public String getRegion() { - return region; - } - public Predicate getUploadFilter() { return uploadFilter; } @@ -64,7 +57,6 @@ public boolean isDeleteOldJars() { public static class Builder { private String bucketName; - private String region; private Predicate uploadFilter = jar -> true; private boolean deleteOldJars = false; @@ -73,11 +65,6 @@ public Builder bucketName(String bucketName) { return this; } - public Builder region(String region) { - this.region = region; - return this; - } - public Builder uploadFilter(Predicate uploadFilter) { this.uploadFilter = uploadFilter; return this; @@ -89,8 +76,7 @@ public Builder deleteOldJars(boolean deleteOldJars) { } public Builder instanceProperties(InstanceProperties instanceProperties) { - return bucketName(instanceProperties.get(JARS_BUCKET)) - .region(instanceProperties.get(REGION)); + return bucketName(instanceProperties.get(JARS_BUCKET)); } public SyncJarsRequest build() { diff --git a/java/clients/src/main/java/sleeper/clients/deploy/localstack/DeployDockerInstance.java b/java/clients/src/main/java/sleeper/clients/deploy/localstack/DeployDockerInstance.java index 28fa9dc42cb..c175e3465b7 100644 --- a/java/clients/src/main/java/sleeper/clients/deploy/localstack/DeployDockerInstance.java +++ b/java/clients/src/main/java/sleeper/clients/deploy/localstack/DeployDockerInstance.java @@ -43,13 +43,13 @@ import java.util.function.Consumer; import static sleeper.configuration.utils.AwsV2ClientHelper.buildAwsV2Client; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.CONFIG_BUCKET; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_RESULTS_BUCKET; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.ENDPOINT_URL; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.OPTIONAL_STACKS; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.instance.CompactionProperty.COMPACTION_TASK_DELAY_BEFORE_RETRY_IN_SECONDS; diff --git a/java/clients/src/main/java/sleeper/clients/deploy/properties/PopulateInstancePropertiesAws.java b/java/clients/src/main/java/sleeper/clients/deploy/properties/PopulateInstancePropertiesAws.java deleted file mode 100644 index acd966499bd..00000000000 --- a/java/clients/src/main/java/sleeper/clients/deploy/properties/PopulateInstancePropertiesAws.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2022-2025 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package sleeper.clients.deploy.properties; - -import software.amazon.awssdk.regions.providers.AwsRegionProvider; -import software.amazon.awssdk.services.sts.StsClient; - -import sleeper.core.deploy.PopulateInstanceProperties; - -public class PopulateInstancePropertiesAws { - - private PopulateInstancePropertiesAws() { - } - - public static PopulateInstanceProperties.Builder builder(StsClient stsClient, AwsRegionProvider regionProvider) { - return PopulateInstanceProperties.builder() - .accountSupplier(stsClient.getCallerIdentity()::account) - .regionIdSupplier(() -> regionProvider.getRegion().id()); - } -} diff --git a/java/clients/src/main/java/sleeper/clients/query/QueryWebSocketConnection.java b/java/clients/src/main/java/sleeper/clients/query/QueryWebSocketConnection.java index d20c95162cd..a8ec82d0708 100644 --- a/java/clients/src/main/java/sleeper/clients/query/QueryWebSocketConnection.java +++ b/java/clients/src/main/java/sleeper/clients/query/QueryWebSocketConnection.java @@ -32,7 +32,7 @@ import java.net.URI; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_WEBSOCKET_API_URL; -import static sleeper.core.properties.instance.CommonProperty.REGION; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; class QueryWebSocketConnection extends WebSocketClient implements QueryWebSocketClient.Connection { public static final Logger LOGGER = LoggerFactory.getLogger(QueryWebSocketConnection.class); diff --git a/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java b/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java index 9c41f413efd..69a8b7bf102 100644 --- a/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java +++ b/java/clients/src/test/java/sleeper/clients/admin/properties/AdminClientPropertiesStoreIT.java @@ -51,12 +51,10 @@ import static org.mockito.Mockito.verifyNoInteractions; import static sleeper.clients.deploy.container.StackDockerImage.dockerBuildImage; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.FARGATE_VERSION; import static sleeper.core.properties.instance.CommonProperty.FORCE_RELOAD_PROPERTIES; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.OPTIONAL_STACKS; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_MEMORY_IN_MB; import static sleeper.core.properties.local.LoadLocalProperties.loadInstancePropertiesFromDirectory; import static sleeper.core.properties.local.LoadLocalProperties.loadTablesFromDirectory; @@ -395,8 +393,6 @@ private void updateInstanceProperty(String instanceId, InstanceProperty property private UploadDockerImagesToEcrRequest withImages(StackDockerImage... images) { return UploadDockerImagesToEcrRequest.builder() .ecrPrefix(instanceProperties.get(ID)) - .account(instanceProperties.get(ACCOUNT)) - .region(instanceProperties.get(REGION)) .version(instanceProperties.get(VERSION)) .images(List.of(images)) .build(); diff --git a/java/clients/src/test/java/sleeper/clients/admin/properties/PropertiesDiffTest.java b/java/clients/src/test/java/sleeper/clients/admin/properties/PropertiesDiffTest.java index dd65ced0329..cec77890eac 100644 --- a/java/clients/src/test/java/sleeper/clients/admin/properties/PropertiesDiffTest.java +++ b/java/clients/src/test/java/sleeper/clients/admin/properties/PropertiesDiffTest.java @@ -32,25 +32,30 @@ import static sleeper.clients.admin.properties.PropertiesDiffTestHelper.newValue; import static sleeper.clients.admin.properties.PropertiesDiffTestHelper.valueChanged; import static sleeper.clients.admin.properties.PropertiesDiffTestHelper.valueDeleted; -import static sleeper.core.deploy.PopulatePropertiesTestHelper.generateTestInstanceProperties; -import static sleeper.core.deploy.PopulatePropertiesTestHelper.generateTestTableProperties; import static sleeper.core.properties.PropertiesUtils.loadProperties; import static sleeper.core.properties.instance.CommonProperty.LOG_RETENTION_IN_DAYS; import static sleeper.core.properties.instance.CommonProperty.MAXIMUM_CONNECTIONS_TO_S3; +import static sleeper.core.properties.instance.CommonProperty.S3_UPLOAD_BLOCK_SIZE; import static sleeper.core.properties.instance.IngestProperty.INGEST_SOURCE_BUCKET; import static sleeper.core.properties.table.TableProperty.ITERATOR_CONFIG; +import static sleeper.core.properties.table.TableProperty.TABLE_NAME; +import static sleeper.core.properties.testutils.InstancePropertiesTestHelper.createTestInstanceProperties; +import static sleeper.core.properties.testutils.InstancePropertiesTestHelper.createTestInstancePropertiesWithId; +import static sleeper.core.schema.SchemaTestHelper.createSchemaWithKey; public class PropertiesDiffTest { @DisplayName("Compare instance properties") @Nested class CompareInstanceProperties { + String id = "test-instance"; @Test void shouldDetectNoChanges() { + // Given - InstanceProperties before = generateTestInstanceProperties(); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); + InstanceProperties after = createTestInstancePropertiesWithId(id); // When / Then assertThat(getChanges(before, after)).isEmpty(); @@ -59,9 +64,9 @@ void shouldDetectNoChanges() { @Test void shouldDetectPropertyHasBeenUpdated() { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.set(MAXIMUM_CONNECTIONS_TO_S3, "30"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.set(MAXIMUM_CONNECTIONS_TO_S3, "50"); // When / Then @@ -72,8 +77,8 @@ void shouldDetectPropertyHasBeenUpdated() { @Test void shouldDetectPropertyIsNewlySet() { // Given - InstanceProperties before = generateTestInstanceProperties(); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.set(INGEST_SOURCE_BUCKET, "some-bucket"); // When / Then @@ -84,9 +89,9 @@ void shouldDetectPropertyIsNewlySet() { @Test void shouldDetectPropertyIsUnset() { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.set(INGEST_SOURCE_BUCKET, "some-bucket"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); // When / Then assertThat(getChanges(before, after)) @@ -96,25 +101,25 @@ void shouldDetectPropertyIsUnset() { @Test void shouldDetectDefaultedPropertyIsNewlySet() { // Given - InstanceProperties before = generateTestInstanceProperties(); - InstanceProperties after = generateTestInstanceProperties(); - after.set(MAXIMUM_CONNECTIONS_TO_S3, "50"); + InstanceProperties before = createTestInstancePropertiesWithId(id); + InstanceProperties after = createTestInstancePropertiesWithId(id); + after.set(S3_UPLOAD_BLOCK_SIZE, "64M"); // When / Then assertThat(getChanges(before, after)) - .containsExactly(newValue(MAXIMUM_CONNECTIONS_TO_S3, "50")); + .containsExactly(newValue(S3_UPLOAD_BLOCK_SIZE, "64M")); } @Test void shouldDetectDefaultedPropertyIsUnset() { // Given - InstanceProperties before = generateTestInstanceProperties(); - before.set(MAXIMUM_CONNECTIONS_TO_S3, "50"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); + before.set(S3_UPLOAD_BLOCK_SIZE, "64M"); + InstanceProperties after = createTestInstancePropertiesWithId(id); // When / Then assertThat(getChanges(before, after)) - .containsExactly(valueDeleted(MAXIMUM_CONNECTIONS_TO_S3, "50")); + .containsExactly(valueDeleted(S3_UPLOAD_BLOCK_SIZE, "64M")); } } @@ -178,15 +183,22 @@ class CompareTableProperties { @Test void shouldDetectPropertyHasBeenUpdated() { // Given - TableProperties before = generateTestTableProperties(); + TableProperties before = generateCompareTestTableProperties(); before.set(ITERATOR_CONFIG, "config-before"); - TableProperties after = generateTestTableProperties(); + TableProperties after = generateCompareTestTableProperties(); after.set(ITERATOR_CONFIG, "config-after"); // When / Then assertThat(getChanges(before, after)) .containsExactly(valueChanged(ITERATOR_CONFIG, "config-before", "config-after")); } + + public static TableProperties generateCompareTestTableProperties() { + TableProperties tableProperties = new TableProperties(createTestInstanceProperties()); + tableProperties.set(TABLE_NAME, "test-table"); + tableProperties.setSchema(createSchemaWithKey("key")); + return tableProperties; + } } @DisplayName("Combine multiple diffs") @@ -236,12 +248,13 @@ void shouldNotCombineDiffsWhenPropertyIsDifferent() { } private PropertiesDiff generateSingleDiff(InstanceProperty property, String oldValue, String newValue) { - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId("simple-id"); before.set(property, oldValue); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId("simple-id"); after.set(property, newValue); return new PropertiesDiff(before, after); } + } private List getChanges(SleeperProperties before, SleeperProperties after) { diff --git a/java/clients/src/test/java/sleeper/clients/admin/properties/UpdatePropertiesWithTextEditorIT.java b/java/clients/src/test/java/sleeper/clients/admin/properties/UpdatePropertiesWithTextEditorIT.java index b665abe0fe3..587dfccc17d 100644 --- a/java/clients/src/test/java/sleeper/clients/admin/properties/UpdatePropertiesWithTextEditorIT.java +++ b/java/clients/src/test/java/sleeper/clients/admin/properties/UpdatePropertiesWithTextEditorIT.java @@ -34,20 +34,24 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.InstanceOfAssertFactories.LIST; import static sleeper.clients.admin.properties.PropertiesDiffTestHelper.valueChanged; -import static sleeper.core.deploy.PopulatePropertiesTestHelper.generateTestInstanceProperties; -import static sleeper.core.deploy.PopulatePropertiesTestHelper.generateTestTableProperties; import static sleeper.core.properties.PropertiesUtils.loadProperties; +import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.MAXIMUM_CONNECTIONS_TO_S3; import static sleeper.core.properties.instance.IngestProperty.INGEST_SOURCE_BUCKET; import static sleeper.core.properties.instance.LoggingLevelsProperty.LOGGING_LEVEL; import static sleeper.core.properties.table.TableProperty.DYNAMODB_STRONGLY_CONSISTENT_READS; import static sleeper.core.properties.table.TableProperty.ROW_GROUP_SIZE; +import static sleeper.core.properties.table.TableProperty.TABLE_NAME; +import static sleeper.core.properties.testutils.InstancePropertiesTestHelper.createTestInstanceProperties; +import static sleeper.core.properties.testutils.InstancePropertiesTestHelper.createTestInstancePropertiesWithId; +import static sleeper.core.schema.SchemaTestHelper.createSchemaWithKey; class UpdatePropertiesWithTextEditorIT { @TempDir private Path tempDir; private UpdatePropertiesWithTextEditorTestHelper helper; + private String id = "test-instance"; @BeforeEach void setUp() { @@ -61,7 +65,7 @@ class OpenInstanceProperties { @Test void shouldInvokeNanoOnInstancePropertiesFile() throws Exception { // Given - InstanceProperties properties = generateTestInstanceProperties(); + InstanceProperties properties = createTestInstanceProperties(); // When / Then assertThat(helper.openInstancePropertiesGetCommandRun(properties)) @@ -71,7 +75,7 @@ void shouldInvokeNanoOnInstancePropertiesFile() throws Exception { @Test void shouldWriteInstancePropertiesFile() throws Exception { // Given - InstanceProperties properties = generateTestInstanceProperties(); + InstanceProperties properties = createTestInstanceProperties(); // When / Then assertThat(helper.openInstancePropertiesGetPropertiesWritten(properties)) @@ -81,9 +85,9 @@ void shouldWriteInstancePropertiesFile() throws Exception { @Test void shouldGetDiffAfterPropertiesChanged() throws Exception { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.set(INGEST_SOURCE_BUCKET, "bucket-before"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.set(INGEST_SOURCE_BUCKET, "bucket-after"); // When / Then @@ -95,8 +99,8 @@ void shouldGetDiffAfterPropertiesChanged() throws Exception { @Test void shouldRetrievePropertiesAfterChange() throws Exception { // Given - InstanceProperties before = generateTestInstanceProperties(); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties before = createTestInstanceProperties(); + InstanceProperties after = createTestInstanceProperties(); after.set(MAXIMUM_CONNECTIONS_TO_S3, "abc"); // When @@ -109,7 +113,7 @@ void shouldRetrievePropertiesAfterChange() throws Exception { @Test void shouldFormatPropertiesUsingPrettyPrinter() throws Exception { // Given - InstanceProperties properties = generateTestInstanceProperties(); + InstanceProperties properties = createTestInstanceProperties(); // When String tempFileString = Files.readString(helper.openInstancePropertiesGetPathToFile(properties)); @@ -128,9 +132,9 @@ class OpenTableProperties { @Test void shouldUpdateTableProperties() throws Exception { // Given - TableProperties before = generateTestTableProperties(); + TableProperties before = generateCompareTestTableProperties(); before.set(ROW_GROUP_SIZE, "123"); - TableProperties after = generateTestTableProperties(); + TableProperties after = generateCompareTestTableProperties(); after.set(ROW_GROUP_SIZE, "456"); // When @@ -145,8 +149,8 @@ void shouldUpdateTableProperties() throws Exception { @Test void shouldRetrieveTablePropertiesAfterChange() throws Exception { // Given - TableProperties before = generateTestTableProperties(); - TableProperties after = generateTestTableProperties(); + TableProperties before = generateCompareTestTableProperties(); + TableProperties after = generateCompareTestTableProperties(); after.set(ROW_GROUP_SIZE, "456"); // When @@ -164,7 +168,7 @@ class FilterByGroup { @Test void shouldWriteSingleInstancePropertyGroupToFile() throws Exception { // Given - InstanceProperties properties = generateTestInstanceProperties(); + InstanceProperties properties = createTestInstanceProperties(); properties.set(LOGGING_LEVEL, "ERROR"); // When / Then @@ -176,7 +180,7 @@ void shouldWriteSingleInstancePropertyGroupToFile() throws Exception { @Test void shouldFormatPropertiesUsingPrettyPrinter() throws Exception { // Given - InstanceProperties properties = generateTestInstanceProperties(); + InstanceProperties properties = createTestInstanceProperties(); properties.set(LOGGING_LEVEL, "ERROR"); // When @@ -193,9 +197,9 @@ void shouldFormatPropertiesUsingPrettyPrinter() throws Exception { @Test void shouldCreateUpdateRequestWithInstanceProperties() throws Exception { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.set(LOGGING_LEVEL, "ERROR"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.set(LOGGING_LEVEL, "INFO"); // When @@ -212,9 +216,9 @@ void shouldCreateUpdateRequestWithInstanceProperties() throws Exception { @Test void shouldCreateUpdateRequestWithTableProperties() throws Exception { // Given - TableProperties before = generateTestTableProperties(); + TableProperties before = generateCompareTestTableProperties(); before.set(DYNAMODB_STRONGLY_CONSISTENT_READS, "false"); - TableProperties after = generateTestTableProperties(); + TableProperties after = generateCompareTestTableProperties(); after.set(DYNAMODB_STRONGLY_CONSISTENT_READS, "true"); // When @@ -231,9 +235,9 @@ void shouldCreateUpdateRequestWithTableProperties() throws Exception { @Test void shouldUnsetPropertyWhenRemovedInEditor() throws Exception { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.set(LOGGING_LEVEL, "ERROR"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.unset(LOGGING_LEVEL); // When @@ -250,7 +254,7 @@ void shouldUnsetPropertyWhenRemovedInEditor() throws Exception { @Test void shouldNotShowUnknownProperties() throws Exception { // Given - InstanceProperties properties = generateTestInstanceProperties(); + InstanceProperties properties = createTestInstanceProperties(); properties.getProperties().setProperty("unknown.property", "some-value"); // When @@ -261,9 +265,9 @@ void shouldNotShowUnknownProperties() throws Exception { @Test void shouldUpdateAnUnknownProperty() throws Exception { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.getProperties().setProperty("unknown.property", "value-before"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.getProperties().setProperty("unknown.property", "value-after"); // When @@ -280,9 +284,9 @@ void shouldUpdateAnUnknownProperty() throws Exception { @Test void shouldUpdateAPropertyOutsideTheSpecifiedGroup() throws Exception { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.set(INGEST_SOURCE_BUCKET, "bucket-before"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.set(INGEST_SOURCE_BUCKET, "bucket-after"); // When @@ -299,9 +303,9 @@ void shouldUpdateAPropertyOutsideTheSpecifiedGroup() throws Exception { @Test void shouldLeaveUnknownPropertyUnchangedWhenEditingAnotherProperty() throws Exception { // Given - InstanceProperties before = generateTestInstanceProperties(); + InstanceProperties before = createTestInstancePropertiesWithId(id); before.getProperties().setProperty("unknown.property", "test-value"); - InstanceProperties after = generateTestInstanceProperties(); + InstanceProperties after = createTestInstancePropertiesWithId(id); after.getProperties().setProperty("unknown.property", "test-value"); after.set(LOGGING_LEVEL, "TRACE"); @@ -316,4 +320,13 @@ void shouldLeaveUnknownPropertyUnchangedWhenEditingAnotherProperty() throws Exce .isEqualTo(new PropertiesDiff(before, after)); } } + + public static TableProperties generateCompareTestTableProperties() { + InstanceProperties instanceProperties = new InstanceProperties(); + instanceProperties.set(ID, "test-instance"); + TableProperties tableProperties = new TableProperties(instanceProperties); + tableProperties.set(TABLE_NAME, "test-table"); + tableProperties.setSchema(createSchemaWithKey("key")); + return tableProperties; + } } diff --git a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java index 5a004832d0b..9168c9fc23e 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrFileIT.java @@ -88,7 +88,7 @@ protected UploadDockerImagesToEcr uploader() { .baseDockerDirectory(dockerDir).jarsDirectory(jarsDir) .version("1.0.0") .build(), - ecrClient); + ecrClient, "123", "test-region"); } private static Map fileToContentUnder(Path directory) throws Exception { diff --git a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java index 715e2befc0a..1bb6a88cf7a 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTest.java @@ -622,6 +622,6 @@ protected UploadDockerImagesToEcr uploader() { .baseDockerDirectory(Path.of("./docker")).jarsDirectory(Path.of("./jars")) .version("1.0.0") .build(), - ecrClient); + ecrClient, "123", "test-region"); } } diff --git a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTestBase.java b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTestBase.java index d71daab78a8..173aea2169b 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTestBase.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/container/UploadDockerImagesToEcrTestBase.java @@ -27,9 +27,7 @@ import static sleeper.clients.util.command.Command.command; import static sleeper.clients.util.command.CommandPipeline.pipeline; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.testutils.InstancePropertiesTestHelper.createTestInstanceProperties; public abstract class UploadDockerImagesToEcrTestBase extends DockerImagesTestBase { @@ -39,8 +37,6 @@ public abstract class UploadDockerImagesToEcrTestBase extends DockerImagesTestBa @BeforeEach void setUpBase() { properties.set(ID, "test-instance"); - properties.set(ACCOUNT, "123"); - properties.set(REGION, "test-region"); properties.set(VERSION, "1.0.0"); } diff --git a/java/clients/src/test/java/sleeper/clients/deploy/properties/PopulateInstancePropertiesAwsIT.java b/java/clients/src/test/java/sleeper/clients/deploy/properties/PopulateInstancePropertiesAwsIT.java deleted file mode 100644 index 797b18d1ce2..00000000000 --- a/java/clients/src/test/java/sleeper/clients/deploy/properties/PopulateInstancePropertiesAwsIT.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2022-2025 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package sleeper.clients.deploy.properties; - -import org.junit.jupiter.api.Test; -import software.amazon.awssdk.regions.Region; - -import sleeper.core.deploy.PopulateInstanceProperties; -import sleeper.core.properties.instance.InstanceProperties; -import sleeper.localstack.test.LocalStackTestBase; - -import java.util.Map; - -import static org.assertj.core.api.Assertions.assertThat; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; -import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; -import static sleeper.core.properties.instance.CommonProperty.SUBNETS; -import static sleeper.core.properties.instance.CommonProperty.VPC_ID; - -public class PopulateInstancePropertiesAwsIT extends LocalStackTestBase { - - @Test - void shouldPopulateInstancePropertiesCorrectly() { - // Given/When - InstanceProperties properties = populateInstancePropertiesBuilder() - .build().populate(new InstanceProperties()); - - // Then - InstanceProperties expected = new InstanceProperties(); - expected.setTags(Map.of("InstanceID", "test-instance")); - expected.set(ID, "test-instance"); - expected.set(VPC_ID, "some-vpc"); - expected.set(SUBNETS, "some-subnet"); - expected.set(ACCOUNT, stsClient.getCallerIdentity().account()); - expected.set(REGION, localStackContainer.getRegion()); - - assertThat(properties).isEqualTo(expected); - } - - private PopulateInstanceProperties.Builder populateInstancePropertiesBuilder() { - return PopulateInstancePropertiesAws.builder(stsClient, () -> Region.of(localStackContainer.getRegion())) - .instanceId("test-instance").vpcId("some-vpc").subnetIds("some-subnet"); - } -} diff --git a/java/clients/src/test/java/sleeper/clients/deploy/properties/SleeperPropertyMarkdownTableTest.java b/java/clients/src/test/java/sleeper/clients/deploy/properties/SleeperPropertyMarkdownTableTest.java index 30f632097d3..83474e01d1c 100644 --- a/java/clients/src/test/java/sleeper/clients/deploy/properties/SleeperPropertyMarkdownTableTest.java +++ b/java/clients/src/test/java/sleeper/clients/deploy/properties/SleeperPropertyMarkdownTableTest.java @@ -36,7 +36,7 @@ public class SleeperPropertyMarkdownTableTest { void shouldWriteTable() throws IOException { // Given TableWriter writer = SleeperPropertyMarkdownTable.createTableWriterForUserDefinedProperties(List.of( - CommonProperty.REGION, + CommonProperty.RETAIN_LOGS_AFTER_DESTROY, CommonProperty.FILE_SYSTEM)); // When diff --git a/java/clients/src/test/java/sleeper/clients/documentation/GeneratePropertiesTemplatesTest.java b/java/clients/src/test/java/sleeper/clients/documentation/GeneratePropertiesTemplatesTest.java index cd72991f558..2523bf8fd8d 100644 --- a/java/clients/src/test/java/sleeper/clients/documentation/GeneratePropertiesTemplatesTest.java +++ b/java/clients/src/test/java/sleeper/clients/documentation/GeneratePropertiesTemplatesTest.java @@ -42,9 +42,7 @@ import static java.util.regex.Pattern.DOTALL; import static org.assertj.core.api.Assertions.assertThat; import static sleeper.core.properties.PropertiesUtils.loadProperties; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.table.TableProperty.ITERATOR_CLASS_NAME; @@ -66,8 +64,6 @@ static class MandatoryInstancePropertyTemplateValues implements ArgumentsProvide public Stream provideArguments(ExtensionContext context) { return Stream.of( Arguments.of(ID, "full-example"), - Arguments.of(ACCOUNT, "1234567890"), - Arguments.of(REGION, "eu-west-2"), Arguments.of(VPC_ID, "1234567890"), Arguments.of(SUBNETS, "subnet-abcdefgh")); } diff --git a/java/clients/src/test/java/sleeper/clients/testutil/JarsBucketITBase.java b/java/clients/src/test/java/sleeper/clients/testutil/JarsBucketITBase.java index 7ed0bf621ad..29f41aaca4f 100644 --- a/java/clients/src/test/java/sleeper/clients/testutil/JarsBucketITBase.java +++ b/java/clients/src/test/java/sleeper/clients/testutil/JarsBucketITBase.java @@ -60,7 +60,6 @@ protected boolean syncJarsToBucket(String bucketName, boolean deleteOld) throws return new SyncJars(s3Client, tempDir) .sync(SyncJarsRequest.builder() .bucketName(bucketName) - .region(localStackContainer.getRegion()) .deleteOldJars(deleteOld) .build()); } diff --git a/java/clients/src/test/resources/reports/table/property.txt b/java/clients/src/test/resources/reports/table/property.txt index 83d001b81bb..ceacf7f6b94 100644 --- a/java/clients/src/test/resources/reports/table/property.txt +++ b/java/clients/src/test/resources/reports/table/property.txt @@ -1,4 +1,4 @@ -| Property Name | Description | Default Value | Run CDK Deploy When Changed | -|--------------------|----------------------------------------------|---------------|-----------------------------| -| sleeper.region | The AWS region to deploy to. | | false | -| sleeper.filesystem | The Hadoop filesystem used to connect to S3. | s3a:// | false | +| Property Name | Description | Default Value | Run CDK Deploy When Changed | +|-----------------------------------|------------------------------------------------------------------------|---------------|-----------------------------| +| sleeper.retain.logs.after.destroy | Whether to keep the sleeper log groups when the instance is destroyed. | true | true | +| sleeper.filesystem | The Hadoop filesystem used to connect to S3. | s3a:// | false | diff --git a/java/core/src/main/java/sleeper/core/deploy/DockerDeployment.java b/java/core/src/main/java/sleeper/core/deploy/DockerDeployment.java index 81ae36377f8..d87abe2ab73 100644 --- a/java/core/src/main/java/sleeper/core/deploy/DockerDeployment.java +++ b/java/core/src/main/java/sleeper/core/deploy/DockerDeployment.java @@ -22,10 +22,10 @@ import java.util.Collections; import java.util.List; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ECR_REPOSITORY_PREFIX; -import static sleeper.core.properties.instance.CommonProperty.REGION; /** * A deployment of a Sleeper component that is deployed with Docker. Can be used to derive the Docker image names and @@ -116,6 +116,7 @@ public boolean isCreateEmrServerlessPolicy() { /** * Retrieves the Docker image name for this deployment. Includes the repository URL and the tag. + * This method requires that CDK defined properties are set due to requiring account and region. * * @param properties the instance properties * @return the Docker image name diff --git a/java/core/src/main/java/sleeper/core/deploy/PopulateInstanceProperties.java b/java/core/src/main/java/sleeper/core/deploy/PopulateInstanceProperties.java deleted file mode 100644 index 62b655d0b2a..00000000000 --- a/java/core/src/main/java/sleeper/core/deploy/PopulateInstanceProperties.java +++ /dev/null @@ -1,180 +0,0 @@ -/* - * Copyright 2022-2025 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package sleeper.core.deploy; - -import org.apache.commons.lang3.ObjectUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import sleeper.core.properties.instance.InstanceProperties; -import sleeper.core.properties.instance.UserDefinedInstanceProperty; - -import java.util.Objects; -import java.util.Properties; -import java.util.function.Consumer; -import java.util.function.Supplier; - -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; -import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; -import static sleeper.core.properties.instance.CommonProperty.SUBNETS; -import static sleeper.core.properties.instance.CommonProperty.VPC_ID; - -/** - * Populates instance properties when deploying a new instance or when tearing down an instance without the properties. - */ -public class PopulateInstanceProperties { - public static final Logger LOGGER = LoggerFactory.getLogger(PopulateInstanceProperties.class); - - private final Supplier accountSupplier; - private final Supplier regionIdSupplier; - private final String instanceId; - private final String vpcId; - private final String subnetIds; - private final Consumer extraInstanceProperties; - - private PopulateInstanceProperties(Builder builder) { - accountSupplier = Objects.requireNonNull(builder.accountSupplier, "accountSupplier must not be null"); - regionIdSupplier = Objects.requireNonNull(builder.regionIdSupplier, "regionIdSupplier must not be null"); - instanceId = ObjectUtils.requireNonEmpty(builder.instanceId, "instanceId must not be empty"); - vpcId = ObjectUtils.requireNonEmpty(builder.vpcId, "vpcId must not be empty"); - subnetIds = ObjectUtils.requireNonEmpty(builder.subnetIds, "subnetIds must not be empty"); - extraInstanceProperties = Objects.requireNonNull(builder.extraInstanceProperties, "extraInstanceProperties must not be null"); - } - - public static Builder builder() { - return new Builder(); - } - - /** - * Sets instance properties when deploying a new instance. - * - * @param properties the properties specified by the user - * @return the populated properties - */ - public InstanceProperties populate(InstanceProperties properties) { - properties.set(ID, instanceId); - Properties tagsProperties = properties.getTagsProperties(); - tagsProperties.setProperty("InstanceID", instanceId); - properties.loadTags(tagsProperties); - properties.set(ACCOUNT, accountSupplier.get()); - properties.set(REGION, regionIdSupplier.get()); - properties.set(VPC_ID, vpcId); - properties.set(SUBNETS, subnetIds); - extraInstanceProperties.accept(properties); - return properties; - } - - /** - * Sets the values of properties that have been set in environment variables. - * - * @param instanceProperties the instance properties - */ - public static void setFromEnvironmentVariables(InstanceProperties instanceProperties) { - for (UserDefinedInstanceProperty property : UserDefinedInstanceProperty.getAll()) { - String value = System.getenv(property.toEnvironmentVariable()); - if (value != null) { - instanceProperties.set(property, value); - } - } - } - - /** - * Builds instances of this class. - */ - public static final class Builder { - private Supplier accountSupplier; - private Supplier regionIdSupplier; - private String instanceId; - private String vpcId; - private String subnetIds; - private Consumer extraInstanceProperties = properties -> { - }; - - private Builder() { - } - - /** - * Sets the AWS code to retrieve the account to deploy to. - * - * @param accountSupplier the code - * @return this builder - */ - public Builder accountSupplier(Supplier accountSupplier) { - this.accountSupplier = accountSupplier; - return this; - } - - /** - * Sets the AWS code to retrieve the ID of the region to deploy to. - * - * @param regionIdSupplier the code - * @return this builder - */ - public Builder regionIdSupplier(Supplier regionIdSupplier) { - this.regionIdSupplier = regionIdSupplier; - return this; - } - - /** - * Sets the instance ID of the Sleeper instance to deploy. - * - * @param instanceId the instance ID - * @return this builder - */ - public Builder instanceId(String instanceId) { - this.instanceId = instanceId; - return this; - } - - /** - * Sets the ID of the VPC to deploy to. - * - * @param vpcId the VPC ID - * @return this builder - */ - public Builder vpcId(String vpcId) { - this.vpcId = vpcId; - return this; - } - - /** - * Sets the comma-separated IDs of the subnets to deploy to. - * - * @param subnetIds the subnet IDs (comma-separated) - * @return this builder - */ - public Builder subnetIds(String subnetIds) { - this.subnetIds = subnetIds; - return this; - } - - /** - * Sets any extra instance properties that should be set for the instance. - * - * @param extraInstanceProperties the function to set the extra properties - * @return this builder - */ - public Builder extraInstanceProperties(Consumer extraInstanceProperties) { - this.extraInstanceProperties = extraInstanceProperties; - return this; - } - - public PopulateInstanceProperties build() { - return new PopulateInstanceProperties(this); - } - } -} diff --git a/java/core/src/main/java/sleeper/core/deploy/SleeperInstanceConfiguration.java b/java/core/src/main/java/sleeper/core/deploy/SleeperInstanceConfiguration.java index 4a06fb3113b..7fa5015a9de 100644 --- a/java/core/src/main/java/sleeper/core/deploy/SleeperInstanceConfiguration.java +++ b/java/core/src/main/java/sleeper/core/deploy/SleeperInstanceConfiguration.java @@ -67,15 +67,13 @@ public static SleeperInstanceConfiguration withNoTables(InstanceProperties insta /** * Creates a configuration for a new instance, setting tables from templates if not specified. * - * @param instancePropertiesPath the path to the local configuration instance properties file - * @param populateInstanceProperties the settings to populate the instance properties - * @param fromTemplates the settings to load the templates - * @return the instance configuration + * @param instancePropertiesPath the path to the local configuration instance properties file + * @param fromTemplates the settings to load the templates + * @return the instance configuration */ public static SleeperInstanceConfiguration forNewInstanceDefaultingTables( - Path instancePropertiesPath, PopulateInstanceProperties populateInstanceProperties, - SleeperInstanceConfigurationFromTemplates fromTemplates) { - SleeperInstanceConfiguration configuration = fromLocalConfiguration(instancePropertiesPath, populateInstanceProperties); + Path instancePropertiesPath, SleeperInstanceConfigurationFromTemplates fromTemplates) { + SleeperInstanceConfiguration configuration = fromLocalConfiguration(instancePropertiesPath); if (configuration.getTableProperties().isEmpty()) { configuration = configuration.withTableProperties(instanceProperties -> List.of( fromTemplates.loadTableProperties(instanceProperties))); @@ -87,21 +85,18 @@ public static SleeperInstanceConfiguration forNewInstanceDefaultingTables( * Creates a configuration for a new instance, setting instance properties from templates if not * specified. * - * @param instancePropertiesPath the path to the local configuration instance properties file, or null if not - * present - * @param populateInstanceProperties the settings to populate the instance properties - * @param templatesDir the directory to load the templates from - * @return the instance configuration + * @param instancePropertiesPath the path to the local configuration instance properties file, or null if not + * present + * @param templatesDir the directory to load the templates from + * @return the instance configuration */ public static SleeperInstanceConfiguration forNewInstanceDefaultingInstance( - Path instancePropertiesPath, PopulateInstanceProperties populateInstanceProperties, - Path templatesDir) { + Path instancePropertiesPath, Path templatesDir) { if (instancePropertiesPath != null) { - return fromLocalConfiguration(instancePropertiesPath, populateInstanceProperties); + return fromLocalConfiguration(instancePropertiesPath); } else { - InstanceProperties instanceProperties = SleeperInstanceConfigurationFromTemplates.loadInstanceProperties(templatesDir); - populateInstanceProperties.populate(instanceProperties); - return new SleeperInstanceConfiguration(instanceProperties, List.of()); + return new SleeperInstanceConfiguration(SleeperInstanceConfigurationFromTemplates.loadInstanceProperties(templatesDir), + List.of()); } } @@ -121,13 +116,6 @@ public static SleeperInstanceConfiguration fromLocalConfiguration(Path instanceP .tableProperties(tableProperties).build(); } - private static SleeperInstanceConfiguration fromLocalConfiguration( - Path instancePropertiesPath, PopulateInstanceProperties populateInstanceProperties) { - SleeperInstanceConfiguration configuration = fromLocalConfiguration(instancePropertiesPath); - populateInstanceProperties.populate(configuration.getInstanceProperties()); - return configuration; - } - /** * Checks that the configuration is valid and throws an exception otherwise. * diff --git a/java/core/src/main/java/sleeper/core/properties/instance/CdkDefinedInstanceProperty.java b/java/core/src/main/java/sleeper/core/properties/instance/CdkDefinedInstanceProperty.java index 909ceb57254..7c6fcc33832 100644 --- a/java/core/src/main/java/sleeper/core/properties/instance/CdkDefinedInstanceProperty.java +++ b/java/core/src/main/java/sleeper/core/properties/instance/CdkDefinedInstanceProperty.java @@ -63,6 +63,16 @@ static List getAllInGroup(PropertyGroup group) { .propertyGroup(InstancePropertyGroup.COMMON) .build(); + //AWS Config + CdkDefinedInstanceProperty ACCOUNT = Index.propertyBuilder("sleeper.account") + .description("The AWS account number. This is the AWS account that the instance is deployed in.") + .propertyGroup(InstancePropertyGroup.COMMON) + .build(); + CdkDefinedInstanceProperty REGION = Index.propertyBuilder("sleeper.region") + .description("The AWS region the instance is deployed in.") + .propertyGroup(InstancePropertyGroup.COMMON) + .build(); + // Data CdkDefinedInstanceProperty DATA_BUCKET = Index.propertyBuilder("sleeper.data.bucket") .description("The S3 bucket name used to store table data.") diff --git a/java/core/src/main/java/sleeper/core/properties/instance/CommonProperty.java b/java/core/src/main/java/sleeper/core/properties/instance/CommonProperty.java index b9d79d273e3..8ade9400801 100644 --- a/java/core/src/main/java/sleeper/core/properties/instance/CommonProperty.java +++ b/java/core/src/main/java/sleeper/core/properties/instance/CommonProperty.java @@ -115,16 +115,6 @@ public interface CommonProperty { .propertyGroup(InstancePropertyGroup.COMMON) .runCdkDeployWhenChanged(true) .build(); - UserDefinedInstanceProperty ACCOUNT = Index.propertyBuilder("sleeper.account") - .description("The AWS account number. This is the AWS account that the instance will be deployed to.") - .validationPredicate(Objects::nonNull) - .propertyGroup(InstancePropertyGroup.COMMON) - .editable(false).build(); - UserDefinedInstanceProperty REGION = Index.propertyBuilder("sleeper.region") - .description("The AWS region to deploy to.") - .validationPredicate(Objects::nonNull) - .propertyGroup(InstancePropertyGroup.COMMON) - .editable(false).build(); UserDefinedInstanceProperty ENDPOINT_URL = Index.propertyBuilder("sleeper.endpoint.url") .description("The AWS endpoint URL. This should only be set for a non-standard service endpoint. Usually " + "this is used to set the URL to LocalStack for a locally deployed instance.") diff --git a/java/core/src/test/java/sleeper/core/deploy/DockerDeploymentTest.java b/java/core/src/test/java/sleeper/core/deploy/DockerDeploymentTest.java index 933af245fc9..b3fd947de67 100644 --- a/java/core/src/test/java/sleeper/core/deploy/DockerDeploymentTest.java +++ b/java/core/src/test/java/sleeper/core/deploy/DockerDeploymentTest.java @@ -20,11 +20,11 @@ import sleeper.core.properties.instance.InstanceProperties; import static org.assertj.core.api.Assertions.assertThat; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ECR_REPOSITORY_PREFIX; import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; public class DockerDeploymentTest { diff --git a/java/core/src/test/java/sleeper/core/deploy/PopulateInstancePropertiesTest.java b/java/core/src/test/java/sleeper/core/deploy/PopulateInstancePropertiesTest.java deleted file mode 100644 index 05016eb72ab..00000000000 --- a/java/core/src/test/java/sleeper/core/deploy/PopulateInstancePropertiesTest.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Copyright 2022-2025 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package sleeper.core.deploy; - -import org.junit.jupiter.api.Test; - -import sleeper.core.properties.instance.InstanceProperties; - -import java.util.Map; - -import static org.assertj.core.api.Assertions.assertThat; -import static sleeper.core.deploy.PopulatePropertiesTestHelper.createTestPopulateInstanceProperties; -import static sleeper.core.deploy.PopulatePropertiesTestHelper.testPopulateInstancePropertiesBuilder; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; -import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; -import static sleeper.core.properties.instance.CommonProperty.SUBNETS; -import static sleeper.core.properties.instance.CommonProperty.VPC_ID; -import static sleeper.core.properties.instance.PartitionSplittingProperty.DEFAULT_PARTITION_SPLIT_THRESHOLD; - -public class PopulateInstancePropertiesTest { - - private InstanceProperties expectedInstanceProperties() { - InstanceProperties expected = new InstanceProperties(); - expected.setTags(Map.of("InstanceID", "test-instance")); - expected.set(ID, "test-instance"); - expected.set(VPC_ID, "some-vpc"); - expected.set(SUBNETS, "some-subnet"); - expected.set(ACCOUNT, "test-account-id"); - expected.set(REGION, "test-region"); - return expected; - } - - @Test - void shouldPopulateInstanceProperties() { - // Given/When - InstanceProperties properties = createTestPopulateInstanceProperties().populate(new InstanceProperties()); - - // Then - assertThat(properties).isEqualTo(expectedInstanceProperties()); - } - - @Test - void shouldGetDefaultTagsWhenNotProvidedAndNotSetInInstanceProperties() { - // Given/When - InstanceProperties properties = createTestPopulateInstanceProperties().populate(new InstanceProperties()); - - // Then - assertThat(properties.getTags()) - .isEqualTo(Map.of("InstanceID", "test-instance")); - } - - @Test - void shouldAddToExistingTagsWhenSetInInstanceProperties() { - // Given/When - InstanceProperties properties = new InstanceProperties(); - properties.setTags(Map.of("TestTag", "TestValue")); - createTestPopulateInstanceProperties().populate(properties); - - // Then - assertThat(properties.getTags()) - .isEqualTo(Map.of("TestTag", "TestValue", - "InstanceID", "test-instance")); - } - - @Test - void shouldSetExtraProperties() { - // Given/When - InstanceProperties properties = new InstanceProperties(); - testPopulateInstancePropertiesBuilder() - .extraInstanceProperties(p -> p.setNumber(DEFAULT_PARTITION_SPLIT_THRESHOLD, 1000)) - .build().populate(properties); - - // Then - assertThat(properties.getInt(DEFAULT_PARTITION_SPLIT_THRESHOLD)) - .isEqualTo(1000); - } - -} diff --git a/java/core/src/test/java/sleeper/core/deploy/PopulatePropertiesTestHelper.java b/java/core/src/test/java/sleeper/core/deploy/PopulatePropertiesTestHelper.java deleted file mode 100644 index 86bf3e0021f..00000000000 --- a/java/core/src/test/java/sleeper/core/deploy/PopulatePropertiesTestHelper.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright 2022-2025 Crown Copyright - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package sleeper.core.deploy; - -import sleeper.core.properties.instance.InstanceProperties; -import sleeper.core.properties.table.TableProperties; - -import static sleeper.core.properties.table.TableProperty.TABLE_NAME; -import static sleeper.core.schema.SchemaTestHelper.createSchemaWithKey; - -/** - * Test helpers to generate properties using PopulateInstanceProperties. - */ -public class PopulatePropertiesTestHelper { - private PopulatePropertiesTestHelper() { - } - - /** - * Generates test instance properties using PopulateInstanceProperties. - * - * @return the generated properties - */ - public static InstanceProperties generateTestInstanceProperties() { - return createTestPopulateInstanceProperties().populate(new InstanceProperties()); - } - - /** - * Creates a test instance of PopulateInstanceProperties with dummy values. - * - * @return the object - */ - public static PopulateInstanceProperties createTestPopulateInstanceProperties() { - return testPopulateInstancePropertiesBuilder().build(); - } - - /** - * Creates a test builder for PopulateInstanceProperties with dummy values. - * - * @return the builder - */ - public static PopulateInstanceProperties.Builder testPopulateInstancePropertiesBuilder() { - return PopulateInstanceProperties.builder() - .accountSupplier(() -> "test-account-id").regionIdSupplier(() -> "test-region") - .instanceId("test-instance").vpcId("some-vpc").subnetIds("some-subnet"); - } - - /** - * Generates test table properties using PopulateInstanceProperties. - * - * @return the generated properties - */ - public static TableProperties generateTestTableProperties() { - TableProperties tableProperties = new TableProperties(generateTestInstanceProperties()); - tableProperties.set(TABLE_NAME, "test-table"); - tableProperties.setSchema(createSchemaWithKey("key")); - return tableProperties; - } -} diff --git a/java/core/src/test/java/sleeper/core/deploy/SleeperInstanceConfigurationIT.java b/java/core/src/test/java/sleeper/core/deploy/SleeperInstanceConfigurationIT.java index bf890fd25f7..90e3c83ccad 100644 --- a/java/core/src/test/java/sleeper/core/deploy/SleeperInstanceConfigurationIT.java +++ b/java/core/src/test/java/sleeper/core/deploy/SleeperInstanceConfigurationIT.java @@ -34,7 +34,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static sleeper.core.deploy.PopulatePropertiesTestHelper.createTestPopulateInstanceProperties; import static sleeper.core.properties.instance.CommonProperty.FILE_SYSTEM; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.table.TableProperty.ROW_GROUP_SIZE; @@ -163,14 +162,13 @@ void shouldPopulateInstancePropertiesFromLocalConfig() throws Exception { Path instancePropertiesPath = Files.writeString( propertiesDir.resolve("instance.properties"), "sleeper.filesystem=test://"); - PopulateInstanceProperties populateProperties = createTestPopulateInstanceProperties(); // When SleeperInstanceConfiguration config = SleeperInstanceConfiguration.forNewInstanceDefaultingInstance( - instancePropertiesPath, populateProperties, templatesDir); + instancePropertiesPath, templatesDir); // Then - InstanceProperties expectedInstanceProperties = populateProperties.populate(new InstanceProperties()); + InstanceProperties expectedInstanceProperties = new InstanceProperties(); expectedInstanceProperties.set(FILE_SYSTEM, "test://"); assertThat(config).isEqualTo(new SleeperInstanceConfiguration(expectedInstanceProperties, List.of())); } @@ -179,14 +177,13 @@ void shouldPopulateInstancePropertiesFromLocalConfig() throws Exception { void shouldPopulateInstancePropertiesFromTemplate() throws Exception { // Given writeTemplates(); - PopulateInstanceProperties populateProperties = createTestPopulateInstanceProperties(); - // When SleeperInstanceConfiguration config = SleeperInstanceConfiguration.forNewInstanceDefaultingInstance( - null, populateProperties, templatesDir); + null, templatesDir); // Then - InstanceProperties expectedInstanceProperties = populateProperties.populate(new InstanceProperties()); + InstanceProperties expectedInstanceProperties = new InstanceProperties(); + Map tags = new HashMap<>(expectedInstanceProperties.getTags()); tags.put("Project", "TemplateProject"); expectedInstanceProperties.setTags(tags); @@ -209,17 +206,16 @@ void shouldPopulateTablePropertiesFromLocalConfig() throws Exception { Files.writeString( propertiesDir.resolve("table.properties"), "sleeper.table.name=some-table"); - PopulateInstanceProperties populateProperties = createTestPopulateInstanceProperties(); SleeperInstanceConfigurationFromTemplates fromTemplates = SleeperInstanceConfigurationFromTemplates.builder() .templatesDir(templatesDir) .tableNameForTemplate("template-table").build(); // When SleeperInstanceConfiguration config = SleeperInstanceConfiguration.forNewInstanceDefaultingTables( - instancePropertiesPath, populateProperties, fromTemplates); + instancePropertiesPath, fromTemplates); // Then - InstanceProperties expectedInstanceProperties = populateProperties.populate(new InstanceProperties()); + InstanceProperties expectedInstanceProperties = new InstanceProperties(); expectedInstanceProperties.set(FILE_SYSTEM, "test://"); TableProperties expectedTableProperties = new TableProperties(expectedInstanceProperties); expectedTableProperties.set(TABLE_NAME, "some-table"); @@ -233,17 +229,16 @@ void shouldPopulateTablePropertiesFromTemplate() throws Exception { Path instancePropertiesPath = Files.writeString( propertiesDir.resolve("instance.properties"), "sleeper.filesystem=test://"); - PopulateInstanceProperties populateProperties = createTestPopulateInstanceProperties(); SleeperInstanceConfigurationFromTemplates fromTemplates = SleeperInstanceConfigurationFromTemplates.builder() .templatesDir(templatesDir) .tableNameForTemplate("template-table").build(); // When SleeperInstanceConfiguration config = SleeperInstanceConfiguration.forNewInstanceDefaultingTables( - instancePropertiesPath, populateProperties, fromTemplates); + instancePropertiesPath, fromTemplates); // Then - InstanceProperties expectedInstanceProperties = populateProperties.populate(new InstanceProperties()); + InstanceProperties expectedInstanceProperties = new InstanceProperties(); expectedInstanceProperties.set(FILE_SYSTEM, "test://"); TableProperties expectedTableProperties = new TableProperties(expectedInstanceProperties); expectedTableProperties.set(TABLE_NAME, "template-table"); diff --git a/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesPrettyPrinterTest.java b/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesPrettyPrinterTest.java index 87b823cbd74..9616c447fc5 100644 --- a/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesPrettyPrinterTest.java +++ b/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesPrettyPrinterTest.java @@ -102,7 +102,7 @@ void shouldPrintPropertiesInTheCorrectOrder() { String output = printEmptyInstanceProperties(); // Then - assertThat(output.indexOf("sleeper.account")) + assertThat(output.indexOf("sleeper.endpoint.url")) .isLessThan(output.indexOf("sleeper.log.retention.days")) .isLessThan(output.indexOf("sleeper.vpc")); assertThat(output.indexOf("sleeper.ingest")) @@ -116,9 +116,13 @@ class PrintValues { @Test void shouldPrintPropertyValueWithDescription() { // When / Then - assertThat(printInstanceProperties("sleeper.account=1234567890")) - .contains("# The AWS account number. This is the AWS account that the instance will be deployed to.\n" + - "sleeper.account=1234567890\n"); + assertThat(printInstanceProperties("sleeper.log.retention.days=30")) + .contains("# The length of time in days that CloudWatch logs from lambda functions, ECS containers, etc., are\n" + + "# retained.\n" + + "# See https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-logs-loggroup.html\n" + + "# for valid options.\n" + + "# Use -1 to indicate infinite retention.\n" + + "sleeper.log.retention.days=30"); } @Test diff --git a/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesTest.java b/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesTest.java index fc692afd285..805180a3293 100644 --- a/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesTest.java +++ b/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesTest.java @@ -29,7 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static sleeper.core.properties.PropertiesUtils.loadProperties; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.OPTIONAL_STACKS; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.USER_JARS; diff --git a/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesValidationTest.java b/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesValidationTest.java index ced58581375..d47d1591308 100644 --- a/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesValidationTest.java +++ b/java/core/src/test/java/sleeper/core/properties/SleeperPropertiesValidationTest.java @@ -30,12 +30,10 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static sleeper.core.properties.PropertiesUtils.loadProperties; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; import static sleeper.core.properties.instance.CommonProperty.LOG_RETENTION_IN_DAYS; import static sleeper.core.properties.instance.CommonProperty.MAXIMUM_CONNECTIONS_TO_S3; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.table.TableProperty.COMPACTION_JOB_SEND_TIMEOUT_SECS; @@ -114,16 +112,14 @@ class ValidateInstanceProperties { void shouldFailValidationIfRequiredPropertyIsMissing() { // Given - no account set InstanceProperties instanceProperties = new InstanceProperties(); - instanceProperties.set(REGION, "eu-west-2"); instanceProperties.set(JARS_BUCKET, "jars"); instanceProperties.set(VERSION, "0.1"); - instanceProperties.set(ID, "test"); instanceProperties.set(VPC_ID, "aVPC"); instanceProperties.set(SUBNETS, "subnet1"); // When / Then assertThatThrownBy(instanceProperties::validate) - .hasMessageContaining(ACCOUNT.getPropertyName()); + .hasMessageContaining(ID.getPropertyName()); } @Test diff --git a/java/core/src/test/java/sleeper/core/properties/instance/InstancePropertiesTest.java b/java/core/src/test/java/sleeper/core/properties/instance/InstancePropertiesTest.java index f030ef250cd..693fa417d6a 100644 --- a/java/core/src/test/java/sleeper/core/properties/instance/InstancePropertiesTest.java +++ b/java/core/src/test/java/sleeper/core/properties/instance/InstancePropertiesTest.java @@ -33,6 +33,7 @@ import static sleeper.core.properties.PropertiesUtils.loadProperties; import static sleeper.core.properties.instance.ArrayListIngestProperty.MAX_IN_MEMORY_BATCH_SIZE; import static sleeper.core.properties.instance.ArrayListIngestProperty.MAX_ROWS_TO_WRITE_LOCALLY; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_AUTO_SCALING_GROUP; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_CLUSTER; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.COMPACTION_JOB_DLQ_URL; @@ -46,8 +47,8 @@ import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_LAMBDA_ROLE; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_QUEUE_URL; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_RESULTS_QUEUE_URL; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.DEFAULT_LAMBDA_CONCURRENCY_RESERVED; import static sleeper.core.properties.instance.CommonProperty.EMAIL_ADDRESS_FOR_ERROR_NOTIFICATION; import static sleeper.core.properties.instance.CommonProperty.FARGATE_VERSION; @@ -56,7 +57,6 @@ import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; import static sleeper.core.properties.instance.CommonProperty.LOG_RETENTION_IN_DAYS; import static sleeper.core.properties.instance.CommonProperty.MAXIMUM_CONNECTIONS_TO_S3; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_MEMORY_IN_MB; import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_TIMEOUT_IN_SECONDS; diff --git a/java/core/src/test/java/sleeper/core/properties/testutils/InstancePropertiesTestHelper.java b/java/core/src/test/java/sleeper/core/properties/testutils/InstancePropertiesTestHelper.java index 2ce8e9bfbc8..9bbe177cb3f 100644 --- a/java/core/src/test/java/sleeper/core/properties/testutils/InstancePropertiesTestHelper.java +++ b/java/core/src/test/java/sleeper/core/properties/testutils/InstancePropertiesTestHelper.java @@ -31,11 +31,13 @@ import static sleeper.core.properties.instance.ArrowIngestProperty.ARROW_INGEST_MAX_LOCAL_STORE_BYTES; import static sleeper.core.properties.instance.ArrowIngestProperty.ARROW_INGEST_MAX_SINGLE_WRITE_TO_FILE_ROWS; import static sleeper.core.properties.instance.ArrowIngestProperty.ARROW_INGEST_WORKING_BUFFER_BYTES; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.CONFIG_BUCKET; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.DATA_BUCKET; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_RESULTS_BUCKET; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_TRACKER_TABLE_NAME; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.QUERY_WEBSOCKET_API_URL; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.TABLE_ID_INDEX_DYNAMO_TABLENAME; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.TABLE_NAME_INDEX_DYNAMO_TABLENAME; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.TABLE_ONLINE_INDEX_DYNAMO_TABLENAME; @@ -44,11 +46,9 @@ import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.TRANSACTION_LOG_LATEST_SNAPSHOTS_TABLENAME; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.TRANSACTION_LOG_PARTITIONS_TABLENAME; import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.VERSION; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; import static sleeper.core.properties.instance.CommonProperty.ID; import static sleeper.core.properties.instance.CommonProperty.JARS_BUCKET; import static sleeper.core.properties.instance.CommonProperty.MAXIMUM_CONNECTIONS_TO_S3; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.instance.IngestProperty.INGEST_PARTITION_REFRESH_PERIOD_IN_SECONDS; @@ -84,15 +84,15 @@ public static InstanceProperties createTestInstanceProperties() { public static InstanceProperties createTestInstancePropertiesWithId(String id) { InstanceProperties instanceProperties = new InstanceProperties(); instanceProperties.set(ID, id); + instanceProperties.set(ACCOUNT, "test-account"); + instanceProperties.set(REGION, "test-region"); + instanceProperties.set(VPC_ID, "test-vpc"); + instanceProperties.set(SUBNETS, "test-subnet"); instanceProperties.set(CONFIG_BUCKET, InstanceProperties.getConfigBucketFromInstanceId(id)); instanceProperties.set(DATA_BUCKET, "sleeper-" + id + "-table-data"); instanceProperties.set(JARS_BUCKET, "sleeper-" + id + "-jars"); instanceProperties.set(QUERY_RESULTS_BUCKET, "sleeper-" + id + "-query-results"); - instanceProperties.set(ACCOUNT, "test-account"); - instanceProperties.set(REGION, "test-region"); instanceProperties.set(VERSION, "1.2.3"); - instanceProperties.set(VPC_ID, "test-vpc"); - instanceProperties.set(SUBNETS, "test-subnet"); instanceProperties.set(TRANSACTION_LOG_FILES_TABLENAME, "sleeper-" + id + "-file-transaction-log"); instanceProperties.set(TRANSACTION_LOG_PARTITIONS_TABLENAME, "sleeper-" + id + "-partition-transaction-log"); instanceProperties.set(TRANSACTION_LOG_ALL_SNAPSHOTS_TABLENAME, "sleeper-" + id + "-transaction-log-all-snapshots"); diff --git a/java/core/src/test/java/sleeper/core/properties/testutils/TablePropertiesTestHelper.java b/java/core/src/test/java/sleeper/core/properties/testutils/TablePropertiesTestHelper.java index 41557dff24e..88ef6da3879 100644 --- a/java/core/src/test/java/sleeper/core/properties/testutils/TablePropertiesTestHelper.java +++ b/java/core/src/test/java/sleeper/core/properties/testutils/TablePropertiesTestHelper.java @@ -63,4 +63,5 @@ public static TableProperties createTestTablePropertiesWithNoSchema(InstanceProp tableProperties.set(TABLE_ID, tableId); return tableProperties; } + } diff --git a/java/system-test/system-test-cdk/src/main/java/sleeper/systemtest/cdk/SystemTestApp.java b/java/system-test/system-test-cdk/src/main/java/sleeper/systemtest/cdk/SystemTestApp.java index 1f831410ff8..e7ed93db5cd 100644 --- a/java/system-test/system-test-cdk/src/main/java/sleeper/systemtest/cdk/SystemTestApp.java +++ b/java/system-test/system-test-cdk/src/main/java/sleeper/systemtest/cdk/SystemTestApp.java @@ -33,9 +33,9 @@ import sleeper.core.properties.instance.InstanceProperties; import sleeper.systemtest.configuration.SystemTestProperties; -import static sleeper.core.properties.instance.CommonProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; import static sleeper.core.properties.instance.CommonProperty.ID; -import static sleeper.core.properties.instance.CommonProperty.REGION; import static sleeper.systemtest.configuration.SystemTestProperty.SYSTEM_TEST_CLUSTER_ENABLED; /** @@ -46,15 +46,14 @@ public class SystemTestApp extends Stack { private final SleeperJarsInBucket jars; private final SystemTestProperties instanceProperties; - public SystemTestApp(App app, String id, StackProps props, SleeperInstanceProps sleeperProps) { - super(app, id, props); - this.props = sleeperProps; + public SystemTestApp(App app, String id, StackProps stackProps, SleeperInstanceProps sleeperProps) { + super(app, id, stackProps); + this.props = setProps(sleeperProps); this.jars = sleeperProps.getJars(); this.instanceProperties = SystemTestProperties.from(sleeperProps.getInstanceProperties()); } public void create() { - LoggingStack loggingStack = new LoggingStack(this, "Logging", instanceProperties); AutoDeleteS3ObjectsStack autoDeleteS3Stack = new AutoDeleteS3ObjectsStack(this, "AutoDeleteS3Objects", instanceProperties, jars, loggingStack); @@ -65,6 +64,7 @@ public void create() { // Sleeper instance SleeperCoreStacks coreStacks = SleeperCoreStacks.create(this, props, loggingStack, autoDeleteS3Stack); SleeperOptionalStacks.create(this, props, coreStacks); + coreStacks.createRoles(); // Stack for writing random data @@ -88,8 +88,8 @@ public static void main(String[] args) { String id = instanceProperties.get(ID); Environment environment = Environment.builder() - .account(instanceProperties.get(ACCOUNT)) - .region(instanceProperties.get(REGION)) + .account(System.getenv("CDK_DEFAULT_ACCOUNT")) + .region(System.getenv("CDK_DEFAULT_REGION")) .build(); new SystemTestApp(app, id, StackProps.builder() @@ -101,4 +101,10 @@ public static void main(String[] args) { app.synth(); } } + + private SleeperInstanceProps setProps(SleeperInstanceProps props) { + props.getInstanceProperties().set(ACCOUNT, getAccount()); + props.getInstanceProperties().set(REGION, getRegion()); + return props; + } } diff --git a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java index a44abaa1cc0..ea78b13e2e4 100644 --- a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java +++ b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/cdk/DeployNewTestInstance.java @@ -15,7 +15,6 @@ */ package sleeper.systemtest.drivers.cdk; -import software.amazon.awssdk.regions.providers.AwsRegionProvider; import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.ecr.EcrClient; @@ -24,9 +23,7 @@ import sleeper.clients.deploy.DeployNewInstance; import sleeper.clients.deploy.container.StackDockerImage; -import sleeper.clients.deploy.properties.PopulateInstancePropertiesAws; import sleeper.clients.util.cdk.InvokeCdk; -import sleeper.core.deploy.PopulateInstanceProperties; import sleeper.core.deploy.SleeperInstanceConfiguration; import sleeper.core.deploy.SleeperInstanceConfigurationFromTemplates; @@ -36,6 +33,9 @@ import static sleeper.clients.deploy.container.StackDockerImage.dockerBuildImage; import static sleeper.clients.util.ClientUtils.optionalArgument; +import static sleeper.core.properties.instance.CommonProperty.ID; +import static sleeper.core.properties.instance.CommonProperty.SUBNETS; +import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.systemtest.configuration.SystemTestProperty.SYSTEM_TEST_REPO; public class DeployNewTestInstance { @@ -50,27 +50,27 @@ public static void main(String[] args) throws IOException, InterruptedException throw new IllegalArgumentException("Usage: " + " "); } - AwsRegionProvider regionProvider = DefaultAwsRegionProviderChain.builder().build(); try (S3Client s3Client = S3Client.create(); DynamoDbClient dynamoClient = DynamoDbClient.create(); StsClient stsClient = StsClient.create(); EcrClient ecrClient = EcrClient.create()) { Path scriptsDirectory = Path.of(args[0]); Path propertiesFile = Path.of(args[1]); - PopulateInstanceProperties populateInstanceProperties = PopulateInstancePropertiesAws.builder(stsClient, regionProvider) - .instanceId(args[2]).vpcId(args[3]).subnetIds(args[4]) - .extraInstanceProperties(properties -> properties.set(SYSTEM_TEST_REPO, args[2] + "/system-test")) - .build(); + boolean deployPaused = "true".equalsIgnoreCase(optionalArgument(args, 5).orElse("false")); Path splitPointsFileForTemplate = optionalArgument(args, 6).map(Path::of).orElse(null); + SleeperInstanceConfiguration config = SleeperInstanceConfiguration.forNewInstanceDefaultingTables( + propertiesFile, templates(scriptsDirectory, splitPointsFileForTemplate)); + config.getInstanceProperties().set(ID, args[2]); + config.getInstanceProperties().set(VPC_ID, args[3]); + config.getInstanceProperties().set(SUBNETS, args[4]); + config.getInstanceProperties().set(SYSTEM_TEST_REPO, args[2] + "/system-test"); DeployNewInstance.builder().scriptsDirectory(scriptsDirectory) - .deployInstanceConfiguration(SleeperInstanceConfiguration.forNewInstanceDefaultingTables( - propertiesFile, populateInstanceProperties, - templates(scriptsDirectory, splitPointsFileForTemplate))) + .deployInstanceConfiguration(config) .extraDockerImages(List.of(SYSTEM_TEST_IMAGE)) .deployPaused(deployPaused) .instanceType(InvokeCdk.Type.SYSTEM_TEST) - .deployWithClients(s3Client, dynamoClient, ecrClient); + .deployWithClients(s3Client, dynamoClient, ecrClient, stsClient, DefaultAwsRegionProviderChain.builder().build()); } } diff --git a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java index 78aa4a3fedf..df4b7c1300d 100644 --- a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java +++ b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSleeperInstanceDriver.java @@ -29,7 +29,6 @@ import sleeper.clients.deploy.DeployExistingInstance; import sleeper.clients.deploy.DeployNewInstance; -import sleeper.clients.deploy.properties.PopulateInstancePropertiesAws; import sleeper.clients.util.cdk.InvokeCdk; import sleeper.clients.util.command.CommandUtils; import sleeper.configuration.properties.S3InstanceProperties; @@ -46,6 +45,8 @@ import java.util.Set; import static sleeper.core.properties.instance.CommonProperty.ID; +import static sleeper.core.properties.instance.CommonProperty.SUBNETS; +import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static software.amazon.awssdk.services.cloudformation.model.StackStatus.CREATE_FAILED; import static software.amazon.awssdk.services.cloudformation.model.StackStatus.ROLLBACK_COMPLETE; @@ -86,16 +87,16 @@ public boolean deployInstanceIfNotPresent(String instanceId, SleeperInstanceConf return false; } LOGGER.info("Deploying instance: {}", instanceId); - PopulateInstancePropertiesAws.builder(sts, regionProvider) - .instanceId(instanceId).vpcId(parameters.getVpcId()).subnetIds(parameters.getSubnetIds()) - .build().populate(deployConfig.getInstanceProperties()); + deployConfig.getInstanceProperties().set(ID, instanceId); + deployConfig.getInstanceProperties().set(VPC_ID, parameters.getVpcId()); + deployConfig.getInstanceProperties().set(SUBNETS, parameters.getSubnetIds()); try { DeployNewInstance.builder().scriptsDirectory(parameters.getScriptsDirectory()) .deployInstanceConfiguration(deployConfig) .instanceType(InvokeCdk.Type.STANDARD) .runCommand(CommandUtils::runCommandLogOutput) .createMultiPlatformBuilder(parameters.isCreateMultiPlatformBuilder()) - .deployWithClients(s3, dynamoDB, ecr); + .deployWithClients(s3, dynamoDB, ecr, sts, regionProvider); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new RuntimeException(e); @@ -124,7 +125,8 @@ private boolean deployedStackIsPresent(String instanceId) { public void redeploy(InstanceProperties instanceProperties, List tableProperties) { try { DeployExistingInstance.builder() - .clients(s3, ecr) + .clients(s3, ecr, sts) + .regionProvider(regionProvider) .properties(instanceProperties) .tablePropertiesList(tableProperties) .scriptsDirectory(parameters.getScriptsDirectory()) diff --git a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSystemTestDeploymentDriver.java b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSystemTestDeploymentDriver.java index 0690e59654d..e068a823074 100644 --- a/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSystemTestDeploymentDriver.java +++ b/java/system-test/system-test-drivers/src/main/java/sleeper/systemtest/drivers/instance/AwsSystemTestDeploymentDriver.java @@ -120,7 +120,6 @@ private void uploadJarsAndDockerImages() throws IOException, InterruptedExceptio new SyncJars(s3, parameters.getJarsDirectory()) .sync(SyncJarsRequest.builder() .bucketName(SleeperArtefactsLocation.getDefaultJarsBucketName(parameters.getArtefactsDeploymentId())) - .region(parameters.getRegion()) .uploadFilter(jar -> LambdaJar.isFileJar(jar, CUSTOM_RESOURCES)) .build()); if (!parameters.isSystemTestClusterEnabled()) { @@ -132,11 +131,9 @@ private void uploadJarsAndDockerImages() throws IOException, InterruptedExceptio .deployConfig(DeployConfiguration.fromScriptsDirectory(parameters.getScriptsDirectory())) .commandRunner(CommandUtils::runCommandLogOutput) .build(), - CheckVersionExistsInEcr.withEcrClient(ecr)); + CheckVersionExistsInEcr.withEcrClient(ecr), parameters.getAccount(), parameters.getRegion()); dockerUploader.upload(UploadDockerImagesToEcrRequest.builder() .ecrPrefix(SleeperArtefactsLocation.getDefaultEcrRepositoryPrefix(parameters.getArtefactsDeploymentId())) - .account(parameters.getAccount()) - .region(parameters.getRegion()) .version(SleeperVersion.getVersion()) .images(List.of(SYSTEM_TEST_IMAGE)) .build()); diff --git a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstanceRedeployTest.java b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstanceRedeployTest.java index 732013010bf..6f4607a8387 100644 --- a/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstanceRedeployTest.java +++ b/java/system-test/system-test-dsl/src/test/java/sleeper/systemtest/dsl/instance/DeployedSleeperInstanceRedeployTest.java @@ -18,7 +18,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import sleeper.core.deploy.PopulateInstanceProperties; import sleeper.core.deploy.SleeperInstanceConfiguration; import sleeper.core.properties.instance.InstanceProperties; import sleeper.systemtest.configuration.SystemTestStandaloneProperties; @@ -28,6 +27,10 @@ import java.util.function.Consumer; import static org.assertj.core.api.Assertions.assertThat; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.ACCOUNT; +import static sleeper.core.properties.instance.CdkDefinedInstanceProperty.REGION; +import static sleeper.core.properties.instance.CommonProperty.ID; +import static sleeper.core.properties.instance.CommonProperty.SUBNETS; import static sleeper.core.properties.instance.CommonProperty.USER_JARS; import static sleeper.core.properties.instance.CommonProperty.VPC_ID; import static sleeper.core.properties.instance.CompactionProperty.COMPACTION_COMMIT_BATCHING_WINDOW_IN_SECONDS; @@ -121,14 +124,13 @@ private InstanceProperties fakeDeploy() { // Replicate system test behaviour InstanceProperties properties = buildDeployConfig().getInstanceProperties(); // Replicate behaviour in DeployNewInstance - return PopulateInstanceProperties.builder() - .accountSupplier(() -> "test-account") - .regionIdSupplier(() -> "test-region") - .instanceId("test-instance") - .vpcId("test-vpc") - .subnetIds("test-subnet") - .build() - .populate(properties); + properties.set(ACCOUNT, "test-account"); + properties.set(REGION, "test-region"); + properties.set(ID, "test-instance"); + properties.set(VPC_ID, "test-vpc"); + properties.set(SUBNETS, "test-subnet"); + + return properties; } private SleeperInstanceConfiguration buildDeployConfig() { diff --git a/scripts/deploy/syncJars.sh b/scripts/deploy/syncJars.sh index 5b7894f1d22..aea12f8bbdf 100755 --- a/scripts/deploy/syncJars.sh +++ b/scripts/deploy/syncJars.sh @@ -16,8 +16,8 @@ set -e unset CDPATH -if [ "$#" -lt 2 ] || [ "$#" -gt 3 ]; then - echo "Usage: $0 " +if [ "$#" -lt 1 ] || [ "$#" -gt 2 ]; then + echo "Usage: $0 " exit 1 fi diff --git a/scripts/templates/instanceproperties.template b/scripts/templates/instanceproperties.template index 441ac9ac045..f35074c7d95 100644 --- a/scripts/templates/instanceproperties.template +++ b/scripts/templates/instanceproperties.template @@ -27,12 +27,6 @@ sleeper.jars.bucket=changeme # ECR repository names are generated in the format `/`. sleeper.ecr.repository.prefix=changeme -# The AWS account number. This is the AWS account that the instance will be deployed to. -sleeper.account=changeme - -# The AWS region to deploy to. -sleeper.region=changeme - # The id of the VPC to deploy to. sleeper.vpc=changeme