diff --git a/config/cosmosbase/agreement_test.go b/config/cosmosbase/agreement_test.go new file mode 100644 index 0000000000..a13982ecca --- /dev/null +++ b/config/cosmosbase/agreement_test.go @@ -0,0 +1,200 @@ +package cosmosbase + +import ( + "fmt" + "sort" + "testing" + + "github.com/spf13/viper" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" +) + +// whatANodeRunsToday is what each diverging key resolves to for a configuration carrying no keys. +// +// A declared value is what seid init writes for a kind of node. That is not what a node with nothing +// written resolves, and these are the keys where the two differ. Most are reads that take no account of +// whether the key was present, so an absent key casts to a zero and the default beside it is lost. +// +// Held as text because the two sides carry different Go types for the same key often enough that comparing +// values would be comparing shapes. What matters here is which keys disagree and what a node gets instead. +var whatANodeRunsToday = map[string]string{ + "api.address": "", + "api.max-open-connections": "0", + "api.rpc-max-body-bytes": "0", + "api.rpc-read-timeout": "0", + "api.swagger": "false", + "grpc.enable": "true", + "minimum-gas-prices": "", + "occ-enabled": "false", + "pruning": "default", + "pruning-keep-every": "", + "telemetry.enabled": "false", + "telemetry.prometheus-retention-time": "0", +} + +// whyItMatters says what a node gets today, for the keys where that is worth stating. +var whyItMatters = map[string]string{ + "pruning": "a command flag of this name carries the standard schedule below the file, so a node with " + + "nothing written prunes on that schedule where a generated file would have said keep everything", + "grpc.enable": "a command flag of this name defaults the interface on, so a validator with nothing " + + "written serves gRPC where a generated file would have written it off. This is the one interface " + + "toggle of the two that diverges; the REST one agrees", + "api.max-open-connections": "zero is unlimited, so the ceiling a generated file states is simply " + + "absent from a node that never wrote it, and the same holds for the body-size ceiling beside it", + "minimum-gas-prices": "an empty price refuses to start, so this key is one no running node can " + + "actually have unwritten", + "occ-enabled": "the transaction execution path, and no command flag carries it, so an absent key " + + "reads as off where a generated file says on", +} + +// readerValues is what a node resolves for a configuration carrying none of these keys. +// +// Driven through the reader rather than reasoned about, because the reader is the authority on what an +// absent key resolves to and its answer differs per key: some reads check that the key was present, most +// do not, and two are rescued by a clamp that does nothing for an absent value. +// +// The start command's flags are bound first, the way a booting node binds them, and that is what makes +// this the answer a node gets rather than the answer the reader gives in isolation. Seventeen of these keys +// are also command flags, so a flag's registration default is what an absent key reaches before the lookup +// comes back empty. Without the binding, a key like the gRPC toggle reads as its type's zero and the +// comparison would report agreement where a node disagrees. +// +// One key has to be supplied. The metric label set is the first thing the reader asks for and it refuses a +// configuration without it, so a reader handed nothing at all answers for no key at all. +func readerValues(t *testing.T) map[string]string { + t.Helper() + v := viper.New() + start := server.StartCmd(nil, t.TempDir(), nil) + if err := v.BindPFlags(start.Flags()); err != nil { + t.Fatalf("bind the start flags: %v", err) + } + v.Set(globalLabelsKey, []any{}) + cfg, err := srvconfig.GetConfig(v) + if err != nil { + t.Fatalf("the reader refused a configuration carrying only the label set: %v", err) + } + + return map[string]string{ + "minimum-gas-prices": fmt.Sprint(cfg.MinGasPrices), + "pruning": fmt.Sprint(cfg.Pruning), + "pruning-keep-recent": fmt.Sprint(cfg.PruningKeepRecent), + "pruning-keep-every": fmt.Sprint(cfg.PruningKeepEvery), + "pruning-interval": fmt.Sprint(cfg.PruningInterval), + "halt-height": fmt.Sprint(cfg.HaltHeight), + "halt-time": fmt.Sprint(cfg.HaltTime), + "freeze-height": fmt.Sprint(cfg.FreezeHeight), + "min-retain-blocks": fmt.Sprint(cfg.MinRetainBlocks), + "inter-block-cache": fmt.Sprint(cfg.InterBlockCache), + "compaction-interval": fmt.Sprint(cfg.CompactionInterval), + "concurrency-workers": fmt.Sprint(cfg.ConcurrencyWorkers), + "occ-enabled": fmt.Sprint(cfg.OccEnabled), + "api.enable": fmt.Sprint(cfg.API.Enable), + "api.swagger": fmt.Sprint(cfg.API.Swagger), + "api.address": fmt.Sprint(cfg.API.Address), + "api.enabled-unsafe-cors": fmt.Sprint(cfg.API.EnableUnsafeCORS), + "api.max-open-connections": fmt.Sprint(cfg.API.MaxOpenConnections), + "api.rpc-read-timeout": fmt.Sprint(cfg.API.RPCReadTimeout), + "api.rpc-write-timeout": fmt.Sprint(cfg.API.RPCWriteTimeout), + "api.rpc-max-body-bytes": fmt.Sprint(cfg.API.RPCMaxBodyBytes), + "grpc.enable": fmt.Sprint(cfg.GRPC.Enable), + "grpc.address": fmt.Sprint(cfg.GRPC.Address), + "grpc.max-recv-msg-size": fmt.Sprint(cfg.GRPC.MaxRecvMsgSize), + "grpc.max-open-connections": fmt.Sprint(cfg.GRPC.MaxOpenConnections), + "grpc.max-connection-idle": fmt.Sprint(cfg.GRPC.MaxConnectionIdle), + "grpc.max-connection-age": fmt.Sprint(cfg.GRPC.MaxConnectionAge), + "grpc.max-connection-age-grace": fmt.Sprint(cfg.GRPC.MaxConnectionAgeGrace), + "grpc.keepalive-time": fmt.Sprint(cfg.GRPC.KeepaliveTime), + "grpc.keepalive-timeout": fmt.Sprint(cfg.GRPC.KeepaliveTimeout), + "grpc.keepalive-min-time": fmt.Sprint(cfg.GRPC.KeepaliveMinTime), + "grpc.keepalive-permit-without-stream": fmt.Sprint(cfg.GRPC.KeepalivePermitWithoutStream), + "telemetry.service-name": fmt.Sprint(cfg.Telemetry.ServiceName), + "telemetry.enabled": fmt.Sprint(cfg.Telemetry.Enabled), + "telemetry.enable-hostname": fmt.Sprint(cfg.Telemetry.EnableHostname), + "telemetry.enable-hostname-label": fmt.Sprint(cfg.Telemetry.EnableHostnameLabel), + "telemetry.enable-service-label": fmt.Sprint(cfg.Telemetry.EnableServiceLabel), + "telemetry.prometheus-retention-time": fmt.Sprint(cfg.Telemetry.PrometheusRetentionTime), + "state-sync.snapshot-interval": fmt.Sprint(cfg.StateSync.SnapshotInterval), + "state-sync.snapshot-keep-recent": fmt.Sprint(cfg.StateSync.SnapshotKeepRecent), + "state-sync.snapshot-directory": fmt.Sprint(cfg.StateSync.SnapshotDirectory), + "index-events": fmt.Sprint(cfg.IndexEvents), + globalLabelsKey: fmt.Sprint(cfg.Telemetry.GlobalLabels), + } +} + +// TestTheDivergencesFromTheReaderAreTheRecordedOnes measures what a comment used to count. +// +// A declared value is what seid init writes for a kind of node, and for a good number of these keys that is +// not what a node with nothing written resolves. Which keys those are was carried in prose, in four +// paragraphs, and one of the counts was wrong. Prose cannot fail when it is wrong. +// +// So the set is measured. A key that starts diverging fails, and so does one that stops, which means +// guarding a read has to account for its row rather than quietly making a sentence stale. +// +// Run for the mode whose declared values match the reader's own mode-blind answer most closely, because +// the reader takes no mode and comparing every mode against it would report the mode rules as divergences. +// The mode-varying keys are held by name in the test beside this one. +func TestTheDivergencesFromTheReaderAreTheRecordedOnes(t *testing.T) { + reader := readerValues(t) + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + var measured []string + for key, got := range reader { + declared, declares := resolved.Values[key] + if !declares { + t.Errorf("%s is read by the upstream reader and no section here declares it", key) + continue + } + if fmt.Sprint(declared) == got { + if _, listed := whatANodeRunsToday[key]; listed { + t.Errorf("%s no longer diverges, both sides being %v. Take it off the record, so the "+ + "record stays the set of keys a generated file states differently from a node that "+ + "never wrote them", key, declared) + } + continue + } + measured = append(measured, key) + want, listed := whatANodeRunsToday[key] + switch { + case !listed: + t.Errorf("%s is declared as %v and a node with nothing written resolves %q, and nothing "+ + "records that. %s", key, declared, got, whyItMatters[key]) + case want != got: + t.Errorf("%s is recorded as resolving %q and resolves %q", key, want, got) + } + } + + sort.Strings(measured) + if len(measured) != len(whatANodeRunsToday) { + t.Errorf("measured %d divergences and %d are recorded: %v", + len(measured), len(whatANodeRunsToday), measured) + } +} + +// TestEveryKeyTheseSectionsDeclareIsOneTheReaderResolves holds the two lists against each other. +// +// The reader's side is written out above, which is a second statement of the same key set. It is the only +// statement available: this reader looks its keys up as inline strings rather than through constants, so +// there is nothing to compare a tag against. A key on one side only is either a setting an operator writes +// that no reader fills, or one the reader fills that no section here declares. +func TestEveryKeyTheseSectionsDeclareIsOneTheReaderResolves(t *testing.T) { + reader := readerValues(t) + for _, section := range []string{ + BaseSectionName, APISectionName, GRPCSectionName, TelemetrySectionName, StateSyncSectionName, + } { + registered, ok := registry.Lookup(section) + if !ok { + t.Fatalf("%s is not registered", section) + } + for _, key := range registered.Keys { + if _, filled := reader[key]; !filled { + t.Errorf("%s declares %s and no field above is paired with it", section, key) + } + } + } +} diff --git a/config/cosmosbase/cosmosbase.go b/config/cosmosbase/cosmosbase.go new file mode 100644 index 0000000000..cc38c75030 --- /dev/null +++ b/config/cosmosbase/cosmosbase.go @@ -0,0 +1,157 @@ +// Package cosmosbase registers the configuration sections whose keys belong to the Cosmos server. +// +// These five register here rather than beside the structs they describe, and the reason is an import edge. +// The mode rules their defaults answer through live in app/params, which imports the upstream server +// configuration, so that package cannot ask for them without a cycle. A vendored tree is not itself the +// obstacle: other sections do register inside one. +// +// A section belongs here only when its keys are upstream's and that edge is in the way. Everything else +// registers in the package that owns its struct, so the struct, the values and the keys stay together. +package cosmosbase + +import ( + "github.com/sei-protocol/sei-chain/app/params" + "github.com/sei-protocol/sei-chain/config/registry" + srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" +) + +// The names these sections have in the configuration key space. +// +// BaseSectionName names a section whose keys carry no prefix at all. The name is for lookups and reports +// and is not part of any key, because giving those settings a section would rename every one of them. +const ( + BaseSectionName = "base" + APISectionName = "api" + GRPCSectionName = "grpc" + TelemetrySectionName = "telemetry" + StateSyncSectionName = "state-sync" +) + +// globalLabelsKey is the metric label set, which is the one key here no environment variable can supply. +const globalLabelsKey = TelemetrySectionName + ".global-labels" + +// Registration puts the upstream server's configuration sections in the registry. +// +// Four of the five register the upstream struct directly, because their mapstructure tags already name the +// keys their reader resolves. +func init() { + registry.RegisterRootKeys(BaseSectionName, &srvconfig.BaseConfig{}, baseDefaults) + registry.RegisterSection(APISectionName, &srvconfig.APIConfig{}, apiDefaults) + registry.RegisterSection(GRPCSectionName, &srvconfig.GRPCConfig{}, grpcDefaults) + registry.RegisterSection(TelemetrySectionName, &telemetrySchema{}, telemetryDefaults) + registry.RegisterSection(StateSyncSectionName, &srvconfig.StateSyncConfig{}, stateSyncDefaults) + + registry.RefuseFromEnvironment(TelemetrySectionName, globalLabelsKey, + "the metric label set is a list of name and value rows, and its reader takes that exact shape "+ + "rather than casting what it finds, so no single environment string can supply it. Write it "+ + "in the configuration file instead") +} + +// forMode is the server configuration the seid init command writes for a node of this kind. +// +// The upstream defaults with the binary's own mode rules applied, which is the pipeline that command +// builds and renders through the template. So a declared value here is what that file would have held, and +// a caller writing a configuration file writes what that command would have written. +// +// Named by the command, because this binary generates a file two ways and they do not agree. A node +// starting without one gets a file from a second pipeline that applies no mode rules at all and carries +// overrides of its own, so it writes the standard pruning strategy where this writes keeping everything, +// a metric retention of sixty where this writes seven thousand two hundred, the REST interface on for a +// validator where this writes it off, and a pruning interval drawn at random each time it runs. This +// follows the command an operator runs to provision a node, not the file a node writes for itself. +// +// That is what a declared value states, and it is deliberately not what a node with nothing written +// resolves. Those differ for a good number of these keys, because most are read with no check that the key +// was present and several are bound to a command flag carrying its own default below the file. The set is +// measured rather than counted, in the agreement test beside this one. +// +// Three settings differ by mode today and each of them matters in a different direction. A node that +// serves queries needs the interfaces that serve them; a validator is meant to expose as little as it can; +// and how many blocks a node retains is a decision about its disk. +func forMode(mode registry.Mode) *srvconfig.Config { + out := srvconfig.DefaultConfig() + params.SetAppConfigByMode(out, params.NodeMode(mode)) + return out +} + +// baseDefaults is what the node-wide settings resolve to for a node of this kind. +// +// One of these keys answers per mode: how many blocks a node retains, which is a hundred thousand for a +// full node and everything for the rest. The other two mode-varying keys in this package are the interface +// toggles, which belong to the sections that own them. +// +// Every one of these keys is read with a casting getter and no check that the key was present, so an +// absent key casts to a zero and clobbers the default beside it. Which keys those are, and what a node +// resolves for each instead, belongs in a measurement rather than in a count here. +// +// A caller resolving for a running node has to supply that node's flag values, and only the ones an +// operator actually set. A flag nobody typed still reports a default, and this resolution ranks flags above +// the file, so passing defaults would put every one of them over an operator's own value. +func baseDefaults(mode registry.Mode) any { return forMode(mode).BaseConfig } + +// apiDefaults is what the REST interface settings resolve to for a node of this kind. +// +// On for a full node and an archive node, off for a validator and a seed. Serving queries is what the +// first two are for, and the second two are meant to expose as little as they can. +func apiDefaults(mode registry.Mode) any { return forMode(mode).API } + +// grpcDefaults is what the gRPC settings resolve to for a node of this kind. +// +// On for a full node and an archive node, off for a validator and a seed, which is the same rule the REST +// interface follows and for the same reason. The upstream default is on for every kind, so declaring that +// would state an open interface on the nodes meant to expose the least. +// +// Six of these eleven keys are read only when the key is present. Two more are durations read through a +// clamp that rescues a negative value and does nothing for an absent one, so those two are unguarded and +// their clobber leaves no trace. The durations are declared as durations and written into a file as text, +// which is the shape the reader parses back. +func grpcDefaults(mode registry.Mode) any { return forMode(mode).GRPC } + +// stateSyncDefaults is what the snapshot settings resolve to for a node of this kind. +// +// All three keys are read with a casting getter and no presence check, and the retention is the one that +// inverts: it is declared as keeping two snapshots and an absent key casts to zero, which the file format +// documents as keeping every snapshot. +func stateSyncDefaults(mode registry.Mode) any { return forMode(mode).StateSync } + +// telemetrySchema declares the keys the metric settings reader resolves. +// +// A schema rather than the upstream type, and the only one of these five that needs one. The difference is +// a single field's type. The upstream struct declares the label set as a list of string pairs, and the +// reader takes a list of untyped rows: it asserts that exact shape rather than casting what it finds, and +// the struct's own type does not satisfy it, including that type's empty value. Registering the upstream +// type would resolve a default the reader refuses, and it refuses by returning an error that is the first +// statement of the whole server configuration, so the node stops. Every node, not only one that wrote the +// key. +// +// Every other field matches the upstream type, so this is one field's shape and not the section's. +type telemetrySchema struct { + ServiceName string `mapstructure:"service-name"` + Enabled bool `mapstructure:"enabled"` + EnableHostname bool `mapstructure:"enable-hostname"` + EnableHostnameLabel bool `mapstructure:"enable-hostname-label"` + EnableServiceLabel bool `mapstructure:"enable-service-label"` + PrometheusRetentionTime int64 `mapstructure:"prometheus-retention-time"` + GlobalLabels []any `mapstructure:"global-labels"` +} + +// telemetryDefaults is what the metric settings resolve to for a node of this kind. +// +// Read out of the upstream defaults rather than written again here, so a changed default moves both at +// once and this states only which key carries which setting. +// +// The label set is empty, which is what the upstream default holds, so there is nothing to convert into +// the untyped rows the reader takes. A test holds that emptiness, because a default that gained rows would +// need converting and would otherwise reach the reader as the shape it refuses. +func telemetryDefaults(mode registry.Mode) any { + live := forMode(mode).Telemetry + return telemetrySchema{ + ServiceName: live.ServiceName, + Enabled: live.Enabled, + EnableHostname: live.EnableHostname, + EnableHostnameLabel: live.EnableHostnameLabel, + EnableServiceLabel: live.EnableServiceLabel, + PrometheusRetentionTime: live.PrometheusRetentionTime, + GlobalLabels: []any{}, + } +} diff --git a/config/cosmosbase/cosmosbase_test.go b/config/cosmosbase/cosmosbase_test.go new file mode 100644 index 0000000000..e038e15435 --- /dev/null +++ b/config/cosmosbase/cosmosbase_test.go @@ -0,0 +1,296 @@ +package cosmosbase + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/app/params" + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/baseapp" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" + "github.com/sei-protocol/sei-chain/sei-cosmos/telemetry" +) + +// requireDeclares holds one section's declared keys against the keys named for it. +func requireDeclares(t *testing.T, section string, reads []string) registry.Section { + t.Helper() + registered, ok := registry.Lookup(section) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", section) + } + want := append([]string(nil), reads...) + sort.Strings(want) + if !reflect.DeepEqual(registered.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", section, registered.Keys, want) + } + return registered +} + +// TestTheNodeWideKeysAreTheOnesTheirReaderResolves holds the root section against the server's constants. +// +// Fourteen keys and not one of them carries a segment in front. The reader looks these up by the constants +// below, so a prefix here would declare fourteen keys no operator writes and leave the real ones +// undeclared. +func TestTheNodeWideKeysAreTheOnesTheirReaderResolves(t *testing.T) { + section := requireDeclares(t, BaseSectionName, []string{ + server.FlagMinGasPrices, server.FlagPruning, server.FlagPruningKeepRecent, + server.FlagPruningKeepEvery, server.FlagPruningInterval, server.FlagHaltHeight, + server.FlagFreezeHeight, server.FlagHaltTime, server.FlagMinRetainBlocks, + server.FlagInterBlockCache, server.FlagIndexEvents, server.FlagCompactionInterval, + server.FlagConcurrencyWorkers, baseapp.FlagOccEnabled, + }) + if section.Prefix != "" { + t.Errorf("the section carries prefix %q, and one here renames every key it declares", section.Prefix) + } +} + +// TestTheSnapshotKeysAreTheOnesTheirReaderResolves holds the snapshot section against the server's +// constants. +func TestTheSnapshotKeysAreTheOnesTheirReaderResolves(t *testing.T) { + requireDeclares(t, StateSyncSectionName, []string{ + server.FlagStateSyncSnapshotInterval, + server.FlagStateSyncSnapshotKeepRecent, + server.FlagStateSyncSnapshotDir, + }) +} + +// TestTheRESTKeysAreTheOnesItsReaderResolves holds the REST section against the keys its reader looks up. +// +// Written out rather than taken from constants, because this reader has none: it looks each key up as a +// literal string where it reads it. That is the whole reason a comparison is worth making here. +func TestTheRESTKeysAreTheOnesItsReaderResolves(t *testing.T) { + requireDeclares(t, APISectionName, []string{ + "api.enable", "api.swagger", "api.enabled-unsafe-cors", "api.address", + "api.max-open-connections", "api.rpc-read-timeout", "api.rpc-write-timeout", + "api.rpc-max-body-bytes", + }) +} + +// TestTheGRPCKeysAreTheOnesItsReaderResolves holds the gRPC section against the keys its reader looks up. +// +// Written out for the same reason as the REST section: the reader has no constants for these. +func TestTheGRPCKeysAreTheOnesItsReaderResolves(t *testing.T) { + requireDeclares(t, GRPCSectionName, []string{ + "grpc.enable", "grpc.address", "grpc.max-recv-msg-size", "grpc.max-open-connections", + "grpc.max-connection-idle", "grpc.max-connection-age", "grpc.max-connection-age-grace", + "grpc.keepalive-time", "grpc.keepalive-timeout", "grpc.keepalive-min-time", + "grpc.keepalive-permit-without-stream", + }) +} + +// TestTheMetricKeysAreTheOnesItsReaderResolves holds the metric section against the keys its reader looks +// up, the label set among them. +func TestTheMetricKeysAreTheOnesItsReaderResolves(t *testing.T) { + requireDeclares(t, TelemetrySectionName, []string{ + "telemetry.service-name", "telemetry.enabled", "telemetry.enable-hostname", + "telemetry.enable-hostname-label", "telemetry.enable-service-label", + "telemetry.prometheus-retention-time", globalLabelsKey, + }) +} + +// TestTheMetricSchemaRestatesTheUpstreamTypeExactlyOnceOver is what a schema costs. +// +// The schema exists for one field's shape, so every other field has to be the upstream field: same name, +// same tag, same type. A field that drifted would declare a key under a spelling the reader does not look +// up, or resolve a value of a type it cannot take, and the section would go on registering cleanly either +// way. +func TestTheMetricSchemaRestatesTheUpstreamTypeExactlyOnceOver(t *testing.T) { + upstream := reflect.TypeOf(telemetry.Config{}) + schema := reflect.TypeOf(telemetrySchema{}) + if schema.NumField() != upstream.NumField() { + t.Fatalf("the schema has %d fields and the upstream type has %d; a field on one side only is "+ + "either a key nothing reads or a setting nothing declares", + schema.NumField(), upstream.NumField()) + } + + differing := 0 + for i := range schema.NumField() { + got, want := schema.Field(i), upstream.Field(i) + if got.Name != want.Name { + t.Errorf("field %d is %s here and %s upstream", i, got.Name, want.Name) + continue + } + if got.Tag != want.Tag { + t.Errorf("%s is tagged %q here and %q upstream, so it declares a key the reader does not "+ + "look up", got.Name, got.Tag, want.Tag) + } + if got.Type == want.Type { + continue + } + differing++ + if got.Name != "GlobalLabels" { + t.Errorf("%s is %s here and %s upstream. The label set is the only field whose shape this "+ + "schema changes, so a second one is a divergence nothing decided", + got.Name, got.Type, want.Type) + } + } + if differing != 1 { + t.Errorf("%d fields differ in type, want exactly one. If the upstream type came to match, this "+ + "schema is a restatement with nothing left to justify it", differing) + } +} + +// TestTheUpstreamDefaultCarriesNoLabels holds the assumption the declared label set is built on. +// +// The declared default is an empty list of rows, which is right only while the upstream default holds no +// labels. A default that gained a pair would need converting into the untyped rows the reader takes, and +// without that it reaches the reader as the shape it refuses. +func TestTheUpstreamDefaultCarriesNoLabels(t *testing.T) { + if got := srvconfig.DefaultConfig().Telemetry.GlobalLabels; len(got) != 0 { + t.Errorf("the upstream default carries %d label rows: %v. They need converting into untyped rows "+ + "here, because the reader asserts that shape rather than casting what it finds", len(got), got) + } +} + +// TestTheLabelSetIsRefusedFromTheEnvironment covers the one key no variable here can supply. +// +// Its reader asserts a list of untyped rows and an environment carries one string, so resolving the +// variable installs a value the reader refuses, and it refuses in the first statement of the whole server +// configuration. The node stops. Leaving the channel out means the file's value applies and the node runs. +func TestTheLabelSetIsRefusedFromTheEnvironment(t *testing.T) { + reason, refused := registry.EnvCannotDeliver()[globalLabelsKey] + if !refused { + t.Fatalf("%s is not refused from the environment, so a variable naming it resolves to a string "+ + "and installing that stops the node", globalLabelsKey) + } + if reason == "" { + t.Error("the refusal carries no reason, so an operator whose variable is ignored cannot be told why") + } + + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{ + LookupEnv: func(name string) (string, bool) { + if name == registry.EnvName(globalLabelsKey) { + return "chain_id=pacific-1", true + } + return "", false + }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got := resolved.Values[globalLabelsKey]; !reflect.DeepEqual(got, []any{}) { + t.Errorf("%s resolved to %#v (%T), want the declared default it was left to", + globalLabelsKey, got, got) + } + for _, key := range resolved.Overrides { + if key == globalLabelsKey { + t.Errorf("%s is reported as a value an operator supplied, and the variable did nothing", + globalLabelsKey) + } + } +} + +// TestEachKindOfNodeResolvesTheInterfacesItIsFor is the mode-varying part of these sections. +// +// Three settings differ by kind of node, and the values are written out here rather than taken from the +// same rules the sections read, so a change to those rules fails this and gets looked at. Each matters in a +// different direction. A node that serves queries needs the two interfaces that serve them, and declaring +// them closed would take a service away from one. A validator is meant to expose as little as it can, and +// declaring gRPC open would state the opposite of that on every validator. And how many blocks a node +// keeps is a decision about its disk. +func TestEachKindOfNodeResolvesTheInterfacesItIsFor(t *testing.T) { + byMode := map[registry.Mode]struct { + api, grpc bool + retain uint64 + }{ + registry.ModeValidator: {api: false, grpc: false, retain: 0}, + registry.ModeSeed: {api: false, grpc: false, retain: 0}, + registry.ModeFull: {api: true, grpc: true, retain: 100000}, + registry.ModeArchive: {api: true, grpc: true, retain: 0}, + } + for _, mode := range registry.Modes() { + want, named := byMode[mode] + if !named { + t.Fatalf("mode %q has no expectation here, so a mode was added and this was not revisited", mode) + } + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for key, expected := range map[string]any{ + "api.enable": want.api, + "grpc.enable": want.grpc, + "min-retain-blocks": want.retain, + } { + if got := resolved.Values[key]; !reflect.DeepEqual(got, expected) { + t.Errorf("mode %q: %s resolves to %#v, want %#v", mode, key, got, expected) + } + } + } +} + +// TestDefaultsAreTheUpstreamOnesApartFromTheModeRules covers everything a mode does not change. +// +// Compared against the upstream defaults with the same mode rules applied, so this holds the sections to +// carrying the whole of that configuration rather than a subset of it, and the three settings the rules +// touch are pinned by name above. +func TestDefaultsAreTheUpstreamOnesApartFromTheModeRules(t *testing.T) { + for _, mode := range registry.Modes() { + live := srvconfig.DefaultConfig() + params.SetAppConfigByMode(live, params.NodeMode(mode)) + for _, c := range []struct { + section string + got any + want any + }{ + {BaseSectionName, baseDefaults(mode), live.BaseConfig}, + {APISectionName, apiDefaults(mode), live.API}, + {GRPCSectionName, grpcDefaults(mode), live.GRPC}, + {StateSyncSectionName, stateSyncDefaults(mode), live.StateSync}, + } { + if !reflect.DeepEqual(c.got, c.want) { + t.Errorf("mode %q: %s resolves to something other than that mode's upstream configuration", + mode, c.section) + } + } + + if _, ok := telemetryDefaults(mode).(telemetrySchema); !ok { + t.Fatalf("mode %q: the metric defaults returned %T, want the schema", mode, telemetryDefaults(mode)) + } + // Every field the schema copies by hand, held against the upstream value, and held as the + // resolved key rather than as a struct field. The section that has to restate its values is the + // one where a field can be assigned from the wrong neighbour, and a struct comparison would not + // see it: each field still holds a value, and the count still matches. + requireResolvesTelemetry(t, mode, live.Telemetry) + } +} + +// requireResolvesTelemetry holds every key the metric schema declares against the upstream value. +func requireResolvesTelemetry(t *testing.T, mode registry.Mode, live telemetry.Config) { + t.Helper() + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for key, want := range map[string]any{ + "telemetry.service-name": live.ServiceName, + "telemetry.enabled": live.Enabled, + "telemetry.enable-hostname": live.EnableHostname, + "telemetry.enable-hostname-label": live.EnableHostnameLabel, + "telemetry.enable-service-label": live.EnableServiceLabel, + "telemetry.prometheus-retention-time": live.PrometheusRetentionTime, + globalLabelsKey: []any{}, + } { + if got := resolved.Values[key]; !reflect.DeepEqual(got, want) { + t.Errorf("mode %q: %s resolves to %#v (%T), want %#v (%T)", mode, key, got, got, want, want) + } + } +} + +// TestTheSectionsThisPackageRegistersAreUsable covers what the registry refuses. +// +// Scoped to the five names this file registers. A refusal that depends on what else has registered is +// not this package's to answer for, and the sweep that covers it belongs where every section is linked. +func TestTheSectionsThisPackageRegistersAreUsable(t *testing.T) { + mine := map[string]bool{ + BaseSectionName: true, APISectionName: true, GRPCSectionName: true, + TelemetrySectionName: true, StateSyncSectionName: true, + } + for _, defect := range registry.Defects() { + if mine[defect.Section] { + t.Errorf("%s was refused, so none of its keys is declared: %v", defect.Section, defect.Err) + } + } +} diff --git a/config/registry/doc.go b/config/registry/doc.go index d1955c08f3..bc1659ecec 100644 --- a/config/registry/doc.go +++ b/config/registry/doc.go @@ -36,7 +36,13 @@ // serve. // // The third argument answers per node mode, because a validator and a seed node do not default -// alike. +// alike. A mode this package does not declare is refused rather than answered for: what a section +// does with an argument it cannot match is not a decision anybody made. +// +// Some settings are node-wide and are written at the top of a file rather than inside a table. +// RegisterRootKeys declares those: the name it takes is what a lookup and a report are keyed by and is +// not part of any key, so the keys are the tags alone. Giving such a section a segment would rename +// every key it declares, and a renamed key is one an operator's existing file no longer reaches. // // # Defaults // @@ -56,6 +62,14 @@ // value and a default are otherwise indistinguishable once merged. The second is why a typo in an // operator's file is visible rather than silently dropped. // +// One channel has a per-key hole. A reader that takes its value's exact type cannot be handed the one +// string an environment carries, so a section may refuse that channel for such a key, and the file's +// value applies instead of a value that would stop the node. The variable is still read and its value +// still discarded, and the key is reported as ignored, because a channel that quietly does nothing is +// the failure this package exists to remove. A refusal carries the reason an operator is owed, and one +// naming a key no section declares is refused in turn: it would cover nothing while reading as though +// it covered something. +// // Resolve either answers for every declared key or returns an error naming what it could not answer // for. A caller is never handed a resolution with a hole in it. // @@ -71,6 +85,15 @@ // tag, an unexported field carrying a tag, two fields declaring one path, a struct that declares no // key, a struct that contains itself, and two keys that collapse onto one environment variable. // +// One more becomes possible once a key can sit at the top of a file, and it could not happen while every +// key carried its section's name: two sections declaring one key, where one default renders over the +// other and which one depends on the order the sections are walked. The environment check refuses it, +// because two identical keys answer to one variable. +// +// Refusing the environment for a key is itself refused when it carries no reason. An operator told +// their variable does nothing has to be told why, and a refusal with nothing to print is worse than +// resolving the variable or leaving it alone. +// // A key segment is also refused if it is upper-case, or if it carries a dot or a space. That rule // holds for the section name and for a field's tag alike, since both become segments of the same // dotted key and answer to the same sources. @@ -82,10 +105,17 @@ // - Not a file format. Nothing here reads or writes a configuration file. // - Not a validator. A section may state rules about its own values; this package invents none. // - Not wired. No section is registered by this package and no reader is migrated onto it. +// - Not a guard against a key and a table sharing one name. A key at the top of the file that is also +// a section's name cannot be written at all, because no file holds both a value for that name and a +// table under it, so one of the two settings is unreachable and nothing says which. One section +// declares keys at the top of the file today and none of its names is a section's, so the collision +// has no instance; a second such section is where it becomes reachable. // // # Adding a Section // -// 1. Give the section a name, and use it as the first segment of every key it declares. +// 1. Give the section a name, and use it as the first segment of every key it declares. A section +// whose settings sit at the top of the file instead declares root keys, and its name is then a +// handle for lookups and reports rather than part of any key. // 2. Register the struct the reader already uses, with a per-mode default. // 3. Assert the registration produced no Defect. // 4. Hold the derived key names against the reader, so a key that reaches nothing fails. diff --git a/config/registry/environment.go b/config/registry/environment.go new file mode 100644 index 0000000000..d3f440b178 --- /dev/null +++ b/config/registry/environment.go @@ -0,0 +1,46 @@ +package registry + +import "fmt" + +// envCannotDeliver holds the keys an environment variable cannot supply, with the reason. +var envCannotDeliver = map[string]string{} + +// RefuseFromEnvironment records that an environment variable cannot supply a key. +// +// An environment carries one string per name. Most readers cast that string into whatever the setting +// needs, so the environment works for them. A reader that takes its value's exact type instead cannot be +// handed a string at all, and no spelling of the variable would satisfy it. +// +// Resolving such a key from the environment puts an unusable value at the top of the order, and installing +// it stops the node. Leaving the channel out means the file's value applies and the node runs. That is +// deliberately not what the machinery this replaces does, which resolves the variable and refuses to +// start, so the difference is recorded rather than assumed. A value silently doing nothing is the failure +// this whole surface exists to remove, which is why the reason is required and not optional. +// +// section is the section that declares the key, so a refused key is attributable to a registration the +// way every other defect is. Whether the key is one that section declares is answered when something +// resolves, because a refusal may be recorded before the registration it belongs to. +// +// Called from the owning package, beside its registration, so the reason sits with the code that knows it. +func RefuseFromEnvironment(section, key, reason string) { + mu.Lock() + defer mu.Unlock() + if reason == "" { + defects = append(defects, Defect{Section: section, Err: fmt.Errorf( + "refusing %q from the environment with no reason; an operator whose variable is ignored has "+ + "to be told why", key)}) + return + } + envCannotDeliver[key] = reason +} + +// EnvCannotDeliver returns the keys an environment variable cannot supply, and why. +func EnvCannotDeliver() map[string]string { + mu.RLock() + defer mu.RUnlock() + out := make(map[string]string, len(envCannotDeliver)) + for key, reason := range envCannotDeliver { + out[key] = reason + } + return out +} diff --git a/config/registry/registry.go b/config/registry/registry.go index 008738680c..06d472a15b 100644 --- a/config/registry/registry.go +++ b/config/registry/registry.go @@ -28,8 +28,16 @@ func Modes() []Mode { return []Mode{ModeValidator, ModeFull, ModeSeed, ModeArchi // Section is one registered configuration section. type Section struct { - // Name is the section's own segment, and the first segment of every key it declares. + // Name identifies the section. A lookup, a report and a defect are keyed by it, and for most + // sections it is also the first segment of every key. Name string + // Prefix is the first segment of every key this section declares, and is empty for a section whose + // keys sit at the root of the file with no section of their own. + // + // Separate from Name because the two do different jobs. A node-wide setting such as the pruning + // strategy is written at the top of app.toml and read as "pruning", so it has no segment to take a + // name from, and it still needs one to be looked up and reported under. + Prefix string // Keys are the dotted paths this section declares, sorted. Keys []string // Defaults returns the section's default for a mode. @@ -68,7 +76,23 @@ var ( // It never panics. A registration this package cannot use is recorded as a Defect and the // section is not registered. func RegisterSection(name string, prototype any, defaults func(Mode) any) { - keys, err := deriveKeys(name, prototype) + record(name, name, prototype, defaults) +} + +// RegisterRootKeys records a section whose keys sit at the root of the file, with no section of their own. +// +// name identifies the section for lookups and reports and is not part of any key. Everything else matches +// RegisterSection: the keys come from the mapstructure tags, and the tags are the only spelling. +// +// Some settings are node-wide and are written at the top of a file rather than inside a table. Giving them +// a section would rename them, and a renamed key is one an operator's existing file no longer reaches. +func RegisterRootKeys(name string, prototype any, defaults func(Mode) any) { + record(name, "", prototype, defaults) +} + +// record is the one path both registrations take. +func record(name, prefix string, prototype any, defaults func(Mode) any) { + keys, err := deriveKeys(name, prefix, prototype) mu.Lock() defer mu.Unlock() @@ -86,7 +110,7 @@ func RegisterSection(name string, prototype any, defaults func(Mode) any) { defects = append(defects, Defect{Section: name, Err: err}) return } - sections[name] = Section{Name: name, Keys: keys, Defaults: defaults} + sections[name] = Section{Name: name, Prefix: prefix, Keys: keys, Defaults: defaults} } } @@ -106,7 +130,17 @@ func envNamesAreDistinct(adding []string) error { } for _, key := range adding { env := EnvName(key) - if other, taken := spellings[env]; taken { + other, taken := spellings[env] + switch { + case taken && other == key: + // Two sections declaring one key, which a prefix made impossible and a key at the root of the + // file does not. One section's default renders over the other's and which one depends on the + // order the sections are walked, so the value a node runs is decided by nothing an operator + // or a reviewer can see. Named as the one key it is, because the spelling reason below is not + // the reason here. + return fmt.Errorf("%q is declared by two sections; one default renders over the other and "+ + "which one wins depends on the order the sections are walked", key) + case taken: return fmt.Errorf("%q and %q both answer to %s, because a dot and a hyphen are the same "+ "character to the environment, so one of them can never be set from it", other, key, env) } @@ -166,19 +200,19 @@ func Keys() []string { // outside state-commit.flatkv.*. Ninety-two operator-facing keys reach their field only through a // spelling the tags do not produce, and a silent fallback is what made that invisible. Refusing to // guess is what keeps the tag authoritative. -func deriveKeys(section string, prototype any) ([]string, error) { - if section == "" { +func deriveKeys(name, prefix string, prototype any) ([]string, error) { + if name == "" { return nil, fmt.Errorf("section name is empty") } - if section != strings.ToLower(section) { + if name != strings.ToLower(name) { return nil, fmt.Errorf("section name %q is not lower case; configuration sources "+ - "enumerate lower-cased, so a key under it would never match a written one", section) + "enumerate lower-cased, so a key under it would never match a written one", name) } - if bad, found := unaddressableChar(section); found { + if bad, found := unaddressableChar(name); found { return nil, fmt.Errorf("section name %q carries %q, and a section is one segment. A dotted name "+ "declares keys inside another section's subtree, where the two sections' defaults land in "+ "one map and whichever renders last silently wins; a space cannot be written in an "+ - "environment variable name at all", section, bad) + "environment variable name at all", name, bad) } if prototype == nil { return nil, fmt.Errorf("no struct") @@ -192,7 +226,7 @@ func deriveKeys(section string, prototype any) ([]string, error) { } var keys []string - if err := walk(t, section, &keys, map[reflect.Type]bool{}); err != nil { + if err := walk(t, prefix, &keys, map[reflect.Type]bool{}); err != nil { return nil, err } if len(keys) == 0 { @@ -254,15 +288,15 @@ func walk(t reflect.Type, prefix string, keys *[]string, open map[reflect.Type]b if ft.Kind() != reflect.Struct { return fmt.Errorf("%s.%s is squashed but is a %s, not a struct", prefix, f.Name, ft.Kind()) } - if err := walkSubtree(ft, prefix, prefix+"."+f.Name, keys, open); err != nil { + if err := walkSubtree(ft, prefix, join(prefix, f.Name), keys, open); err != nil { return err } continue } - path := prefix + "." + tag + path := join(prefix, tag) if ft.Kind() == reflect.Struct && !isLeaf(ft) { - if err := walkSubtree(ft, path, prefix+"."+f.Name, keys, open); err != nil { + if err := walkSubtree(ft, path, join(prefix, f.Name), keys, open); err != nil { return err } continue @@ -272,6 +306,14 @@ func walk(t reflect.Type, prefix string, keys *[]string, open map[reflect.Type]b return nil } +// join appends a key segment to a prefix, and returns the segment alone when there is no prefix. +func join(prefix, segment string) string { + if prefix == "" { + return segment + } + return prefix + "." + segment +} + // walkSubtree appends the keys a struct-typed field declares, and refuses one that declares none. // // A struct configuration cannot reach is a setting an operator writes into nothing. A defined type @@ -362,6 +404,7 @@ func Reset() { defer mu.Unlock() sections = map[string]Section{} defects = nil + envCannotDeliver = map[string]string{} } // envPrefix is the environment namespace for every derived key. diff --git a/config/registry/resolve.go b/config/registry/resolve.go index dfcca8f9e3..5ea1c2bbbe 100644 --- a/config/registry/resolve.go +++ b/config/registry/resolve.go @@ -16,6 +16,12 @@ type Resolved struct { // The keys an operator has taken responsibility for, as distinct from the ones tracking the // binary's judgement. This is what a diff renders. Overrides []string + // Ignored are declared keys an environment variable was set for and could not supply, sorted. + // + // Separate from Unknown because the two are different mistakes. An unknown key is one nothing reads. + // An ignored one is read, and the operator reached for the one channel that cannot carry it, so the + // value they wrote elsewhere is what applies. EnvCannotDeliver says why, per key. + Ignored []string // Unknown are keys a source carried that no section declares, sorted. // // Reported rather than an error, because what to do about one is the caller's decision: a @@ -37,6 +43,16 @@ type Sources struct { Flags map[string]any } +// known reports whether this package declares defaults for a mode. +func known(mode Mode) bool { + for _, m := range Modes() { + if m == mode { + return true + } + } + return false +} + // Resolve reduces a node's configuration sources to one value per declared key. // // The precedence is stated once, in this function, and a caller cannot reorder its way to a different @@ -54,6 +70,16 @@ type Sources struct { func Resolve(mode Mode, from Sources) (Resolved, error) { var out Resolved + // Refused before anything is resolved, because a section's defaults answer per mode and a mode this + // package does not know reaches whatever each section does with an argument it cannot match. What that + // is varies by section and none of them is a decision anyone made: the upstream mode rules answer for + // an unrecognised mode as though it were a full node, so an empty string, a capitalised name or one + // with a trailing space resolves the interfaces a full node serves onto whatever asked. + if !known(mode) { + return out, fmt.Errorf("%q is not a mode this binary declares defaults for; the modes are %v", + mode, Modes()) + } + // One snapshot, read once and passed everywhere below. Every part of the answer has to describe the // same registry: asking again leaves a window a concurrent registration fits through, and a section // arriving in that window is declared by one part of the answer and not by another. @@ -63,6 +89,17 @@ func Resolve(mode Mode, from Sources) (Resolved, error) { return out, err } declared := declaredKeys(registered) + undeliverable := EnvCannotDeliver() + // A refusal is recorded by a key, and a key that no section declares is one the environment layer + // would never have offered anyway, so the refusal protects nothing and reads as though it did. Held + // here because a refusal may be recorded before the section that declares its key registers, so this + // is the first point both sets exist. + for key := range undeliverable { + if !declared[key] { + return out, fmt.Errorf("%q is refused from the environment and no section declares it, so the "+ + "refusal covers nothing", key) + } + } out.Values = make(map[string]any, len(declared)) for key, v := range defaults { @@ -73,9 +110,11 @@ func Resolve(mode Mode, from Sources) (Resolved, error) { unknown := map[string]bool{} // Lowest precedence first, so a later source overwrites an earlier one. The one statement of the // order, which is why nothing exports it. + fromEnv, ignored := envValues(declared, undeliverable, from.LookupEnv) + out.Ignored = ignored for _, values := range []map[string]any{ fileValues(from.File), - envValues(declared, from.LookupEnv), + fromEnv, from.Flags, } { for key, v := range values { @@ -128,7 +167,7 @@ func declaredKeys(registered []Section) map[string]bool { func defaultValues(mode Mode, registered []Section) (map[string]any, error) { out := map[string]any{} for _, s := range registered { - values, err := sectionValues(s.Name, s.Defaults(mode)) + values, err := sectionValues(s.Prefix, s.Defaults(mode)) if err != nil { return out, fmt.Errorf("section %q default for mode %q: %w", s.Name, mode, err) } @@ -252,7 +291,7 @@ func walkValues(v reflect.Value, prefix string, out map[string]any) error { } continue } - path := prefix + "." + tag + path := join(prefix, tag) if fv.Kind() == reflect.Struct && !isLeaf(fv.Type()) { if err := walkValues(fv, path, out); err != nil { return err @@ -272,12 +311,27 @@ func walkValues(v reflect.Value, prefix string, out map[string]any) error { // declared is passed in rather than read here, so this shares Resolve's snapshot. Reading the registry // again would ask for a key the caller's declared set does not hold, and the answer would come back // only to be reported as one no section declares. -func envValues(declared map[string]bool, lookup func(string) (string, bool)) map[string]any { +func envValues(declared map[string]bool, undeliverable map[string]string, + lookup func(string) (string, bool)) (map[string]any, []string) { if lookup == nil { - return nil + return nil, nil } out := map[string]any{} + var ignored []string for key := range declared { + // A key no variable can carry is left to the sources that can. Resolving it would put a string + // at the top of the order for a reader that takes the exact type, and installing that stops the + // node. What an operator loses is the channel; what they keep is a node that boots. + // + // The variable is still read, and the value still discarded. Asking is what turns this from a + // silent skip into something a caller can report: a reason nothing can attach to an operator's + // own action is a reason nobody is ever told. + if _, refused := undeliverable[key]; refused { + if v, set := lookup(EnvName(key)); set && v != "" { + ignored = append(ignored, key) + } + continue + } // An empty value is treated as unset. A variable exported empty is far more often a shell // artefact than a deliberate empty string, and the two are indistinguishable here. The cost is // that clearing a key by exporting it empty reads as touching nothing, and Overrides will not @@ -286,7 +340,8 @@ func envValues(declared map[string]bool, lookup func(string) (string, bool)) map out[key] = v } } - return out + sort.Strings(ignored) + return out, ignored } // fileValues normalises a configuration file's keys to lower case. diff --git a/config/registry/rootkeys_test.go b/config/registry/rootkeys_test.go new file mode 100644 index 0000000000..d0a698d135 --- /dev/null +++ b/config/registry/rootkeys_test.go @@ -0,0 +1,275 @@ +package registry_test + +import ( + "reflect" + "sort" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// nodeWide is a probe for the settings written at the top of a file rather than inside a table. +type nodeWide struct { + Pruning string `mapstructure:"pruning"` + HaltHeight uint64 `mapstructure:"halt-height"` + Concurrency int `mapstructure:"concurrency-workers"` +} + +// TestARootSectionDeclaresKeysWithNoPrefix is the whole of what registering root keys adds. +// +// Some settings are node-wide and are written at the top of a file. Giving them a section would rename +// them, and a renamed key is one an operator's existing file no longer reaches. +func TestARootSectionDeclaresKeysWithNoPrefix(t *testing.T) { + registry.Reset() + registry.RegisterRootKeys("base", &nodeWide{}, func(registry.Mode) any { + return nodeWide{Pruning: "nothing", Concurrency: 4} + }) + for _, d := range registry.Defects() { + t.Fatalf("registering root keys was refused: %v", d.Err) + } + + section, ok := registry.Lookup("base") + if !ok { + t.Fatal("the section did not register under its name, so nothing can look it up or report on it") + } + if section.Prefix != "" { + t.Errorf("the section carries prefix %q, and one here renames every key it declares", section.Prefix) + } + if got := strings.Join(section.Keys, ","); got != "concurrency-workers,halt-height,pruning" { + t.Errorf("derived %q, want the three keys with no prefix. A leading segment is a key no operator "+ + "writes", got) + } + + // The default has to render under the same prefix-free names, or a declared key states no value and + // the resolution is refused rather than short. + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got := resolved.Values["pruning"]; got != "nothing" { + t.Errorf("pruning resolved to %#v, want %q", got, "nothing") + } +} + +// TestTwoSectionsCannotDeclareTheSameKey was impossible while every key carried its section's name. +// +// Two prefixes cannot collide. Two root sections can, and the default rendered for such a key would be +// whichever section the walk reached last. Refused by the environment check, which two identical keys +// reach by answering to one variable, and named as the one key it is rather than as two spellings. +func TestTwoSectionsCannotDeclareTheSameKey(t *testing.T) { + registry.Reset() + same := func(name string) { + registry.RegisterRootKeys(name, &struct { + Pruning string `mapstructure:"pruning"` + }{}, func(registry.Mode) any { + return struct { + Pruning string `mapstructure:"pruning"` + }{Pruning: "nothing"} + }) + } + same("base") + same("other") + + if _, ok := registry.Lookup("other"); ok { + t.Fatal("both sections declared the same key. One default renders over the other and which one " + + "wins depends on the order the sections are walked, so the value a node runs is not decided " + + "by anything an operator or a reviewer can see") + } + defects := registry.Defects() + if len(defects) != 1 { + t.Fatalf("recorded %d defects, want one", len(defects)) + } + // Named as one key two sections declare rather than as two spellings of one variable, which is the + // reason the same check gives for the collision it was written for. + if got := defects[0].Err.Error(); !strings.Contains(got, "is declared by two sections") { + t.Errorf("the refusal reads %q, and an identical key is not an environment spelling collision", got) + } +} + +// TestAKeyTheEnvironmentCannotDeliverIsLeftToTheOtherSources is what refusing a channel buys. +// +// The environment carries one string per name. A reader taking its value's exact type cannot be handed +// one, so resolving the variable installs a value that stops the node. Skipping it means the file's value +// applies and the node runs. +func TestAKeyTheEnvironmentCannotDeliverIsLeftToTheOtherSources(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{}, func(registry.Mode) any { + return struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{Rows: []any{}, Plain: "from the default"} + }) + registry.RefuseFromEnvironment("probe", "probe.rows", "its reader takes the exact type rather than casting") + for _, d := range registry.Defects() { + t.Fatalf("the registration was refused: %v", d.Err) + } + + // Both variables are set. Only the one the environment can carry is allowed to answer. + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{ + LookupEnv: func(name string) (string, bool) { + switch name { + case "SEID_PROBE_ROWS": + return "chain_id=pacific-1", true + case "SEID_PROBE_PLAIN": + return "from the environment", true + } + return "", false + }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + if got := resolved.Values["probe.rows"]; !reflect.DeepEqual(got, []any{}) { + t.Errorf("probe.rows resolved to %#v (%T), want the default it was left to. Its reader takes a "+ + "list of rows, so installing the environment's string stops the node", got, got) + } + if got := resolved.Values["probe.plain"]; got != "from the environment" { + t.Errorf("probe.plain resolved to %#v; refusing one key's channel closed another's", got) + } + sort.Strings(resolved.Overrides) + if got := strings.Join(resolved.Overrides, ","); got != "probe.plain" { + t.Errorf("overrides are %q, want only probe.plain. A key nothing supplied is not one an operator "+ + "has taken responsibility for", got) + } +} + +// TestRefusingAChannelWithoutAReasonIsItselfRefused keeps the exemption from being unexplainable. +// +// A key left out of the environment layer is one whose variable does nothing, and an operator told that +// has to be told why. A refusal with no reason gives a diagnostic nothing to print. +func TestRefusingAChannelWithoutAReasonIsItselfRefused(t *testing.T) { + registry.Reset() + registry.RefuseFromEnvironment("probe", "probe.rows", "") + if len(registry.Defects()) != 1 { + t.Fatalf("recorded %d defects, want one naming the key with no reason", len(registry.Defects())) + } + if _, refused := registry.EnvCannotDeliver()["probe.rows"]; refused { + t.Error("the key was refused from the environment anyway. Its variable would then be ignored " + + "with nothing able to say why, which is worse than either resolving it or not") + } + + registry.Reset() + registry.RefuseFromEnvironment("probe", "probe.rows", "its reader takes the exact type") + if _, refused := registry.EnvCannotDeliver()["probe.rows"]; !refused { + t.Error("a refusal carrying a reason was not recorded") + } +} + +// TestAModeThisBinaryDoesNotDeclareIsRefused closes a resolution that answered for anything. +// +// A section's defaults answer per mode, and a mode this package does not know reaches whatever each +// section does with an argument it cannot match. Nothing about that is a decision anyone made: the mode +// rules these sections read answer for an unrecognised mode as though it were a full node, so an empty +// string, a capitalised name or one with a trailing space resolved the interfaces a full node serves onto +// whichever node asked. +func TestAModeThisBinaryDoesNotDeclareIsRefused(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Serves bool `mapstructure:"serves"` + }{}, func(mode registry.Mode) any { + return struct { + Serves bool `mapstructure:"serves"` + }{Serves: mode == registry.ModeFull || mode == registry.ModeArchive} + }) + for _, d := range registry.Defects() { + t.Fatalf("the probe was refused: %v", d.Err) + } + + for _, mode := range registry.Modes() { + if _, err := registry.Resolve(mode, registry.Sources{}); err != nil { + t.Errorf("mode %q is declared and did not resolve: %v", mode, err) + } + } + for _, mode := range []registry.Mode{"", "Validator", "validator ", "VALIDATOR", "sentry"} { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err == nil { + t.Errorf("mode %q resolved, to serves=%v. A mode nothing declares has no answer, and the one "+ + "it reached is whatever the rules do with an argument they cannot match", + mode, resolved.Values["probe.serves"]) + } + } +} + +// TestARefusalNamingAKeyNothingDeclaresIsRefused keeps a refusal from covering nothing. +// +// A refusal is recorded by a key, so a slip in the spelling names a key no section declares. The +// environment layer would never have offered that key, so the refusal protects nothing while reading as +// though it did, and the key it was meant to cover resolves from the environment as before. +// +// Answered when something resolves rather than when the refusal is recorded, because a refusal may be +// recorded before the section declaring its key registers. Resolving is the first point both sets exist. +func TestARefusalNamingAKeyNothingDeclaresIsRefused(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Rows []any `mapstructure:"rows"` + }{}, func(registry.Mode) any { + return struct { + Rows []any `mapstructure:"rows"` + }{Rows: []any{}} + }) + registry.RefuseFromEnvironment("probe", "probe.rowz", "a slip in the spelling") + + if _, err := registry.Resolve(registry.ModeFull, registry.Sources{}); err == nil { + t.Error("a refusal naming a key nothing declares was accepted, so it covers nothing and the key " + + "it was written for still resolves from the environment") + } +} + +// TestAVariableSetForARefusedKeyIsReported is what makes the required reason worth requiring. +// +// The channel is skipped and the value discarded, which is the point. But an operator who set the variable +// believes otherwise, and a reason nothing can attach to their own action is a reason nobody is told. So +// the variable is still read, and the key comes back named. +func TestAVariableSetForARefusedKeyIsReported(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{}, func(registry.Mode) any { + return struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{Rows: []any{}, Plain: "from the default"} + }) + registry.RefuseFromEnvironment("probe", "probe.rows", "its reader takes the exact type") + for _, d := range registry.Defects() { + t.Fatalf("the probe was refused: %v", d.Err) + } + + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{ + LookupEnv: func(name string) (string, bool) { + if name == registry.EnvName("probe.rows") { + return "chain_id=pacific-1", true + } + return "", false + }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + if got := strings.Join(resolved.Ignored, ","); got != "probe.rows" { + t.Errorf("the ignored variables are %q, want probe.rows. An operator set it and nothing here can "+ + "tell them it did nothing", got) + } + if !reflect.DeepEqual(resolved.Values["probe.rows"], []any{}) { + t.Errorf("probe.rows resolved to %#v, and the channel was supposed to be skipped", + resolved.Values["probe.rows"]) + } + + // A refused key nobody set is not news, so it is not reported. + quiet, err := registry.Resolve(registry.ModeFull, registry.Sources{ + LookupEnv: func(string) (string, bool) { return "", false }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if len(quiet.Ignored) != 0 { + t.Errorf("a refused key nobody set is reported as ignored: %v", quiet.Ignored) + } +}