Skip to content

Commit

Permalink
[processor/k8sattributes] Move config validation into Validate (#25153)
Browse files Browse the repository at this point in the history 
**Description:** 
Moves existing config validation into the Validate function

**Testing:** 
Added more unit tests

---------

Co-authored-by: Dmitrii Anoshin <anoshindx@gmail.com>
  • Loading branch information
@TylerHelmuth @dmitryax
TylerHelmuth and dmitryax authored Aug 15, 2023
1 parent edd4bfd commit ec549be
Show file tree
Hide file tree
Showing 7 changed files with 236 additions and 153 deletions.
63 changes: 63 additions & 0 deletions processor/k8sattributesprocessor/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ package k8sattributesprocessor // import "github.com/open-telemetry/opentelemetr

import (
"fmt"
"regexp"

conventions "go.opentelemetry.io/collector/semconv/v1.6.1"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/k8sconfig"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/k8sattributesprocessor/internal/kube"
Expand Down Expand Up @@ -48,6 +51,66 @@ func (cfg *Config) Validate() error {
}
}

for _, f := range append(cfg.Extract.Labels, cfg.Extract.Annotations...) {
if f.Key != "" && f.KeyRegex != "" {
return fmt.Errorf("Out of Key or KeyRegex only one option is expected to be configured at a time, currently Key:%s and KeyRegex:%s", f.Key, f.KeyRegex)
}

switch f.From {
case "", kube.MetadataFromPod, kube.MetadataFromNamespace:
default:
return fmt.Errorf("%s is not a valid choice for From. Must be one of: pod, namespace", f.From)
}

if f.Regex != "" {
r, err := regexp.Compile(f.Regex)
if err != nil {
return err
}
names := r.SubexpNames()
if len(names) != 2 || names[1] != "value" {
return fmt.Errorf("regex must contain exactly one named submatch (value)")
}
}

if f.KeyRegex != "" {
_, err := regexp.Compile("^(?:" + f.KeyRegex + ")$")
if err != nil {
return err
}
}
}

for _, field := range cfg.Extract.Metadata {
switch field {
case conventions.AttributeK8SNamespaceName, conventions.AttributeK8SPodName, conventions.AttributeK8SPodUID,
specPodHostName, metadataPodStartTime, conventions.AttributeK8SDeploymentName, conventions.AttributeK8SDeploymentUID,
conventions.AttributeK8SReplicaSetName, conventions.AttributeK8SReplicaSetUID, conventions.AttributeK8SDaemonSetName,
conventions.AttributeK8SDaemonSetUID, conventions.AttributeK8SStatefulSetName, conventions.AttributeK8SStatefulSetUID,
conventions.AttributeK8SContainerName, conventions.AttributeK8SJobName, conventions.AttributeK8SJobUID,
conventions.AttributeK8SCronJobName, conventions.AttributeK8SNodeName, conventions.AttributeContainerID,
conventions.AttributeContainerImageName, conventions.AttributeContainerImageTag, clusterUID:
default:
return fmt.Errorf("\"%s\" is not a supported metadata field", field)
}
}

for _, f := range cfg.Filter.Labels {
switch f.Op {
case "", filterOPEquals, filterOPNotEquals, filterOPExists, filterOPDoesNotExist:
default:
return fmt.Errorf("'%s' is not a valid label filter operation for key=%s, value=%s", f.Op, f.Key, f.Value)
}
}

for _, f := range cfg.Filter.Fields {
switch f.Op {
case "", filterOPEquals, filterOPNotEquals:
default:
return fmt.Errorf("'%s' is not a valid label filter operation for key=%s, value=%s", f.Op, f.Key, f.Value)
}
}

return nil
}

Expand Down
50 changes: 50 additions & 0 deletions processor/k8sattributesprocessor/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,51 @@ func TestLoadConfig(t *testing.T) {
},
},
},
{
id: component.NewIDWithName(metadata.Type, "too_many_sources"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_keys_labels"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_keys_annotations"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_from_labels"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_from_annotations"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_regex_labels"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_regex_annotations"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_keyregex_labels"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_keyregex_annotations"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_regex_groups_labels"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_regex_groups_annotations"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_regex_name_labels"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_regex_name_annotations"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_filter_label_op"),
},
{
id: component.NewIDWithName(metadata.Type, "bad_filter_field_op"),
},
}

