sebidude / kubecrypt

Helper for dealing with secrets in kubernetes.
MIT License
23 stars 2 forks source link

add possibility to specify multiple keys at once (-k key1 -k key2 ...) #5

Closed jonny08152 closed 5 years ago

jonny08152 commented 5 years ago

New Feature :boom:

sebidude commented 5 years ago

I like the idea, but it introduces a confusing behavior when converting from a yaml file with encrypted values to a kubernetes secret. If mutliple keys are specified to find the maps in the yaml file and a key with same name exists in more than one map, the last one will win and the kubernetes secret will not state all the data.

I suggest to not convert to a kubernetes secret if multiple keys are specified and within the maps behind those keys, keys are same.

Here is how I tested:

cat << EOF > values.yaml 
data:
  password: testme
  token: foo
data2:
  password: test2
  token: foo2
safe:
  username: foobary
EOF

./kubecrypt yaml -e -i values.yaml -k data -k data2 -o safe.yaml
./kubecrypt convert foobar --key data -i safe.yaml 
./kubecrypt convert foobar --key data -k data2 -i safe.yaml 

It looks fine for a yaml file like this:

data:
  password: testme
  token: foo
data2:
  password2: test2
  token2: foo2
safe:
  username: foobary
jonny08152 commented 5 years ago

Hmm I see. A solution for this would be to limit the "multiple keys" functionality to the get and yaml sub-commands even if that introduces some inconsistency in sub-command behaviors. But I think the benefits of having the functionality in those commands outweighs the disadvantages of introduced inconsistencies.

sebidude commented 5 years ago

What about checking for duplos and stop if a key is already stated in the secrets data?

sebidude commented 5 years ago

Ok. Let's not allow multiple keys for convert. I'll do a review and you can do the fix then.

jonny08152 commented 5 years ago

multiple keys are now only allowed for yaml and get sub commands