diff --git a/build.sbt b/build.sbt index 59c287f8f..40f3cfc3a 100644 --- a/build.sbt +++ b/build.sbt @@ -305,7 +305,12 @@ def testkitProject(esVersion: String, ss: Def.SettingsDefinition*): Project = { "ch.qos.logback" % "logback-classic" % Versions.logback, "app.softnetwork.persistence" %% "persistence-core-testkit" % Versions.genericPersistence, "org.testcontainers" % "testcontainers-elasticsearch" % Versions.testContainers excludeAll (jacksonExclusions: _*), - "org.testcontainers" % "testcontainers-minio" % Versions.testContainers, + // MinioTestKit drives `adobe/s3mock` through a plain GenericContainer: `minio/minio` was + // removed from Docker Hub (so `testcontainers-minio` has no image to pull), and the s3mock + // Testcontainers MODULE is Java-17 bytecode built against Testcontainers 1.x. Declared + // explicitly even though testcontainers-elasticsearch already brings it in transitively. + // (Same line exists in testkit/build.sbt — keep them in step.) + "org.testcontainers" % "testcontainers" % Versions.testContainers, // Required at test runtime for COPY INTO ... FROM 's3a://...' tests via MinioTestKit // "org.apache.hadoop" % "hadoop-aws" % Versions.hadoop % Test excludeAll (excludeSlf4jAndLog4j: _*) ), diff --git a/core/src/test/scala/app/softnetwork/elastic/client/file/LocalPathSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/file/LocalPathSpec.scala index 51fade8ca..c9381be7f 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/file/LocalPathSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/file/LocalPathSpec.scala @@ -344,6 +344,72 @@ class LocalPathSpec extends AnyWordSpec with Matchers with OptionValues { } } + // --------------------------------------------------------------------------------------------- + // The AWS_* SYSTEM-PROPERTY contract, which nothing else in this repository asserts. + // + // `MinioTestKit` sets AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY / AWS_ENDPOINT_URL as JVM system + // properties precisely so that the PRODUCTION s3aConf() picks them up through envOrProp(). Under + // the old MinIO container the COPY INTO integration tests proved that end to end: MinIO answers + // 403 to a wrong key. The replacement (adobe/s3mock) does NOT verify credentials at ALL - a wrong + // key AND secret still succeed - so those integration tests can no longer detect a broken + // propagation. This is where that lost protection lives now. + // + // The env var, when set, WINS over the system property by design (`sys.env.get(...).orElse(...)`), + // so a shell exporting AWS_ACCESS_KEY_ID turns these into a measurement of that shell. The clue + // says so rather than an `assume`, which would cancel silently and leave the gate green-by-absence. + // --------------------------------------------------------------------------------------------- + "HadoopConfigurationFactory.s3aConf" should { + + val awsProperties = Seq("AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_ENDPOINT_URL") + val accessKey = "test-access-key" + val secretKey = "test-secret-key" + val endpoint = "http://localhost:19090" + + def withAwsProperties(test: => Unit): Unit = { + System.setProperty("AWS_ACCESS_KEY_ID", accessKey) + System.setProperty("AWS_SECRET_ACCESS_KEY", secretKey) + System.setProperty("AWS_ENDPOINT_URL", endpoint) + try test + finally awsProperties.foreach(System.clearProperty) + } + + // `core` tests share one JVM (Test / fork is false unless -Dtest.jdk.home is set), and + // `Test / parallelExecution := false` is per-project - so a system property another module's + // tests forgot to clear would silently reach the no-credentials case below. Clearing on the way + // IN as well as out makes that case deterministic without hiding a real env var, which + // envOrProp still prefers over any property. + def withoutAwsProperties(test: => Unit): Unit = { + awsProperties.foreach(System.clearProperty) + try test + finally awsProperties.foreach(System.clearProperty) + } + + "read credentials and endpoint from JVM system properties (MinioTestKit's contract)" in { + withAwsProperties { + val conf = HadoopConfigurationFactory.forPath("s3a://bucket/customers.jsonl") + withClue( + "AWS_* env vars take precedence over the system properties this test sets - " + + "unset them in the shell running sbt: " + ) { + conf.get("fs.s3a.access.key") shouldBe accessKey + conf.get("fs.s3a.secret.key") shouldBe secretKey + conf.get("fs.s3a.endpoint") shouldBe endpoint + } + // An S3-compatible endpoint is only reachable path-style; the vhost form would resolve + // `bucket.localhost`. s3aConf sets this together with the endpoint - assert the pair. + conf.getBoolean("fs.s3a.path.style.access", false) shouldBe true + } + } + + "set no static key when none is available" in { + withoutAwsProperties { + val conf = HadoopConfigurationFactory.forPath("s3a://bucket/customers.jsonl") + Option(conf.get("fs.s3a.access.key")) shouldBe None + Option(conf.get("fs.s3a.secret.key")) shouldBe None + } + } + } + "LocalPath.scheme" should { "lowercase, and refuse a one-character prefix" in { LocalPath.scheme("S3A://b/x") shouldBe Some("s3a") diff --git a/testkit/build.sbt b/testkit/build.sbt index c15604fdc..08ef50b19 100644 --- a/testkit/build.sbt +++ b/testkit/build.sbt @@ -15,5 +15,9 @@ libraryDependencies ++= elasticClientDependencies(elasticSearchVersion.value) ++ // "org.apache.logging.log4j" % "log4j-slf4j-impl" % log4jVersion(elasticSearchVersion.value), "app.softnetwork.persistence" %% "persistence-core-testkit" % Versions.genericPersistence, "org.testcontainers" % "testcontainers-elasticsearch" % Versions.testContainers excludeAll (jacksonExclusions: _*), - "org.testcontainers" % "testcontainers-minio" % Versions.testContainers + // MinioTestKit drives `adobe/s3mock` through a plain GenericContainer: `minio/minio` was removed + // from Docker Hub (so `testcontainers-minio` has no image to pull), and the s3mock Testcontainers + // MODULE is Java-17 bytecode built against Testcontainers 1.x. Declared explicitly even though + // testcontainers-elasticsearch already brings it in transitively. + "org.testcontainers" % "testcontainers" % Versions.testContainers ) diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/CopyIntoS3IntegrationSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/CopyIntoS3IntegrationSpec.scala index 5dc2464d3..812543c0c 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/CopyIntoS3IntegrationSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/CopyIntoS3IntegrationSpec.scala @@ -21,7 +21,7 @@ import app.softnetwork.elastic.scalatest.{ElasticTestKit, MinioTestKit} import java.time.LocalDate -/** Integration tests for `COPY INTO ... FROM 's3a://...'` backed by a MinIO container. +/** Integration tests for `COPY INTO ... FROM 's3a://...'` backed by an S3 container. * * Mix this trait into a concrete test class together with * [[app.softnetwork.elastic.scalatest.ElasticDockerTestKit]] and [[MinioTestKit]]: @@ -42,7 +42,7 @@ trait CopyIntoS3IntegrationSpec extends GatewayIntegrationTestKit { self: ElasticTestKit with MinioTestKit => // --------------------------------------------------------------------------- - // COPY INTO from S3 (MinIO) integration tests + // COPY INTO from S3 integration tests // --------------------------------------------------------------------------- behavior of "COPY INTO from S3" diff --git a/testkit/src/main/scala/app/softnetwork/elastic/scalatest/MinioTestKit.scala b/testkit/src/main/scala/app/softnetwork/elastic/scalatest/MinioTestKit.scala index 5a2eaf2c0..8d9716c24 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/scalatest/MinioTestKit.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/scalatest/MinioTestKit.scala @@ -16,16 +16,68 @@ package app.softnetwork.elastic.scalatest +import app.softnetwork.elastic.client.file.HadoopConfigurationFactory import org.apache.hadoop.conf.Configuration import org.apache.hadoop.fs.{FileSystem, Path} import org.scalatest.Suite -import org.testcontainers.containers.MinIOContainer +import org.testcontainers.containers.GenericContainer +import org.testcontainers.containers.wait.strategy.Wait +import org.testcontainers.utility.DockerImageName -/** A test kit trait that starts a MinIO container and provides helper methods for uploading files - * to MinIO via the Hadoop S3A filesystem. +import java.time.Duration +import scala.util.control.NonFatal + +object MinioTestKit { + + /** The S3-compatible server the COPY INTO tests run against. + * + * It is NOT MinIO any more: `minio/minio` was REMOVED from Docker Hub, so every + * `*CopyIntoS3Spec` aborted at container start with `ContainerFetchException ... pull access + * denied for minio/minio`. `adobe/s3mock` is the replacement; the trait keeps its historical + * name so the four concrete specs and `CopyIntoS3IntegrationSpec` are untouched (renaming it to + * `S3TestKit` is a follow-up). + */ + val S3MockImage: String = "adobe/s3mock:5.2.2" + + /** Port s3mock serves HTTP on. + * + * The image declares NO `EXPOSE` (it is a buildpacks image: `Config.ExposedPorts` is null), so + * `withExposedPorts` is REQUIRED — Testcontainers has nothing to infer a mapping from. + */ + val S3MockPort: Int = 9090 + + /** Env var s3mock reads the comma-separated list of buckets to pre-create from. + * + * Adobe RENAMED this property between the 4.x and 5.x lines. Passing the OLD name is SILENT: the + * container starts, answers `200` on `/`, and `ListBuckets` returns an empty list — every upload + * then fails deep inside S3A with a confusing 404. That is why `start()` asserts the bucket + * really exists and names this constant when it does not. + */ + val InitialBucketsEnv: String = "COM_ADOBE_TESTING_S3MOCK_STORE_INITIAL_BUCKETS" + + /** s3mock does not verify credentials at all — any non-empty key/secret pair is accepted. These + * exist only because S3A (and `HadoopConfigurationFactory.s3aConf()`) require a non-empty pair + * to select the static-credentials provider instead of the default AWS chain. + */ + val AccessKey: String = "s3mock-access-key" + val SecretKey: String = "s3mock-secret-key" + + /** A `GenericContainer` subclass rather than a raw `new GenericContainer(...)`: + * `GenericContainer` is F-bounded (`SELF extends GenericContainer`), which Scala cannot + * express inline. + * + * The `s3mock-testcontainers` MODULE is deliberately NOT used: 5.2.2 is Java-17 bytecode (class + * file major 61) built against Testcontainers 1.21.3, while this build's default JDK is 11 and + * it pins Testcontainers 2.x. + */ + final class S3MockContainer(image: DockerImageName) + extends GenericContainer[S3MockContainer](image) +} + +/** A test kit trait that starts an S3-compatible container (`adobe/s3mock`) and provides helper + * methods for uploading files to it via the Hadoop S3A filesystem. * - * Uses the official Testcontainers MinIO module (`testcontainers-minio`). Mix this trait into a - * concrete test class together with [[ElasticDockerTestKit]]. + * Mix this trait into a concrete test class together with [[ElasticDockerTestKit]]. * * The concrete module must declare `hadoop-aws` as a `% Test` dependency so that the * `S3AFileSystem` implementation is available on the test classpath at runtime. @@ -42,16 +94,24 @@ import org.testcontainers.containers.MinIOContainer */ trait MinioTestKit extends ElasticTestKit { _: Suite => + import MinioTestKit._ + lazy val minioBucket: String = "copy-into-test" - lazy val minioContainer: MinIOContainer = - new MinIOContainer("minio/minio:latest") + lazy val minioContainer: S3MockContainer = + new S3MockContainer(DockerImageName.parse(S3MockImage)) + .withExposedPorts(Integer.valueOf(S3MockPort)) + .withEnv(InitialBucketsEnv, minioBucket) + .waitingFor(Wait.forHttp("/").forStatusCode(200)) + .withStartupTimeout(Duration.ofMinutes(2)) - def minioEndpoint: String = minioContainer.getS3URL - def minioAccessKey: String = minioContainer.getUserName - def minioSecretKey: String = minioContainer.getPassword + def minioEndpoint: String = + s"http://${minioContainer.getHost}:${minioContainer.getMappedPort(S3MockPort)}" - /** Returns a Hadoop [[Configuration]] pointing at the in-process MinIO container. + def minioAccessKey: String = AccessKey + def minioSecretKey: String = SecretKey + + /** Returns a Hadoop [[Configuration]] pointing at the in-process S3 container. * * The S3AFileSystem implementation (`hadoop-aws`) must be on the classpath at runtime. */ @@ -85,39 +145,99 @@ trait MinioTestKit extends ElasticTestKit { _: Suite => } } - /** Starts the MinIO container, creates the test bucket, and then starts Elasticsearch. */ + /** Fails loudly if [[minioBucket]] was not pre-created by the container. + * + * Bucket seeding is SILENTLY fallible (see [[MinioTestKit.InitialBucketsEnv]]): a wrong env-var + * name leaves a perfectly healthy container serving an empty bucket list, and the first symptom + * would be a failure deep inside an upload, with a message naming neither the bucket nor the + * container. + * + * It deliberately builds its [[Configuration]] from the PRODUCTION + * `HadoopConfigurationFactory.forPath`, not from [[minioHadoopConf]]: that is the only thing in + * the suite that exercises the AWS_* system properties `start()` has just set. With + * `minioHadoopConf` (explicit credentials) a typo in one of those three property names would go + * unnoticed - s3mock accepts any credentials, so no COPY INTO assertion can see it either. What + * the properties must then produce is asserted as a unit test in core's `LocalPathSpec`. + */ + private def assertBucketSeeded(): Unit = { + val conf = HadoopConfigurationFactory.forPath(s"s3a://$minioBucket/") + // `fs.s3a.bucket.probe = 2` makes initialize() HEAD the bucket and fail with + // UnknownStoreException when it is absent. It is NOT redundant with the listStatus below, and + // neither is redundant with the obvious `fs.exists(s3a:///)` - that one is VACUOUS: + // MEASURED against this very container with a wrong seeding env var, `exists` on the ROOT path + // returned true (S3A answers it from a synthetic directory status without contacting the + // store) and the run failed much later, inside an upload, with an unrelated-looking message. + conf.setInt("fs.s3a.bucket.probe", 2) + val bucketPath = new Path(s"s3a://$minioBucket/") + + var fs: FileSystem = null + try { + fs = FileSystem.get(bucketPath.toUri, conf) + fs.listStatus(bucketPath) + () + } catch { + case NonFatal(e) => + // Only an absent STORE points at the seeding env var. A missing `hadoop-aws` on the test + // classpath, or Docker networking, must not be reported as "check that env var name". + val storeAbsent = + e.getClass.getSimpleName == "UnknownStoreException" || + e.isInstanceOf[java.io.FileNotFoundException] + val diagnosis = + if (storeAbsent) + s"S3 bucket '$minioBucket' does not exist on $S3MockImage at $minioEndpoint. " + + s"The container seeds buckets from the env var '$InitialBucketsEnv' - check that name: " + + "an unknown one is ignored silently and leaves the container healthy but empty." + else + s"Could not reach the S3 bucket '$minioBucket' on $S3MockImage at $minioEndpoint. " + + "Check that this module declares hadoop-aws as a Test dependency (the S3AFileSystem " + + "class is loaded by name) and that Docker networking to the mapped port works." + throw new IllegalStateException(diagnosis, e) + } finally { + // Outside the diagnosed region on purpose: a throw from close() here would REPLACE the real + // "bucket absent" exception with a teardown failure. + if (fs != null) fs.close() + } + } + + /** Starts the S3 container, verifies the test bucket exists, and then starts Elasticsearch. */ abstract override def start(): Unit = { minioContainer.start() - // Expose MinIO credentials as JVM system properties so that + // Expose the S3 credentials as JVM system properties so that // HadoopConfigurationFactory.s3aConf() picks them up via envOrProp(). + // (core's LocalPathSpec, under the subject "HadoopConfigurationFactory.s3aConf", is what + // guards that contract as a unit test — s3mock itself accepts ANY credentials, so a COPY INTO + // test passing here proves nothing about it.) System.setProperty("AWS_ACCESS_KEY_ID", minioAccessKey) System.setProperty("AWS_SECRET_ACCESS_KEY", minioSecretKey) System.setProperty("AWS_ENDPOINT_URL", minioEndpoint) - // Create the test bucket using the mc CLI bundled in the minio/minio image. - // mc connects to localhost:9000 (the internal port) from within the container. - minioContainer.execInContainer( - "mc", - "alias", - "set", - "myminio", - "http://localhost:9000", - minioContainer.getUserName, - minioContainer.getPassword - ) - minioContainer.execInContainer("mc", "mb", "--ignore-existing", s"myminio/$minioBucket") + // Defensive, and the reason is NOT "afterAll does not run": ScalaTest 3.2.19 DOES run it after + // a failed beforeAll. It is that `ElasticTestKit.afterAll` closes `restClient` BEFORE calling + // `stop()`, and with Elasticsearch never started that throws first and ScalaTest swallows it - + // so `stop()` is never reached and Ryuk would reap this container minutes later. + try assertBucketSeeded() + catch { + case NonFatal(e) => + clearS3SystemProperties() + minioContainer.stop() + throw e + } super.start() } - /** Stops Elasticsearch first, then tears down the MinIO container and clears system properties. + /** Stops Elasticsearch first, then tears down the S3 container and clears system properties. */ abstract override def stop(): Unit = { super.stop() + clearS3SystemProperties() + if (minioContainer.isRunning) minioContainer.stop() + } + + private def clearS3SystemProperties(): Unit = { System.clearProperty("AWS_ACCESS_KEY_ID") System.clearProperty("AWS_SECRET_ACCESS_KEY") System.clearProperty("AWS_ENDPOINT_URL") - if (minioContainer.isRunning) minioContainer.stop() } }