-
Notifications
You must be signed in to change notification settings - Fork 306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FISH-5878 Add Certificate Alias Property Names Into Payara API as Constants #5527
Conversation
jenkins test please |
api/payara-api/src/main/java/fish/payara/security/client/CertificateAlias.java
Outdated
Show resolved
Hide resolved
api/payara-api/src/main/java/fish/payara/security/client/CertificateAlias.java
Outdated
Show resolved
Hide resolved
api/payara-api/src/main/java/fish/payara/security/client/CertificateAlias.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
jenkins test please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The properties are wrong?
You're missing a property: PAYARA_MP_CONFIG_CLIENT_CERTIFICATE_ALIAS
Also we should really not have duplicate properties if we can help it:
https://github.com/payara/Payara/blob/payara-server-5.2021.9/appserver/payara-appserver-modules/microprofile/rest-client-ssl/src/main/java/fish/payara/microprofile/jaxrs/client/ssl/PayaraConstants.java
https://github.com/payara/ecosystem-rest-ssl-configuration/blob/master/src/main/java/fish/payara/ecosystem/jaxrs/client/ssl/PayaraConstantsExtension.java
@Pandrex247 The properties are correct, they are the same as in the documentation. I believe the one mentioned in https://github.com/payara/Payara/blob/payara-server-5.2021.9/appserver/payara-appserver-modules/microprofile/rest-client-ssl/src/main/java/fish/payara/microprofile/jaxrs/client/ssl/PayaraConstants.java are for adding it to the Microprofile Config? And I also don't believe this file is included in the Payara API so it is not accessible when the API is added as a dependency. However, https://github.com/payara/ecosystem-rest-ssl-configuration/blob/master/src/main/java/fish/payara/ecosystem/jaxrs/client/ssl/PayaraConstantsExtension.java is included when the user adds the rest-ssl dependency, but this only includes the JAX-RS Client Certificate Alias |
api/payara-api/src/main/java/fish/payara/security/client/PayaraConstants.java
Outdated
Show resolved
Hide resolved
I'm not suggesting including the PayaraConstants file in the API, I'm suggesting that you drop the duplicate property from the PayaraConstants file and have rest-client-ssl use the property from the API, rather than relying on us to keep the two in sync. |
I have made the requested changes. I will release a new version of ecosystem-rest-client-ssl once Payara 5.2022.1 is released. payara/ecosystem-rest-ssl-configuration#1 |
jenkins test please |
FISH-5878 Add Certificate Alias Property Names Into Payara API as Constants
Description
Added certificate alias properties as constants to Payara API
Important Info
Blockers
N/A
Testing
New tests
N/A
Testing Performed
Build Payara and checked to see if
CertificateAlias.JAX_RS_CLIENT
andCertificateAlias.REST_CLIENT
were available from API DependencyTesting Environment
maven 3.6.3 openjdk version "1.8.0_302" ubuntu 20.04
Documentation
n/a
Notes for Reviewers
n/a