rkt / rkt

[Project ended] rkt is a pod-native container engine for Linux. It is composable, secure, and built on standards.
Apache License 2.0
8.82k stars 883 forks source link

rkt can't handle environment variable names with periods #3532

Open blalor opened 7 years ago

blalor commented 7 years ago

Environment

rkt Version: 1.22.0
appc Version: 0.8.9
Go Version: go1.7.3
Go OS/Arch: linux/amd64
Features: -TPM +SDJOURNAL
--
Linux 4.9.3-1.el7.elrepo.x86_64 x86_64
--
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/"
BUG_REPORT_URL="https://bugs.centos.org/"

CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"
--
systemd 219
+PAM +AUDIT +SELINUX +IMA -APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ -LZ4 -SECCOMP +BLKID +ELFUTILS +KMOD +IDN

What did you do?

rkt run \
    --debug \
    --insecure-options=ondisk,image \
    docker://alpine \
    --set-env=network.publish_host=10.112.3.42 \
    --exec=/bin/sh -- \
    -c 'env | grep network'

What did you expect to see?

[ 7065.940611] alpine[5]: network.publish_host=10.112.3.42

What did you see instead?

image: using image from local store for image name coreos.com/rkt/stage1-coreos:1.22.0
image: using image from local store for url docker://alpine
stage0: Preparing stage1
stage0: Writing image manifest
stage0: Loading image sha512-9056a5a4b9a82be12dd505dca3b6f6f42b8cb0772f053d087e670e615c57bd07
stage0: Writing image manifest
run: error setting up stage0
  └─error marshalling pod manifest
    └─json: error calling MarshalJSON for type schema.PodManifest: json: error calling MarshalJSON for type schema.AppList: json: error calling MarshalJSON for type *types.App: environment variable does not have valid identifier "network.publish_host"
squeed commented 7 years ago

Hah, good find! I'll clean this up.

jonboulle commented 7 years ago

Hmm, that is unfortunate. Arrived via https://github.com/appc/spec/commit/08f169c5ba43ae032fe7bf36aa6a10fe86e35afa#diff-916b3e1e005fd96e3a0546715235477dR523

Are you passing this through to elasticsearch via a script or something..?

squeed commented 7 years ago

Looks like Linux allows anything except nulls in environment variables.

blalor commented 7 years ago

@jonboulle I am. It's a custom script, and I came up with a workaround of encoding dots as ES:

## configure from env vars; cribbed from
## https://github.com/elastic/elasticsearch-docker/blob/af148c7f486ef6e78718da0f9e2a314851b6dea6/build/elasticsearch/bin/es-docker
## encoding '.' as 'ES'
while IFS='=' read -r envvar_key envvar_value ; do
    # Elasticsearch env vars need to have at least two ES-separated lowercase
    # words, e.g. `clusterESname`
    if [[ "$envvar_key" =~ ^[a-z]+ES[a-z]+ ]] ; then
        if [[ ! -z $envvar_value ]]; then
            newopt="-E${envvar_key//ES/.}=${envvar_value}"

            echo "adding param: '${newopt}'"
            launch_cmd=("${launch_cmd[@]}" "${newopt}")
        fi
    fi
done < <( env )

So cluster.name becomes clusterESname. It's ugly. And a better solution would be to pass these as arguments to the script instead of environment variables.

While I think adopting the IEEE spec for environment variables is well-intentioned, it goes against Postel's law. :-)

blalor commented 7 years ago

I'll also point out that you can't directly access environment variables with dots in them via bash. But this isn't the first time I've had to do something like this, so from a user's perspective, it'd be preferable to have rkt (and the spec, I guess) be a little more forgiving.

jonboulle commented 7 years ago

Sure, I think we can just loosen this up.

s-urbaniak commented 7 years ago

This needs an appc bump, which did not happen yet, hence bumping to the next release.

lucab commented 7 years ago

This is not on our radar at the moment, but it should be easy enough to tackle for external contributors. Contributions welcome!

kfirufk commented 7 years ago

welp.. no updates ?