fix: remove rds panics

This commit is contained in:
Julian Sommer
2026-08-19 14:58:35 +10:00
parent 941affcf3b
commit 60c9dfa89d
15 changed files with 237 additions and 93 deletions
+1 -3
View File
@@ -108,9 +108,7 @@ var engineLayouts = map[string]engineLayout{
dataMount: "/var/lib/postgresql",
port: 5432,
newProbe: newPostgresProbe,
newEngine: func(cfg config, run commandRunner, startSess sessionRunner, probe *engineProbe) (engine, error) {
return newPostgresEngine(cfg, run, startSess, probe), nil
},
newEngine: newPostgresEngineFromCatalog,
},
engineMariaDB: {
binDir: "/usr/bin",
+3 -2
View File
@@ -129,10 +129,11 @@ func newScriptedTestMariaDBEngine(t *testing.T, run commandRunner) *mariadbEngin
// that is registered: what a spec says about a parameter is what the guest must
// see, and an unlisted name must read as static rather than as dynamic.
func TestControlPlaneRulesFrom_TakesTheClassificationFromTheCatalog(t *testing.T) {
rules := controlPlaneRulesFrom(postgresEngineMeta)
meta := testPostgresEngineMeta(t)
rules := controlPlaneRulesFrom(meta)
for name, want := range map[string]bool{"shared_buffers": true, "work_mem": false} {
spec, ok := postgresEngineMeta.LookupParameter(name)
spec, ok := meta.LookupParameter(name)
if !ok {
t.Fatalf("the catalog no longer carries %s", name)
}
+11 -15
View File
@@ -70,19 +70,6 @@ func postgresProbeState(host string, run probeRunner) probeStateFn {
}
}
// Resolved once, under the name the control plane knows this implementation by.
// A lookup failure is a mismatch between the agent and the control plane it was
// built against, so it fails at startup rather than at the first rotation.
var postgresEngineMeta = mustLookupEngine(enginePostgres)
func mustLookupEngine(name string) handlers_rds.Engine {
engine, err := handlers_rds.LookupEngine(name)
if err != nil {
panic("rds-agent: " + err.Error())
}
return engine
}
// The include the resolved set is rendered to, and the copy of the last one the
// engine accepted. Both live beside the data rather than in /etc, matching
// rds-init: a class change boots a fresh root volume and would revert them.
@@ -114,9 +101,18 @@ hostnossl all all ::/0 reject
postgresForceSSLRuleCount = 2
)
func newPostgresEngine(cfg config, run commandRunner, startSess sessionRunner, probe *engineProbe) *postgresEngine {
// The layout's factory resolves the control plane metadata during startup.
func newPostgresEngineFromCatalog(cfg config, run commandRunner, startSess sessionRunner, probe *engineProbe) (engine, error) {
meta, err := handlers_rds.LookupEngine(enginePostgres)
if err != nil {
return nil, fmt.Errorf("this image bakes %s, which this build's control plane does not offer: %w", enginePostgres, err)
}
return newPostgresEngine(cfg, meta, run, startSess, probe), nil
}
func newPostgresEngine(cfg config, meta handlers_rds.Engine, run commandRunner, startSess sessionRunner, probe *engineProbe) *postgresEngine {
return &postgresEngine{
meta: postgresEngineMeta,
meta: meta,
run: run,
startSess: startSess,
psql: filepath.Join(cfg.EngineBinDir, "psql"),
+23 -13
View File
@@ -140,10 +140,19 @@ func (r *recordingRunner) run(_ context.Context, c command) (string, error) {
return r.out, r.err
}
func testPostgresEngineMeta(t *testing.T) handlers_rds.Engine {
t.Helper()
meta, err := handlers_rds.LookupEngine(enginePostgres)
if err != nil {
t.Fatalf("LookupEngine(%s): %v", enginePostgres, err)
}
return meta
}
func newTestEngine(t *testing.T, run commandRunner) *postgresEngine {
t.Helper()
cfg := testEngineConfig(t)
return newPostgresEngine(cfg, withPostgresReadBacks(cfg.EngineDataDir, run), nil, newPostgresProbe(cfg, staticProbe(0)))
return newPostgresEngine(cfg, testPostgresEngineMeta(t), withPostgresReadBacks(cfg.EngineDataDir, run), nil, newPostgresProbe(cfg, staticProbe(0)))
}
// The same engine with the read-backs left to the runner, for the cases that are
@@ -152,7 +161,7 @@ func newTestEngine(t *testing.T, run commandRunner) *postgresEngine {
func newScriptedTestEngine(t *testing.T, run commandRunner) *postgresEngine {
t.Helper()
cfg := testEngineConfig(t)
return newPostgresEngine(cfg, run, nil, newPostgresProbe(cfg, staticProbe(0)))
return newPostgresEngine(cfg, testPostgresEngineMeta(t), run, nil, newPostgresProbe(cfg, staticProbe(0)))
}
func testEngineConfig(t *testing.T) config {
@@ -479,10 +488,11 @@ func (r *tlsReadBackRunner) run(ctx context.Context, c command) (string, error)
return r.recordingRunner.run(ctx, c)
}
func forceSSLParameter(value string) []handlers_rds.Parameter {
func forceSSLParameter(t *testing.T, value string) []handlers_rds.Parameter {
t.Helper()
return []handlers_rds.Parameter{
{Name: "work_mem", Value: "4096"},
{Name: postgresEngineMeta.TLSEnforcementParameter(), Value: value},
{Name: testPostgresEngineMeta(t).TLSEnforcementParameter(), Value: value},
}
}
@@ -492,7 +502,7 @@ func TestPostgresEngine_ApplyParametersWritesTheEnforcementRule(t *testing.T) {
runner := &tlsReadBackRunner{ssl: "on", ruleRows: postgresForceSSLRuleCount}
engine := newScriptedTestEngine(t, runner.run)
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter("1")); err != nil {
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "1")); err != nil {
t.Fatalf("ApplyParameters: %v", err)
}
rule, err := os.ReadFile(engine.forceSSLRulePath())
@@ -516,7 +526,7 @@ func TestPostgresEngine_ApplyParametersRemovesAStaleEnforcementRule(t *testing.T
t.Fatalf("write the rule the restored volume carried: %v", err)
}
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter("0")); err != nil {
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "0")); err != nil {
t.Fatalf("ApplyParameters: %v", err)
}
if _, err := os.Stat(engine.forceSSLRulePath()); !os.IsNotExist(err) {
@@ -547,7 +557,7 @@ func TestPostgresEngine_ApplyParametersRefusesAnUnparsableEnforcementValue(t *te
runner := &tlsReadBackRunner{ssl: "on"}
engine := newScriptedTestEngine(t, runner.run)
_, err := engine.ApplyParameters(t.Context(), forceSSLParameter("yes"))
_, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "yes"))
if err == nil || !strings.Contains(err.Error(), "neither 1 nor 0") {
t.Fatalf("ApplyParameters error = %v, want a refusal naming the unreadable value", err)
}
@@ -562,7 +572,7 @@ func TestPostgresEngine_ApplyParametersRefusesToEnforceWithoutTLS(t *testing.T)
runner := &tlsReadBackRunner{ssl: "off"}
engine := newScriptedTestEngine(t, runner.run)
_, err := engine.ApplyParameters(t.Context(), forceSSLParameter("1"))
_, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "1"))
if err == nil || !strings.Contains(err.Error(), "serving without TLS") {
t.Fatalf("ApplyParameters error = %v, want a refusal naming the engine's own TLS state", err)
}
@@ -592,7 +602,7 @@ func TestPostgresEngine_ApplyParametersFailsAnUnverifiableEnforcement(t *testing
runner := &tlsReadBackRunner{ssl: "on", ruleRows: tc.ruleRows, brokenRows: tc.brokenRows}
engine := newScriptedTestEngine(t, runner.run)
_, err := engine.ApplyParameters(t.Context(), forceSSLParameter(tc.value))
_, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, tc.value))
if err == nil || !strings.Contains(err.Error(), tc.want) {
t.Fatalf("ApplyParameters error = %v, want %q", err, tc.want)
}
@@ -606,13 +616,13 @@ func TestPostgresEngine_ApplyParametersFailsAnUnverifiableEnforcement(t *testing
func TestPostgresEngine_ApplyParametersWithdrawsBothFilesTogether(t *testing.T) {
runner := &tlsReadBackRunner{ssl: "on", ruleRows: postgresForceSSLRuleCount}
engine := newScriptedTestEngine(t, runner.run)
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter("1")); err != nil {
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "1")); err != nil {
t.Fatalf("first ApplyParameters: %v", err)
}
// Turning enforcement off, verified against a pg_hba that still carries the
// rule — so the apply fails after both files have already been replaced.
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter("0")); err == nil {
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "0")); err == nil {
t.Fatal("ApplyParameters succeeded against an unverifiable enforcement")
}
installed, err := os.ReadFile(engine.params.installedPath())
@@ -639,14 +649,14 @@ func TestPostgresEngine_RestoreLastKnownGoodReDerivesEnforcement(t *testing.T) {
runner := &tlsReadBackRunner{ssl: "on", ruleRows: postgresForceSSLRuleCount}
engine := newScriptedTestEngine(t, runner.run)
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter("1")); err != nil {
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "1")); err != nil {
t.Fatalf("ApplyParameters(enforcing): %v", err)
}
if err := engine.RecordServingParameters(t.Context()); err != nil {
t.Fatalf("RecordServingParameters: %v", err)
}
runner.ruleRows = 0
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter("0")); err != nil {
if _, err := engine.ApplyParameters(t.Context(), forceSSLParameter(t, "0")); err != nil {
t.Fatalf("ApplyParameters(not enforcing): %v", err)
}
if _, err := os.Stat(engine.forceSSLRulePath()); !os.IsNotExist(err) {
+3
View File
@@ -1808,6 +1808,9 @@ func (d *Daemon) startCluster() error {
// whichever node the queue group picks, so a node without it would make the
// first boot of a DB instance fail intermittently rather than not at all.
d.rdsService, err = initServiceWithRetry("RDS service", func() (*handlers_rds.Service, error) {
if registryErr := handlers_rds.ValidateEngineRegistry(); registryErr != nil {
return nil, registryErr
}
deps, depsErr := d.buildRDSDeps()
if depsErr != nil {
return nil, depsErr
+10 -14
View File
@@ -31,12 +31,6 @@ const (
// would pay the whole storage cost above for a fraction of the coverage.
defaultBackupRetentionDays = 7
// The UTC blocks an unnamed window is assigned inside. Deliberately disjoint:
// the two derived windows then cannot overlap, which is the pair AWS refuses
// to accept and the pair that would quiesce an engine mid-maintenance.
defaultBackupWindowBlock = "03:00-11:00" //nolint:gosec // a UTC window block, not a credential
defaultMaintenanceWindowBlock = "11:00-19:00"
// How many automated snapshots one retention pass may delete. A pass that
// under-collects is corrected by the next one two minutes later; a pass that
// walks an unbounded number of snapshots is not.
@@ -60,6 +54,13 @@ const (
automatedBackupRetryShiftCap = 3
)
// The UTC blocks an unnamed window is assigned inside. Deliberately disjoint,
// so derived backup and maintenance windows cannot overlap.
var (
defaultBackupWindowBlock = dailyWindow{start: 3 * time.Hour, end: 11 * time.Hour}
defaultMaintenanceWindowBlock = dailyWindow{start: 11 * time.Hour, end: 19 * time.Hour}
)
// The operator-tunable backup settings: bounds and defaults, never a per-instance
// setting — those live on the DB instance record.
type BackupPolicy struct {
@@ -114,20 +115,15 @@ func (s *Service) maintenanceWindowBlock() dailyWindow {
return s.windowBlock(s.deps.Backup.MaintenanceWindowBlock, defaultMaintenanceWindowBlock, "maintenance")
}
func (s *Service) windowBlock(configured, fallback, kind string) dailyWindow {
func (s *Service) windowBlock(configured string, fallback dailyWindow, kind string) dailyWindow {
if configured != "" {
if block, err := parseDailyWindow("block", configured); err == nil {
return block
}
slog.Warn("rds: the configured "+kind+" window block is malformed; using the built-in block",
"block", configured, "fallback", fallback)
"block", configured, "fallback", fallback.String())
}
block, err := parseDailyWindow("block", fallback)
if err != nil {
// Unreachable: the fallbacks are constants this package's tests parse.
panic("rds: built-in " + kind + " window block is malformed: " + err.Error())
}
return block
return fallback
}
// The window in force for this instance. A record that names one uses it; one
+2 -7
View File
@@ -321,13 +321,8 @@ func TestWindowBlock_FallsBackToTheBuiltInBlock(t *testing.T) {
MaintenanceWindowBlock: "19:00",
}})
builtIn, err := parseDailyWindow("block", defaultBackupWindowBlock)
require.NoError(t, err)
assert.Equal(t, builtIn, svc.backupWindowBlock())
builtIn, err = parseDailyWindow("block", defaultMaintenanceWindowBlock)
require.NoError(t, err)
assert.Equal(t, builtIn, svc.maintenanceWindowBlock())
assert.Equal(t, defaultBackupWindowBlock, svc.backupWindowBlock())
assert.Equal(t, defaultMaintenanceWindowBlock, svc.maintenanceWindowBlock())
}
func TestValidateRetentionPeriod_BoundsTheRetention(t *testing.T) {
+20 -15
View File
@@ -63,40 +63,45 @@ var engines = map[string]Engine{
// The same registry keyed by parameter-group family, for the callers that hold
// only a family string and have no instance to derive an engine from. Family
// and engine are 1:1 by construction, so one registry serves both.
var enginesByFamily = indexEnginesByFamily()
var enginesByFamily, engineRegistryValidationErr = indexEnginesByFamily(engines)
func indexEnginesByFamily() map[string]Engine {
out := make(map[string]Engine, len(engines))
for _, engine := range engines {
// An engine registered without its own name rule would panic on the first
// create that names a database, rather than here where every build sees it.
// ValidateEngineRegistry reports invalid built-in engine metadata before RDS starts.
func ValidateEngineRegistry() error {
if err := validateParameterCatalogs(); err != nil {
return err
}
return engineRegistryValidationErr
}
func indexEnginesByFamily(registry map[string]Engine) (map[string]Engine, error) {
out := make(map[string]Engine, len(registry))
for _, engine := range registry {
if engine.validateDBName == nil {
panic("rds: engine " + engine.Name + " registers no DBName rule")
return nil, errors.New("rds: engine " + engine.Name + " registers no DBName rule")
}
// An engine without these would tell a customer nothing about what a
// crash-consistent snapshot or an unclean stop of it recovers, on the two
// events that say so.
// Without these, snapshot and unclean-stop events cannot explain what the
// engine will recover.
if engine.crashRecoveryNote == "" {
panic("rds: engine " + engine.Name + " registers no crash-recovery note")
return nil, errors.New("rds: engine " + engine.Name + " registers no crash-recovery note")
}
if engine.uncleanStopNote == "" {
panic("rds: engine " + engine.Name + " registers no unclean-stop note")
return nil, errors.New("rds: engine " + engine.Name + " registers no unclean-stop note")
}
// A name no catalog entry answers to would leave the guest deriving
// enforcement from a key nothing ever writes, which reads as not enforcing.
if name := engine.tlsEnforcementParameter; name != "" {
spec, ok := engine.catalog[name]
if !ok || spec.DataType != ParamTypeBoolean {
panic("rds: engine " + engine.Name + " names " + name + ", which is not a boolean parameter it exposes")
return nil, errors.New("rds: engine " + engine.Name + " names " + name + ", which is not a boolean parameter it exposes")
}
}
family := engine.ParameterGroupFamily()
if _, exists := out[family]; exists {
panic("rds: parameter group family " + family + " is claimed by two engines")
return nil, errors.New("rds: parameter group family " + family + " is claimed by two engines")
}
out[family] = engine
}
return out
return out, nil
}
// The pinned version an AMI lookup resolves against, which is the major alone:
+1 -1
View File
@@ -49,7 +49,7 @@ var engineMariaDB = Engine{
// Platform-owned settings are absent, except the ones AWS exposes as modifiable
// and this platform pins: those are present and unmodifiable, so a refusal reads
// as policy rather than as a missing feature.
var mariadbParameterCatalog = buildParameterCatalog(
var mariadbParameterCatalog, mariadbParameterCatalogErr = buildParameterCatalog(
// Connections and threads. max_connections is what a size-derived default
// matters most for: RDS's own formula is {DBInstanceClassMemory/12582880}.
ParameterSpec{
+1 -1
View File
@@ -43,7 +43,7 @@ var enginePostgres = Engine{
// Platform-owned settings are absent, except the ones AWS exposes as modifiable
// and this platform pins: those are present and unmodifiable, so a refusal reads
// as policy rather than as a missing feature.
var postgresParameterCatalog = buildParameterCatalog(
var postgresParameterCatalog, postgresParameterCatalogErr = buildParameterCatalog(
// Connections. max_connections is what a size-derived default matters most
// for: RDS's own formula is LEAST({DBInstanceClassMemory/9531392}, 5000).
ParameterSpec{
+68 -6
View File
@@ -34,18 +34,18 @@ var engineUnderTest = Engine{
reservedUsernamePrefixes: []string{"testdb_"},
maxUsernameLen: 80,
validateDBName: dbNameRule(64),
catalog: buildParameterCatalog(
ParameterSpec{
catalog: map[string]ParameterSpec{
"buffer_size": {
Name: "buffer_size", DataType: ParamTypeInteger, ApplyType: ApplyTypeStatic,
IsModifiable: true, Min: 1, Max: 1024, Default: "64", Unit: "MB",
Description: "Buffer the server allocates at startup, in MB.",
},
ParameterSpec{
"connection_limit": {
Name: "connection_limit", DataType: ParamTypeInteger, ApplyType: ApplyTypeDynamic,
IsModifiable: true, Min: 1, Max: 1000, Default: "100",
Description: "Maximum concurrent connections to the server.",
},
),
},
validateCombinations: func([]Parameter) error { return nil },
crashRecoveryNote: "It will recover when it is restored.",
uncleanStopNote: "It will recover on the next start.",
@@ -57,13 +57,75 @@ func registerTestEngine(t *testing.T) {
t.Helper()
require.NotContains(t, engines, engineUnderTest.Name)
engines[engineUnderTest.Name] = engineUnderTest
enginesByFamily = indexEnginesByFamily()
indexed, err := indexEnginesByFamily(engines)
require.NoError(t, err)
enginesByFamily = indexed
t.Cleanup(func() {
delete(engines, engineUnderTest.Name)
enginesByFamily = indexEnginesByFamily()
indexed, err := indexEnginesByFamily(engines)
require.NoError(t, err)
enginesByFamily = indexed
})
}
func TestValidateEngineRegistry(t *testing.T) {
require.NoError(t, ValidateEngineRegistry())
}
func TestIndexEnginesByFamily_RejectsInvalidMetadata(t *testing.T) {
noDBNameRule := engineUnderTest
noDBNameRule.validateDBName = nil
noCrashRecoveryNote := engineUnderTest
noCrashRecoveryNote.crashRecoveryNote = ""
noUncleanStopNote := engineUnderTest
noUncleanStopNote.uncleanStopNote = ""
invalidTLSParameter := engineUnderTest
invalidTLSParameter.tlsEnforcementParameter = "not-a-boolean-parameter"
tests := []struct {
name string
registry map[string]Engine
wantErr string
}{
{
name: "missing DB name rule",
registry: map[string]Engine{"test": noDBNameRule},
wantErr: "registers no DBName rule",
},
{
name: "missing crash recovery note",
registry: map[string]Engine{"test": noCrashRecoveryNote},
wantErr: "registers no crash-recovery note",
},
{
name: "missing unclean stop note",
registry: map[string]Engine{"test": noUncleanStopNote},
wantErr: "registers no unclean-stop note",
},
{
name: "invalid TLS enforcement parameter",
registry: map[string]Engine{"test": invalidTLSParameter},
wantErr: "is not a boolean parameter it exposes",
},
{
name: "duplicate parameter group family",
registry: map[string]Engine{
"first": engineUnderTest,
"second": engineUnderTest,
},
wantErr: "is claimed by two engines",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
indexed, err := indexEnginesByFamily(tt.registry)
require.ErrorContains(t, err, tt.wantErr)
assert.Nil(t, indexed)
})
}
}
func testEngineGroupInput(name string) *rds.CreateDBParameterGroupInput {
return &rds.CreateDBParameterGroupInput{
DBParameterGroupName: aws.String(name),
+9 -5
View File
@@ -31,7 +31,7 @@ var InternalAgentActions = []string{
// Never rds:*: granting the customer surface here would let a Postgres RCE on
// one DB VM manage its account's whole fleet.
var instanceRoleInlinePolicy = func() string {
func instanceRoleInlinePolicy() (string, error) {
actions := make(handlers_iam.StringOrArr, 0, len(InternalAgentActions))
for _, action := range InternalAgentActions {
actions = append(actions, "rds:"+action)
@@ -45,10 +45,10 @@ var instanceRoleInlinePolicy = func() string {
}},
})
if err != nil {
panic("rds: cannot marshal the instance-role policy: " + err.Error())
return "", fmt.Errorf("marshal instance-role policy: %w", err)
}
return string(doc)
}()
return string(doc), nil
}
// Resolved per launch rather than held, mirroring EKS: the KV-backed IAM
// service has no responders until JetStream is up, so an eager build races
@@ -65,8 +65,12 @@ func ensureInstanceProfile(provider IAMProvider, accountID string) (string, erro
if iamSvc == nil {
return "", errors.New("rds: IAM service is unavailable")
}
policy, err := instanceRoleInlinePolicy()
if err != nil {
return "", fmt.Errorf("rds: construct instance-role inline policy: %w", err)
}
profileARN, err := handlers_iam.EnsureSystemInstanceProfile(iamSvc, accountID,
InstanceRoleName, instanceRoleInlinePolicyName, instanceRoleInlinePolicy)
InstanceRoleName, instanceRoleInlinePolicyName, policy)
if err != nil {
return "", fmt.Errorf("rds: ensure instance profile %s for account %s: %w",
InstanceRoleName, accountID, err)
+6 -2
View File
@@ -28,8 +28,10 @@ func rdsInstanceProfileARN(accountID string) string {
// internal actions and nothing wildcarded that could reach the customer surface.
func TestInstanceRolePolicy_GrantsOnlyTheInternalActions(t *testing.T) {
t.Parallel()
policy, err := instanceRoleInlinePolicy()
require.NoError(t, err)
var doc handlers_iam.PolicyDocument
require.NoError(t, json.Unmarshal([]byte(instanceRoleInlinePolicy), &doc))
require.NoError(t, json.Unmarshal([]byte(policy), &doc))
require.Len(t, doc.Statement, 1)
stmt := doc.Statement[0]
@@ -81,9 +83,11 @@ func TestEnsureInstanceProfile_IsIdempotent(t *testing.T) {
assert.Equal(t, 1, f.CreateRoleCalls, "the second launch must find the role, not create it")
assert.Equal(t, 1, f.CreateInstanceProfileCalls, "the second launch must find the instance profile")
require.Len(t, f.PolicyCalls, 2, "the grant is re-asserted on every launch")
policy, err := instanceRoleInlinePolicy()
require.NoError(t, err)
for _, put := range f.PolicyCalls {
assert.Equal(t, instanceRoleInlinePolicyName, aws.StringValue(put.PolicyName))
assert.Equal(t, instanceRoleInlinePolicy, aws.StringValue(put.PolicyDocument))
assert.Equal(t, policy, aws.StringValue(put.PolicyDocument))
}
}
+19 -9
View File
@@ -1,6 +1,7 @@
package handlers_rds
import (
"errors"
"fmt"
"maps"
"math"
@@ -89,27 +90,36 @@ type ParameterSpec struct {
optionFileName string
}
// Indexes the specs by name and fails the build-equivalent — process start — on
// a malformed entry, so a catalog typo cannot reach a customer's create.
func buildParameterCatalog(specs ...ParameterSpec) map[string]ParameterSpec {
// Indexes the specs by name and returns malformed entries to startup validation.
func buildParameterCatalog(specs ...ParameterSpec) (map[string]ParameterSpec, error) {
out := make(map[string]ParameterSpec, len(specs))
for _, spec := range specs {
switch {
case spec.Name == "":
panic("rds: parameter catalog holds an unnamed entry")
return nil, errors.New("rds: parameter catalog holds an unnamed entry")
case spec.Default == "" && spec.DefaultFor == nil:
panic("rds: parameter catalog entry " + spec.Name + " has no default")
return nil, fmt.Errorf("rds: parameter catalog entry %s has no default", spec.Name)
case spec.Default != "" && spec.DefaultFor != nil:
panic("rds: parameter catalog entry " + spec.Name + " has both a literal and a computed default")
return nil, fmt.Errorf("rds: parameter catalog entry %s has both a literal and a computed default", spec.Name)
case (spec.DefaultFor == nil) != (spec.MaxFor == nil):
panic("rds: parameter catalog entry " + spec.Name + " must pair its computed default and class ceiling")
return nil, fmt.Errorf("rds: parameter catalog entry %s must pair its computed default and class ceiling", spec.Name)
}
if _, exists := out[spec.Name]; exists {
panic("rds: parameter catalog entry " + spec.Name + " is duplicated")
return nil, fmt.Errorf("rds: parameter catalog entry %s is duplicated", spec.Name)
}
out[spec.Name] = spec
}
return out
return out, nil
}
func validateParameterCatalogs() error {
if postgresParameterCatalogErr != nil {
return fmt.Errorf("rds: validate PostgreSQL parameter catalog: %w", postgresParameterCatalogErr)
}
if mariadbParameterCatalogErr != nil {
return fmt.Errorf("rds: validate MariaDB parameter catalog: %w", mariadbParameterCatalogErr)
}
return nil
}
// The catalog entry for a parameter name, or false when the engine has no such
@@ -15,6 +15,66 @@ import (
// customer can reach it.
var catalogEngines = []Engine{enginePostgres, engineMariaDB}
func TestBuildParameterCatalog_ReturnsValidationErrors(t *testing.T) {
computedDefault := func(int64) string { return "1" }
classCeiling := func(int64) int64 { return 2 }
tests := []struct {
name string
specs []ParameterSpec
wantErr string
}{
{
name: "valid entry",
specs: []ParameterSpec{{Name: "valid", Default: "1"}},
},
{
name: "unnamed entry",
specs: []ParameterSpec{{Default: "1"}},
wantErr: "holds an unnamed entry",
},
{
name: "missing default",
specs: []ParameterSpec{{Name: "missing"}},
wantErr: "has no default",
},
{
name: "literal and computed defaults",
specs: []ParameterSpec{{
Name: "both", Default: "1", DefaultFor: computedDefault, MaxFor: classCeiling,
}},
wantErr: "has both a literal and a computed default",
},
{
name: "computed default without ceiling",
specs: []ParameterSpec{{
Name: "unpaired", DefaultFor: computedDefault,
}},
wantErr: "must pair its computed default and class ceiling",
},
{
name: "duplicate entry",
specs: []ParameterSpec{
{Name: "duplicate", Default: "1"},
{Name: "duplicate", Default: "2"},
},
wantErr: "is duplicated",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
catalog, err := buildParameterCatalog(tt.specs...)
if tt.wantErr != "" {
require.ErrorContains(t, err, tt.wantErr)
assert.Nil(t, catalog)
return
}
require.NoError(t, err)
assert.Len(t, catalog, len(tt.specs))
})
}
}
// The failure this guards is the one the phase opens by warning about, arriving
// through the default path rather than a customer typo: a catalog change that
// makes the smallest class unbootable has to fail here rather than in a create.