Exempt only the user list's content from the grants step's guard, and test the guard in the send itself (review of #230)
mesh/merge-gate pass: builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus step; every machine composes with the change as it…
mesh/repo-check fail: its merge-check.sh failed: --- FAIL: TestTheGrantsStepSendsTheUserListAndNothingElse (1.69s)
mesh/delivery superseded: a newer head of the same pull request
mesh/merge-gate pass: builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus step; every machine composes with the change as it…
mesh/repo-check fail: its merge-check.sh failed: --- FAIL: TestTheGrantsStepSendsTheUserListAndNothingElse (1.69s)
mesh/delivery superseded: a newer head of the same pull request
The guard let any field of the list's resource move; its path or mode changing is a change the step must not carry. The store test now calls sendToEach, so removing the guard there fails it.
This commit is contained in:
@@ -3,11 +3,13 @@ package main
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/novox/mesh-controller/internal/catalogue"
|
"github.com/novox/mesh-controller/internal/catalogue"
|
||||||
|
"github.com/novox/mesh-controller/internal/identity"
|
||||||
"github.com/novox/mesh-controller/internal/inventory"
|
"github.com/novox/mesh-controller/internal/inventory"
|
||||||
"github.com/novox/mesh-controller/internal/link"
|
"github.com/novox/mesh-controller/internal/link"
|
||||||
)
|
)
|
||||||
@@ -169,4 +171,10 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) {
|
|||||||
if !strings.Contains(only, "wallpaper newly assigned") || !strings.Contains(only, "+wallpaper") {
|
if !strings.Contains(only, "wallpaper newly assigned") || !strings.Contains(only, "+wallpaper") {
|
||||||
t.Fatalf("a new assignment on the bus's machine reads %q; want the step refused, naming it", only)
|
t.Fatalf("a new assignment on the bus's machine reads %q; want the step refused, naming it", only)
|
||||||
}
|
}
|
||||||
|
// And the send itself refuses it, unsent: judged where it is composed, before the bus is dialled.
|
||||||
|
identity.ForTest(t)
|
||||||
|
if _, err := sendToEach(kept, open, []string{"anchor"}); !errors.Is(err, errNotGrantsOnly) ||
|
||||||
|
!strings.Contains(err.Error(), "wallpaper") {
|
||||||
|
t.Fatalf("the grants step's send of a new assignment was not refused by the send: %v", err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"slices"
|
"slices"
|
||||||
@@ -140,6 +141,33 @@ func TestAGrantsStepThatWouldCarryMoreIsNotSent(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The step's exemption is the list's content alone: its path or mode moving is a change like any other.
|
||||||
|
func TestOnlyTheUserListsContentIsExempt(t *testing.T) {
|
||||||
|
list := "nats.bus-users"
|
||||||
|
file := func(fields map[string]any) sentResource {
|
||||||
|
r := map[string]any{"id": list, "type": "file", "path": "/conf/accounts.conf", "mode": "0600", "content": "a"}
|
||||||
|
for k, v := range fields {
|
||||||
|
r[k] = v
|
||||||
|
}
|
||||||
|
body, _ := json.Marshal(map[string]any{"resources": []any{r}})
|
||||||
|
s, err := summarize(body)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
return s[0]
|
||||||
|
}
|
||||||
|
was := file(nil)
|
||||||
|
if other := otherThanTheList(diffSent([]sentResource{was}, []sentResource{file(map[string]any{"content": "b"})}), list); len(other) != 0 {
|
||||||
|
t.Errorf("the list's new content reads as more: %v", other)
|
||||||
|
}
|
||||||
|
for _, f := range []map[string]any{{"mode": "0644"}, {"path": "/elsewhere"}, {"content": "b", "owner": "nats"}} {
|
||||||
|
other := otherThanTheList(diffSent([]sentResource{was}, []sentResource{file(f)}), list)
|
||||||
|
if len(other) != 1 || !strings.HasPrefix(other[0], "~"+list) {
|
||||||
|
t.Errorf("the list's resource changed by %v reads %v; want it refused", f, other)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// The delivery plan says the step before the merge.
|
// The delivery plan says the step before the merge.
|
||||||
func TestAChangePlanSaysTheGrantsStep(t *testing.T) {
|
func TestAChangePlanSaysTheGrantsStep(t *testing.T) {
|
||||||
const catalogue_ = "http://forge.internal:20000/novox/mesh-catalog.git"
|
const catalogue_ = "http://forge.internal:20000/novox/mesh-catalog.git"
|
||||||
|
|||||||
@@ -1048,7 +1048,19 @@ func grantsOnlyIn(ctx context.Context, open *stores, holder string, plan catalog
|
|||||||
if err := json.Unmarshal(prev, &before); err != nil {
|
if err := json.Unmarshal(prev, &before); err != nil {
|
||||||
return "", fmt.Errorf("what %s was last sent is kept in a form this version does not read: %w", holder, err)
|
return "", fmt.Errorf("what %s was last sent is kept in a form this version does not read: %w", holder, err)
|
||||||
}
|
}
|
||||||
d := diffSent(before, after)
|
other = append(other, otherThanTheList(diffSent(before, after), listID)...)
|
||||||
|
sort.Strings(other)
|
||||||
|
if len(other) == 0 {
|
||||||
|
return "", nil
|
||||||
|
}
|
||||||
|
return "its send would change more than the bus's user list: " + strings.Join(other, "; "), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// otherThanTheList is what a diff against the last send changes besides the user list's content: every
|
||||||
|
// resource added, removed or changed, but the list's own resource when its content is all that changed —
|
||||||
|
// its path, mode and every other field must be as last sent (novox/hq issue 490).
|
||||||
|
func otherThanTheList(d sentDiff, listID string) []string {
|
||||||
|
var other []string
|
||||||
for _, id := range d.Added {
|
for _, id := range d.Added {
|
||||||
other = append(other, "+"+id)
|
other = append(other, "+"+id)
|
||||||
}
|
}
|
||||||
@@ -1056,16 +1068,12 @@ func grantsOnlyIn(ctx context.Context, open *stores, holder string, plan catalog
|
|||||||
other = append(other, "-"+id)
|
other = append(other, "-"+id)
|
||||||
}
|
}
|
||||||
for _, c := range d.Changed {
|
for _, c := range d.Changed {
|
||||||
if c.ID == listID {
|
if c.ID == listID && len(c.Fields) == 1 && c.Fields[0] == "content" {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
other = append(other, fmt.Sprintf("~%s (%s)", c.ID, strings.Join(c.Fields, ", ")))
|
other = append(other, fmt.Sprintf("~%s (%s)", c.ID, strings.Join(c.Fields, ", ")))
|
||||||
}
|
}
|
||||||
sort.Strings(other)
|
return other
|
||||||
if len(other) == 0 {
|
|
||||||
return "", nil
|
|
||||||
}
|
|
||||||
return "its send would change more than the bus's user list: " + strings.Join(other, "; "), nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// sendJudged is a gated send's sends: the grants step when the user list changed, then the machine judged.
|
// sendJudged is a gated send's sends: the grants step when the user list changed, then the machine judged.
|
||||||
|
|||||||
Reference in New Issue
Block a user