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

ISPN-12101 Credential Store #8841

Merged

Conversation

tristantarrant
Copy link
Member

@tristantarrant
Copy link
Member Author

@oraNod some docs in here that will probably need your expertise

@tristantarrant
Copy link
Member Author

oops, I think I may have messed up with the SSL docs. I'll fix that.

@tristantarrant tristantarrant force-pushed the ISPN-12101/credential-store branch 3 times, most recently from f4d0e1e to e6cc584 Compare November 10, 2020 14:12
@oraNod oraNod added the Documentation Pull request containing only documentation changes label Nov 10, 2020
@pruivo
Copy link
Member

pruivo commented Nov 11, 2020

@tristantarrant is this PR completed? A quick look shows CredentialStoresConfiguration is empty and I don't see where it is used. what am I missing?

@tristantarrant
Copy link
Member Author

The only purpose for that is for the Json configuration serialization

Copy link
Member

@pruivo pruivo left a comment

Choose a reason for hiding this comment

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

partial look #1 :)

@tristantarrant
Copy link
Member Author

@diegolovison can you look at the tests

@pruivo
Copy link
Member

pruivo commented Nov 11, 2020

@tristantarrant the CLI creates the keystore in $PWD and not in server/conf by default. can't be changed?

[pedro@pedro-desktop infinispan-server-12.0.0-SNAPSHOT]$ ./bin/cli.sh credentials add dbpassword -p "secret1234!"
Set a credential: 
Confirm the credential: 
[pedro@pedro-desktop infinispan-server-12.0.0-SNAPSHOT]$ ls
bin  boot  Copyright.txt  credentials.pfx  docs  lib  README.md  server  static

@tristantarrant
Copy link
Member Author

Working on that

@tristantarrant tristantarrant force-pushed the ISPN-12101/credential-store branch 3 times, most recently from e47086a to 2c37a37 Compare November 13, 2020 07:46
Copy link
Member

@pruivo pruivo left a comment

Choose a reason for hiding this comment

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

offtopic change if possible:
org.infinispan.server.configuration.ServerConfigurationParser#coreLog field can be final

@tristantarrant tristantarrant removed the Documentation Pull request containing only documentation changes label Nov 24, 2020
@tristantarrant
Copy link
Member Author

@pruivo addressed all comments and repushed

@pruivo
Copy link
Member

pruivo commented Nov 24, 2020

what about the hidden comments? :)

@tristantarrant tristantarrant force-pushed the ISPN-12101/credential-store branch 2 times, most recently from 083c708 to e608605 Compare November 24, 2020 14:05
@tristantarrant
Copy link
Member Author

Damn, now it's done

cli/src/main/resources/help/credentials.adoc Show resolved Hide resolved
return new AttributeSet(CredentialStoreConfiguration.class, NAME, PATH, RELATIVE_TO, TYPE, CREDENTIAL);
}

private static ElementDefinition<CredentialStoreConfiguration> ELEMENT_DEFINITION = new DefaultElementDefinition(Element.CREDENTIAL_STORE.toString());
Copy link
Member

Choose a reason for hiding this comment

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

this was partially applied :)

* @since 12.0
**/
public class CredentialStoresConfiguration implements ConfigurationInfo {
private static final ElementDefinition<CredentialStoresConfiguration> ELEMENT_DEFINITION = new DefaultElementDefinition(Element.CREDENTIAL_STORES.toString());
Copy link
Member

Choose a reason for hiding this comment

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

change not applied.

public class CredentialStoresConfiguration implements ConfigurationInfo {
private static final ElementDefinition<CredentialStoresConfiguration> ELEMENT_DEFINITION = new DefaultElementDefinition(Element.CREDENTIAL_STORES.toString());

private final List<CredentialStoreConfiguration> credentialStores;
Copy link
Member

Choose a reason for hiding this comment

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

can be removed :)

@tristantarrant
Copy link
Member Author

@pruivo The CLI now behaves as follows:
if the argument to path is plain (i.e. just the filename), then it is relative to $ISPN_ROOT/conf. Otherwise it will be relative to cwd or absolute (if it starts with a /)

Copy link
Member

@pruivo pruivo left a comment

Choose a reason for hiding this comment

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

final nitpicks. I would have merged it but I've noticed the formatting error in "help" message; so I added the other 2 nits :)

@tristantarrant
Copy link
Member Author

@pruivo done

@pruivo pruivo merged commit 507a6ab into infinispan:master Nov 25, 2020
@pruivo
Copy link
Member

pruivo commented Nov 25, 2020

integrated! thanks @tristantarrant !

@tristantarrant tristantarrant deleted the ISPN-12101/credential-store branch July 5, 2022 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants