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

Fix labels copying value from environment variables #1671

Merged
merged 3 commits into from
Mar 19, 2019

Commits on Mar 19, 2019

  1. Fix labels copying value from environment variables

    This patch fixes a bug where labels use the same behavior as `--env`, resulting
    in a value to be copied from environment variables with the same name as the
    label if no value is set (i.e. a simple key, no `=` sign, no value).
    
    An earlier pull request addressed similar cases for `docker run`;
    2b17f4c, but this did not address the
    same situation for (e.g.) `docker service create`.
    
    Digging in history for this bug, I found that use of the `ValidateEnv`
    function for  labels was added in the original implementation of the labels feature in
    moby/moby@abb5e9a#diff-ae476143d40e21ac0918630f7365ed3cR34
    
    However, the design never intended it to expand environment variables,
    and use of this function was either due to either a "copy/paste" of the
    equivalent `--env` flags, or a misunderstanding (the name `ValidateEnv` does
    not communicate that it also expands environment variables), and the existing
    `ValidateLabel` was designed for _engine_ labels (which required a value to
    be set).
    
    Following the initial implementation, other parts of the code followed
    the same (incorrect) approach, therefore leading the bug to be introduced
    in services as well.
    
    This patch:
    
    - updates the `ValidateLabel` to match the expected validation
      rules (this function is no longer used since 31dc5c0),
      and the daemon has its own implementation)
    - corrects various locations in the code where `ValidateEnv` was used instead of `ValidateLabel`.
    
    Before this patch:
    
    ```bash
    export SOME_ENV_VAR=I_AM_SOME_ENV_VAR
    docker service create --label SOME_ENV_VAR --tty --name test busybox
    
    docker service inspect --format '{{json .Spec.Labels}}' test
    {"SOME_ENV_VAR":"I_AM_SOME_ENV_VAR"}
    ```
    
    After this patch:
    
    ```bash
    export SOME_ENV_VAR=I_AM_SOME_ENV_VAR
    docker service create --label SOME_ENV_VAR --tty --name test busybox
    
    docker container inspect --format '{{json .Config.Labels}}' test
    {"SOME_ENV_VAR":""}
    ```
    
    Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
    thaJeztah committed Mar 19, 2019
    Configuration menu
    Copy the full SHA
    f2424bd View commit details
    Browse the repository at this point in the history
  2. Add back validation for invalid label values on containers

    This adds validation to `docker container run` / `docker container create`;
    
    Validation of labels provided through flags was removed in 31dc5c0,
    after the validation was changed to fix labels without values, and to prevent
    labels from being expanded with environment variables in 2b17f4c
    
    However, now empty label names from _files_ (`--label-file`) followed different
    validation rules than labels passed through `--label`.
    
    This patch adds back minimal validation for labels passed through the command-line
    
    Before this patch:
    
    ```bash
    docker container create \
      --name label \
      --label==with-leading-equal-sign \
      --label=without-value \
      --label=somelabel=somevalue \
      --label "  =  " \
      --label=with-quotes-in-value='{"foo"}' \
      --label='with"quotes"in-key=test' \
      busybox
    
    docker container inspect --format '{{json .Config.Labels}}' label
    ```
    
    ```json
    {
      "": "with-leading-equal-sign",
      "  ": "  ",
      "somelabel": "somevalue",
      "with\"quotes\"in-key": "test",
      "with-quotes-in-value": "{\"foo\"}",
      "without-value": ""
    }
    ```
    
    After this patch:
    
    ```bash
    docker container create \
      --name label \
      --label==with-leading-equal-sign \
      --label=without-value \
      --label=somelabel=somevalue \
      --label "  =  " \
      --label=with-quotes-in-value='{"foo"}' \
      --label='with"quotes"in-key=test' \
      busybox
    
    invalid argument "=with-leading-equal-sign" for "-l, --label" flag: invalid label format: "=with-leading-equal-sign"
    ```
    
    Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
    thaJeztah committed Mar 19, 2019
    Configuration menu
    Copy the full SHA
    b5d0d17 View commit details
    Browse the repository at this point in the history
  3. Tweak validation messages

    Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
    thaJeztah committed Mar 19, 2019
    Configuration menu
    Copy the full SHA
    e5702e0 View commit details
    Browse the repository at this point in the history