smolnar82 commented on code in PR #1015: URL: https://github.com/apache/knox/pull/1015#discussion_r2030651750
########## gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java: ########## @@ -92,6 +92,8 @@ public interface GatewayConfig { String CREDENTIAL_STORE_ALG = "gateway.credential.store.alg"; String DEFAULT_CREDENTIAL_STORE_ALG = "AES"; + String CREDENTIAL_SELF_SIGNING_ALG = "gateway.credential.self-signing.alg"; Review Comment: Two notes: - please use `.` as the separator character - I'm not sure the property name is correct. I think `gateway.self.signing.cert.alg` is more accurate ########## gateway-server/src/test/java/org/apache/knox/gateway/services/security/impl/DefaultKeystoreServiceTest.java: ########## @@ -814,7 +815,7 @@ private GatewayConfigImpl createGatewayConfig(Path baseDir) { } - private void createKeystore(DefaultKeystoreService keystoreService, Path keystoreFilePath, String alias, char[] password) + private void createKeystore(DefaultKeystoreService keystoreService, Path keystoreFilePath, String alias, char[] password, String selfSigningAlgorithm) Review Comment: If I were you, I'd pass the `GatewayConfig` object here like this: ``` private void createKeystore(DefaultKeystoreService keystoreService, char[] password, final GatewayConfig config) ``` and inline the `keystoreFilePath`, `alias`, and `selfSigningCertificateAlgorithm` when they are used within this private method. This way it will be more self-contained and resilient for future changes to the gateway config (i.e. if more properties are added, no need to update the method signature). ########## gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java: ########## @@ -266,6 +268,11 @@ public interface GatewayConfig { */ String getCredentialStoreAlgorithm(); + /** + * @return the algorithm that is used when generating a self-signing certificate. + */ + String getCredentialSelfSigningAlgorithm(); Review Comment: According to the above, I think `getSelfSigningCertificateAlgorithm()` is a better method name choice. ########## gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/DefaultKeystoreService.java: ########## @@ -117,6 +118,7 @@ public void init(GatewayConfig config, Map<String, String> options) } this.credentialStoreAlgorithm = config.getCredentialStoreAlgorithm(); + this.credentialSelfSigningAlgorithm = config.getCredentialSelfSigningAlgorithm(); Review Comment: nit: `selfSignedCertificateAlgorithm` -- 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: dev-unsubscr...@knox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org