-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Mount each private key into single kubernetes secret #14950
Conversation
Set status to draft. Need to be checked on OpenShift environment. Just checked only on Kubernetes. |
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
E2E tests of Eclipse Che Multiuser on OCP has been successful:
|
Checked on Minishift. All private keys stored in single secret. |
ci-build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM, a few comments inline.
...ava/org/eclipse/che/workspace/infrastructure/kubernetes/provision/VcsSshKeysProvisioner.java
Show resolved
Hide resolved
...ava/org/eclipse/che/workspace/infrastructure/kubernetes/provision/VcsSshKeysProvisioner.java
Outdated
Show resolved
Hide resolved
...ava/org/eclipse/che/workspace/infrastructure/kubernetes/provision/VcsSshKeysProvisioner.java
Outdated
Show resolved
Hide resolved
...ava/org/eclipse/che/workspace/infrastructure/kubernetes/provision/VcsSshKeysProvisioner.java
Outdated
Show resolved
Hide resolved
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
crw-ci-test |
E2E tests of Eclipse Che Multiuser on OCP has been successful:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
.stream() | ||
.filter( | ||
sshPair -> | ||
!isNullOrEmpty(sshPair.getName()) && !isNullOrEmpty(sshPair.getPrivateKey())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that name must not be null or empty https://github.com/eclipse/che/blob/07263f1e30089689d71b057f747a44a29283e3c4/wsmaster/che-core-api-ssh-shared/src/main/java/org/eclipse/che/api/ssh/shared/model/SshPair.java#L25
And PrivateKey may be null, so the check is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a filter that checks only non nullable ssh keys, isn't it?
...ava/org/eclipse/che/workspace/infrastructure/kubernetes/provision/VcsSshKeysProvisioner.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please take a look my minor inline comments
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
Do we need any changes in docs? |
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
@skabashnyuk no, this is an internal modification, that is not reflected in any docs. |
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
ci-test |
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
ci-test |
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
ci-test |
E2E tests of Eclipse Che Multiuser on OCP has been successful:
|
Signed-off-by: Vlad Zhukovskyi <vzhukovs@redhat.com>
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:
|
E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:
|
E2E tests of Eclipse Che Multiuser on OCP has failed:
|
What does this PR do?
This changes proposal changes default behavior of storing ssh private keys in Kubernetes secrets. There was a mechanism, that stored each private key in dedicated secret, that might faced with exception related to secret count limit in internal infrastructure. New behavior puts all ssh private keys in one Kubernetes secret in following structure:
host_name:private_key
. Location of private ssh keys was changes on file system and become:/etc/ssh/private/{private_key_with_host_name}
. Kubernetes secret type was changed fromkubernetes.io/ssh-auth
toopaque
to allow multi value secret.Here is example of secret stored in Kubernetes 1 and file system structure 2.
Signed-off-by: Vlad Zhukovskyi vzhukovs@redhat.com
What issues does this PR fix or reference?
#14438
Mount each private key into single kubernetes secret
Release Notes
Mount each private key into single kubernetes secret
Docs PR
N/A