You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "oscerd (via GitHub)" <gi...@apache.org> on 2026/09/21 08:24:39 UTC

[PR] CAMEL-24751: camel-opa - unregister the OpaSecurityPolicy health check when its routes stop [camel]

oscerd opened a new pull request, #26667:
URL: https://github.com/apache/camel/pull/26667

   This fixes [CAMEL-24751](https://issues.apache.org/jira/browse/CAMEL-24751) (raised by @davsclaus reviewing #26436 and deferred there as out of scope for that change).
   
   `OpaSecurityPolicy` registered a readiness check in `beforeWrap` but nothing removed it when the routes it guards stop or reload, so a route reload left the check behind in the `HealthCheckRegistry` reporting on a policy that was no longer enforcing anything. Registration is deduplicated by id, so this was *wrong* rather than a leak.
   
   `OpaSecurityProcessor` is a `Service`, so its start/stop now drive a ref-count on the policy: the check is registered on the first guarded route to start and unregistered once the last one stops. Registration moved out of `beforeWrap` into the processor lifecycle because `beforeWrap` does not run again when a route is merely restarted — a check unregistered on stop has to be restored on the next start. A policy shared by several routes keeps its check until all of them stop.
   
   The existing `OpaSecurityPolicyHealthCheckTest` still passes: the `ownsClient` guard and every registration decision are unchanged, only *when* registration runs moved. New `OpaSecurityPolicyHealthCheckLifecycleTest` covers the shared-route ref-count, the unregister-on-last-stop and the restore-on-restart, and fails against the previous code (verified by reverting the unregister).
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-24751: camel-opa - unregister the OpaSecurityPolicy health check when its routes stop [camel]

Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on code in PR #26667:
URL: https://github.com/apache/camel/pull/26667#discussion_r4063739084


##########
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:
   Good catch — addressed in ba51c5d5. Added `doesNotRegisterACheckWhenHealthCheckIsDisabled`, which starts a `healthCheckEnabled=false` policy through a route and asserts nothing registers, so the guard is now exercised through the `onProcessorStart` path rather than only `beforeWrap`. I gave the disabled policy a distinct `serverUrl` so its would-be check can't deduplicate against the one the enabled routes share, and confirmed the test goes red when the `!healthCheckEnabled` guard is removed from `registerHealthCheck`.
   



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-24751: camel-opa - unregister the OpaSecurityPolicy health check when its routes stop [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26667:
URL: https://github.com/apache/camel/pull/26667#issuecomment-5757554863

   :star2: Thank you for your contribution to the Apache Camel project! :star2:
     :robot: CI automation will test this PR automatically.
   
     :camel: Apache Camel Committers, please review the following items:
   
     * First-time contributors **require MANUAL approval** for the GitHub Actions to run
     * You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot although they are normally detected and executed by CI.
     * You can label PRs using `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
     * Build and test logs are available in the summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary.
     
     :warning: Be careful when sharing logs. Review their contents before sharing them publicly.


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-24751: camel-opa - unregister the OpaSecurityPolicy health check when its routes stop [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26667:
URL: https://github.com/apache/camel/pull/26667#issuecomment-5757924697

   <!-- ci-tested-modules -->
   :test_tube: **CI tested the following changed modules:**
   
   - `components/camel-opa`
   
   ---
   
   <details><summary>:microscope: Scalpel shadow comparison — Scalpel: 9 of 692 tested, 27 compile-only — current: 9 all tested</summary>
   
   [Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **36 affected modules** (current approach: 9).
   
   **Skip-tests mode would test 9 modules** (1 direct + 8 downstream), **skip tests for 27** (generated code, meta-modules)
   
   <details><summary>:warning: Modules only in Scalpel (27)</summary>
   
   - `apache-camel`
   - `camel-allcomponents`
   - `camel-catalog`
   - `camel-catalog-console`
   - `camel-catalog-maven`
   - `camel-catalog-suggest`
   - `camel-componentdsl`
   - `camel-endpointdsl`
   - `camel-endpointdsl-support`
   - `camel-itest`
   - `camel-jbang-core`
   - `camel-jbang-it`
   - `camel-jbang-main`
   - `camel-jbang-plugin-edit`
   - `camel-jbang-plugin-generate`
   - `camel-jbang-plugin-kubernetes`
   - `camel-jbang-plugin-test`
   - `camel-kamelet-main`
   - `camel-launcher`
   - `camel-report-maven-plugin`
   - `camel-route-parser`
   - `camel-yaml-dsl`
   - `camel-yaml-dsl-deserializers`
   - `camel-yaml-dsl-maven-plugin`
   - `coverage`
   - `docs`
   - `dummy-component`
   
   </details>
   
   
   <details><summary>Modules Scalpel would test (9)</summary>
   
   - `camel-jbang-mcp` ← downstream of org.apache.camel:camel-catalog
   - `camel-jbang-plugin-mcp` ← downstream of org.apache.camel:camel-jbang-core
   - `camel-jbang-plugin-route-parser` ← downstream of org.apache.camel:camel-route-parser
   - `camel-jbang-plugin-tui` ← downstream of org.apache.camel:camel-catalog
   - `camel-jbang-plugin-validate` ← downstream of org.apache.camel:camel-yaml-dsl-validator
   - `camel-launcher-container` ← downstream of org.apache.camel:camel-launcher
   - `camel-opa` ← components/camel-opa/src/main/java/org/apache/camel/component/opa/security/OpaSecurityPolicy.java, components/camel-opa/src/main/java/org/apache/camel/component/opa/security/OpaSecurityProcessor.java, components/camel-opa/src/test/java/org/apache/camel/component/opa/security/OpaSecurityPolicyHealthCheckLifecycleTest.java
   - `camel-yaml-dsl-validator` ← downstream of org.apache.camel:camel-catalog
   - `camel-yaml-dsl-validator-maven-plugin` ← downstream of org.apache.camel:camel-yaml-dsl-validator
   
   </details>
   
   <details><summary>Modules with tests skipped (27)</summary>
   
   - `apache-camel`
   - `camel-allcomponents`
   - `camel-catalog`
   - `camel-catalog-console`
   - `camel-catalog-maven`
   - `camel-catalog-suggest`
   - `camel-componentdsl`
   - `camel-endpointdsl`
   - `camel-endpointdsl-support`
   - `camel-itest`
   - `camel-jbang-core`
   - `camel-jbang-it`
   - `camel-jbang-main`
   - `camel-jbang-plugin-edit`
   - `camel-jbang-plugin-generate`
   - `camel-jbang-plugin-kubernetes`
   - `camel-jbang-plugin-test`
   - `camel-kamelet-main`
   - `camel-launcher`
   - `camel-report-maven-plugin`
   - `camel-route-parser`
   - `camel-yaml-dsl`
   - `camel-yaml-dsl-deserializers`
   - `camel-yaml-dsl-maven-plugin`
   - `coverage`
   - `docs`
   - `dummy-component`
   
   </details>
   
   > :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)
   
   </details>
   
   <details><summary>All tested modules (36 modules, 5m 30s total)</summary>
   
   **Total reactor time:** 5m 30s
   
   | Module | Duration | Status |
   | --- | --- | --- |
   | Camel :: Launcher | 51.8s | SUCCESS |
   | Camel :: JBang :: MCP | 42.3s | SUCCESS |
   | Camel :: JBang :: Plugin :: TUI | 34.4s | SUCCESS |
   | Camel :: OPA | 31.4s | SUCCESS |
   | Camel :: Component DSL | 24.3s | SUCCESS |
   | Camel :: Catalog :: Camel Catalog | 23.9s | SUCCESS |
   | Camel :: JBang :: Plugin :: Kubernetes | 15.3s | SUCCESS |
   | Camel :: Docs | 12.7s | SUCCESS |
   | Camel :: YAML DSL :: Validator | 12.0s | SUCCESS |
   | Camel :: Kamelet Main | 10.7s | SUCCESS |
   | Camel :: Catalog :: Camel Report Maven Plugin | 10.4s | SUCCESS |
   | Camel :: Catalog :: Camel Route Parser | 8.5s | SUCCESS |
   | Camel :: JBang :: Plugin :: Testing | 8.1s | SUCCESS |
   | Camel :: YAML DSL :: Deserializers | 6.2s | SUCCESS |
   | Camel :: All Components Sync point | 6.1s | SUCCESS |
   | Camel :: JBang :: Plugin :: Validate | 6.0s | SUCCESS |
   | Camel :: YAML DSL :: Maven Plugins | 3.6s | SUCCESS |
   | Camel :: YAML DSL :: Validator Maven Plugin | 3.2s | SUCCESS |
   | Camel :: Catalog :: Maven | 2.9s | SUCCESS |
   | Camel :: Catalog :: Suggest (deprecated) | 2.7s | SUCCESS |
   | Camel :: Assembly | 2.4s | SUCCESS |
   | Camel :: JBang :: Plugin :: Edit | 1.9s | SUCCESS |
   | Camel :: Coverage | 1.7s | SUCCESS |
   | Camel :: Endpoint DSL :: Support | 1.2s | SUCCESS |
   | Camel :: Catalog :: Dummy Component | 1.1s | SUCCESS |
   | Camel :: JBang :: Main | 1.0s | SUCCESS |
   | Camel :: JBang :: Plugin :: Generate | 0.9s | SUCCESS |
   | Camel :: JBang :: Integration tests | 0.9s | SUCCESS |
   | Camel :: Catalog :: Console | 0.8s | SUCCESS |
   | Camel :: JBang :: Plugin :: MCP | 0.8s | SUCCESS |
   | Camel :: Launcher :: Container | 0.6s | SUCCESS |
   | Camel :: JBang :: Plugin :: Route Parser | 0.5s | SUCCESS |
   | Camel :: Endpoint DSL | n/a |  |
   | Camel :: Integration Tests | n/a |  |
   | Camel :: JBang :: Core | n/a |  |
   | Camel :: YAML DSL | n/a |  |
   
   **Top 20 slowest modules:**
   - `Camel :: Launcher` (51.8s)
   - `Camel :: JBang :: MCP` (42.3s)
   - `Camel :: JBang :: Plugin :: TUI` (34.4s)
   - `Camel :: OPA` (31.4s)
   - `Camel :: Component DSL` (24.3s)
   - `Camel :: Catalog :: Camel Catalog` (23.9s)
   - `Camel :: JBang :: Plugin :: Kubernetes` (15.3s)
   - `Camel :: Docs` (12.7s)
   - `Camel :: YAML DSL :: Validator` (12.0s)
   - `Camel :: Kamelet Main` (10.7s)
   - `Camel :: Catalog :: Camel Report Maven Plugin` (10.4s)
   - `Camel :: Catalog :: Camel Route Parser` (8.5s)
   - `Camel :: JBang :: Plugin :: Testing` (8.1s)
   - `Camel :: YAML DSL :: Deserializers` (6.2s)
   - `Camel :: All Components Sync point` (6.1s)
   - `Camel :: JBang :: Plugin :: Validate` (6.0s)
   - `Camel :: YAML DSL :: Maven Plugins` (3.6s)
   - `Camel :: YAML DSL :: Validator Maven Plugin` (3.2s)
   - `Camel :: Catalog :: Maven` (2.9s)
   - `Camel :: Catalog :: Suggest (deprecated)` (2.7s)
   
   </details>
   
   ---
   :gear: [View full build and test results](https://github.com/apache/camel/actions/runs/35577750455)


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-24751: camel-opa - unregister the OpaSecurityPolicy health check when its routes stop [camel]

Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-24751: camel-opa - unregister the OpaSecurityPolicy health check when its routes stop [camel]

Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd merged PR #26667:
URL: https://github.com/apache/camel/pull/26667


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org