gnodet-bot commented on code in PR #29829:
URL: https://github.com/apache/camel/pull/29829#discussion_r4562382994
##########
test-infra/camel-test-infra-spiffe/src/main/java/org/apache/camel/test/infra/spiffe/services/SpiffeLocalContainerInfraService.java:
##########
@@ -155,6 +178,51 @@ private void doInitialize() {
LOG.info("SPIRE Workload API available at {}", socketPath);
}
+ String agentConfiguration() {
+ try (var input =
getClass().getClassLoader().getResourceAsStream(AGENT_CONF_RESOURCE)) {
+ if (input == null) {
+ throw new IllegalStateException("Missing SPIRE agent
configuration");
+ }
+ // Restarting a process must retain its node key, rather than try
to reuse a consumed join token.
+ String configuration = new String(input.readAllBytes(),
StandardCharsets.UTF_8);
+ if (workloadSvidTtl.equals(Duration.ofHours(1)) &&
caTtl.equals(Duration.ofHours(24))) {
+ return configuration;
+ }
+ return configuration
+ .replace("KeyManager \"memory\" {\n plugin_data {}",
+ "KeyManager \"disk\" {\n plugin_data {
directory = \"/tmp/spire-agent/keys\" }");
Review Comment:
⚠️ **`replace()` without match verification** — if the agent config resource
file's whitespace, quoting, or the `plugin_data {}` formatting ever changes,
this `replace()` is a silent no-op: the agent keeps in-memory keys, the
rotation test fails at runtime with no hint that configuration was wrong.
Assert the replacement took effect:
```suggestion
String patched = configuration.replace(
"KeyManager \"memory\" {\n plugin_data {}",
"KeyManager \"disk\" {\n plugin_data { directory
= \"/tmp/spire-agent/keys\" }");
if (patched.equals(configuration)) {
throw new IllegalStateException(
"SPIRE agent config patch failed: KeyManager target
string not found — check agent config resource formatting");
}
return patched;
```
##########
test-infra/camel-test-infra-spiffe/src/main/java/org/apache/camel/test/infra/spiffe/services/SpiffeLocalContainerInfraService.java:
##########
@@ -155,6 +178,51 @@ private void doInitialize() {
LOG.info("SPIRE Workload API available at {}", socketPath);
}
+ String agentConfiguration() {
+ try (var input =
getClass().getClassLoader().getResourceAsStream(AGENT_CONF_RESOURCE)) {
+ if (input == null) {
+ throw new IllegalStateException("Missing SPIRE agent
configuration");
+ }
+ // Restarting a process must retain its node key, rather than try
to reuse a consumed join token.
+ String configuration = new String(input.readAllBytes(),
StandardCharsets.UTF_8);
+ if (workloadSvidTtl.equals(Duration.ofHours(1)) &&
caTtl.equals(Duration.ofHours(24))) {
+ return configuration;
+ }
+ return configuration
+ .replace("KeyManager \"memory\" {\n plugin_data {}",
+ "KeyManager \"disk\" {\n plugin_data {
directory = \"/tmp/spire-agent/keys\" }");
+ } catch (IOException e) {
+ throw new RuntimeException("Could not read SPIRE agent
configuration", e);
+ }
+ }
+
+ String serverConfiguration() {
+ try (var input =
getClass().getClassLoader().getResourceAsStream(SERVER_CONF_RESOURCE)) {
+ if (input == null) {
+ throw new IllegalStateException("Missing SPIRE server
configuration");
+ }
+ return new String(input.readAllBytes(), StandardCharsets.UTF_8)
+ .replace("ca_ttl = \"24h\"", "ca_ttl = \"" +
caTtl.getSeconds() + "s\"")
+ .replace("default_x509_svid_ttl = \"1h\"",
+ "default_x509_svid_ttl = \"" +
workloadSvidTtl.getSeconds() + "s\"");
Review Comment:
⚠️ **`replace()` without match verification** — same issue as
`agentConfiguration()`. If the server config resource file is updated (e.g.
`"24h"` becomes `"86400s"` or the key name changes), both `replace()` calls are
silent no-ops and rotation tests run with default 24h/1h TTLs, waiting forever
and timing out without a useful error.
Verify both replacements succeeded (check that the result differs from the
original):
```suggestion
String base = new String(input.readAllBytes(),
StandardCharsets.UTF_8);
String patched = base
.replace("ca_ttl = \"24h\"", "ca_ttl = \"" +
caTtl.getSeconds() + "s\"")
.replace("default_x509_svid_ttl = \"1h\"",
"default_x509_svid_ttl = \"" +
workloadSvidTtl.getSeconds() + "s\"");
if (patched.equals(base)) {
throw new IllegalStateException(
"SPIRE server config patch failed: TTL target strings
not found — check server config resource formatting");
}
return patched;
```
##########
test-infra/camel-test-infra-spiffe/src/main/java/org/apache/camel/test/infra/spiffe/services/SpiffeLocalContainerInfraService.java:
##########
@@ -155,6 +178,51 @@ private void doInitialize() {
LOG.info("SPIRE Workload API available at {}", socketPath);
}
+ String agentConfiguration() {
+ try (var input =
getClass().getClassLoader().getResourceAsStream(AGENT_CONF_RESOURCE)) {
+ if (input == null) {
+ throw new IllegalStateException("Missing SPIRE agent
configuration");
+ }
+ // Restarting a process must retain its node key, rather than try
to reuse a consumed join token.
+ String configuration = new String(input.readAllBytes(),
StandardCharsets.UTF_8);
+ if (workloadSvidTtl.equals(Duration.ofHours(1)) &&
caTtl.equals(Duration.ofHours(24))) {
Review Comment:
⚠️ **Magic Duration constants duplicated from the no-arg constructor** — the
guard `workloadSvidTtl.equals(Duration.ofHours(1)) &&
caTtl.equals(Duration.ofHours(24))` hard-codes the same values as the no-arg
constructor `this(Duration.ofHours(1), Duration.ofHours(24))`. If the defaults
change, this guard silently routes the agent to the wrong key manager.
Extract the defaults as named constants so both sites stay in sync:
```suggestion
static final Duration DEFAULT_WORKLOAD_SVID_TTL = Duration.ofHours(1);
static final Duration DEFAULT_CA_TTL = Duration.ofHours(24);
```
Then reference them in the no-arg constructor and this guard:
```java
this(DEFAULT_WORKLOAD_SVID_TTL, DEFAULT_CA_TTL);
// ...
if (workloadSvidTtl.equals(DEFAULT_WORKLOAD_SVID_TTL) &&
caTtl.equals(DEFAULT_CA_TTL)) {
```
--
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]
