Skip to content

Conversation

baranowb
Copy link
Contributor

@baranowb baranowb commented May 9, 2025

Copy link
Contributor

@darranl darranl left a 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.
Copy link
Contributor

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.
Copy link
Contributor

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
Copy link
Contributor

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?

Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stability-level/community "Community" stability level
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants