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.

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

Don't overwrite fields of struct field with tag child:"true" #638

Merged
merged 11 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 6 additions & 38 deletions cfg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,14 +259,13 @@ func Parse(ptr any, values map[string]int) error {
return nil
}

childs := make([]reflect.Value, 0)
nestedConfigs := make([]reflect.Value, 0)
for i := 0; i < t.NumField(); i++ {
vField := v.Field(i)
tField := t.Field(i)

childTag := tField.Tag.Get("child")
if childTag == trueValue {
childs = append(childs, vField)
if vField.Kind() == reflect.Struct {
nestedConfigs = append(nestedConfigs, vField)
continue
}

Expand All @@ -284,48 +283,17 @@ func Parse(ptr any, values map[string]int) error {
}
}

for _, child := range childs {
if err := ParseChild(v, child, values); err != nil {
for _, nestedConfig := range nestedConfigs {
if err := Parse(nestedConfig.Addr().Interface(), values); err != nil {
return err
}
}

return nil
}

// it isn't just a recursion
// it also captures values with the same name from parent
// i.e. take this config:
//
// {
// "T": 10,
// "Child": { // has `child:true` in a tag
// "T": null
// }
// }
//
// this function will set `config.Child.T = config.T`
// see file.d/cfg/config_test.go:TestHierarchy for an example
func ParseChild(parent reflect.Value, v reflect.Value, values map[string]int) error {
if v.CanAddr() {
for i := 0; i < v.NumField(); i++ {
name := v.Type().Field(i).Name
val := parent.FieldByName(name)
if val.CanAddr() {
v.Field(i).Set(val)
}
}

err := Parse(v.Addr().Interface(), values)
if err != nil {
return err
}
}
return nil
}

// ParseSlice recursively parses elements of an slice
// calls Parse, not ParseChild (!)
// calls Parse
func ParseSlice(v reflect.Value, values map[string]int) error {
for i := 0; i < v.Len(); i++ {
if err := Parse(v.Index(i).Addr().Interface(), values); err != nil {
Expand Down
38 changes: 20 additions & 18 deletions cfg/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,6 @@ type strDataUnit struct {
T_ uint
}

type hierarchyChild struct {
T string `required:"true"`
}

type hierarchy struct {
T string `default:"sync"`
Child hierarchyChild `child:"true"`
}

type sliceChild struct {
Value string `default:"child"`
}
Expand Down Expand Up @@ -346,15 +337,6 @@ func TestParseFieldSelectorEnding(t *testing.T) {
assert.Equal(t, "c.", path[2], "wrong field")
}

func TestHierarchy(t *testing.T) {
s := &hierarchy{T: "10"}
err := Parse(s, map[string]int{})

assert.Nil(t, err, "shouldn't be an error")
assert.Equal(t, "10", s.T, "wrong value")
assert.Equal(t, "10", s.Child.T, "wrong value")
}

func TestSlice(t *testing.T) {
s := &sliceStruct{Value: "parent_value", Childs: []sliceChild{{"child_1"}, {}}}
SetDefaultValues(s)
Expand Down Expand Up @@ -643,3 +625,23 @@ func TestExpression_UnmarshalJSON(t *testing.T) {
require.Equal(t, Expression("2"), val.E2)
require.Equal(t, Expression("2+2"), val.E3)
}

type parentCfg struct {
Field childCfg
}

type childCfg struct {
T Duration `default:"5s" parse:"duration"`
T_ time.Duration
}

func TestParseNested(t *testing.T) {
s := &parentCfg{Field: childCfg{T: "20s"}}
err := SetDefaultValues(s)
require.NoError(t, err)

err = Parse(s, nil)
require.NoError(t, err, "error not expected")

require.Equal(t, 20*time.Second, s.Field.T_, "wrong value")
}
5 changes: 3 additions & 2 deletions plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,11 @@ pipelines:
example_k8s_pipeline:
input:
type: k8s
offsets_file: /data/offsets.yaml
file_config: // customize file plugin
file_config: # customize file plugin
persistence_mode: sync
read_buffer_size: 2048
offsets_file: /data/offsets.yaml
watching_dir: /var/log/containers
```

[More details...](plugin/input/k8s/README.md)
Expand Down
2 changes: 1 addition & 1 deletion plugin/action/throttle/throttle.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ type Config struct {
// > @3@4@5@6
// >
// > It contains redis settings
RedisBackendCfg RedisBackendConfig `json:"redis_backend_config" child:"true"` // *
RedisBackendCfg RedisBackendConfig `json:"redis_backend_config"` // *

// > @3@4@5@6
// >
Expand Down
5 changes: 3 additions & 2 deletions plugin/input/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,11 @@ pipelines:
example_k8s_pipeline:
input:
type: k8s
offsets_file: /data/offsets.yaml
file_config: // customize file plugin
file_config: # customize file plugin
persistence_mode: sync
read_buffer_size: 2048
offsets_file: /data/offsets.yaml
watching_dir: /var/log/containers
```

[More details...](plugin/input/k8s/README.md)
Expand Down
4 changes: 2 additions & 2 deletions plugin/input/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ type Config struct {
// > Disabled by default.
// > See AuthConfig for details.
// > You can use 'warn' log level for logging authorizations.
Auth AuthConfig `json:"auth" child:"true"` // *
Auth AuthConfig `json:"auth"` // *

// > @3@4@5@6
// >
Expand All @@ -165,7 +165,7 @@ type Config struct {
// >
// > CORS config.
// > See CORSConfig for details.
CORS CORSConfig `json:"cors" child:"true"` // *
CORS CORSConfig `json:"cors"` // *
}

type AuthStrategy byte
Expand Down
9 changes: 7 additions & 2 deletions plugin/input/k8s/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ pipelines:
example_k8s_pipeline:
input:
type: k8s
offsets_file: /data/offsets.yaml
file_config: // customize file plugin
file_config: # customize file plugin
persistence_mode: sync
read_buffer_size: 2048
offsets_file: /data/offsets.yaml
watching_dir: /var/log/containers
```

### Config params
Expand Down Expand Up @@ -55,12 +56,16 @@ Skips retrieving Kubernetes meta information using Kubernetes API and adds only

**`watching_dir`** *`string`* *`default=/var/log/containers`*

DEPRECATED: you must fill `file_config.watching_dir` instead!

Kubernetes dir with container logs. It's like `watching_dir` parameter from [file plugin](/plugin/input/file/README.md) config.

<br>

**`offsets_file`** *`string`* *`required`*

DEPRECATED: you must fill `file_config.offsets_file` instead!

The filename to store offsets of processed files. It's like `offsets_file` parameter from [file plugin](/plugin/input/file/README.md) config.

<br>
Expand Down
22 changes: 19 additions & 3 deletions plugin/input/k8s/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@
example_k8s_pipeline:
input:
type: k8s
offsets_file: /data/offsets.yaml
file_config: // customize file plugin
file_config: # customize file plugin
persistence_mode: sync
read_buffer_size: 2048
offsets_file: /data/offsets.yaml
watching_dir: /var/log/containers
```
}*/

Expand Down Expand Up @@ -79,19 +80,23 @@

// > @3@4@5@6
// >
// > DEPRECATED: you must fill `file_config.watching_dir` instead!
// >
// > Kubernetes dir with container logs. It's like `watching_dir` parameter from [file plugin](/plugin/input/file/README.md) config.
WatchingDir string `json:"watching_dir" default:"/var/log/containers"` // *
WatchingDir_ string

// > @3@4@5@6
// >
// > DEPRECATED: you must fill `file_config.offsets_file` instead!
// >
// > The filename to store offsets of processed files. It's like `offsets_file` parameter from [file plugin](/plugin/input/file/README.md) config.
OffsetsFile string `json:"offsets_file" required:"true"` // *

// > @3@4@5@6
// >
// > Under the hood this plugin uses [file plugin](/plugin/input/file/README.md) to collect logs from files. So you can change any [file plugin](/plugin/input/file/README.md) config parameter using `file_config` section. Check out an example.
FileConfig file.Config `json:"file_config" child:"true"` // *
FileConfig file.Config `json:"file_config"` // *
}

var startCounter atomic.Int32
Expand Down Expand Up @@ -127,6 +132,17 @@
p.params = params
p.config = config.(*Config)

if p.config.OffsetsFile != "" {
p.logger.Error("Field 'offsets_file' DEPRECATED and will be deleted soon! You must fill 'file_config.offsets_file' instead")
}
if p.config.WatchingDir_ != "" {
p.logger.Error("Field 'watching_dir' DEPRECATED and will be deleted soon! You must fill 'file_config.watching_dir' instead")
}

Check warning on line 140 in plugin/input/k8s/k8s.go

View check run for this annotation

Codecov / codecov/patch

plugin/input/k8s/k8s.go#L135-L140

Added lines #L135 - L140 were not covered by tests

// for backward compatibility; will be removed
p.config.FileConfig.OffsetsFile = p.config.OffsetsFile
p.config.FileConfig.WatchingDir = p.config.WatchingDir_

Check warning on line 145 in plugin/input/k8s/k8s.go

View check run for this annotation

Codecov / codecov/patch

plugin/input/k8s/k8s.go#L143-L145

Added lines #L143 - L145 were not covered by tests
startCounter := startCounter.Inc()

if startCounter == 1 {
Expand Down
9 changes: 8 additions & 1 deletion plugin/input/k8s/k8s_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/ozontech/file.d/logger"
"github.com/ozontech/file.d/pipeline"
"github.com/ozontech/file.d/plugin/input/file"
"github.com/ozontech/file.d/test"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -53,7 +54,13 @@ func getPodInfo(item *metaItem, isWhite bool) *corev1.Pod {
}

func config() *Config {
config := &Config{AllowedPodLabels: []string{"allowed_label"}, OffsetsFile: "offsets.yaml"}
config := &Config{
AllowedPodLabels: []string{"allowed_label"}, OffsetsFile: "offsets.yaml",
FileConfig: file.Config{
WatchingDir: "/var/log/containers",
OffsetsFile: "offsets.yaml",
},
}
test.NewConfig(config, map[string]int{"gomaxprocs": 1})
return config
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/output/s3/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ type Config struct {
// > @3@4@5@6
// >
// > Under the hood this plugin uses /plugin/output/file/ to collect logs.
FileConfig file.Config `json:"file_config" child:"true"` // *
FileConfig file.Config `json:"file_config"` // *

// > @3@4@5@6
// >
Expand Down
Loading