Skip to content

Conversation

@bberto
Copy link
Contributor

@bberto bberto commented May 19, 2021

fixes gh-953

* The KeyStore type (key store file extension used as default).
*/
private String type = "jks";
private String type;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove the default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently this property is not used in TextEncryptorUtils.createTextEncryptor and the default behaviour is to infer the keystore type from the file extension.

Removing the default value this behaviour is preserved, avoiding a breaking change for who is using, for example, encryption.key-store.location=classpath:test.jceks with no encryption.key-store.type set.

However, while writing this comment, I noticed a default value is expected here:
https://github.com/spring-cloud/spring-cloud-config/blob/e645c802157caa88a8ed50ecb9067335f0c7522f/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/config/EncryptionAutoConfiguration.java#L104

What do you suggest?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you enable your added code below only if it is not the default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to rollback to default value. The current behaviour (infer type from extension) is not documented anywhere and the error in case of keystore type mismatch is pretty clear.

@bberto bberto marked this pull request as draft May 27, 2021 06:34
@bberto bberto closed this May 27, 2021
@bberto
Copy link
Contributor Author

bberto commented May 27, 2021

Sorry, I made a mess with this PR. I opened a new one:
#963

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Property type is ignored when loading encryption keystore

3 participants