Merge pull request sensu#705 from sensu/refactor/hook-list

Refactor CheckHook to HookList for more accurate reusability

backend/apid/actions/checks.go

@@ -167,7 +167,7 @@ func (a CheckController) Destroy(ctx context.Context, name string) error {
 }
 
 // AddCheckHook adds an association between a hook and a check
-func (a CheckController) AddCheckHook(ctx context.Context, check string, checkHook types.CheckHook) error {
+func (a CheckController) AddCheckHook(ctx context.Context, check string, checkHook types.HookList) error {
 	return a.findAndUpdateCheckConfig(ctx, check, func(check *types.CheckConfig) error {
 		var exists bool
 		for i, r := range check.CheckHooks {

backend/apid/routers/checks.go

@@ -73,7 +73,7 @@ func (r *ChecksRouter) destroy(req *http.Request) (interface{}, error) {
 }
 
 func (r *ChecksRouter) addCheckHook(req *http.Request) (interface{}, error) {
-	cfg := types.CheckHook{}
+	cfg := types.HookList{}
 	if err := unmarshalBody(req, &cfg); err != nil {
 		return nil, err
 	}

backend/schedulerd/check_scheduler_test.go

@@ -154,7 +154,7 @@ func (suite *CheckExecSuite) TestBuild() {
 	suite.Len(request.Hooks, 1)
 
 	check.RuntimeAssets = []string{}
-	check.CheckHooks = []types.CheckHook{}
+	check.CheckHooks = []types.HookList{}
 	request = suite.exec.BuildRequest(check)
 	suite.NotNil(request)
 	suite.NotNil(request.Config)

cli/client/check.go

@@ -101,7 +101,7 @@ func (client *RestClient) ListChecks(org string) ([]types.CheckConfig, error) {
 }
 
 // AddCheckHook associates an existing hook with an existing check
-func (client *RestClient) AddCheckHook(check *types.CheckConfig, checkHook *types.CheckHook) error {
+func (client *RestClient) AddCheckHook(check *types.CheckConfig, checkHook *types.HookList) error {
 	key := checksPath(check.Name, "hooks", checkHook.Type)
 	res, err := client.R().SetQueryParam("org", check.Organization).SetQueryParam("env", check.Environment).SetBody(checkHook).Put(key)
 	if err != nil {

cli/client/interface.go

@@ -45,7 +45,7 @@ type CheckAPIClient interface {
 	FetchCheck(string) (*types.CheckConfig, error)
 	ListChecks(string) ([]types.CheckConfig, error)
 
-	AddCheckHook(check *types.CheckConfig, checkHook *types.CheckHook) error
+	AddCheckHook(check *types.CheckConfig, checkHook *types.HookList) error
 	RemoveCheckHook(check *types.CheckConfig, checkHookType string, hookName string) error
 }
 

cli/client/testing/mock_check_client.go

@@ -33,7 +33,7 @@ func (c *MockClient) ListChecks(org string) ([]types.CheckConfig, error) {
 }
 
 // AddCheckHook for use with mock lib
-func (c *MockClient) AddCheckHook(check *types.CheckConfig, checkHook *types.CheckHook) error {
+func (c *MockClient) AddCheckHook(check *types.CheckConfig, checkHook *types.HookList) error {
 	args := c.Called(check, checkHook)
 	return args.Error(0)
 }

cli/commands/check/add_hook.go

@@ -46,7 +46,7 @@ func AddCheckHookCommand(cli *cli.SensuCli) *cobra.Command {
 			}
 
 			// Instantiate check hook from input
-			checkHook := types.CheckHook{}
+			checkHook := types.HookList{}
 			opts.Copy(&checkHook)
 
 			// Ensure that the given checkHook is valid
@@ -113,7 +113,7 @@ func (opts *checkHookOpts) administerQuestionnaire() error {
 	return survey.Ask(qs, opts)
 }
 
-func (opts *checkHookOpts) Copy(checkHook *types.CheckHook) {
+func (opts *checkHookOpts) Copy(checkHook *types.HookList) {
 	checkHook.Type = opts.Type
 	checkHook.Hooks = helpers.SafeSplitCSV(opts.Hooks)
 }

cli/commands/check/add_hook_test.go

@@ -27,7 +27,7 @@ func TestAddCheckHookCommandRunEClosureSucess(t *testing.T) {
 	cli := test.NewMockCLI()
 
 	client := cli.Client.(*clientmock.MockClient)
-	client.On("AddCheckHook", mock.AnythingOfType("*types.CheckConfig"), mock.AnythingOfType("*types.CheckHook")).Return(nil)
+	client.On("AddCheckHook", mock.AnythingOfType("*types.CheckConfig"), mock.AnythingOfType("*types.HookList")).Return(nil)
 	client.On("FetchCheck", "name").Return(types.FixtureCheckConfig("name"), nil)
 
 	cmd := AddCheckHookCommand(cli)

cli/commands/check/list.go

@@ -95,7 +95,7 @@ func printToTable(results interface{}, writer io.Writer) {
 			Title: "Hooks",
 			CellTransformer: func(data interface{}) string {
 				check, _ := data.(types.CheckConfig)
-				return globals.FormatCheckHooks(check.CheckHooks)
+				return globals.FormatHookLists(check.CheckHooks)
 			},
 		},
 		{

cli/commands/check/show.go

@@ -85,7 +85,7 @@ func printCheckToList(r *types.CheckConfig, writer io.Writer) {
 			},
 			{
 				Label: "Hooks",
-				Value: globals.FormatCheckHooks(r.CheckHooks),
+				Value: globals.FormatHookLists(r.CheckHooks),
 			},
 			{
 				Label: "Publish?",

cli/elements/globals/maps.go

@@ -7,12 +7,12 @@ import (
 	"github.com/sensu/sensu-go/types"
 )
 
-// FormatCheckHooks formats the Check Hook struct into a string mapping
-func FormatCheckHooks(checkHooks []types.CheckHook) string {
+// FormatHookLists formats the Check Hook struct into a string mapping
+func FormatHookLists(hookLists []types.HookList) string {
 	hooksString := []string{}
-	for _, checkHook := range checkHooks {
-		hookString := fmt.Sprintf("%s: [", checkHook.Type)
-		hookString += fmt.Sprintf("%s]", strings.Join(checkHook.Hooks, ", "))
+	for _, hookList := range hookLists {
+		hookString := fmt.Sprintf("%s: [", hookList.Type)
+		hookString += fmt.Sprintf("%s]", strings.Join(hookList.Hooks, ", "))
 		hooksString = append(hooksString, hookString)
 	}
 	return strings.Join(hooksString, ", ")

testing/e2e/rbac_test.go

@@ -113,7 +113,7 @@ func TestRBAC(t *testing.T) {
 	)
 	assert.NoError(t, err, string(output))
 
-	checkHook := types.FixtureCheckHook("hook1")
+	checkHook := types.FixtureHookList("hook1")
 	output, err = adminctl.run("check", "add-hook", defaultCheck.Name,
 		"--organization", defaultCheck.Organization,
 		"--environment", defaultCheck.Environment,

types/check.go

@@ -1,25 +1,14 @@
 package types
 
 import (
-	"encoding/json"
 	"errors"
 	"fmt"
-	"regexp"
 	"sort"
 	"time"
 
 	"github.com/sensu/sensu-go/types/dynamic"
 )
 
-// CheckHookRegexStr used to validate type of check hook
-var CheckHookRegexStr = `([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])`
-
-// CheckHookRegex used to validate type of check hook
-var CheckHookRegex = regexp.MustCompile("^" + CheckHookRegexStr + "$")
-
-// Severities used to validate type of check hook
-var Severities = []string{"ok", "warning", "critical", "unknown", "non-zero"}
-
 // CheckRequestType is the message type string for check request.
 const CheckRequestType = "check_request"
 
@@ -53,27 +42,6 @@ func (c *CheckConfig) SetExtendedAttributes(e []byte) {
 	c.ExtendedAttributes = e
 }
 
-// MarshalJSON implements the json.Marshaler interface.
-func (h *CheckHook) MarshalJSON() ([]byte, error) {
-	result := make(map[string][]string)
-	result[h.Type] = append(result[h.Type], h.Hooks...)
-	return json.Marshal(result)
-}
-
-// UnmarshalJSON implements the json.Marshaler interface.
-func (h *CheckHook) UnmarshalJSON(b []byte) error {
-	result := map[string][]string{}
-	if err := json.Unmarshal(b, &result); err != nil {
-		return err
-	}
-	for key := range result {
-		h.Type = key
-		h.Hooks = result[key]
-	}
-
-	return nil
-}
-
 // Get implements govaluate.Parameters
 func (c *CheckConfig) Get(name string) (interface{}, error) {
 	return dynamic.GetField(c, name)
@@ -114,30 +82,6 @@ func (c *CheckConfig) Validate() error {
 	return nil
 }
 
-// Validate returns an error if the check hook does not pass validation tests.
-func (h *CheckHook) Validate() error {
-	if h.Type == "" {
-		return errors.New("type cannot be empty")
-	}
-
-	if !(CheckHookRegex.MatchString(h.Type) || isSeverity(h.Type)) {
-		return errors.New(
-			"valid check hook types are \"1\"-\"255\", \"ok\", \"warning\", \"critical\", \"unknown\", and \"non-zero\"",
-		)
-	}
-
-	return nil
-}
-
-func isSeverity(name string) bool {
-	for _, sev := range Severities {
-		if sev == name {
-			return true
-		}
-	}
-	return false
-}
-
 // ByExecuted implements the sort.Interface for []CheckHistory based on the
 // Executed field.
 //
@@ -194,7 +138,7 @@ func FixtureCheckConfig(id string) *CheckConfig {
 		Command:       "command",
 		Handlers:      []string{},
 		RuntimeAssets: []string{"ruby-2-4-2"},
-		CheckHooks:    []CheckHook{*FixtureCheckHook("hook1")},
+		CheckHooks:    []HookList{*FixtureHookList("hook1")},
 		Environment:   "default",
 		Organization:  "default",
 		Publish:       true,
@@ -223,11 +167,3 @@ func FixtureCheck(id string) *Check {
 		Config:   config,
 	}
 }
-
-// FixtureCheckHook returns a fixture for a CheckHook object.
-func FixtureCheckHook(hookName string) *CheckHook {
-	return &CheckHook{
-		Hooks: []string{hookName},
-		Type:  "non-zero",
-	}
-}