From 56e3cdf106b1083d00d63e578728a3106fd93152 Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Tue, 29 Oct 2024 15:30:17 -0700 Subject: [PATCH] fix: update S3ParquetTestBase testReadWriteUsingProfile (#6310) This fixes an nightly error that #6305 introduced where `S3ParquetLocalStackTest.testReadWriteUsingProfile` was not erroring out with the use of an invalid profile or credentials data (as far as the earlier images were concerned, localstack was failing based on the region and MinIO was failing based on the keys). While we might want to have some sort of test like this, it should be standalone and not tied to the usage of profiles. The invalid keys based test was separated out and only enabled for MinIO. An "invalid" region does not trigger this error for MinIO nor Localstack. Ultimately, the behavior was traced down to a difference in behavior between localstack 3.6 and localstack 3.7; the logic must have changed in some way. We could do a deep dive into the commits to figure out why the behavior changed, or file an issue, if we care to pursue this further. https://github.com/localstack/localstack/releases/tag/v3.7.0 --- .../table/S3ParquetLocalStackTest.java | 14 ++++++++ .../parquet/table/S3ParquetTestBase.java | 36 +++++++++---------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetLocalStackTest.java b/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetLocalStackTest.java index 0b0e8775376..d4c69017d78 100644 --- a/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetLocalStackTest.java +++ b/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetLocalStackTest.java @@ -5,9 +5,13 @@ import io.deephaven.extensions.s3.S3Instructions.Builder; import io.deephaven.extensions.s3.testlib.SingletonContainers.LocalStack; +import org.junit.Assume; import org.junit.BeforeClass; +import org.junit.Ignore; import software.amazon.awssdk.services.s3.S3AsyncClient; +import java.io.IOException; + public class S3ParquetLocalStackTest extends S3ParquetTestBase { @BeforeClass @@ -45,4 +49,14 @@ public String secretAccessKey() { public S3AsyncClient s3AsyncClient() { return LocalStack.s3AsyncClient(); } + + // Ignore annotation does not seem to work with Override + // @Ignore + @Override + public void testInvalidKeysFromProfile() throws IOException { + Assume.assumeTrue( + "localstack 3.7 behavior changed, an 'invalid' key does not seem to cause an error anymore (potentially silently fails?)", + false); + super.testInvalidKeysFromProfile(); + } } diff --git a/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetTestBase.java b/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetTestBase.java index afb1c44e790..7086fad8312 100644 --- a/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetTestBase.java +++ b/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/S3ParquetTestBase.java @@ -518,20 +518,17 @@ public void indexByLongKey() { } @Test - public void testReadWriteUsingProfile() throws IOException { + public void testInvalidKeysFromProfile() throws IOException { final Table table = TableTools.emptyTable(5).update("someIntColumn = (int) i"); - Path tempConfigFile = null; - Path tempCredentialsFile = null; + // Create temporary config and credentials file and write correct credentials and region to them + final Path tempConfigFile = Files.createTempFile("config", ".tmp"); + final Path tempCredentialsFile = Files.createTempFile("credentials", ".tmp"); try { - // Create temporary config and credentials file and write wrong credentials to them - tempConfigFile = Files.createTempFile("config", ".tmp"); - final String configData = "[profile test-user]\nregion = wrong-region"; + final String configData = "[profile test-user]\nregion = " + region(); Files.write(tempConfigFile, configData.getBytes()); - - tempCredentialsFile = Files.createTempFile("credentials", ".tmp"); - final String credentialsData = "[test-user]\naws_access_key_id = foo\naws_secret_access_key = bar"; + final String credentialsData = "[test-user]\naws_access_key_id = badaccesskey" + + "\naws_secret_access_key = badsecretkey"; Files.write(tempCredentialsFile, credentialsData.getBytes()); - final S3Instructions s3Instructions = S3Instructions.builder() .readTimeout(Duration.ofSeconds(3)) .endpointOverride(s3Endpoint()) @@ -551,21 +548,20 @@ public void testReadWriteUsingProfile() throws IOException { } } finally { // Delete the temporary files - if (tempConfigFile != null) { - Files.deleteIfExists(tempConfigFile); - } - if (tempCredentialsFile != null) { - Files.delete(tempCredentialsFile); - } + Files.delete(tempConfigFile); + Files.delete(tempCredentialsFile); } + } + @Test + public void testReadWriteUsingProfile() throws IOException { + final Table table = TableTools.emptyTable(5).update("someIntColumn = (int) i"); + // Create temporary config and credentials file and write correct credentials and region to them + final Path tempConfigFile = Files.createTempFile("config", ".tmp"); + final Path tempCredentialsFile = Files.createTempFile("credentials", ".tmp"); try { - // Create temporary config and credentials file and write correct credentials and region to them - tempConfigFile = Files.createTempFile("config", ".tmp"); final String configData = "[profile test-user]\nregion = " + region(); Files.write(tempConfigFile, configData.getBytes()); - - tempCredentialsFile = Files.createTempFile("credentials", ".tmp"); final String credentialsData = "[test-user]\naws_access_key_id = " + accessKey() + "\naws_secret_access_key = " + secretAccessKey(); Files.write(tempCredentialsFile, credentialsData.getBytes());