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

logging#Config: Fix Options can no longer be parsed by go-yaml #84

Merged
merged 4 commits into from
Oct 29, 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
42 changes: 28 additions & 14 deletions logging/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package logging

import (
"fmt"
"github.com/creasty/defaults"
"github.com/pkg/errors"
"go.uber.org/zap/zapcore"
"os"
Expand Down Expand Up @@ -38,39 +39,52 @@ func (o *Options) UnmarshalText(text []byte) error {
return nil
}

// UnmarshalYAML implements yaml.InterfaceUnmarshaler to allow Options to be parsed go-yaml.
func (o *Options) UnmarshalYAML(unmarshal func(any) error) error {
optionsMap := make(map[string]zapcore.Level)

if err := unmarshal(&optionsMap); err != nil {
return err
}

*o = optionsMap

return nil
}

// Config defines Logger configuration.
type Config struct {
// zapcore.Level at 0 is for info level.
Level zapcore.Level `yaml:"level" env:"LEVEL" default:"0"`
Output string `yaml:"output" env:"OUTPUT"`
// Interval for periodic logging.
Interval time.Duration `yaml:"interval" env:"INTERVAL" default:"20s"`

Options `yaml:"options" env:"OPTIONS"`
Options Options `yaml:"options" env:"OPTIONS"`
}

// Validate checks constraints in the configuration and returns an error if they are violated.
// Also configures the log output if it is not configured:
// SetDefaults implements defaults.Setter to configure the log output if it is not set:
// systemd-journald is used when Icinga DB is running under systemd, otherwise stderr.
func (l *Config) Validate() error {
if l.Interval <= 0 {
return errors.New("periodic logging interval must be positive")
}

if l.Output == "" {
func (c *Config) SetDefaults() {
if defaults.CanUpdate(c.Output) {
if _, ok := os.LookupEnv("NOTIFY_SOCKET"); ok {
// When started by systemd, NOTIFY_SOCKET is set by systemd for Type=notify supervised services,
// which is the default setting for the Icinga DB service.
// This assumes that Icinga DB is running under systemd, so set output to systemd-journald.
l.Output = JOURNAL
c.Output = JOURNAL
} else {
// Otherwise set it to console, i.e. write log messages to stderr.
l.Output = CONSOLE
c.Output = CONSOLE
}
}
}

// Validate checks constraints in the configuration and returns an error if they are violated.
func (c *Config) Validate() error {
if c.Interval <= 0 {
return errors.New("periodic logging interval must be positive")
}

// To be on the safe side, always call AssertOutput.
return AssertOutput(l.Output)
return AssertOutput(c.Output)
}

// AssertOutput returns an error if output is not a valid logger output.
Expand Down
130 changes: 90 additions & 40 deletions logging/config_test.go
Original file line number Diff line number Diff line change
@@ -1,52 +1,82 @@
package logging

import (
"fmt"
"github.com/creasty/defaults"
"github.com/icinga/icinga-go-library/config"
"github.com/icinga/icinga-go-library/testutils"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zapcore"
"os"
"testing"
"time"
)

func TestConfig(t *testing.T) {
subtests := []struct {
name string
opts config.EnvOptions
expected Config
error bool
}{
var defaultConfig Config
require.NoError(t, defaults.Set(&defaultConfig), "setting default config")

configTests := []testutils.TestCase[Config, testutils.ConfigTestData]{
{
Name: "Defaults",
Data: testutils.ConfigTestData{
// An empty YAML file causes an error,
// so specify a valid key without a value to trigger fallback to the default.
Yaml: `level:`,
},
Expected: defaultConfig,
},
{
name: "empty",
opts: config.EnvOptions{},
expected: Config{
Output: "console",
Interval: 20 * time.Second,
Name: "periodic logging interval must be positive",
Data: testutils.ConfigTestData{
Yaml: `interval: 0s`,
Env: map[string]string{"INTERVAL": "0s"},
},
Error: testutils.ErrorContains("periodic logging interval must be positive"),
},
{
name: "invalid-output",
opts: config.EnvOptions{Environment: map[string]string{"OUTPUT": "☃"}},
error: true,
Name: "invalid logger output",
Data: testutils.ConfigTestData{
Yaml: `output: invalid`,
Env: map[string]string{"OUTPUT": "invalid"},
},
Error: testutils.ErrorContains("invalid is not a valid logger output"),
},
{
name: "customized",
opts: config.EnvOptions{Environment: map[string]string{
"LEVEL": zapcore.DebugLevel.String(),
"OUTPUT": JOURNAL,
"INTERVAL": "3m14s",
}},
expected: Config{
Name: "Customized",
Data: testutils.ConfigTestData{
Yaml: fmt.Sprintf(
`
level: debug
output: %s
interval: 3m14s`,
JOURNAL,
),
Env: map[string]string{
"LEVEL": zapcore.DebugLevel.String(),
"OUTPUT": JOURNAL,
"INTERVAL": "3m14s",
},
},
Expected: Config{
Level: zapcore.DebugLevel,
Output: JOURNAL,
Interval: 3*time.Minute + 14*time.Second,
},
},
{
name: "options",
opts: config.EnvOptions{Environment: map[string]string{"OPTIONS": "foo:debug,bar:info,buz:panic"}},
expected: Config{
Output: "console",
Interval: 20 * time.Second,
Name: "Options",
Data: testutils.ConfigTestData{
Yaml: `
options:
foo: debug
bar: info
buz: panic`,
Env: map[string]string{"OPTIONS": "foo:debug,bar:info,buz:panic"},
},
Expected: Config{
Output: defaultConfig.Output,
Interval: defaultConfig.Interval,
Options: map[string]zapcore.Level{
"foo": zapcore.DebugLevel,
"bar": zapcore.InfoLevel,
Expand All @@ -55,21 +85,41 @@ func TestConfig(t *testing.T) {
},
},
{
name: "options-invalid-levels",
opts: config.EnvOptions{Environment: map[string]string{"OPTIONS": "foo:foo,bar:0"}},
error: true,
Name: "Options with invalid level",
Data: testutils.ConfigTestData{
Yaml: `
options:
foo: foo`,
Env: map[string]string{"OPTIONS": "foo:foo"},
},
Error: testutils.ErrorContains(`unrecognized level: "foo"`),
},
}

for _, test := range subtests {
t.Run(test.name, func(t *testing.T) {
var out Config
if err := config.FromEnv(&out, test.opts); test.error {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, test.expected, out)
}
})
}
t.Run("FromEnv", func(t *testing.T) {
for _, tc := range configTests {
t.Run(tc.Name, tc.F(func(data testutils.ConfigTestData) (Config, error) {
var actual Config

err := config.FromEnv(&actual, config.EnvOptions{Environment: data.Env})

return actual, err
}))
}
})

t.Run("FromYAMLFile", func(t *testing.T) {
for _, tc := range configTests {
t.Run(tc.Name+"/FromYAMLFile", tc.F(func(data testutils.ConfigTestData) (Config, error) {
var actual Config

var err error
testutils.WithYAMLFile(t, data.Yaml, func(file *os.File) {
err = config.FromYAMLFile(file.Name(), &actual)
})

return actual, err
}))
}
})
}
2 changes: 1 addition & 1 deletion testutils/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
// TestCase represents a generic test case structure.
// It is parameterized by T, the type of the expected result, and D, the type of the test data.
// This struct is useful for defining test cases with expected outcomes and associated data.
type TestCase[T comparable, D any] struct {
type TestCase[T any, D any] struct {
// Name is the identifier for the test case, used for reporting purposes.
Name string
// Expected is the anticipated result of the test. It should be left empty if an error is expected.
Expand Down
Loading