diff --git a/docs/config.md b/docs/config.md index 85a416021..368684cf5 100644 --- a/docs/config.md +++ b/docs/config.md @@ -138,10 +138,14 @@ The explanation and default value of each configuration item are as follows: # runtime_type is the runtime type to use in containerd e.g. io.containerd.runtime.v1.linux runtime_type = "io.containerd.runc.v1" - # pod_annotations is list of pod annotations passed to both pod sandbox as well as - # container OCI annotations. Pod_annotations also support golang supported - # regular expression - https://github.com/google/re2/wiki/Syntax. - # e.g. ["runc.com.github.containers.runc.*"] + # pod_annotations is a list of pod annotations passed to both pod + # sandbox as well as container OCI annotations. Pod_annotations also + # supports golang path match pattern - https://golang.org/pkg/path/#Match. + # e.g. ["runc.com.*"], ["*.runc.com"], ["runc.com/*"]. + # + # For the naming convention of annotation keys, please reference: + # * Kubernetes: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set + # * OCI: https://github.com/opencontainers/image-spec/blob/master/annotations.md pod_annotations = [] # "plugins.cri.containerd.runtimes.runc.options" is options specific to diff --git a/pkg/config/config.go b/pkg/config/config.go index 3aa096fab..3e7a2aedf 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -31,7 +31,7 @@ type Runtime struct { // This only works for runtime type "io.containerd.runtime.v1.linux". // DEPRECATED: use Options instead. Remove when shim v1 is deprecated. Engine string `toml:"runtime_engine" json:"runtimeEngine"` - // PodAnnotations is list of pod annotations passed to both pod sandbox as well as + // PodAnnotations is a list of pod annotations passed to both pod sandbox as well as // container OCI annotations. PodAnnotations []string `toml:"pod_annotations" json:"PodAnnotations"` // Root is the directory used by containerd for runtime state. diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index d245c247c..df85cab6c 100644 --- a/pkg/server/container_create.go +++ b/pkg/server/container_create.go @@ -169,7 +169,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta if err != nil { return nil, errors.Wrap(err, "failed to get sandbox runtime") } - logrus.Debugf("Use OCI %+v for sandbox %q and container %q", ociRuntime, sandboxID, id) + logrus.Debugf("Use OCI runtime %+v for sandbox %q and container %q", ociRuntime, sandboxID, id) spec, err := c.generateContainerSpec(id, sandboxID, sandboxPid, config, sandboxConfig, &image.ImageSpec.Config, append(mounts, volumeMounts...), ociRuntime.PodAnnotations) diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go index e79d84ab6..3e5ef29ab 100644 --- a/pkg/server/container_create_test.go +++ b/pkg/server/container_create_test.go @@ -36,7 +36,6 @@ import ( runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" "github.com/containerd/cri/pkg/annotations" - criconfig "github.com/containerd/cri/pkg/config" ostesting "github.com/containerd/cri/pkg/os/testing" "github.com/containerd/cri/pkg/util" ) @@ -295,82 +294,64 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { testPid := uint32(1234) for desc, test := range map[string]struct { - configCriService func(*criService) - configChange func(*runtime.PodSandboxConfig) - specCheck func(*testing.T, *runtimespec.Spec) + podAnnotations []string + configChange func(*runtime.PodSandboxConfig) + specCheck func(*testing.T, *runtimespec.Spec) }{ - // Scenario 1 - containerd config allows "c" and podSpec defines "c" to be passed to OCI - "passthroughAnnotations scenario 1": { - configCriService: func(c *criService) { - c.config.Runtimes = make(map[string]criconfig.Runtime) - c.config.Runtimes["runc"] = criconfig.Runtime{ - PodAnnotations: []string{"c"}, - } - }, + "a passthrough annotation should be passed as an OCI annotation": { + podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") }, }, - // Scenario 2 - containerd config allows only "c" but podSpec defines "c" and "d" - // Only annotation "c" will be injected in OCI annotations and "d" should be dropped. - "passthroughAnnotations scenario 2": { + "a non-passthrough annotation should not be passed as an OCI annotation": { configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["d"] = "e" }, - configCriService: func(c *criService) { - c.config.Runtimes = make(map[string]criconfig.Runtime) - c.config.Runtimes["runc"] = criconfig.Runtime{ - PodAnnotations: []string{"c"}, - } - }, + podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") _, ok := spec.Annotations["d"] - assert.Equal(t, ok, false) + assert.False(t, ok) }, }, - // Scenario 3 - Let's test some wildcard support - // podSpec has following annotations - "passthroughAnnotations scenario 3": { + "passthrough annotations should support wildcard match": { configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["t.f"] = "j" c.Annotations["z.g"] = "o" + c.Annotations["z"] = "o" c.Annotations["y.ca"] = "b" + c.Annotations["y"] = "b" }, - configCriService: func(c *criService) { - c.config.Runtimes = make(map[string]criconfig.Runtime) - c.config.Runtimes["runc"] = criconfig.Runtime{ - PodAnnotations: []string{"t*", "z.*", "y.c*"}, - } - }, + podAnnotations: []string{"t*", "z.*", "y.c*"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { + t.Logf("%+v", spec.Annotations) assert.Equal(t, spec.Annotations["t.f"], "j") assert.Equal(t, spec.Annotations["z.g"], "o") assert.Equal(t, spec.Annotations["y.ca"], "b") + _, ok := spec.Annotations["y"] + assert.False(t, ok) + _, ok = spec.Annotations["z"] + assert.False(t, ok) }, }, } { + t.Run(desc, func(t *testing.T) { + c := newTestCRIService() + config, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() + if test.configChange != nil { + test.configChange(sandboxConfig) + } - t.Logf("TestCase %q", desc) - c := newTestCRIService() - config, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() - if test.configChange != nil { - test.configChange(sandboxConfig) - } - - if test.configCriService != nil { - test.configCriService(c) - } - - spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, - config, sandboxConfig, imageConfig, nil, c.config.Runtimes["runc"].PodAnnotations) - - assert.NoError(t, err) - assert.NotNil(t, spec) - specCheck(t, testID, testSandboxID, testPid, spec) - if test.specCheck != nil { - test.specCheck(t, spec) - } + spec, err := c.generateContainerSpec(testID, testSandboxID, testPid, + config, sandboxConfig, imageConfig, nil, test.podAnnotations) + assert.NoError(t, err) + assert.NotNil(t, spec) + specCheck(t, testID, testSandboxID, testPid, spec) + if test.specCheck != nil { + test.specCheck(t, spec) + } + }) } } diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index 451752d94..b915d46a0 100644 --- a/pkg/server/helpers.go +++ b/pkg/server/helpers.go @@ -634,9 +634,11 @@ func getPassthroughAnnotations(podAnnotations map[string]string, passthroughAnnotations = make(map[string]string) for podAnnotationKey, podAnnotationValue := range podAnnotations { - for _, r := range runtimePodAnnotations { - match, _ := regexp.MatchString(r, podAnnotationKey) - if match { + for _, pattern := range runtimePodAnnotations { + // Use path.Match instead of filepath.Match here. + // filepath.Match treated `\\` as path separator + // on windows, which is not what we want. + if ok, _ := path.Match(pattern, podAnnotationKey); ok { passthroughAnnotations[podAnnotationKey] = podAnnotationValue } } diff --git a/pkg/server/helpers_test.go b/pkg/server/helpers_test.go index 0b73550c9..b0eac632b 100644 --- a/pkg/server/helpers_test.go +++ b/pkg/server/helpers_test.go @@ -413,33 +413,105 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { runtimePodAnnotations []string passthroughAnnotations map[string]string }{ - // Scenario 1 - containerd config allows "c" and podSpec defines "c" to be passed to OCI - "scenario1": { - podAnnotations: map[string]string{"c": "d"}, - runtimePodAnnotations: []string{"c"}, - passthroughAnnotations: map[string]string{"c": "d"}, - }, - // Scenario 2 - containerd config allows only "c" but podSpec defines "c" and "d" - // Only annotation "c" will be injected in OCI annotations and "d" should be dropped. - "scenario2": { + "should support direct match": { podAnnotations: map[string]string{"c": "d", "d": "e"}, runtimePodAnnotations: []string{"c"}, passthroughAnnotations: map[string]string{"c": "d"}, }, - // Scenario 3 - Let's test some wildcard support - // podSpec has following annotations - "scenario3": { - podAnnotations: map[string]string{"t.f": "j", + "should support wildcard match": { + podAnnotations: map[string]string{ + "t.f": "j", "z.g": "o", - "y.ca": "b"}, - runtimePodAnnotations: []string{"t*", "z.*", "y.c*"}, - passthroughAnnotations: map[string]string{"t.f": "j", + "z": "o", + "y.ca": "b", + "y": "b", + }, + runtimePodAnnotations: []string{"*.f", "z*g", "y.c*"}, + passthroughAnnotations: map[string]string{ + "t.f": "j", "z.g": "o", - "y.ca": "b"}, + "y.ca": "b", + }, + }, + "should support wildcard match all": { + podAnnotations: map[string]string{ + "t.f": "j", + "z.g": "o", + "z": "o", + "y.ca": "b", + "y": "b", + }, + runtimePodAnnotations: []string{"*"}, + passthroughAnnotations: map[string]string{ + "t.f": "j", + "z.g": "o", + "z": "o", + "y.ca": "b", + "y": "b", + }, + }, + "should support match including path separator": { + podAnnotations: map[string]string{ + "matchend.com/end": "1", + "matchend.com/end1": "2", + "matchend.com/1end": "3", + "matchmid.com/mid": "4", + "matchmid.com/mi1d": "5", + "matchmid.com/mid1": "6", + "matchhead.com/head": "7", + "matchhead.com/1head": "8", + "matchhead.com/head1": "9", + "matchall.com/abc": "10", + "matchall.com/def": "11", + "end/matchend": "12", + "end1/matchend": "13", + "1end/matchend": "14", + "mid/matchmid": "15", + "mi1d/matchmid": "16", + "mid1/matchmid": "17", + "head/matchhead": "18", + "1head/matchhead": "19", + "head1/matchhead": "20", + "abc/matchall": "21", + "def/matchall": "22", + "match1/match2": "23", + "nomatch/nomatch": "24", + }, + runtimePodAnnotations: []string{ + "matchend.com/end*", + "matchmid.com/mi*d", + "matchhead.com/*head", + "matchall.com/*", + "end*/matchend", + "mi*d/matchmid", + "*head/matchhead", + "*/matchall", + "match*/match*", + }, + passthroughAnnotations: map[string]string{ + "matchend.com/end": "1", + "matchend.com/end1": "2", + "matchmid.com/mid": "4", + "matchmid.com/mi1d": "5", + "matchhead.com/head": "7", + "matchhead.com/1head": "8", + "matchall.com/abc": "10", + "matchall.com/def": "11", + "end/matchend": "12", + "end1/matchend": "13", + "mid/matchmid": "15", + "mi1d/matchmid": "16", + "head/matchhead": "18", + "1head/matchhead": "19", + "abc/matchall": "21", + "def/matchall": "22", + "match1/match2": "23", + }, }, } { - t.Logf("TestCase %q", desc) - passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) - assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) + t.Run(desc, func(t *testing.T) { + passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) + assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) + }) } } diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index 5339755fc..dd9fc0de4 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -86,8 +86,8 @@ func TestGenerateSandboxContainerSpec(t *testing.T) { testID := "test-id" nsPath := "test-cni" for desc, test := range map[string]struct { - configCriService func(*criService) configChange func(*runtime.PodSandboxConfig) + podAnnotations []string imageConfigChange func(*imagespec.ImageConfig) specCheck func(*testing.T, *runtimespec.Spec) expectErr bool @@ -158,54 +158,40 @@ func TestGenerateSandboxContainerSpec(t *testing.T) { assert.Contains(t, spec.Process.User.AdditionalGids, uint32(2222)) }, }, - // Scenario 1 - containerd config allows "c" and podSpec defines "c" to be passed to OCI - "passthroughAnnotations scenario 1": { - configCriService: func(c *criService) { - c.config.Runtimes = make(map[string]criconfig.Runtime) - c.config.Runtimes["runc"] = criconfig.Runtime{ - PodAnnotations: []string{"c"}, - } - }, //assert.Equal(t, passthroughAnnotations["c"], "d") + "a passthrough annotation should be passed as an OCI annotation": { + podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") }, }, - // Scenario 2 - containerd config allows only "c" but podSpec defines "c" and "d" - // Only annotation "c" will be injected in OCI annotations and "d" should be dropped. - "passthroughAnnotations scenario 2": { + "a non-passthrough annotation should not be passed as an OCI annotation": { configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["d"] = "e" }, - configCriService: func(c *criService) { - c.config.Runtimes = make(map[string]criconfig.Runtime) - c.config.Runtimes["runc"] = criconfig.Runtime{ - PodAnnotations: []string{"c"}, - } - }, + podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") _, ok := spec.Annotations["d"] - assert.Equal(t, ok, false) + assert.False(t, ok) }, }, - // Scenario 3 - Let's test some wildcard support - // podSpec has following annotations - "passthroughAnnotations scenario 3": { + "passthrough annotations should support wildcard match": { configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["t.f"] = "j" c.Annotations["z.g"] = "o" + c.Annotations["z"] = "o" c.Annotations["y.ca"] = "b" + c.Annotations["y"] = "b" }, - configCriService: func(c *criService) { - c.config.Runtimes = make(map[string]criconfig.Runtime) - c.config.Runtimes["runc"] = criconfig.Runtime{ - PodAnnotations: []string{"t*", "z.*", "y.c*"}, - } - }, + podAnnotations: []string{"t*", "z.*", "y.c*"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["t.f"], "j") assert.Equal(t, spec.Annotations["z.g"], "o") assert.Equal(t, spec.Annotations["y.ca"], "b") + _, ok := spec.Annotations["y"] + assert.False(t, ok) + _, ok = spec.Annotations["z"] + assert.False(t, ok) }, }, } { @@ -216,15 +202,11 @@ func TestGenerateSandboxContainerSpec(t *testing.T) { test.configChange(config) } - if test.configCriService != nil { - test.configCriService(c) - } - if test.imageConfigChange != nil { test.imageConfigChange(imageConfig) } spec, err := c.generateSandboxContainerSpec(testID, config, imageConfig, nsPath, - c.config.Runtimes["runc"].PodAnnotations) + test.podAnnotations) if test.expectErr { assert.Error(t, err) assert.Nil(t, spec) @@ -412,7 +394,6 @@ options timeout:1 c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) { return realhostname, nil } - cfg := &runtime.PodSandboxConfig{ Hostname: test.hostname, DnsConfig: test.dnsConfig,