From 251d15672bf8da4596395ddbee693ab248d1959b Mon Sep 17 00:00:00 2001 From: Alirie Gray Date: Tue, 10 Jul 2018 11:28:58 -0700 Subject: [PATCH] Guard against length-0 slice in org_config get and FindOrCreate methods Added ErrOrganizationConfigNotFound error Refactored FindOrCreate to use helper get method Ensure that FindOrCreate creates a new org config if config not found and returns any other errors Co-authored-by: Jared Scheib --- bolt/org_config.go | 9 ++++-- chronograf.go | 60 +++++++++++++++++++-------------------- noop/org_config.go | 2 +- server/org_config_test.go | 14 ++++----- 4 files changed, 44 insertions(+), 41 deletions(-) diff --git a/bolt/org_config.go b/bolt/org_config.go index 7c49cd067..b1c9f96ac 100644 --- a/bolt/org_config.go +++ b/bolt/org_config.go @@ -41,6 +41,9 @@ func (s *OrganizationConfigStore) Get(ctx context.Context, orgID string) (*chron func (s *OrganizationConfigStore) get(ctx context.Context, tx *bolt.Tx, orgID string, cfg *chronograf.OrganizationConfig) error { v := tx.Bucket(OrganizationConfigBucket).Get([]byte(orgID)) + if len(v) == 0 { + return chronograf.ErrOrganizationConfigNotFound + } return internal.UnmarshalOrganizationConfig(v, cfg) } @@ -48,12 +51,12 @@ func (s *OrganizationConfigStore) get(ctx context.Context, tx *bolt.Tx, orgID st func (s *OrganizationConfigStore) FindOrCreate(ctx context.Context, orgID string) (*chronograf.OrganizationConfig, error) { var cfg chronograf.OrganizationConfig err := s.client.db.Update(func(tx *bolt.Tx) error { - v := tx.Bucket(OrganizationConfigBucket).Get([]byte(orgID)) - if v == nil { + err := s.get(ctx, tx, orgID, &cfg) + if err == chronograf.ErrOrganizationConfigNotFound { cfg = newOrganizationConfig(orgID) return s.update(ctx, tx, &cfg) } - return internal.UnmarshalOrganizationConfig(v, &cfg) + return err }) if err != nil { diff --git a/chronograf.go b/chronograf.go index f7919ef07..bddba9fe7 100644 --- a/chronograf.go +++ b/chronograf.go @@ -9,36 +9,36 @@ import ( // General errors. const ( - ErrUpstreamTimeout = Error("request to backend timed out") - ErrSourceNotFound = Error("source not found") - ErrServerNotFound = Error("server not found") - ErrLayoutNotFound = Error("layout not found") - ErrDashboardNotFound = Error("dashboard not found") - ErrUserNotFound = Error("user not found") - ErrLayoutInvalid = Error("layout is invalid") - ErrDashboardInvalid = Error("dashboard is invalid") - ErrSourceInvalid = Error("source is invalid") - ErrServerInvalid = Error("server is invalid") - ErrAlertNotFound = Error("alert not found") - ErrAuthentication = Error("user not authenticated") - ErrUninitialized = Error("client uninitialized. Call Open() method") - ErrInvalidAxis = Error("Unexpected axis in cell. Valid axes are 'x', 'y', and 'y2'") - ErrInvalidColorType = Error("Invalid color type. Valid color types are 'min', 'max', 'threshold', 'text', and 'background'") - ErrInvalidColor = Error("Invalid color. Accepted color format is #RRGGBB") - ErrInvalidLegend = Error("Invalid legend. Both type and orientation must be set") - ErrInvalidLegendType = Error("Invalid legend type. Valid legend type is 'static'") - ErrInvalidLegendOrient = Error("Invalid orientation type. Valid orientation types are 'top', 'bottom', 'right', 'left'") - ErrUserAlreadyExists = Error("user already exists") - ErrOrganizationNotFound = Error("organization not found") - ErrMappingNotFound = Error("mapping not found") - ErrOrganizationAlreadyExists = Error("organization already exists") - ErrCannotDeleteDefaultOrganization = Error("cannot delete default organization") - ErrConfigNotFound = Error("cannot find configuration") - ErrAnnotationNotFound = Error("annotation not found") - ErrInvalidCellOptionsText = Error("invalid text wrapping option. Valid wrappings are 'truncate', 'wrap', and 'single line'") - ErrInvalidCellOptionsSort = Error("cell options sortby cannot be empty'") - ErrInvalidCellOptionsColumns = Error("cell options columns cannot be empty'") - ErrOrganizationConfigFindOrCreateFailed = Error("failed to find or create organization config") + ErrUpstreamTimeout = Error("request to backend timed out") + ErrSourceNotFound = Error("source not found") + ErrServerNotFound = Error("server not found") + ErrLayoutNotFound = Error("layout not found") + ErrDashboardNotFound = Error("dashboard not found") + ErrUserNotFound = Error("user not found") + ErrLayoutInvalid = Error("layout is invalid") + ErrDashboardInvalid = Error("dashboard is invalid") + ErrSourceInvalid = Error("source is invalid") + ErrServerInvalid = Error("server is invalid") + ErrAlertNotFound = Error("alert not found") + ErrAuthentication = Error("user not authenticated") + ErrUninitialized = Error("client uninitialized. Call Open() method") + ErrInvalidAxis = Error("Unexpected axis in cell. Valid axes are 'x', 'y', and 'y2'") + ErrInvalidColorType = Error("Invalid color type. Valid color types are 'min', 'max', 'threshold', 'text', and 'background'") + ErrInvalidColor = Error("Invalid color. Accepted color format is #RRGGBB") + ErrInvalidLegend = Error("Invalid legend. Both type and orientation must be set") + ErrInvalidLegendType = Error("Invalid legend type. Valid legend type is 'static'") + ErrInvalidLegendOrient = Error("Invalid orientation type. Valid orientation types are 'top', 'bottom', 'right', 'left'") + ErrUserAlreadyExists = Error("user already exists") + ErrOrganizationNotFound = Error("organization not found") + ErrMappingNotFound = Error("mapping not found") + ErrOrganizationAlreadyExists = Error("organization already exists") + ErrCannotDeleteDefaultOrganization = Error("cannot delete default organization") + ErrConfigNotFound = Error("cannot find configuration") + ErrAnnotationNotFound = Error("annotation not found") + ErrInvalidCellOptionsText = Error("invalid text wrapping option. Valid wrappings are 'truncate', 'wrap', and 'single line'") + ErrInvalidCellOptionsSort = Error("cell options sortby cannot be empty'") + ErrInvalidCellOptionsColumns = Error("cell options columns cannot be empty'") + ErrOrganizationConfigNotFound = Error("could not find organization config") ) // Error is a domain error encountered while processing chronograf requests diff --git a/noop/org_config.go b/noop/org_config.go index 8906f4de6..50c110c20 100644 --- a/noop/org_config.go +++ b/noop/org_config.go @@ -13,7 +13,7 @@ var _ chronograf.OrganizationConfigStore = &OrganizationConfigStore{} type OrganizationConfigStore struct{} func (s *OrganizationConfigStore) FindOrCreate(context.Context, string) (*chronograf.OrganizationConfig, error) { - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } func (s *OrganizationConfigStore) Update(context.Context, *chronograf.OrganizationConfig) error { diff --git a/server/org_config_test.go b/server/org_config_test.go index 37746af87..7eeb582fd 100644 --- a/server/org_config_test.go +++ b/server/org_config_test.go @@ -152,7 +152,7 @@ func TestOrganizationConfig(t *testing.T) { }, }, nil default: - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } }, }, @@ -254,7 +254,7 @@ func TestLogViewerOrganizationConfig(t *testing.T) { }, }, nil default: - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } }, }, @@ -353,7 +353,7 @@ func TestReplaceLogViewerOrganizationConfig(t *testing.T) { }, }, nil default: - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } }, UpdateF: func(ctx context.Context, target *chronograf.OrganizationConfig) error { @@ -445,7 +445,7 @@ func TestReplaceLogViewerOrganizationConfig(t *testing.T) { }, }, nil default: - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } }, UpdateF: func(ctx context.Context, target *chronograf.OrganizationConfig) error { @@ -489,7 +489,7 @@ func TestReplaceLogViewerOrganizationConfig(t *testing.T) { }, }, nil default: - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } }, UpdateF: func(ctx context.Context, target *chronograf.OrganizationConfig) error { @@ -554,7 +554,7 @@ func TestReplaceLogViewerOrganizationConfig(t *testing.T) { }, }, nil default: - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } }, UpdateF: func(ctx context.Context, target *chronograf.OrganizationConfig) error { @@ -624,7 +624,7 @@ func TestReplaceLogViewerOrganizationConfig(t *testing.T) { }, }, nil default: - return nil, chronograf.ErrOrganizationConfigFindOrCreateFailed + return nil, chronograf.ErrOrganizationConfigNotFound } }, UpdateF: func(ctx context.Context, target *chronograf.OrganizationConfig) error {