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

Crash on swap with multi-line env vars (container method) #301

Closed
donaldguy opened this issue Sep 25, 2017 · 5 comments
Closed

Crash on swap with multi-line env vars (container method) #301

donaldguy opened this issue Sep 25, 2017 · 5 comments

Comments

@donaldguy
Copy link

@donaldguy donaldguy commented Sep 25, 2017

What were you trying to do?

run telepresence with --swap-deployment in a pod where an existing env var is a multi-line string. In particular it is a json struct as needed by any node app built using the meteor framework https://docs.meteor.com/api/core.html#Meteor-settings

(the version run locally with telepresence will in fact ignore it in favor of a file; but the swapped out version needs the env var; obviously in this case in can be worked around by using e.g. jq -c rather than cat when loading the file contents into the ConfigMap the deployment is sourcing the value from)

What did you expect to happen?

The telepresence proxy container to run and proxy the variable, regardless of whether it is a single line or multiple lines

What happened instead?

telepresence crashed.

In general the approach of kubectl exec ... env is somewhat suspect as env isn't strictly guaranteed to be present.

I would suggest as an alternative looking at the contents of /proc/1/environ - which has the following advantages (see also: https://unix.stackexchange.com/a/29132):

  1. its null-seperated
  2. it includes changes the process has made to its own environment since starting (this one could be argued either way, but for swap seems mostly like a positive)
  3. its more guarenteed to be there as its a kernel api

Automatically included information

Command line: ['/usr/local/bin/telepresence', '--also-proxy', 'dev-mongo-mongodb-replicaset-0.dev-mongo-mongodb-replicaset.default.svc.cluster.local', '--swap-deployment', '$a_meteor_based_node_app_deployment', '--expose', '3434:8080', '--run', 'bash', '-c', 'meteor -p 3434 --settings private/settings.json']
Version: 0.65
Python version: 3.6.2 (default, Jul 17 2017, 16:44:45) [GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.42)]
kubectl version: Client Version: v1.7.5
oc version: (error: [Errno 2] No such file or directory: 'oc')
OS: Darwin stickers.local 16.7.0 Darwin Kernel Version 16.7.0: Thu Jun 15 17:36:27 PDT 2017; root:xnu-3789.70.16~2/RELEASE_X86_64 x86_64
Traceback:

Traceback (most recent call last):
  File "/usr/local/bin/telepresence", line 257, in call_f
    return f(*args, **kwargs)
  File "/usr/local/bin/telepresence", line 2314, in go
    runner, args
  File "/usr/local/bin/telepresence", line 1493, in start_proxy
    env = get_env_variables(runner, remote_info, args.context)
  File "/usr/local/bin/telepresence", line 720, in get_env_variables
    remote_info.container_name
  File "/usr/local/bin/telepresence", line 707, in _get_remote_env
    key, value = line.split("=", 1)
ValueError: not enough values to unpack (expected 2, got 1)

Logs:

', '-oStrictHostKeyChecking=no', '-oUserKnownHostsFile=/dev/null', '-p', '60698', 'telepresence@localhost', '/bin/true'],)... Running: (['ssh', '-F', '/dev/null', '-q', '-oStrictHostKeyChecking=no', '-oUserKnownHostsFile=/dev/null', '-p', '60698', 'telepresence@localhost', '/bin/true'],)... Running: (['ssh', '-F', '/dev/null', '-q', '-oStrictHostKeyChecking=no', '-oUserKnownHostsFile=/dev/null', '-p', '60698', 'telepresence@localhost', '/bin/true'],)... Forwarding from 127.0.0.1:60698 -> 8022
Forwarding from [::1]:60698 -> 8022
Handling connection for 60698
 ran!
Running: (['ssh', '-N', '-oServerAliveInterval=1', '-oServerAliveCountMax=10', '-F', '/dev/null', '-q', '-oStrictHostKeyChecking=no', '-oUserKnownHostsFile=/dev/null', '-p', '60698', 'telepresence@localhost', '-R', '*:8080:127.0.0.1:3434'],)...Running: (['kubectl', '--context', 'minikube', '--namespace', 'default', 'exec', 'dev-factory-3764790423-k11bj', '--container', 'factory', 'env'],)...Handling connection for 60698
 ran!

@donaldguy donaldguy changed the title Telepresence crashes when pod has multi-line env vars Telepresence crashes on swap when target container has multi-line env vars Sep 25, 2017
@donaldguy
Copy link
Author

@donaldguy donaldguy commented Sep 25, 2017

I think this will also be hit for any env var loaded from a secret or configmap defined with a

key: |
     value

syntax even if the value itself is a single line? (at least with a helm package using a {{ .Values.thing | indent 4 }}

@plombardi89
Copy link
Contributor

@plombardi89 plombardi89 commented Sep 26, 2017

Thanks for the report @donaldguy. Someone will take a look at this shortly. It definitely should not crash the program.

@ark3
Copy link
Contributor

@ark3 ark3 commented Nov 7, 2017

@donaldguy Does release 0.68 or later fix this issue for you?

@ark3
Copy link
Contributor

@ark3 ark3 commented Jan 12, 2018

FWIW, this will still not work with the container method because Docker does not support multi-line values in its env-file format. Perhaps we can do something like this comment suggests. In short, for multi-line values, we set the key/value pairs in the calling environment and then pass -e KEY to the docker command.

@plombardi89 plombardi89 added this to Bugs in Roadmap Feb 21, 2018
@richarddli richarddli added this to Robustness in T Roadmap (v2) Feb 21, 2018
@rhs rhs added this to Bug in Buckets Mar 8, 2018
@ark3
Copy link
Contributor

@ark3 ark3 commented Mar 9, 2018

We have removed testing of our fixes to this. See tests/parameterize_utils.py:

        # XXXX
        # Container method doesn't support multi-line environment variables.
        # Therefore disable this for all methods or the container tests all
        # fail...
        # XXXX

Let's make that test work correctly when we fix this.

@ark3 ark3 changed the title Telepresence crashes on swap when target container has multi-line env vars Crash on swap with multi-line env vars (container method) Mar 28, 2018
@ark3 ark3 removed their assignment Mar 28, 2018
@ark3 ark3 added this to To Do in Container UX via automation Apr 6, 2018
@ark3 ark3 closed this in 8a96bd2 May 11, 2018
Container UX automation moved this from To Do to Done May 11, 2018
Roadmap automation moved this from Bugs to Completed May 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Roadmap
  
Completed
T Roadmap (v2)
Robustness
Container UX
  
Done
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.