zfs.NilBool: document its purpose and move it to its own package 'nodefault'

daemon/job/build_jobs_sendrecvoptions.go

@@ -6,6 +6,7 @@ import (
 	"github.com/zrepl/zrepl/config"
 	"github.com/zrepl/zrepl/daemon/filters"
 	"github.com/zrepl/zrepl/endpoint"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/zfs"
 )
 
@@ -23,7 +24,7 @@ func buildSenderConfig(in SendingJobConfig, jobID endpoint.JobID) (*endpoint.Sen
 
 	return &endpoint.SenderConfig{
 		FSF:     fsf,
-		Encrypt: &zfs.NilBool{B: in.GetSendOptions().Encrypted},
+		Encrypt: &nodefault.Bool{B: in.GetSendOptions().Encrypted},
 		JobID:   jobID,
 	}, nil
 }

daemon/job/snapjob.go

@@ -10,6 +10,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 
 	"github.com/zrepl/zrepl/daemon/logging/trace"
+	"github.com/zrepl/zrepl/util/nodefault"
 
 	"github.com/zrepl/zrepl/config"
 	"github.com/zrepl/zrepl/daemon/filters"
@@ -179,7 +180,7 @@ func (j *SnapJob) doPrune(ctx context.Context) {
 		JobID: j.name,
 		FSF:   j.fsfilter,
 		// FIXME encryption setting is irrelevant for SnapJob because the endpoint is only used as pruner.Target
-		Encrypt: &zfs.NilBool{B: true},
+		Encrypt: &nodefault.Bool{B: true},
 	})
 	j.prunerMtx.Lock()
 	j.pruner = j.prunerFactory.BuildLocalPruner(ctx, sender, alwaysUpToDateReplicationCursorHistory{sender})

endpoint/endpoint.go

