diff --git a/internal/cri/server/container_create.go b/internal/cri/server/container_create.go index 254d2079b..e7e56f163 100644 --- a/internal/cri/server/container_create.go +++ b/internal/cri/server/container_create.go @@ -287,7 +287,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta return nil, fmt.Errorf("failed to get container spec opts: %w", err) } - containerLabels := buildLabels(config.Labels, image.ImageSpec.Config.Labels, crilabels.ContainerKindContainer) + containerLabels := util.BuildLabels(config.Labels, image.ImageSpec.Config.Labels, crilabels.ContainerKindContainer) // TODO the sandbox in the cache should hold this info runtimeName, runtimeOption, err := c.runtimeInfo(ctx, sandboxID) @@ -795,12 +795,12 @@ func (c *criService) buildLinuxSpec( specOpts = append(specOpts, oci.WithRdt(rdtClass, "", "")) } - for pKey, pValue := range getPassthroughAnnotations(sandboxConfig.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(sandboxConfig.Annotations, ociRuntime.PodAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } - for pKey, pValue := range getPassthroughAnnotations(config.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(config.Annotations, ociRuntime.ContainerAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } @@ -930,12 +930,12 @@ func (c *criService) buildWindowsSpec( // when trying to run the init process. specOpts = append(specOpts, oci.WithUser(username)) - for pKey, pValue := range getPassthroughAnnotations(sandboxConfig.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(sandboxConfig.Annotations, ociRuntime.PodAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } - for pKey, pValue := range getPassthroughAnnotations(config.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(config.Annotations, ociRuntime.ContainerAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } @@ -982,12 +982,12 @@ func (c *criService) buildDarwinSpec( specOpts = append(specOpts, customopts.WithDarwinMounts(c.os, config, extraMounts)) - for pKey, pValue := range getPassthroughAnnotations(sandboxConfig.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(sandboxConfig.Annotations, ociRuntime.PodAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } - for pKey, pValue := range getPassthroughAnnotations(config.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(config.Annotations, ociRuntime.ContainerAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } diff --git a/internal/cri/server/helpers.go b/internal/cri/server/helpers.go index d94360b7c..afdd87c4c 100644 --- a/internal/cri/server/helpers.go +++ b/internal/cri/server/helpers.go @@ -33,10 +33,8 @@ import ( containerd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/containers" - crilabels "github.com/containerd/containerd/v2/internal/cri/labels" containerstore "github.com/containerd/containerd/v2/internal/cri/store/container" imagestore "github.com/containerd/containerd/v2/internal/cri/store/image" - clabels "github.com/containerd/containerd/v2/pkg/labels" "github.com/containerd/errdefs" "github.com/containerd/log" ) @@ -220,27 +218,6 @@ func filterLabel(k, v string) string { return fmt.Sprintf("labels.%q==%q", k, v) } -// buildLabel builds the labels from config to be passed to containerd -func buildLabels(configLabels, imageConfigLabels map[string]string, containerType string) map[string]string { - labels := make(map[string]string) - - for k, v := range imageConfigLabels { - if err := clabels.Validate(k, v); err == nil { - labels[k] = v - } else { - // In case the image label is invalid, we output a warning and skip adding it to the - // container. - log.L.WithError(err).Warnf("unable to add image label with key %s to the container", k) - } - } - // labels from the CRI request (config) will override labels in the image config - for k, v := range configLabels { - labels[k] = v - } - labels[crilabels.ContainerKindLabel] = containerType - return labels -} - // getRuntimeOptions get runtime options from container metadata. func getRuntimeOptions(c containers.Container) (interface{}, error) { from := c.Runtime.Options @@ -273,25 +250,6 @@ func unknownContainerStatus() containerstore.Status { } } -// getPassthroughAnnotations filters requested pod annotations by comparing -// against permitted annotations for the given runtime. -func getPassthroughAnnotations(podAnnotations map[string]string, - runtimePodAnnotations []string) (passthroughAnnotations map[string]string) { - passthroughAnnotations = make(map[string]string) - - for podAnnotationKey, podAnnotationValue := range podAnnotations { - 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 - } - } - } - return passthroughAnnotations -} - // copyResourcesToStatus copys container resource contraints from spec to // container status. // This will need updates when new fields are added to ContainerResources. diff --git a/internal/cri/server/helpers_test.go b/internal/cri/server/helpers_test.go index 6d70d6794..b676e7588 100644 --- a/internal/cri/server/helpers_test.go +++ b/internal/cri/server/helpers_test.go @@ -20,7 +20,6 @@ import ( "context" "os" goruntime "runtime" - "strings" "testing" "time" @@ -29,7 +28,6 @@ import ( runcoptions "github.com/containerd/containerd/api/types/runc/options" "github.com/containerd/containerd/v2/core/containers" criconfig "github.com/containerd/containerd/v2/internal/cri/config" - crilabels "github.com/containerd/containerd/v2/internal/cri/labels" containerstore "github.com/containerd/containerd/v2/internal/cri/store/container" "github.com/containerd/containerd/v2/pkg/oci" "github.com/containerd/containerd/v2/pkg/protobuf/types" @@ -90,29 +88,6 @@ func TestGetUserFromImage(t *testing.T) { } } -func TestBuildLabels(t *testing.T) { - imageConfigLabels := map[string]string{ - "a": "z", - "d": "y", - "long-label": strings.Repeat("example", 10000), - } - configLabels := map[string]string{ - "a": "b", - "c": "d", - } - newLabels := buildLabels(configLabels, imageConfigLabels, crilabels.ContainerKindSandbox) - assert.Len(t, newLabels, 4) - assert.Equal(t, "b", newLabels["a"]) - assert.Equal(t, "d", newLabels["c"]) - assert.Equal(t, "y", newLabels["d"]) - assert.Equal(t, crilabels.ContainerKindSandbox, newLabels[crilabels.ContainerKindLabel]) - assert.NotContains(t, newLabels, "long-label") - - newLabels["a"] = "e" - assert.Empty(t, configLabels[crilabels.ContainerKindLabel], "should not add new labels into original label") - assert.Equal(t, "b", configLabels["a"], "change in new labels should not affect original label") -} - func TestGenerateRuntimeOptions(t *testing.T) { nilOpts := ` systemd_cgroup = true @@ -262,121 +237,6 @@ func TestEnvDeduplication(t *testing.T) { } } -func TestPassThroughAnnotationsFilter(t *testing.T) { - for _, test := range []struct { - desc string - podAnnotations map[string]string - runtimePodAnnotations []string - passthroughAnnotations map[string]string - }{ - { - desc: "should support direct match", - podAnnotations: map[string]string{"c": "d", "d": "e"}, - runtimePodAnnotations: []string{"c"}, - passthroughAnnotations: map[string]string{"c": "d"}, - }, - { - desc: "should support wildcard match", - podAnnotations: map[string]string{ - "t.f": "j", - "z.g": "o", - "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", - }, - }, - { - desc: "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", - }, - }, - { - desc: "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", - }, - }, - } { - test := test - t.Run(test.desc, func(t *testing.T) { - passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) - assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) - }) - } -} - func TestEnsureRemoveAllNotExist(t *testing.T) { // should never return an error for a non-existent path if err := ensureRemoveAll(context.Background(), "/non/existent/path"); err != nil { diff --git a/internal/cri/server/images/image_pull.go b/internal/cri/server/images/image_pull.go index 488136b43..3dd100d99 100644 --- a/internal/cri/server/images/image_pull.go +++ b/internal/cri/server/images/image_pull.go @@ -37,7 +37,6 @@ import ( "github.com/containerd/imgcrypt/images/encryption" "github.com/containerd/log" distribution "github.com/distribution/reference" - imagedigest "github.com/opencontainers/go-digest" imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -49,6 +48,7 @@ import ( "github.com/containerd/containerd/v2/internal/cri/annotations" criconfig "github.com/containerd/containerd/v2/internal/cri/config" crilabels "github.com/containerd/containerd/v2/internal/cri/labels" + "github.com/containerd/containerd/v2/internal/cri/util" snpkg "github.com/containerd/containerd/v2/pkg/snapshotters" "github.com/containerd/containerd/v2/pkg/tracing" ) @@ -220,7 +220,7 @@ func (c *CRIImageService) PullImage(ctx context.Context, name string, credential } imageID := configDesc.Digest.String() - repoDigest, repoTag := getRepoDigestAndTag(namedRef, image.Target().Digest, isSchema1) + repoDigest, repoTag := util.GetRepoDigestAndTag(namedRef, image.Target().Digest, isSchema1) for _, r := range []string{imageID, repoTag, repoDigest} { if r == "" { continue @@ -252,21 +252,6 @@ func (c *CRIImageService) PullImage(ctx context.Context, name string, credential return imageID, nil } -// getRepoDigestAngTag returns image repoDigest and repoTag of the named image reference. -func getRepoDigestAndTag(namedRef distribution.Named, digest imagedigest.Digest, schema1 bool) (string, string) { - var repoTag, repoDigest string - if _, ok := namedRef.(distribution.NamedTagged); ok { - repoTag = namedRef.String() - } - if _, ok := namedRef.(distribution.Canonical); ok { - repoDigest = namedRef.String() - } else if !schema1 { - // digest is not actual repo digest for schema1 image. - repoDigest = namedRef.Name() + "@" + digest.String() - } - return repoDigest, repoTag -} - // ParseAuth parses AuthConfig and returns username and password/secret required by containerd. func ParseAuth(auth *runtime.AuthConfig, host string) (string, string, error) { if auth == nil { diff --git a/internal/cri/server/images/image_pull_test.go b/internal/cri/server/images/image_pull_test.go index e981c7e30..bc79e35f0 100644 --- a/internal/cri/server/images/image_pull_test.go +++ b/internal/cri/server/images/image_pull_test.go @@ -21,8 +21,6 @@ import ( "encoding/base64" "testing" - docker "github.com/distribution/reference" - "github.com/opencontainers/go-digest" "github.com/stretchr/testify/assert" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -440,52 +438,6 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) { } } -func TestGetRepoDigestAndTag(t *testing.T) { - digest := digest.Digest("sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582") - for _, test := range []struct { - desc string - ref string - schema1 bool - expectedRepoDigest string - expectedRepoTag string - }{ - { - desc: "repo tag should be empty if original ref has no tag", - ref: "gcr.io/library/busybox@" + digest.String(), - expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), - }, - { - desc: "repo tag should not be empty if original ref has tag", - ref: "gcr.io/library/busybox:latest", - expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), - expectedRepoTag: "gcr.io/library/busybox:latest", - }, - { - desc: "repo digest should be empty if original ref is schema1 and has no digest", - ref: "gcr.io/library/busybox:latest", - schema1: true, - expectedRepoDigest: "", - expectedRepoTag: "gcr.io/library/busybox:latest", - }, - { - desc: "repo digest should not be empty if original ref is schema1 but has digest", - ref: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", - schema1: true, - expectedRepoDigest: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", - expectedRepoTag: "", - }, - } { - test := test - t.Run(test.desc, func(t *testing.T) { - named, err := docker.ParseDockerRef(test.ref) - assert.NoError(t, err) - repoDigest, repoTag := getRepoDigestAndTag(named, digest, test.schema1) - assert.Equal(t, test.expectedRepoDigest, repoDigest) - assert.Equal(t, test.expectedRepoTag, repoTag) - }) - } -} - func TestImageGetLabels(t *testing.T) { criService, _ := newTestCRIService() diff --git a/internal/cri/server/images/image_status_test.go b/internal/cri/server/images/image_status_test.go index e4405be78..a0db783cc 100644 --- a/internal/cri/server/images/image_status_test.go +++ b/internal/cri/server/images/image_status_test.go @@ -26,7 +26,6 @@ import ( runtime "k8s.io/cri-api/pkg/apis/runtime/v1" imagestore "github.com/containerd/containerd/v2/internal/cri/store/image" - "github.com/containerd/containerd/v2/internal/cri/util" ) func TestImageStatus(t *testing.T) { @@ -74,22 +73,6 @@ func TestImageStatus(t *testing.T) { assert.Equal(t, expected, resp.GetImage()) } -func TestParseImageReferences(t *testing.T) { - refs := []string{ - "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", - "gcr.io/library/busybox:1.2", - "sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", - "arbitrary-ref", - } - expectedTags := []string{ - "gcr.io/library/busybox:1.2", - } - expectedDigests := []string{"gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"} - tags, digests := util.ParseImageReferences(refs) - assert.Equal(t, expectedTags, tags) - assert.Equal(t, expectedDigests, digests) -} - // TestGetUserFromImage tests the logic of getting image uid or user name of image user. func TestGetUserFromImage(t *testing.T) { newI64 := func(i int64) *int64 { return &i } diff --git a/internal/cri/server/podsandbox/helpers.go b/internal/cri/server/podsandbox/helpers.go index 8354f6c8c..c960270b9 100644 --- a/internal/cri/server/podsandbox/helpers.go +++ b/internal/cri/server/podsandbox/helpers.go @@ -19,13 +19,9 @@ package podsandbox import ( "context" "fmt" - "path" "path/filepath" - "github.com/containerd/log" "github.com/containerd/typeurl/v2" - docker "github.com/distribution/reference" - imagedigest "github.com/opencontainers/go-digest" runtimespec "github.com/opencontainers/runtime-spec/specs-go" containerd "github.com/containerd/containerd/v2/client" @@ -34,7 +30,6 @@ import ( imagestore "github.com/containerd/containerd/v2/internal/cri/store/image" sandboxstore "github.com/containerd/containerd/v2/internal/cri/store/sandbox" ctrdutil "github.com/containerd/containerd/v2/internal/cri/util" - clabels "github.com/containerd/containerd/v2/pkg/labels" "github.com/containerd/containerd/v2/pkg/oci" ) @@ -65,21 +60,6 @@ func (c *Controller) getVolatileSandboxRootDir(id string) string { return filepath.Join(c.config.StateDir, sandboxesDir, id) } -// getRepoDigestAngTag returns image repoDigest and repoTag of the named image reference. -func getRepoDigestAndTag(namedRef docker.Named, digest imagedigest.Digest, schema1 bool) (string, string) { - var repoTag, repoDigest string - if _, ok := namedRef.(docker.NamedTagged); ok { - repoTag = namedRef.String() - } - if _, ok := namedRef.(docker.Canonical); ok { - repoDigest = namedRef.String() - } else if !schema1 { - // digest is not actual repo digest for schema1 image. - repoDigest = namedRef.Name() + "@" + digest.String() - } - return repoDigest, repoTag -} - // toContainerdImage converts an image object in image store to containerd image handler. func (c *Controller) toContainerdImage(ctx context.Context, image imagestore.Image) (containerd.Image, error) { // image should always have at least one reference. @@ -89,64 +69,6 @@ func (c *Controller) toContainerdImage(ctx context.Context, image imagestore.Ima return c.client.GetImage(ctx, image.References[0]) } -// buildLabel builds the labels from config to be passed to containerd -func buildLabels(configLabels, imageConfigLabels map[string]string, containerType string) map[string]string { - labels := make(map[string]string) - - for k, v := range imageConfigLabels { - if err := clabels.Validate(k, v); err == nil { - labels[k] = v - } else { - // In case the image label is invalid, we output a warning and skip adding it to the - // container. - log.L.WithError(err).Warnf("unable to add image label with key %s to the container", k) - } - } - // labels from the CRI request (config) will override labels in the image config - for k, v := range configLabels { - labels[k] = v - } - labels[crilabels.ContainerKindLabel] = containerType - return labels -} - -// parseImageReferences parses a list of arbitrary image references and returns -// the repotags and repodigests -func parseImageReferences(refs []string) ([]string, []string) { - var tags, digests []string - for _, ref := range refs { - parsed, err := docker.ParseAnyReference(ref) - if err != nil { - continue - } - if _, ok := parsed.(docker.Canonical); ok { - digests = append(digests, parsed.String()) - } else if _, ok := parsed.(docker.Tagged); ok { - tags = append(tags, parsed.String()) - } - } - return tags, digests -} - -// getPassthroughAnnotations filters requested pod annotations by comparing -// against permitted annotations for the given runtime. -func getPassthroughAnnotations(podAnnotations map[string]string, - runtimePodAnnotations []string) (passthroughAnnotations map[string]string) { - passthroughAnnotations = make(map[string]string) - - for podAnnotationKey, podAnnotationValue := range podAnnotations { - 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 - } - } - } - return passthroughAnnotations -} - // runtimeSpec returns a default runtime spec used in cri-containerd. func (c *Controller) runtimeSpec(id string, baseSpecFile string, opts ...oci.SpecOpts) (*runtimespec.Spec, error) { // GenerateSpec needs namespace. diff --git a/internal/cri/server/podsandbox/helpers_test.go b/internal/cri/server/podsandbox/helpers_test.go index af4112169..2fa393666 100644 --- a/internal/cri/server/podsandbox/helpers_test.go +++ b/internal/cri/server/podsandbox/helpers_test.go @@ -19,102 +19,13 @@ package podsandbox import ( "context" "os" - "strings" "testing" - crilabels "github.com/containerd/containerd/v2/internal/cri/labels" "github.com/containerd/containerd/v2/pkg/oci" - docker "github.com/distribution/reference" - imagedigest "github.com/opencontainers/go-digest" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" ) -func TestGetRepoDigestAndTag(t *testing.T) { - digest := imagedigest.Digest("sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582") - for _, test := range []struct { - desc string - ref string - schema1 bool - expectedRepoDigest string - expectedRepoTag string - }{ - { - desc: "repo tag should be empty if original ref has no tag", - ref: "gcr.io/library/busybox@" + digest.String(), - expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), - }, - { - desc: "repo tag should not be empty if original ref has tag", - ref: "gcr.io/library/busybox:latest", - expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), - expectedRepoTag: "gcr.io/library/busybox:latest", - }, - { - desc: "repo digest should be empty if original ref is schema1 and has no digest", - ref: "gcr.io/library/busybox:latest", - schema1: true, - expectedRepoDigest: "", - expectedRepoTag: "gcr.io/library/busybox:latest", - }, - { - desc: "repo digest should not be empty if original ref is schema1 but has digest", - ref: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", - schema1: true, - expectedRepoDigest: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", - expectedRepoTag: "", - }, - } { - test := test - t.Run(test.desc, func(t *testing.T) { - named, err := docker.ParseDockerRef(test.ref) - assert.NoError(t, err) - repoDigest, repoTag := getRepoDigestAndTag(named, digest, test.schema1) - assert.Equal(t, test.expectedRepoDigest, repoDigest) - assert.Equal(t, test.expectedRepoTag, repoTag) - }) - } -} - -func TestBuildLabels(t *testing.T) { - imageConfigLabels := map[string]string{ - "a": "z", - "d": "y", - "long-label": strings.Repeat("example", 10000), - } - configLabels := map[string]string{ - "a": "b", - "c": "d", - } - newLabels := buildLabels(configLabels, imageConfigLabels, crilabels.ContainerKindSandbox) - assert.Len(t, newLabels, 4) - assert.Equal(t, "b", newLabels["a"]) - assert.Equal(t, "d", newLabels["c"]) - assert.Equal(t, "y", newLabels["d"]) - assert.Equal(t, crilabels.ContainerKindSandbox, newLabels[crilabels.ContainerKindLabel]) - assert.NotContains(t, newLabels, "long-label") - - newLabels["a"] = "e" - assert.Empty(t, configLabels[crilabels.ContainerKindLabel], "should not add new labels into original label") - assert.Equal(t, "b", configLabels["a"], "change in new labels should not affect original label") -} - -func TestParseImageReferences(t *testing.T) { - refs := []string{ - "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", - "gcr.io/library/busybox:1.2", - "sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", - "arbitrary-ref", - } - expectedTags := []string{ - "gcr.io/library/busybox:1.2", - } - expectedDigests := []string{"gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"} - tags, digests := parseImageReferences(refs) - assert.Equal(t, expectedTags, tags) - assert.Equal(t, expectedDigests, digests) -} - func TestEnvDeduplication(t *testing.T) { for _, test := range []struct { desc string @@ -195,121 +106,6 @@ func TestEnvDeduplication(t *testing.T) { } } -func TestPassThroughAnnotationsFilter(t *testing.T) { - for _, test := range []struct { - desc string - podAnnotations map[string]string - runtimePodAnnotations []string - passthroughAnnotations map[string]string - }{ - { - desc: "should support direct match", - podAnnotations: map[string]string{"c": "d", "d": "e"}, - runtimePodAnnotations: []string{"c"}, - passthroughAnnotations: map[string]string{"c": "d"}, - }, - { - desc: "should support wildcard match", - podAnnotations: map[string]string{ - "t.f": "j", - "z.g": "o", - "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", - }, - }, - { - desc: "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", - }, - }, - { - desc: "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", - }, - }, - } { - test := test - t.Run(test.desc, func(t *testing.T) { - passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) - assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) - }) - } -} - func TestEnsureRemoveAllNotExist(t *testing.T) { // should never return an error for a non-existent path if err := ensureRemoveAll(context.Background(), "/non/existent/path"); err != nil { diff --git a/internal/cri/server/podsandbox/sandbox_run.go b/internal/cri/server/podsandbox/sandbox_run.go index 702b56801..53d949fdb 100644 --- a/internal/cri/server/podsandbox/sandbox_run.go +++ b/internal/cri/server/podsandbox/sandbox_run.go @@ -163,7 +163,7 @@ func (c *Controller) Start(ctx context.Context, id string) (cin sandbox.Controll return cin, fmt.Errorf("failed to generate sandbox container spec options: %w", err) } - sandboxLabels := buildLabels(config.Labels, image.ImageSpec.Config.Labels, crilabels.ContainerKindSandbox) + sandboxLabels := ctrdutil.BuildLabels(config.Labels, image.ImageSpec.Config.Labels, crilabels.ContainerKindSandbox) snapshotterOpt := []snapshots.Opt{snapshots.WithLabels(snapshots.FilterInheritedLabels(config.Annotations))} extraSOpts, err := sandboxSnapshotterOpts(config) diff --git a/internal/cri/server/podsandbox/sandbox_run_linux.go b/internal/cri/server/podsandbox/sandbox_run_linux.go index b9ad3047d..030cd542a 100644 --- a/internal/cri/server/podsandbox/sandbox_run_linux.go +++ b/internal/cri/server/podsandbox/sandbox_run_linux.go @@ -33,6 +33,7 @@ import ( "github.com/containerd/containerd/v2/core/snapshots" "github.com/containerd/containerd/v2/internal/cri/annotations" customopts "github.com/containerd/containerd/v2/internal/cri/opts" + "github.com/containerd/containerd/v2/internal/cri/util" ) func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxConfig, @@ -187,7 +188,7 @@ func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxC specOpts = append(specOpts, customopts.WithPodOOMScoreAdj(int(defaultSandboxOOMAdj), c.config.RestrictOOMScoreAdj)) - for pKey, pValue := range getPassthroughAnnotations(config.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(config.Annotations, runtimePodAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } diff --git a/internal/cri/server/podsandbox/sandbox_run_windows.go b/internal/cri/server/podsandbox/sandbox_run_windows.go index a7ad590df..067f4a850 100644 --- a/internal/cri/server/podsandbox/sandbox_run_windows.go +++ b/internal/cri/server/podsandbox/sandbox_run_windows.go @@ -28,6 +28,7 @@ import ( "github.com/containerd/containerd/v2/core/snapshots" "github.com/containerd/containerd/v2/internal/cri/annotations" customopts "github.com/containerd/containerd/v2/internal/cri/opts" + "github.com/containerd/containerd/v2/internal/cri/util" ) func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxConfig, @@ -75,7 +76,7 @@ func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxC // when trying to run the init process. specOpts = append(specOpts, oci.WithUser(username)) - for pKey, pValue := range getPassthroughAnnotations(config.Annotations, + for pKey, pValue := range util.GetPassthroughAnnotations(config.Annotations, runtimePodAnnotations) { specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } diff --git a/internal/cri/util/references.go b/internal/cri/util/references.go index 4813f89e1..6c9791bfa 100644 --- a/internal/cri/util/references.go +++ b/internal/cri/util/references.go @@ -16,7 +16,10 @@ package util -import reference "github.com/distribution/reference" +import ( + reference "github.com/distribution/reference" + imagedigest "github.com/opencontainers/go-digest" +) // ParseImageReferences parses a list of arbitrary image references and returns // the repotags and repodigests @@ -35,3 +38,18 @@ func ParseImageReferences(refs []string) ([]string, []string) { } return tags, digests } + +// GetRepoDigestAndTag returns image repoDigest and repoTag of the named image reference. +func GetRepoDigestAndTag(namedRef reference.Named, digest imagedigest.Digest, schema1 bool) (string, string) { + var repoTag, repoDigest string + if _, ok := namedRef.(reference.NamedTagged); ok { + repoTag = namedRef.String() + } + if _, ok := namedRef.(reference.Canonical); ok { + repoDigest = namedRef.String() + } else if !schema1 { + // digest is not actual repo digest for schema1 image. + repoDigest = namedRef.Name() + "@" + digest.String() + } + return repoDigest, repoTag +} diff --git a/internal/cri/util/references_test.go b/internal/cri/util/references_test.go new file mode 100644 index 000000000..eaea819a2 --- /dev/null +++ b/internal/cri/util/references_test.go @@ -0,0 +1,88 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/distribution/reference" + "github.com/opencontainers/go-digest" +) + +func TestParseImageReferences(t *testing.T) { + refs := []string{ + "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + "gcr.io/library/busybox:1.2", + "sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + "arbitrary-ref", + } + expectedTags := []string{ + "gcr.io/library/busybox:1.2", + } + expectedDigests := []string{"gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"} + tags, digests := ParseImageReferences(refs) + assert.Equal(t, expectedTags, tags) + assert.Equal(t, expectedDigests, digests) +} + +func TestGetRepoDigestAndTag(t *testing.T) { + digest := digest.Digest("sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582") + for _, test := range []struct { + desc string + ref string + schema1 bool + expectedRepoDigest string + expectedRepoTag string + }{ + { + desc: "repo tag should be empty if original ref has no tag", + ref: "gcr.io/library/busybox@" + digest.String(), + expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), + }, + { + desc: "repo tag should not be empty if original ref has tag", + ref: "gcr.io/library/busybox:latest", + expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), + expectedRepoTag: "gcr.io/library/busybox:latest", + }, + { + desc: "repo digest should be empty if original ref is schema1 and has no digest", + ref: "gcr.io/library/busybox:latest", + schema1: true, + expectedRepoDigest: "", + expectedRepoTag: "gcr.io/library/busybox:latest", + }, + { + desc: "repo digest should not be empty if original ref is schema1 but has digest", + ref: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", + schema1: true, + expectedRepoDigest: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", + expectedRepoTag: "", + }, + } { + test := test + t.Run(test.desc, func(t *testing.T) { + named, err := reference.ParseDockerRef(test.ref) + assert.NoError(t, err) + repoDigest, repoTag := GetRepoDigestAndTag(named, digest, test.schema1) + assert.Equal(t, test.expectedRepoDigest, repoDigest) + assert.Equal(t, test.expectedRepoTag, repoTag) + }) + } +} diff --git a/internal/cri/util/util.go b/internal/cri/util/util.go index 3eb1d644d..e5ca181b1 100644 --- a/internal/cri/util/util.go +++ b/internal/cri/util/util.go @@ -18,11 +18,14 @@ package util import ( "context" + "path" "time" - "github.com/containerd/containerd/v2/pkg/namespaces" - "github.com/containerd/containerd/v2/internal/cri/constants" + crilabels "github.com/containerd/containerd/v2/internal/cri/labels" + clabels "github.com/containerd/containerd/v2/pkg/labels" + "github.com/containerd/containerd/v2/pkg/namespaces" + "github.com/containerd/log" ) // deferCleanupTimeout is the default timeout for containerd cleanup operations @@ -44,3 +47,43 @@ func NamespacedContext() context.Context { func WithNamespace(ctx context.Context) context.Context { return namespaces.WithNamespace(ctx, constants.K8sContainerdNamespace) } + +// GetPassthroughAnnotations filters requested pod annotations by comparing +// against permitted annotations for the given runtime. +func GetPassthroughAnnotations(podAnnotations map[string]string, + runtimePodAnnotations []string) (passthroughAnnotations map[string]string) { + passthroughAnnotations = make(map[string]string) + + for podAnnotationKey, podAnnotationValue := range podAnnotations { + 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 + } + } + } + return passthroughAnnotations +} + +// BuildLabel builds the labels from config to be passed to containerd +func BuildLabels(configLabels, imageConfigLabels map[string]string, containerType string) map[string]string { + labels := make(map[string]string) + + for k, v := range imageConfigLabels { + if err := clabels.Validate(k, v); err == nil { + labels[k] = v + } else { + // In case the image label is invalid, we output a warning and skip adding it to the + // container. + log.L.WithError(err).Warnf("unable to add image label with key %s to the container", k) + } + } + // labels from the CRI request (config) will override labels in the image config + for k, v := range configLabels { + labels[k] = v + } + labels[crilabels.ContainerKindLabel] = containerType + return labels +} diff --git a/internal/cri/util/util_test.go b/internal/cri/util/util_test.go new file mode 100644 index 000000000..6bd558542 --- /dev/null +++ b/internal/cri/util/util_test.go @@ -0,0 +1,163 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package util + +import ( + "strings" + "testing" + + crilabels "github.com/containerd/containerd/v2/internal/cri/labels" + "github.com/stretchr/testify/assert" +) + +func TestPassThroughAnnotationsFilter(t *testing.T) { + for _, test := range []struct { + desc string + podAnnotations map[string]string + runtimePodAnnotations []string + passthroughAnnotations map[string]string + }{ + { + desc: "should support direct match", + podAnnotations: map[string]string{"c": "d", "d": "e"}, + runtimePodAnnotations: []string{"c"}, + passthroughAnnotations: map[string]string{"c": "d"}, + }, + { + desc: "should support wildcard match", + podAnnotations: map[string]string{ + "t.f": "j", + "z.g": "o", + "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", + }, + }, + { + desc: "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", + }, + }, + { + desc: "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", + }, + }, + } { + test := test + t.Run(test.desc, func(t *testing.T) { + passthroughAnnotations := GetPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) + assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) + }) + } +} + +func TestBuildLabels(t *testing.T) { + imageConfigLabels := map[string]string{ + "a": "z", + "d": "y", + "long-label": strings.Repeat("example", 10000), + } + configLabels := map[string]string{ + "a": "b", + "c": "d", + } + newLabels := BuildLabels(configLabels, imageConfigLabels, crilabels.ContainerKindSandbox) + assert.Len(t, newLabels, 4) + assert.Equal(t, "b", newLabels["a"]) + assert.Equal(t, "d", newLabels["c"]) + assert.Equal(t, "y", newLabels["d"]) + assert.Equal(t, crilabels.ContainerKindSandbox, newLabels[crilabels.ContainerKindLabel]) + assert.NotContains(t, newLabels, "long-label") + + newLabels["a"] = "e" + assert.Empty(t, configLabels[crilabels.ContainerKindLabel], "should not add new labels into original label") + assert.Equal(t, "b", configLabels["a"], "change in new labels should not affect original label") +}