2014-03-10 17:10:23 -04:00
|
|
|
package opts
|
|
|
|
|
|
|
|
import (
|
2015-05-04 17:39:48 -04:00
|
|
|
"fmt"
|
2015-02-09 09:59:05 -05:00
|
|
|
"strings"
|
2014-03-10 17:10:23 -04:00
|
|
|
"testing"
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
|
2020-02-22 12:12:14 -05:00
|
|
|
"gotest.tools/v3/assert"
|
2014-03-10 17:10:23 -04:00
|
|
|
)
|
|
|
|
|
2014-07-09 17:47:55 -04:00
|
|
|
func TestValidateIPAddress(t *testing.T) {
|
|
|
|
if ret, err := ValidateIPAddress(`1.2.3.4`); err != nil || ret == "" {
|
|
|
|
t.Fatalf("ValidateIPAddress(`1.2.3.4`) got %s %s", ret, err)
|
2014-03-10 17:10:23 -04:00
|
|
|
}
|
|
|
|
|
2014-07-09 17:47:55 -04:00
|
|
|
if ret, err := ValidateIPAddress(`127.0.0.1`); err != nil || ret == "" {
|
|
|
|
t.Fatalf("ValidateIPAddress(`127.0.0.1`) got %s %s", ret, err)
|
2014-03-10 17:10:23 -04:00
|
|
|
}
|
|
|
|
|
2014-07-09 17:47:55 -04:00
|
|
|
if ret, err := ValidateIPAddress(`::1`); err != nil || ret == "" {
|
|
|
|
t.Fatalf("ValidateIPAddress(`::1`) got %s %s", ret, err)
|
2014-03-10 17:10:23 -04:00
|
|
|
}
|
|
|
|
|
2014-07-09 17:47:55 -04:00
|
|
|
if ret, err := ValidateIPAddress(`127`); err == nil || ret != "" {
|
|
|
|
t.Fatalf("ValidateIPAddress(`127`) got %s %s", ret, err)
|
2014-06-13 08:02:12 -04:00
|
|
|
}
|
|
|
|
|
2014-07-09 17:47:55 -04:00
|
|
|
if ret, err := ValidateIPAddress(`random invalid string`); err == nil || ret != "" {
|
|
|
|
t.Fatalf("ValidateIPAddress(`random invalid string`) got %s %s", ret, err)
|
2014-03-10 17:10:23 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
}
|
2014-02-07 11:48:14 -05:00
|
|
|
|
2015-05-04 17:39:48 -04:00
|
|
|
func TestMapOpts(t *testing.T) {
|
|
|
|
tmpMap := make(map[string]string)
|
2015-05-05 00:18:28 -04:00
|
|
|
o := NewMapOpts(tmpMap, logOptsValidator)
|
2015-05-04 17:39:48 -04:00
|
|
|
o.Set("max-size=1")
|
|
|
|
if o.String() != "map[max-size:1]" {
|
|
|
|
t.Errorf("%s != [map[max-size:1]", o.String())
|
|
|
|
}
|
|
|
|
|
|
|
|
o.Set("max-file=2")
|
|
|
|
if len(tmpMap) != 2 {
|
|
|
|
t.Errorf("map length %d != 2", len(tmpMap))
|
|
|
|
}
|
|
|
|
|
|
|
|
if tmpMap["max-file"] != "2" {
|
|
|
|
t.Errorf("max-file = %s != 2", tmpMap["max-file"])
|
|
|
|
}
|
|
|
|
|
|
|
|
if tmpMap["max-size"] != "1" {
|
|
|
|
t.Errorf("max-size = %s != 1", tmpMap["max-size"])
|
|
|
|
}
|
|
|
|
if o.Set("dummy-val=3") == nil {
|
2016-11-14 04:01:17 -05:00
|
|
|
t.Error("validator is not being called")
|
2015-05-04 17:39:48 -04:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2015-07-12 04:33:30 -04:00
|
|
|
func TestListOptsWithoutValidator(t *testing.T) {
|
2014-08-09 21:13:44 -04:00
|
|
|
o := NewListOpts(nil)
|
|
|
|
o.Set("foo")
|
2014-10-06 21:54:52 -04:00
|
|
|
if o.String() != "[foo]" {
|
|
|
|
t.Errorf("%s != [foo]", o.String())
|
|
|
|
}
|
|
|
|
o.Set("bar")
|
|
|
|
if o.Len() != 2 {
|
|
|
|
t.Errorf("%d != 2", o.Len())
|
|
|
|
}
|
2015-07-12 04:33:30 -04:00
|
|
|
o.Set("bar")
|
|
|
|
if o.Len() != 3 {
|
|
|
|
t.Errorf("%d != 3", o.Len())
|
|
|
|
}
|
2014-10-06 21:54:52 -04:00
|
|
|
if !o.Get("bar") {
|
|
|
|
t.Error("o.Get(\"bar\") == false")
|
|
|
|
}
|
|
|
|
if o.Get("baz") {
|
|
|
|
t.Error("o.Get(\"baz\") == true")
|
|
|
|
}
|
|
|
|
o.Delete("foo")
|
2015-07-12 04:33:30 -04:00
|
|
|
if o.String() != "[bar bar]" {
|
|
|
|
t.Errorf("%s != [bar bar]", o.String())
|
|
|
|
}
|
|
|
|
listOpts := o.GetAll()
|
|
|
|
if len(listOpts) != 2 || listOpts[0] != "bar" || listOpts[1] != "bar" {
|
|
|
|
t.Errorf("Expected [[bar bar]], got [%v]", listOpts)
|
2014-10-06 21:54:52 -04:00
|
|
|
}
|
2015-07-12 04:33:30 -04:00
|
|
|
mapListOpts := o.GetMap()
|
|
|
|
if len(mapListOpts) != 1 {
|
|
|
|
t.Errorf("Expected [map[bar:{}]], got [%v]", mapListOpts)
|
|
|
|
}
|
|
|
|
|
2014-08-09 21:13:44 -04:00
|
|
|
}
|
|
|
|
|
2015-07-12 04:33:30 -04:00
|
|
|
func TestListOptsWithValidator(t *testing.T) {
|
|
|
|
// Re-using logOptsvalidator (used by MapOpts)
|
|
|
|
o := NewListOpts(logOptsValidator)
|
|
|
|
o.Set("foo")
|
2017-03-30 21:35:04 -04:00
|
|
|
if o.String() != "" {
|
|
|
|
t.Errorf(`%s != ""`, o.String())
|
2015-07-12 04:33:30 -04:00
|
|
|
}
|
|
|
|
o.Set("foo=bar")
|
2017-03-30 21:35:04 -04:00
|
|
|
if o.String() != "" {
|
|
|
|
t.Errorf(`%s != ""`, o.String())
|
2015-07-12 04:33:30 -04:00
|
|
|
}
|
|
|
|
o.Set("max-file=2")
|
|
|
|
if o.Len() != 1 {
|
|
|
|
t.Errorf("%d != 1", o.Len())
|
|
|
|
}
|
|
|
|
if !o.Get("max-file=2") {
|
|
|
|
t.Error("o.Get(\"max-file=2\") == false")
|
|
|
|
}
|
|
|
|
if o.Get("baz") {
|
|
|
|
t.Error("o.Get(\"baz\") == true")
|
|
|
|
}
|
|
|
|
o.Delete("max-file=2")
|
2017-03-30 21:35:04 -04:00
|
|
|
if o.String() != "" {
|
|
|
|
t.Errorf(`%s != ""`, o.String())
|
2015-07-12 04:33:30 -04:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2022-07-13 06:29:49 -04:00
|
|
|
//nolint:lll
|
2015-07-12 04:33:30 -04:00
|
|
|
func TestValidateDNSSearch(t *testing.T) {
|
2014-02-07 11:48:14 -05:00
|
|
|
valid := []string{
|
2014-06-26 07:03:23 -04:00
|
|
|
`.`,
|
2014-02-07 11:48:14 -05:00
|
|
|
`a`,
|
|
|
|
`a.`,
|
|
|
|
`1.foo`,
|
|
|
|
`17.foo`,
|
|
|
|
`foo.bar`,
|
|
|
|
`foo.bar.baz`,
|
|
|
|
`foo.bar.`,
|
|
|
|
`foo.bar.baz`,
|
|
|
|
`foo1.bar2`,
|
|
|
|
`foo1.bar2.baz`,
|
|
|
|
`1foo.2bar.`,
|
|
|
|
`1foo.2bar.baz`,
|
|
|
|
`foo-1.bar-2`,
|
|
|
|
`foo-1.bar-2.baz`,
|
|
|
|
`foo-1.bar-2.`,
|
|
|
|
`foo-1.bar-2.baz`,
|
|
|
|
`1-foo.2-bar`,
|
|
|
|
`1-foo.2-bar.baz`,
|
|
|
|
`1-foo.2-bar.`,
|
|
|
|
`1-foo.2-bar.baz`,
|
|
|
|
}
|
|
|
|
|
|
|
|
invalid := []string{
|
|
|
|
``,
|
2014-06-26 07:03:23 -04:00
|
|
|
` `,
|
|
|
|
` `,
|
2014-02-07 11:48:14 -05:00
|
|
|
`17`,
|
|
|
|
`17.`,
|
|
|
|
`.17`,
|
|
|
|
`17-.`,
|
|
|
|
`17-.foo`,
|
|
|
|
`.foo`,
|
|
|
|
`foo-.bar`,
|
|
|
|
`-foo.bar`,
|
|
|
|
`foo.bar-`,
|
|
|
|
`foo.bar-.baz`,
|
|
|
|
`foo.-bar`,
|
|
|
|
`foo.-bar.baz`,
|
2017-05-21 21:39:06 -04:00
|
|
|
`foo.bar.baz.this.should.fail.on.long.name.because.it.is.longer.thanisshouldbethis.should.fail.on.long.name.because.it.is.longer.thanisshouldbethis.should.fail.on.long.name.because.it.is.longer.thanisshouldbethis.should.fail.on.long.name.because.it.is.longer.thanisshouldbe`,
|
2014-02-07 11:48:14 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
for _, domain := range valid {
|
2015-07-12 04:33:30 -04:00
|
|
|
if ret, err := ValidateDNSSearch(domain); err != nil || ret == "" {
|
|
|
|
t.Fatalf("ValidateDNSSearch(`"+domain+"`) got %s %s", ret, err)
|
2014-02-07 11:48:14 -05:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
for _, domain := range invalid {
|
2015-07-12 04:33:30 -04:00
|
|
|
if ret, err := ValidateDNSSearch(domain); err == nil || ret != "" {
|
|
|
|
t.Fatalf("ValidateDNSSearch(`"+domain+"`) got %s %s", ret, err)
|
2014-02-07 11:48:14 -05:00
|
|
|
}
|
2015-07-12 04:33:30 -04:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestValidateLabel(t *testing.T) {
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
tests := []struct {
|
|
|
|
name string
|
|
|
|
value string
|
|
|
|
expectedErr string
|
|
|
|
}{
|
|
|
|
{
|
|
|
|
name: "empty",
|
2019-03-18 22:17:02 -04:00
|
|
|
expectedErr: `invalid label '': empty name`,
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "whitespace only ",
|
|
|
|
value: " ",
|
2019-03-18 22:17:02 -04:00
|
|
|
expectedErr: `invalid label ' ': empty name`,
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "whitespace around equal-sign",
|
|
|
|
value: " = ",
|
2019-03-18 22:17:02 -04:00
|
|
|
expectedErr: `invalid label ' = ': empty name`,
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "leading whitespace",
|
|
|
|
value: " label=value",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "whitespaces in key without value",
|
|
|
|
value: "this is a label without value",
|
2019-03-18 22:17:02 -04:00
|
|
|
expectedErr: `label 'this is a label without value' contains whitespaces`,
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "whitespaces in key",
|
|
|
|
value: "this is a label=value",
|
2019-03-18 22:17:02 -04:00
|
|
|
expectedErr: `label 'this is a label' contains whitespaces`,
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "whitespaces in value",
|
|
|
|
value: "label=a value that has whitespace",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "trailing whitespace in value",
|
|
|
|
value: "label=value ",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "leading whitespace in value",
|
|
|
|
value: "label= value",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "no value",
|
|
|
|
value: "label",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "no key",
|
|
|
|
value: "=label",
|
2019-03-18 22:17:02 -04:00
|
|
|
expectedErr: `invalid label '=label': empty name`,
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "empty value",
|
|
|
|
value: "label=",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "key value",
|
|
|
|
value: "key1=value1",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "double equal-signs",
|
|
|
|
value: "key1=value1=value2",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "multiple equal-signs",
|
|
|
|
value: "key1=value1=value2=value",
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "double quotes in key and value",
|
|
|
|
value: `key"with"quotes={"hello"}`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "double quotes around key and value",
|
|
|
|
value: `"quoted-label"="quoted value"`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "single quotes in key and value",
|
|
|
|
value: `key'with'quotes=hello'with'quotes`,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "single quotes around key and value",
|
|
|
|
value: `'quoted-label'='quoted value''`,
|
|
|
|
},
|
2015-07-12 04:33:30 -04:00
|
|
|
}
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
|
|
|
|
for _, tc := range tests {
|
2019-10-29 09:08:12 -04:00
|
|
|
tc := tc
|
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`;
2b17f4c8a8caad552025edb05a73db683fb8a5c6, 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
https://github.com/docker/docker/commit/abb5e9a0777469e64fe2c7ecfa66ea01083d2071#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 31dc5c0a9a8bdc11c7ad335aebb753ed527caa5a),
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>
2019-02-13 10:47:30 -05:00
|
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
|
|
val, err := ValidateLabel(tc.value)
|
|
|
|
if tc.expectedErr != "" {
|
|
|
|
assert.Error(t, err, tc.expectedErr)
|
|
|
|
return
|
|
|
|
}
|
|
|
|
assert.NilError(t, err)
|
|
|
|
assert.Equal(t, val, tc.value)
|
|
|
|
})
|
2015-07-12 04:33:30 -04:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2015-05-04 17:39:48 -04:00
|
|
|
func logOptsValidator(val string) (string, error) {
|
|
|
|
allowedKeys := map[string]string{"max-size": "1", "max-file": "2"}
|
|
|
|
vals := strings.Split(val, "=")
|
|
|
|
if allowedKeys[vals[0]] != "" {
|
|
|
|
return val, nil
|
|
|
|
}
|
|
|
|
return "", fmt.Errorf("invalid key %s", vals[0])
|
|
|
|
}
|
2015-12-10 18:35:10 -05:00
|
|
|
|
|
|
|
func TestNamedListOpts(t *testing.T) {
|
|
|
|
var v []string
|
|
|
|
o := NewNamedListOptsRef("foo-name", &v, nil)
|
|
|
|
|
|
|
|
o.Set("foo")
|
|
|
|
if o.String() != "[foo]" {
|
|
|
|
t.Errorf("%s != [foo]", o.String())
|
|
|
|
}
|
|
|
|
if o.Name() != "foo-name" {
|
|
|
|
t.Errorf("%s != foo-name", o.Name())
|
|
|
|
}
|
|
|
|
if len(v) != 1 {
|
|
|
|
t.Errorf("expected foo to be in the values, got %v", v)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestNamedMapOpts(t *testing.T) {
|
|
|
|
tmpMap := make(map[string]string)
|
|
|
|
o := NewNamedMapOpts("max-name", tmpMap, nil)
|
|
|
|
|
|
|
|
o.Set("max-size=1")
|
|
|
|
if o.String() != "map[max-size:1]" {
|
|
|
|
t.Errorf("%s != [map[max-size:1]", o.String())
|
|
|
|
}
|
|
|
|
if o.Name() != "max-name" {
|
|
|
|
t.Errorf("%s != max-name", o.Name())
|
|
|
|
}
|
|
|
|
if _, exist := tmpMap["max-size"]; !exist {
|
|
|
|
t.Errorf("expected map-size to be in the values, got %v", tmpMap)
|
|
|
|
}
|
|
|
|
}
|
2016-12-23 14:09:12 -05:00
|
|
|
|
|
|
|
func TestValidateMACAddress(t *testing.T) {
|
|
|
|
if _, err := ValidateMACAddress(`92:d0:c6:0a:29:33`); err != nil {
|
|
|
|
t.Fatalf("ValidateMACAddress(`92:d0:c6:0a:29:33`) got %s", err)
|
|
|
|
}
|
|
|
|
|
|
|
|
if _, err := ValidateMACAddress(`92:d0:c6:0a:33`); err == nil {
|
|
|
|
t.Fatalf("ValidateMACAddress(`92:d0:c6:0a:33`) succeeded; expected failure on invalid MAC")
|
|
|
|
}
|
|
|
|
|
|
|
|
if _, err := ValidateMACAddress(`random invalid string`); err == nil {
|
|
|
|
t.Fatalf("ValidateMACAddress(`random invalid string`) succeeded; expected failure on invalid MAC")
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestValidateLink(t *testing.T) {
|
|
|
|
valid := []string{
|
|
|
|
"name",
|
|
|
|
"dcdfbe62ecd0:alias",
|
|
|
|
"7a67485460b7642516a4ad82ecefe7f57d0c4916f530561b71a50a3f9c4e33da",
|
|
|
|
"angry_torvalds:linus",
|
|
|
|
}
|
|
|
|
invalid := map[string]string{
|
|
|
|
"": "empty string specified for links",
|
|
|
|
"too:much:of:it": "bad format for links: too:much:of:it",
|
|
|
|
}
|
|
|
|
|
|
|
|
for _, link := range valid {
|
|
|
|
if _, err := ValidateLink(link); err != nil {
|
|
|
|
t.Fatalf("ValidateLink(`%q`) should succeed: error %q", link, err)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
for link, expectedError := range invalid {
|
|
|
|
if _, err := ValidateLink(link); err == nil {
|
|
|
|
t.Fatalf("ValidateLink(`%q`) should have failed validation", link)
|
|
|
|
} else {
|
|
|
|
if !strings.Contains(err.Error(), expectedError) {
|
|
|
|
t.Fatalf("ValidateLink(`%q`) error should contain %q", link, expectedError)
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func TestParseLink(t *testing.T) {
|
|
|
|
name, alias, err := ParseLink("name:alias")
|
|
|
|
if err != nil {
|
|
|
|
t.Fatalf("Expected not to error out on a valid name:alias format but got: %v", err)
|
|
|
|
}
|
|
|
|
if name != "name" {
|
|
|
|
t.Fatalf("Link name should have been name, got %s instead", name)
|
|
|
|
}
|
|
|
|
if alias != "alias" {
|
|
|
|
t.Fatalf("Link alias should have been alias, got %s instead", alias)
|
|
|
|
}
|
|
|
|
// short format definition
|
|
|
|
name, alias, err = ParseLink("name")
|
|
|
|
if err != nil {
|
|
|
|
t.Fatalf("Expected not to error out on a valid name only format but got: %v", err)
|
|
|
|
}
|
|
|
|
if name != "name" {
|
|
|
|
t.Fatalf("Link name should have been name, got %s instead", name)
|
|
|
|
}
|
|
|
|
if alias != "name" {
|
|
|
|
t.Fatalf("Link alias should have been name, got %s instead", alias)
|
|
|
|
}
|
|
|
|
// empty string link definition is not allowed
|
|
|
|
if _, _, err := ParseLink(""); err == nil || !strings.Contains(err.Error(), "empty string specified for links") {
|
|
|
|
t.Fatalf("Expected error 'empty string specified for links' but got: %v", err)
|
|
|
|
}
|
|
|
|
// more than two colons are not allowed
|
|
|
|
if _, _, err := ParseLink("link:alias:wrong"); err == nil || !strings.Contains(err.Error(), "bad format for links: link:alias:wrong") {
|
|
|
|
t.Fatalf("Expected error 'bad format for links: link:alias:wrong' but got: %v", err)
|
|
|
|
}
|
|
|
|
}
|