@@ -17,19 +17,20 @@ import (
 	"github.com/zrepl/zrepl/util/chainedio"
 	"github.com/zrepl/zrepl/util/chainlock"
 	"github.com/zrepl/zrepl/util/envconst"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/util/semaphore"
 	"github.com/zrepl/zrepl/zfs"
 )
 
 type SenderConfig struct {
 	FSF     zfs.DatasetFilter
-	Encrypt *zfs.NilBool
+	Encrypt *nodefault.Bool
 	JobID   JobID
 }
 
 func (c *SenderConfig) Validate() error {
 	c.JobID.MustValidate()
-	if err := c.Encrypt.Validate(); err != nil {
+	if err := c.Encrypt.ValidateNoDefault(); err != nil {
 		return errors.Wrap(err, "`Encrypt` field invalid")
 	}
 	if _, err := StepHoldTag(c.JobID); err != nil {
@@ -43,7 +44,7 @@ type Sender struct {
 	pdu.UnsafeReplicationServer // prefer compilation errors over default 'method X not implemented' impl
 
 	FSFilter zfs.DatasetFilter
-	encrypt  *zfs.NilBool
+	encrypt  *nodefault.Bool
 	jobId    JobID
 }
 

endpoint/endpoint_zfs_abstraction.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/zrepl/zrepl/daemon/logging/trace"
 	"github.com/zrepl/zrepl/util/envconst"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/util/semaphore"
 	"github.com/zrepl/zrepl/zfs"
 )
@@ -257,7 +258,7 @@ type ListZFSHoldsAndBookmarksQuery struct {
 
 type CreateTXGRangeBound struct {
 	CreateTXG uint64
-	Inclusive *zfs.NilBool // must not be nil
+	Inclusive *nodefault.Bool // must not be nil
 }
 
 // A non-empty range of CreateTXGs
@@ -300,7 +301,7 @@ func (q *ListZFSHoldsAndBookmarksQuery) Validate() error {
 var createTXGRangeBoundAllowCreateTXG0 = envconst.Bool("ZREPL_ENDPOINT_LIST_ABSTRACTIONS_QUERY_CREATETXG_RANGE_BOUND_ALLOW_0", false)
 
 func (i *CreateTXGRangeBound) Validate() error {
-	if err := i.Inclusive.Validate(); err != nil {
+	if err := i.Inclusive.ValidateNoDefault(); err != nil {
 		return errors.Wrap(err, "Inclusive")
 	}
 	if i.CreateTXG == 0 && !createTXGRangeBoundAllowCreateTXG0 {
@@ -419,7 +420,7 @@ func (r *CreateTXGRange) String() string {
 	if r.Since == nil {
 		fmt.Fprintf(&buf, "~")
 	} else {
-		if err := r.Since.Inclusive.Validate(); err != nil {
+		if err := r.Since.Inclusive.ValidateNoDefault(); err != nil {
 			fmt.Fprintf(&buf, "?")
 		} else if r.Since.Inclusive.B {
 			fmt.Fprintf(&buf, "[")
@@ -435,7 +436,7 @@ func (r *CreateTXGRange) String() string {
 		fmt.Fprintf(&buf, "~")
 	} else {
 		fmt.Fprintf(&buf, "%d", r.Until.CreateTXG)
-		if err := r.Until.Inclusive.Validate(); err != nil {
+		if err := r.Until.Inclusive.ValidateNoDefault(); err != nil {
 			fmt.Fprintf(&buf, "?")
 		} else if r.Until.Inclusive.B {
 			fmt.Fprintf(&buf, "]")
@@ -829,14 +830,14 @@ func listStaleFiltering(abs []Abstraction, sinceBound *CreateTXGRangeBound) *Sta
 					untilBound = &CreateTXGRangeBound{
 						CreateTXG: (*sfnsc.cursor).GetCreateTXG(),
 						// if we have a cursor, can throw away step hold on both From and To
-						Inclusive: &zfs.NilBool{B: true},
+						Inclusive: &nodefault.Bool{B: true},
 					}
 				} else if sfnsc.step != nil {
 					untilBound = &CreateTXGRangeBound{
 						CreateTXG: (*sfnsc.step).GetCreateTXG(),
 						// if we don't have a cursor, the step most recent step hold is our
 						// initial replication cursor and it's possibly still live (interrupted initial replication)
-						Inclusive: &zfs.NilBool{B: false},
+						Inclusive: &nodefault.Bool{B: false},
 					}
 				} else {
 					untilBound = nil // consider everything stale

endpoint/endpoint_zfs_abstraction_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/zrepl/zrepl/zfs"
+	"github.com/zrepl/zrepl/util/nodefault"
 )
 
 func TestCreateTXGRange(t *testing.T) {
@@ -47,8 +47,8 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "wrong order obvious",
 			expectInvalid: true,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{23, &zfs.NilBool{B: true}},
+				Since: &CreateTXGRangeBound{23, &nodefault.Bool{B: true}},
-				Until: &CreateTXGRangeBound{20, &zfs.NilBool{B: true}},
+				Until: &CreateTXGRangeBound{20, &nodefault.Bool{B: true}},
 			},
 			expectString: "[23,20]",
 		},
@@ -56,8 +56,8 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "wrong order edge-case could also be empty",
 			expectInvalid: true,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{23, &zfs.NilBool{B: false}},
+				Since: &CreateTXGRangeBound{23, &nodefault.Bool{B: false}},
-				Until: &CreateTXGRangeBound{22, &zfs.NilBool{B: true}},
+				Until: &CreateTXGRangeBound{22, &nodefault.Bool{B: true}},
 			},
 			expectString: "(23,22]",
 		},
@@ -65,8 +65,8 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "empty",
 			expectInvalid: true,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{2, &zfs.NilBool{B: false}},
+				Since: &CreateTXGRangeBound{2, &nodefault.Bool{B: false}},
-				Until: &CreateTXGRangeBound{2, &zfs.NilBool{B: false}},
+				Until: &CreateTXGRangeBound{2, &nodefault.Bool{B: false}},
 			},
 			expectString: "(2,2)",
 		},
@@ -74,8 +74,8 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "inclusive-since-exclusive-until",
 			expectInvalid: false,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{2, &zfs.NilBool{B: true}},
+				Since: &CreateTXGRangeBound{2, &nodefault.Bool{B: true}},
-				Until: &CreateTXGRangeBound{5, &zfs.NilBool{B: false}},
+				Until: &CreateTXGRangeBound{5, &nodefault.Bool{B: false}},
 			},
 			expectString: "[2,5)",
 			expect: []testCaseExpectation{
@@ -92,8 +92,8 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "exclusive-since-inclusive-until",
 			expectInvalid: false,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{2, &zfs.NilBool{B: false}},
+				Since: &CreateTXGRangeBound{2, &nodefault.Bool{B: false}},
-				Until: &CreateTXGRangeBound{5, &zfs.NilBool{B: true}},
+				Until: &CreateTXGRangeBound{5, &nodefault.Bool{B: true}},
 			},
 			expectString: "(2,5]",
 			expect: []testCaseExpectation{
@@ -111,7 +111,7 @@ func TestCreateTXGRange(t *testing.T) {
 			expectInvalid: true,
 			config: &CreateTXGRange{
 				Since: nil,
-				Until: &CreateTXGRangeBound{0, &zfs.NilBool{B: true}},
+				Until: &CreateTXGRangeBound{0, &nodefault.Bool{B: true}},
 			},
 			expectString: "~,0]",
 		},
@@ -119,7 +119,7 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "half-open-no-until",
 			expectInvalid: false,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{2, &zfs.NilBool{B: false}},
+				Since: &CreateTXGRangeBound{2, &nodefault.Bool{B: false}},
 				Until: nil,
 			},
 			expectString: "(2,~",
@@ -138,7 +138,7 @@ func TestCreateTXGRange(t *testing.T) {
 			expectInvalid: false,
 			config: &CreateTXGRange{
 				Since: nil,
-				Until: &CreateTXGRangeBound{4, &zfs.NilBool{B: true}},
+				Until: &CreateTXGRangeBound{4, &nodefault.Bool{B: true}},
 			},
 			expectString: "~,4]",
 			expect: []testCaseExpectation{
@@ -154,7 +154,7 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "edgeSince",
 			expectInvalid: false,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{math.MaxUint64, &zfs.NilBool{B: true}},
+				Since: &CreateTXGRangeBound{math.MaxUint64, &nodefault.Bool{B: true}},
 				Until: nil,
 			},
 			expectString: "[18446744073709551615,~",
@@ -169,7 +169,7 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "edgeSinceNegative",
 			expectInvalid: true,
 			config: &CreateTXGRange{
-				Since: &CreateTXGRangeBound{math.MaxUint64, &zfs.NilBool{B: false}},
+				Since: &CreateTXGRangeBound{math.MaxUint64, &nodefault.Bool{B: false}},
 				Until: nil,
 			},
 			expectString: "(18446744073709551615,~",
@@ -178,7 +178,7 @@ func TestCreateTXGRange(t *testing.T) {
 			name:          "edgeUntil",
 			expectInvalid: false,
 			config: &CreateTXGRange{
-				Until: &CreateTXGRangeBound{0, &zfs.NilBool{B: true}},
+				Until: &CreateTXGRangeBound{0, &nodefault.Bool{B: true}},
 			},
 			configAllowZeroCreateTXG: true,
 			expectString:             "~,0]",
@@ -193,7 +193,7 @@ func TestCreateTXGRange(t *testing.T) {
 			expectInvalid:            true,
 			configAllowZeroCreateTXG: true,
 			config: &CreateTXGRange{
-				Until: &CreateTXGRangeBound{0, &zfs.NilBool{B: false}},
+				Until: &CreateTXGRangeBound{0, &nodefault.Bool{B: false}},
 			},
 			expectString: "~,0)",
 		},

platformtest/tests/recvForceIntoEncryptedErr.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/zrepl/zrepl/platformtest"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/zfs"
 )
 
@@ -32,7 +33,7 @@ func ReceiveForceIntoEncryptedErr(ctx *platformtest.Context) {
 
 	sendArgs, err := zfs.ZFSSendArgsUnvalidated{
 		FS:          sfs,
-		Encrypted:   &zfs.NilBool{B: false},
+		Encrypted:   &nodefault.Bool{B: false},
 		From:        nil,
 		To:          &sfsSnap1,
 		ResumeToken: "",

platformtest/tests/recvRollback.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/zrepl/zrepl/platformtest"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/zfs"
 )
 
@@ -28,7 +29,7 @@ func ReceiveForceRollbackWorksUnencrypted(ctx *platformtest.Context) {
 
 	sendArgs, err := zfs.ZFSSendArgsUnvalidated{
 		FS:          sfs,
-		Encrypted:   &zfs.NilBool{B: false},
+		Encrypted:   &nodefault.Bool{B: false},
 		From:        nil,
 		To:          &sfsSnap1,
 		ResumeToken: "",

platformtest/tests/replication.go

@@ -19,6 +19,7 @@ import (
 	"github.com/zrepl/zrepl/replication/logic/pdu"
 	"github.com/zrepl/zrepl/replication/report"
 	"github.com/zrepl/zrepl/util/limitio"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/zfs"
 )
 
@@ -57,7 +58,7 @@ func (i replicationInvocation) Do(ctx *platformtest.Context) *report.Report {
 	}
 	sender := i.interceptSender(endpoint.NewSender(endpoint.SenderConfig{
 		FSF:     i.sfilter.AsFilter(),
-		Encrypt: &zfs.NilBool{B: false},
+		Encrypt: &nodefault.Bool{B: false},
 		JobID:   i.sjid,
 	}))
 	receiver := i.interceptReceiver(endpoint.NewReceiver(endpoint.ReceiverConfig{

platformtest/tests/resumableRecvAndTokenHandling.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/zrepl/zrepl/platformtest"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/zfs"
 )
 
@@ -28,7 +29,7 @@ func ResumableRecvAndTokenHandling(ctx *platformtest.Context) {
 	s := makeResumeSituation(ctx, src, recvFS, zfs.ZFSSendArgsUnvalidated{
 		FS:          sendFS,
 		To:          src.snapA,
-		Encrypted:   &zfs.NilBool{B: false},
+		Encrypted:   &nodefault.Bool{B: false},
 		ResumeToken: "",
 	}, zfs.RecvOptions{
 		RollbackAndForceRecv: false, // doesnt' exist yet

platformtest/tests/sendArgsValidation.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/zrepl/zrepl/platformtest"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/zfs"
 )
 
@@ -42,7 +43,7 @@ func sendArgsValidationEncryptedSendOfUnencryptedDatasetForbidden_impl(ctx *plat
 			RelName: "@a snap",
 			GUID:    props.Guid,
 		},
-		Encrypted:   &zfs.NilBool{B: true},
+		Encrypted:   &nodefault.Bool{B: true},
 		ResumeToken: "",
 	}.Validate(ctx)
 
@@ -97,7 +98,7 @@ func SendArgsValidationResumeTokenEncryptionMismatchForbidden(ctx *platformtest.
 	unencS := makeResumeSituation(ctx, src, unencRecvFS, zfs.ZFSSendArgsUnvalidated{
 		FS:        sendFS,
 		To:        src.snapA,
-		Encrypted: &zfs.NilBool{B: false}, // !
+		Encrypted: &nodefault.Bool{B: false}, // !
 	}, zfs.RecvOptions{
 		RollbackAndForceRecv: false,
 		SavePartialRecvState: true,
@@ -106,7 +107,7 @@ func SendArgsValidationResumeTokenEncryptionMismatchForbidden(ctx *platformtest.
 	encS := makeResumeSituation(ctx, src, encRecvFS, zfs.ZFSSendArgsUnvalidated{
 		FS:        sendFS,
 		To:        src.snapA,
-		Encrypted: &zfs.NilBool{B: true}, // !
+		Encrypted: &nodefault.Bool{B: true}, // !
 	}, zfs.RecvOptions{
 		RollbackAndForceRecv: false,
 		SavePartialRecvState: true,
@@ -174,7 +175,7 @@ func SendArgsValidationResumeTokenDifferentFilesystemForbidden(ctx *platformtest
 	rs := makeResumeSituation(ctx, src1, recvFS, zfs.ZFSSendArgsUnvalidated{
 		FS:        sendFS1,
 		To:        src1.snapA,
-		Encrypted: &zfs.NilBool{B: false},
+		Encrypted: &nodefault.Bool{B: false},
 	}, zfs.RecvOptions{
 		RollbackAndForceRecv: false,
 		SavePartialRecvState: true,
@@ -188,7 +189,7 @@ func SendArgsValidationResumeTokenDifferentFilesystemForbidden(ctx *platformtest
 			RelName: src2.snapA.RelName,
 			GUID:    src2.snapA.GUID,
 		},
-		Encrypted:   &zfs.NilBool{B: false},
+		Encrypted:   &nodefault.Bool{B: false},
 		ResumeToken: rs.recvErrDecoded.ResumeTokenRaw,
 	}
 	_, err = maliciousSend.Validate(ctx)

util/nodefault/nodefault.go

@@ -0,0 +1,32 @@
+// Package nodefault provides newtypes around builtin Go types
+// so that if the newtype is used as a field in a struct and that field
+// is zero-initialized (https://golang.org/ref/spec#The_zero_value),
+// accessing the field will cause a null pointer deref panic.
+// Or in other terms: It soft-enforces that the caller sets the field
+// explicitly when constructing the struct.
+//
+// Example:
+//
+//  type Config struct {
+//      // This field must be set to a non-nil value,
+//      // forcing the caller to make their mind up
+//      // about this field.
+//      CriticalSetting   *nodefault.Bool
+//  }
+//
+// An function that takes such a Config should _not_ check for nil-ness:
+// and instead unconditionally dereference:
+//
+//  func f(c Config) {
+//      if (c.CriticalSetting) { }
+//  }
+//
+// If the caller of f forgot to specify the .CriticalSetting
+// field, the Go runtime will issue a nil-pointer deref panic
+// and it'll be clear that the caller did not read the docs of Config.
+//
+//  f(Config{}) // crashes
+//
+//  f Config{ CriticalSetting: &nodefault.Bool{B: false}} // doesn't crash
+//
+package nodefault

util/nodefault/nodefault_bool.go

@@ -0,0 +1,19 @@
+package nodefault
+
+import "fmt"
+
+type Bool struct{ B bool }
+
+func (n *Bool) ValidateNoDefault() error {
+	if n == nil {
+		return fmt.Errorf("must explicitly set `true` or `false`")
+	}
+	return nil
+}
+
+func (n *Bool) String() string {
+	if n == nil {
+		return "unset"
+	}
+	return fmt.Sprintf("%v", n.B)
+}

zfs/zfs.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/zrepl/zrepl/util/circlog"
 	"github.com/zrepl/zrepl/util/envconst"
+	"github.com/zrepl/zrepl/util/nodefault"
 	"github.com/zrepl/zrepl/zfs/zfscmd"
 )
 
@@ -563,27 +564,11 @@ func (v ZFSSendArgVersion) MustBeBookmark() {
 	}
 }
 
-type NilBool struct{ B bool }
-
-func (n *NilBool) Validate() error {
-	if n == nil {
-		return fmt.Errorf("must explicitly set `true` or `false`")
-	}
-	return nil
-}
-
-func (n *NilBool) String() string {
-	if n == nil {
-		return "unset"
-	}
-	return fmt.Sprintf("%v", n.B)
-}
-
 // When updating this struct, check Validate and ValidateCorrespondsToResumeToken (POTENTIALLY SECURITY SENSITIVE)
 type ZFSSendArgsUnvalidated struct {
 	FS        string
 	From, To  *ZFSSendArgVersion // From may be nil
-	Encrypted *NilBool
+	Encrypted *nodefault.Bool
 
 	// Preferred if not empty
 	ResumeToken string // if not nil, must match what is specified in From, To (covered by ValidateCorrespondsToResumeToken)
@@ -596,7 +581,7 @@ type ZFSSendArgsValidated struct {
 }
 
 type zfsSendArgsValidationContext struct {
-	encEnabled *NilBool
+	encEnabled *nodefault.Bool
 }
 
 type ZFSSendArgsValidationErrorCode int
@@ -653,7 +638,7 @@ func (a ZFSSendArgsUnvalidated) Validate(ctx context.Context) (v ZFSSendArgsVali
 		// fallthrough
 	}
 
-	if err := a.Encrypted.Validate(); err != nil {
+	if err := a.Encrypted.ValidateNoDefault(); err != nil {
 		return v, newGenericValidationError(a, errors.Wrap(err, "`Raw` invalid"))
 	}
 
@@ -663,7 +648,7 @@ func (a ZFSSendArgsUnvalidated) Validate(ctx context.Context) (v ZFSSendArgsVali
 		return v, newValidationError(a, ZFSSendArgsFSEncryptionCheckFail,
 			errors.Wrapf(err, "cannot check whether filesystem %q is encrypted", a.FS))
 	}
-	valCtx.encEnabled = &NilBool{fsEncrypted}
+	valCtx.encEnabled = &nodefault.Bool{B: fsEncrypted}
 
 	if a.Encrypted.B && !fsEncrypted {
 		return v, newValidationError(a, ZFSSendArgsEncryptedSendRequestedButFSUnencrypted,
@@ -790,7 +775,7 @@ func ZFSSend(ctx context.Context, sendArgs ZFSSendArgsValidated) (*SendStream, e
 
 	// pre-validation of sendArgs for plain ErrEncryptedSendNotSupported error
 	// TODO go1.13: push this down to sendArgs.Validate
-	if encryptedSendValid := sendArgs.Encrypted.Validate(); encryptedSendValid == nil && sendArgs.Encrypted.B {
+	if encryptedSendValid := sendArgs.Encrypted.ValidateNoDefault(); encryptedSendValid == nil && sendArgs.Encrypted.B {
 		supported, err := EncryptionCLISupported(ctx)
 		if err != nil {
 			return nil, errors.Wrap(err, "cannot determine CLI native encryption support")