gnodet-bot commented on code in PR #27429: URL: https://github.com/apache/camel/pull/27429#discussion_r4193403306
########## components/camel-spiffe/src/test/java/org/apache/camel/component/spiffe/integration/SpiffeWorkloadApiIT.java: ########## @@ -0,0 +1,108 @@ +/* + * 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.component.spiffe.integration; + +import io.spiffe.exception.JwtSvidException; +import org.apache.camel.Exchange; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.component.spiffe.SpiffeConstants; +import org.apache.camel.test.infra.spiffe.services.SpiffeService; +import org.apache.camel.test.infra.spiffe.services.SpiffeServiceFactory; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Exercises the camel-spiffe producer against a real SPIRE Workload API: fetching an X509-SVID and a JWT-SVID, and + * validating a JWT-SVID (accepting the configured audience, rejecting another). + */ +class SpiffeWorkloadApiIT extends CamelTestSupport { + + @RegisterExtension + static SpiffeService service = SpiffeServiceFactory.createSingletonService(); + + private static String socket() { + return "RAW(" + service.getWorkloadApiSocketPath() + ")"; + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + from("direct:x509") + .toF("spiffe:x509?operation=fetchX509Svid&spiffeSocketPath=%s", socket()); + from("direct:jwt") + .toF("spiffe:jwt?operation=fetchJwtSvid&audience=my-audience&spiffeSocketPath=%s", socket()); + from("direct:validateCorrectAudience") + .toF("spiffe:valOk?operation=validateJwtSvid&audience=my-audience&spiffeSocketPath=%s", socket()); + from("direct:validateWrongAudience") + .toF("spiffe:valWrong?operation=validateJwtSvid&audience=other-audience&spiffeSocketPath=%s", + socket()); + } + }; + } + + @Test + void fetchesAnX509SvidForThisWorkload() { + Exchange out = template.request("direct:x509", e -> { + }); + + assertThat(out.getException()).isNull(); + assertThat(out.getMessage().getHeader(SpiffeConstants.SPIFFE_ID)) + .isEqualTo(service.getWorkloadSpiffeId()); + // the default x509Response is the certificate chain, which must carry at least the leaf + assertThat(out.getMessage().getBody()).isNotNull(); Review Comment: 💡 **Still not addressed from previous review.** `isNotNull()` only proves the body field was set — it doesn't verify the response is actually a certificate chain. A slightly stronger check: ```suggestion assertThat(out.getMessage().getBody(String.class)).as("X509-SVID response").isNotEmpty(); ``` Or, if the body is an `X509Certificate[]`, assert `hasSize` ≥ 1. ########## test-infra/camel-test-infra-spiffe/src/main/java/org/apache/camel/test/infra/spiffe/services/SpiffeLocalContainerInfraService.java: ########## @@ -0,0 +1,270 @@ +/* + * 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.test.infra.spiffe.services; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; + +import com.sun.security.auth.module.UnixSystem; +import org.apache.camel.spi.annotations.InfraService; +import org.apache.camel.test.infra.common.LocalPropertyResolver; +import org.apache.camel.test.infra.common.services.ContainerService; +import org.apache.camel.test.infra.spiffe.common.SpiffeProperties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.BindMode; +import org.testcontainers.containers.Container; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.SelinuxContext; +import org.testcontainers.containers.startupcheck.IsRunningStartupCheckStrategy; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.utility.MountableFile; + +/** + * Runs a SPIRE server and agent so a test can fetch SVIDs from a real Workload API. + * <p> + * The agent's Workload API Unix socket is bind-mounted to a short host path (the {@code AF_UNIX} {@code sun_path} limit + * is ~108 bytes, so a socket under a deep temp tree would fail to connect from the host). The agent runs in the host + * PID namespace because the {@code unix} workload attestor resolves the calling process through {@code /proc}, which a + * container cannot see otherwise; a single registration entry is created for {@code unix:uid:<the test process uid>} so + * the test JVM is issued {@code spiffe://<trustDomain>/workload}. + */ +@InfraService(service = SpiffeInfraService.class, + description = "SPIFFE/SPIRE server and agent exposing the Workload API", + serviceAlias = { "spiffe" }) +public class SpiffeLocalContainerInfraService implements SpiffeInfraService, ContainerService<GenericContainer<?>> { + + public static final String TRUST_DOMAIN = "example.org"; + public static final String AGENT_SPIFFE_ID = "spiffe://" + TRUST_DOMAIN + "/agent"; + public static final String WORKLOAD_SPIFFE_ID = "spiffe://" + TRUST_DOMAIN + "/workload"; + + private static final Logger LOG = LoggerFactory.getLogger(SpiffeLocalContainerInfraService.class); + + private static final String SERVER_ALIAS = "spire-server"; + private static final int SERVER_PORT = 8081; + private static final String SERVER_BIN = "/opt/spire/bin/spire-server"; + private static final String SERVER_CONF = "/opt/spire/conf/server/server.conf"; + private static final String AGENT_CONF = "/opt/spire/conf/agent/agent.conf"; + private static final String SOCKET_CONTAINER_DIR = "/tmp/spire-agent/public"; + private static final String SOCKET_FILE = "api.sock"; + private static final String SERVER_CONF_RESOURCE = "org/apache/camel/test/infra/spiffe/services/spire-server.conf"; + private static final String AGENT_CONF_RESOURCE = "org/apache/camel/test/infra/spiffe/services/spire-agent.conf"; + + private final String serverImage; + private final String agentImage; + + private Network network; + private GenericContainer<?> server; + private GenericContainer<?> agent; + private Path hostSocketDir; + private String socketPath; + + public SpiffeLocalContainerInfraService() { + this(LocalPropertyResolver.getProperty(SpiffeLocalContainerInfraService.class, + SpiffeProperties.SPIFFE_SERVER_CONTAINER), + LocalPropertyResolver.getProperty(SpiffeLocalContainerInfraService.class, + SpiffeProperties.SPIFFE_AGENT_CONTAINER)); + } + + public SpiffeLocalContainerInfraService(String serverImage, String agentImage) { + this.serverImage = serverImage; + this.agentImage = agentImage; + } + + @Override + public void initialize() { + createHostSocketDir(); + network = Network.newNetwork(); + + // the image entrypoint is "spire-server run", so only -config is passed; the config is copied in (no bind mount + // needed for it) and port 8081 is exposed purely so the listening-port wait can tell when the API is up + server = new GenericContainer<>(serverImage) + .withNetwork(network) + .withNetworkAliases(SERVER_ALIAS) + .withCopyFileToContainer(MountableFile.forClasspathResource(SERVER_CONF_RESOURCE), SERVER_CONF) + .withExposedPorts(SERVER_PORT) + .withCommand("-config", SERVER_CONF) + .waitingFor(Wait.forListeningPort()); + LOG.info("Starting the SPIRE server container"); + server.start(); + waitForServerHealthy(); + + String joinToken = generateJoinToken(); + createWorkloadEntry(); + + // the agent needs the host PID namespace so the unix workload attestor can resolve the host test process, and + // the socket directory bind-mounted (SELinux-shared) so the host can reach the Workload API socket it creates + agent = new GenericContainer<>(agentImage) + .withNetwork(network) + .withCopyFileToContainer(MountableFile.forClasspathResource(AGENT_CONF_RESOURCE), AGENT_CONF) + .withCreateContainerCmdModifier(cmd -> cmd.getHostConfig().withPidMode("host")) + .withStartupCheckStrategy(new IsRunningStartupCheckStrategy()) + .withCommand("-config", AGENT_CONF, "-joinToken", joinToken); + agent.addFileSystemBind(hostSocketDir.toString(), SOCKET_CONTAINER_DIR, BindMode.READ_WRITE, SelinuxContext.SHARED); + LOG.info("Starting the SPIRE agent container"); + agent.start(); + waitForSocket(); + + socketPath = "unix://" + hostSocketDir.resolve(SOCKET_FILE); + registerProperties(); + LOG.info("SPIRE Workload API available at {}", socketPath); Review Comment: ⚠️ **Still not addressed from previous review.** If `generateJoinToken()`, `createWorkloadEntry()`, or `agent.start()` throws after the server container is already started, the server, the Docker `Network`, and the `hostSocketDir` are left allocated. `TestServiceUtil.tryInitialize()` will call `initialize()` again, overwriting the field references and orphaning the previous resources. Wrap the body after `createHostSocketDir()` in a try/catch: ```suggestion public void initialize() { createHostSocketDir(); try { network = Network.newNetwork(); // the image entrypoint is "spire-server run", so only -config is passed; the config is copied in (no bind mount // needed for it) and port 8081 is exposed purely so the listening-port wait can tell when the API is up server = new GenericContainer<>(serverImage) .withNetwork(network) .withNetworkAliases(SERVER_ALIAS) .withCopyFileToContainer(MountableFile.forClasspathResource(SERVER_CONF_RESOURCE), SERVER_CONF) .withExposedPorts(SERVER_PORT) .withCommand("-config", SERVER_CONF) .waitingFor(Wait.forListeningPort()); LOG.info("Starting the SPIRE server container"); server.start(); waitForServerHealthy(); String joinToken = generateJoinToken(); createWorkloadEntry(); // the agent needs the host PID namespace so the unix workload attestor can resolve the host test process, and // the socket directory bind-mounted (SELinux-shared) so the host can reach the Workload API socket it creates agent = new GenericContainer<>(agentImage) .withNetwork(network) .withCopyFileToContainer(MountableFile.forClasspathResource(AGENT_CONF_RESOURCE), AGENT_CONF) .withCreateContainerCmdModifier(cmd -> cmd.getHostConfig().withPidMode("host")) .withStartupCheckStrategy(new IsRunningStartupCheckStrategy()) .withCommand("-config", AGENT_CONF, "-joinToken", joinToken); agent.addFileSystemBind(hostSocketDir.toString(), SOCKET_CONTAINER_DIR, BindMode.READ_WRITE, SelinuxContext.SHARED); LOG.info("Starting the SPIRE agent container"); agent.start(); waitForSocket(); socketPath = "unix://" + hostSocketDir.resolve(SOCKET_FILE); registerProperties(); LOG.info("SPIRE Workload API available at {}", socketPath); } catch (RuntimeException e) { shutdown(); throw e; } } ``` -- 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]
