Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion cmd/agent/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type settingState struct {

// settings is every per-cluster setting this agent reads. Adding one is adding
// an entry here, and there is no second place to forget.
var settings = []setting{actionLevel, openStackCredential}
var settings = []setting{actionLevel, instanceProfile, openStackCredential}

// actionLevel is the cluster's own action level (ADR 0011).
//
Expand All @@ -69,6 +69,37 @@ var actionLevel = setting{
},
}

// instanceProfile is what this cluster creates: flavour, image and network
// (ADR 0016, slice 3).
//
// Absent is the ordinary state and not an error — a cluster that has not opted
// in creates nothing, and says which file would change that. A file the
// operator wrote and this agent cannot honour is broken and not applied, so a
// half-configured profile creates nothing rather than something half-chosen:
// the empty field would otherwise reach the resolver, which refuses anyway,
// but one loud line at startup beats the same refusal discovered per call.
var instanceProfile = setting{
name: "instance profile",
file: toolplane.ProfileFileName,
load: func(dir string) settingState {
profile, err := toolplane.ReadInstanceProfile(dir)
switch {
case errors.Is(err, toolplane.ErrNoProfile):
return settingState{line: "instance profile: not configured; creates will refuse until one is"}
case err != nil:
return settingState{
broken: true,
line: fmt.Sprintf("instance profile: %v; creates will refuse until it is corrected", err),
}
}
return settingState{
line: fmt.Sprintf("instance profile: flavor %s, image %s, network %s",
profile.Flavor, profile.Image, profile.Network),
apply: func(r *toolplane.Registry) { r.ConfigureInstanceProfile(profile) },
}
},
}

// openStackCredential is the application credential creates are made with.
//
// Absent is the ordinary state of every cluster that has not opted in: it
Expand Down
51 changes: 49 additions & 2 deletions cmd/agent/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,18 @@ func effects(t *testing.T) []effect {
mode: 0o644,
observe: func(r *toolplane.Registry) string { return r.Level().String() },
},
{
name: "instance profile",
file: toolplane.ProfileFileName,
content: `{"flavor":"m1.large","image":"ubuntu-24.04","network":"tenant-net"}` + "\n",
// The action level's rule, not the credential's: no secret, but
// whoever may write it chooses the image.
mode: 0o644,
observe: func(r *toolplane.Registry) string {
p := r.Profile()
return strings.Join([]string{p.Flavor, p.Image, p.Network}, "/")
},
},
{
name: "OpenStack credential",
file: openstack.CredentialFileName,
Expand Down Expand Up @@ -167,6 +179,41 @@ func TestABrokenSettingIsMarkedAndStillConfigures(t *testing.T) {
}
}

// A profile naming two of three fields is broken rather than unset, and is not
// applied: a half-configured profile creates nothing rather than something
// half-chosen. The agent still configures and still serves its reads.
func TestAHalfConfiguredProfileIsBrokenAndNotApplied(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, toolplane.ProfileFileName)
if err := os.WriteFile(path, []byte(`{"flavor":"m1.large","image":"ubuntu-24.04"}`), 0o644); err != nil {
t.Fatal(err)
}
if err := os.Chmod(path, 0o644); err != nil {
t.Fatal(err)
}

reg, states, err := configure(dir, toolplane.Allowlist, discardAuditor())
if err != nil {
t.Fatalf("configure refused to build a registry over a half-configured profile: %v", err)
}
if got := reg.Profile(); got.Flavor != "" || got.Image != "" {
t.Errorf("Profile() = %+v over a half-configured file, want nothing applied", got)
}

var broken []string
for _, st := range states {
if st.broken {
broken = append(broken, st.line)
}
}
if len(broken) != 1 {
t.Fatalf("broken settings = %d, want exactly the profile: %v", len(broken), broken)
}
if !strings.Contains(broken[0], "network") {
t.Errorf("the line should name the missing field, not the file in general: %q", broken[0])
}
}

