From d6b4de3874b3abc7beeb57a21d17fd2ee04c7e00 Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Tue, 2 Jan 2018 16:22:45 -0800 Subject: [PATCH] Refactor osquery options interface (#1674) After discussion with @groob and @marpaia, we have decided that the service methods should not be aware of any YAML/JSON definitions, and should work directly with objects. The new pattern we will use will involve converting YAML to JSON at the client, and then sending the JSON which will be decoded using the familiar go-kit mechanisms before being passed to the service methods. --- server/kolide/meta.go | 6 + server/kolide/osquery_options.go | 13 +- server/kolide/osquery_options_test.go | 2 +- server/service/service_osquery_options.go | 31 +--- .../service/service_osquery_options_test.go | 138 ------------------ 5 files changed, 18 insertions(+), 172 deletions(-) create mode 100644 server/kolide/meta.go delete mode 100644 server/service/service_osquery_options_test.go diff --git a/server/kolide/meta.go b/server/kolide/meta.go new file mode 100644 index 0000000000..de87000674 --- /dev/null +++ b/server/kolide/meta.go @@ -0,0 +1,6 @@ +package kolide + +type ObjectMetadata struct { + ApiVersion string `json:"apiVersion"` + Kind string `json:"kind"` +} diff --git a/server/kolide/osquery_options.go b/server/kolide/osquery_options.go index ea87473de5..9f15c2331b 100644 --- a/server/kolide/osquery_options.go +++ b/server/kolide/osquery_options.go @@ -9,14 +9,13 @@ type OsqueryOptionsStore interface { } type OsqueryOptionsService interface { - ApplyOptionsYaml(yml string) error - GetOptionsYaml() (string, error) + ApplyOptionsSpec(spec *OptionsSpec) error + GetOptionsSpec() (*OptionsSpec, error) } -type OptionsYaml struct { - ApiVersion string `json:"apiVersion"` - Kind string `json:"kind"` - Spec OptionsSpec `json:"spec"` +type OptionsObject struct { + ObjectMetadata + Spec OptionsSpec `json:"spec"` } type OptionsSpec struct { @@ -29,7 +28,7 @@ type OptionsOverrides struct { } const ( - OptionsSpecKind = "OsqueryOptions" + OptionsKind = "Options" ) // OptionOverrideType is used to designate which override type a given set of diff --git a/server/kolide/osquery_options_test.go b/server/kolide/osquery_options_test.go index 2c167243a7..b0a96cf45a 100644 --- a/server/kolide/osquery_options_test.go +++ b/server/kolide/osquery_options_test.go @@ -99,7 +99,7 @@ spec: ] }` - var foo OptionsYaml + var foo OptionsObject err := yaml.Unmarshal(y, &foo) require.Nil(t, err) diff --git a/server/service/service_osquery_options.go b/server/service/service_osquery_options.go index 3c03f978a2..b303df4581 100644 --- a/server/service/service_osquery_options.go +++ b/server/service/service_osquery_options.go @@ -1,41 +1,20 @@ package service import ( - "github.com/ghodss/yaml" "github.com/kolide/fleet/server/kolide" "github.com/pkg/errors" ) -func (svc service) ApplyOptionsYaml(yml string) error { - var conf kolide.OptionsYaml - err := yaml.Unmarshal([]byte(yml), &conf) - if err != nil { - return errors.Wrap(err, "unmarshal options YAML") - } - - if conf.Kind != kolide.OptionsSpecKind { - return errors.Errorf("expected kind '%s', got '%s'", kolide.OptionsSpecKind, conf.Kind) - } - - err = svc.ds.ApplyOptions(&conf.Spec) +func (svc service) ApplyOptionsSpec(spec *kolide.OptionsSpec) error { + err := svc.ds.ApplyOptions(spec) return errors.Wrap(err, "apply options") } -func (svc service) GetOptionsYaml() (string, error) { +func (svc service) GetOptionsSpec() (*kolide.OptionsSpec, error) { spec, err := svc.ds.GetOptions() if err != nil { - return "", errors.Wrap(err, "get options from datastore") + return nil, errors.Wrap(err, "get options from datastore") } - ymlObj := kolide.OptionsYaml{ - ApiVersion: kolide.ApiVersion, - Kind: kolide.OptionsSpecKind, - Spec: *spec, - } - - yml, err := yaml.Marshal(ymlObj) - if err != nil { - return "", errors.Wrap(err, "marshal options yaml") - } - return string(yml), nil + return spec, nil } diff --git a/server/service/service_osquery_options_test.go b/server/service/service_osquery_options_test.go deleted file mode 100644 index 286133e4e2..0000000000 --- a/server/service/service_osquery_options_test.go +++ /dev/null @@ -1,138 +0,0 @@ -package service - -import ( - "encoding/json" - "testing" - - "github.com/kolide/fleet/server/kolide" - "github.com/kolide/fleet/server/mock" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestApplyOptionsYaml(t *testing.T) { - var testCases = []struct { - yml string - options *kolide.OptionsSpec - shouldErr bool - }{ - {"notyaml", nil, true}, - { - yml: ` -apiVersion: k8s.kolide.com/v1alpha1 -kind: OsqueryQuery -spec: - name: osquery_schedule - description: Report performance stats - query: select * from osquery_schedule -`, // Wrong kind of yaml - options: nil, - shouldErr: true, - }, - { - yml: ` -apiVersion: k8s.kolide.com/v1alpha1 -kind: OsqueryOptions -spec: - config: - foo: bar - overrides: - # Note configs in overrides take precedence over base configs - platforms: - darwin: - bing: bang -`, - options: &kolide.OptionsSpec{ - Config: json.RawMessage(`{"foo":"bar"}`), - Overrides: kolide.OptionsOverrides{ - Platforms: map[string]json.RawMessage{ - "darwin": json.RawMessage(`{"bing":"bang"}`), - }, - }, - }, - shouldErr: false, - }, - } - - var gotOptions *kolide.OptionsSpec - ds := &mock.Store{ - OsqueryOptionsStore: mock.OsqueryOptionsStore{ - ApplyOptionsFunc: func(options *kolide.OptionsSpec) error { - gotOptions = options - return nil - }, - }, - } - svc := service{ - ds: ds, - } - - for _, tt := range testCases { - gotOptions = nil - t.Run("", func(t *testing.T) { - err := svc.ApplyOptionsYaml(tt.yml) - if tt.shouldErr { - assert.NotNil(t, err) - } else { - assert.Nil(t, err) - assert.Equal(t, tt.options, gotOptions) - } - }) - } -} - -func TestOptionsYamlRoundtrip(t *testing.T) { - var testCases = []struct { - spec kolide.OptionsSpec - }{ - { - kolide.OptionsSpec{ - json.RawMessage(`{"foo":"bar"}`), - kolide.OptionsOverrides{}, - }, - }, - { - kolide.OptionsSpec{ - json.RawMessage(`{"bing":"bang","boom":2}`), - kolide.OptionsOverrides{ - map[string]json.RawMessage{ - "foobar": json.RawMessage(`{"manzanita":"scratch"}`), - "froobling": json.RawMessage(`{"doornail":"mumble"}`), - }, - }, - }, - }, - } - - var returnOptions, gotOptions *kolide.OptionsSpec - ds := &mock.Store{ - OsqueryOptionsStore: mock.OsqueryOptionsStore{ - GetOptionsFunc: func() (*kolide.OptionsSpec, error) { - return returnOptions, nil - }, - ApplyOptionsFunc: func(options *kolide.OptionsSpec) error { - gotOptions = options - return nil - }, - }, - } - svc := service{ - ds: ds, - } - - for _, tt := range testCases { - t.Run("", func(t *testing.T) { - returnOptions = &tt.spec - gotOptions = nil - - yml, err := svc.GetOptionsYaml() - require.Nil(t, err) - - err = svc.ApplyOptionsYaml(yml) - require.Nil(t, err) - - assert.Equal(t, returnOptions, gotOptions) - - }) - } -}