Refactor hooks package

internal/config/config.go

@@ -303,13 +303,13 @@ type SnapshottingEnum struct {
 }
 
 type SnapshottingPeriodic struct {
-	Type            string   `yaml:"type" validate:"required"`
-	Prefix          string   `yaml:"prefix" validate:"required"`
-	Interval        Duration `yaml:"interval"`
-	Cron            string   `yaml:"cron"`
-	Hooks           HookList `yaml:"hooks"`
-	TimestampFormat string   `yaml:"timestamp_format" default:"dense" validate:"required"`
-	TimestampLocal  bool     `yaml:"timestamp_local" default:"true"`
+	Type            string        `yaml:"type" validate:"required"`
+	Prefix          string        `yaml:"prefix" validate:"required"`
+	Interval        Duration      `yaml:"interval"`
+	Cron            string        `yaml:"cron"`
+	Hooks           []HookCommand `yaml:"hooks" validate:"dive"`
+	TimestampFormat string        `yaml:"timestamp_format" default:"dense" validate:"required"`
+	TimestampLocal  bool          `yaml:"timestamp_local" default:"true"`
 }
 
 func (self *SnapshottingPeriodic) CronSpec() string {
@@ -579,23 +579,23 @@ type GlobalStdinServer struct {
 	SockDir string `yaml:"sockdir" default:"/var/run/zrepl/stdinserver" validate:"required"`
 }
 
-type HookList []HookEnum
-
-type HookEnum struct {
-	Ret any `validate:"required"`
-}
-
 type HookCommand struct {
-	Path               string            `yaml:"path" validate:"required"`
-	Timeout            time.Duration     `yaml:"timeout" default:"30s" validate:"gt=0s"`
-	Filesystems        FilesystemsFilter `yaml:"filesystems" validate:"required_without=Datasets"`
-	Datasets           []DatasetFilter   `yaml:"datasets" validate:"required_without=Filesystems,dive"`
-	HookSettingsCommon `yaml:",inline"`
-}
-
-type HookSettingsCommon struct {
-	Type       string `yaml:"type" validate:"required"`
-	ErrIsFatal bool   `yaml:"err_is_fatal"`
+	Path        string            `yaml:"path" validate:"required"`
+	Timeout     time.Duration     `yaml:"timeout" default:"30s" validate:"min=0s"`
+	Filesystems FilesystemsFilter `yaml:"filesystems"`
+	Datasets    []DatasetFilter   `yaml:"datasets" validate:"dive"`
+	ErrIsFatal  bool              `yaml:"err_is_fatal"`
+}
+
+func (self *HookCommand) UnmarshalYAML(value *yaml.Node) error {
+	type hookCommand HookCommand
+	v := (*hookCommand)(self)
+	if err := value.Decode(v); err != nil {
+		return fmt.Errorf("UnmarshalYAML %T: %w", self, err)
+	} else if err := defaults.Set(v); err != nil {
+		return fmt.Errorf("set defaults for %T: %w", self, err)
+	}
+	return nil
 }
 
 func enumUnmarshal(value *yaml.Node, types map[string]any) (any, error) {
@@ -705,15 +705,6 @@ func (t *SyslogFacility) UnmarshalYAML(value *yaml.Node) (err error) {
 	return t.UnmarshalJSON([]byte(s))
 }
 
-var _ yaml.Unmarshaler = (*HookEnum)(nil)
-
-func (t *HookEnum) UnmarshalYAML(value *yaml.Node) (err error) {
-	t.Ret, err = enumUnmarshal(value, map[string]any{
-		"command": new(HookCommand),
-	})
-	return
-}
-
 var ConfigFileDefaultLocations = []string{
 	"/etc/zrepl/zrepl.yml",
 	"/usr/local/etc/zrepl/zrepl.yml",

internal/config/config_snapshotting_test.go

@@ -104,8 +104,8 @@ jobs:
 	t.Run("hooks", func(t *testing.T) {
 		c = testValidConfig(t, fillSnapshotting(hooks))
 		hs := c.Jobs[0].Ret.(*PushJob).Snapshotting.Ret.(*SnapshottingPeriodic).Hooks
-		assert.True(t, hs[0].Ret.(*HookCommand).Filesystems["<"])
-		assert.True(t, hs[1].Ret.(*HookCommand).Filesystems["zroot<"])
+		assert.True(t, hs[0].Filesystems["<"])
+		assert.True(t, hs[1].Filesystems["zroot<"])
 	})
 }
 

internal/daemon/filters/fsmapfilter.go

@@ -150,3 +150,5 @@ func (self *DatasetFilter) UserSpecifiedDatasets() zfs.UserSpecifiedDatasetsSet
 // mapping results are mapped to accepting filter results. All rejecting mapping
 // results are mapped to rejecting filter results.
 func (self *DatasetFilter) AsFilter() endpoint.FSFilter { return self }
+
+func (self *DatasetFilter) Empty() bool { return len(self.entries) == 0 }

internal/daemon/hooks/hook_config.go

@@ -9,40 +9,30 @@ import (
 
 type List []Hook
 
-func HookFromConfig(in config.HookEnum) (Hook, error) {
-	switch v := in.Ret.(type) {
-	case *config.HookCommand:
-		return NewCommandHook(v)
-	default:
-		return nil, fmt.Errorf("unknown hook type %T", v)
-	}
-}
-
-func ListFromConfig(in *config.HookList) (r *List, err error) {
-	hl := make(List, len(*in))
-
-	for i, h := range *in {
-		hl[i], err = HookFromConfig(h)
+func ListFromConfig(in []config.HookCommand) (List, error) {
+	hl := make(List, len(in))
+	for i, h := range in {
+		h, err := NewCommandHook(&h)
 		if err != nil {
 			return nil, fmt.Errorf("create hook #%d: %s", i+1, err)
 		}
+		hl[i] = h
 	}
-
-	return &hl, nil
+	return hl, nil
 }
 
-func (l List) CopyFilteredForFilesystem(fs *zfs.DatasetPath) (ret List, err error) {
-	ret = make(List, 0, len(l))
-
-	for _, h := range l {
-		var passFilesystem bool
-		if passFilesystem, err = h.Filesystems().Filter(fs); err != nil {
-			return nil, err
+func (self List) CopyFilteredForFilesystem(fs *zfs.DatasetPath) (List, error) {
+	ret := make(List, 0, len(self))
+	for _, h := range self {
+		if h.Filesystems().Empty() {
+			ret = append(ret, h)
+			continue
 		}
-		if passFilesystem {
+		if ok, err := h.Filesystems().Filter(fs); err != nil {
+			return nil, err
+		} else if ok {
 			ret = append(ret, h)
 		}
 	}
-
 	return ret, nil
 }

internal/daemon/hooks/hook_docs.go

@@ -1,33 +1,40 @@
 // Package hooks implements pre- and post snapshot hooks.
 //
-// Plan is a generic executor for ExpectStepReports before and after an activity specified in a callback.
-// It provides a reporting facility that can be polled while the plan is executing to gather progress information.
+// Plan is a generic executor for ExpectStepReports before and after an activity
+// specified in a callback. It provides a reporting facility that can be polled
+// while the plan is executing to gather progress information.
 //
-// This package also provides all supported hook type implementations and abstractions around them.
+// This package also provides all supported hook type implementations and
+// abstractions around them.
 //
 // # Use For Other Kinds Of ExpectStepReports
 //
-// This package REQUIRES REFACTORING before it can be used for other activities than snapshots, e.g. pre- and post-replication:
+// This package REQUIRES REFACTORING before it can be used for other activities
+// than snapshots, e.g. pre- and post-replication:
 //
-// The Hook interface requires a hook to provide a Filesystems() filter, which doesn't make sense for
-// all kinds of activities.
+// The Hook interface requires a hook to provide a Filesystems() filter, which
+// doesn't make sense for all kinds of activities.
 //
-// The hook implementations should move out of this package.
-// However, there is a lot of tight coupling which to untangle isn't worth it ATM.
+// The hook implementations should move out of this package. However, there is a
+// lot of tight coupling which to untangle isn't worth it ATM.
 //
 // # How This Package Is Used By Package Snapper
 //
-// Deserialize a config.List using ListFromConfig().
-// Then it MUST filter the list to only contain hooks for a particular filesystem using
+// Deserialize a config.List using ListFromConfig(). Then it MUST filter the
+// list to only contain hooks for a particular filesystem using
 // hooksList.CopyFilteredForFilesystem(fs).
 //
 // Then create a CallbackHook using NewCallbackHookForFilesystem().
 //
-// Pass all of the above to NewPlan() which provides a Report() and Run() method:
+// Pass all of the above to NewPlan() which provides a Report() and Run()
+// method:
 //
-// Plan.Run(ctx context.Context,dryRun bool) executes the plan and take a context as argument that should contain a logger added using hooks.WithLogger()).
-// The value of dryRun is passed through to the hooks' Run() method.
-// Command hooks make it available in the environment variable ZREPL_DRYRUN.
+// Plan.Run(ctx context.Context,dryRun bool) executes the plan and take a
+// context as argument that should contain a logger added using
+// hooks.WithLogger()). The value of dryRun is passed through to the hooks'
+// Run() method. Command hooks make it available in the environment variable
+// ZREPL_DRYRUN.
 //
-// Plan.Report() can be called while Plan.Run() is executing to give an overview of plan execution progress (future use in "zrepl status").
+// Plan.Report() can be called while Plan.Run() is executing to give an overview
+// of plan execution progress (future use in "zrepl status").
 package hooks