In preparation for 19.0.0 this removes all references
to legacy or broken record keys. The background key
conversion will have already converted all legacy or broken
keys thus allowing the removal of the migration and the key types.
This change adds the error handling for Retry logic
in pgbk to also handle `pgerrcode.DuplicateTable`.
When two (or more) auth instances both try to create/setup
the same schema at nearly the same time, even with `IF NOT EXISTS`,
Postgres can still return `duplicate_table` (`42P07`) in a concurrent
create race. Retrying lets the losing instance re-check after the
winning instance finishes creating the table.
* Fix CRUD expiry behavior in dynamodbbk
* Fix AtomicWrite expiry behavior in dynamodbbk
* DynamoDB relies on server-side TTL expiry so it can't use a fake clock
* fixup! Fix CRUD expiry behavior in dynamodbbk
* Allow Delete to delete expired items
* Remove some unnecessary whitespace
* Take the time before the backend call in Get and Delete
The BatchWriteItem API can return a response with items that failed to
be written without returning an error. DeleteRange could silently
allow items to never be removed under certain conditions because it
never consumed the response.
A new batchWriteItem helper now manages interactions with the BatchWriteItem
API. Any responses with UnprocessedItems are now retried with backoff.
Resolves https://github.com/gravitational/pressure-washing/issues/137.
Ensures the backend table and events table use the same revision when
updating items during KeepAlive operations. Previously a revision was
generated for _both_ tables causing watchers and Get calls to observe
different revisions.
The backend test suite was updated to validate revisions in the event
stream match revisions in the backend.
Resolves https://github.com/gravitational/pressure-washing/issues/139.
* fix(auth): expire Access Graph API sessions at session TTL, not bearer-token TTL
ACCESS_GRAPH_API web sessions authenticate via the client TLS
certificate in Handler.AuthenticateReqForAccessGraphAPI and never
consume a bearer token. Today they still get one generated and a
paired WebToken upserted. Two consumers
(lib/services/local.webSessions.Upsert computing the backend item
TTL, and lib/web.SessionContext.expired deciding aliveness) then
read the bearer-token expiry directly; for these sessions that's
misleading (the bearer is zero, and expired() silently treats a
zero expiry as "never expires"). This change lifts the usage-aware
choice into api/types.WebSession.GetEarliestExpiry, stops the
producers from generating the bearer artefacts, and updates both
consumers.
Behavioural changes:
- lib/auth.newWebSession no longer generates a BearerToken or
BearerTokenExpires for ACCESS_GRAPH_API sessions
- lib/auth.upsertWebSession skips UpsertWebToken for these sessions
(no paired WebToken would be meaningful, and creating one with an
empty token / zero expiry is misleading)
Centralisation:
- api/types: WebSession.GetEarliestExpiry() returns the time after
which a session should be considered dead, accounting for
Usage. Standard sessions return min(bearer, session) with zero
treated as infinity; ACCESS_GRAPH_API sessions return the
session expiry. Interface doc spells out the per-usage rule;
full rationale lives on WebSessionV2.GetEarliestExpiry.
- lib/services/local.webSessions.Upsert and lib/web
SessionContext.expired now call session.GetEarliestExpiry()
Tests:
- api/types: GetEarliestExpiry covers bearer<session,
bearer>session (capped), zero-bearer, zero-session, and
ACCESS_GRAPH_API
- lib/auth: CreateWebSessionFromReq for ACCESS_GRAPH_API leaves
bearer empty + zero expiry; types.NewWebToken's empty-token
validation means a successful call also proves UpsertWebToken
was skipped. UNSPECIFIED still populates bearer and upserts the
paired WebToken.
- lib/services/local: backend item Expires matches the per-usage
expiry choice through Upsert
* fix: remove backend.EarliestExpiry and update doc
When polling shards, after a new child shard is created it is
possible that a write event lands on the shard before the backend
creates an iterator at LATEST position. In which case the iterator
will point the newest event after that point leading to occasional
lost events when polling the shard. According to the documentation
the parent should be processed first and then the children will
continue for further events, meaning that for split shards we can use
TRIM_HORIZON iterator type to always return all events on the freshly
created shard.
Newly created poll loop sets the watcher init after the initial registration,
meaning that we can use the LATEST type to avoid processing old events. The
cache is then expected to fetch its items.
Further removed the first poll sleep on a new shard to avoid unneccessary
delay on first events.
Added a check to ensure that newly found active shards return a valid iterator
before entering polling loop, even in the case of a closed shard the iterator
should still be valid, check defensively and set error if this happens.
Changelog: Fixes potential race condition in dynamoDB backend which can lead to missed events, resulting in a inconsistent cache state.
* enforce region validation on all AWS config loading
Replace direct uses of `github.com/aws/aws-sdk-go-v2/config.LoadDefaultConfig`
with the internal `lib/cloud/aws/config.LoadDefaultConfig` wrapper, which
validates any explicitly requested AWS region before use. Enforces this via a
new forbidigo linter rule.
Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
* Apply suggestions from code review
Co-authored-by: Nic Klaassen <nic@nicklaassen.ca>
* fix unit tests
* include underscores and uppercase letters
* add ConfigureRegion
---------
Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Nic Klaassen <nic@nicklaassen.ca>
Added a new env var `TELEPORT_FIRESTORE_TEST_DATABASE` used
to configure the database ID to use in the tests. This is useful
as when unspecified this defaults to `(default)` which may not
be in the ideal region.
Also changes the retunred type to LimitExceeded which seems more appropiate.
This error path was never hit before and it would be returned as is without
trace wrappers.
Locked, interrupted, and read only error checks were comparing
against the error code directly. The check should return true for the
base error code _and_ any extended error codes for that class of
error. This is now correctly achieved by masking the error code
and comparing with the base error code.
Note: the constraint error should only return true for primary
key errors as that was done prior. As such it has not been updated
to use the mask and check the base error coded.
The lifetime of `runPeriodicOperations` was only loosely tied to
the lite.Backend via lite.Backend.ctx. When closing the backend
the context was cancelled which would eventually terminate
`runPeriodicOperations`. However, `lite.Backend.Close` could return
while `runPeriodicOperations` was still processing as a result
of the periodic ticker firing. To prevent this race the backend
now utilizies a sync.WaitGroup to launch `runPeriodicOperations`
and waits for the group to be completed _prior_ to returning from
`(lite.Backend) Close`.
`runPeriodicOperations` also attempted to close the sql.DB when it
detected that the context had been cancelled. While the double close
is fine due to the idempotence of (lite.Backed) closeDatabase, it
was confusing and unnecesarry. Ownership of closing the database now
lies exclusively on explicitly calling `(lite.Backend) Close`.
The closed flag has also been removed. It existed solely to prevent
logging or rolling back transactions that had failed after the
underlying database had been closed. Both operations are always attempted
now. All logs are silenced in the event that the database is unreachable
during process termination.
On failure to create the database schema the underlying sqlite database
is now closed before returning an error from lite.NewWithConfig.
UX changes:
Progress of the clone operation is now logged every 250ms
instead of a single time after a 1s delay. This helps users know
that cloning is active and doing something. The total number of items
copied and items that failed to be cloned is also logged at the conclusion
of the command.
Friendly messages are emitted at the conclusion of the command to let
users know if there is any action on their part.
Performance changes:
Clone now makes use of backend.Items instead of backend.GetRange. This
helps reduce allocations since each page doesn't need to be provided one
slice at a time. Instead of broadcasting each item from the GetRange result
to a channel, we now spawn new work directly while processing each item
when iterating the backend.Items.
The number of allocations required to process each item has also been
reduced by holding off on creating a retry object until an error is
encountered pushing an item to the new backend.
Comparison of BenchmarkClone before and after this change below.
```bash
benchstat old.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/backend
cpu: Apple M2 Pro
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
Clone-12 146.2m ± 2% 148.5m ± 1% ~ (p=0.165 n=10)
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
Clone-12 12.380Mi ± 0% 5.489Mi ± 0% -55.66% (p=0.000 n=10)
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Clone-12 94.39k ± 0% 72.06k ± 0% -23.66% (p=0.000 n=10)
```
* add client side reloading to certreloader
Moves certreloader to its own package to avoid cyclic dependency.
Adds client side cert check as well as postgres specific function for
certificate reloading.
* remove duration
* refactor certreloader to include expiry metric
* rework to callback
* fix nil check on callback
* fix lint
* feedback
Plugins had their own backoff utils when they were a separate repo,
but now that they're in Teleport's repo we can reuse existing retry
utils.
Closes#47102
The StreamRange function was added to make iteration over the results
from backend.GetRange easier. However, it's utility no longer exists
now that iterators are a first class citizen in Go and the backend.Items
has been added. To facilitate the removal, all existing calls to
StreamRange have been updated to call Items instead and directly
loop over all items.
The IterateRange function was added to make iteration over the
results from backend.GetRange easier. However, it's utility no
longer exists now that the backend.Items has been added. To
facilitate the removal, all existing calls to IterateRange have
been updated to call Items instead and directly loop over all
items.
Moves all test related logger initialization and creation to the
logtest package to reduce testing symbols in production code. The
existing helpers in lib/utils have been left in place until the
enterprise references can be converted.
Updates #51023.
During the cache storage refactoring, all of the cache metrics
that were recorded via the backend.Reporter that wrapped the
cache backend were lost. This attempts to restore the metrics
as close to the original meaning as possible, though due to the
changes some labels are different and batch metrics don't apply.
The biggest change is in the top requests metrics which has
historically been based on the backend.Key, but since that
the cache no longer deals with backends, the metric is now labeled
by the resource kind. All existing dashboards and tctl top should
still work as expected.
The error checks were performed against error returned from
functions without assigning them to a local variable which resulted
in a return of nil, nil instead of returning the appropriate error.
In practice this isn't very likely to be hit since most keys should
be neither the legacy or broken format.
```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x5af856fc3494]
goroutine 62984 [running]:
github.com/gravitational/teleport/lib/backend/firestore.(*Backend).Items.func3-range1(0xc0001991f0, {0x0?, 0x0?})
github.com/gravitational/teleport/lib/backend/firestore/firestorebk.go:586 +0x134
github.com/gravitational/teleport/lib/backend/firestore.(*Backend).Items.func3.(*Backend).mergedRecords.2-range1(0x5af85ed273a0?, {0x0, 0x0?})
github.com/gravitational/teleport/lib/backend/firestore/firestorebk.go:655 +0x4f
github.com/gravitational/teleport/lib/itertools/stream.MergeStreams[...].func1()
github.com/gravitational/teleport/lib/itertools/stream/stream.go:378 +0x236
github.com/gravitational/teleport/lib/backend/firestore.(*Backend).Items.func3.(*Backend).mergedRecords.2(0xc000e52500)
github.com/gravitational/teleport/lib/backend/firestore/firestorebk.go:654 +0x89b
github.com/gravitational/teleport/lib/backend/firestore.(*Backend).Items.func3(0xc003bb4cc0)
github.com/gravitational/teleport/lib/backend/firestore/firestorebk.go:579 +0x3bf
```
The above panic is caused by the snapShot not being set and thus
resulting in a nil dereference. The snapShot is now assigned outside
of the switch to ensure that all records that are successfully
created contain a reference to a valid DocumentSnapshot.
(ServerWithRoles) ListUserGroups was returning the next page token
with a / preceeding the name of the group. When switching over to
the new cache collection, the cache was not accounting for this
and would result in an infinite loop. The prefix was a result of
calling backend.NextPaginationKey instead of providing the unique
group name.
This was not a problem in the past because the local UserGroup
storage service was joining the next token with the key prefix which
correctly built the key /user_groups/<next>. The new cache collection
is now trimming the `/` prefix to prevent the infinite looping.
The AuthServer implementation was also updated to no longer include
the prefix, which should be backward compatibile because of the
way the local services are constructing the backend key. Additionally,
tests were added to both the new caching layer and local storage
layer to validate both work if the nextKey provided has a `/` prefix
or not.
backend.NextPaginationKey was also removed since it was no longer
being used.
* Delete audit stream version check for v13
* Remove string parsing for bpf min kernel version
* Remove string parsing for min etcd version
* Remove version string parsing for route not found
* Remove string parsing for min mariadb version
* Remove string parsing for min mongodb custom data version
* Remove string parsing for min openat2 kernel version
* Use consts in api/version.go
* Generate api/version.go with a go command
* Avoid using a global variable SemVersion
* Keep but deprecate SemVersion in api
* fix-license
rosstimothy