Skip to content
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

[WFCORE-2671][JBEAP-10328] key-managers to key-manager, key-manager:init() #146

Merged
merged 25 commits into from May 18, 2017

Conversation

hkalina
Copy link

@hkalina hkalina commented Apr 25, 2017

https://issues.jboss.org/browse/WFCORE-2671
https://issues.jboss.org/browse/JBEAP-10328

Need to be merged in coordination with wildfly-security-incubator/wildfly#205

  • key/trust-managers capability changed to key/trust-manager (no array) for simple wrapping/reloading
  • added operation key-manager:init()

@hkalina hkalina changed the title [WFCORE-2671] key-managers:init, ssl-context:key-refresh [WFCORE-2671][JBEAP-10328] key-managers:init, ssl-context:key-refresh Apr 25, 2017
Copy link

@kabir kabir left a comment

Choose a reason for hiding this comment

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

Approving paperwork

@@ -102,7 +102,7 @@
static final String KEY_MANAGERS_CAPABILITY = CAPABILITY_BASE + "key-managers";

static final RuntimeCapability<Void> KEY_MANAGERS_RUNTIME_CAPABILITY = RuntimeCapability
.Builder.of(KEY_MANAGERS_CAPABILITY, true, KeyManager[].class)
.Builder.of(KEY_MANAGERS_CAPABILITY, true, InjectedValue.class) // InjectedValue<KeyManager[]>
Copy link

Choose a reason for hiding this comment

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

TBH I am not completely sure about this one - by switching to InjectedValue we are loosing the capability typing. I think we may need to see if we have alternatives that can avoid loosing the type information.

Copy link
Author

Choose a reason for hiding this comment

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

What if I add special injector type, like KeyManagersInjector ? Will it be sufficient solution?

Copy link
Author

@hkalina hkalina May 9, 2017

Choose a reason for hiding this comment

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

@darranl Will be suggested solution sufficient?
Edit: no, because it would require to know our KeyManagerInjector to implement own capability provider

Copy link

@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.

Still thinking about this one, I don't think we should loose the type information from the capability - I think we need to find an alternative to handle this.

Eva Jarkovska and others added 20 commits May 5, 2017 12:22
…ng singleton deployment primary->backup transition
[WFCORE-2672] JDBC Realm Validation
[WFCORE-2730] allow empty target-name and action in permission
Fix for WFCORE-2798. CLI, cd command --no-validation handling
WFCORE-2795: Added a custom validator for Cipher Suites filter in Elytron
ElytronSubsystemMessages#credentialStoreIssueEncountered is never thr…
WFCORE-2794 Wildfly Build Tools 1.2.1.Final
[WFCORE-2790] WildFly Elytron Tool scripts
[WFCORE-2779] Improve help message in module CLI operation
@kabir
Copy link

kabir commented May 16, 2017

@darranl @honza889 it is marked for the beta, so please give some consideration again

kabir added 3 commits May 16, 2017 14:14
[WFCORE-2810] Let CapabilityServiceSupport users check for caps
[WFCORE-2807] removed server-side attributes from client-ssl-context
WFCORE-2791 Auto-remove lingering deployment unit attachments following singleton deployment primary->backup transition
@darranl
Copy link

darranl commented May 16, 2017

I think I will mention in chat - rather than relying on a different type being injected I think we should look at a custom KeyStore implementation so we can still inject a KeyStore but then add a type check and cast. We already have atomic keystores, maybe we can add something that also supports notifications?

@hkalina
Copy link
Author

hkalina commented May 17, 2017

Reworked as requested, tests green, so prepared for merge now.

@hkalina hkalina changed the title [WFCORE-2671][JBEAP-10328] key-managers:init, ssl-context:key-refresh [WFCORE-2671][JBEAP-10328] key-managers to key-manager, key-manager:init() May 17, 2017
@darranl darranl merged commit de41fa2 into wildfly-security-incubator:ladybird May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet