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

dot character in environment variable name yields "invalid value" #2707

Closed
mike-hogan opened this issue Dec 2, 2014 · 22 comments
Closed

dot character in environment variable name yields "invalid value" #2707

mike-hogan opened this issue Dec 2, 2014 · 22 comments
Labels
kind/design Categorizes issue or PR as related to design. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@mike-hogan
Copy link

I modified the pod json from here https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/getting-started-guides/aws-coreos.md to have this in it:

  "containers": [{
    "name": "hello",
    "env": [
            {
              "name": 'oms.port',
              "value": 9000,
            }],
    "image": "quay.io/kelseyhightower/hello",
    "ports": [{
      "containerPort": 80,
      "hostPort": 80
    }]
  }]

and when I do kubecfg -c pod.json create pods, I get

F1202 08:48:01.907263 04825 kubecfg.go:403] Got request error: pod "hello" is invalid: desiredState.manifest.containers[0].env[0].name: invalid value 'oms.port'

I am using Kubernetes 0.5.4

@erictune
Copy link
Member

erictune commented Dec 2, 2014

The following doesn't work in bash:

$ oms.port=9000
-bash: oms.port=9000: command not found

so it doesn't work in kubernetes either.

@erictune erictune changed the title dot character in environment variable value yields "invalid value" dot character in environment variable name yields "invalid value" Dec 2, 2014
@erictune
Copy link
Member

erictune commented Dec 2, 2014

Please feel free to reopen if I've misunderstood.

@erictune erictune closed this as completed Dec 2, 2014
@wstrange
Copy link
Contributor

wstrange commented Mar 7, 2017

The issue (which is still present in Kube 1.5.3) is that a dot is not allowed in the podspec for
environment variables names.

Some systems (elasticsearch for example) use dotted env var names. Example:

 - name: transport.host
      value: 127.0.0.1

The above fails with

Deployment.extensions "es-es-kibana" is invalid: spec.template.spec.containers[0].env[2].name: Invalid value: "transport.host": must match the regex [A-Za-z_][A-Za-z0-9_]*

@keegancsmith
Copy link
Contributor

@erictune Funnily enough this hit us as well today for elasticsearch. Docker had this restriction and reverted it since POSIX pretty clearly states that you should tolerate env strings you don't understand. See more in the relevant docker issue moby/moby#16585

@timoreimann
Copy link
Contributor

timoreimann commented Mar 9, 2017

With the explanation from here quoted on the referenced Docker issue, it seems to me that Kubernetes is similar to Docker in the sense that it's just an intermediary that should always pass environment variables through to the application even when they contain "illegal characters"; the reason being that applications may not be subject to the same restrictions as bash.

@erictune Do you think this is worth creating a new issue or reopening this one?

@yongzhang
Copy link

+1
We have the same issue to start elasticsearch with Kubernetes.

@AlmogBaku
Copy link
Member

AlmogBaku commented Apr 22, 2017

Since docker have changed it, and large-scaled applications such as ElasticSearch requires it, I guess it worth reconsideration.

/reopen

@k8s-ci-robot
Copy link
Contributor

@AlmogBaku: you can't re-open an issue unless you authored it or you are assigned to it.

In response to this:

Since docker have changed it, and large-scaled applications such as ElasticSearch requires it, I guess it worth reconsideration.

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot reopened this Apr 22, 2017
dliappis added a commit to elastic/elasticsearch-docker that referenced this issue Apr 26, 2017
Shells allow a specific character class for environment variables:
`[a-zA-Z_]+[a-zA-Z0-9_]*`.  While Docker and POSIX[1] can deal with
dots in environment names, Kubernetes is currently also affected[2] by
this and as Elasticsearch settings contain dots, this creates
frustration for users who prefer customizing Elasticsearch using
environment vars.

This commit offers the possibility for users to define settings using
a schema prefixed with `ess__` and `__` instead of dots, like
`ess__cluster__name=my-es-on-docker`.

As Elasticsearch also uses _ in some env vars (like
`bootstrap.memory_lock`) and other env vars like `ES_JAVA_OPTS` are
already prefixed with `ES_` we have to resort to the prefix `ess__`
and using `__` as a delimiter.

[1]
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
[2]
kubernetes/kubernetes#2707 (comment)
@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label May 31, 2017
@k8s-github-robot
Copy link

@mike-hogan There are no sig labels on this issue. Please add a sig label by:
(1) mentioning a sig: @kubernetes/sig-<team-name>-misc
(2) specifying the label manually: /sig <label>

