diff --git a/internal/catalogue/shares.go b/internal/catalogue/shares.go index f23a5d17..87b56f3a 100644 --- a/internal/catalogue/shares.go +++ b/internal/catalogue/shares.go @@ -46,8 +46,9 @@ func nfsServerVerbs() []Verb { "address": "a client's address on the private network (optional)", }, []string{"share"}), Replaces: []string{"showmount -e"}}, - {Name: "reload", Description: "Write this machine's export file again from the module's settings and " + - "have the kernel take it. Changes no shared path.", + {Name: "reload", Description: "Have the kernel read this machine's export files again now (exportfs " + + "-ra) and answer the shares as it then holds them. The module's own process writes its export " + + "file; this is for after a change made outside it. Changes no shared path.", Input: schema(map[string]string{}, nil), Replaces: []string{"exportfs -ra"}}, {Name: "adopt", Description: "Take over an export a person made by hand: clear a dataset's sharenfs " + @@ -55,7 +56,7 @@ func nfsServerVerbs() []Verb { "confirm, says what it would do and changes nothing.", Input: schema(map[string]string{ "path": "the shared path whose hand-made export is taken over", - "confirm": "true to change it; anything else is a dry run", + "confirm": "\"true\": change it (needs why); anything else is a dry run", "why": "why, for the record", }, []string{"path"}, "confirm"), Replaces: []string{"zfs set sharenfs=off"}}, @@ -87,7 +88,7 @@ func mountsVerbs() []Verb { "the mesh's automount for it is armed. Without confirm, says what it would do and changes nothing.", Input: schema(map[string]string{ "mountpoint": "the mount point whose fstab line is taken over", - "confirm": "true to change it; anything else is a dry run", + "confirm": "\"true\": change it (needs why); anything else is a dry run", "why": "why, for the record", }, []string{"mountpoint"}, "confirm"), Replaces: []string{"sed -i /etc/fstab"}}, diff --git a/internal/catalogue/shares_test.go b/internal/catalogue/shares_test.go index 51ef5ee9..5fbe0f10 100644 --- a/internal/catalogue/shares_test.go +++ b/internal/catalogue/shares_test.go @@ -126,3 +126,36 @@ func TestTheShareVerbsSayWhatTheyReplace(t *testing.T) { t.Errorf("%s is not a verb of the compiled seats", verb) } } + +// The confirm switch is the seat's string "true", as every verb's switch is, and its description says so: +// a holder handed the boolean reading of "true to change it" would treat the string as a dry run. +func TestTheConfirmSwitchIsTheStringTrue(t *testing.T) { + for _, seat := range []string{NFSServerSeat, MountsSeat} { + s, _ := SeatNamed(seat) + for _, v := range s.Serves { + props, _ := v.Input["properties"].(map[string]any) + confirm, _ := props["confirm"].(map[string]any) + if confirm == nil { + continue + } + if confirm["type"] != "string" { + t.Errorf("%s.%s confirm is %v, not the string switch", seat, v.Name, confirm["type"]) + } + if d, _ := confirm["description"].(string); !strings.Contains(d, `"true"`) { + t.Errorf("%s.%s confirm does not name the string \"true\": %q", seat, v.Name, d) + } + } + } +} + +// reload has the kernel read the export files; the module's process writes its file. The description +// must not promise a write it does not do. +func TestReloadSaysWhatItDoes(t *testing.T) { + s, _ := SeatNamed(NFSServerSeat) + for _, v := range s.Serves { + if v.Name == "reload" && (strings.Contains(v.Description, "Write this machine's export file") || + !strings.Contains(v.Description, "exportfs")) { + t.Errorf("reload's description promises a write or does not name exportfs: %s", v.Description) + } + } +}