-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[core] Config loader: first draft #4
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| This package is responsible of scanning different sources searching for Agent checks' configuration files. | ||
|
|
||
| Check configurations may be contained within files on disk, environment variables, external databases: for | ||
| each source, the Agent has a specific _Provider_ implementing the `ConfigProvider` interface. | ||
|
|
||
| Check configurations may come in different format, for example Yaml code in the case of config files on disk. | ||
| Every configuration, regardless of the format, must be unmarshalled into a `CheckConfig` struct. | ||
|
|
||
| Usage example: | ||
| ```go | ||
| var configs []loader.CheckConfig | ||
| for _, provider := range configProviders { | ||
| c, _ := provider.Collect() | ||
| configs = append(configs, c...) | ||
| } | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package loader | ||
|
|
||
| import ( | ||
| "io/ioutil" | ||
| "path/filepath" | ||
|
|
||
| "github.com/op/go-logging" | ||
|
|
||
| "gopkg.in/yaml.v2" | ||
| ) | ||
|
|
||
| var log = logging.MustGetLogger("datadog-agent") | ||
|
|
||
| // FileConfigProvider collect configuration files from disk | ||
| type FileConfigProvider struct { | ||
| paths []string | ||
| } | ||
|
|
||
| // NewFileConfigProvider creates a new FileConfigProvider searching for | ||
| // configuration files on the given paths | ||
| func NewFileConfigProvider(paths []string) *FileConfigProvider { | ||
| return &FileConfigProvider{paths: paths} | ||
| } | ||
|
|
||
| // Collect scans provided paths searching for configuration files. When found, | ||
| // it parses the files and try to unmarshall Yaml contents into a CheckConfig | ||
| // instance | ||
| func (c *FileConfigProvider) Collect() ([]CheckConfig, error) { | ||
| configs := []CheckConfig{} | ||
|
|
||
| for _, path := range c.paths { | ||
| log.Debug("Searching for yaml files at:", path) | ||
|
|
||
| files, err := ioutil.ReadDir(path) | ||
| if err != nil { | ||
| log.Warningf("Unable to access dir: %s, skipping...", err) | ||
| continue | ||
| } | ||
|
|
||
| for _, f := range files { | ||
| if f.IsDir() { | ||
| log.Warningf("%s is a dir, skipping...", f.Name()) | ||
| continue | ||
| } | ||
|
|
||
| fName := f.Name() | ||
| extName := filepath.Ext(fName) | ||
| bName := fName[:len(f.Name())-len(extName)] | ||
| conf, err := getCheckConfig(bName, filepath.Join(path, fName)) | ||
| if err != nil { | ||
| log.Warningf("%s is not a valid config file: %s", f.Name(), err) | ||
| continue | ||
| } | ||
|
|
||
| log.Debug("Found valid configuration in file:", f.Name()) | ||
| configs = append(configs, conf) | ||
| } | ||
| } | ||
|
|
||
| return configs, nil | ||
| } | ||
|
|
||
| // getCheckConfig returns an instance of CheckConfig if `fpath` points to a valid config file | ||
| func getCheckConfig(name, fpath string) (CheckConfig, error) { | ||
| conf := CheckConfig{Name: name} | ||
|
|
||
| // Read file contents | ||
| // FIXME: ReadFile reads the entire file, possible security implications | ||
| yamlFile, err := ioutil.ReadFile(fpath) | ||
| if err != nil { | ||
| return conf, err | ||
| } | ||
|
|
||
| // Parse configuration | ||
| err = yaml.Unmarshal(yamlFile, &conf.Data) | ||
| return conf, err | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package loader | ||
|
|
||
| import "testing" | ||
|
|
||
| func Test_GetCheckConfig(t *testing.T) { | ||
| config, err := getCheckConfig("foo", "tests/wrong.yaml") | ||
| if err == nil { | ||
| t.Fatal("Expecting error") | ||
| } | ||
|
|
||
| config, err = getCheckConfig("foo", "foo.yaml") | ||
| if err == nil { | ||
| t.Fatal("Expecting error") | ||
| } | ||
|
|
||
| config, err = getCheckConfig("foo", "tests/testcheck.yaml") | ||
| if err != nil { | ||
| t.Fatalf("Expecting nil, found: %s", err) | ||
| } | ||
| if config.Name != "foo" { | ||
| t.Fatalf("Expecting `foo`, found: %s", config.Name) | ||
| } | ||
| } | ||
|
|
||
| func TestNewYamlConfigProvider(t *testing.T) { | ||
| paths := []string{"foo", "bar", "foo/bar"} | ||
| provider := NewFileConfigProvider(paths) | ||
| if len(provider.paths) != len(paths) { | ||
| t.Fatalf("Expecting length %d, found: %d", len(provider.paths), len(paths)) | ||
| } | ||
|
|
||
| for i, p := range provider.paths { | ||
| if p != paths[i] { | ||
| t.Fatalf("Expecting %s, found: %s", paths[i], p) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func TestCollect(t *testing.T) { | ||
| paths := []string{"tests", "foo/bar"} | ||
| provider := NewFileConfigProvider(paths) | ||
| configs, err := provider.Collect() | ||
|
|
||
| if err != nil { | ||
| t.Fatalf("Expecting nil, found: %s", err) | ||
| } | ||
|
|
||
| if len(configs) != 1 { | ||
| t.Fatalf("Expecting length 1, found: %d", len(configs)) | ||
| } | ||
|
|
||
| config := configs[0] | ||
| if config.Name != "testcheck" { | ||
| t.Fatalf("Expecting testcheck, found: %s", config.Name) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| init_config: | ||
|
|
||
| instances: | ||
| # No configuration is needed for this check. | ||
| - foo: bar |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // not valid Yaml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package loader | ||
|
|
||
| // CheckConfig is a generic container for configuration files | ||
| type CheckConfig struct { | ||
| Name string // the name of the check | ||
| Data map[string]interface{} // raw configuration content, unmarshalled from Yaml | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we enforce
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the point. With the current approach it's up to the check that receives a |
||
| } | ||
|
|
||
| // ConfigProvider is the interface that wraps the Collect method | ||
| // | ||
| // Collect is responsible of populating a list of CheckConfig instances | ||
| // by retrieving configuration patterns from external resources: files | ||
| // on disk, databases, environment variables are just few examples. | ||
| // | ||
| // Any type implementing the interface will take care of any dependency | ||
| // or data needed to access the resource providing the configuration. | ||
| type ConfigProvider interface { | ||
| Collect() ([]CheckConfig, error) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like
go-loggingbut we've been usingseelogalmost everywhere else...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will open a PR to switch the logging library