diff --git a/config/cosmosbase/cosmosbase.go b/config/cosmosbase/cosmosbase.go new file mode 100644 index 0000000000..7def33ed90 --- /dev/null +++ b/config/cosmosbase/cosmosbase.go @@ -0,0 +1,150 @@ +// 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 a node of this kind is meant to run. +// +// The upstream defaults with the binary's own mode rules applied. Every section here answers through this, +// so a section states what a kind of node is meant to run rather than what the type holds before any mode +// is considered, and a rule added to those rules later moves these sections with nothing here changing. +// +// 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. +// +// Several of these are not what a running node resolves today, and the causes differ: a bound command flag +// of the same name carries its own default below the file, and the command that assembles the server +// configuration overrides some of them before a node starts. The pruning strategy is the one worth naming, +// because the flag defaults it to the standard schedule while this declares it keeps everything. +// +// A caller resolving for a running node therefore 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 that has written nothing. +// +// 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 that has written nothing. +// +// 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 that has written nothing. +// +// 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 that has written nothing. +// +// 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/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..a9753d270d 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() @@ -82,14 +106,73 @@ func RegisterSection(name string, prototype any, defaults func(Mode) any) { defects = append(defects, Defect{Section: name, Err: fmt.Errorf("section registered twice")}) return } + if err := refuseOverlap(name, prefix, keys); err != nil { + defects = append(defects, Defect{Section: name, Err: err}) + return + } if err := envNamesAreDistinct(keys); err != nil { 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} } } +// refuseOverlap rejects a registration whose keys cannot coexist with what is already registered. +// Callers hold mu. +// +// Two shapes of overlap, and neither could happen while every key carried its section's name. A key two +// sections both declare has one default rendered over the other, and which one depends on the order the +// sections are walked. And a root key that is also a section's name cannot be written at all: a file +// holding both a value for that name and a table under it is not valid TOML, so one of the two is +// unreachable and nothing says which. +// +// The first shape reaches the environment check below as well, which would refuse it for the wrong +// reason: two spellings of one variable, when the keys are in fact the same key. This names it as itself. +func refuseOverlap(name, prefix string, keys []string) error { + declaredBy := map[string]string{} + sectionNamed := map[string]string{} + for _, s := range sections { + for _, key := range s.Keys { + declaredBy[key] = s.Name + } + if s.Prefix != "" { + sectionNamed[s.Prefix] = s.Name + } + } + + for _, key := range keys { + if owner, taken := declaredBy[key]; taken { + return fmt.Errorf("%s declares %q and so does %s; one default renders over the other and "+ + "which one wins depends on the order the sections are walked", name, key, owner) + } + if prefix != "" { + continue + } + if owner, taken := sectionNamed[key]; taken { + return fmt.Errorf("%s declares %q at the root of the file and %s is a section of that name; "+ + "a file cannot hold both a value for %q and a table under it, so one of them is "+ + "unreachable", name, key, owner, key) + } + } + + if prefix == "" { + return nil + } + for _, s := range sections { + if s.Prefix != "" { + continue + } + for _, key := range s.Keys { + if key == prefix { + return fmt.Errorf("%s is a section named %q and %s declares %q at the root of the file; "+ + "a file cannot hold both a table and a value under that name", name, prefix, s.Name, key) + } + } + } + return nil +} + // envNamesAreDistinct refuses keys that share one environment spelling. Callers hold mu. // // Dots and hyphens both become underscores, so two keys differing only in that punctuation answer to @@ -166,19 +249,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 +275,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 +337,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 +355,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 +453,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..a8c4083c87 --- /dev/null +++ b/config/registry/rootkeys_test.go @@ -0,0 +1,325 @@ +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") + } +} + +// TestARootKeyAndASectionCannotShareAName holds a limit of the file format, not a matter of taste. +// +// TOML cannot express a value for pruning and a table under pruning in one file, so one of the two is +// unwritable and which one an operator lost would depend on where in the file they wrote it. Registration +// order is not something an operator can see, so the refusal cannot depend on it either. +func TestARootKeyAndASectionCannotShareAName(t *testing.T) { + nested := func() (string, any, func(registry.Mode) any) { + return "pruning", &struct { + Mode string `mapstructure:"mode"` + }{}, func(registry.Mode) any { + return struct { + Mode string `mapstructure:"mode"` + }{Mode: "nothing"} + } + } + root := func() (string, any, func(registry.Mode) any) { + return "base", &struct { + Pruning string `mapstructure:"pruning"` + }{}, func(registry.Mode) any { + return struct { + Pruning string `mapstructure:"pruning"` + }{Pruning: "nothing"} + } + } + + t.Run("the section registers first", func(t *testing.T) { + registry.Reset() + registry.RegisterSection(nested()) + registry.RegisterRootKeys(root()) + if _, ok := registry.Lookup("base"); ok { + t.Error("the root section registered a key that is also a section name. A file cannot hold " + + "both, so one of them is unreachable and nothing says which") + } + if len(registry.Defects()) != 1 { + t.Errorf("recorded %d defects, want one naming the collision", len(registry.Defects())) + } + }) + + t.Run("the root key registers first", func(t *testing.T) { + registry.Reset() + registry.RegisterRootKeys(root()) + registry.RegisterSection(nested()) + if _, ok := registry.Lookup("pruning"); ok { + t.Error("a section registered under a name a root key already holds") + } + if len(registry.Defects()) != 1 { + t.Errorf("recorded %d defects, want one naming the collision", len(registry.Defects())) + } + }) +} + +// 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. +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 what + // the environment check would have called it. + if got := defects[0].Err.Error(); !strings.Contains(got, "and so does") { + 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) + } +}