[ADD] ssh key text as env variable #31

vrenaville · 2021-03-26T12:09:20Z

Purpose
We need to use an old rancher, that do not support secrets deploy with CLI, we want to use environment variable instead
Before
ssh key was only available with secret or volume
Before
ssh key can be define with a secret, volume or env variable

jnovack

Please make the changes requested.

Additionally, please add an additional test to validate this configuration.

jnovack · 2021-03-26T12:28:55Z

README.md

@@ -190,6 +190,12 @@ the `known_hosts` file is provided.  This can help avoid issues for hosts with
 dynamic IP addresses, but removes some additional protection against DNS
 spoofing attacks.  Host IP Checking is enabled by default.

+#### ENV_SSH_KEY


Standard _FILE convention for VARIABLE is VARIABLE_FILE. Please use VARIABLE, not ENV_VARIABLE.

Additionally, VARIABLE_FILE is an override for VARIABLE, not VARIABLE being an override for VARIABLE_FILE. Thus, if VARIABLE_FILE exists, it shall override VARIABLE, always.

jnovack · 2021-03-26T12:34:58Z

rootfs/entrypoint.sh

@@ -1,8 +1,11 @@
 #!/usr/bin/dumb-init /bin/sh
 source version.sh
-
+if [ -n "${ENV_SSH_KEY}" ]; then


jnovack · 2021-03-26T12:35:21Z

rootfs/entrypoint.sh

+if [ -n "${ENV_SSH_KEY}" ]; then
+    echo "$ENV_SSH_KEY" > /id_rsa/custom_rsa
+    KEY_FILE=/id_rsa/custom_rsa
+else KEY_FILE=${SSH_KEY_FILE:=/id_rsa}


split into two lines and indent

vrenaville · 2021-03-26T14:46:17Z

@jnovack

Thanks for your comments !
I have update my pr regarding your returns.
Let me know if it's not the right way

Thanks for your help and work

jnovack

Please do not forget to run your test before committing.

jnovack · 2021-03-26T15:24:33Z

README.md

+#### SSH_KEY
+
+You can specify the SSH key using Environnement variable, please note
+that SSH_KEY_FILE parameter will be ignored


I believe it is the opposite. If SSH_KEY_FILE is specified, SSH_KEY is ignored.

