diff --git a/weed/credential/credential_manager.go b/weed/credential/credential_manager.go index 190260b3e..b7d52c781 100644 --- a/weed/credential/credential_manager.go +++ b/weed/credential/credential_manager.go @@ -305,6 +305,20 @@ func (cm *CredentialManager) ListUserInlinePolicies(ctx context.Context, userNam return nil, nil } +// RenameUser atomically renames a user along with all FK-backed +// dependents (credentials, inline policies, ...) when the underlying +// store implements UserRenamer. Returns false if the store does not +// support an atomic rename, in which case the caller is expected to +// fall back to its own best-effort path. A returned non-nil error +// always means the rename was attempted and failed. +func (cm *CredentialManager) RenameUser(ctx context.Context, oldName, newName string) (bool, error) { + store, ok := cm.Store.(UserRenamer) + if !ok { + return false, nil + } + return true, store.RenameUser(ctx, oldName, newName) +} + // LoadS3ConfigFile reads a static S3 identity config file and registers // the identities so they appear in LoadConfiguration and listing results. func (cm *CredentialManager) LoadS3ConfigFile(path string) error { diff --git a/weed/credential/credential_store.go b/weed/credential/credential_store.go index 9c8e44461..440f281b3 100644 --- a/weed/credential/credential_store.go +++ b/weed/credential/credential_store.go @@ -148,5 +148,17 @@ type InlinePolicyStore interface { ListUserInlinePolicies(ctx context.Context, userName string) ([]string, error) } +// UserRenamer is an optional interface for credential stores that can +// atomically rename a user along with all rows that reference the old +// username (credentials, inline policies, etc). Backends with referential +// integrity (e.g. PostgreSQL with ON DELETE CASCADE foreign keys on +// user_inline_policies) MUST implement this so the IAM rename path can +// move the dependents within a single transaction; copying them via +// per-row Put / Get / Delete calls would violate the FK at statement +// time because the new user row does not yet exist. +type UserRenamer interface { + RenameUser(ctx context.Context, oldName, newName string) error +} + // Stores holds all available credential store implementations var Stores []CredentialStore diff --git a/weed/credential/memory/memory_identity.go b/weed/credential/memory/memory_identity.go index 4430a7bf1..1cf0d3179 100644 --- a/weed/credential/memory/memory_identity.go +++ b/weed/credential/memory/memory_identity.go @@ -176,6 +176,46 @@ func (store *MemoryStore) DeleteUser(ctx context.Context, username string) error // Remove user delete(store.users, username) + delete(store.inlinePolicies, username) + + return nil +} + +// RenameUser implements credential.UserRenamer for the memory store. +// Renames the user and re-points access-key and inline-policy indexes +// to the new name. The whole operation is performed under the store +// lock so it is atomic with respect to concurrent reads. +func (store *MemoryStore) RenameUser(ctx context.Context, oldName, newName string) error { + store.mu.Lock() + defer store.mu.Unlock() + + if !store.initialized { + return fmt.Errorf("store not initialized") + } + if oldName == newName { + return nil + } + + user, exists := store.users[oldName] + if !exists { + return credential.ErrUserNotFound + } + if _, clash := store.users[newName]; clash { + return credential.ErrUserAlreadyExists + } + + user.Name = newName + store.users[newName] = user + delete(store.users, oldName) + + for _, cred := range user.Credentials { + store.accessKeys[cred.AccessKey] = newName + } + + if policies, ok := store.inlinePolicies[oldName]; ok { + store.inlinePolicies[newName] = policies + delete(store.inlinePolicies, oldName) + } return nil } diff --git a/weed/credential/postgres/postgres_group.go b/weed/credential/postgres/postgres_group.go index dca8c6817..7b77de78b 100644 --- a/weed/credential/postgres/postgres_group.go +++ b/weed/credential/postgres/postgres_group.go @@ -27,7 +27,7 @@ func (store *PostgresStore) CreateGroup(ctx context.Context, group *iam_pb.Group _, err = store.db.ExecContext(ctx, `INSERT INTO groups (name, members, policy_names, disabled) VALUES ($1, $2, $3, $4)`, - group.Name, membersJSON, policyNamesJSON, group.Disabled) + group.Name, jsonbParam(membersJSON), jsonbParam(policyNamesJSON), group.Disabled) if err != nil { var pgErr *pgconn.PgError if errors.As(err, &pgErr) && pgErr.Code == "23505" { @@ -112,7 +112,7 @@ func (store *PostgresStore) UpdateGroup(ctx context.Context, group *iam_pb.Group result, err := store.db.ExecContext(ctx, `UPDATE groups SET members = $1, policy_names = $2, disabled = $3, updated_at = CURRENT_TIMESTAMP WHERE name = $4`, - membersJSON, policyNamesJSON, group.Disabled, group.Name) + jsonbParam(membersJSON), jsonbParam(policyNamesJSON), group.Disabled, group.Name) if err != nil { return fmt.Errorf("failed to update group: %w", err) } diff --git a/weed/credential/postgres/postgres_identity.go b/weed/credential/postgres/postgres_identity.go index 288734ead..40c4e9d96 100644 --- a/weed/credential/postgres/postgres_identity.go +++ b/weed/credential/postgres/postgres_identity.go @@ -73,9 +73,10 @@ func (store *PostgresStore) LoadConfiguration(ctx context.Context) (*iam_pb.S3Ap SecretKey: secretKey, }) } + credErr := credRows.Err() credRows.Close() - if err := credRows.Err(); err != nil { - return nil, fmt.Errorf("failed iterating credential rows for user %s: %w", username, err) + if credErr != nil { + return nil, fmt.Errorf("failed iterating credential rows for user %s: %w", username, credErr) } config.Identities = append(config.Identities, identity) } @@ -100,34 +101,36 @@ func (store *PostgresStore) SaveConfiguration(ctx context.Context, config *iam_p // Track which usernames are in the incoming config for pruning configUsernames := make(map[string]bool, len(config.Identities)) - for _, identity := range config.Identities { configUsernames[identity.Name] = true + } - var accountDataParam any + // Upsert each user's row. + for _, identity := range config.Identities { + var accountDataJSON []byte if identity.Account != nil { - b, err := json.Marshal(identity.Account) + accountDataJSON, err = json.Marshal(identity.Account) if err != nil { return fmt.Errorf("failed to marshal account data for user %s: %v", identity.Name, err) } - accountDataParam = string(b) } - var actionsParam any + + var actionsJSON []byte if identity.Actions != nil { - b, err := json.Marshal(identity.Actions) + actionsJSON, err = json.Marshal(identity.Actions) if err != nil { return fmt.Errorf("failed to marshal actions for user %s: %v", identity.Name, err) } - actionsParam = string(b) } - var policyNamesParam any + + var policyNamesJSON []byte if identity.PolicyNames != nil { - b, err := json.Marshal(identity.PolicyNames) + policyNamesJSON, err = json.Marshal(identity.PolicyNames) if err != nil { return fmt.Errorf("failed to marshal policy names for user %s: %v", identity.Name, err) } - policyNamesParam = string(b) } + // Upsert user — preserves the row (and its CASCADE dependents) if it already exists _, err = tx.ExecContext(ctx, `INSERT INTO users (username, email, account_data, actions, policy_names) @@ -138,28 +141,20 @@ func (store *PostgresStore) SaveConfiguration(ctx context.Context, config *iam_p actions = EXCLUDED.actions, policy_names = EXCLUDED.policy_names, updated_at = CURRENT_TIMESTAMP`, - identity.Name, "", accountDataParam, actionsParam, policyNamesParam) + identity.Name, "", jsonbParam(accountDataJSON), jsonbParam(actionsJSON), jsonbParam(policyNamesJSON)) if err != nil { return fmt.Errorf("failed to upsert user %s: %v", identity.Name, err) } - - // Replace credentials for this user — credentials carry no independent - // state worth preserving (unlike inline policies) - if _, err := tx.ExecContext(ctx, "DELETE FROM credentials WHERE username = $1", identity.Name); err != nil { - return fmt.Errorf("failed to clear credentials for user %s: %v", identity.Name, err) - } - for _, cred := range identity.Credentials { - _, err := tx.ExecContext(ctx, - "INSERT INTO credentials (username, access_key, secret_key) VALUES ($1, $2, $3)", - identity.Name, cred.AccessKey, cred.SecretKey) - if err != nil { - return fmt.Errorf("failed to insert credential for user %s: %v", identity.Name, err) - } - } } - // Prune users no longer in config — CASCADE correctly removes their - // credentials and inline policies since they were intentionally deleted + // Prune users no longer in config BEFORE replacing credentials. The + // IAM rename path (s3api UpdateUser) renames an identity in place and + // keeps its access keys: the renamed user shows up in the incoming + // config, the old name shows up in the pruned set. If we inserted the + // new credentials first, the renamed user's access keys would still be + // owned by the old row in this transaction and the INSERT would + // violate the global UNIQUE constraint on credentials.access_key. + // CASCADE on the old row releases those keys before the insert. rows, err := tx.QueryContext(ctx, "SELECT username FROM users") if err != nil { return fmt.Errorf("failed to list existing users for pruning: %w", err) @@ -175,19 +170,59 @@ func (store *PostgresStore) SaveConfiguration(ctx context.Context, config *iam_p toDelete = append(toDelete, username) } } + scanErr := rows.Err() rows.Close() - if err := rows.Err(); err != nil { - return fmt.Errorf("failed iterating user rows for pruning: %w", err) + if scanErr != nil { + return fmt.Errorf("failed iterating user rows for pruning: %w", scanErr) } - for _, username := range toDelete { - if _, err := tx.ExecContext(ctx, "DELETE FROM users WHERE username = $1", username); err != nil { - return fmt.Errorf("failed to prune user %s: %v", username, err) + if len(toDelete) > 0 { + var inlineCount int + if err := tx.QueryRowContext(ctx, + "SELECT COUNT(*) FROM user_inline_policies WHERE username = ANY($1)", + toDelete).Scan(&inlineCount); err != nil { + glog.Warningf("credential postgres: SaveConfiguration failed to count inline policies for prune candidates: %v", err) + } else if inlineCount > 0 { + glog.Warningf("credential postgres: SaveConfiguration pruning %d users will CASCADE-remove %d inline policies; if this was a rename, re-create them under the new name", len(toDelete), inlineCount) + } + if _, err := tx.ExecContext(ctx, "DELETE FROM users WHERE username = ANY($1)", toDelete); err != nil { + return fmt.Errorf("failed to prune users: %w", err) } } - glog.V(0).Infof("credential postgres: SaveConfiguration saved %d identities, pruned %d", len(config.Identities), len(toDelete)) - return tx.Commit() + // Two-pass credential replace: first clear every user we are about to + // rewrite in a single round-trip, then insert. Doing the per-user delete + // + insert in a single pass would violate the global UNIQUE constraint + // on credentials.access_key when an access key gets reassigned from one + // user to another within the same SaveConfiguration call. + usernames := make([]string, 0, len(configUsernames)) + for name := range configUsernames { + usernames = append(usernames, name) + } + if _, err := tx.ExecContext(ctx, "DELETE FROM credentials WHERE username = ANY($1)", usernames); err != nil { + return fmt.Errorf("failed to clear credentials for incoming users: %w", err) + } + for _, identity := range config.Identities { + for _, cred := range identity.Credentials { + if _, err := tx.ExecContext(ctx, + "INSERT INTO credentials (username, access_key, secret_key) VALUES ($1, $2, $3)", + identity.Name, cred.AccessKey, cred.SecretKey); err != nil { + return fmt.Errorf("failed to insert credential for user %s: %v", identity.Name, err) + } + } + } + + if err := tx.Commit(); err != nil { + glog.Errorf("credential postgres: SaveConfiguration commit failed: %v", err) + return fmt.Errorf("failed to commit SaveConfiguration: %w", err) + } + + if len(toDelete) > 0 { + glog.Warningf("credential postgres: SaveConfiguration saved %d identities, pruned %d", len(config.Identities), len(toDelete)) + } else { + glog.V(0).Infof("credential postgres: SaveConfiguration saved %d identities", len(config.Identities)) + } + return nil } func (store *PostgresStore) CreateUser(ctx context.Context, identity *iam_pb.Identity) error { @@ -212,33 +247,33 @@ func (store *PostgresStore) CreateUser(ctx context.Context, identity *iam_pb.Ide } defer tx.Rollback() - var accountDataParam any + var accountDataJSON []byte if identity.Account != nil { - b, err := json.Marshal(identity.Account) + accountDataJSON, err = json.Marshal(identity.Account) if err != nil { return fmt.Errorf("failed to marshal account data: %w", err) } - accountDataParam = string(b) } - var actionsParam any + + var actionsJSON []byte if identity.Actions != nil { - b, err := json.Marshal(identity.Actions) + actionsJSON, err = json.Marshal(identity.Actions) if err != nil { return fmt.Errorf("failed to marshal actions: %w", err) } - actionsParam = string(b) } - var policyNamesParam any + + var policyNamesJSON []byte if identity.PolicyNames != nil { - b, err := json.Marshal(identity.PolicyNames) + policyNamesJSON, err = json.Marshal(identity.PolicyNames) if err != nil { return fmt.Errorf("failed to marshal policy names: %w", err) } - policyNamesParam = string(b) } + _, err = tx.ExecContext(ctx, "INSERT INTO users (username, email, account_data, actions, policy_names) VALUES ($1, $2, $3, $4, $5)", - identity.Name, "", accountDataParam, actionsParam, policyNamesParam) + identity.Name, "", jsonbParam(accountDataJSON), jsonbParam(actionsJSON), jsonbParam(policyNamesJSON)) if err != nil { glog.Errorf("credential postgres: CreateUser insert failed user=%s: %v", identity.Name, err) return fmt.Errorf("failed to insert user: %w", err) @@ -256,7 +291,7 @@ func (store *PostgresStore) CreateUser(ctx context.Context, identity *iam_pb.Ide if err := tx.Commit(); err != nil { glog.Errorf("credential postgres: CreateUser commit failed user=%s: %v", identity.Name, err) - return fmt.Errorf("failed to commit: %w", err) + return fmt.Errorf("failed to commit create user %s: %w", identity.Name, err) } glog.V(0).Infof("credential postgres: CreateUser user=%s credentials=%d actions=%d", identity.Name, len(identity.Credentials), len(identity.Actions)) @@ -329,7 +364,6 @@ func (store *PostgresStore) GetUser(ctx context.Context, username string) (*iam_ return identity, nil } - func (store *PostgresStore) UpdateUser(ctx context.Context, username string, identity *iam_pb.Identity) error { if !store.configured { return fmt.Errorf("store not configured") @@ -350,33 +384,33 @@ func (store *PostgresStore) UpdateUser(ctx context.Context, username string, ide return credential.ErrUserNotFound } - var accountDataParam any + var accountDataJSON []byte if identity.Account != nil { - b, err := json.Marshal(identity.Account) + accountDataJSON, err = json.Marshal(identity.Account) if err != nil { return fmt.Errorf("failed to marshal account data: %w", err) } - accountDataParam = string(b) } - var actionsParam any + + var actionsJSON []byte if identity.Actions != nil { - b, err := json.Marshal(identity.Actions) + actionsJSON, err = json.Marshal(identity.Actions) if err != nil { return fmt.Errorf("failed to marshal actions: %w", err) } - actionsParam = string(b) } - var policyNamesParam any + + var policyNamesJSON []byte if identity.PolicyNames != nil { - b, err := json.Marshal(identity.PolicyNames) + policyNamesJSON, err = json.Marshal(identity.PolicyNames) if err != nil { return fmt.Errorf("failed to marshal policy names: %w", err) } - policyNamesParam = string(b) } + _, err = tx.ExecContext(ctx, "UPDATE users SET email = $2, account_data = $3, actions = $4, policy_names = $5, updated_at = CURRENT_TIMESTAMP WHERE username = $1", - username, "", accountDataParam, actionsParam, policyNamesParam) + username, "", jsonbParam(accountDataJSON), jsonbParam(actionsJSON), jsonbParam(policyNamesJSON)) if err != nil { glog.Errorf("credential postgres: UpdateUser failed user=%s: %v", username, err) return fmt.Errorf("failed to update user: %w", err) @@ -398,7 +432,7 @@ func (store *PostgresStore) UpdateUser(ctx context.Context, username string, ide if err := tx.Commit(); err != nil { glog.Errorf("credential postgres: UpdateUser commit failed user=%s: %v", username, err) - return fmt.Errorf("failed to commit: %w", err) + return fmt.Errorf("failed to commit update user %s: %w", username, err) } glog.V(0).Infof("credential postgres: UpdateUser user=%s credentials=%d", username, len(identity.Credentials)) @@ -430,6 +464,81 @@ func (store *PostgresStore) DeleteUser(ctx context.Context, username string) err return nil } +// RenameUser atomically renames oldName to newName, re-pointing every +// FK-backed dependent row in a single transaction. The schema's foreign +// keys (credentials.username, user_inline_policies.username) reference +// users.username with ON DELETE CASCADE and the default ON UPDATE +// NO ACTION, which means a plain UPDATE users SET username = newName +// would fail because the children still reference the old name. Instead: +// 1. Insert the new users row by copying every non-key column from old. +// 2. Re-point the dependent tables (credentials, user_inline_policies) +// to the new username — both rows now exist so the FK is satisfied. +// 3. Delete the old users row, which has no remaining dependents. +// +// Implements credential.UserRenamer. +func (store *PostgresStore) RenameUser(ctx context.Context, oldName, newName string) error { + if !store.configured { + return fmt.Errorf("store not configured") + } + if oldName == newName { + return nil + } + + tx, err := store.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("failed to begin transaction: %w", err) + } + defer tx.Rollback() + + // Refuse if oldName is missing or newName already exists, so the + // caller surfaces a recognisable error instead of a constraint + // violation midway through. + var oldExists, newExists bool + if err := tx.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM users WHERE username = $1)", oldName).Scan(&oldExists); err != nil { + return fmt.Errorf("failed to check old user %s: %w", oldName, err) + } + if !oldExists { + return credential.ErrUserNotFound + } + if err := tx.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM users WHERE username = $1)", newName).Scan(&newExists); err != nil { + return fmt.Errorf("failed to check new user %s: %w", newName, err) + } + if newExists { + return credential.ErrUserAlreadyExists + } + + if _, err := tx.ExecContext(ctx, + `INSERT INTO users (username, email, account_data, actions, policy_names, created_at, updated_at) + SELECT $1, email, account_data, actions, policy_names, created_at, CURRENT_TIMESTAMP + FROM users WHERE username = $2`, + newName, oldName); err != nil { + return fmt.Errorf("failed to insert renamed user %s: %w", newName, err) + } + + if _, err := tx.ExecContext(ctx, + "UPDATE credentials SET username = $1 WHERE username = $2", newName, oldName); err != nil { + return fmt.Errorf("failed to re-point credentials to %s: %w", newName, err) + } + + if _, err := tx.ExecContext(ctx, + "UPDATE user_inline_policies SET username = $1 WHERE username = $2", newName, oldName); err != nil { + return fmt.Errorf("failed to re-point inline policies to %s: %w", newName, err) + } + + if _, err := tx.ExecContext(ctx, + "DELETE FROM users WHERE username = $1", oldName); err != nil { + return fmt.Errorf("failed to drop old user %s: %w", oldName, err) + } + + if err := tx.Commit(); err != nil { + glog.Errorf("credential postgres: RenameUser commit failed %s -> %s: %v", oldName, newName, err) + return fmt.Errorf("failed to commit rename %s -> %s: %w", oldName, newName, err) + } + + glog.V(0).Infof("credential postgres: RenameUser %s -> %s", oldName, newName) + return nil +} + func (store *PostgresStore) ListUsers(ctx context.Context) ([]string, error) { if !store.configured { return nil, fmt.Errorf("store not configured") @@ -567,7 +676,7 @@ func (store *PostgresStore) AttachUserPolicy(ctx context.Context, username strin return err } - glog.V(0).Infof("credential postgres: AttachUserPolicy user=%s policy=%s", username, policyName) + glog.V(1).Infof("credential postgres: AttachUserPolicy user=%s policy=%s", username, policyName) return nil } @@ -600,7 +709,7 @@ func (store *PostgresStore) DetachUserPolicy(ctx context.Context, username strin return err } - glog.V(0).Infof("credential postgres: DetachUserPolicy user=%s policy=%s", username, policyName) + glog.V(1).Infof("credential postgres: DetachUserPolicy user=%s policy=%s", username, policyName) return nil } diff --git a/weed/credential/postgres/postgres_inline_policy.go b/weed/credential/postgres/postgres_inline_policy.go index 8e47f9a4b..db676c79f 100644 --- a/weed/credential/postgres/postgres_inline_policy.go +++ b/weed/credential/postgres/postgres_inline_policy.go @@ -26,7 +26,7 @@ func (store *PostgresStore) PutUserInlinePolicy(ctx context.Context, userName, p VALUES ($1, $2, $3) ON CONFLICT (username, policy_name) DO UPDATE SET document = $3, updated_at = CURRENT_TIMESTAMP`, - userName, policyName, string(docJSON)) + userName, policyName, jsonbParam(docJSON)) if err != nil { glog.Errorf("credential postgres: PutUserInlinePolicy failed user=%s policy=%s: %v", userName, policyName, err) return fmt.Errorf("failed to upsert inline policy: %w", err) @@ -111,7 +111,6 @@ func (store *PostgresStore) ListUserInlinePolicies(ctx context.Context, userName return names, nil } - func (store *PostgresStore) LoadInlinePolicies(ctx context.Context) (map[string]map[string]policy_engine.PolicyDocument, error) { if !store.configured { return nil, fmt.Errorf("store not configured") diff --git a/weed/credential/postgres/postgres_policy.go b/weed/credential/postgres/postgres_policy.go index 68fb6e044..e1382c455 100644 --- a/weed/credential/postgres/postgres_policy.go +++ b/weed/credential/postgres/postgres_policy.go @@ -78,7 +78,7 @@ func (store *PostgresStore) CreatePolicy(ctx context.Context, name string, docum _, err = store.db.ExecContext(ctx, "INSERT INTO policies (name, document) VALUES ($1, $2) ON CONFLICT (name) DO UPDATE SET document = $2, updated_at = CURRENT_TIMESTAMP", - name, documentJSON) + name, jsonbParam(documentJSON)) if err != nil { return fmt.Errorf("failed to insert policy: %w", err) } @@ -104,7 +104,7 @@ func (store *PostgresStore) UpdatePolicy(ctx context.Context, name string, docum result, err := store.db.ExecContext(ctx, "UPDATE policies SET document = $2, updated_at = CURRENT_TIMESTAMP WHERE name = $1", - name, documentJSON) + name, jsonbParam(documentJSON)) if err != nil { return fmt.Errorf("failed to update policy: %w", err) } diff --git a/weed/credential/postgres/postgres_service_account.go b/weed/credential/postgres/postgres_service_account.go index 14bf4d50f..8cd23a448 100644 --- a/weed/credential/postgres/postgres_service_account.go +++ b/weed/credential/postgres/postgres_service_account.go @@ -33,7 +33,7 @@ func (store *PostgresStore) CreateServiceAccount(ctx context.Context, sa *iam_pb _, err = store.db.ExecContext(ctx, "INSERT INTO service_accounts (id, access_key, content) VALUES ($1, $2, $3)", - sa.Id, accessKey, data) + sa.Id, accessKey, jsonbParam(data)) if err != nil { return fmt.Errorf("failed to insert service account: %w", err) } @@ -63,7 +63,7 @@ func (store *PostgresStore) UpdateServiceAccount(ctx context.Context, id string, result, err := store.db.ExecContext(ctx, "UPDATE service_accounts SET access_key = $2, content = $3, updated_at = CURRENT_TIMESTAMP WHERE id = $1", - id, accessKey, data) + id, accessKey, jsonbParam(data)) if err != nil { return fmt.Errorf("failed to update service account: %w", err) } diff --git a/weed/credential/postgres/postgres_store.go b/weed/credential/postgres/postgres_store.go index 0f7a43f28..f62e19143 100644 --- a/weed/credential/postgres/postgres_store.go +++ b/weed/credential/postgres/postgres_store.go @@ -3,15 +3,31 @@ package postgres import ( "database/sql" "fmt" - "time" "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/util" - - _ "github.com/jackc/pgx/v5/stdlib" + "github.com/seaweedfs/seaweedfs/weed/util/pgxutil" ) +const ( + defaultMaxOpenConns = 25 + defaultMaxIdleConns = 5 + defaultConnMaxLifetimeSecs = 300 +) + +// jsonbParam adapts JSON bytes for an ExecContext call against a JSONB +// column. Returns nil (so the driver writes SQL NULL) when b is nil or +// empty; otherwise returns string(b) so pgx drives it as JSONB text — []byte +// would be encoded as bytea under simple_protocol (pgbouncer mode) and +// rejected by Postgres with "invalid input syntax for type json". +func jsonbParam(b []byte) interface{} { + if len(b) == 0 { + return nil + } + return string(b) +} + func init() { credential.Stores = append(credential.Stores, &PostgresStore{}) } @@ -41,9 +57,12 @@ func (store *PostgresStore) Initialize(configuration util.Configuration, prefix sslcert := configuration.GetString(prefix + "sslcert") sslkey := configuration.GetString(prefix + "sslkey") sslrootcert := configuration.GetString(prefix + "sslrootcert") + sslcrl := configuration.GetString(prefix + "sslcrl") pgbouncerCompatible := configuration.GetBool(prefix + "pgbouncer_compatible") + maxIdle := configuration.GetInt(prefix + "connection_max_idle") + maxOpen := configuration.GetInt(prefix + "connection_max_open") + maxLifetimeSeconds := configuration.GetInt(prefix + "connection_max_lifetime_seconds") - // Set defaults if hostname == "" { hostname = "localhost" } @@ -53,50 +72,47 @@ func (store *PostgresStore) Initialize(configuration util.Configuration, prefix if sslmode == "" { sslmode = "disable" } + if maxOpen == 0 { + maxOpen = defaultMaxOpenConns + } + if maxIdle == 0 { + maxIdle = defaultMaxIdleConns + } + if maxLifetimeSeconds == 0 { + maxLifetimeSeconds = defaultConnMaxLifetimeSecs + } glog.V(0).Infof("credential postgres: initializing store host=%s port=%d user=%s db=%s sslmode=%s pgbouncer=%v", hostname, port, username, database, sslmode, pgbouncerCompatible) - connStr := fmt.Sprintf("host=%s port=%d user=%s password=%s dbname=%s sslmode=%s", - hostname, port, username, password, database, sslmode) - if schema != "" { - connStr += fmt.Sprintf(" search_path=%s", schema) - } - if sslcert != "" { - connStr += fmt.Sprintf(" sslcert=%s", sslcert) - } - if sslkey != "" { - connStr += fmt.Sprintf(" sslkey=%s", sslkey) - } - if sslrootcert != "" { - connStr += fmt.Sprintf(" sslrootcert=%s", sslrootcert) - } - if pgbouncerCompatible { - connStr += " default_query_exec_mode=simple_protocol" - } + dsn, adaptedDSN := pgxutil.BuildDSN(pgxutil.DSNOptions{ + Hostname: hostname, + Port: port, + User: username, + Password: password, + Database: database, + Schema: schema, + SSLMode: sslmode, + SSLCert: sslcert, + SSLKey: sslkey, + SSLRootCert: sslrootcert, + SSLCRL: sslcrl, + PgBouncerCompatible: pgbouncerCompatible, + }) - db, err := sql.Open("pgx", connStr) + db, err := pgxutil.OpenDB(dsn, adaptedDSN, pgbouncerCompatible, maxIdle, maxOpen, maxLifetimeSeconds) if err != nil { glog.Errorf("credential postgres: failed to open database: %v", err) return fmt.Errorf("failed to open database: %w", err) } - if err := db.Ping(); err != nil { - db.Close() - glog.Errorf("credential postgres: failed to ping database: %v", err) - return fmt.Errorf("failed to ping database: %w", err) - } - glog.V(0).Infof("credential postgres: connection established") - db.SetMaxOpenConns(25) - db.SetMaxIdleConns(5) - db.SetConnMaxLifetime(5 * time.Minute) - store.db = db if err := store.createTables(); err != nil { db.Close() + store.db = nil glog.Errorf("credential postgres: failed to create tables: %v", err) return fmt.Errorf("failed to create tables: %w", err) } diff --git a/weed/credential/test/inline_policy_test.go b/weed/credential/test/inline_policy_test.go index f5fa3f2ab..791e67c0d 100644 --- a/weed/credential/test/inline_policy_test.go +++ b/weed/credential/test/inline_policy_test.go @@ -6,6 +6,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/credential/memory" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" _ "github.com/seaweedfs/seaweedfs/weed/credential/filer_etc" @@ -168,3 +169,98 @@ func TestInlinePolicyOperations(t *testing.T) { t.Errorf("Expected empty LoadInlinePolicies after cleanup, got %d users", len(allAfter)) } } + +// TestMemoryRenameUserMovesIdentityAndPolicies exercises the new +// credential.UserRenamer path on the memory store: the identity row, +// its access keys, and any inline policies all have to land under the +// new name in a single operation. +func TestMemoryRenameUserMovesIdentityAndPolicies(t *testing.T) { + ctx := context.Background() + + cm, err := credential.NewCredentialManager(credential.StoreTypeMemory, nil, "") + if err != nil { + t.Fatalf("Failed to create credential manager: %v", err) + } + + const ( + oldName = "renameme" + newName = "renamed" + policyName = "ReadBucket" + ) + + cred := &iam_pb.Credential{AccessKey: "AKIA-RENAME", SecretKey: "secret"} + if err := cm.CreateUser(ctx, &iam_pb.Identity{ + Name: oldName, + Credentials: []*iam_pb.Credential{cred}, + }); err != nil { + t.Fatalf("CreateUser failed: %v", err) + } + doc := policy_engine.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy_engine.PolicyStatement{{ + Effect: policy_engine.PolicyEffectAllow, + Action: policy_engine.NewStringOrStringSlice("s3:GetObject"), + Resource: policy_engine.NewStringOrStringSlicePtr("arn:aws:s3:::bucket/*"), + }}, + } + if err := cm.PutUserInlinePolicy(ctx, oldName, policyName, doc); err != nil { + t.Fatalf("PutUserInlinePolicy failed: %v", err) + } + + supported, err := cm.RenameUser(ctx, oldName, newName) + if err != nil { + t.Fatalf("RenameUser failed: %v", err) + } + if !supported { + t.Fatal("memory store should implement credential.UserRenamer") + } + + if _, err := cm.GetUser(ctx, oldName); err == nil { + t.Errorf("expected old user %q to be gone after rename", oldName) + } + got, err := cm.GetUser(ctx, newName) + if err != nil { + t.Fatalf("GetUser(%q) after rename failed: %v", newName, err) + } + if got.Name != newName { + t.Errorf("renamed identity name = %q, want %q", got.Name, newName) + } + if len(got.Credentials) != 1 || got.Credentials[0].AccessKey != cred.AccessKey { + t.Errorf("renamed identity should still own its access key, got %+v", got.Credentials) + } + + gotByKey, err := cm.GetUserByAccessKey(ctx, cred.AccessKey) + if err != nil { + t.Fatalf("GetUserByAccessKey failed: %v", err) + } + if gotByKey == nil || gotByKey.Name != newName { + t.Errorf("access key should resolve to %q, got %+v", newName, gotByKey) + } + + gotPolicy, err := cm.GetUserInlinePolicy(ctx, newName, policyName) + if err != nil { + t.Fatalf("GetUserInlinePolicy under new name failed: %v", err) + } + if gotPolicy == nil { + t.Fatal("inline policy should be readable under the new name") + } + if gotPolicy.Statement[0].Effect != policy_engine.PolicyEffectAllow { + t.Errorf("policy effect: got %q want %q", gotPolicy.Statement[0].Effect, policy_engine.PolicyEffectAllow) + } + stalePolicy, err := cm.GetUserInlinePolicy(ctx, oldName, policyName) + if err != nil { + t.Fatalf("GetUserInlinePolicy under old name failed: %v", err) + } + if stalePolicy != nil { + t.Errorf("inline policy should be gone from the old name, got %+v", stalePolicy) + } + + // Clean up — NewCredentialManager hands out a shared MemoryStore singleton + // so leftover state would leak into TestMemoryStoreIntegration. + if err := cm.DeleteUserInlinePolicy(ctx, newName, policyName); err != nil { + t.Fatalf("DeleteUserInlinePolicy cleanup failed: %v", err) + } + if err := cm.DeleteUser(ctx, newName); err != nil { + t.Fatalf("DeleteUser cleanup failed: %v", err) + } +} diff --git a/weed/filer/postgres/pgx_conn.go b/weed/filer/postgres/pgx_conn.go index d53c3811d..81b6be0af 100644 --- a/weed/filer/postgres/pgx_conn.go +++ b/weed/filer/postgres/pgx_conn.go @@ -2,49 +2,13 @@ package postgres import ( "database/sql" - "fmt" - "time" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/stdlib" + "github.com/seaweedfs/seaweedfs/weed/util/pgxutil" ) -// OpenPGXDB parses the given DSN into a pgx ConnConfig, applies PgBouncer -// compatibility settings when requested, opens a *sql.DB via -// stdlib.OpenDB, and verifies it with Ping. -// -// In pgx/v5 the prefer_simple_protocol DSN parameter was removed, so simple -// protocol mode must be configured on the ConnConfig via -// DefaultQueryExecMode. We use stdlib.OpenDB(config) rather than -// RegisterConnConfig + sql.Open so we don't leak entries in stdlib's global -// connection config map on either success or failure paths. -// -// adaptedSqlUrl is used only for error messages (the caller is expected to -// have redacted any password). +// OpenPGXDB is a thin alias kept for callers (notably the postgres2 filer +// store) that already depend on this entry point. New code should use +// pgxutil.OpenDB directly. func OpenPGXDB(sqlUrl, adaptedSqlUrl string, pgbouncerCompatible bool, maxIdle, maxOpen, maxLifetimeSeconds int) (*sql.DB, error) { - connConfig, parseErr := pgx.ParseConfig(sqlUrl) - if parseErr != nil { - return nil, fmt.Errorf("can not parse connection config for %s error:%v", adaptedSqlUrl, parseErr) - } - - // PgBouncer compatibility: use the simple query protocol and disable - // statement caching. This avoids prepared statement issues with - // PgBouncer's transaction pooling mode. - if pgbouncerCompatible { - connConfig.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol - connConfig.StatementCacheCapacity = 0 - connConfig.DescriptionCacheCapacity = 0 - } - - db := stdlib.OpenDB(*connConfig) - db.SetMaxIdleConns(maxIdle) - db.SetMaxOpenConns(maxOpen) - db.SetConnMaxLifetime(time.Duration(maxLifetimeSeconds) * time.Second) - - if err := db.Ping(); err != nil { - db.Close() - return nil, fmt.Errorf("connect to %s error:%v", adaptedSqlUrl, err) - } - - return db, nil + return pgxutil.OpenDB(sqlUrl, adaptedSqlUrl, pgbouncerCompatible, maxIdle, maxOpen, maxLifetimeSeconds) } diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index 7e02f39b8..69c666714 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -353,7 +353,7 @@ func (e *EmbeddedIamApi) GetUser(s3cfg *iam_pb.S3ApiConfiguration, userName stri } // UpdateUser updates an IAM user. -func (e *EmbeddedIamApi) UpdateUser(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (*iamUpdateUserResponse, *iamError) { +func (e *EmbeddedIamApi) UpdateUser(ctx context.Context, s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (*iamUpdateUserResponse, *iamError) { resp := &iamUpdateUserResponse{} userName := values.Get("UserName") newUserName := values.Get("NewUserName") @@ -385,6 +385,44 @@ func (e *EmbeddedIamApi) UpdateUser(s3cfg *iam_pb.S3ApiConfiguration, values url } } + // Migrate the credential store's view of this user before the rename + // takes effect. Stores with referential integrity (Postgres) MUST + // implement UserRenamer: per-row Put / Get / Delete migration would + // violate the FK on user_inline_policies(username) at statement time + // because the new users row does not exist yet. UserRenamer does the + // rename, credential re-pointing and inline-policy re-pointing inside + // a single transaction. + // + // For stores that don't have FK enforcement (memory, filer_etc) the + // renamer is also implemented as a plain rebinding so the regression + // test exercises the same code path. + if e.credentialManager != nil { + switch supported, err := e.credentialManager.RenameUser(ctx, userName, newUserName); { + case err != nil: + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("rename user %s -> %s: %w", userName, newUserName, err)} + case !supported: + // Fallback: copy the inline policies under the new name and + // let the subsequent SaveConfiguration prune CASCADE the old + // rows. Only safe for stores without FK enforcement. + policyNames, err := e.credentialManager.ListUserInlinePolicies(ctx, userName) + if err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("list inline policies for %s: %w", userName, err)} + } + for _, policyName := range policyNames { + doc, err := e.credentialManager.GetUserInlinePolicy(ctx, userName, policyName) + if err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("read inline policy %s for %s: %w", policyName, userName, err)} + } + if doc == nil { + continue + } + if err := e.credentialManager.PutUserInlinePolicy(ctx, newUserName, policyName, *doc); err != nil { + return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("copy inline policy %s to %s: %w", policyName, newUserName, err)} + } + } + } + } + sourceIdent.Name = newUserName // Update group membership references for _, g := range s3cfg.Groups { @@ -2167,7 +2205,7 @@ func (e *EmbeddedIamApi) ExecuteAction(ctx context.Context, values url.Values, s changed = false case "UpdateUser": var iamErr *iamError - response, iamErr = e.UpdateUser(s3cfg, values) + response, iamErr = e.UpdateUser(ctx, s3cfg, values) if iamErr != nil { return nil, iamErr } diff --git a/weed/s3api/s3api_embedded_iam_test.go b/weed/s3api/s3api_embedded_iam_test.go index 4d2412dee..925f663f1 100644 --- a/weed/s3api/s3api_embedded_iam_test.go +++ b/weed/s3api/s3api_embedded_iam_test.go @@ -862,6 +862,46 @@ func TestEmbeddedIamUpdateUser(t *testing.T) { assert.Equal(t, http.StatusOK, response.Code) } +// TestEmbeddedIamUpdateUserPreservesInlinePolicies makes sure renaming a +// user keeps that user's stored inline policies under the new name. +// Without the explicit policy migration in UpdateUser, SaveConfiguration +// prunes the old name and CASCADE drops the policies. +func TestEmbeddedIamUpdateUserPreservesInlinePolicies(t *testing.T) { + api := NewEmbeddedIamApiForTest() + const ( + oldName = "RenameMe" + newName = "Renamed" + policyName = "ReadBucket" + ) + api.mockConfig = &iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{Name: oldName}}, + } + doc := policy_engine.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy_engine.PolicyStatement{{ + Effect: policy_engine.PolicyEffectAllow, + Action: policy_engine.NewStringOrStringSlice("s3:GetObject"), + Resource: policy_engine.NewStringOrStringSlicePtr("arn:aws:s3:::bucket/*"), + }}, + } + require.NoError(t, api.credentialManager.PutUserInlinePolicy(context.Background(), oldName, policyName, doc)) + + params := &iam.UpdateUserInput{NewUserName: aws.String(newName), UserName: aws.String(oldName)} + req, _ := iam.New(session.New()).UpdateUserRequest(params) + _ = req.Build() + out := iamUpdateUserResponse{} + response, err := executeEmbeddedIamRequest(api, req.HTTPRequest, &out) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, response.Code) + + got, err := api.credentialManager.GetUserInlinePolicy(context.Background(), newName, policyName) + require.NoError(t, err) + require.NotNil(t, got, "inline policy should have been migrated to the new username") + assert.Equal(t, doc.Version, got.Version) + require.Len(t, got.Statement, 1) + assert.Equal(t, policy_engine.PolicyEffectAllow, got.Statement[0].Effect) +} + // TestEmbeddedIamDeleteUser tests deleting a user func TestEmbeddedIamDeleteUser(t *testing.T) { api := NewEmbeddedIamApiForTest() diff --git a/weed/util/pgxutil/pgx_conn.go b/weed/util/pgxutil/pgx_conn.go new file mode 100644 index 000000000..ca34693c4 --- /dev/null +++ b/weed/util/pgxutil/pgx_conn.go @@ -0,0 +1,158 @@ +// Package pgxutil holds shared helpers for opening *sql.DB handles backed +// by jackc/pgx, used by the postgres filer and credential stores so they +// stay consistent on connection setup, mTLS handling and PgBouncer +// compatibility. +package pgxutil + +import ( + "database/sql" + "fmt" + "strconv" + "strings" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/stdlib" +) + +// DSNOptions describes the parameters used to assemble a libpq-style +// keyword=value connection string. Empty fields are omitted. +type DSNOptions struct { + Hostname string + Port int + User string + Password string + Database string + Schema string + SSLMode string + SSLCert string + SSLKey string + SSLRootCert string + SSLCRL string + // PgBouncerCompatible toggles two things at the caller's site: the + // search_path is omitted from the DSN (PgBouncer rejects it under + // transaction pooling) and OpenDB is told to use the simple query + // protocol. + PgBouncerCompatible bool +} + +// quoteDSNValue wraps v per the libpq keyword/value rules: empty values +// and values containing whitespace, single quotes, or backslashes are +// single-quoted with internal `'` and `\` escaped. Plain alphanumeric +// values are returned unchanged. See PostgreSQL docs §"Connection Strings". +func quoteDSNValue(v string) string { + if v == "" { + return "''" + } + needsQuote := false + for i := 0; i < len(v); i++ { + c := v[i] + if c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\'' || c == '\\' { + needsQuote = true + break + } + } + if !needsQuote { + return v + } + var b strings.Builder + b.Grow(len(v) + 2) + b.WriteByte('\'') + for i := 0; i < len(v); i++ { + c := v[i] + if c == '\'' || c == '\\' { + b.WriteByte('\\') + } + b.WriteByte(c) + } + b.WriteByte('\'') + return b.String() +} + +// BuildDSN assembles two libpq-style connection strings from opts: the +// real DSN passed to pgx, and an adapted copy with the password redacted +// for use in error messages and logs. Values containing spaces, single +// quotes, or backslashes are quoted per libpq rules so passwords and +// cert paths sourced from secret managers can carry arbitrary characters +// without breaking pgx.ParseConfig. +func BuildDSN(opts DSNOptions) (dsn, adaptedDSN string) { + dsn = "connect_timeout=30" + if opts.Hostname != "" { + dsn += " host=" + quoteDSNValue(opts.Hostname) + } + if opts.Port != 0 { + dsn += " port=" + strconv.Itoa(opts.Port) + } + if opts.SSLMode != "" { + dsn += " sslmode=" + quoteDSNValue(opts.SSLMode) + } + if opts.SSLCert != "" { + dsn += " sslcert=" + quoteDSNValue(opts.SSLCert) + } + if opts.SSLKey != "" { + dsn += " sslkey=" + quoteDSNValue(opts.SSLKey) + } + if opts.SSLRootCert != "" { + dsn += " sslrootcert=" + quoteDSNValue(opts.SSLRootCert) + } + if opts.SSLCRL != "" { + dsn += " sslcrl=" + quoteDSNValue(opts.SSLCRL) + } + if opts.User != "" { + dsn += " user=" + quoteDSNValue(opts.User) + } + adaptedDSN = dsn + if opts.Password != "" { + dsn += " password=" + quoteDSNValue(opts.Password) + adaptedDSN += " password=ADAPTED" + } + if opts.Database != "" { + dsn += " dbname=" + quoteDSNValue(opts.Database) + adaptedDSN += " dbname=" + quoteDSNValue(opts.Database) + } + if opts.Schema != "" && !opts.PgBouncerCompatible { + dsn += " search_path=" + quoteDSNValue(opts.Schema) + adaptedDSN += " search_path=" + quoteDSNValue(opts.Schema) + } + return dsn, adaptedDSN +} + +// OpenDB parses dsn into a pgx ConnConfig, applies PgBouncer compatibility +// settings when requested, opens a *sql.DB via stdlib.OpenDB, and verifies +// it with Ping. +// +// In pgx/v5 the prefer_simple_protocol DSN parameter was removed, so simple +// protocol mode must be configured on the ConnConfig via DefaultQueryExecMode. +// We use stdlib.OpenDB(config) rather than RegisterConnConfig + sql.Open so +// we don't leak entries in stdlib's global connection config map on either +// success or failure paths. +// +// adaptedDSN is used only for error messages (the caller is expected to +// have redacted any password). +func OpenDB(dsn, adaptedDSN string, pgbouncerCompatible bool, maxIdle, maxOpen, maxLifetimeSeconds int) (*sql.DB, error) { + connConfig, parseErr := pgx.ParseConfig(dsn) + if parseErr != nil { + return nil, fmt.Errorf("can not parse connection config for %s error:%v", adaptedDSN, parseErr) + } + + // PgBouncer compatibility: use the simple query protocol and disable + // statement caching. This avoids prepared statement issues with + // PgBouncer's transaction pooling mode. + if pgbouncerCompatible { + connConfig.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol + connConfig.StatementCacheCapacity = 0 + connConfig.DescriptionCacheCapacity = 0 + } + + db := stdlib.OpenDB(*connConfig) + db.SetMaxIdleConns(maxIdle) + db.SetMaxOpenConns(maxOpen) + db.SetConnMaxLifetime(time.Duration(maxLifetimeSeconds) * time.Second) + + if err := db.Ping(); err != nil { + db.Close() + return nil, fmt.Errorf("connect to %s error:%v", adaptedDSN, err) + } + + return db, nil +} diff --git a/weed/util/pgxutil/pgx_conn_test.go b/weed/util/pgxutil/pgx_conn_test.go new file mode 100644 index 000000000..9a1ef0b64 --- /dev/null +++ b/weed/util/pgxutil/pgx_conn_test.go @@ -0,0 +1,59 @@ +package pgxutil + +import ( + "strings" + "testing" + + "github.com/jackc/pgx/v5" +) + +func TestQuoteDSNValue(t *testing.T) { + cases := []struct { + in, want string + }{ + {"", "''"}, + {"plain", "plain"}, + {"with space", "'with space'"}, + {"it's", `'it\'s'`}, + {`back\slash`, `'back\\slash'`}, + {`mix it's \and spaces`, `'mix it\'s \\and spaces'`}, + } + for _, c := range cases { + if got := quoteDSNValue(c.in); got != c.want { + t.Errorf("quoteDSNValue(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// TestBuildDSN_QuotesProblematicValues verifies that the assembled DSN +// stays parseable by pgx when values contain spaces, single quotes or +// backslashes — the kind of password/cert paths a secret manager can +// hand us in production. +func TestBuildDSN_QuotesProblematicValues(t *testing.T) { + opts := DSNOptions{ + Hostname: "db.example.com", + Port: 5432, + User: "iam user", + Password: `p@ss it's word\\`, + Database: "weed", + Schema: "public", + SSLMode: "disable", + } + dsn, adapted := BuildDSN(opts) + if strings.Contains(adapted, opts.Password) { + t.Errorf("adapted DSN must redact the password, got %q", adapted) + } + cfg, err := pgx.ParseConfig(dsn) + if err != nil { + t.Fatalf("pgx.ParseConfig rejected the assembled DSN: %v\nDSN: %s", err, dsn) + } + if cfg.User != opts.User { + t.Errorf("User: got %q want %q", cfg.User, opts.User) + } + if cfg.Password != opts.Password { + t.Errorf("Password: got %q want %q", cfg.Password, opts.Password) + } + if cfg.Database != opts.Database { + t.Errorf("Database: got %q want %q", cfg.Database, opts.Database) + } +}