gnodet-bot commented on code in PR #26667: URL: https://github.com/apache/camel/pull/26667#discussion_r4060405927
########## components/camel-opa/src/test/java/org/apache/camel/component/opa/security/OpaSecurityPolicyHealthCheckLifecycleTest.java: ########## @@ -0,0 +1,106 @@ +/* + * 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.opa.security; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; + +import com.sun.net.httpserver.HttpServer; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.health.HealthCheckRegistry; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * The readiness check registered by {@link OpaSecurityPolicy} must follow the lifecycle of the routes it guards: it is + * kept while any guarded route runs, removed once the last one stops, and restored when a route starts again. Otherwise + * a stopped or reloaded route leaves a check behind reporting on a policy that is no longer enforcing anything + * (CAMEL-24751). + */ +public class OpaSecurityPolicyHealthCheckLifecycleTest extends CamelTestSupport { + + private static HttpServer server; + private static String serverUrl; + + private final OpaSecurityPolicy policy = new OpaSecurityPolicy(); + + @AfterEach + void stopServer() { + if (server != null) { + server.stop(0); + server = null; + } + } + + private static String startHealthyServer() throws IOException { + server = HttpServer.create(new InetSocketAddress("localhost", 0), 0); + server.createContext("/health", exchange -> { + exchange.sendResponseHeaders(200, -1); + try (OutputStream out = exchange.getResponseBody()) { + out.flush(); + } + }); + server.start(); + return "http://localhost:"; + server.getAddress().getPort(); + } + + @Override + protected RouteBuilder createRouteBuilder() throws Exception { + serverUrl = startHealthyServer(); + policy.setPolicyPath("authz/allow"); Review Comment: The new test only covers `healthCheckEnabled=true` (the default). The `healthCheckEnabled=false` guard in `registerHealthCheck()` is now exercised only through `onProcessorStart`, not through `beforeWrap` as in the old flow. `OpaSecurityPolicyHealthCheckTest.registersNothingWhenTheCheckIsDisabled()` still passes after this PR, but it now calls `beforeWrap` directly — which no longer touches the registry at all. That test no longer validates the `healthCheckEnabled=false` guard through the route-start path. Consider adding a case here: ```java @Test void registersNothingWhenHealthCheckIsDisabled() throws Exception { // the check is registered on processor start; disabled policies must not register it OpaSecurityPolicy disabled = new OpaSecurityPolicy(); disabled.setPolicyPath("authz/allow"); disabled.setServerUrl(serverUrl); disabled.setHealthCheckEnabled(false); // share the same CamelContext to observe registry side-effects context.addRoutes(new RouteBuilder() { @Override public void configure() { from("direct:disabled").routeId("disabled-route").policy(disabled).to("mock:result"); } }); context.getRouteController().startRoute("disabled-route"); assertThat(registeredChecks()).isEqualTo(1); // only the one from the two main routes } ``` Without this, a regression that removes the `!healthCheckEnabled` guard from `registerHealthCheck` would go undetected via the new lifecycle path. -- 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]