_FILE takes precedence (unless I'm wrong here, in which case show me an app where non _FILE takes precedence over _FILE).

jnovack · 2021-03-26T16:26:19Z

rootfs/entrypoint.sh

+    SSH_KEY_FILE=/id_rsa/custom_rsa
+else
+    KEY_FILE=${SSH_KEY_FILE:=/id_rsa}
+fi


Your order is backwards here. Since SSH_KEY_FILE overrides SSH_KEY, the logic has to flow differently.

Why set KEY_FILE twice? KEY_FILE is set to SSH_KEY_FILE if and only if it exists, use that logic.

jnovack · 2021-03-26T16:28:55Z

rootfs/entrypoint.sh

@@ -1,8 +1,14 @@
 #!/usr/bin/dumb-init /bin/sh
 source version.sh
-
+if [ -n "${SSH_KEY}" ]; then
+    echo "$SSH_KEY" > /id_rsa/custom_rsa


There should be no reason to have an extra file on disk.

We've already defined /id_rsa as the custom to file to use, (a) why create another, and (b) why use a completely non-standard directory?

jnovack · 2021-03-26T16:33:41Z

docker-compose.test.yml

@@ -80,6 +80,45 @@ services:
      - SSH_TARGET_PORT=22
      - SSH_TUNNEL_PORT=11111
      - SSH_KEY_FILE=/opt/id_rsa
+      - SSH_KEY= |-


One always overrides the other. You cannot test both in the same container. In fact, you need a third test to test that one does properly override the other.

vrenaville · 2021-03-30T12:19:13Z

Thanks for you time, I will work on it the week , not a lot a spare time actually

gurneyalex · 2021-10-08T06:49:21Z

hello @jnovack, I'm a colleague of @vrenaville who's been busy on other topics lately and was not able to follow up on this PR. I'll take over from him as this issue is still important for us (we are migrating away from Rancher but this migration won't be complete for some time, and so we need to patch our way around to keep this tunnel up for one of our customers. I'll update the PR next week. Thanks for you patience.

gurneyalex · 2021-10-13T14:09:54Z

I just updated that PR, taking your remarks into account.

vrenaville · 2021-10-19T09:47:44Z

@gurneyalex Thanks for your time and work, It seems LGTM to me

arsa-dev · 2023-11-09T18:37:57Z

Hi @jnovack, I'm also interested on this feature to be merged, there is anything else to do or where I can help after latest @gurneyalex changes, so this can be merged?

gurneyalex · 2023-11-15T12:22:19Z

@arsa-dev we lost interest in getting this merged, and I won't be touching the PR anymore. Feel free to fork our code and try to resubmit the PR, maybe you will have success.

jnovack · 2024-05-28T15:59:46Z

Hi @jnovack, I'm also interested on this feature to be merged, there is anything else to do or where I can help after latest @gurneyalex changes, so this can be merged?

If you'd like to help, the huge thing is the tests are not passing. This does not look close to functional. make docker-test fails, so I cannot merge a PR that actively breaks other user's environments, would not be responsible.

For some reason, there was an rm added which fails, and then a simple test of connection gets connection refused, so something else failed.

target-1          | rm: can't remove '/root/.ssh/target.txt': No such file or directory
remote-1          | rm: can't remove '/root/.ssh/remote.txt': No such file or directory
target-1          | sed: unmatched '/'
remote-1          | sed: unmatched '/'
remote-1          | sed: unmatched '/'
remote-1          | sed: unmatched '/'
target-1          | chpasswd: password for 'root' changed
remote-1          | chpasswd: password for 'root' changed
local-1           | user/repo dirty revision local built 1970-01-01T00:00:00Z
local-with-env-1  | user/repo dirty revision local built 1970-01-01T00:00:00Z
local-with-env-1  | Agent pid 10
local-1           | Agent pid 8
local-1           | Identity added: (stdin) (testing)
local-with-env-1  | Identity added: (stdin) (testing)
local-with-env-1  | [INFO ] Using autossh 1.4g
local-with-env-1  | [INFO ] Tunneling 203.0.113.10:11112  on root@203.0.113.10:22  to 203.0.113.100:22
local-with-env-1  | [INFO ] # autossh -M 0 -N -o StrictHostKeyChecking=no -o ServerAliveInterval=10 -o ServerAliveCountMax=3 -o ExitOnForwardFailure=yes -t -t -R 203.0.113.10:11112:203.0.113.100:22 -p 22 root@203.0.113.10
local-1           | [INFO ] Using autossh 1.4g
local-1           | [INFO ] Tunneling 203.0.113.10:11111  on root@203.0.113.10:22  to 203.0.113.100:22
local-1           | [INFO ] # autossh -M 0 -N -o StrictHostKeyChecking=no -o ServerAliveInterval=10 -o ServerAliveCountMax=3 -o ExitOnForwardFailure=yes -t -t -R 203.0.113.10:11111:203.0.113.100:22 -p 22 root@203.0.113.10
local-1           | ssh: connect to host 203.0.113.10 port 22: Connection refused
local-with-env-1  | ssh: connect to host 203.0.113.10 port 22: Connection refused
remote-1          | ssh-keygen: generating new host keys: RSA ECDSA ED25519
remote-1          | Server listening on 0.0.0.0 port 22.
remote-1          | Server listening on :: port 22.
local-1 exited with code 1

arsa-dev · 2024-05-30T19:54:47Z

Hi @jnovack, thanks you for your time reviewing again this old PR. Unfortunately I'm currently really busy but having this idea working and released would be great! I'll really try to manage it to free some time and fork this PR, review the reason behind the failing tests and resubmit in a new PR.

jnovack requested changes Mar 26, 2021

View reviewed changes

vrenaville force-pushed the env_ssh_key branch from 68b8cb0 to 6746fff Compare March 26, 2021 14:44

jnovack requested changes Mar 26, 2021

View reviewed changes

gurneyalex force-pushed the env_ssh_key branch from 6746fff to d589794 Compare October 13, 2021 14:09

[ADD] ssh key text as env variable

2ed374f

gurneyalex force-pushed the env_ssh_key branch from d589794 to 2ed374f Compare October 25, 2021 08:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] ssh key text as env variable #31

[ADD] ssh key text as env variable #31

vrenaville commented Mar 26, 2021

jnovack left a comment

jnovack Mar 26, 2021 •

edited

Loading

jnovack Mar 26, 2021

jnovack Mar 26, 2021

vrenaville commented Mar 26, 2021

jnovack left a comment

jnovack Mar 26, 2021

jnovack Mar 26, 2021

jnovack Mar 26, 2021

jnovack Mar 26, 2021

vrenaville commented Mar 30, 2021

gurneyalex commented Oct 8, 2021

gurneyalex commented Oct 13, 2021

vrenaville commented Oct 19, 2021

arsa-dev commented Nov 9, 2023

gurneyalex commented Nov 15, 2023

jnovack commented May 28, 2024

arsa-dev commented May 30, 2024

[ADD] ssh key text as env variable #31

Are you sure you want to change the base?

[ADD] ssh key text as env variable #31

Conversation

vrenaville commented Mar 26, 2021

jnovack left a comment

Choose a reason for hiding this comment

jnovack Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

jnovack Mar 26, 2021

Choose a reason for hiding this comment

jnovack Mar 26, 2021

Choose a reason for hiding this comment

vrenaville commented Mar 26, 2021

jnovack left a comment

Choose a reason for hiding this comment

jnovack Mar 26, 2021

Choose a reason for hiding this comment

jnovack Mar 26, 2021

Choose a reason for hiding this comment

jnovack Mar 26, 2021

Choose a reason for hiding this comment

jnovack Mar 26, 2021

Choose a reason for hiding this comment

vrenaville commented Mar 30, 2021

gurneyalex commented Oct 8, 2021

gurneyalex commented Oct 13, 2021

vrenaville commented Oct 19, 2021

arsa-dev commented Nov 9, 2023

gurneyalex commented Nov 15, 2023

jnovack commented May 28, 2024

arsa-dev commented May 30, 2024

jnovack Mar 26, 2021 •

edited

Loading