Add --encryption to helm install mode #1583
Conversation
This enables the helm install to use the --encryption flag by moving the key creation into the shared pre-install hook. Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>
|
I just now noticed there is a test matrix now for both modes, will add e2e tests |
This now uses the same --encryption flag for both helm mode and classic installation mode. Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>
|
|
||
| if k.params.Encryption == encryptionIPsec { | ||
| // TODO(aanm) automate this as well in form of helm chart | ||
| if err := k.createEncryptionSecret(ctx); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
let me push back a bit on carrying this logic forward.
in general, i'd like to get to the point where cilium-cli does not create any additional resources that are not managed by helm. otherwise we'll end up in a similar situation as the classic mode where resources are managed in 2 different places.
my proposal would be to do either:
- address this todo
// TODO(aanm) automate this as well in form of helm chartand put the logic in the helm chart if we think this resource should be managed by cilium.
or
- keep the logic as is to make it clear that the user is responsible for managing this resource.
There was a problem hiding this comment.
Very good remark
I would opt to place the secret creation inside the helm chart. However the generation of the secret data itself would be hard as as far as I know helm has no provision for key generation? Would adding a data field to the values be an option that when it is set it will create the secret?
We then would have to store the key inside the values, would that be a potential security risk?
maintainer-s-little-helper
bot
added
the
ready-to-merge
michi-covalent
added
the
dont-merge/blocked
|
we are having an offline discussion regarding the security implications of this change. added |
michi-covalent
removed
the
ready-to-merge
|
I’ve marked this PR as draft while you work through the feedback in the PR above. When you’re ready to continue with review & testing of the PR, please click “Ready for Review” at the bottom of the page. |
|
Forgot this one existed, after long discussions we did not go with this approach. Closing it. |
This enables the helm install to use the --encryption flag by moving the key creation into the shared pre-install hook.
This will un-block the work of bringing the cilium/cilium tests to Helm mode to test new features that require those mode like Spire and the Envoy DS.