-
Notifications
You must be signed in to change notification settings - Fork 81
[WFCORE-5744] Improve key-store certificate health checks #724
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
base: main
Are you sure you want to change the base?
Conversation
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.
I have added some comments inline.
I think the main things to begin with are we need to lose the references to the EAP7 as this is Community stability level and begin the WildFly Feature development process and call for volunteers to participate in the feature team.
[[test_plan]] | ||
== Test Plan | ||
|
||
Integration tests should be fairly acceptable. |
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.
We should expand on this section, we have a number of supported KeyStore types and some of these even have sub types so we should specify the tests and the permutations we will run.
|
||
== Admin Clients | ||
|
||
CLI should not be affected. However, HAL will most likely require follow up, since this feature will introduce new model params/context. Given defaults, its not a breaking change. |
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.
We should be able to work with @hpehl on this one once the configuration model is agreed to check if any specifics are needed for HAL, if they are they will need to be addressed before we can merge at the community stability level.
feature-team: | ||
developer: bbaranow@redhat.com | ||
sme: | ||
- dlofthou@redhat.com |
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.
Have you asked me to be SME?
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.
not yet, consider this a grace period.
|
||
=== Future Work | ||
|
||
N/A |
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.
This should not block this specific implementation but it does bring up a feature again I want to explore I have been calling "Administrator Encouragement" ideally it would be a service registered more centrally maybe in the management subsystem but we could do the equivalent local to the Elytron subsystem.
The idea is there are a lot of things we discover could be improved in the configuration of the server at runtime, possibly some could be detected by static analysis of config but many are runtime only and this certificate expiration is a good example.
So once the server is running the idea is we have this capability that individual subsystems can depend on and as runtime problems are discovered they can be registered with this service.
As the service is central our admin clients can then use management ops on this service to query the current known issues, so HAL could detect we have warnings about certificate expiration. The admin clients could detect some common issues and even provide guidance to resolve them, In the certificate space another example is self signed certificates.
Anyway that should not block this solution to log to the server logs but it could be a future feature we can consider. IMO if we did consider it I would suggest that features starts at Preview so we can start to add subsuystems to use it and then promote after we have demonstrated some stability.
* OpenShift Source-to-Image (S2I) | ||
* Bootable jar | ||
|
||
== Requirements |
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.
There should be a requirement(s) around administrative RBAC controls related to this.
This is something we've probably missed in a lot of analyses, so I'm being a bit unfair asking for it here, but we should start making it a practice to cover this and thus hopefully ensure the code does too.
@darranl Do you think this is something to mention in the template comment in 'Security Considerations'? The info would naturally be Requirement items, but the suggestion comment more naturally fits in security considerations.
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.
For this proposal I think it should be an ACK that the Elytron subsystem has RBAC defined at the root - but +1 I think the relationship to RBAC would be good to call out and test in the default template.
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.
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.
@darranl so no need to include this in proposal?
Issues:
https://issues.redhat.com/browse/WFCORE-5744
https://issues.redhat.com/browse/EAP7-1863