gnodet commented on code in PR #25012: URL: https://github.com/apache/camel/pull/25012#discussion_r3630946669
########## components/camel-cyberark-vault/src/test/java/org/apache/camel/component/cyberark/vault/integration/CyberArkTestSupport.java: ########## @@ -0,0 +1,126 @@ +/* + * 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.cyberark.vault.integration; + +import java.net.URI; +import java.net.URLEncoder; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.time.Duration; + +import org.apache.camel.EndpointInject; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +abstract class CyberArkTestSupport extends CamelTestSupport { + + static final Logger LOG = LoggerFactory.getLogger(CyberArkTestSupport.class); + + static HttpClient httpClient; + static String authToken; + + @EndpointInject("mock:result") + MockEndpoint mockResult; + + @BeforeAll + public static void beforeAll() throws Exception { + httpClient = HttpClient.newBuilder() + .connectTimeout(Duration.ofSeconds(10)) + .build(); + + authToken = authenticate(); + } + + @AfterAll + public static void afterAll() throws Exception { + authToken = null; + httpClient = null; + } + + private static String authenticate() throws Exception { + String url = String.format("%s/authn/%s/%s/authenticate", + System.getProperty("camel.cyberark.url"), + System.getProperty("camel.cyberark.account"), + System.getProperty("camel.cyberark.username")); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create(url)) + .header("Content-Type", "text/plain") + .header("Accept-Encoding", "base64") + .POST(HttpRequest.BodyPublishers.ofString(System.getProperty("camel.cyberark.apiKey"))) + .build(); + + HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); + requireSuccess("Authenticate", response); + + return response.body(); + } + + static void loadPolicy(String policy) throws Exception { + + String policyUrl = String.format("%s/policies/%s/policy/%s", + System.getProperty("camel.cyberark.url"), + System.getProperty("camel.cyberark.account"), + URLEncoder.encode("root", StandardCharsets.UTF_8)); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create(policyUrl)) + .header("Authorization", "Token token=\"" + authToken + "\"") + .header("Content-Type", "text/plain") + .method("PATCH", HttpRequest.BodyPublishers.ofString(policy)) + .build(); + + HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); + requireSuccess("Load policy", response); + } + + static void createSecret(String secretId, String secretValue) { + try { + String url = String.format("%s/secrets/%s/variable/%s", + System.getProperty("camel.cyberark.url"), Review Comment: Minor: `secretId` is not URL-encoded here, but test secrets use paths with `/` (e.g., `app/config`, `test/secret`). The production code in `ConjurClientImpl.createSecret()` correctly calls `URLEncoder.encode(secretId, StandardCharsets.UTF_8)`, and the `loadPolicy()` method below also uses `URLEncoder.encode()`. Consider adding encoding here for consistency: ```java String encodedSecretId = URLEncoder.encode(secretId, StandardCharsets.UTF_8); ``` This is pre-existing from the old per-test code, so not a blocker. ########## components/camel-cyberark-vault/src/test/java/org/apache/camel/component/cyberark/vault/integration/CyberArkTestSupport.java: ########## @@ -0,0 +1,126 @@ +/* + * 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.cyberark.vault.integration; + +import java.net.URI; +import java.net.URLEncoder; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.time.Duration; + +import org.apache.camel.EndpointInject; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +abstract class CyberArkTestSupport extends CamelTestSupport { + + static final Logger LOG = LoggerFactory.getLogger(CyberArkTestSupport.class); + + static HttpClient httpClient; + static String authToken; + + @EndpointInject("mock:result") + MockEndpoint mockResult; + + @BeforeAll + public static void beforeAll() throws Exception { + httpClient = HttpClient.newBuilder() + .connectTimeout(Duration.ofSeconds(10)) + .build(); + + authToken = authenticate(); + } + + @AfterAll + public static void afterAll() throws Exception { + authToken = null; + httpClient = null; + } + + private static String authenticate() throws Exception { + String url = String.format("%s/authn/%s/%s/authenticate", + System.getProperty("camel.cyberark.url"), + System.getProperty("camel.cyberark.account"), + System.getProperty("camel.cyberark.username")); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create(url)) + .header("Content-Type", "text/plain") + .header("Accept-Encoding", "base64") + .POST(HttpRequest.BodyPublishers.ofString(System.getProperty("camel.cyberark.apiKey"))) + .build(); + + HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); + requireSuccess("Authenticate", response); + + return response.body(); + } + + static void loadPolicy(String policy) throws Exception { + + String policyUrl = String.format("%s/policies/%s/policy/%s", + System.getProperty("camel.cyberark.url"), + System.getProperty("camel.cyberark.account"), + URLEncoder.encode("root", StandardCharsets.UTF_8)); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create(policyUrl)) + .header("Authorization", "Token token=\"" + authToken + "\"") + .header("Content-Type", "text/plain") + .method("PATCH", HttpRequest.BodyPublishers.ofString(policy)) + .build(); + + HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); + requireSuccess("Load policy", response); + } + + static void createSecret(String secretId, String secretValue) { + try { + String url = String.format("%s/secrets/%s/variable/%s", + System.getProperty("camel.cyberark.url"), + System.getProperty("camel.cyberark.account"), + secretId); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create(url)) + .header("Authorization", "Token token=\"" + authToken + "\"") + .header("Content-Type", "application/octet-stream") + .POST(HttpRequest.BodyPublishers.ofString(secretValue)) + .build(); + + HttpResponse<String> response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); + requireSuccess("Created secret", response); + + } catch (Exception e) { + LOG.warn("Could not create secret '{}': {}", secretId, e.getMessage()); Review Comment: The `catch (Exception e)` here swallows exceptions from `requireSuccess()` (called at line 113), rendering the success check ineffective. If secret creation fails, subsequent tests will get misleading errors about missing secrets rather than the actual HTTP failure. In contrast, `loadPolicy()` below declares `throws Exception` and lets `requireSuccess()` propagate. Consider the same pattern here for consistency: ```java static void createSecret(String secretId, String secretValue) throws Exception { // ... (remove the try/catch or at least re-throw) } ``` -- 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]
