These are chat archives for spring-cloud/spring-cloud

10th
Apr 2015
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:38
Hi, we're trying to extend cloud config with support for keys per application-profile but we're stuck on some initial PR with some minor changes
spring-cloud/spring-cloud-config#121
Is there a chance that this PR can be merged or maybe there is something you want to discuss first before we move on with new functionality?
Dave Syer
@dsyer
Apr 10 2015 08:47
I can look at it later
It just hasn't bubbled up to the top yet
Actually I remember that one
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:48
thanks :-)
Dave Syer
@dsyer
Apr 10 2015 08:48
The only thing I wanted to check was whether we needed a *Locator abstraction
Would ObjectFactory<TextEncryptor> suffice?
I hadn't had time to play with it to check
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:50
The only reason that *Locator abstraction was introduced was that we could use single TextEncryptor
we could'nt*
Dave Syer
@dsyer
Apr 10 2015 08:50
I think you are probably right. It depends on what is needed to locate a TextEncryptor.
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:51
We actually prepared PoC a while ago so I can show you what we have in mind
Dave Syer
@dsyer
Apr 10 2015 08:51
Spencer had a comment about that
You plan to add additional methods to an interface?
It's not even an interface as it stands?
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:53
Yeah, we tried to create something simple first to support existing funcionality
Dave Syer
@dsyer
Apr 10 2015 08:53
I'd rather see a proposal for an actual TextEncryptorLocator interface that would be useful to you
Dave Syer
@dsyer
Apr 10 2015 08:54
You can't have fields with CapitalLetters
Just little things like that stopped me from getting very far
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:55
Sorry, silly refactoring
Dave Syer
@dsyer
Apr 10 2015 08:55
It needs some polish
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:55
That was just PoC to see how much effort it would take
Dave Syer
@dsyer
Apr 10 2015 08:55
You have an EncryptorFactory and a TextEncryptorLocator
Aren't they sort of the same thing?
Well, anyway, I think it's a good idea if it gets tidied up a bit.
But I also wanted to think about other strategies
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:57
From what I understood EncryptorFactory creates Encryptors given a key
Dave Syer
@dsyer
Apr 10 2015 08:57
It's hard enough to manage a single key
You would have to manage multiple keys
wojtkiewicz
@wojtkiewicz
Apr 10 2015 08:57
That was our plan actually
Dave Syer
@dsyer
Apr 10 2015 08:58
What is the actual business requirement?
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:01
We want to use cloud config in such a way that vulnerable data (like passwords) can be only decrypted by specific application
Dave Syer
@dsyer
Apr 10 2015 09:01
Good.
I have a feeling that might be possible with a single key
Travis CI do it
So, actually that was why I didn't merge the PR. I wanted time to think about a different approach. Didn't get it yet.
I don't know though
You might be right, and we end up needing a key per app
But as long as we know that's the requirement, we can use that to focus the API
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:04
Our approach was to encrypt data with different keys so password at-rests cannot be decrypted by different applications/teams
passwords at-rest
The next step it would be to create a distributed storage for keys
Dave Syer
@dsyer
Apr 10 2015 09:05
That's the scary bit
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:06
Yeah :-)
Idea was that we would roll our own implementation and use EmbeddedConfigServer
Inject different key storage via interface
By default we wanted to support standard Java keystores
Dave Syer
@dsyer
Apr 10 2015 09:07
What's an EmbeddedConfigServer?
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:08
Oh, I think I messed up a name of that annotation
Dave Syer
@dsyer
Apr 10 2015 09:09
@EnableConfigServer?
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:09
yeah thats the one
Dave Syer
@dsyer
Apr 10 2015 09:10
I think if TextEncryptorLocator was an interface with 1 method it wouldn't hurt
Easier to document and understand
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:11
no problem, that was just an initial PR to get some initial feedback
Dave Syer
@dsyer
Apr 10 2015 09:11
What's the EnvironmentAlias thing?
I don't see it in the PR
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:12
That PoC that we did seemed to be a little too large and we have troubles with supporting and testing existing functionality
so we thought that if we do some minor incremental changes we can roll out some working solution
with your help
Rafał Żukowski
@rzukow
Apr 10 2015 09:14
Hi. Sorry for interrupting, I will try to explain our basic business requirement:
Our example scenario is as follows:
  • We want DB password to be encrypted and available for service A but NOT for service B.
  • We ask DBA for password, DBA is requesting a configuration service for an encrypted value. DBA is sending us an encrypted value, so we can put it into configuration repo.
  • This value will be encrypted correctly by configuration service ONLY when service A is asking for configuration, but will not be encrypted when I put it for configuration files of the service B, as different key is used.
    Please, let me know if something is unclear.
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:14
EnvironmentAlias thing was just a hacked thing that provides unique alias to store key for application/profile in a key store
Rafał Żukowski
@rzukow
Apr 10 2015 09:18
*In my last point I'm talking about decryption (not encryption)
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:19
PoC has plenty of rough edges and it wasn't refactored enough to suggest a PR
Rafał Żukowski
@rzukow
Apr 10 2015 09:22
Maybe we don't need a key-per-application to achieve our goal, but at least we need an encryptor-per-application (possible with some salting strategy).
Dave Syer
@dsyer
Apr 10 2015 09:24
Salting was what I wanted to look at
So anyway, some version of the code you propose will almost certainly be necessary.
It would be good if you tidied it up a bit
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:34
ok, so what should we do with existing PR?
Dave Syer
@dsyer
Apr 10 2015 09:34
Clean it up. Rebase it. Would that be OK?
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:35
yeah :-)
scope is fine?
Dave Syer
@dsyer
Apr 10 2015 09:36
I think so. As long as the TextEncryptorLocator can be extracted and simplified a bit
wojtkiewicz
@wojtkiewicz
Apr 10 2015 09:40
Ok. Thanks for your feedback.
Matt Reynolds
@mattreyuk
Apr 10 2015 21:38
Hi I’m hoping to get some help with a hystrix config issue I’m running into. I have created a custom concurrency strategy so I can pass logging MDC data through but that results in the request log getting turned on. I should be able to turn that off but hystrix.command.default.requestLogEnabled: false in the properties file doesn’t work nor does HystrixCommandProperties.Setter().withRequestLogEnabled(false) I still get an illegalState exception from the request log trying to run. I put together a minimal repo based on the Spring Cloud Tests Hystrix example here: https://github.com/mattreyuk/hystrix-config-issue can someone take a look and tell me what I’m doing wrong? Thanks
Spencer Gibb
@spencergibb
Apr 10 2015 22:07
I’m not sure I know much more about it than you do