mirror of
https://github.com/pgsty/minio.git
synced 2026-08-09 07:43:29 +03:00
162ded3438
GetNotifyNATS reads user_credentials, nkey_seed and tls_handshake_first and GetNotifyAMQP reads immediate, but none of them were registered in DefaultNATSKVS/DefaultAMQPKVS or the help schema, so CheckValidKeys rejected any enable=on target carrying them. Worse, the legacy config migration wrote exactly these keys - including the env var name MINIO_NOTIFY_NATS_USER_CREDENTIALS used as a config key, because the NATSUserCredentials constant doubled as both - so a migrated NATS config failed validation on every load, and the FetchEnabledTargets fail-fast then silently disabled all bucket notification targets. - Register user_credentials/nkey_seed/tls_handshake_first (NATS) and immediate (AMQP) in the default KVS and help schema; split NATSUserCredentials into a real config key plus EnvNATSUserCredentials (all env var names byte-stable) - Fix legacy migration: SetNotifyNATS writes the proper key; SetNotifyAMQP no longer writes cfg.Immediate under the internal key and now carries both immediate and internal - Tolerate the legacy MINIO_NOTIFY_NATS_USER_CREDENTIALS key written by pre-fix migrations (NATS-scoped, load path only) with fallback read; env > user_credentials > legacy key - Print key names only, never values, in the invalid-keys error of both CheckValidKeys forms; rejected values can carry credentials - Add an AST-based audit test asserting parser reads, migration writes and help entries stay within the registered key set for all ten notify subsystems, with floor assertions so collector drift fails loudly - Document (unchanged) FetchEnabledTargets fail-fast and pin it with a characterization test Known same-class gap left in place and pinned by the audit's allowlist: SetNotifyPostgres/SetNotifyMySQL write five unregistered DSN-era keys; tracked for a follow-up issue. Closes #39 Co-authored-by: ChatGPT <noreply@openai.com> Co-authored-by: Claude <noreply@anthropic.com>
758 lines
27 KiB
Go
758 lines
27 KiB
Go
// Copyright (c) 2015-2026 MinIO, Inc.
|
|
//
|
|
// This file is part of MinIO Object Storage stack
|
|
//
|
|
// This program is free software: you can redistribute it and/or modify
|
|
// it under the terms of the GNU Affero General Public License as published by
|
|
// the Free Software Foundation, either version 3 of the License, or
|
|
// (at your option) any later version.
|
|
//
|
|
// This program is distributed in the hope that it will be useful
|
|
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
// GNU Affero General Public License for more details.
|
|
//
|
|
// You should have received a copy of the GNU Affero General Public License
|
|
// along with this program. If not, see <http://www.gnu.org/licenses/>.
|
|
|
|
package notify
|
|
|
|
import (
|
|
"context"
|
|
"crypto/tls"
|
|
"go/ast"
|
|
"go/parser"
|
|
"go/token"
|
|
"io/fs"
|
|
"net/http"
|
|
"path/filepath"
|
|
"slices"
|
|
"sort"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/minio/minio/internal/config"
|
|
"github.com/minio/minio/internal/event/target"
|
|
)
|
|
|
|
// Test fixtures. Every value here is deliberately non-functional: the paths do
|
|
// not exist and the credentials are placeholders, so a leaked fixture is inert.
|
|
const (
|
|
testCredsPath = "/nonexistent/test.creds"
|
|
testNKeyPath = "/nonexistent/test.nk"
|
|
testNATSAddr = "127.0.0.1:4222"
|
|
testNATSSubj = "test-subject"
|
|
testAMQPURL = "amqp://guest:guest@127.0.0.1:5672"
|
|
testTargetName = "FITCHECK" // named target from issue #39
|
|
|
|
// The config key an operator types into `mc admin config set`. Spelled as a
|
|
// literal so these tests pin the on-disk schema rather than tracking
|
|
// whatever the Go constant happens to say.
|
|
natsCredsKey = "user_credentials"
|
|
// The env var name that the pre-fix migration wrote into the config store
|
|
// as if it were a config key.
|
|
legacyNATSCredsKey = "MINIO_NOTIFY_NATS_USER_CREDENTIALS"
|
|
)
|
|
|
|
// natsKVSFor builds a stored (on-disk) NATS target KVS carrying the three keys
|
|
// that issue #39 reported as rejected. config.Merge passes a stored target
|
|
// through verbatim rather than layering it over the defaults, so the numeric
|
|
// keys the parser reads must be present here too.
|
|
func natsKVSFor(credsKey string) config.KVS {
|
|
return config.KVS{
|
|
config.KV{Key: config.Enable, Value: config.EnableOn},
|
|
config.KV{Key: target.NATSAddress, Value: testNATSAddr},
|
|
config.KV{Key: target.NATSSubject, Value: testNATSSubj},
|
|
config.KV{Key: credsKey, Value: testCredsPath},
|
|
config.KV{Key: target.NATSNKeySeed, Value: testNKeyPath},
|
|
config.KV{Key: target.NATSTLSHandshakeFirst, Value: config.EnableOn},
|
|
config.KV{Key: target.NATSPingInterval, Value: "0"},
|
|
config.KV{Key: target.NATSQueueLimit, Value: "0"},
|
|
}
|
|
}
|
|
|
|
// T1: an enable=on NATS target carrying user_credentials / nkey_seed /
|
|
// tls_handshake_first must pass key validation. This is the exact shape from
|
|
// issue #39 (`mc admin config set us notify_nats:FITCHECK ...`).
|
|
func TestCheckValidNotificationKeysNATSJWTKeys(t *testing.T) {
|
|
for _, tgtName := range []string{config.Default, testTargetName} {
|
|
t.Run(tgtName, func(t *testing.T) {
|
|
tgt := map[string]config.KVS{
|
|
tgtName: natsKVSFor(natsCredsKey),
|
|
}
|
|
if err := checkValidNotificationKeysForSubSys(config.NotifyNATSSubSys, tgt); err != nil {
|
|
t.Fatalf("expected NATS JWT/nkey/handshake keys to validate, got: %v", err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// T2: the parser must surface the three values from stored config (no env).
|
|
func TestGetNotifyNATSFromStoredKVS(t *testing.T) {
|
|
for _, tgtName := range []string{config.Default, testTargetName} {
|
|
t.Run(tgtName, func(t *testing.T) {
|
|
natsKVS := map[string]config.KVS{
|
|
tgtName: natsKVSFor(natsCredsKey),
|
|
}
|
|
targets, err := GetNotifyNATS(natsKVS, nil)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyNATS: %v", err)
|
|
}
|
|
args, ok := targets[tgtName]
|
|
if !ok {
|
|
t.Fatalf("target %q missing from result %v", tgtName, targets)
|
|
}
|
|
if args.UserCredentials != testCredsPath {
|
|
t.Errorf("UserCredentials = %q, want %q", args.UserCredentials, testCredsPath)
|
|
}
|
|
if args.NKeySeed != testNKeyPath {
|
|
t.Errorf("NKeySeed = %q, want %q", args.NKeySeed, testNKeyPath)
|
|
}
|
|
if !args.TLSHandshakeFirst {
|
|
t.Errorf("TLSHandshakeFirst = false, want true")
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// T3: AMQP `immediate` must validate and parse.
|
|
func TestNotifyAMQPImmediate(t *testing.T) {
|
|
amqpKVS := map[string]config.KVS{
|
|
config.Default: {
|
|
config.KV{Key: config.Enable, Value: config.EnableOn},
|
|
config.KV{Key: target.AmqpURL, Value: testAMQPURL},
|
|
config.KV{Key: target.AmqpImmediate, Value: config.EnableOn},
|
|
config.KV{Key: target.AmqpDeliveryMode, Value: "0"},
|
|
config.KV{Key: target.AmqpQueueLimit, Value: "0"},
|
|
},
|
|
}
|
|
if err := checkValidNotificationKeysForSubSys(config.NotifyAMQPSubSys, amqpKVS); err != nil {
|
|
t.Fatalf("expected amqp immediate key to validate, got: %v", err)
|
|
}
|
|
targets, err := GetNotifyAMQP(amqpKVS)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyAMQP: %v", err)
|
|
}
|
|
args, ok := targets[config.Default]
|
|
if !ok {
|
|
t.Fatalf("default target missing from result %v", targets)
|
|
}
|
|
if !args.Immediate {
|
|
t.Errorf("Immediate = false, want true")
|
|
}
|
|
if args.Internal {
|
|
t.Errorf("Internal = true, want false (immediate must not bleed into internal)")
|
|
}
|
|
}
|
|
|
|
// T4 (guard, must hold before and after the fix): environment variables win
|
|
// over stored config, for both the default and the `_<TARGET>` suffixed form.
|
|
// Env var names are asserted as raw literals so that a rename of the Go
|
|
// constant cannot silently change the public interface.
|
|
func TestNotifyEnvOverridesStoredKVS(t *testing.T) {
|
|
const (
|
|
envCreds = "MINIO_NOTIFY_NATS_USER_CREDENTIALS"
|
|
envNKeySeed = "MINIO_NOTIFY_NATS_NKEY_SEED"
|
|
envHandshakeFirst = "MINIO_NOTIFY_NATS_TLS_HANDSHAKE_FIRST"
|
|
envAMQPImmediate = "MINIO_NOTIFY_AMQP_IMMEDIATE"
|
|
envCredsOverride = "/nonexistent/env-override.creds"
|
|
envNKeySeedOverride = "/nonexistent/env-override.nk"
|
|
)
|
|
|
|
t.Run("nats-default", func(t *testing.T) {
|
|
t.Setenv(envCreds, envCredsOverride)
|
|
t.Setenv(envNKeySeed, envNKeySeedOverride)
|
|
t.Setenv(envHandshakeFirst, config.EnableOff)
|
|
|
|
natsKVS := map[string]config.KVS{config.Default: natsKVSFor(natsCredsKey)}
|
|
targets, err := GetNotifyNATS(natsKVS, nil)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyNATS: %v", err)
|
|
}
|
|
args := targets[config.Default]
|
|
if args.UserCredentials != envCredsOverride {
|
|
t.Errorf("UserCredentials = %q, want env value %q", args.UserCredentials, envCredsOverride)
|
|
}
|
|
if args.NKeySeed != envNKeySeedOverride {
|
|
t.Errorf("NKeySeed = %q, want env value %q", args.NKeySeed, envNKeySeedOverride)
|
|
}
|
|
if args.TLSHandshakeFirst {
|
|
t.Errorf("TLSHandshakeFirst = true, want env value false")
|
|
}
|
|
})
|
|
|
|
t.Run("nats-named-target", func(t *testing.T) {
|
|
t.Setenv(envCreds+config.Default+testTargetName, envCredsOverride)
|
|
t.Setenv(envNKeySeed+config.Default+testTargetName, envNKeySeedOverride)
|
|
t.Setenv(envHandshakeFirst+config.Default+testTargetName, config.EnableOff)
|
|
|
|
natsKVS := map[string]config.KVS{testTargetName: natsKVSFor(natsCredsKey)}
|
|
targets, err := GetNotifyNATS(natsKVS, nil)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyNATS: %v", err)
|
|
}
|
|
args := targets[testTargetName]
|
|
if args.UserCredentials != envCredsOverride {
|
|
t.Errorf("UserCredentials = %q, want env value %q", args.UserCredentials, envCredsOverride)
|
|
}
|
|
if args.NKeySeed != envNKeySeedOverride {
|
|
t.Errorf("NKeySeed = %q, want env value %q", args.NKeySeed, envNKeySeedOverride)
|
|
}
|
|
if args.TLSHandshakeFirst {
|
|
t.Errorf("TLSHandshakeFirst = true, want env value false")
|
|
}
|
|
})
|
|
|
|
t.Run("amqp-immediate", func(t *testing.T) {
|
|
t.Setenv(envAMQPImmediate, config.EnableOn)
|
|
amqpKVS := map[string]config.KVS{
|
|
config.Default: {
|
|
config.KV{Key: config.Enable, Value: config.EnableOn},
|
|
config.KV{Key: target.AmqpURL, Value: testAMQPURL},
|
|
config.KV{Key: target.AmqpDeliveryMode, Value: "0"},
|
|
config.KV{Key: target.AmqpQueueLimit, Value: "0"},
|
|
},
|
|
}
|
|
targets, err := GetNotifyAMQP(amqpKVS)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyAMQP: %v", err)
|
|
}
|
|
if !targets[config.Default].Immediate {
|
|
t.Errorf("Immediate = false, want env value true")
|
|
}
|
|
})
|
|
|
|
// Pin the env var names that the constants must keep producing. These are
|
|
// public interface: renaming one silently breaks every deployment that
|
|
// sets it.
|
|
t.Run("env-names-are-stable", func(t *testing.T) {
|
|
for _, tc := range []struct{ got, want string }{
|
|
{target.EnvNATSUserCredentials, envCreds},
|
|
{target.EnvNATSNKeySeed, envNKeySeed},
|
|
{target.EnvNatsTLSHandshakeFirst, envHandshakeFirst},
|
|
{target.EnvAMQPImmediate, envAMQPImmediate},
|
|
} {
|
|
if tc.got != tc.want {
|
|
t.Errorf("env var name = %q, want %q", tc.got, tc.want)
|
|
}
|
|
}
|
|
})
|
|
|
|
// The config key must be the snake_case name, not the env var name.
|
|
t.Run("config-key-names-are-snake-case", func(t *testing.T) {
|
|
for _, tc := range []struct{ got, want string }{
|
|
{target.NATSUserCredentials, natsCredsKey},
|
|
{target.NATSNKeySeed, "nkey_seed"},
|
|
{target.NATSTLSHandshakeFirst, "tls_handshake_first"},
|
|
{target.AmqpImmediate, "immediate"},
|
|
} {
|
|
if tc.got != tc.want {
|
|
t.Errorf("config key = %q, want %q", tc.got, tc.want)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
|
|
// T6: a store written by the pre-fix migration carries the literal env-var name
|
|
// as a config key. Such a store must keep loading, and the value must still
|
|
// reach the parser. When both the legacy and the current key are present, the
|
|
// current key wins.
|
|
func TestNotifyNATSLegacyUserCredentialsKey(t *testing.T) {
|
|
const legacyKey = legacyNATSCredsKey
|
|
const newValue = "/nonexistent/new-key.creds"
|
|
|
|
t.Run("legacy-key-alone-validates-and-is-read", func(t *testing.T) {
|
|
natsKVS := map[string]config.KVS{
|
|
config.Default: natsKVSFor(legacyKey),
|
|
}
|
|
if err := checkValidNotificationKeysForSubSys(config.NotifyNATSSubSys, natsKVS); err != nil {
|
|
t.Fatalf("legacy-migrated store must keep validating, got: %v", err)
|
|
}
|
|
targets, err := GetNotifyNATS(natsKVS, nil)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyNATS: %v", err)
|
|
}
|
|
if got := targets[config.Default].UserCredentials; got != testCredsPath {
|
|
t.Errorf("UserCredentials = %q, want fallback to legacy key value %q", got, testCredsPath)
|
|
}
|
|
})
|
|
|
|
t.Run("new-key-wins-over-legacy", func(t *testing.T) {
|
|
kvs := natsKVSFor(legacyKey)
|
|
kvs = append(kvs, config.KV{Key: natsCredsKey, Value: newValue})
|
|
natsKVS := map[string]config.KVS{config.Default: kvs}
|
|
|
|
if err := checkValidNotificationKeysForSubSys(config.NotifyNATSSubSys, natsKVS); err != nil {
|
|
t.Fatalf("mixed old/new store must validate, got: %v", err)
|
|
}
|
|
targets, err := GetNotifyNATS(natsKVS, nil)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyNATS: %v", err)
|
|
}
|
|
if got := targets[config.Default].UserCredentials; got != newValue {
|
|
t.Errorf("UserCredentials = %q, want new key value %q", got, newValue)
|
|
}
|
|
})
|
|
|
|
t.Run("env-wins-over-legacy-key", func(t *testing.T) {
|
|
const envOverride = "/nonexistent/env-wins.creds"
|
|
t.Setenv(legacyNATSCredsKey, envOverride)
|
|
|
|
natsKVS := map[string]config.KVS{config.Default: natsKVSFor(legacyKey)}
|
|
targets, err := GetNotifyNATS(natsKVS, nil)
|
|
if err != nil {
|
|
t.Fatalf("GetNotifyNATS: %v", err)
|
|
}
|
|
if got := targets[config.Default].UserCredentials; got != envOverride {
|
|
t.Errorf("UserCredentials = %q, want env value %q", got, envOverride)
|
|
}
|
|
})
|
|
|
|
t.Run("legacy-key-rejected-for-other-subsystems", func(t *testing.T) {
|
|
// The tolerance is NATS-scoped; it must not become a global escape hatch.
|
|
amqpKVS := map[string]config.KVS{
|
|
config.Default: {
|
|
config.KV{Key: config.Enable, Value: config.EnableOn},
|
|
config.KV{Key: target.AmqpURL, Value: testAMQPURL},
|
|
config.KV{Key: legacyKey, Value: testCredsPath},
|
|
},
|
|
}
|
|
if err := checkValidNotificationKeysForSubSys(config.NotifyAMQPSubSys, amqpKVS); err == nil {
|
|
t.Fatal("expected the NATS legacy key to be rejected for the AMQP sub-system")
|
|
}
|
|
})
|
|
}
|
|
|
|
// T9 (characterization, not an endorsement): FetchEnabledTargets fails fast.
|
|
// A single sub-system with an invalid key aborts the whole target list, which
|
|
// is why one bad notify config disables every other notification target. This
|
|
// test pins the current behavior so that a future change to it is deliberate.
|
|
func TestFetchEnabledTargetsFailsFastAcrossSubSystems(t *testing.T) {
|
|
cfg := config.Config{
|
|
config.NotifyNATSSubSys: map[string]config.KVS{
|
|
config.Default: {
|
|
config.KV{Key: config.Enable, Value: config.EnableOn},
|
|
config.KV{Key: target.NATSAddress, Value: testNATSAddr},
|
|
config.KV{Key: target.NATSSubject, Value: testNATSSubj},
|
|
config.KV{Key: target.NATSPingInterval, Value: "0"},
|
|
config.KV{Key: target.NATSQueueLimit, Value: "0"},
|
|
config.KV{Key: "this_key_does_not_exist", Value: "junk"},
|
|
},
|
|
},
|
|
// A healthy sub-system that would yield a working target if the call
|
|
// got that far. NotifySubSystems.ToSlice() is sorted, so notify_nats
|
|
// is reached before notify_webhook and the error aborts the loop
|
|
// before this target is ever constructed -- the healthy config is lost
|
|
// without anything being built or dialed.
|
|
config.NotifyWebhookSubSys: map[string]config.KVS{
|
|
config.Default: {
|
|
config.KV{Key: config.Enable, Value: config.EnableOn},
|
|
config.KV{Key: target.WebhookEndpoint, Value: "http://127.0.0.1:65535/"},
|
|
config.KV{Key: target.WebhookQueueLimit, Value: "0"},
|
|
},
|
|
},
|
|
}
|
|
|
|
// FetchEnabledTargets dereferences transport.TLSClientConfig unconditionally
|
|
// for some sub-systems, so a real transport is required even though no
|
|
// target here performs I/O.
|
|
transport := &http.Transport{TLSClientConfig: &tls.Config{}}
|
|
|
|
targetList, err := FetchEnabledTargets(context.Background(), cfg, transport)
|
|
if err == nil {
|
|
t.Fatal("expected FetchEnabledTargets to fail fast on the invalid sub-system")
|
|
}
|
|
if targetList != nil {
|
|
t.Errorf("target list = %v, want nil (fail-fast discards healthy targets)", targetList)
|
|
}
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// T7: source-level consistency guard.
|
|
//
|
|
// Every config key the parser reads or the legacy migration writes must be
|
|
// registered in the sub-system's default KVS, otherwise config.CheckValidKeys
|
|
// rejects a config that the parser would happily consume (issue #39).
|
|
// ---------------------------------------------------------------------------
|
|
|
|
// notifySubSysAudit enumerates the notify sub-systems and the symbols that make
|
|
// up their config surface. Keep in sync with DefaultNotificationKVS; the audit
|
|
// itself asserts that every registered sub-system appears here.
|
|
// tolerated lists keys that are not registered but are still accepted by
|
|
// checkValidNotificationKeysForSubSys as deprecated keys. The parser may read
|
|
// them; the migration must never write them and they stay undocumented.
|
|
var notifySubSysAudit = []struct {
|
|
subSys string
|
|
getFn string
|
|
setFn string
|
|
help config.HelpKVS
|
|
tolerated []string
|
|
}{
|
|
{config.NotifyAMQPSubSys, "GetNotifyAMQP", "SetNotifyAMQP", HelpAMQP, nil},
|
|
{config.NotifyESSubSys, "GetNotifyES", "SetNotifyES", HelpES, nil},
|
|
{config.NotifyKafkaSubSys, "GetNotifyKafka", "SetNotifyKafka", HelpKafka, nil},
|
|
{config.NotifyMQTTSubSys, "GetNotifyMQTT", "SetNotifyMQTT", HelpMQTT, nil},
|
|
{config.NotifyMySQLSubSys, "GetNotifyMySQL", "SetNotifyMySQL", HelpMySQL, nil},
|
|
{config.NotifyNATSSubSys, "GetNotifyNATS", "SetNotifyNATS", HelpNATS, []string{legacyNATSUserCredentialsKey}},
|
|
{config.NotifyNSQSubSys, "GetNotifyNSQ", "SetNotifyNSQ", HelpNSQ, nil},
|
|
{config.NotifyPostgresSubSys, "GetNotifyPostgres", "SetNotifyPostgres", HelpPostgres, nil},
|
|
{config.NotifyRedisSubSys, "GetNotifyRedis", "SetNotifyRedis", HelpRedis, nil},
|
|
{config.NotifyWebhookSubSys, "GetNotifyWebhook", "SetNotifyWebhook", HelpWebhook, nil},
|
|
}
|
|
|
|
// notifyPkgConsts resolves bare identifiers used as config keys in this
|
|
// package. Compiler-resolved, so they cannot drift.
|
|
var notifyPkgConsts = map[string]string{
|
|
"legacyNATSUserCredentialsKey": legacyNATSUserCredentialsKey,
|
|
}
|
|
|
|
// configPkgConsts resolves the `config.X` selectors that appear as config keys
|
|
// in parse.go / legacy.go. These are compiler-resolved, so they cannot drift.
|
|
var configPkgConsts = map[string]string{
|
|
"Enable": config.Enable,
|
|
"Comment": config.Comment,
|
|
}
|
|
|
|
// knownUnregisteredWrites records pre-existing instances of the exact defect
|
|
// this audit exists to catch: a legacy migration writing config keys that no
|
|
// default KVS registers, so the migrated config is rejected on the next load.
|
|
//
|
|
// These are inherited from upstream and are the same class as issue #39, but
|
|
// they are NOT part of the issue #39 fix and were left untouched deliberately.
|
|
// The Postgres/MySQL keys below are the pre-connection-string DSN fields; the
|
|
// migration still writes them and `password` carries a plaintext database
|
|
// password.
|
|
//
|
|
// This list must only ever shrink. Do not add entries to silence a new gap.
|
|
var knownUnregisteredWrites = map[string][]string{
|
|
"SetNotifyPostgres": {"host", "port", "username", "password", "database"},
|
|
"SetNotifyMySQL": {"host", "port", "username", "password", "database"},
|
|
}
|
|
|
|
func TestNotifyConfigKeysAreRegistered(t *testing.T) {
|
|
targetConsts, err := parseTargetPkgStringConsts("../../event/target")
|
|
if err != nil {
|
|
t.Fatalf("resolving internal/event/target constants: %v", err)
|
|
}
|
|
|
|
fset := token.NewFileSet()
|
|
readKeys := map[string]map[string]token.Position{} // fn name -> key -> pos
|
|
writtenKeys := map[string]map[string]token.Position{} // fn name -> key -> pos
|
|
|
|
for _, src := range []string{"parse.go", "legacy.go"} {
|
|
f, err := parser.ParseFile(fset, src, nil, 0)
|
|
if err != nil {
|
|
t.Fatalf("parsing %s: %v", src, err)
|
|
}
|
|
resolve := func(e ast.Expr) (string, bool) {
|
|
return resolveKeyExpr(t, fset, src, e, targetConsts)
|
|
}
|
|
for _, decl := range f.Decls {
|
|
fn, ok := decl.(*ast.FuncDecl)
|
|
if !ok || fn.Body == nil {
|
|
continue
|
|
}
|
|
name := fn.Name.Name
|
|
switch {
|
|
case strings.HasPrefix(name, "GetNotify"):
|
|
readKeys[name] = collectKVGetKeys(fset, fn, resolve)
|
|
case strings.HasPrefix(name, "SetNotify"):
|
|
writtenKeys[name] = collectKVWriteKeys(fset, fn, resolve)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Guard against a new sub-system slipping past the audit table.
|
|
audited := map[string]bool{}
|
|
for _, a := range notifySubSysAudit {
|
|
audited[a.subSys] = true
|
|
}
|
|
for subSys := range DefaultNotificationKVS {
|
|
if !audited[subSys] {
|
|
t.Errorf("sub-system %q is registered in DefaultNotificationKVS but missing from notifySubSysAudit", subSys)
|
|
}
|
|
}
|
|
|
|
for _, a := range notifySubSysAudit {
|
|
t.Run(a.subSys, func(t *testing.T) {
|
|
defaults := DefaultNotificationKVS[a.subSys]
|
|
registered := map[string]bool{}
|
|
for _, kv := range defaults {
|
|
registered[kv.Key] = true
|
|
}
|
|
|
|
read, ok := readKeys[a.getFn]
|
|
if !ok {
|
|
t.Fatalf("%s not found in parse.go/legacy.go", a.getFn)
|
|
}
|
|
for _, key := range sortedKeys(read) {
|
|
if !registered[key] && !slices.Contains(a.tolerated, key) {
|
|
t.Errorf("%s reads key %q (%s) which is not registered in the default KVS for %s",
|
|
a.getFn, key, read[key], a.subSys)
|
|
}
|
|
}
|
|
// Floor assertion, in the opposite direction. Every registered key
|
|
// is in fact read by every GetNotifyX today, so this holds. Its
|
|
// real job is to fail loudly if the read collector ever stops
|
|
// matching the source — a collector that silently returns nothing
|
|
// would otherwise make the check above pass vacuously.
|
|
for _, kv := range defaults {
|
|
if _, ok := read[kv.Key]; !ok {
|
|
t.Errorf("registered key %q for %s is never read by %s; either the key is dead or this audit has gone blind",
|
|
kv.Key, a.subSys, a.getFn)
|
|
}
|
|
}
|
|
// A tolerated key must genuinely be accepted by validation,
|
|
// otherwise reading it is pointless.
|
|
for _, key := range a.tolerated {
|
|
kvs := map[string]config.KVS{config.Default: {
|
|
config.KV{Key: config.Enable, Value: config.EnableOn},
|
|
config.KV{Key: key, Value: "x"},
|
|
}}
|
|
if err := checkValidNotificationKeysForSubSys(a.subSys, kvs); err != nil {
|
|
t.Errorf("tolerated key %q is still rejected for %s: %v", key, a.subSys, err)
|
|
}
|
|
}
|
|
|
|
written, ok := writtenKeys[a.setFn]
|
|
if !ok {
|
|
t.Fatalf("%s not found in parse.go/legacy.go", a.setFn)
|
|
}
|
|
allowed := knownUnregisteredWrites[a.setFn]
|
|
for _, key := range sortedKeys(written) {
|
|
if !registered[key] && !slices.Contains(allowed, key) {
|
|
t.Errorf("%s writes key %q (%s) which is not registered in the default KVS for %s",
|
|
a.setFn, key, written[key], a.subSys)
|
|
}
|
|
}
|
|
// Keep the allowlist honest: an entry that no longer corresponds to
|
|
// a real gap must be deleted, not left to rot.
|
|
for _, key := range allowed {
|
|
if registered[key] {
|
|
t.Errorf("%s: %q is registered now; remove it from knownUnregisteredWrites", a.setFn, key)
|
|
} else if _, ok := written[key]; !ok {
|
|
t.Errorf("%s no longer writes %q; remove it from knownUnregisteredWrites", a.setFn, key)
|
|
}
|
|
}
|
|
|
|
// Help entries must describe registered keys only. `comment` is
|
|
// accepted for every sub-system by CheckValidKeys and is therefore
|
|
// documented without being registered.
|
|
for _, hkv := range a.help {
|
|
if hkv.Key == config.Comment {
|
|
continue
|
|
}
|
|
if !registered[hkv.Key] {
|
|
t.Errorf("Help entry %q for %s is not registered in the default KVS", hkv.Key, a.subSys)
|
|
}
|
|
}
|
|
|
|
// Conversely: every registered key a user can set should be
|
|
// documented, so `mc admin config get` explains it. `enable` is
|
|
// implicit for every target and HiddenIfEmpty keys are deprecated
|
|
// leftovers that are deliberately undocumented.
|
|
for _, kv := range defaults {
|
|
if kv.Key == config.Enable || kv.HiddenIfEmpty {
|
|
continue
|
|
}
|
|
if _, ok := a.help.Lookup(kv.Key); !ok {
|
|
t.Errorf("registered key %q for %s has no Help entry", kv.Key, a.subSys)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func sortedKeys(m map[string]token.Position) []string {
|
|
out := make([]string, 0, len(m))
|
|
for k := range m {
|
|
out = append(out, k)
|
|
}
|
|
sort.Strings(out)
|
|
return out
|
|
}
|
|
|
|
// resolveKeyExpr turns a config-key expression into its string value. Only the
|
|
// shapes actually used in this package are supported; anything else fails the
|
|
// test loudly rather than being skipped, so the audit cannot silently go blind.
|
|
func resolveKeyExpr(t *testing.T, fset *token.FileSet, src string, e ast.Expr, targetConsts map[string]string) (string, bool) {
|
|
t.Helper()
|
|
switch v := e.(type) {
|
|
case *ast.Ident:
|
|
val, ok := notifyPkgConsts[v.Name]
|
|
if !ok {
|
|
t.Errorf("%s: %s used as a config key but not listed in notifyPkgConsts; add it",
|
|
fset.Position(v.Pos()), v.Name)
|
|
return "", false
|
|
}
|
|
return val, true
|
|
case *ast.BasicLit:
|
|
if v.Kind == token.STRING {
|
|
s, err := strconv.Unquote(v.Value)
|
|
if err != nil {
|
|
t.Errorf("%s: unquoting %s: %v", fset.Position(v.Pos()), v.Value, err)
|
|
return "", false
|
|
}
|
|
return s, true
|
|
}
|
|
case *ast.SelectorExpr:
|
|
pkg, ok := v.X.(*ast.Ident)
|
|
if !ok {
|
|
break
|
|
}
|
|
switch pkg.Name {
|
|
case "target":
|
|
val, ok := targetConsts[v.Sel.Name]
|
|
if !ok {
|
|
t.Errorf("%s: cannot resolve target.%s to a string constant", fset.Position(v.Pos()), v.Sel.Name)
|
|
return "", false
|
|
}
|
|
return val, true
|
|
case "config":
|
|
val, ok := configPkgConsts[v.Sel.Name]
|
|
if !ok {
|
|
t.Errorf("%s: config.%s used as a config key but not listed in configPkgConsts; add it",
|
|
fset.Position(v.Pos()), v.Sel.Name)
|
|
return "", false
|
|
}
|
|
return val, true
|
|
}
|
|
}
|
|
t.Errorf("%s: unsupported config-key expression %T in %s; extend resolveKeyExpr", fset.Position(e.Pos()), e, src)
|
|
return "", false
|
|
}
|
|
|
|
// collectKVGetKeys finds every `<x>.Get(<key>)` call inside fn. This is how
|
|
// GetNotifyX reads a stored config value.
|
|
func collectKVGetKeys(fset *token.FileSet, fn *ast.FuncDecl, resolve func(ast.Expr) (string, bool)) map[string]token.Position {
|
|
keys := map[string]token.Position{}
|
|
ast.Inspect(fn.Body, func(n ast.Node) bool {
|
|
call, ok := n.(*ast.CallExpr)
|
|
if !ok || len(call.Args) != 1 {
|
|
return true
|
|
}
|
|
sel, ok := call.Fun.(*ast.SelectorExpr)
|
|
if !ok || sel.Sel.Name != "Get" {
|
|
return true
|
|
}
|
|
recv, ok := sel.X.(*ast.Ident)
|
|
if !ok || recv.Name != "kv" {
|
|
return true
|
|
}
|
|
if key, ok := resolve(call.Args[0]); ok {
|
|
keys[key] = fset.Position(call.Pos())
|
|
}
|
|
return true
|
|
})
|
|
return keys
|
|
}
|
|
|
|
// collectKVWriteKeys finds every `{Key: <key>, ...}` composite literal inside
|
|
// fn. This is how SetNotifyX writes a migrated config.
|
|
//
|
|
// Both the explicit `config.KV{...}` form and the elided `{...}` form that Go
|
|
// permits inside a `config.KVS{...}` literal must be matched: the elided form
|
|
// is idiomatic and would otherwise slip past this audit silently.
|
|
func collectKVWriteKeys(fset *token.FileSet, fn *ast.FuncDecl, resolve func(ast.Expr) (string, bool)) map[string]token.Position {
|
|
keys := map[string]token.Position{}
|
|
ast.Inspect(fn.Body, func(n ast.Node) bool {
|
|
lit, ok := n.(*ast.CompositeLit)
|
|
if !ok {
|
|
return true
|
|
}
|
|
// A typed literal that is neither config.KV nor config.KVS cannot hold
|
|
// or contain a config key. An untyped (elided) literal has no type to
|
|
// check, so it must be inspected rather than skipped.
|
|
if sel, ok := lit.Type.(*ast.SelectorExpr); ok && sel.Sel.Name != "KV" && sel.Sel.Name != "KVS" {
|
|
return true
|
|
}
|
|
for _, elt := range lit.Elts {
|
|
kve, ok := elt.(*ast.KeyValueExpr)
|
|
if !ok {
|
|
continue
|
|
}
|
|
ident, ok := kve.Key.(*ast.Ident)
|
|
if !ok || ident.Name != "Key" {
|
|
continue
|
|
}
|
|
if key, ok := resolve(kve.Value); ok {
|
|
keys[key] = fset.Position(lit.Pos())
|
|
}
|
|
}
|
|
return true
|
|
})
|
|
return keys
|
|
}
|
|
|
|
// parseTargetPkgStringConsts reads every `Name = "literal"` constant from the
|
|
// internal/event/target sources. Those constants are the single source of truth
|
|
// for both config keys and env var names.
|
|
func parseTargetPkgStringConsts(dir string) (map[string]string, error) {
|
|
fset := token.NewFileSet()
|
|
pkgs, err := parser.ParseDir(fset, filepath.Clean(dir), func(fi fs.FileInfo) bool {
|
|
return !strings.HasSuffix(fi.Name(), "_test.go")
|
|
}, 0)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
out := map[string]string{}
|
|
for _, pkg := range pkgs {
|
|
for _, f := range pkg.Files {
|
|
for _, decl := range f.Decls {
|
|
gd, ok := decl.(*ast.GenDecl)
|
|
if !ok || gd.Tok != token.CONST {
|
|
continue
|
|
}
|
|
for _, spec := range gd.Specs {
|
|
vs, ok := spec.(*ast.ValueSpec)
|
|
if !ok || len(vs.Names) != 1 || len(vs.Values) != 1 {
|
|
continue
|
|
}
|
|
lit, ok := vs.Values[0].(*ast.BasicLit)
|
|
if !ok || lit.Kind != token.STRING {
|
|
continue
|
|
}
|
|
s, err := strconv.Unquote(lit.Value)
|
|
if err != nil {
|
|
continue
|
|
}
|
|
out[vs.Names[0].Name] = s
|
|
}
|
|
}
|
|
}
|
|
}
|
|
if len(out) == 0 {
|
|
return nil, errNoTargetConsts
|
|
}
|
|
return out, nil
|
|
}
|
|
|
|
var errNoTargetConsts = errConst("no string constants found in internal/event/target; the audit would be vacuous")
|
|
|
|
type errConst string
|
|
|
|
func (e errConst) Error() string { return string(e) }
|
|
|
|
// Sanity check for the resolver itself: if this ever stops finding known
|
|
// constants the audit above would pass vacuously.
|
|
func TestParseTargetPkgStringConsts(t *testing.T) {
|
|
consts, err := parseTargetPkgStringConsts("../../event/target")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
for name, want := range map[string]string{
|
|
"NATSAddress": "address",
|
|
"NATSNKeySeed": "nkey_seed",
|
|
"AmqpImmediate": "immediate",
|
|
"EnvAMQPImmediate": "MINIO_NOTIFY_AMQP_IMMEDIATE",
|
|
} {
|
|
if got := consts[name]; got != want {
|
|
t.Errorf("target.%s = %q, want %q", name, got, want)
|
|
}
|
|
}
|
|
if slices.Contains([]string{""}, consts["NATSSubject"]) {
|
|
t.Error("target.NATSSubject resolved to empty string")
|
|
}
|
|
}
|