novox/mesh-controller
3
0
Fork 0
Code Issues Pull Requests 1 Releases Wiki Activity

needs is now own-secrets, named for whose it is

Browse Source 
It sat beside `secrets` — where a *provision's* credential lands on a consumer.
Both were name-to-path, both held something secret, and the names
distinguished them not at all. Reaching for the wrong one parsed cleanly and
failed somewhere else entirely, which is the shape of fault this whole design
exists to prevent, sitting in the manifest format.

The axis that separates them is not how secret they are — both are — but
whose. `secrets` is keyed by the provision it is for and belongs to a
relationship with another machine. `own-secrets` is keyed by a name the module
chose and belongs to nobody else.

A manifest using the old name is told the new one rather than refused with
"unknown field": whoever wrote it knew what they meant, and the mesh knows what
it is called now. An invented key is still refused as one rather than guessed
at.

Found by auditing the 19 manifest fields for whether any could be mistaken for
another. This was the only pair that could — and while checking it, a second
instance of the same collision turned up one layer down: `Manifest.Needs` and
`Resolution.Needs` were different concepts sharing a name in Go. The rename
separates those too.
This commit is contained in:
jschoubben
2026-08-31 13:47:21 +02:00
parent e3a2790acd
commit a18c3b9d13
5 changed files with 65 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/catalogue/declaration.go
+2 -2
View File
@@ -164,7 +164,7 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) {
})
}
}
for _, name := range sortedKeys(m.Needs) {
for _, name := range sortedKeys(m.OwnSecrets) {
sealed := with.Needed[m.Module][name]
if sealed == "" {
// Declared and not made. Refused rather than skipped: a module whose own
@@ -174,7 +174,7 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) {
"%s needs a secret called %q and none was made for it", m.Module, name)
}
first = append(first, map[string]any{
"id": NeedID(name), "type": "file", "path": m.Needs[name], "sealed": sealed,
"id": NeedID(name), "type": "file", "path": m.OwnSecrets[name], "sealed": sealed,
})
}
for _, to := range sortedKeys(m.Secrets) {
internal/catalogue/filtering_test.go
+1 -1
View File
@@ -276,7 +276,7 @@ func TestWhatTheMeshComputesIsAppliedBeforeWhatTheModuleDeclared(t *testing.T) {
func TestAComputedModuleStillGetsWhatTheMeshMadeForIt(t *testing.T) {
r := Resolution{Modules: []Manifest{{
Module: "networking", Computed: "mesh-network",
Needs: map[string]string{"key": "/var/lib/mesh/key"},
OwnSecrets: map[string]string{"key": "/var/lib/mesh/key"},
}}}
out, err := r.Declaration(Rendering{
Needed: map[string]map[string]string{"networking": {"key": "sealed"}},
internal/catalogue/manifest.go
+28 -3
View File
@@ -29,6 +29,16 @@ const (
//
// Constrained because these become resource identities, permission patterns and error messages,
// and a name that is valid in one and not the others is a fault found late.
// renamed is what a field used to be called, and what it is now.
//
// Kept rather than dropped once the rename is done: a manifest written against the old name is
// refused either way, and the difference is whether whoever wrote it has to go and find out why.
var renamed = map[string]string{
// `needs` and `secrets` were both name-to-path and differed only in whose secret it was, so
// reaching for the wrong one parsed cleanly and failed somewhere else entirely.
"needs": "own-secrets",
}
var name = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*(\.[a-z0-9][a-z0-9-]*)*$`)
// Claim is a singular resource a module takes over.
@@ -200,7 +210,13 @@ type Manifest struct {
// makes `restart-on` precise.
Secrets map[string]string `json:"secrets,omitempty"`
// Needs is a secret this module needs for itself, and where to put it.
// OwnSecrets are secrets this module needs in order to be itself, and where to put them.
//
// **Named for whose they are, not how secret they are.** `secrets` above is a credential for
// reaching something else, keyed by the provision it belongs to. These are keyed by a name the
// module chose and belong to nobody else. Both were `map[string]string` of name to path, and
// the field was called `needs` — so reaching for the wrong one parsed cleanly and failed
// somewhere else entirely, which is the shape of fault this whole design exists to prevent.
//
// Not tied to a consumer. A database has a superuser password, a broker has an administrator,
// a registry has an account — each is a secret the module needs in order to be itself, and
@@ -211,7 +227,7 @@ type Manifest struct {
// module running on three machines has three passwords and the mesh can read none of them. A
// manifest carrying one instead would put the same secret on every machine that ever runs the
// module, in a file anybody can read, for ever.
Needs map[string]string `json:"needs,omitempty"`
OwnSecrets map[string]string `json:"own-secrets,omitempty"`
// Listens is what this module accepts connections on, and from where.
//
@@ -404,6 +420,15 @@ func ParseManifest(raw []byte) (Manifest, error) {
decoder := json.NewDecoder(bytes.NewReader(raw))
decoder.DisallowUnknownFields()
if err := decoder.Decode(&m); err != nil {
// A key that used to mean something says what it became. Refusing a renamed field with
// "unknown field" is correct and unhelpful: whoever wrote it knew what they meant, and
// the mesh knows what it is called now.
for was, is := range renamed {
if strings.Contains(err.Error(), `"`+was+`"`) {
return Manifest{}, fmt.Errorf(
"this manifest says %q, which is now called %q: %w", was, is, err)
}
}
return Manifest{}, fmt.Errorf("this is not a module manifest: %w", err)
}
@@ -539,7 +564,7 @@ func ParseManifest(raw []byte) (Manifest, error) {
m.Module, c.Authority))
}
}
for name, where := range m.Needs {
for name, where := range m.OwnSecrets {
if !strings.HasPrefix(where, "/") {
problems = append(problems, fmt.Sprintf(
"%s needs %q at %q, which is not an absolute path", m.Module, name, where))
internal/catalogue/needs_test.go
+33 -3
View File
@@ -14,7 +14,7 @@ import (
func needy() Manifest {
return Manifest{
Module: "postgres", Version: "1",
Needs: map[string]string{"superuser": "/var/lib/mesh/postgres/superuser"},
OwnSecrets: map[string]string{"superuser": "/var/lib/mesh/postgres/superuser"},
Resources: []map[string]any{
{"id": "store", "type": "container", "name": "mesh-postgres", "image": "postgres@sha256:x"},
},
@@ -62,7 +62,7 @@ func TestADeclaredNeedThatWasNotMadeIsRefused(t *testing.T) {
func TestANeedIsAnAbsolutePath(t *testing.T) {
_, err := ParseManifest([]byte(`{"module":"postgres","version":"1",
"needs":{"superuser":"superuser.txt"}}`))
"own-secrets":{"superuser":"superuser.txt"}}`))
if err == nil {
t.Fatal("a relative path was accepted")
}
@@ -74,7 +74,7 @@ func TestANeedIsAnAbsolutePath(t *testing.T) {
func TestAModuleMayNeedSeveralThings(t *testing.T) {
// A password and a token, say. Telling them apart is the module's business, not the mesh's.
m := needy()
m.Needs["replication"] = "/var/lib/mesh/postgres/replication"
m.OwnSecrets["replication"] = "/var/lib/mesh/postgres/replication"
got, _ := Resolve(shelf(m), []string{"postgres"}, reachable(), World{})
out, err := got.Declaration(Rendering{Needed: map[string]map[string]string{
"postgres": {"superuser": "b25l", "replication": "dHdv"},
@@ -92,3 +92,33 @@ func TestAModuleMayNeedSeveralThings(t *testing.T) {
t.Fatalf("two needs did not land as two secrets: %v", seen)
}
}
// A manifest written against the old name is told what the field became.
//
// `needs` and `secrets` were both name-to-path and differed only in whose secret it was, so
// reaching for the wrong one parsed cleanly and failed somewhere else entirely. Refusing the old
// name with "unknown field" would be correct and unhelpful: whoever wrote it knew what they meant,
// and the mesh knows what it is called now.
func TestAManifestUsingTheOldNameIsToldTheNewOne(t *testing.T) {
_, err := ParseManifest([]byte(
`{"module":"postgres","version":"1","needs":{"superuser":"/var/lib/superuser"}}`))
if err == nil {
t.Fatal("a manifest using the old name was accepted, so two fields now mean one thing")
}
for _, want := range []string{"needs", "own-secrets"} {
if !strings.Contains(err.Error(), want) {
t.Fatalf("the refusal does not mention %q: %v", want, err)
}
}
}
// And an ordinary unknown key is still refused as one, rather than being guessed at.
func TestAnInventedKeyIsNotTreatedAsARename(t *testing.T) {
_, err := ParseManifest([]byte(`{"module":"postgres","version":"1","nonsense":{}}`))
if err == nil {
t.Fatal("an invented key was accepted")
}
if strings.Contains(err.Error(), "is now called") {
t.Fatalf("an invented key was reported as a rename: %v", err)
}
}