Note: method (1) will trigger a notification to the team. You can find the team list here.

@timoreimann
Copy link
Contributor

I'd like to take a stab at relaxing Kubernetes' validation logic so that dotted environment variables are allowed.

If there's anything I should do or any design work that's needed up front prior to shooting out a PR, please let me know.

@0xmichalis
Copy link
Contributor

@kubernetes/sig-node-proposals

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. kind/design Categorizes issue or PR as related to design. labels Jun 2, 2017
@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jun 2, 2017
@mtaufen
Copy link
Contributor

mtaufen commented Jun 6, 2017

It sounds reasonable to me. I'll throw one point out there though: relaxing validation can be tricky because you never know who's come to rely on existing validation - e.g. does anyone rely on Kubernetes preventing dotted env var names? I personally don't think this should stop us from making this specific change, but opinions may differ from mine.

@timoreimann I would just make a PR and see what people think.

@timoreimann
Copy link
Contributor

thanks @mtaufen -- will follow up with a PR and see what folks have to say.

@martinmosegaard
Copy link

A workaround for Elasticsearch until this is fixed is to use command arguments in your Kubernetes deployment spec like so:

spec:
  containers:
  - image: docker.elastic.co/elasticsearch/elasticsearch:5.4.2
    ...
    command: ["bin/elasticsearch"]
    args: ["-Ehttp.host=0.0.0.0", "-Etransport.host=127.0.0.1"]

As documented under configuration here:
https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html

@timoreimann
Copy link
Contributor

Super quick update: I've started work on this. Should hopefully be able to submit a PR soonish.

@dliappis
Copy link

dliappis commented Aug 2, 2017

@martinmosegaard Is correct in that currently a workaround to configuring the Elasticsearch Docker image on Kubernetes (on top of bind mounting a config file or using a customized image) is by overriding CMD, i.e. option D in the Elasticsearch Docker documentation.

k8s-github-robot pushed a commit that referenced this issue Aug 8, 2017
…ictions

Automatic merge from submit-queue (batch tested with PRs 50208, 50259, 49702, 50267, 48986)

Relax restrictions on environment variable names.

Fixes #2707

