JiriOndrusek commented on code in PR #8466: URL: https://github.com/apache/camel-quarkus/pull/8466#discussion_r3000572405
########## integration-test-groups/aws2/aws2-msk/README.adoc: ########## @@ -0,0 +1,38 @@ += AWS MSK tests + +By default the tests run in WireMock playback mode using pre-recorded mappings from `src/test/resources/mappings/`. Review Comment: Mention why the wiremock (and not `camel-quarkus-integration-tests-support-aws2`) is used and that it is also reason to not use Aws2TestEnvCustomizer ########## integration-test-groups/aws2/aws2-msk/README.adoc: ########## @@ -0,0 +1,38 @@ += AWS MSK tests + +By default the tests run in WireMock playback mode using pre-recorded mappings from `src/test/resources/mappings/`. + +== Running against real AWS + +Refer to the xref:../README.adoc[AWS 2 integration tests README] for general instructions on how to set up AWS credentials. + +The AWS credentials must have the following IAM permissions: + +* `kafka:ListClusters` +* `kafka:CreateCluster` +* `kafka:DescribeCluster` +* `kafka:DeleteCluster` + +=== Prerequisites for real AWS environment + +The following environment variables are required when running against a real AWS account: + +[source,shell] +---- +# Provide at least 2 subnet IDs from different Availability Zones in your VPC +export AWS_MSK_CLIENT_SUBNETS=<subnet-id-1>,<subnet-id-2> +# Provide a currently supported MSK Kafka version +export AWS_MSK_KAFKA_VERSION=3.6.0 +---- + +NOTE: The number of broker nodes is automatically set to match the number of subnets provided. + +=== Running tests directly against real AWS + +[source,shell] +---- +export AWS_ACCESS_KEY=<your-access-key-id> Review Comment: This is correct, but it might be better to add those properties also to application.properties (similar to [this](https://github.com/apache/camel-quarkus/blob/main/integration-test-groups/aws2/aws2-s3/src/main/resources/application.properties#L20-L23)) * application.properties are the first place where to look for any properties and default values) * it will be consistent with other aws2 test modules. ########## integration-test-groups/aws2/aws2-msk/src/test/java/org/apache/camel/quarkus/component/aws2/msk/it/GroupedAws2MskTestResource.java: ########## @@ -14,18 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.camel.quarkus.component.support.langchain4j.deployment; +package org.apache.camel.quarkus.component.aws2.msk.it; -import java.util.function.BooleanSupplier; - -public class QuarkusLangchain4jPresent implements BooleanSupplier { - @Override - public boolean getAsBoolean() { - try { - Thread.currentThread().getContextClassLoader().loadClass("io.quarkiverse.langchain4j.RegisterAiService"); - return true; - } catch (ClassNotFoundException e) { - return false; - } - } +public class GroupedAws2MskTestResource extends Aws2MskTestResource { Review Comment: Please add comment why this class is required (It is not referenced from any other classes, but seems to be required for aws2-grouped module) ########## integration-test-groups/aws2/aws2-msk/README.adoc: ########## @@ -0,0 +1,38 @@ += AWS MSK tests + +By default the tests run in WireMock playback mode using pre-recorded mappings from `src/test/resources/mappings/`. + +== Running against real AWS + +Refer to the xref:../README.adoc[AWS 2 integration tests README] for general instructions on how to set up AWS credentials. + +The AWS credentials must have the following IAM permissions: + +* `kafka:ListClusters` +* `kafka:CreateCluster` +* `kafka:DescribeCluster` +* `kafka:DeleteCluster` + +=== Prerequisites for real AWS environment + +The following environment variables are required when running against a real AWS account: + +[source,shell] +---- +# Provide at least 2 subnet IDs from different Availability Zones in your VPC +export AWS_MSK_CLIENT_SUBNETS=<subnet-id-1>,<subnet-id-2> Review Comment: same as above ########## integration-test-groups/aws2/aws2-msk/src/main/java/org/apache/camel/quarkus/component/aws2/msk/it/Aws2MskResource.java: ########## @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.camel.quarkus.component.aws2.msk.it; + +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.stream.Collectors; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.QueryParam; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import org.apache.camel.ProducerTemplate; +import org.apache.camel.component.aws2.msk.MSK2Constants; +import org.apache.camel.component.aws2.msk.MSK2Operations; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import software.amazon.awssdk.services.kafka.model.BrokerNodeGroupInfo; +import software.amazon.awssdk.services.kafka.model.ClusterInfo; +import software.amazon.awssdk.services.kafka.model.CreateClusterRequest; +import software.amazon.awssdk.services.kafka.model.CreateClusterResponse; +import software.amazon.awssdk.services.kafka.model.DeleteClusterRequest; +import software.amazon.awssdk.services.kafka.model.DescribeClusterRequest; +import software.amazon.awssdk.services.kafka.model.DescribeClusterResponse; +import software.amazon.awssdk.services.kafka.model.ListClustersResponse; + +import static org.apache.camel.component.aws2.msk.MSK2Operations.*; + +@Path("/aws2-msk") +@ApplicationScoped +public class Aws2MskResource { + + @Inject + ProducerTemplate producerTemplate; + + @ConfigProperty(name = "aws.msk.client.subnets", defaultValue = "subnet-1,subnet-2") + List<String> clientSubnets; + + @ConfigProperty(name = "aws.msk.kafka.version", defaultValue = "3.6.0") + String kafkaVersion; + + @Path("/clusters") + @GET + @Produces(MediaType.APPLICATION_JSON) + public List<String> listClusters(@QueryParam("filter") String filter) { + LinkedHashMap<String, Object> headers = new LinkedHashMap<>(); + if (filter != null && !filter.isEmpty()) { + headers.put(MSK2Constants.CLUSTERS_FILTER, filter); + } + return producerTemplate.requestBodyAndHeaders( + componentUri(listClusters), + null, + headers, + ListClustersResponse.class) + .clusterInfoList().stream() + .map(ClusterInfo::clusterName) + .collect(Collectors.toList()); + } + + @Path("/cluster/{clusterName}") + @POST + @Produces(MediaType.TEXT_PLAIN) + public Response createCluster(@PathParam("clusterName") String clusterName) throws Exception { + BrokerNodeGroupInfo brokerNodeGroupInfo = BrokerNodeGroupInfo.builder() + .clientSubnets(clientSubnets) + .instanceType("kafka.t3.small") + .build(); + + CreateClusterResponse response = producerTemplate.requestBodyAndHeaders( + componentUri(createCluster), + null, + new LinkedHashMap<>() { Review Comment: We are using `Map.of` instead of `{{}}` in CQ (Map.of is cleaner, safer, and more efficient.) ########## integration-test-groups/aws2/aws2-msk/src/main/java/org/apache/camel/quarkus/component/aws2/msk/it/Aws2MskResource.java: ########## @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.camel.quarkus.component.aws2.msk.it; + +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.stream.Collectors; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import jakarta.ws.rs.DELETE; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.QueryParam; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import org.apache.camel.ProducerTemplate; +import org.apache.camel.component.aws2.msk.MSK2Constants; +import org.apache.camel.component.aws2.msk.MSK2Operations; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import software.amazon.awssdk.services.kafka.model.BrokerNodeGroupInfo; +import software.amazon.awssdk.services.kafka.model.ClusterInfo; +import software.amazon.awssdk.services.kafka.model.CreateClusterRequest; +import software.amazon.awssdk.services.kafka.model.CreateClusterResponse; +import software.amazon.awssdk.services.kafka.model.DeleteClusterRequest; +import software.amazon.awssdk.services.kafka.model.DescribeClusterRequest; +import software.amazon.awssdk.services.kafka.model.DescribeClusterResponse; +import software.amazon.awssdk.services.kafka.model.ListClustersResponse; + +import static org.apache.camel.component.aws2.msk.MSK2Operations.*; + +@Path("/aws2-msk") +@ApplicationScoped +public class Aws2MskResource { + + @Inject + ProducerTemplate producerTemplate; + + @ConfigProperty(name = "aws.msk.client.subnets", defaultValue = "subnet-1,subnet-2") Review Comment: There is no need to define default values here (when the values will be defined in application.properties) ########## integration-test-groups/aws2/aws2-msk/src/test/java/org/apache/camel/quarkus/component/aws2/msk/it/Aws2MskTest.java: ########## @@ -0,0 +1,138 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.camel.quarkus.component.aws2.msk.it; + +import java.util.concurrent.TimeUnit; + +import io.quarkus.test.common.QuarkusTestResource; +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.RestAssured; +import org.awaitility.Awaitility; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; + +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.notNullValue; + +@QuarkusTest +@QuarkusTestResource(Aws2MskTestResource.class) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +class Aws2MskTest { + + static final String CLUSTER_NAME = "cq-test-cluster"; + static final String CLUSTER_POJO_NAME = "cq-pojo-test-cluster"; + + static String clusterArn; + static String clusterPojoArn; + + @Test + @Order(1) + public void testCreateClusters() { + clusterArn = RestAssured.given() + .post("/aws2-msk/cluster/" + CLUSTER_NAME) + .then() + .statusCode(201) + .extract().body().asString(); + + clusterPojoArn = RestAssured.given() + .post("/aws2-msk/cluster/" + CLUSTER_POJO_NAME + "/pojo") + .then() + .statusCode(201) + .extract().body().asString(); + + // Wait for both clusters to become ACTIVE before running further tests + Awaitility.await() + .atMost(30, TimeUnit.MINUTES) + .pollDelay(0, TimeUnit.SECONDS) + .pollInterval(1, TimeUnit.MINUTES) + .until(() -> "ACTIVE".equals(clusterState(clusterArn)) + && "ACTIVE".equals(clusterState(clusterPojoArn))); + } + + @Test + @Order(2) + public void testClusterOperations() { + // List all clusters + RestAssured.given() + .get("/aws2-msk/clusters") + .then() + .statusCode(200) + .body("$", hasItem(CLUSTER_NAME)); + + // List cluster with filter + RestAssured.given() + .queryParam("filter", CLUSTER_NAME) + .get("/aws2-msk/clusters") + .then() + .statusCode(200) + .body("$", hasItem(CLUSTER_NAME)); + + // Describe cluster + RestAssured.given() + .queryParam("clusterArn", clusterArn) + .get("/aws2-msk/cluster/describe") + .then() + .statusCode(200) + .body(notNullValue()); Review Comment: Please assert actual value (not just notNull response), or better just comment that the describeCluster is used in clusterState() and remove the whole rest call -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
