Skip to content

Commit

Permalink
unify inclusion/exclusion logic for container images (#32013)
Browse files Browse the repository at this point in the history 
Co-authored-by: DeForest Richards <[email protected]>
  • Loading branch information
@adel121 @drichards-87
adel121 and drichards-87 authored Dec 16, 2024
1 parent 918a8c3 commit e681f1f
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 12 deletions.
2 changes: 1 addition & 1 deletion pkg/collector/corechecks/containers/generic/filters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (f LegacyContainerFilter) IsExcluded(container *workloadmeta.Container) boo
annotations = pod.Annotations
}

return f.OldFilter.IsExcluded(annotations, container.Name, container.Image.Name, container.Labels[kubernetes.CriContainerNamespaceLabel])
return f.OldFilter.IsExcluded(annotations, container.Name, container.Image.RawName, container.Labels[kubernetes.CriContainerNamespaceLabel])
}

// RuntimeContainerFilter filters containers by runtime
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/corechecks/containers/kubelet/common/pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func GetContainerID(store workloadmeta.Component, metric model.Metric, filter *c
return "", ErrContainerNotFound
}

if filter.IsExcluded(pod.EntityMeta.Annotations, container.Name, container.Image.Name, pod.Namespace) {
if filter.IsExcluded(pod.EntityMeta.Annotations, container.Name, container.Image.RawName, pod.Namespace) {
return "", ErrContainerExcluded
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/process/util/containers/containers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (p *containerProvider) GetContainers(cacheValidity time.Duration, previousC
annotations = pod.Annotations
}

if p.filter != nil && p.filter.IsExcluded(annotations, container.Name, container.Image.Name, container.Labels[kubernetes.CriContainerNamespaceLabel]) {
if p.filter != nil && p.filter.IsExcluded(annotations, container.Name, container.Image.RawName, container.Labels[kubernetes.CriContainerNamespaceLabel]) {
continue
}

Expand Down Expand Up @@ -209,7 +209,7 @@ func (p *containerProvider) GetPidToCid(cacheValidity time.Duration) map[int]str
annotations = pod.Annotations
}

if p.filter != nil && p.filter.IsExcluded(annotations, container.Name, container.Image.Name, container.Labels[kubernetes.CriContainerNamespaceLabel]) {
if p.filter != nil && p.filter.IsExcluded(annotations, container.Name, container.Image.RawName, container.Labels[kubernetes.CriContainerNamespaceLabel]) {
continue
}

Expand Down
23 changes: 23 additions & 0 deletions pkg/util/containers/filter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ func parseFilters(filters []string) (imageFilters, nameFilters, namespaceFilters
for _, filter := range filters {
switch {
case strings.HasPrefix(filter, imageFilterPrefix):
filter = preprocessImageFilter(filter)
r, err := filterToRegex(filter, imageFilterPrefix)
if err != nil {
filterErrs = append(filterErrs, err.Error())
Expand Down Expand Up @@ -145,6 +146,19 @@ func parseFilters(filters []string) (imageFilters, nameFilters, namespaceFilters
return imageFilters, nameFilters, namespaceFilters, filterWarnings, nil
}

// preprocessImageFilter modifies image filters having the format `name$`, where {name} doesn't include a colon (e.g. nginx$, ^nginx$), to
// `name:.*`.
// This is done so that image filters can still match even if the matched image contains the tag or digest.
func preprocessImageFilter(imageFilter string) string {
regexVal := strings.TrimPrefix(imageFilter, imageFilterPrefix)
if strings.HasSuffix(regexVal, "$") && !strings.Contains(regexVal, ":") {
mutatedRegexVal := regexVal[:len(regexVal)-1] + "(@sha256)?:.*"
return imageFilterPrefix + mutatedRegexVal
}

return imageFilter
}

// filterToRegex checks a filter's regex
func filterToRegex(filter string, filterPrefix string) (*regexp.Regexp, error) {
pat := strings.TrimPrefix(filter, filterPrefix)
Expand Down Expand Up @@ -327,7 +341,16 @@ func NewAutodiscoveryFilter(ft FilterType) (*Filter, error) {
// based on the filters in the containerFilter instance. Consider also using
// Note: exclude filters are not applied to empty container names, empty
// images and empty namespaces.
//
// containerImage may or may not contain the image tag or image digest. (e.g. nginx:latest and nginx are both valid)
func (cf Filter) IsExcluded(annotations map[string]string, containerName, containerImage, podNamespace string) bool {

// If containerImage doesn't include the tag or digest, add a colon so that it
// can match image filters
if len(containerImage) > 0 && !strings.Contains(containerImage, ":") {
containerImage += ":"
}

if cf.isExcludedByAnnotation(annotations, containerName) {
return true
}
Expand Down
51 changes: 43 additions & 8 deletions pkg/util/containers/filter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ func TestFilter(t *testing.T) {
c ctnDef
ns string
}{
{
c: ctnDef{
ID: "0",
Name: "container-with-sha",
Image: "docker-dd-agent@sha256:1892862abcdef61516516",
},
ns: "default",
},
{
c: ctnDef{
ID: "1",
Expand Down Expand Up @@ -268,25 +276,33 @@ func TestFilter(t *testing.T) {
expectedIDs []string
}{
{
expectedIDs: []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
expectedIDs: []string{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
excludeList: []string{"name:secret"},
excludeList: []string{"image:^docker-dd-agent$"},
expectedIDs: []string{"2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
excludeList: []string{"image:^apache$"},
expectedIDs: []string{"0", "1", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
excludeList: []string{"name:secret"},
expectedIDs: []string{"0", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
excludeList: []string{"image:secret"},
expectedIDs: []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
expectedIDs: []string{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
includeList: []string{},
excludeList: []string{"image:apache", "image:alpine"},
expectedIDs: []string{"1", "3", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
expectedIDs: []string{"0", "1", "3", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
includeList: []string{"name:mysql"},
excludeList: []string{"name:dd"},
expectedIDs: []string{"3", "5", "6", "7", "8", "9", "10", "11", "12", "13", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
expectedIDs: []string{"0", "3", "5", "6", "7", "8", "9", "10", "11", "12", "13", "16", "17", "18", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
excludeList: []string{"kube_namespace:.*"},
Expand All @@ -295,7 +311,7 @@ func TestFilter(t *testing.T) {
},
{
excludeList: []string{"kube_namespace:bar"},
expectedIDs: []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
expectedIDs: []string{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "19", "20", "23", "24", "25", "26", "27", "28", "29", "30", "31"},
},
{
excludeList: []string{"name:.*"},
Expand All @@ -305,7 +321,17 @@ func TestFilter(t *testing.T) {
{
excludeList: []string{"image:.*"},
includeList: []string{"image:docker-dd-agent"},
expectedIDs: []string{"1", "30"},
expectedIDs: []string{"0", "1", "30"},
},
{
excludeList: []string{"image:.*"},
includeList: []string{"image:^docker-dd-agent$"},
expectedIDs: []string{"0", "1", "30"},
},
{
excludeList: []string{"image:.*"},
includeList: []string{"image:^docker-dd-agent$"},
expectedIDs: []string{"0", "1", "30"},
},
// Test kubernetes defaults
{
Expand All @@ -326,7 +352,7 @@ func TestFilter(t *testing.T) {
pauseContainerUpstream,
pauseContainerCDK,
},
expectedIDs: []string{"1", "2", "3", "4", "5", "14", "15", "29", "30", "31"},
expectedIDs: []string{"0", "1", "2", "3", "4", "5", "14", "15", "29", "30", "31"},
},
} {
t.Run("", func(t *testing.T) {
Expand Down Expand Up @@ -624,6 +650,15 @@ func TestParseFilters(t *testing.T) {
expectedErrMsg: nil,
filterErrors: nil,
},
{
desc: "valid filters, image filter strict match without tag or digest",
filters: []string{"image:^nginx$", "name:xyz-.*", "kube_namespace:sandbox.*", "name:abc"},
imageFilters: []*regexp.Regexp{regexp.MustCompile("^nginx(@sha256)?:.*")},
nameFilters: []*regexp.Regexp{regexp.MustCompile("xyz-.*"), regexp.MustCompile("abc")},
namespaceFilters: []*regexp.Regexp{regexp.MustCompile("sandbox.*")},
expectedErrMsg: nil,
filterErrors: nil,
},
{
desc: "invalid regex",
filters: []string{"image:apache.*", "name:a(?=b)", "kube_namespace:sandbox.*", "name:abc"},
Expand Down
16 changes: 16 additions & 0 deletions releasenotes/notes/make_image_filters_consistent-14fd44a352723e50.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
fixes:
- |
Fixes consistency issue with container image filters.
Depending on the Agent configuration, filters were sometimes behaving differently
for metrics and logs. For example, an image filter that worked for excluding logs
didn't work when used to exclude metrics, and vice versa.
The exclusion logic is now consistent between metrics and logs.

0 comments on commit e681f1f

Please sign in to comment.