diff --git a/cmd/seid/cmd/check_through_root_test.go b/cmd/seid/cmd/check_through_root_test.go new file mode 100644 index 0000000000..c1f8b1427c --- /dev/null +++ b/cmd/seid/cmd/check_through_root_test.go @@ -0,0 +1,144 @@ +package cmd + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/sei-cosmos/client" + "github.com/sei-protocol/sei-chain/sei-cosmos/client/flags" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + tmcli "github.com/sei-protocol/sei-chain/sei-tendermint/libs/cli" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// runCheckThroughRoot runs `sei-config check --home ` the way an operator and a runbook run it. +// +// Through the real root command, because that is what makes the difference. A command executed on its own +// has no parent, so nothing the root does before it happens: no hook runs, no flag is marked changed by +// anything but the caller, and no file is generated. Every one of those is a thing this command has to be +// right about, and a test that builds the command directly cannot see any of them. +func runCheckThroughRoot(t *testing.T, home string, extraArgs ...string) (string, error) { + t.Helper() + + root, _ := NewRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SilenceUsage = true + root.SilenceErrors = true + + // The same wiring the binary uses. --home is registered by PrepareBaseCmd rather than by the root + // command itself, so a root built without it is not the command an operator runs. + srvCtx := server.NewDefaultContext() + ctx := context.WithValue(context.Background(), client.ClientContextKey, &client.Context{}) + ctx = context.WithValue(ctx, server.ServerContextKey, srvCtx) + root.PersistentFlags().String(flags.FlagLogLevel, "", "") + root.PersistentFlags().String(flags.FlagLogFormat, "", "") + executor := tmcli.PrepareBaseCmd(root, "", home) + + root.SetArgs(append([]string{"sei-config", "check", "--home", home}, extraArgs...)) + err := executor.ExecuteContext(ctx) + return out.String(), err +} + +// writeSeiToml puts a sei.toml in a home that holds nothing else. +func writeSeiToml(t *testing.T, body string) string { + t.Helper() + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(home, "config", "sei.toml"), []byte(body), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + return home +} + +// TestTheCheckPassesACorrectFileWhenRunAsAnOperatorRunsIt is the answer the command exists to give. +// +// The one flag every real invocation carries is --home, and it names no setting: the node is told where its +// files are, not what to put in them. It arrives in the resolution beside the file's own keys, and counted +// with them it is a key nothing declares, so the command failed on a correct file and named a key the file +// does not contain. A pre-flight that fails every time carries nothing, and it is the whole compensating +// control for a boot that may not refuse a file. +func TestTheCheckPassesACorrectFileWhenRunAsAnOperatorRunsIt(t *testing.T) { + configtest.Isolate(t) + home := writeSeiToml(t, "schema_version = 1\nnode_mode = \"validator\"\n\n[mempool]\nsize = 4321\n") + + out, err := runCheckThroughRoot(t, home) + if err != nil { + t.Errorf("a correct file was refused when the command was run with --home: %v\n%s", err, out) + } + for _, flag := range []string{"home", "trace", "chain-id"} { + if strings.Contains(out, flag+": sei.toml writes this") { + t.Errorf("the report names --%s as something sei.toml wrote. The file does not contain it, "+ + "and an operator told this on every run stops reading the one signal they have", flag) + } + } +} + +// TestTheCheckDoesNotGenerateTheFilesItIsAskedAbout holds the command to answering rather than acting. +// +// The root command's hook runs the configuration handler, which writes config.toml and app.toml when they +// are absent. A command that reports on one file would then create two others as a side effect, on a node +// an operator was only asking a question about. +func TestTheCheckDoesNotGenerateTheFilesItIsAskedAbout(t *testing.T) { + configtest.Isolate(t) + home := writeSeiToml(t, "schema_version = 1\nnode_mode = \"validator\"\n\n[mempool]\nsize = 4321\n") + + // The verdict is not what this measures, and asserting it first would stop the measurement whenever + // something else about the check is wrong. + if out, err := runCheckThroughRoot(t, home); err != nil { + t.Logf("the check reported a problem, which is not what this test is about: %v\n%s", err, out) + } + for _, name := range []string{"config.toml", "app.toml"} { + if _, err := os.Stat(filepath.Join(home, "config", name)); err == nil { + t.Errorf("%s was created by a command that answers a question about a different file", name) + } + } +} + +// TestTheCheckStillFailsAFileWithSomethingWrongInIt keeps the fixes above from making it answer nothing. +// +// Passing a correct file and generating no files are both satisfied by a command that does nothing at all, +// so the failure has to be shown to still happen through the same path. +func TestTheCheckStillFailsAFileWithSomethingWrongInIt(t *testing.T) { + configtest.Isolate(t) + for _, tc := range []struct { + name string + body string + says string + }{ + { + name: "a key no section declares", + body: "schema_version = 1\nnode_mode = \"validator\"\n\n[mempool]\nsizze = 4321\n", + says: "no section declares it", + }, + { + name: "a length of time as a plain number", + body: "schema_version = 1\nnode_mode = \"validator\"\n\n[mempool]\nttl-duration = 60\n", + says: "nanoseconds", + }, + { + name: "a number larger than the setting holds", + body: "schema_version = 1\nnode_mode = \"validator\"\n\n[p2p]\nmax-connections = 1e20\n", + says: "larger than this setting can hold", + }, + } { + t.Run(tc.name, func(t *testing.T) { + out, err := runCheckThroughRoot(t, writeSeiToml(t, tc.body)) + if err == nil { + t.Fatalf("the file was accepted, so a boot would apply what it could and the operator "+ + "would find out afterwards\n%s", out) + } + if !strings.Contains(out, tc.says) { + t.Errorf("the report does not say %q, so it does not tell an operator what to change:\n%s", + tc.says, out) + } + }) + } +} diff --git a/cmd/seid/cmd/configmanager/check.go b/cmd/seid/cmd/configmanager/check.go new file mode 100644 index 0000000000..9f201da06f --- /dev/null +++ b/cmd/seid/cmd/configmanager/check.go @@ -0,0 +1,205 @@ +package configmanager + +import ( + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/sei-protocol/sei-chain/config/registry" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// CheckCmd answers, without starting a node, whether this binary can use a sei.toml. +// +// A boot may not refuse a file. A node that stopped because one line was mistyped is worse than a node +// running the value it ran yesterday, so every failure at boot is a report and the node keeps going. That +// makes the report the only signal, and a fleet rolling a configuration change forward reads it after the +// change is already on every node. +// +// The same questions have exact answers before then. The file, the binary and the environment are all the +// input, so the same file against the same binary gives the same answer here as it will at boot, for the +// same environment. That last part is a real condition and not a formality: this reads the environment of +// whoever runs it, and a node started by an init system or a container runtime has a different one. A +// variable that answers a declared key is a variable this cannot see unless it is set here too. +// +// What it does not rehearse is the install into the source a node builds, because that source does not +// exist until a boot builds it. A key can be refused there for a reason nothing here can see, and the whole +// install is dropped when it is. That is worth adding when the surface it covers is more than a handful of +// keys on a live node. +// +// This asks what it can answer where an answer costs a failed check rather than a restart. +func CheckCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "check", + Short: "Report whether this binary can use the node's sei.toml", + Long: "Resolves the node's sei.toml the way a boot resolves it and reports every value this " + + "binary would refuse, without starting anything. Exits non-zero if there is one.\n\n" + + "A boot cannot refuse a file, so it applies what it can and reports the rest. Running this " + + "first is how a mistyped value costs a failed check rather than a restart.", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + problems, found, err := checkSeiToml(cmd) + if err != nil { + return err + } + if !found { + report(cmd.OutOrStdout(), "this node has no sei.toml, so every key reads as it always "+ + "has and there is nothing here to be wrong") + return nil + } + out := cmd.OutOrStdout() + reportWhetherABootWouldReadThisFile(out, os.Getenv) + for _, line := range problems { + report(out, line) + } + if len(problems) > 0 { + return fmt.Errorf("%d problem(s); a boot would apply what it could and report the rest", + len(problems)) + } + report(out, "every value this file supplies is one this binary can use") + return nil + }, + } + return cmd +} + +// reportWhetherABootWouldReadThisFile says whether this node is set up to use the file at all. +// +// Without it, a passing check reads as "this file is in use and correct" on a node where a boot ignores it +// completely, which is the state every node is in until an operator switches the gate. That is the wrong +// conclusion in the more dangerous direction: it invites somebody to trust a file nothing reads. +// +// Answered from the environment this command runs in, which is the same limitation the resolution has. +func reportWhetherABootWouldReadThisFile(out io.Writer, getenv func(string) string) { + if _, err := Select(getenv); err != nil { + report(out, fmt.Sprintf("%s is set to something this binary does not accept, so a boot would "+ + "refuse before reaching this file: %v", EnvVar, err)) + return + } + if getenv(EnvVar) != "v2" { + report(out, fmt.Sprintf("%s is not set to v2 for this command, so a boot in the same environment "+ + "reads none of this file. What follows is what it would reach if it were", EnvVar)) + } +} + +// report writes one line of the answer. +// +// A failed write is dropped rather than returned. Where this runs the answer is the exit status, and a +// caller that cannot read the report still gets that. +func report(out io.Writer, line string) { _, _ = fmt.Fprintln(out, line) } + +// checkSeiToml resolves the node's file and returns what a boot would refuse, in the order it would. +// +// The absence of a file is not a problem to report: a node without one reads exactly as it always has, so +// there is nothing here that could be wrong. A file that exists and will not read is the opposite, and is +// reported as a problem of a file that was found. +func checkSeiToml(cmd *cobra.Command) (problems []string, found bool, err error) { + home, err := resolveHomeDir(cmd) + if err != nil { + return nil, false, fmt.Errorf("resolve the home directory: %w", err) + } + file, err := readSeiTomlAt(home) + switch { + case errors.Is(err, fs.ErrNotExist): + // The absence of a file is the one case with nothing to report. + return nil, false, nil + case err != nil: + // A file that exists and will not read is the case this command exists for. Reported as a problem + // of a file that was found, so the command exits non-zero: an operator running this before a + // restart is asking whether their file is right, and answering that they have no file is both + // wrong and the answer least likely to make them look. + return []string{fmt.Sprintf("sei.toml cannot be read: %v", err)}, true, nil + } + mode, err := file.Mode() + if err != nil { + return []string{fmt.Sprintf("sei.toml records no usable node mode: %v", err)}, true, nil + } + written, err := file.Values() + if err != nil { + return []string{fmt.Sprintf("sei.toml cannot be read: %v", err)}, true, nil + } + + resolved, err := registry.Resolve(registry.Mode(mode), registry.Sources{ + File: written, + LookupEnv: os.LookupEnv, + Flags: flagValues(TypedFlags(cmd)), + }) + if err != nil { + return []string{fmt.Sprintf("this node's configuration cannot be resolved: %v", err)}, true, nil + } + + // Only the file's own keys. A flag matching no declared key arrives in the same resolution and is not + // a mistake: every command carries flags that name no setting, so reporting those would fail this + // check on every invocation that types one, including a correct file. + for _, key := range resolved.UnknownInFile { + problems = append(problems, fmt.Sprintf("%s: sei.toml writes this and no section declares it, "+ + "so it has no effect", key)) + } + if running := theModeTheNodesOwnFileRecords(home); modesDisagree(mode, running) { + problems = append(problems, fmt.Sprintf("sei.toml says this is a %s and the node's own "+ + "configuration file says %s. Every value resolved here is the answer for the first and the "+ + "node would run as the second", mode, running)) + } + problems = append(problems, whatADecodeWouldRefuse(resolved)...) + return problems, true, nil +} + +// theModeTheNodesOwnFileRecords reads what kind of node the node's own configuration file says this is. +// +// Read from the file here rather than taken from a running node, because nothing is running. An absent file +// or an absent key answers empty, which is not a disagreement: a node that was never initialised has +// nothing to disagree with. +func theModeTheNodesOwnFileRecords(home string) string { + v := viper.New() + v.SetConfigFile(filepath.Join(home, "config", "config.toml")) + if err := v.ReadInConfig(); err != nil { + return "" + } + return v.GetString("mode") +} + +// whatADecodeWouldRefuse rehearses each decoded section the way the boot's delivery does. +// +// Rehearsed against a fresh configuration rather than a running node's, because there is no node here. That +// is a weaker target than the delivery uses, and the difference is the point: a value this accepts may still +// be refused at boot if the field it lands on holds something this cannot see. It is why this reports what +// it can answer and the boot still reports what it finds. +func whatADecodeWouldRefuse(resolved registry.Resolved) []string { + bySection := registry.SuppliedByDecodedSection(resolved) + var problems []string + for _, name := range sortedKeys(bySection) { + values := bySection[name] + base := tmcfg.DefaultConfig() + + // Each message says what is wrong with the value it names, and there is more than one thing that + // can be. Stating one of them here would describe the others wrongly. + if bad := whatDecodesToSomethingElse(base, values); len(bad) > 0 { + problems = append(problems, fmt.Sprintf("[%s]: %s", name, strings.Join(bad, "; "))) + continue + } + + source := viper.New() + for key, value := range values { + source.Set(key, value) + } + candidate, err := copyNodeConfig(base) + if err != nil { + problems = append(problems, fmt.Sprintf("[%s]: cannot be rehearsed: %v", name, err)) + continue + } + if err := source.Unmarshal(candidate); err != nil { + problems = append(problems, fmt.Sprintf("[%s]: %v, so none of this section would apply "+ + "(keys: %s)", name, err, strings.Join(sortedKeys(values), ","))) + } + } + sort.Strings(problems) + return problems +} diff --git a/cmd/seid/cmd/configmanager/check_test.go b/cmd/seid/cmd/configmanager/check_test.go new file mode 100644 index 0000000000..1bd21bb656 --- /dev/null +++ b/cmd/seid/cmd/configmanager/check_test.go @@ -0,0 +1,319 @@ +package configmanager + +import ( + "bytes" + "context" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/sei-cosmos/client/flags" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + serverconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" + "github.com/sei-protocol/sei-chain/testutil/configtest" + "github.com/spf13/cobra" + "go.opentelemetry.io/otel/sdk/trace" +) + +// runCheck runs the command against a home holding the given sei.toml, and returns what it printed and +// whether it failed. +func runCheck(t *testing.T, body string) (string, error) { + t.Helper() + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + if body != "" { + if err := os.WriteFile(filepath.Join(home, "config", seiTomlName), []byte(body), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + } + + cmd := CheckCmd() + cmd.Flags().String(flags.FlagHome, home, "") + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + return out.String(), err +} + +// TestTheCheckFailsOnWhatABootWouldRefuse is the point of the command. +// +// A boot may not refuse a file, so every value it cannot use is a report on a node that has already +// restarted. The same questions have exact answers beforehand, and this is where an answer costs a failed +// check instead. +func TestTheCheckFailsOnWhatABootWouldRefuse(t *testing.T) { + const header = "schema_version = 1\nnode_mode = \"validator\"\n" + + t.Run("a file this binary can use passes", func(t *testing.T) { + out, err := runCheck(t, header+"\n[mempool]\nttl-duration = \"60s\"\nsize = 4321\n") + if err != nil { + t.Errorf("a usable file was refused: %v\n%s", err, out) + } + }) + + t.Run("no file at all is not a problem", func(t *testing.T) { + out, err := runCheck(t, "") + if err != nil { + t.Errorf("a node with no sei.toml was refused: %v", err) + } + if !strings.Contains(out, "no sei.toml") { + t.Errorf("the report does not say the file is absent, so a missing file reads as a clean "+ + "one:\n%s", out) + } + }) + + t.Run("a length of time written as a plain number fails", func(t *testing.T) { + out, err := runCheck(t, header+"\n[mempool]\nttl-duration = 60\n") + if err == nil { + t.Errorf("a plain number in a length of time passed:\n%s", out) + } + if !strings.Contains(out, "nanoseconds") { + t.Errorf("the report does not say what is wrong with it:\n%s", out) + } + }) + + t.Run("a value the decode refuses fails", func(t *testing.T) { + out, err := runCheck(t, header+"\n[instrumentation]\nmax-open-connections = \"not a number\"\n") + if err == nil { + t.Errorf("a value no decode accepts passed:\n%s", out) + } + }) + + t.Run("a key no section declares is reported", func(t *testing.T) { + out, err := runCheck(t, header+"\n[mempool]\nnot-a-key = 1\n") + if err == nil { + t.Errorf("a key nothing declares passed:\n%s", out) + } + if !strings.Contains(out, "no effect") { + t.Errorf("the report does not say the key has no effect:\n%s", out) + } + }) + + t.Run("a mode this binary does not know fails", func(t *testing.T) { + out, err := runCheck(t, "schema_version = 1\nnode_mode = \"sentry\"\n") + if err == nil { + t.Errorf("a mode nothing declares passed:\n%s", out) + } + }) +} + +// TestADisagreementAboutTheKindOfNodeIsFound covers a fact two files state under different names. +// +// sei.toml records the kind of node at its top and every value resolved through this manager is the answer +// for that kind. The node's own configuration file states it again in a key of its own, and that one is what +// the node runs as. Nothing here declares the second on purpose, so the two can be written to disagree, and +// a node that resolves a validator's values while running as a full node reads correctly in every report +// about it. +func TestADisagreementAboutTheKindOfNodeIsFound(t *testing.T) { + for _, tc := range []struct { + recorded, running string + disagree bool + why string + }{ + {"validator", "validator", false, "the same kind is not a disagreement"}, + {"validator", "full", true, "a validator that runs as a query-serving node serves queries"}, + {"full", "validator", true, "a node resolved for queries that runs as a validator holds a key"}, + {"seed", "full", true, "a seed exists to serve peers and would be serving queries"}, + {"archive", "full", false, "the kind that keeps every version has no name of its own in that " + + "file, so the command that writes it writes this one"}, + {"archive", "validator", true, "an archive that runs as a validator is a disagreement"}, + } { + if got := modesDisagree(tc.recorded, tc.running); got != tc.disagree { + t.Errorf("sei.toml %q against a node running %q reports disagree=%v, want %v: %s", + tc.recorded, tc.running, got, tc.disagree, tc.why) + } + } +} + +// TestApplyReportsADisagreementAboutTheKindOfNode drives the real Apply, so the wiring is what is asserted. +// +// The test beside this one holds the decision, which a comparison never reached would still pass. This one +// gives the two files different kinds of node and looks for the report, so removing the call fails here. +func TestApplyReportsADisagreementAboutTheKindOfNode(t *testing.T) { + configtest.Isolate(t) + root := writeMinimalHome(t, "mode = \"full\"\n", "") + if err := os.WriteFile(filepath.Join(root, "config", seiTomlName), + []byte("schema_version = 1\nnode_mode = \"validator\"\n"), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + + cmd := server.StartCmd(nil, "/foobar", []trace.TracerProviderOption{}) + if err := cmd.Flags().Set(flags.FlagHome, root); err != nil { + t.Fatalf("set --home: %v", err) + } + serverCtx := &server.Context{} + cmd.SetContext(context.WithValue(context.Background(), server.ServerContextKey, serverCtx)) + + capture := &capturingHandler{} + mgr := SeiConfigManager{logger: slog.New(capture)} + if err := mgr.Apply(cmd, serverconfig.DefaultConfigTemplate, serverconfig.DefaultConfig()); err != nil { + t.Fatalf("the fixture is meant to boot, so this is the fixture: %v", err) + } + + var found bool + for _, r := range capture.records { + if strings.Contains(r.Message, "one kind of node") { + found = true + } + } + if !found { + t.Error("sei.toml said validator, the node's own file said full, and nothing reported it. A " + + "node resolving a validator's values while running as a query-serving node reads correctly " + + "in every other report about it") + } +} + +// TestCheckReportsAFileItCannotRead holds the difference the command exists to tell an operator. +// +// Running this before a restart asks whether the file is right. A file that will not parse, or records a +// schema this binary does not know, or names no node kind, is the case where the answer matters most, and +// answering that the node has no file is both wrong and the answer least likely to make anyone look. +func TestCheckReportsAFileItCannotRead(t *testing.T) { + for _, tc := range []struct { + name string + body string + }{ + {"unparseable", "[evm\n"}, + {"unknown schema", "schema_version = 99\nnode_mode = \"validator\"\n"}, + {"no node mode", "schema_version = 1\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "config"), 0o750); err != nil { + t.Fatalf("make a home: %v", err) + } + if err := os.WriteFile(filepath.Join(home, "config", seiTomlName), + []byte(tc.body), 0o600); err != nil { + t.Fatalf("write the file: %v", err) + } + + cmd := &cobra.Command{} + cmd.Flags().String(flags.FlagHome, home, "") + problems, found, err := checkSeiToml(cmd) + if err != nil { + t.Fatalf("checkSeiToml: %v", err) + } + if !found { + t.Errorf("a %s sei.toml is reported as no file at all, so the command exits zero on it", + tc.name) + } + if len(problems) == 0 { + t.Errorf("a %s sei.toml produced no problem to report", tc.name) + } + }) + } +} + +// TestTheCheckSaysWhetherABootWouldReadTheFileAtAll covers the conclusion an operator would otherwise draw. +// +// Until the gate is switched, a boot reads none of this file. A check that answers only about the file's +// contents reads as "in use and correct" on every one of those nodes, which invites somebody to trust a +// file nothing reads. That is the wrong conclusion in the dangerous direction. +func TestTheCheckSaysWhetherABootWouldReadTheFileAtAll(t *testing.T) { + for _, tc := range []struct { + name string + value string + says bool + }{ + {"unset", "", true}, + {"legacy", "legacy", true}, + {"v2", "v2", false}, + } { + t.Run(tc.name, func(t *testing.T) { + var out bytes.Buffer + reportWhetherABootWouldReadThisFile(&out, func(string) string { return tc.value }) + said := strings.Contains(out.String(), "reads none of this file") + if said != tc.says { + t.Errorf("with %s=%q the command says a boot reads none of the file: %v, want %v.\n%s", + EnvVar, tc.value, said, tc.says, out.String()) + } + }) + } +} + +// TestTheCheckSaysWhenTheGateItselfIsWrong covers a value a boot refuses outright. +// +// The gate is matched exactly and never falls back, so a misspelling stops the node before it reaches any +// file. A check that reported only on the file would pass, and the node would not start. +func TestTheCheckSaysWhenTheGateItselfIsWrong(t *testing.T) { + var out bytes.Buffer + reportWhetherABootWouldReadThisFile(&out, func(string) string { return "V2" }) + if !strings.Contains(out.String(), "does not accept") { + t.Errorf("a gate value this binary refuses is not reported, so the check passes for a node that "+ + "would not start:\n%s", out.String()) + } +} + +// runCheckWithNodeFile runs the command against a home holding both files. +func runCheckWithNodeFile(t *testing.T, seiToml, configToml string) (string, error) { + t.Helper() + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(home, "config", seiTomlName), []byte(seiToml), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + if configToml != "" { + path := filepath.Join(home, "config", "config.toml") + if err := os.WriteFile(path, []byte(configToml), 0o600); err != nil { + t.Fatalf("write config.toml: %v", err) + } + } + + cmd := CheckCmd() + cmd.Flags().String(flags.FlagHome, home, "") + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + return out.String(), err +} + +// TestTheCheckFindsADisagreementAboutWhatKindOfNodeThisIs covers the question with the largest consequence. +// +// Two files record what kind of node this is, under different names, and nothing keeps them in step. A node +// whose sei.toml says validator while its own file says full resolves a validator's answers and runs as a +// full node, serving queries. Every report about it reads correctly, which is what makes it worth catching +// before a restart rather than after. +// +// The boot reports this at its loudest level. A pre-flight that does not ask is silent on the one thing an +// operator most needs to know before they restart. +func TestTheCheckFindsADisagreementAboutWhatKindOfNodeThisIs(t *testing.T) { + const seiToml = "schema_version = 1\nnode_mode = \"validator\"\n\n[mempool]\nsize = 4321\n" + + t.Run("a disagreement is reported", func(t *testing.T) { + out, err := runCheckWithNodeFile(t, seiToml, "mode = \"full\"\n") + if err == nil { + t.Errorf("sei.toml says validator and the node's own file says full, and the check passed:\n%s", + out) + } + if !strings.Contains(out, "node's own") { + t.Errorf("the report does not name the disagreement:\n%s", out) + } + }) + + t.Run("agreement is not a problem", func(t *testing.T) { + out, err := runCheckWithNodeFile(t, seiToml, "mode = \"validator\"\n") + if err != nil { + t.Errorf("both files say validator and the check failed: %v\n%s", err, out) + } + }) + + t.Run("a node with no configuration file of its own has nothing to disagree with", func(t *testing.T) { + // Every node is in this state before it is initialised, and answering that it disagrees with a file + // it does not have would fail the check on a correct sei.toml. + out, err := runCheckWithNodeFile(t, seiToml, "") + if err != nil { + t.Errorf("a node with no configuration file of its own failed the check: %v\n%s", err, out) + } + }) +} diff --git a/cmd/seid/cmd/configmanager/install.go b/cmd/seid/cmd/configmanager/install.go index bab0cc4054..2fea064be7 100644 --- a/cmd/seid/cmd/configmanager/install.go +++ b/cmd/seid/cmd/configmanager/install.go @@ -219,7 +219,7 @@ func sortedKeys[V any](m map[string]V) []string { // So it is compared and reported. Reported rather than corrected, because what kind of node this is gets // decided when it is provisioned, and a configuration manager is not the thing that should change it. func reportWhatTheFileSaysTheNodeIs(ctx *server.Context, mode string, log *slog.Logger) { - if ctx == nil || ctx.Config == nil || ctx.Config.Mode == "" { + if ctx == nil || ctx.Config == nil { return } running := ctx.Config.Mode @@ -234,11 +234,13 @@ func reportWhatTheFileSaysTheNodeIs(ctx *server.Context, mode string, log *slog. // modesDisagree reports whether the kind of node sei.toml records and the kind the node runs as are // different kinds. // -// One pairing is not a disagreement. The kind that keeps every version of history has no name of its own in -// the node's own configuration file, so the command that writes that file writes the query-serving name -// instead, and the difference between them lives in settings the node's own file does not carry. +// Two cases are not a disagreement. An empty running mode means the node's own file does not state one, so +// there is nothing to disagree with: a node that was never initialised, or one whose file predates the key. +// And the kind that keeps every version of history has no name of its own in the node's own configuration +// file, so the command that writes that file writes the query-serving name instead, and the difference +// between them lives in settings the node's own file does not carry. func modesDisagree(recorded, running string) bool { - if recorded == running { + if running == "" || recorded == running { return false } return recorded != string(registry.ModeArchive) || running != string(registry.ModeFull) diff --git a/cmd/seid/cmd/root.go b/cmd/seid/cmd/root.go index 4b161cf4de..0f04753da5 100644 --- a/cmd/seid/cmd/root.go +++ b/cmd/seid/cmd/root.go @@ -145,6 +145,7 @@ func initRootCmd( tmcli.NewCompletionCmd(rootCmd, true), debugCmd, config.Cmd(), + seiConfigCmd(), tools.ToolCmd(), SnapshotCmd(), LogLevelCmd(), @@ -477,3 +478,27 @@ supply_enabled = {{ .LightInvariance.SupplyEnabled }} return customAppTemplate, customAppConfig } + +// seiConfigCmd groups the commands that answer questions about a node's sei.toml. +// +// Its own group rather than a subcommand of the existing configuration command, which reads and writes the +// files this one is about rather than the file that replaces them. +func seiConfigCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "sei-config", + Short: "Inspect the node's sei.toml", + // A hook of its own, which stops the root one from running. Two things follow, and both are + // required rather than convenient. + // + // The root hook writes files. It runs the configuration handler, which generates config.toml and + // app.toml when they are absent, so a command that answers a question about a file would create + // two others as a side effect. + // + // It also copies configuration values into flags and marks them changed, which is exactly the + // state that makes a flag indistinguishable from a key an operator's app.toml holds. A command + // reading its flags after that cannot tell what was typed, and it reports on what was typed. + PersistentPreRunE: func(*cobra.Command, []string) error { return nil }, + } + cmd.AddCommand(configmanager.CheckCmd()) + return cmd +}