Repository navigation
fix(management): refuse access key import unless allow_credential_import is true - #840
Merged
Merged
Conversation
…ort is true An access key import through the management API, and the import-access-key command that calls it, now runs only when the runtime setting allow_credential_import is true. When the catalog has no row for the setting, the server refuses the import with 403, as it does for false. Before this change the server allowed the import when the row was missing. No release writes the row, so every deployment that never set the setting accepted imports. The console settings page already listed the default as false. An import stores an access key ID and secret that the caller chooses. With the feature off by default, only deployments that turn it on accept caller-chosen keys. The setting does not affect keys imported earlier, or the credentials a dev-mode server seeds at startup. The check moves into credential_import_gate. Unit tests cover no row, false, true, other values and a store error. A store error still answers 500, and the refusal message does not change. devtools/run-tests sets allow_credential_import to true with the other runtime settings, before any suite starts. The integration jobs that import keys keep passing, and no test changes the setting while xdist workers run. tests/cli/test-cli-comprehensive.sh enables it before its import step, and the manual test workflows in AGENTS.md and the developer test guide set it before pytest runs. The settings tables, getting-started.md, the admin and deployment guides and the troubleshooting guide state the new default. The upgrade manual tells a deployment that imports keys to run extenddb settings --config extenddb.toml set allow_credential_import true before it upgrades. Signed-off-by: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com>
…port row refuses an access key import The unit tests for credential_import_gate call the function directly, and every integration suite runs with allow_credential_import set to true, because devtools/run-tests stores it before the suites start. Without this test, no test fails if import_access_key stops calling the gate or calls it after the store write. tests/test_allow_credential_import_sqlite.py runs extenddb init --backend sqlite in a temporary directory and serves that deployment on a free port on 127.0.0.1. Its catalog has no allow_credential_import row. As the admin that init created, the test creates an account and a user and posts an access key import for the user. The server must answer 403 with the body "Credential import is disabled (allow_credential_import = false)", and the user's key list must not show the key. The test then runs extenddb settings --config extenddb.toml set allow_credential_import true while the server runs, posts the same import, and expects 201 and the key in the list. After it sets the value to false, a second import gets 403 and the first key still lists. The test uses its own deployment so that it does not change the shared server's settings while other xdist workers import keys. It stops the server and deletes the directory on teardown, also when an assertion fails. init writes its TLS certificate and key under ~/.extenddb, so the extenddb commands run with HOME set inside that directory. The module skips unless extenddb version reports the SQLite backend. In CI the run-integration-sqlite job runs it. The PostgreSQL, MongoDB and Cassandra jobs build their binaries with other backends, so it skips there. Signed-off-by: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com>
LeeroyHannigan
requested review from
amrith,
c33howard,
eliseong,
hitesh96db,
jcshepherd,
navareamol,
pdf-amzn and
robinnsc
as code owners
November 5, 2026 02:29
This resolves the conflict in the upgrade manual. Main added the v0.1.15 entry for IAM policy variables in condition values at the place where this branch adds the entry for credential import. The merge keeps both entries: main's entry first, then the entry of this branch. Signed-off-by: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com>
hitesh96db
approved these changes
Nov 9, 2026
hitesh96db
left a comment
hitesh96db
left a comment
Contributor
There was a problem hiding this comment.
The fix is correct, the tests catch a regression, and I found no bugs. Approving. The inline comments are non-blocking cleanups.
What I checked
On this PR's head (93361fa5), with Rust 1.97.0 and a SQLite release build:
- The gate is the only way in. The management route is the only server path that imports a key. Dev-mode seeding writes to the store directly, and
extenddb manage import-access-keycalls the route. Nothing writes theallow_credential_importrow exceptsettings set, so onmainevery deployment that never set it accepted imports, while the console showedfalse. - Placement. The gate runs after authentication and authorization, and before the body is validated and the key is stored. Only the stored value
trueallows the import, and a store error still answers 500. - Values.
validate_boolaccepts onlytrueorfalse, and the CLI and the settings API both use it.ops_settings.rsdoes not change, so the upgrade step (settings set allow_credential_import trueon the earlier release) holds. - Tests. The 5 new unit tests pass.
test_allow_credential_import_sqlite.pypassed in 3 of 3 runs locally, and it runs and passes in CI (run-integration-sqlite (3)), so it is not skipped. - Negative control. With
Ok(None) => Ok(())put back in the gate (the old behaviour), 1 of the 5 unit tests failed, and the pytest failed too. cargo fmt --checkandcargo clippy -p extenddb-server --all-targets -- -D warningsare clean.- Docs: every doc that gave the old default is updated.
Not checked
- PostgreSQL, MongoDB and Cassandra locally. CI covers them, and the gate does not depend on the backend.
tests/cli/test-cli-comprehensive.sh.
| Ok(Some(value)) if value == "true" => Ok(()), | ||
| Ok(_) => Err(( | ||
| StatusCode::FORBIDDEN, | ||
| "Credential import is disabled (allow_credential_import = false)", |
Contributor
There was a problem hiding this comment.
Nit (optional): make the refusal say what to do. With the default now false, the message is accurate even when there is no row. But it does not say how to turn import on. Something like Credential import is disabled. Set allow_credential_import to true to enable it would save a trip to the troubleshooting guide. I see that the PR keeps the old text on purpose, so this is your call. If you change it, the troubleshooting heading changes with it.
Collaborator
Author
There was a problem hiding this comment.
I kept the text in this PR, because the troubleshooting heading and the pytest match it. #919 lists the change for after this PR merges.
| } | ||
| } | ||
|
|
||
| fn refused() -> Result<(), (StatusCode, String)> { |
Contributor
There was a problem hiding this comment.
Nit: one constant for the refusal text. The string is written out in the gate, here in refused(), and as REFUSED in the pytest. A const CREDENTIAL_IMPORT_DISABLED: &str = ... next to credential_import_gate, used by the gate and by refused(), keeps the Rust copies from drifting apart.
Collaborator
Author
There was a problem hiding this comment.
#919 lists this for after this PR merges.
| REFUSED = "Credential import is disabled (allow_credential_import = false)" | ||
|
|
||
|
|
||
| def _free_port() -> int: |
Contributor
There was a problem hiding this comment.
Nit: these helpers are copies. _free_port, _wait_for_health and _patch_port are near line-for-line copies of the ones in test_gsi_async_queue_sqlite.py, and lifecycle_helpers.py has its own versions (_find_free_port, _wait_for_server, _patch_config_port). _free_port alone is now defined in 4 test files. Copying is already the pattern in tests/, so I'm not asking for it here. A shared helper for a scratch SQLite deployment (init, patch port, serve, wait for health, stop) would cut about 50 lines from this file and could be a follow-up.
Collaborator
Author
There was a problem hiding this comment.
Agreed. I left the copies here, and a shared helper for a scratch SQLite deployment can come in a later change.
| if "$BINARY" settings --config "$CONFIG_FOR_SETTINGS" set throttling_enabled true 2>/dev/null; then | ||
| echo " ✓ throttling_enabled set to true" | ||
| fi | ||
| # Access key import is refused unless this is true. Set once here, before |
Contributor
There was a problem hiding this comment.
Nit: make the failure clear outside this script. test_auth_permissions.py::test_import_access_key now passes only because this sets the value. Run with plain pytest tests/, it fails with assert 403 in (200, 201) and the refusal text, which doesn't point here. The AGENTS.md and test-guide lines help, but a message in that assert, for example run via devtools/run-tests, or set allow_credential_import to true, would point the next person here directly.
Collaborator
Author
There was a problem hiding this comment.
#919 lists this for after this PR merges.
| | `log_level` | `info` | Log level: trace, debug, info, warn, error | | ||
| | `control_plane_delay_seconds` | `0.25` | Delay for table status transitions, in seconds (0 = instant). PostgreSQL and SQLite write 0.25 into the catalog when it is created; MongoDB and Cassandra use 0.25 when the setting is absent. On PostgreSQL and SQLite a value below 1 makes DeleteTable remove the table in the request; on PostgreSQL, if a session holds a lock on the table's data (a reader, an open write transaction, an index build), DeleteTable returns `DELETING` instead (after at most about a second), the drop finishes in the background within about a second of that session's release, and a CreateTable of the same name is refused until then. | | ||
| | `allow_credential_import` | `true` | Whether `import-access-key` is allowed | | ||
| | `allow_credential_import` | `false` | Whether `import-access-key` is allowed. Only `true` enables it | |
Contributor
There was a problem hiding this comment.
Follow-up, not this PR: the CLI does not show this default. On a fresh SQLite catalog from this branch, extenddb settings get allow_credential_import prints Setting 'allow_credential_import' not found. and exits 1, and extenddb settings list leaves the key out. The console shows false. The server default lives only in the console's RUNTIME_KEYS (settings_pages.rs), and the CLI falls back only to extenddb_storage::operations::setting_default, which has no entry for this key. This was the same before this PR, but now that the default matters, an operator checking whether import is off gets no answer from the CLI. One shared source for server defaults, read by both the CLI and the console, would fix it. A follow-up issue fits better than this PR.
LeeroyHannigan
added a commit
that referenced
this pull request
Nov 10, 2026
Main at 887b6e2 adds two commits since the previous merge. #842 lists what the audit log records. #840 refuses access key import unless allow_credential_import is true. Neither commit adds a migration, so attribute_name_escape stays at id 10. The merge has no conflicts. Both sides edited docs/manuals/07-upgrade-manual.md, docs/differences-from-dynamodb.md, docs/getting-started.md and docs/manuals/05-admin-guide.md. Main changed the text on credential import and audit logging, and added a v0.1.15 behavior change that needs no catalog migration. This branch changed the migration list, the sample outputs and the escape. The changes are in separate paragraphs and table rows. The Current Status section lists migrations 1 to 10. Each sample output and example still names migration 10, so the merge changes no line beyond main's.
7 of 8 tasks
This was referenced Nov 10, 2026
Open
robinnsc
added a commit
that referenced
this pull request
Nov 10, 2026
The integration workflow's PostgreSQL and SQLite jobs ran the wire suite with `--filter "not import_export"`. The filter came with the PostgreSQL job's first version (687e46f) and was copied into the SQLite job (556c501); neither commit gives a reason. A foreground server wrote a PID file in every tree up to e0d6a0a, which made foreground mode write none by default and added `--write-pid-file` to the job in the same commit; 556c501 came after e0d6a0a. So the restart `devtools/run-tests` makes for the import and export tests could stop the job's server in every tree, and the filter was not working around it. The MongoDB and Cassandra jobs already run `tests/test_import_export.py` unfiltered. The expression selects out that file's ten tests and nothing else. Run the job's way on release builds of this change they pass 10 of 10 on SQLite and on PostgreSQL, before and after its rebase onto main 887b6e2, so both filters go and no narrower one stays. Both lines already carry main's `--terraform` (#474); this change removes the filter only, so each reads `devtools/run-tests --extenddb --pytest --comprehensive --terraform --parallel`. `docs/design/15-backup-restore-bench-pr11.md` (row 15d of the design index) records the cells the testing summary left open, in the PR 9 record's method: release builds, a filesystem store, the server restarted before each phase, `VmHWM` sampled every 200 ms, on the 16-core host class of the earlier records. 1M items of 200 bytes restore in 33.8 s on MongoDB (its own backup) and 49.0 s on Cassandra (a SQLite backup adopted with `backup sync`), 52 and 33 MiB over idle. A 1M-item table with a 128-dimension vector index backs up in 243 s on PostgreSQL and 210 s on SQLite and restores in 246 s and 214 s, 111 MiB and 552 MiB over idle; SQLite's figure is higher than at 100k under glibc's default arenas and is 204 MiB, flat from 100k, under `MALLOC_ARENA_MAX=1`, where the backup took 222 s and the restore 283 s. The Cassandra cells ran on a node of their own with the Cassandra workflow's service settings. No cell is left open. Later runs on the same host, also in the record, reproduce the cells and show that SQLite's restore memory is its write batch and decode chunks, not the vector index: it grows with the item's size up to the 32 MiB batch bound whatever the attribute types (253 MiB, 130 MiB under one arena, for 100k items of 2.5 KB strings), and lists of numbers decode larger (397 to 424 MiB for the same 100k items of 128 numbers with no index, 301 to 316 MiB under one arena for 20,000 items of 1,536 numbers). Two restores at once under one arena took 57.5 s against 24.9 s. Five pairs of PostgreSQL runs with and without the vector index, each pair from one harness, put its share of the 100k restore at 27 to 53 MiB and 33 to 47% of the duration. A MongoDB restore becomes ACTIVE at the backfill worker's next 5 s cycle after its load, so it takes the load's time plus up to 5 s. These figures come from release builds of earlier versions of this change, on PR 10 at 358ff8b and main 94b5f68, before main's later changes to the backup and restore code (#700 and #740 as merged, and #474); the 100k cells, run again on release builds after the rebase onto main 887b6e2, reproduce in kind (peaks within 8.4%, durations up to 32% longer at two to four times the host load). `docs/manuals/14-backup-and-restore.md` is the operator's manual, derived from the design: the strategy (its first sentence: a filesystem store keeps the host's failure domain, S3 is the production recommendation), RPO and RTO per store kind with the three records' figures and host class (a key-ordered PostgreSQL backup also needs the text key columns at collation `C` or `POSIX`, and the server logs a warning when it streams an older table unordered; the vector index's share of the SQLite restore comes from two sets of runs on a lightly loaded host, and the record's loaded set is given and left out of the share), the configuration (pointing at the admin guide's rows and the store crate's README; an S3 store needs root certificates, `ca-certificates` or `SSL_CERT_FILE`; a split-root identity mismatch is cleared by no command in this release, since `backup purge --confirm`, plain `backup sync` and `--prune-missing` refuse an unhealthy store, and the manual and the deployment guide give the two remedies that exist, a catalog copy from before the split or one directory's rows deleted by hand and its backups copied into the kept directory), securing the store, what a running backup costs each database, a restore drill, catalog-loss recovery with `backup sync`, `--adopt-store` and the in-memory startup sync (the old key pairs imported once `allow_credential_import` is turned on, with `extenddb settings` or, on an in-memory server, the management API, and off again after), legacy backups and what to do with them while `migrate-legacy` (#821) is not shipped, what a restore preserves, and moving data to and from Amazon DynamoDB with ImportTable and export (a store-prefix import is #748). It states each deployment statement the design's PR 11 acceptance lists. Figures measured for it, with their method in the record's "Figures measured for the manual": a 25 s PostgreSQL backup beside 12,156 overwrites of 200-byte items grew the written table from 0.4 to 4.4 MiB, its dead row versions not removable by a `VACUUM` during the backup (0.5 MiB with no backup); on SQLite the WAL grows by the pages each write changes, from 4.0 to 61.1 MiB over 15,472 writes to a table with no index during a 22.8 s backup and from 4.1 to 1,294 MiB over 13,996 writes to a table with two GSIs during a 25.6 s backup (about 20 to 24 pages per write over four runs). The manual's process-level statements were checked on release builds of this change before its rebase onto main 887b6e2, SQLite and PostgreSQL: the root placement refusals and their texts, the drill, an off-host copy under account and table directories made 0700 (a `mkdir -p` under a 022 umask stops sync until `chmod -R` on the account directory), a re-created catalog's config file (`init --overwrite` regenerates every section from defaults and carries over only the storage settings, so a `port` or TLS path edited in a default section reverts too and the copy taken first goes back whole apart from those settings) and its store health with an explicit store and with SQLite's default root (unconfigured, with CreateBackup writing in-catalog backups until the adoption), the accounts re-created with their old ids and old key pairs imported, the adoption, the in-memory startup sync and, on the in-memory dev build, the adopted backups refused until their account is re-created, the in-catalog create's warning and the restore of its backup through a store-configured server, an S3 store at moto with the default spool, `backup sync` exiting 6 beside a SQLite server, ImportTable refusing a gzip file and both kinds of symbolic link and importing a decompressed backup data file, an off-host copy with loose modes refused by `backup sync`, and on PostgreSQL the claim session found by its lock key, whose termination stops the job at its next heartbeat and leaves the row to the reconciler, the registration session's termination failing the server's running backups, the wait of `--prune-missing` on a recently failed backup, a catalog restored alone beside a newer data database, and the catalog and data dumps restored into new databases. On PostgreSQL 16 containers the restore the admin guide now gives ran end to end, on the old server and on a new, empty one, and an access key was imported through the management API from a request body read from a file. Since main's #840 the server refuses an access key import unless `allow_credential_import` is `true`, and a re-created catalog has no value for it: on PostgreSQL and on an in-memory SQLite server, release builds of this tree answer the manual's import with 403 "Credential import is disabled (allow_credential_import = false)" and import the pair once the setting is on (`extenddb settings set` beside the running server; on the in-memory server, which `extenddb settings` cannot reach, `PUT /management/settings/allow_credential_import`), and the imported pairs keep signing requests after it is turned off. Release builds after the rebase repeated the root placement refusals, what a restore preserves, the drill on SQLite, PostgreSQL and an S3 store at moto, the off-host copy shapes, a catalog restored from dumps and the claim and registration sessions on PostgreSQL, a catalog lost and re-created on SQLite with `--adopt-store`, the in-memory startup sync, the legacy path, ImportTable and export, the split-root remedy by hand on SQLite, the admin guide's Data Recovery entry on PostgreSQL 16 and `init`'s database names. The admin guide's Data Recovery entry named the PostgreSQL data database `extenddb_catalog_data`; `init` names it after the catalog without its `_catalog` suffix (`extenddb` by default). The entry now lists each backend's databases, says a catalog name without the suffix needs `--data-db` and that the failed `init` leaves the catalog database to drop first, and treats the two PostgreSQL databases as one copy: both dumps with the server stopped, under `umask 077` since the catalog dump holds the encryption key and the application role's password, or a base backup; the restore goes into new, empty databases created with `createdb -O` by the admin role, since one over the existing databases restores nothing and one into databases the admin role owns leaves the server unable to create tables. The old databases are renamed aside until the restored server has read every table; the dumps use the server's major version of `pg_dump` and are checked non-empty, since an older one exits 1 leaving an empty file that `psql` restores without an error; and on a new server the application role is created first with the dump's password and, at another address, the catalog's recorded data-database address is set, or the server keeps using the old one. The SQLite row gives the restore with the current `-wal` and `-shm` removed. The security model and the deployment guide's checklist say the same, and the upgrade manual's dumps take `umask 077` and are checked (`test -s`), and its rollback renames both databases aside, restores them with `ON_ERROR_STOP` and drops the renamed copies once the previous binary has read every table; the checklist limits a base backup to both databases on one instance. The architecture, design, quickstart and security-model guides, AGENTS.md, the troubleshooting guide, the technical-debt register (whose A-1 entry now cites functions, not moved line numbers) and the storage-extension guide carry the right name, the quickstart's `init` table gives the PostgreSQL admin and application roles' flags with their defaults, and `init --help` states both database defaults and that only the PostgreSQL backend reads the two flags. The admin guide's `[backup]` section and the upgrade manual's backup paragraph point at the manual; the upgrade manual's shared-root and re-created-catalog sentences move there. The admin guide's `max_concurrent_jobs` row says that SQLite restores of items larger than a few hundred bytes hold more and points at the manual, its backup commands' paragraph gives the exit codes 3, 4 and 5 on a catalog `serve` refuses, and the deployment guide's checklist has a `[backup.store]` line. ADR-0019's Status line says it is superseded in part by the design, whose section 10 states what that covers, a form `docs/adr/README.md` now names; the Cassandra RFC's "Backup and restore" section carries a note, superseded in part, since CreateBackup on Cassandra still writes their in-catalog format, and its lines on synchronous operations and on restored indexes carry notes too. The differences document gives the 1M MongoDB and Cassandra restore figures. `docs/build-docs.py` lists the manual as document 19 (the build renders all 19), and the README's and AGENTS.md's documentation tables have a row for it (AGENTS.md's Extending Storage row now names its file). The quickstart and the `extenddb` skill say that `init` refuses an existing config file without `--overwrite`. The design's testing summary states the measured cells in place of "still to be measured", section 7 says what a SQLite restore of items larger than a few hundred bytes holds, the PR 11 section's line says what shipped, and revision item 35 records the filter history, the measurements, the manual and the database-name correction. Co-authored-by: Lee <leeroyhannigan@yahoo.ie> Signed-off-by: Scott Robinson <robinnsc@amazon.com>
robinnsc
added a commit
that referenced
this pull request
Nov 10, 2026
The integration workflow's PostgreSQL and SQLite jobs ran the wire suite with `--filter "not import_export"`. The filter came with the PostgreSQL job's first version (687e46f) and was copied into the SQLite job (556c501); neither commit gives a reason. A foreground server wrote a PID file in every tree up to e0d6a0a, which made foreground mode write none by default and added `--write-pid-file` to the job in the same commit; 556c501 came after e0d6a0a. So the restart `devtools/run-tests` makes for the import and export tests could stop the job's server in every tree, and the filter was not working around it. The MongoDB and Cassandra jobs already run `tests/test_import_export.py` unfiltered. The expression selects out that file's ten tests and nothing else. Run the job's way on release builds of this change they pass 10 of 10 on SQLite and on PostgreSQL, before and after its rebase onto main 887b6e2, so both filters go and no narrower one stays. Both lines already carry main's `--terraform` (#474); this change removes the filter only, so each reads `devtools/run-tests --extenddb --pytest --comprehensive --terraform --parallel`. `docs/design/15-backup-restore-bench-pr11.md` (row 15d of the design index) records the cells the testing summary left open, in the PR 9 record's method: release builds, a filesystem store, the server restarted before each phase, `VmHWM` sampled every 200 ms, on the 16-core host class of the earlier records. 1M items of 200 bytes restore in 33.8 s on MongoDB (its own backup) and 49.0 s on Cassandra (a SQLite backup adopted with `backup sync`), 52 and 33 MiB over idle. A 1M-item table with a 128-dimension vector index backs up in 243 s on PostgreSQL and 210 s on SQLite and restores in 246 s and 214 s, 111 MiB and 552 MiB over idle; SQLite's figure is higher than at 100k under glibc's default arenas and is 204 MiB, flat from 100k, under `MALLOC_ARENA_MAX=1`, where the backup took 222 s and the restore 283 s. The Cassandra cells ran on a node of their own with the Cassandra workflow's service settings. No cell is left open. Later runs on the same host, also in the record, reproduce the cells and show that SQLite's restore memory is its write batch and decode chunks, not the vector index: it grows with the item's size up to the 32 MiB batch bound whatever the attribute types (253 MiB, 130 MiB under one arena, for 100k items of 2.5 KB strings), and lists of numbers decode larger (397 to 424 MiB for the same 100k items of 128 numbers with no index, 301 to 316 MiB under one arena for 20,000 items of 1,536 numbers). Two restores at once under one arena took 57.5 s against 24.9 s. Five pairs of PostgreSQL runs with and without the vector index, each pair from one harness, put its share of the 100k restore at 27 to 53 MiB and 33 to 47% of the duration. A MongoDB restore becomes ACTIVE at the backfill worker's next 5 s cycle after its load, so it takes the load's time plus up to 5 s. These figures come from release builds of earlier versions of this change, on PR 10 at 358ff8b and main 94b5f68, before main's later changes to the backup and restore code (#700 and #740 as merged, and #474); the 100k cells, run again on release builds after the rebase onto main 887b6e2, reproduce in kind (peaks within 8.4%, durations up to 32% longer at a host load of 5.3 to 11.0 against 1.3 to 6.4 for the earlier cells). `docs/manuals/14-backup-and-restore.md` is the operator's manual, derived from the design: the strategy (its first sentence: a filesystem store keeps the host's failure domain, S3 is the production recommendation), RPO and RTO per store kind with the three records' figures and host class (a key-ordered PostgreSQL backup also needs the text key columns at collation `C` or `POSIX`, and the server logs a warning when it streams an older table unordered; the vector index's share of the SQLite restore comes from two sets of runs on a lightly loaded host, and the record's loaded set is given and left out of the share), the configuration (pointing at the admin guide's rows and the store crate's README; an S3 store needs root certificates, `ca-certificates` or `SSL_CERT_FILE`; a split-root identity mismatch is cleared by no command in this release, since `backup purge --confirm`, plain `backup sync` and `--prune-missing` refuse an unhealthy store, and the manual and the deployment guide give the two remedies that exist, a catalog copy from before the split, keeping the directory whose store id its rows carry, or one directory's rows deleted by hand from the `backup_index` table or collection (on Cassandra `backups_by_arn` and `backup_index_by_status`) by `store_id`, with the other directory's account directories copied into the kept one under the drill's copy rules before `backup sync`), securing the store, what a running backup costs each database, a restore drill, catalog-loss recovery with `backup sync`, `--adopt-store` and the in-memory startup sync (the old key pairs imported once `allow_credential_import` is turned on, with `extenddb settings` or, on an in-memory server, the management API, and off again after), legacy backups and what to do with them while `migrate-legacy` (#821) is not shipped, what a restore preserves, and moving data to and from Amazon DynamoDB with ImportTable and export (a store-prefix import is #748). It states each deployment statement the design's PR 11 acceptance lists. Figures measured for it, with their method in the record's "Figures measured for the manual": a 25 s PostgreSQL backup beside 12,156 overwrites of 200-byte items grew the written table from 0.4 to 4.4 MiB, its dead row versions not removable by a `VACUUM` during the backup (0.5 MiB with no backup); on SQLite the WAL grows by the pages each write changes, from 4.0 to 61.1 MiB over 15,472 writes to a table with no index during a 22.8 s backup and from 4.1 to 1,294 MiB over 13,996 writes to a table with two GSIs during a 25.6 s backup (about 20 to 24 pages per write over four runs). The manual's process-level statements were checked on release builds of this change before its rebase onto main 887b6e2, SQLite and PostgreSQL: the root placement refusals and their texts, the drill, an off-host copy under account and table directories made 0700 (a `mkdir -p` under a 022 umask stops sync until `chmod -R` on the account directory), a re-created catalog's config file (`init --overwrite` regenerates every section from defaults and carries over only the storage settings, so a `port` or TLS path edited in a default section reverts too and the copy taken first goes back whole apart from those settings) and its store health with an explicit store and with SQLite's default root (unconfigured, with CreateBackup writing in-catalog backups until the adoption), the accounts re-created with their old ids and old key pairs imported, the adoption, the in-memory startup sync and, on the in-memory dev build, the adopted backups refused until their account is re-created, the in-catalog create's warning and the restore of its backup through a store-configured server, an S3 store at moto with the default spool, `backup sync` exiting 6 beside a SQLite server, ImportTable refusing a gzip file and both kinds of symbolic link and importing a decompressed backup data file, an off-host copy with loose modes refused by `backup sync`, and on PostgreSQL the claim session found by its lock key, whose termination stops the job at its next heartbeat and leaves the row to the reconciler, the registration session's termination failing the server's running backups, the wait of `--prune-missing` on a recently failed backup, a catalog restored alone beside a newer data database, and the catalog and data dumps restored into new databases. On PostgreSQL 16 containers the restore the admin guide now gives ran end to end, on the old server and on a new, empty one, and an access key was imported through the management API from a request body read from a file. Since main's #840 the server refuses an access key import unless `allow_credential_import` is `true`, and a re-created catalog has no value for it: on PostgreSQL and on an in-memory SQLite server, release builds of this tree answer the manual's import with 403 "Credential import is disabled (allow_credential_import = false)" and import the pair once the setting is on (`extenddb settings set` beside the running server; on the in-memory server, which `extenddb settings` cannot reach, `PUT /management/settings/allow_credential_import`), and the imported pairs keep signing requests after it is turned off. Release builds after the rebase repeated the root placement refusals, what a restore preserves, the drill on SQLite, PostgreSQL and an S3 store at moto, the off-host copy shapes, a catalog restored from dumps and the claim and registration sessions on PostgreSQL, a catalog lost and re-created on SQLite with `--adopt-store`, the in-memory startup sync, the legacy path, ImportTable and export, the split-root remedy by hand on SQLite, the admin guide's Data Recovery entry on PostgreSQL 16 and `init`'s database names. The admin guide's Data Recovery entry named the PostgreSQL data database `extenddb_catalog_data`; `init` names it after the catalog without its `_catalog` suffix (`extenddb` by default). The entry now lists each backend's databases, says a catalog name without the suffix needs `--data-db` and that the failed `init` leaves the catalog database to drop first, and treats the two PostgreSQL databases as one copy: both dumps with the server stopped, under `umask 077` since the catalog dump holds the encryption key and the application role's password, or a base backup; the restore goes into new, empty databases created with `createdb -O` by the admin role, since one over the existing databases restores nothing and one into databases the admin role owns leaves the server unable to create tables. The old databases are renamed aside until the restored server has read every table; the dumps use the server's major version of `pg_dump` and are checked non-empty, since an older one exits 1 leaving an empty file that `psql` restores without an error; and on a new server the application role is created first with the dump's password and, at another address, the catalog's recorded data-database address is set, or the server keeps using the old one. The SQLite row gives the restore with the current `-wal` and `-shm` removed. The security model and the deployment guide's checklist say the same, and the upgrade manual's dumps take `umask 077` and are checked (`test -s`), and its rollback renames both databases aside, restores them with `ON_ERROR_STOP` and drops the renamed copies once the previous binary has read every table; the checklist limits a base backup to both databases on one instance. The architecture, design, quickstart and security-model guides, AGENTS.md, the troubleshooting guide, the technical-debt register (whose A-1 entry now cites functions, not moved line numbers) and the storage-extension guide carry the right name, the quickstart's `init` table gives the PostgreSQL admin and application roles' flags with their defaults, and `init --help` states both database defaults and that only the PostgreSQL backend reads the two flags. The admin guide's `[backup]` section and the upgrade manual's backup paragraph point at the manual; the upgrade manual's shared-root and re-created-catalog sentences move there. The admin guide's `max_concurrent_jobs` row says that SQLite restores of items larger than a few hundred bytes hold more and points at the manual, its backup commands' paragraph gives the exit codes 3, 4 and 5 on a catalog `serve` refuses, and the deployment guide's checklist has a `[backup.store]` line. ADR-0019's Status line says it is superseded in part by the design, whose section 10 states what that covers, a form `docs/adr/README.md` now names; the Cassandra RFC's "Backup and restore" section carries a note, superseded in part, since CreateBackup on Cassandra still writes their in-catalog format, and its lines on synchronous operations and on restored indexes carry notes too. The differences document gives the 1M MongoDB and Cassandra restore figures. `docs/build-docs.py` lists the manual as document 19 (the build renders all 19), and the README's and AGENTS.md's documentation tables have a row for it (AGENTS.md's Extending Storage row now names its file). The quickstart and the `extenddb` skill say that `init` refuses an existing config file without `--overwrite`. The design's testing summary states the measured cells in place of "still to be measured", section 7 says what a SQLite restore of items larger than a few hundred bytes holds, the PR 11 section's line says what shipped, and revision item 35 records the filter history, the measurements, the manual and the database-name correction. Co-authored-by: Lee <leeroyhannigan@yahoo.ie> Signed-off-by: Scott Robinson <robinnsc@amazon.com>
robinnsc
added a commit
that referenced
this pull request
Nov 10, 2026
The integration workflow's PostgreSQL and SQLite jobs ran the wire suite with `--filter "not import_export"`. The filter came with the PostgreSQL job's first version (687e46f) and was copied into the SQLite job (556c501); neither commit gives a reason. A foreground server wrote a PID file in every tree up to e0d6a0a, which made foreground mode write none by default and added `--write-pid-file` to the job in the same commit; 556c501 came after e0d6a0a. So the restart `devtools/run-tests` makes for the import and export tests could stop the job's server in every tree, and the filter was not working around it. The MongoDB and Cassandra jobs already run `tests/test_import_export.py` unfiltered. The expression selects out that file's ten tests and nothing else. Run the job's way on release builds of this change they pass 10 of 10 on SQLite and on PostgreSQL, before and after its rebase onto main 887b6e2, so both filters go and no narrower one stays. Both lines already carry main's `--terraform` (#474); this change removes the filter only, so each reads `devtools/run-tests --extenddb --pytest --comprehensive --terraform --parallel`. `docs/design/15-backup-restore-bench-pr11.md` (row 15d of the design index) records the cells the testing summary left open, in the PR 9 record's method: release builds, a filesystem store, the server restarted before each phase, `VmHWM` sampled every 200 ms, on the 16-core host class of the earlier records. 1M items of 200 bytes restore in 33.8 s on MongoDB (its own backup) and 49.0 s on Cassandra (a SQLite backup adopted with `backup sync`), 52 and 33 MiB over idle. A 1M-item table with a 128-dimension vector index backs up in 243 s on PostgreSQL and 210 s on SQLite and restores in 246 s and 214 s, 111 MiB and 552 MiB over idle; SQLite's figure is higher than at 100k under glibc's default arenas and is 204 MiB, flat from 100k, under `MALLOC_ARENA_MAX=1`, where the backup took 222 s and the restore 283 s. The Cassandra cells ran on a node of their own with the Cassandra workflow's service settings. No cell is left open. Later runs on the same host, also in the record, reproduce the cells and show that SQLite's restore memory is its write batch and decode chunks, not the vector index: it grows with the item's size up to the 32 MiB batch bound whatever the attribute types (253 MiB, 130 MiB under one arena, for 100k items of 2.5 KB strings), and lists of numbers decode larger (397 to 424 MiB for the same 100k items of 128 numbers with no index, 301 to 316 MiB under one arena for 20,000 items of 1,536 numbers). Two restores at once under one arena took 57.5 s against 24.9 s. Five pairs of PostgreSQL runs with and without the vector index, each pair from one harness, put its share of the 100k restore at 27 to 53 MiB and 33 to 47% of the duration. A MongoDB restore becomes ACTIVE at the backfill worker's next 5 s cycle after its load, so it takes the load's time plus up to 5 s. These figures come from release builds of earlier versions of this change, on PR 10 at 358ff8b and main 94b5f68, before main's later changes to the backup and restore code (#700 and #740 as merged, and #474); the 100k cells, run again on release builds after the rebase onto main 887b6e2, reproduce in kind (peaks within 8.4%, durations up to 32% longer at a host load of 5.3 to 11.0 against 1.3 to 8.5 for the earlier cells). `docs/manuals/14-backup-and-restore.md` is the operator's manual, derived from the design: the strategy (its first sentence: a filesystem store keeps the host's failure domain, S3 is the production recommendation), RPO and RTO per store kind with the three records' figures and host class (a key-ordered PostgreSQL backup also needs the text key columns at collation `C` or `POSIX`, and the server logs a warning when it streams an older table unordered; the vector index's share of the SQLite restore comes from two sets of runs on a lightly loaded host, and the record's loaded set is given and left out of the share), the configuration (pointing at the admin guide's rows and the store crate's README; an S3 store needs root certificates, `ca-certificates` or `SSL_CERT_FILE`; a split-root identity mismatch is cleared by no command in this release, since `backup purge --confirm`, plain `backup sync` and `--prune-missing` refuse an unhealthy store, and the manual and the deployment guide give the two remedies that exist, a catalog copy from before the split, keeping the directory whose store id its rows carry, or one directory's rows deleted by hand by `store_id` from the `backup_index` table or collection (on Cassandra from `backups_by_arn` and `backup_index_by_status`, with the rows' entries in `backups_by_account` and `backups_by_table`), with the other directory's account directories copied into the kept one under the drill's copy rules before `backup sync`), securing the store, what a running backup costs each database, a restore drill, catalog-loss recovery with `backup sync`, `--adopt-store` and the in-memory startup sync (the old key pairs imported once `allow_credential_import` is turned on, with `extenddb settings` or, on an in-memory server, the management API, and off again after), legacy backups and what to do with them while `migrate-legacy` (#821) is not shipped, what a restore preserves, and moving data to and from Amazon DynamoDB with ImportTable and export (a store-prefix import is #748). It states each deployment statement the design's PR 11 acceptance lists. Figures measured for it, with their method in the record's "Figures measured for the manual": a 25 s PostgreSQL backup beside 12,156 overwrites of 200-byte items grew the written table from 0.4 to 4.4 MiB, its dead row versions not removable by a `VACUUM` during the backup (0.5 MiB with no backup); on SQLite the WAL grows by the pages each write changes, from 4.0 to 61.1 MiB over 15,472 writes to a table with no index during a 22.8 s backup and from 4.1 to 1,294 MiB over 13,996 writes to a table with two GSIs during a 25.6 s backup (about 20 to 24 pages per write over four runs). The manual's process-level statements were checked on release builds of this change before its rebase onto main 887b6e2, SQLite and PostgreSQL: the root placement refusals and their texts, the drill, an off-host copy under account and table directories made 0700 (a `mkdir -p` under a 022 umask stops sync until `chmod -R` on the account directory), a re-created catalog's config file (`init --overwrite` regenerates every section from defaults and carries over only the storage settings, so a `port` or TLS path edited in a default section reverts too and the copy taken first goes back whole apart from those settings) and its store health with an explicit store and with SQLite's default root (unconfigured, with CreateBackup writing in-catalog backups until the adoption), the accounts re-created with their old ids and old key pairs imported, the adoption, the in-memory startup sync and, on the in-memory dev build, the adopted backups refused until their account is re-created, the in-catalog create's warning and the restore of its backup through a store-configured server, an S3 store at moto with the default spool, `backup sync` exiting 6 beside a SQLite server, ImportTable refusing a gzip file and both kinds of symbolic link and importing a decompressed backup data file, an off-host copy with loose modes refused by `backup sync`, and on PostgreSQL the claim session found by its lock key, whose termination stops the job at its next heartbeat and leaves the row to the reconciler, the registration session's termination failing the server's running backups, the wait of `--prune-missing` on a recently failed backup, a catalog restored alone beside a newer data database, and the catalog and data dumps restored into new databases. On PostgreSQL 16 containers the restore the admin guide now gives ran end to end, on the old server and on a new, empty one, and an access key was imported through the management API from a request body read from a file. Since main's #840 the server refuses an access key import unless `allow_credential_import` is `true`, and a re-created catalog has no value for it: on PostgreSQL and on an in-memory SQLite server, release builds of this tree answer the manual's import with 403 "Credential import is disabled (allow_credential_import = false)" and import the pair once the setting is on (`extenddb settings set` beside the running server; on the in-memory server, which `extenddb settings` cannot reach, `PUT /management/settings/allow_credential_import`), and the imported pairs keep signing requests after it is turned off. Release builds after the rebase repeated the root placement refusals, what a restore preserves, the drill on SQLite, PostgreSQL and an S3 store at moto, the off-host copy shapes, a catalog restored from dumps and the claim and registration sessions on PostgreSQL, a catalog lost and re-created on SQLite with `--adopt-store`, the in-memory startup sync, the legacy path, ImportTable and export, the split-root remedy by hand on SQLite, the admin guide's Data Recovery entry on PostgreSQL 16 and `init`'s database names. The admin guide's Data Recovery entry named the PostgreSQL data database `extenddb_catalog_data`; `init` names it after the catalog without its `_catalog` suffix (`extenddb` by default). The entry now lists each backend's databases, says a catalog name without the suffix needs `--data-db` and that the failed `init` leaves the catalog database to drop first, and treats the two PostgreSQL databases as one copy: both dumps with the server stopped, under `umask 077` since the catalog dump holds the encryption key and the application role's password, or a base backup; the restore goes into new, empty databases created with `createdb -O` by the admin role, since one over the existing databases restores nothing and one into databases the admin role owns leaves the server unable to create tables. The old databases are renamed aside until the restored server has read every table; the dumps use the server's major version of `pg_dump` and are checked non-empty, since an older one exits 1 leaving an empty file that `psql` restores without an error; and on a new server the application role is created first with the dump's password and, at another address, the catalog's recorded data-database address is set, or the server keeps using the old one. The SQLite row gives the restore with the current `-wal` and `-shm` removed. The security model and the deployment guide's checklist say the same, and the upgrade manual's dumps take `umask 077` and are checked (`test -s`), and its rollback renames both databases aside, restores them with `ON_ERROR_STOP` and drops the renamed copies once the previous binary has read every table; the checklist limits a base backup to both databases on one instance. The architecture, design, quickstart and security-model guides, AGENTS.md, the troubleshooting guide, the technical-debt register (whose A-1 entry now cites functions, not moved line numbers) and the storage-extension guide carry the right name, the quickstart's `init` table gives the PostgreSQL admin and application roles' flags with their defaults, and `init --help` states both database defaults and that only the PostgreSQL backend reads the two flags. The admin guide's `[backup]` section and the upgrade manual's backup paragraph point at the manual; the upgrade manual's shared-root and re-created-catalog sentences move there. The admin guide's `max_concurrent_jobs` row says that SQLite restores of items larger than a few hundred bytes hold more and points at the manual, its backup commands' paragraph gives the exit codes 3, 4 and 5 on a catalog `serve` refuses, and the deployment guide's checklist has a `[backup.store]` line. ADR-0019's Status line says it is superseded in part by the design, whose section 10 states what that covers, a form `docs/adr/README.md` now names; the Cassandra RFC's "Backup and restore" section carries a note, superseded in part, since CreateBackup on Cassandra still writes their in-catalog format, and its lines on synchronous operations and on restored indexes carry notes too. The differences document gives the 1M MongoDB and Cassandra restore figures. `docs/build-docs.py` lists the manual as document 19 (the build renders all 19), and the README's and AGENTS.md's documentation tables have a row for it (AGENTS.md's Extending Storage row now names its file). The quickstart and the `extenddb` skill say that `init` refuses an existing config file without `--overwrite`. The design's testing summary states the measured cells in place of "still to be measured", section 7 says what a SQLite restore of items larger than a few hundred bytes holds, the PR 11 section's line says what shipped, and revision item 35 records the filter history, the measurements, the manual and the database-name correction. Co-authored-by: Lee <leeroyhannigan@yahoo.ie> Signed-off-by: Scott Robinson <robinnsc@amazon.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
An access key import stores an access key ID and a secret that the caller chooses. After this change, the management API route
POST /management/accounts/{id}/users/{name}/access-keys/importimports a key only when the runtime settingallow_credential_importistrue. Theextenddb manage import-access-keycommand calls this route.falseas the default.falseand any other stored value also refuse the import. The settings CLI and the settings API accept onlytrueorfalsefor this key.Credential import is disabled (allow_credential_import = false). A store error still returns HTTP 500.credential_import_gate, which takes the settings store, so a unit test can give it each case.Test harness:
devtools/run-testssetsallow_credential_importtotruewith the other runtime settings. It does this one time, before any suite starts, so no test changes the setting while xdist workers run. CI runs the pytest,--comprehensiveand Rust integration suites through this script on all four backends.tests/cli/test-cli-comprehensive.sh, and the manual test workflows inAGENTS.mdand the developer test guide, set the value before they import a key.Docs: the settings tables in
docs/differences-from-dynamodb.mdand the admin guide,docs/getting-started.md, the deployment checklist and the troubleshooting guide give the new default. The upgrade manual has a v0.1.15 entry.Why
Each deployment that never set the value accepted imported keys, with an ID and a secret that the caller chose. With import off by default, only a deployment that turns import on accepts them. The server default now also agrees with the console settings page.
Testing done
cargo test -p extenddb-server --lib management: 91 passed. This includes 5 new tests for the gate: no row,false,true, other values (TRUE,True,1,yes, an empty string andtrue) and a store error.cargo fmt --checkandcargo clippy -p extenddb-server --all-targets -- -D warningspass.New
tests/test_allow_credential_import_sqlite.pyrunsinit --backend sqlitein a temporary directory and serves that deployment on a free port on 127.0.0.1. So it does not change the settings of the shared test server. On the new catalog, which has no row for the setting, it does these steps:extenddb settings set allow_credential_import trueruns in the deployment's directory while the server runs. The same import then gets HTTP 201, and the key lists.false, a second import gets HTTP 403, and the first key still lists.It passed 6 times. The
run-integration-sqlitejob runs it, and the jobs that build other backends skip it.Negative controls for the new test: I deleted the gate call, moved it below the store write, and let a missing row allow the import. Each time, the test failed.
devtools/run-tests --filter TestAccessKeyson a new SQLite catalog: with the harness frommain,test_import_access_keyfails with the 403. With the harness from this branch, all 5 tests pass.Merged with fix(storage-mongodb): store each access key ID once #823, which adds 3 tests that import keys: the 8 tests in
TestAccessKeysandTestAccessKeyIdIsStoredOncepass.I did not run
tests/cli/test-cli-comprehensive.sh, because it needs PostgreSQL, the AWS CLI and jq. I ran its two new commands by hand.Checklist
cargo test --workspace)cargo fmt --check)cargo clippy -- -W clippy::pedantic)Storagetrait, auth model, on-diskformat, or public CLI surface, an RFC has been accepted or is linked
below. Otherwise, an ADR captures the decision (link below).
ADR / RFC: n/a. A missing row for one runtime setting now means
false, the default that the console already shows. The name of the setting, its values and the route do not change.Breaking changes
A deployment that imports access keys and never set
allow_credential_importgets HTTP 403 from each import after the upgrade. Before the upgrade, runextenddb settings --config extenddb.toml set allow_credential_import trueon that deployment. The current release accepts the command, and the server reads the value on each import, so a restart is not necessary.By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.