Skip to content
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

Use the hashed name for the path to each ssh-secret. (eclipse#14151). #14243

Closed
wants to merge 1 commit into from
Closed

Use the hashed name for the path to each ssh-secret. (eclipse#14151). #14243

wants to merge 1 commit into from

Conversation

monaka
Copy link
Member

@monaka monaka commented Aug 15, 2019

Signed-off-by: Masaki Muranaka monaka@monami-ya.com

What does this PR do?

Uses the hashed name for the path to each ssh-secret.
As I commented, the name for key-pair can be contain characters that is not fit for file path.

What issues does this PR fix or reference?

eclipse#14151 eclipse#14152 eclipse#14156

@monaka
Copy link
Member Author

monaka commented Aug 15, 2019

I'm testing it now on my Che instance.

@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

Can one of the admins verify this patch?

1 similar comment
@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

Can one of the admins verify this patch?

@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Aug 15, 2019
@monaka
Copy link
Member Author

monaka commented Aug 16, 2019

I checked this on my instance. It works well.

$ kubectl exec -it --namespace=banco workspacehxpkv5npvvllwbwo.dockerimage-7f569c8849-psvdg find /etc/ssh
Defaulting container name to container.
Use 'kubectl describe pod/workspacehxpkv5npvvllwbwo.dockerimage-7f569c8849-psvdg -n banco' to see all of the containers in this pod.
/etc/ssh
/etc/ssh/ssh_host_ed25519_key
/etc/ssh/ssh_host_ecdsa_key
/etc/ssh/ssh_host_dsa_key
/etc/ssh/sshd_config
/etc/ssh/ssh_import_id
/etc/ssh/ssh_config
/etc/ssh/moduli
/etc/ssh/ssh_host_rsa_key
/etc/ssh/ssh_host_dsa_key.pub
/etc/ssh/ssh_host_ed25519_key.pub
/etc/ssh/ssh_host_rsa_key.pub
/etc/ssh/ssh_host_ecdsa_key.pub
/etc/ssh/3aeb002460381c6f258e8395d3026f571f0d9a76488dcd837639b13aed316560
/etc/ssh/3aeb002460381c6f258e8395d3026f571f0d9a76488dcd837639b13aed316560/..data
/etc/ssh/3aeb002460381c6f258e8395d3026f571f0d9a76488dcd837639b13aed316560/ssh-privatekey
/etc/ssh/3aeb002460381c6f258e8395d3026f571f0d9a76488dcd837639b13aed316560/..2019_08_16_01_23_05.465061888
/etc/ssh/3aeb002460381c6f258e8395d3026f571f0d9a76488dcd837639b13aed316560/..2019_08_16_01_23_05.465061888/ssh-privatekey
/etc/ssh/470aada635507690fc3a5391e4111d6e013c55a63d61497a079c60fad8d4899c
/etc/ssh/470aada635507690fc3a5391e4111d6e013c55a63d61497a079c60fad8d4899c/..data
/etc/ssh/470aada635507690fc3a5391e4111d6e013c55a63d61497a079c60fad8d4899c/ssh-privatekey
/etc/ssh/470aada635507690fc3a5391e4111d6e013c55a63d61497a079c60fad8d4899c/..2019_08_16_01_23_05.253730078
/etc/ssh/470aada635507690fc3a5391e4111d6e013c55a63d61497a079c60fad8d4899c/..2019_08_16_01_23_05.253730078/ssh-privatekey
$ kubectl exec -it --namespace=banco workspacehxpkv5npvvllwbwo.dockerimage-7f569c8849-psvdg cat /etc/ssh/ssh_config
Defaulting container name to container.
Use 'kubectl describe pod/workspacehxpkv5npvvllwbwo.dockerimage-7f569c8849-psvdg -n banco' to see all of the containers in this pod.
host ssh.dev.azure.com
IdentityFile /etc/ssh/470aada635507690fc3a5391e4111d6e013c55a63d61497a079c60fad8d4899c/ssh-privatekey
host github.com
IdentityFile /etc/ssh/3aeb002460381c6f258e8395d3026f571f0d9a76488dcd837639b13aed316560/ssh-privatekey

@monaka monaka marked this pull request as ready for review August 16, 2019 01:30
Signed-off-by: Masaki Muranaka <monaka@monami-ya.com>
@monaka monaka changed the title Use the hashed name for the path to each ssh-secret. (eclse#14151). Use the hashed name for the path to each ssh-secret. (eclipse#14151). Aug 16, 2019
@monaka monaka added this to the 7.x milestone Aug 18, 2019
@monaka
Copy link
Member Author

monaka commented Aug 18, 2019

After inspecting code, I rethink my concern should be fixed by apply some patches to the /api/ssh endpoint. I close this and will make new issue.

@l0rd
Copy link
Contributor

l0rd commented Aug 22, 2019

Removing label do-not-merge/hold. @monaka can you rebase please?

@monaka
Copy link
Member Author

monaka commented Aug 27, 2019

@l0rd I see. IMO, It's better to add some more patches to this PR. I'll do it later.

@vinokurig
Copy link
Contributor

When creating a workspace from dashboard an error occurs:

Failed
Devfile schema validation failed. Error: (/metadata/generateName):The object must not have a property whose name is "generateName".

screenshot-che-che 192 168 39 246 nip io-2019 09 27-15-01-20

@vinokurig
Copy link
Contributor

The DevFile error is gone when I merged master, but I see an error when starting a workspace If an SSH key with name $name is generated:

Error: Failed to run the workspace: "Failure executing: POST at: https://10.96.0.1/api/v1/namespaces/che/secrets. Message: Secret "workspaceemenqvt9e8bg51us-$name" is invalid: metadata.name: Invalid value: "workspaceemenqvt9e8bg51us-$name": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'). Received status: Status(apiVersion=v1, code=422, details=StatusDetails(causes=[StatusCause(field=metadata.name, message=Invalid value: "workspaceemenqvt9e8bg51us-$name": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'), reason=FieldValueInvalid, additionalProperties={})], group=null, kind=Secret, name=workspaceemenqvt9e8bg51us-$name, retryAfterSeconds=null, uid=null, additionalProperties={}), kind=Status, message=Secret "workspaceemenqvt9e8bg51us-$name" is invalid: metadata.name: Invalid value: "workspaceemenqvt9e8bg51us-$name": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'), metadata=ListMeta(_continue=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=Invalid, status=Failure, additionalProperties={})."

screenshot-che-che 192 168 39 252 nip io-2019 09 27-16-32-28

@amisevsk
Copy link
Contributor

@vinokurig The issue with pasting the devfile is the .metadata.generateName field; I've opened #14695 to track that issue.

@vparfonov
Copy link
Contributor

Hello, @monaka! Do you have plan continue work on this PR?

@monaka
Copy link
Member Author

monaka commented Nov 15, 2019

@vparfonov Thanks for the heads up. This PR is no longer required after #14950 applied.
Close this as 'won't fix'.

@monaka monaka closed this Nov 15, 2019
@monaka monaka deleted the pr-use-hashed-name-for-pathes-to-privkey branch November 15, 2019 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants