Skip to content

Commit

Permalink
river: add encode/decode tests (grafana#4443)
Browse files Browse the repository at this point in the history
  • Loading branch information
thampiotr committed Jul 26, 2023
1 parent d34afce commit c74a4f8
Show file tree
Hide file tree
Showing 6 changed files with 247 additions and 18 deletions.
5 changes: 1 addition & 4 deletions component/loki/source/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/grafana/agent/component"
"github.com/grafana/agent/component/common/config"
commonk8s "github.com/grafana/agent/component/common/kubernetes"
"github.com/grafana/agent/component/common/loki"
"github.com/grafana/agent/component/common/loki/positions"
Expand Down Expand Up @@ -45,9 +44,7 @@ type Arguments struct {

// DefaultArguments holds default settings for loki.source.kubernetes.
var DefaultArguments = Arguments{
Client: commonk8s.ClientArguments{
HTTPClientConfig: config.DefaultHTTPClientConfig,
},
Client: commonk8s.DefaultClientArguments,
}

// SetToDefault implements river.Defaulter.
Expand Down
5 changes: 1 addition & 4 deletions component/loki/source/kubernetes_events/kubernetes_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/grafana/agent/component"
"github.com/grafana/agent/component/common/config"
"github.com/grafana/agent/component/common/kubernetes"
"github.com/grafana/agent/component/common/loki"
"github.com/grafana/agent/component/common/loki/positions"
Expand Down Expand Up @@ -53,9 +52,7 @@ type Arguments struct {
var DefaultArguments = Arguments{
JobName: "loki.source.kubernetes_events",

Client: kubernetes.ClientArguments{
HTTPClientConfig: config.DefaultHTTPClientConfig,
},
Client: kubernetes.DefaultClientArguments,
}

// SetToDefault implements river.Defaulter.
Expand Down
4 changes: 1 addition & 3 deletions component/loki/source/podlogs/podlogs.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ type Arguments struct {

// DefaultArguments holds default settings for loki.source.kubernetes.
var DefaultArguments = Arguments{
Client: commonk8s.ClientArguments{
HTTPClientConfig: config.DefaultHTTPClientConfig,
},
Client: commonk8s.DefaultClientArguments,
}

// SetToDefault implements river.Defaulter.
Expand Down
11 changes: 11 additions & 0 deletions pkg/river/internal/value/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ import (

// The Defaulter interface allows a type to implement default functionality
// in River evaluation.
//
// Defaulter will be called only on block and body river types.
//
// When using nested blocks, the wrapping type must also implement
// Defaulter to propagate the defaults of the wrapped type. Otherwise,
// defaults used for the wrapped type become inconsistent:
//
// - If the wrapped block is NOT defined in the River config, the wrapping
// type's defaults are used.
// - If the wrapped block IS defined in the River config, the wrapped type's
// defaults are used.
type Defaulter interface {
// SetToDefault is called when evaluating a block or body to set the value
// to its defaults.
Expand Down
7 changes: 0 additions & 7 deletions pkg/river/token/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,6 @@ func (b *Body) encodeField(prefix []string, field rivertags.Field, fieldValue re
fullName := mergeStringSlice(prefix, field.Name)

switch {
case fieldValue.IsZero():
// It shouldn't be possible to have a required block which is unset, but
// we'll encode something anyway.
inner := NewBlock(fullName, "")
inner.body.SetValueOverrideHook(b.valueOverrideHook)
b.AppendBlock(inner)

case fieldValue.Kind() == reflect.Map:
// Iterate over the map and add each element as an attribute into it.
if fieldValue.Type().Key().Kind() != reflect.String {
Expand Down
233 changes: 233 additions & 0 deletions pkg/river/token/builder/nested_defaults_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
package builder_test

import (
"fmt"
"reflect"
"testing"

"github.com/grafana/agent/pkg/river/ast"
"github.com/grafana/agent/pkg/river/parser"
"github.com/grafana/agent/pkg/river/token/builder"
"github.com/grafana/agent/pkg/river/vm"
"github.com/stretchr/testify/require"
)

const (
defaultNumber = 123
otherDefaultNumber = 321
)

var testCases = []struct {
name string
input interface{}
expectedRiver string
}{
{
name: "struct propagating default - input matching default",
input: StructPropagatingDefault{Inner: AttrWithDefault{Number: defaultNumber}},
expectedRiver: "",
},
{
name: "struct propagating default - input with zero-value struct",
input: StructPropagatingDefault{},
expectedRiver: `
inner {
number = 0
}
`,
},
{
name: "struct propagating default - input with non-default value",
input: StructPropagatingDefault{Inner: AttrWithDefault{Number: 42}},
expectedRiver: `
inner {
number = 42
}
`,
},
{
name: "pointer propagating default - input matching default",
input: PtrPropagatingDefault{Inner: &AttrWithDefault{Number: defaultNumber}},
expectedRiver: "",
},
{
name: "pointer propagating default - input with zero value",
input: PtrPropagatingDefault{Inner: &AttrWithDefault{}},
expectedRiver: `
inner {
number = 0
}
`,
},
{
name: "pointer propagating default - input with non-default value",
input: PtrPropagatingDefault{Inner: &AttrWithDefault{Number: 42}},
expectedRiver: `
inner {
number = 42
}
`,
},
{
name: "zero default - input with zero value",
input: ZeroDefault{Inner: &AttrWithDefault{}},
expectedRiver: "",
},
{
name: "zero default - input with non-default value",
input: ZeroDefault{Inner: &AttrWithDefault{Number: 42}},
expectedRiver: `
inner {
number = 42
}
`,
},
{
name: "no default - input with zero value",
input: NoDefaultDefined{Inner: &AttrWithDefault{}},
expectedRiver: `
inner {
number = 0
}
`,
},
{
name: "no default - input with non-default value",
input: NoDefaultDefined{Inner: &AttrWithDefault{Number: 42}},
expectedRiver: `
inner {
number = 42
}
`,
},
{
name: "mismatching default - input matching outer default",
input: MismatchingDefault{Inner: &AttrWithDefault{Number: otherDefaultNumber}},
expectedRiver: "",
},
{
name: "mismatching default - input matching inner default",
input: MismatchingDefault{Inner: &AttrWithDefault{Number: defaultNumber}},
expectedRiver: "inner { }",
},
{
name: "mismatching default - input with non-default value",
input: MismatchingDefault{Inner: &AttrWithDefault{Number: 42}},
expectedRiver: `
inner {
number = 42
}
`,
},
}

func TestNestedDefaults(t *testing.T) {
for _, tc := range testCases {
t.Run(fmt.Sprintf("%T/%s", tc.input, tc.name), func(t *testing.T) {
f := builder.NewFile()
f.Body().AppendFrom(tc.input)
actualRiver := string(f.Bytes())
expected := format(t, tc.expectedRiver)
require.Equal(t, expected, actualRiver, "generated river didn't match expected")

// Now decode the River produced above and make sure it's the same as the input.
eval := vm.New(parseBlock(t, actualRiver))
vPtr := reflect.New(reflect.TypeOf(tc.input)).Interface()
require.NoError(t, eval.Evaluate(nil, vPtr), "river evaluation error")

actualOut := reflect.ValueOf(vPtr).Elem().Interface()
require.Equal(t, tc.input, actualOut, "Invariant violated: encoded and then decoded block didn't match the original value")
})
}
}

func TestPtrPropagatingDefaultWithNil(t *testing.T) {
// This is a special case - when defaults are correctly defined, the `Inner: nil` should mean to use defaults.
// Encoding will encode to empty string and decoding will produce the default value - `Inner: {Number: 123}`.
input := PtrPropagatingDefault{}
expectedEncodedRiver := ""
expectedDecoded := PtrPropagatingDefault{Inner: &AttrWithDefault{Number: 123}}

f := builder.NewFile()
f.Body().AppendFrom(input)
actualRiver := string(f.Bytes())
expected := format(t, expectedEncodedRiver)
require.Equal(t, expected, actualRiver, "generated river didn't match expected")

// Now decode the River produced above and make sure it's the same as the input.
eval := vm.New(parseBlock(t, actualRiver))
vPtr := reflect.New(reflect.TypeOf(input)).Interface()
require.NoError(t, eval.Evaluate(nil, vPtr), "river evaluation error")

actualOut := reflect.ValueOf(vPtr).Elem().Interface()
require.Equal(t, expectedDecoded, actualOut)
}

// StructPropagatingDefault has the outer defaults matching the inner block's defaults. The inner block is a struct.
type StructPropagatingDefault struct {
Inner AttrWithDefault `river:"inner,block,optional"`
}

func (o *StructPropagatingDefault) SetToDefault() {
inner := &AttrWithDefault{}
inner.SetToDefault()
*o = StructPropagatingDefault{Inner: *inner}
}

// PtrPropagatingDefault has the outer defaults matching the inner block's defaults. The inner block is a pointer.
type PtrPropagatingDefault struct {
Inner *AttrWithDefault `river:"inner,block,optional"`
}

func (o *PtrPropagatingDefault) SetToDefault() {
inner := &AttrWithDefault{}
inner.SetToDefault()
*o = PtrPropagatingDefault{Inner: inner}
}

// MismatchingDefault has the outer defaults NOT matching the inner block's defaults. The inner block is a pointer.
type MismatchingDefault struct {
Inner *AttrWithDefault `river:"inner,block,optional"`
}

func (o *MismatchingDefault) SetToDefault() {
*o = MismatchingDefault{Inner: &AttrWithDefault{
Number: otherDefaultNumber,
}}
}

// ZeroDefault has the outer defaults setting to zero values. The inner block is a pointer.
type ZeroDefault struct {
Inner *AttrWithDefault `river:"inner,block,optional"`
}

func (o *ZeroDefault) SetToDefault() {
*o = ZeroDefault{Inner: &AttrWithDefault{}}
}

// NoDefaultDefined has no defaults defined. The inner block is a pointer.
type NoDefaultDefined struct {
Inner *AttrWithDefault `river:"inner,block,optional"`
}

// AttrWithDefault has a default value of a non-zero number.
type AttrWithDefault struct {
Number int `river:"number,attr,optional"`
}

func (i *AttrWithDefault) SetToDefault() {
*i = AttrWithDefault{Number: defaultNumber}
}

func parseBlock(t *testing.T, input string) *ast.BlockStmt {
t.Helper()

input = fmt.Sprintf("test { %s }", input)
res, err := parser.ParseFile("", []byte(input))
require.NoError(t, err)
require.Len(t, res.Body, 1)

stmt, ok := res.Body[0].(*ast.BlockStmt)
require.True(t, ok, "Expected stmt to be a ast.BlockStmt, got %T", res.Body[0])
return stmt
}

0 comments on commit c74a4f8

Please sign in to comment.