// Nothing configured is the ordinary state of a cluster that has not opted in:
// every setting still reports, and the agent serves reads.
func TestAnUnconfiguredDirectoryReportsEverySettingAndServesReads(t *testing.T) {
Expand Down Expand Up @@ -207,8 +254,8 @@ func writeSetting(t *testing.T, dir string, e effect) {
if err := os.WriteFile(filepath.Join(dir, e.file), []byte(e.content), e.mode); err != nil {
t.Fatal(err)
}
// WriteFile's mode is masked by the process umask, and both settings check
// their own mode. Set it explicitly so the test does not depend on one.
// WriteFile's mode is masked by the process umask, and every setting checks
// its own mode. Set it explicitly so the test does not depend on one.
if err := os.Chmod(filepath.Join(dir, e.file), e.mode); err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 3 additions & 3 deletions internal/toolplane/allowlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -808,19 +808,19 @@ func (t Tool) validateGet() error {
if _, declared := t.Params[dcPlaceholder]; declared {
return fmt.Errorf("tool %q declares %s; it is executor context, not a parameter", t.Name, dcPlaceholder)
}
return t.checkPlaceholders(pathSegments(t.Get), map[string]bool{dcPlaceholder: true})
return t.checkPlaceholders(pathSegments(t.Get), map[string]string{dcPlaceholder: ""})
}

// checkPlaceholders enforces that every placeholder in tokens is either an
// exempt executor slot or a declared model parameter, and that every declared
// parameter is used with a finite value set.
func (t Tool) checkPlaceholders(tokens []string, exempt map[string]bool) error {
func (t Tool) checkPlaceholders(tokens []string, exempt map[string]string) error {
seen := map[string]bool{}
for _, tok := range tokens {
if !isPlaceholder(tok) {
continue
}
if exempt[tok] {
if _, ok := exempt[tok]; ok {
continue
}
_, enumerated := t.Params[tok]
Expand Down
100 changes: 100 additions & 0 deletions internal/toolplane/profile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package toolplane

import (
"bytes"
"encoding/json"
"fmt"
"os"
"path/filepath"
)

// ProfileFileName is the file in the agent's own directory that describes what
// this cluster creates (ADR 0011, ADR 0016). Beside the action level and the
// enrollment identity, for the reason that directory holds all of them: it is
// per-cluster state the customer's operator owns, already exists, and already
// has its modes enforced.
const ProfileFileName = "instance-profile.json"

// ErrNoProfile is returned when no profile file exists.
//
// Distinguished from a malformed one because they are different operator
// problems: one is configuration not yet done, the other is configuration done
// wrongly. Both refuse a create; only the second is worth shouting about at
// startup. Same split as ErrNoCredential.
var ErrNoProfile = fmt.Errorf("no instance profile is configured")

// profileFile is the on-disk shape. Separate from InstanceProfile so the JSON
// names are stated once, here, rather than as tags on a type that also travels
// through the resolver.
type profileFile struct {
Flavor string `json:"flavor"`
Image string `json:"image"`
Network string `json:"network"`
Project string `json:"project,omitempty"`
}

// ReadInstanceProfile loads the instance profile from dir.
//
// Absent is ErrNoProfile and not a failure: an agent with no profile is the
// ordinary state of every cluster that has not opted in, and it refuses creates
// cleanly rather than refusing to start.
//
// A group- or world-writable file is refused, and readability deliberately is
// not. This is the action level's rule rather than the credential's, and the
// asymmetry is the point: the profile holds no secret — flavour, image and
// network are ids of shared cloud resources, and an operator should be able to
// read what their own agent creates without root. But whoever can write it
// chooses the image, and choosing the image is choosing what code runs on the
// instance this agent will be asked to create. Write access is the escalation;
// read access reveals nothing.
//
// Unknown fields are rejected rather than ignored. The obvious typo here is
// "flavour" — the spelling this codebase's own prose uses — and ignoring it
// would leave the field empty, refusing every create with a message about a
// flavour the operator believes they configured.
func ReadInstanceProfile(dir string) (InstanceProfile, error) {
path := filepath.Join(dir, ProfileFileName)

info, err := os.Stat(path)
if os.IsNotExist(err) {
return InstanceProfile{}, ErrNoProfile
}
if err != nil {
return InstanceProfile{}, fmt.Errorf("toolplane: stat %s: %w", path, err)
}
if perm := info.Mode().Perm(); perm&0o022 != 0 {
return InstanceProfile{}, fmt.Errorf(
"toolplane: %s has mode %04o; it must not be group- or world-writable — anyone who can write it chooses the image this cluster boots",
path, perm)
}

b, err := os.ReadFile(path)
if err != nil {
return InstanceProfile{}, fmt.Errorf("toolplane: read %s: %w", path, err)
}

var f profileFile
dec := json.NewDecoder(bytes.NewReader(b))
dec.DisallowUnknownFields()
if err := dec.Decode(&f); err != nil {
return InstanceProfile{}, fmt.Errorf("toolplane: %s: %v", path, err)
}

for _, field := range []struct{ name, value string }{
{"flavor", f.Flavor},
{"image", f.Image},
{"network", f.Network},
} {
if field.value == "" {
return InstanceProfile{}, fmt.Errorf(
"toolplane: %s has no %s; a half-configured profile creates nothing", path, field.name)
}
}

return InstanceProfile{
Flavor: f.Flavor,
Image: f.Image,
Network: f.Network,
Project: f.Project,
}, nil
}
Loading
Loading