Skip to content

Commit

Permalink
Revert "Don't overwrite fields of struct field with tag child:"true" (
Browse files Browse the repository at this point in the history
#638)" (#648)

This reverts commit 91aed6b.
  • Loading branch information
goshansmails authored Jun 27, 2024
1 parent c7436c3 commit d02c290
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 70 deletions.
44 changes: 38 additions & 6 deletions cfg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,14 @@ func Parse(ptr any, values map[string]int) error {
return nil
}

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

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

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

for _, nestedConfig := range nestedConfigs {
if err := Parse(nestedConfig.Addr().Interface(), values); err != nil {
for _, child := range childs {
if err := ParseChild(v, child, 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
// calls Parse, not ParseChild (!)
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: 18 additions & 20 deletions cfg/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ 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 @@ -337,6 +346,15 @@ 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 @@ -625,23 +643,3 @@ 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: 2 additions & 3 deletions plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,10 @@ pipelines:
example_k8s_pipeline:
input:
type: k8s
file_config: # customize file plugin
offsets_file: /data/offsets.yaml
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"` // *
RedisBackendCfg RedisBackendConfig `json:"redis_backend_config" child:"true"` // *

// > @3@4@5@6
// >
Expand Down
5 changes: 2 additions & 3 deletions plugin/input/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,10 @@ pipelines:
example_k8s_pipeline:
input:
type: k8s
file_config: # customize file plugin
offsets_file: /data/offsets.yaml
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"` // *
Auth AuthConfig `json:"auth" child:"true"` // *

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

type AuthStrategy byte
Expand Down
9 changes: 2 additions & 7 deletions plugin/input/k8s/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ pipelines:
example_k8s_pipeline:
input:
type: k8s
file_config: # customize file plugin
offsets_file: /data/offsets.yaml
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 @@ -56,16 +55,12 @@ 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: 3 additions & 19 deletions plugin/input/k8s/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ pipelines:
example_k8s_pipeline:
input:
type: k8s
file_config: # customize file plugin
offsets_file: /data/offsets.yaml
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 @@ -80,23 +79,19 @@ type Config struct {

// > @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"` // *
FileConfig file.Config `json:"file_config" child:"true"` // *
}

var startCounter atomic.Int32
Expand Down Expand Up @@ -132,17 +127,6 @@ func (p *Plugin) Start(config pipeline.AnyConfig, params *pipeline.InputPluginPa
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")
}

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

startCounter := startCounter.Inc()

if startCounter == 1 {
Expand Down
9 changes: 1 addition & 8 deletions plugin/input/k8s/k8s_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ 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 @@ -54,13 +53,7 @@ func getPodInfo(item *metaItem, isWhite bool) *corev1.Pod {
}

func config() *Config {
config := &Config{
AllowedPodLabels: []string{"allowed_label"}, OffsetsFile: "offsets.yaml",
FileConfig: file.Config{
WatchingDir: "/var/log/containers",
OffsetsFile: "offsets.yaml",
},
}
config := &Config{AllowedPodLabels: []string{"allowed_label"}, 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"` // *
FileConfig file.Config `json:"file_config" child:"true"` // *

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

0 comments on commit d02c290

Please sign in to comment.