The POSIX standard restricts environment variable names to uppercase letters, digits, and the underscore character in shell contexts only. For generic application usage, it is stated that all other characters shall be tolerated. (Reference [here](http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html), my prose reasoning [here](#2707 (comment)).)

This change relaxes the rules to some degree. Namely, we stop requiring environment variable names to be strict `C_IDENTIFIERS` and start permitting lowercase, dot, and dash characters.

Public container images using environment variable names beyond the shell-only context can benefit from this relaxation. Elasticsearch is one popular example.
@timoreimann
Copy link
Contributor

My PR just got merged. Once released, environment variable names with dots (and dashes) should generally be supported.

@webwurst
Copy link
Contributor

I see that the fix got merged at Aug 8, but when trying Kubernetes v1.7.7, released on Sep 29 the following error still shows up for me:

The Deployment "elasticsearch" is invalid: spec.template.spec.containers[0].env[1].name: Invalid value: "bootstrap.memory_lock": a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name',  or 'MY_NAME',  or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')

In which Kubernetes version is this issue supposed to be fixed?

@timoreimann
Copy link
Contributor

@webwurst it'll only be available in 1.8.

@johnculviner
Copy link

If you are stuck on <1.8 for the time being this fixed it all for me with elasticsearch (the similar answer above didn't quite cut it) hope this helps someone

      - image: docker.elastic.co/elasticsearch/elasticsearch:6.2.2
...
        command: ["/usr/local/bin/docker-entrypoint.sh"]
        args: ["bin/elasticsearch", "-Ediscovery.type=single-node"]

@ObviousDWest
Copy link

ObviousDWest commented Jun 6, 2019

I just got burned trying to put override values from an application.properties file for a spring boot application into a configMap (in a deployment). It appears to have dotted keys stripped out before they are added as environment variables in the container (without any error/warning message). I don't know if the issue is with "env", "envFrom" or somewhere else. But given how prevalent spring boot is, and how it is designed to pick up settings from environment variables, it would be super useful if the could be passed through from a config map.
Running 1.14.

@ObviousDWest
Copy link

Sorry to necro this thread. My issue turned out to be because I used "sh" in the command, which strips dotted names. Switching to "bash" allowed the variables to pass through as expected.

Iristyle added a commit to puppetlabs/puppetdb that referenced this issue Jan 27, 2021
 - Both docker (per moby/moby#16585) and
   Kubernetes (per kubernetes/kubernetes#2707)
   support passing environment variables with dots as config, despite
   dot not technically being supported in POSIX.

   This is necessary to be able to specify hocon array values like
   FOO.0, FOO.1, etc now that the Hocon library is updated to
   typesafe/config 1.4.1 via puppetlabs/typesafe-config 0.2.0 /
   clj-parent 4.6.11

   /bin/sh will not add such variables to the environment, but /bin/bash
   will properly expose them, so that they may be consumed by
   puppetserver and hocon.

   NOTE: ElasticSearch is notorious for passing such values, and it also
   uses the tini init system, so it served as a good design to follow:

   https://github.com/elastic/elasticsearch/tree/master/distribution/docker
Iristyle added a commit to puppetlabs/puppetserver that referenced this issue Mar 29, 2021
 - Both docker (per moby/moby#16585) and
   Kubernetes (per kubernetes/kubernetes#2707)
   support passing environment variables with dots as config, despite
   dot not technically being supported in POSIX.

   This is necessary to be able to specify hocon array values like
   FOO.0, FOO.1, etc now that the Hocon library is updated to
   typesafe/config 1.4.1 via puppetlabs/typesafe-config 0.2.0 /
   clj-parent 4.6.11

   /bin/sh will not add such variables to the environment, but /bin/bash
   will properly expose them, so that they may be consumed by
   puppetserver and hocon.

   NOTE: ElasticSearch is notorious for passing such values, and it also
   uses the tini init system, so it served as a good design to follow:

   https://github.com/elastic/elasticsearch/tree/master/distribution/docker
Iristyle added a commit to puppetlabs/puppetserver that referenced this issue Mar 29, 2021
 - Both docker (per moby/moby#16585) and
   Kubernetes (per kubernetes/kubernetes#2707)
   support passing environment variables with dots as config, despite
   dot not technically being supported in POSIX.

   This is necessary to be able to specify hocon array values like
   FOO.0, FOO.1, etc now that the Hocon library is updated to
   typesafe/config 1.4.1 via puppetlabs/typesafe-config 0.2.0 /
   clj-parent 4.6.11

   /bin/sh will not add such variables to the environment, but /bin/bash
   will properly expose them, so that they may be consumed by
   puppetserver and hocon.

   NOTE: ElasticSearch is notorious for passing such values, and it also
   uses the tini init system, so it served as a good design to follow:

   https://github.com/elastic/elasticsearch/tree/master/distribution/docker
akhilerm pushed a commit to akhilerm/apimachinery that referenced this issue Sep 20, 2022
…ictions

Automatic merge from submit-queue (batch tested with PRs 50208, 50259, 49702, 50267, 48986)

Relax restrictions on environment variable names.

Fixes #2707

The POSIX standard restricts environment variable names to uppercase letters, digits, and the underscore character in shell contexts only. For generic application usage, it is stated that all other characters shall be tolerated. (Reference [here](http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html), my prose reasoning [here](kubernetes/kubernetes#2707 (comment)).)

This change relaxes the rules to some degree. Namely, we stop requiring environment variable names to be strict `C_IDENTIFIERS` and start permitting lowercase, dot, and dash characters.

Public container images using environment variable names beyond the shell-only context can benefit from this relaxation. Elasticsearch is one popular example.

Kubernetes-commit: 243e65516160fabf849dfe7d81352b9f87a42abf
binford2k pushed a commit to voxpupuli/container-puppetdb that referenced this issue Nov 1, 2022
 - Both docker (per moby/moby#16585) and
   Kubernetes (per kubernetes/kubernetes#2707)
   support passing environment variables with dots as config, despite
   dot not technically being supported in POSIX.

   This is necessary to be able to specify hocon array values like
   FOO.0, FOO.1, etc now that the Hocon library is updated to
   typesafe/config 1.4.1 via puppetlabs/typesafe-config 0.2.0 /
   clj-parent 4.6.11

   /bin/sh will not add such variables to the environment, but /bin/bash
   will properly expose them, so that they may be consumed by
   puppetserver and hocon.

   NOTE: ElasticSearch is notorious for passing such values, and it also
   uses the tini init system, so it served as a good design to follow:

   https://github.com/elastic/elasticsearch/tree/master/distribution/docker
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

No branches or pull requests