for _, tt := range tests {
Expand All @@ -137,6 +182,11 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, err)
require.NoError(t, component.UnmarshalConfig(sub, cfg))

if tt.expected == nil {
err = component.ValidateConfig(cfg)
assert.Error(t, err)
return
}
assert.NoError(t, component.ValidateConfig(cfg))
assert.Equal(t, tt.expected, cfg)
})
Expand Down
18 changes: 0 additions & 18 deletions processor/k8sattributesprocessor/factory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package k8sattributesprocessor // import "github.com/open-telemetry/opentelemetr

import (
"context"
"fmt"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer"
Expand Down Expand Up @@ -142,11 +141,6 @@ func createKubernetesProcessor(
) (*kubernetesprocessor, error) {
kp := &kubernetesprocessor{logger: params.Logger}

err := errWrongKeyConfig(cfg)
if err != nil {
return nil, err
}

allOptions := append(createProcessorOpts(cfg), options...)

for _, opt := range allOptions {
Expand Down Expand Up @@ -191,15 +185,3 @@ func createProcessorOpts(cfg component.Config) []option {

return opts
}

func errWrongKeyConfig(cfg component.Config) error {
oCfg := cfg.(*Config)

for _, r := range append(oCfg.Extract.Labels, oCfg.Extract.Annotations...) {
if r.Key != "" && r.KeyRegex != "" {
return fmt.Errorf("Out of Key or KeyRegex only one option is expected to be configured at a time, currently Key:%s and KeyRegex:%s", r.Key, r.KeyRegex)
}
}

return nil
}
30 changes: 3 additions & 27 deletions processor/k8sattributesprocessor/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ func withExtractMetadata(fields ...string) option {
p.rules.ContainerImageTag = true
case clusterUID:
p.rules.ClusterUID = true
default:
return fmt.Errorf("\"%s\" is not a supported metadata field", field)
}
}
return nil
Expand Down Expand Up @@ -211,14 +209,8 @@ func extractFieldRules(fieldType string, fields ...FieldExtractConfig) ([]kube.F
for _, a := range fields {
name := a.TagName

switch a.From {
// By default if the From field is not set for labels and annotations we want to extract them from pod
case "", kube.MetadataFromPod:
if a.From == "" {
a.From = kube.MetadataFromPod
case kube.MetadataFromNamespace:
a.From = kube.MetadataFromNamespace
default:
return rules, fmt.Errorf("%s is not a valid choice for From. Must be one of: pod, namespace", a.From)
}

if name == "" && a.Key != "" {
Expand All @@ -237,10 +229,6 @@ func extractFieldRules(fieldType string, fields ...FieldExtractConfig) ([]kube.F
if err != nil {
return rules, err
}
names := r.SubexpNames()
if len(names) != 2 || names[1] != "value" {
return rules, fmt.Errorf("regex must contain exactly one named submatch (value)")
}
}

var keyRegex *regexp.Regexp
Expand Down Expand Up @@ -289,22 +277,16 @@ func withFilterLabels(filters ...FieldFilterConfig) option {
return func(p *kubernetesprocessor) error {
var labels []kube.FieldFilter
for _, f := range filters {
if f.Op == "" {
f.Op = filterOPEquals
}

var op selection.Operator
switch f.Op {
case filterOPEquals:
op = selection.Equals
case filterOPNotEquals:
op = selection.NotEquals
case filterOPExists:
op = selection.Exists
case filterOPDoesNotExist:
op = selection.DoesNotExist
default:
return fmt.Errorf("'%s' is not a valid label filter operation for key=%s, value=%s", f.Op, f.Key, f.Value)
op = selection.Equals
}
labels = append(labels, kube.FieldFilter{
Key: f.Key,
Expand All @@ -322,18 +304,12 @@ func withFilterFields(filters ...FieldFilterConfig) option {
return func(p *kubernetesprocessor) error {
var fields []kube.FieldFilter
for _, f := range filters {
if f.Op == "" {
f.Op = filterOPEquals
}

var op selection.Operator
switch f.Op {
case filterOPEquals:
op = selection.Equals
case filterOPNotEquals:
op = selection.NotEquals
default:
return fmt.Errorf("'%s' is not a valid field filter operation for key=%s, value=%s", f.Op, f.Key, f.Value)
op = selection.Equals
}
fields = append(fields, kube.FieldFilter{
Key: f.Key,
Expand Down
97 changes: 0 additions & 97 deletions processor/k8sattributesprocessor/options_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,6 @@ func TestWithExtractAnnotations(t *testing.T) {
nil,
"",
},
{
"bad",
[]FieldExtractConfig{
{
TagName: "tag1",
Key: "key1",
Regex: "[",
From: kube.MetadataFromPod,
},
},
[]kube.FieldExtractionRule{},
"error parsing regexp: missing closing ]: `[`",
},
{
"basic",
[]FieldExtractConfig{
Expand Down Expand Up @@ -203,17 +190,6 @@ func TestWithExtractLabels(t *testing.T) {
nil,
"",
},
{
"bad",
[]FieldExtractConfig{{
TagName: "t1",
Key: "k1",
Regex: "[",
From: kube.MetadataFromPod,
}},
[]kube.FieldExtractionRule{},
"error parsing regexp: missing closing ]: `[`",
},
{
"basic",
[]FieldExtractConfig{
Expand Down Expand Up @@ -316,11 +292,6 @@ func TestWithExtractMetadata(t *testing.T) {
assert.True(t, p.rules.DeploymentName)
assert.True(t, p.rules.Node)

p = &kubernetesprocessor{}
err := withExtractMetadata("randomfield")(p)
assert.Error(t, err)
assert.Equal(t, `"randomfield" is not a supported metadata field`, err.Error())

p = &kubernetesprocessor{}
assert.NoError(t, withExtractMetadata(conventions.AttributeK8SNamespaceName, conventions.AttributeK8SPodName, conventions.AttributeK8SPodUID)(p))
assert.True(t, p.rules.Namespace)
Expand Down Expand Up @@ -429,18 +400,6 @@ func TestWithFilterLabels(t *testing.T) {
},
"",
},
{
"unknown",
[]FieldFilterConfig{
{
Key: "k1",
Value: "v1",
Op: "unknown-op",
},
},
[]kube.FieldFilter{},
"'unknown-op' is not a valid label filter operation for key=k1, value=v1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -525,50 +484,6 @@ func TestWithFilterFields(t *testing.T) {
},
"",
},
{
"exists",
[]FieldFilterConfig{
{
Key: "k1",
Op: "exists",
},
},
[]kube.FieldFilter{
{
Key: "k1",
Op: selection.Exists,
},
},
"'exists' is not a valid field filter operation for key=k1, value=",
},
{
"does-not-exist",
[]FieldFilterConfig{
{
Key: "k1",
Op: "does-not-exist",
},
},
[]kube.FieldFilter{
{
Key: "k1",
Op: selection.DoesNotExist,
},
},
"'does-not-exist' is not a valid field filter operation for key=k1, value=",
},
{
"unknown",
[]FieldFilterConfig{
{
Key: "k1",
Value: "v1",
Op: "unknown-op",
},
},
[]kube.FieldFilter{},
"'unknown-op' is not a valid field filter operation for key=k1, value=v1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -630,18 +545,6 @@ func Test_extractFieldRules(t *testing.T) {
},
},
},
{
name: "regex-without-match",
args: args{"field", []FieldExtractConfig{
{
TagName: "name",
Key: "key",
Regex: "^h$",
From: kube.MetadataFromPod,
},
}},
wantErr: true,
},
{
name: "badregex",
args: args{"field", []FieldExtractConfig{
Expand Down
Loading

0 comments on commit ec549be

Please sign in to comment.