Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processor/k8sattributes] Support name:tag@digest image name format #36145

Merged
merged 3 commits into from
Nov 25, 2024
Merged

[processor/k8sattributes] Support name:tag@digest image name format #36145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
285aec3
[processor/k8sattributes] Support name:tag@digest image name format
spiffyy99 Nov 2, 2024
35b2a0b
add to documentation and add latest by default logic
spiffyy99 Nov 8, 2024
115ec54
small changes
spiffyy99 Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/fix-k8s-image-parsing.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: processor/k8sattribute

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: fixes parsing of k8s image names to support images with tags and digests.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [36131]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
5 changes: 3 additions & 2 deletions processor/k8sattributesprocessor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ spec:
- --duration=10s
- --rate=1
- --otlp-attributes=k8s.container.name="telemetrygen"
image: ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen:latest
image: ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen:0.112.0@sha256:b248ef911f93ae27cbbc85056d1ffacc87fd941bbdc2ffd951b6df8df72b8096
name: telemetrygen
status:
podIP: 10.244.0.11
Expand All @@ -191,7 +191,8 @@ the processor associates the received trace to the pod, based on the connection
"k8s.pod.name": "telemetrygen-pod",
"k8s.pod.uid": "038e2267-b473-489b-b48c-46bafdb852eb",
"container.image.name": "telemetrygen",
"container.image.tag": "latest"
"container.image.tag": "0.112.0",
"container.image.repo_digests": ["ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen@sha256:b248ef911f93ae27cbbc85056d1ffacc87fd941bbdc2ffd951b6df8df72b8096"]
}
}
```
Expand Down
2 changes: 1 addition & 1 deletion processor/k8sattributesprocessor/documentation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
| container.id | Container ID. Usually a UUID, as for example used to identify Docker containers. The UUID might be abbreviated. Requires k8s.container.restart_count. | Any Str | false |
| container.image.name | Name of the image the container was built on. Requires container.id or k8s.container.name. | Any Str | true |
| container.image.repo_digests | Repo digests of the container image as provided by the container runtime. | Any Slice | false |
| container.image.tag | Container image tag. Requires container.id or k8s.container.name. | Any Str | true |
| container.image.tag | Container image tag. Defaults to "latest" if not provided (unless digest also in image path) Requires container.id or k8s.container.name. | Any Str | true |
| k8s.cluster.uid | Gives cluster uid identified with kube-system namespace | Any Str | false |
| k8s.container.name | The name of the Container in a Pod template. Requires container.id. | Any Str | false |
| k8s.cronjob.name | The name of the CronJob. | Any Str | false |
Expand Down
22 changes: 12 additions & 10 deletions processor/k8sattributesprocessor/e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func newExpectedValue(mode int, value string) *expectedValue {

// TestE2E_ClusterRBAC tests the k8s attributes processor in a k8s cluster with the collector's service account having
// cluster-wide permissions to list/watch namespaces, nodes, pods and replicasets. The config in the test does not
// set filter::namespace.
// set filter::namespace, and the telemetrygen image has a latest tag but no digest.
// The test requires a prebuilt otelcontribcol image uploaded to a kind k8s cluster defined in
// `/tmp/kube-config-otelcol-e2e-testing`. Run the following command prior to running the test locally:
//
Expand Down Expand Up @@ -432,7 +432,8 @@ func TestE2E_ClusterRBAC(t *testing.T) {
}
}

// Test with `filter::namespace` set and only role binding to collector's SA. We can't get node and namespace labels/annotations.
// Test with `filter::namespace` set and only role binding to collector's SA. We can't get node and namespace labels/annotations,
// and the telemetrygen image has a digest but no tag.
func TestE2E_NamespacedRBAC(t *testing.T) {

testDir := filepath.Join("testdata", "e2e", "namespacedrbac")
Expand Down Expand Up @@ -504,7 +505,7 @@ func TestE2E_NamespacedRBAC(t *testing.T) {
"k8s.container.name": newExpectedValue(equal, "telemetrygen"),
"container.image.name": newExpectedValue(equal, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen"),
"container.image.repo_digests": newExpectedValue(regex, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen@sha256:[0-9a-fA-f]{64}"),
"container.image.tag": newExpectedValue(equal, "latest"),
"container.image.tag": newExpectedValue(shouldnotexist, ""),
ChrsMark marked this conversation as resolved.
Show resolved Hide resolved
"container.id": newExpectedValue(exist, ""),
},
},
Expand All @@ -528,7 +529,7 @@ func TestE2E_NamespacedRBAC(t *testing.T) {
"k8s.container.name": newExpectedValue(equal, "telemetrygen"),
"container.image.name": newExpectedValue(equal, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen"),
"container.image.repo_digests": newExpectedValue(regex, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen@sha256:[0-9a-fA-f]{64}"),
"container.image.tag": newExpectedValue(equal, "latest"),
"container.image.tag": newExpectedValue(shouldnotexist, ""),
"container.id": newExpectedValue(exist, ""),
},
},
Expand All @@ -552,7 +553,7 @@ func TestE2E_NamespacedRBAC(t *testing.T) {
"k8s.container.name": newExpectedValue(equal, "telemetrygen"),
"container.image.name": newExpectedValue(equal, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen"),
"container.image.repo_digests": newExpectedValue(regex, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen@sha256:[0-9a-fA-f]{64}"),
"container.image.tag": newExpectedValue(equal, "latest"),
"container.image.tag": newExpectedValue(shouldnotexist, ""),
"container.id": newExpectedValue(exist, ""),
},
},
Expand All @@ -575,7 +576,7 @@ func TestE2E_NamespacedRBAC(t *testing.T) {
}

// Test with `filter::namespace` set, role binding for namespace-scoped objects (pod, replicaset) and clusterrole
// binding for node and namespace objects.
// binding for node and namespace objects, and the telemetrygen image has a tag and digest.
func TestE2E_MixRBAC(t *testing.T) {

testDir := filepath.Join("testdata", "e2e", "mixrbac")
Expand Down Expand Up @@ -662,7 +663,7 @@ func TestE2E_MixRBAC(t *testing.T) {
"k8s.container.name": newExpectedValue(equal, "telemetrygen"),
"container.image.name": newExpectedValue(equal, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen"),
"container.image.repo_digests": newExpectedValue(regex, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen@sha256:[0-9a-fA-f]{64}"),
"container.image.tag": newExpectedValue(equal, "latest"),
"container.image.tag": newExpectedValue(equal, "0.112.0"),
"container.id": newExpectedValue(exist, ""),
"k8s.namespace.labels.foons": newExpectedValue(equal, "barns"),
"k8s.node.labels.foo": newExpectedValue(equal, "too"),
Expand All @@ -689,7 +690,7 @@ func TestE2E_MixRBAC(t *testing.T) {
"k8s.container.name": newExpectedValue(equal, "telemetrygen"),
"container.image.name": newExpectedValue(equal, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen"),
"container.image.repo_digests": newExpectedValue(regex, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen@sha256:[0-9a-fA-f]{64}"),
"container.image.tag": newExpectedValue(equal, "latest"),
"container.image.tag": newExpectedValue(equal, "0.112.0"),
"container.id": newExpectedValue(exist, ""),
"k8s.namespace.labels.foons": newExpectedValue(equal, "barns"),
"k8s.node.labels.foo": newExpectedValue(equal, "too"),
Expand All @@ -716,7 +717,7 @@ func TestE2E_MixRBAC(t *testing.T) {
"k8s.container.name": newExpectedValue(equal, "telemetrygen"),
"container.image.name": newExpectedValue(equal, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen"),
"container.image.repo_digests": newExpectedValue(regex, "ghcr.io/open-telemetry/opentelemetry-collector-contrib/telemetrygen@sha256:[0-9a-fA-f]{64}"),
"container.image.tag": newExpectedValue(equal, "latest"),
"container.image.tag": newExpectedValue(equal, "0.112.0"),
"container.id": newExpectedValue(exist, ""),
"k8s.namespace.labels.foons": newExpectedValue(equal, "barns"),
"k8s.node.labels.foo": newExpectedValue(equal, "too"),
Expand Down Expand Up @@ -745,7 +746,8 @@ func TestE2E_MixRBAC(t *testing.T) {
// While `k8s.pod.ip` is not set in `k8sattributes:extract:metadata` and the `pod_association` is not `connection`
// we expect that the `k8s.pod.ip` metadata is not added.
// While `container.image.repo_digests` is not set in `k8sattributes::extract::metadata`, we expect
// that the `container.image.repo_digests` metadata is not added
// that the `container.image.repo_digests` metadata is not added.
// The telemetrygen image has neither a tag nor digest (implicitly latest version)
func TestE2E_NamespacedRBACNoPodIP(t *testing.T) {
testDir := filepath.Join("testdata", "e2e", "namespaced_rbac_no_pod_ip")

Expand Down
41 changes: 32 additions & 9 deletions processor/k8sattributesprocessor/internal/kube/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package kube // import "github.com/open-telemetry/opentelemetry-collector-contri

import (
"context"
"errors"
"fmt"
"regexp"
"strings"
Expand Down Expand Up @@ -632,6 +633,30 @@ func removeUnnecessaryPodData(pod *api_v1.Pod, rules ExtractionRules) *api_v1.Po
return &transformedPod
}

// parseNameAndTagFromImage parses the image name and tag for differently-formatted image names.
// returns "latest" as the default if tag not present. also checks if the image contains a digest.
// if it does, no latest tag is assumed.
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
func parseNameAndTagFromImage(image string) (name, tag string, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use the already available internal docker.ParseImageName function instead?

I would prefer to all components use the same strategy for common functionalities. Note that the shared function cannot parse images with digest at the moment and should be fixed beforehand: #36279

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a single implementation makes sense.
Also I think the package should be renamed to container and not docker since it doesn't only cover docker.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a private implementation I am ok moving forward with this approach for now, but I agree it should used the shared function. Created #36418

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spiffyy99 I was gonna merge this before the release but now I want to make our test coverage is still handling when no digest is set. If you get to using docker.ParseImageName before this is merged I'll close the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I can update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, looking at this further, wouldn't it be better to use the official docker api code for things like this? i'm sure this regex is correct as well but I'd think that the official one is a better practice (reference.Parse).

another option is we can update docker.ParseImageName to use reference.Parse, but it seems like that function is used in a bunch

i'm not the one maintaining this though, so I don't feel strongly. just pointing it out

ref, err := reference.Parse(image)
if err != nil {
return
}
namedRef, ok := ref.(reference.Named)
if !ok {
return "", "", errors.New("cannot retrieve image name")
}
name = namedRef.Name()
if taggedRef, ok := namedRef.(reference.Tagged); ok {
tag = taggedRef.Tag()
}
if tag == "" {
if digestedRef, ok := namedRef.(reference.Digested); !ok || digestedRef.String() == "" {
tag = "latest"
}
}
return
}

func (c *WatchClient) extractPodContainersAttributes(pod *api_v1.Pod) PodContainers {
containers := PodContainers{
ByID: map[string]*Container{},
Expand All @@ -643,16 +668,14 @@ func (c *WatchClient) extractPodContainersAttributes(pod *api_v1.Pod) PodContain
if c.Rules.ContainerImageName || c.Rules.ContainerImageTag {
for _, spec := range append(pod.Spec.Containers, pod.Spec.InitContainers...) {
container := &Container{}
nameTagSep := strings.LastIndex(spec.Image, ":")
if c.Rules.ContainerImageName {
if nameTagSep > 0 {
container.ImageName = spec.Image[:nameTagSep]
} else {
container.ImageName = spec.Image
name, tag, err := parseNameAndTagFromImage(spec.Image)
ChrsMark marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
if c.Rules.ContainerImageName {
container.ImageName = name
}
if c.Rules.ContainerImageTag {
ChrsMark marked this conversation as resolved.
Show resolved Hide resolved
container.ImageTag = tag
}
}
if c.Rules.ContainerImageTag && nameTagSep > 0 {
container.ImageTag = spec.Image[nameTagSep+1:]
}
containers.ByName[spec.Name] = container
}
Expand Down
76 changes: 60 additions & 16 deletions processor/k8sattributesprocessor/internal/kube/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,17 +1469,21 @@ func Test_extractPodContainersAttributes(t *testing.T) {
Containers: []api_v1.Container{
{
Name: "container1",
Image: "test/image1:0.1.0",
Image: "example.com:5000/test/image1:0.1.0",
},
{
Name: "container2",
Image: "example.com:port1/image2:0.2.0",
Image: "example.com:81/image2@sha256:430ac608abaa332de4ce45d68534447c7a206edc5e98aaff9923ecc12f8a80d9",
},
{
Name: "container3",
Image: "example-website.com/image3:1.0@sha256:4b0b1b6f6cdd3e5b9e55f74a1e8d19ed93a3f5a04c6b6c3c57c4e6d19f6b7c4d",
},
},
InitContainers: []api_v1.Container{
{
Name: "init_container",
Image: "test/init-image:1.0.2",
Image: "test/init-image",
},
},
},
Expand All @@ -1494,7 +1498,13 @@ func Test_extractPodContainersAttributes(t *testing.T) {
{
Name: "container2",
ContainerID: "docker://container2-id-456",
ImageID: "sha256:430ac608abaa332de4ce45d68534447c7a206edc5e98aaff9923ecc12f8a80d9",
ImageID: "sha256:4b0b1b6f6cdd3e5b9e55f74a1e8d19ed93a3f5a04c6b6c3c57c4e6d19f6b7c4d",
RestartCount: 2,
},
{
Name: "container3",
ContainerID: "docker://container3-id-abc",
ImageID: "docker.io/otel/collector:2.0.0@sha256:430ac608abaa332de4ce45d68534447c7a206edc5e98aaff9923ecc12f8a80d9",
RestartCount: 2,
},
},
Expand Down Expand Up @@ -1538,13 +1548,15 @@ func Test_extractPodContainersAttributes(t *testing.T) {
pod: &pod,
want: PodContainers{
ByID: map[string]*Container{
"container1-id-123": {ImageName: "test/image1"},
"container2-id-456": {ImageName: "example.com:port1/image2"},
"container1-id-123": {ImageName: "example.com:5000/test/image1"},
"container2-id-456": {ImageName: "example.com:81/image2"},
"container3-id-abc": {ImageName: "example-website.com/image3"},
"init-container-id-789": {ImageName: "test/init-image"},
},
ByName: map[string]*Container{
"container1": {ImageName: "test/image1"},
"container2": {ImageName: "example.com:port1/image2"},
"container1": {ImageName: "example.com:5000/test/image1"},
"container2": {ImageName: "example.com:81/image2"},
"container3": {ImageName: "example-website.com/image3"},
"init_container": {ImageName: "test/init-image"},
},
},
Expand Down Expand Up @@ -1589,6 +1601,11 @@ func Test_extractPodContainersAttributes(t *testing.T) {
2: {ContainerID: "container2-id-456"},
},
},
"container3-id-abc": {
Statuses: map[int]ContainerStatus{
2: {ContainerID: "container3-id-abc"},
},
},
"init-container-id-789": {
Statuses: map[int]ContainerStatus{
0: {ContainerID: "init-container-id-789"},
Expand All @@ -1606,6 +1623,11 @@ func Test_extractPodContainersAttributes(t *testing.T) {
2: {ContainerID: "container2-id-456"},
},
},
"container3": {
Statuses: map[int]ContainerStatus{
2: {ContainerID: "container3-id-abc"},
},
},
"init_container": {
Statuses: map[int]ContainerStatus{
0: {ContainerID: "init-container-id-789"},
Expand All @@ -1632,6 +1654,11 @@ func Test_extractPodContainersAttributes(t *testing.T) {
2: {},
},
},
"container3-id-abc": {
Statuses: map[int]ContainerStatus{
2: {ImageRepoDigest: "docker.io/otel/collector:2.0.0@sha256:430ac608abaa332de4ce45d68534447c7a206edc5e98aaff9923ecc12f8a80d9"},
},
},
"init-container-id-789": {
Statuses: map[int]ContainerStatus{
0: {ImageRepoDigest: "ghcr.io/initimage1@sha256:42e8ba40f9f70d604684c3a2a0ed321206b7e2e3509fdb2c8836d34f2edfb57b"},
Expand All @@ -1649,6 +1676,11 @@ func Test_extractPodContainersAttributes(t *testing.T) {
2: {},
},
},
"container3": {
Statuses: map[int]ContainerStatus{
2: {ImageRepoDigest: "docker.io/otel/collector:2.0.0@sha256:430ac608abaa332de4ce45d68534447c7a206edc5e98aaff9923ecc12f8a80d9"},
},
},
"init_container": {
Statuses: map[int]ContainerStatus{
0: {ImageRepoDigest: "ghcr.io/initimage1@sha256:42e8ba40f9f70d604684c3a2a0ed321206b7e2e3509fdb2c8836d34f2edfb57b"},
Expand All @@ -1669,45 +1701,57 @@ func Test_extractPodContainersAttributes(t *testing.T) {
want: PodContainers{
ByID: map[string]*Container{
"container1-id-123": {
ImageName: "test/image1",
ImageName: "example.com:5000/test/image1",
ImageTag: "0.1.0",
Statuses: map[int]ContainerStatus{
0: {ContainerID: "container1-id-123", ImageRepoDigest: "docker.io/otel/collector@sha256:55d008bc28344c3178645d40e7d07df30f9d90abe4b53c3fc4e5e9c0295533da"},
},
},
"container2-id-456": {
ImageName: "example.com:port1/image2",
ImageTag: "0.2.0",
ImageName: "example.com:81/image2",
Statuses: map[int]ContainerStatus{
2: {ContainerID: "container2-id-456"},
},
},
"container3-id-abc": {
ImageName: "example-website.com/image3",
ImageTag: "1.0",
Statuses: map[int]ContainerStatus{
2: {ContainerID: "container3-id-abc", ImageRepoDigest: "docker.io/otel/collector:2.0.0@sha256:430ac608abaa332de4ce45d68534447c7a206edc5e98aaff9923ecc12f8a80d9"},
},
},
"init-container-id-789": {
ImageName: "test/init-image",
ImageTag: "1.0.2",
ImageTag: "latest",
Statuses: map[int]ContainerStatus{
0: {ContainerID: "init-container-id-789", ImageRepoDigest: "ghcr.io/initimage1@sha256:42e8ba40f9f70d604684c3a2a0ed321206b7e2e3509fdb2c8836d34f2edfb57b"},
},
},
},
ByName: map[string]*Container{
"container1": {
ImageName: "test/image1",
ImageName: "example.com:5000/test/image1",
ImageTag: "0.1.0",
Statuses: map[int]ContainerStatus{
0: {ContainerID: "container1-id-123", ImageRepoDigest: "docker.io/otel/collector@sha256:55d008bc28344c3178645d40e7d07df30f9d90abe4b53c3fc4e5e9c0295533da"},
},
},
"container2": {
ImageName: "example.com:port1/image2",
ImageTag: "0.2.0",
ImageName: "example.com:81/image2",
Statuses: map[int]ContainerStatus{
2: {ContainerID: "container2-id-456"},
},
},
"container3": {
ImageName: "example-website.com/image3",
ImageTag: "1.0",
Statuses: map[int]ContainerStatus{
2: {ContainerID: "container3-id-abc", ImageRepoDigest: "docker.io/otel/collector:2.0.0@sha256:430ac608abaa332de4ce45d68534447c7a206edc5e98aaff9923ecc12f8a80d9"},
},
},
"init_container": {
ImageName: "test/init-image",
ImageTag: "1.0.2",
ImageTag: "latest",
Statuses: map[int]ContainerStatus{
0: {ContainerID: "init-container-id-789", ImageRepoDigest: "ghcr.io/initimage1@sha256:42e8ba40f9f70d604684c3a2a0ed321206b7e2e3509fdb2c8836d34f2edfb57b"},
},
Expand Down
2 changes: 1 addition & 1 deletion processor/k8sattributesprocessor/metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ resource_attributes:
type: slice
enabled: false
container.image.tag:
description: Container image tag. Requires container.id or k8s.container.name.
description: Container image tag. Defaults to "latest" if not provided (unless digest also in image path) Requires container.id or k8s.container.name.
type: string
enabled: true

Expand Down
Loading