* (test) added more context to error details for cli commands
* (fix) refactor solution to utilize custom error type casting
* (test) added api unit test to validate InvalidArgumentError return message
* Update internal/types/subtypes/error.go
Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
* (refactor) rename InvalidArgumentError into UnknownSubtypeIDError
Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
It was determined that it is a more common use case for users to request
non-terminated sessions (i.e. sessions in `pending`, `active`, or
`canceling` when requesting sessions. Notably, end users would need
non-terminated sessions to:
- Establish a new connection to a session.
- Terminate a session
So now by default a list request to `/v1/sessions` will *not* include
terminated sessions.
To still enable use cases where terminated sessions are needed, this
adds a new query parameter to include the terminated session:
`/v1/sessions?include_terminated=true`.
Note that this query parameter is considered experimental and subject to
change.
Including the nested connection information in the session list response
had a negative impact on performance as it created a significant
multiplier on the number of rows that needed to be fetched from the
database.
It was also determined that this information was used by neither the
admin ui nor the desktop client.
Note that nested connection information can still be retrieved via a
read request for a specific session.
There is a new periodic job that will delete terminated sessions that
are older than one hour. However, since until now terminated sessions
were not deleted, the first time the job runs there could be a large
number of sessions that need deleted. This could have a negative impact
on a running cluster. To mitigate the operational risk, this adds a
migration to perform an initial delete while the cluster is offline.
* server refuses to start if max_open_connections is below 5
* suggested changes - Update internal/db/db.go
Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Both the init and migrate commands were ignoring
the configured max_open_connections database
setting. This fix ensures that both commands
respect the limits.
This test was using the third element in the result from a list request
to craft the input for the next assertion. However, one of the test
targets that is created does not have a default port set. Since the
target list call is not ordered, sometimes this element could be the
third element and a panic would result when default port interface was
converted to a float64.
This fixes the panic like:
--- FAIL: TestList (1.31s)
panic: interface conversion: interface {} is nil, not float64 [recovered]
panic: interface conversion: interface {} is nil, not float64
goroutine 1694 [running]:
testing.tRunner.func1.2({0x14b2320, 0xc002f486f0})
/prefix/usr/lib/go/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
/prefix/usr/lib/go/src/testing/testing.go:1392 +0x39f
panic({0x14b2320, 0xc002f486f0})
/prefix/usr/lib/go/src/runtime/panic.go:838 +0x207
github.com/hashicorp/boundary/internal/tests/api/targets_test.TestList(0xc000f81380)
/go/src/github.com/hashicorp/boundary/internal/tests/api/targets/target_test.go:193 +0x174b
testing.tRunner(0xc000f81380, 0x2b933a8)
/prefix/usr/lib/go/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
/prefix/usr/lib/go/src/testing/testing.go:1486 +0x35f
FAIL github.com/hashicorp/boundary/internal/tests/api/targets 5.742s
FAIL
Refs: #2003
* feature (db): add Db.UnderlyingDB()
This is needed to allow for integration with bits that need a *dbw.DB
* feature (kms): add new go-kms-wrapping/extras/kms dependency
* refactor (kms): schema changes required to adopt extras/kms pkg
* refactor (kms/iam): rename to DeprecateCreateKeysTx(...)
* refactor (kms): remove deprecated test
* refactor (kms): refactor Kms type
This refactor includes renaming the existing type to DeprecatedKms
and then refactoring the Kms type to become a wrapper of
go-kms-wrapping/extras/kms
* refactor (kms): delete all the unneeded bits
All this stuff is not needed now that we're using
go-kms-wrapping/extras/kms
* refactor (db): fixup tests after kms refactor
* refactor (kms/tests): use testing.TB for test helpers
Our test helper packages were not possible to use
with benchmarks. This refactor switches to testing.TB
to allow both tests and benchmarks to use the helpers.
* tests (session benchmarks): refactor to use new Kms type
* refactor (schema): rm test which relies on deprecated Kms code
* refactor (kms): post rebase on main changes required
* chore (kms refactor): re-add unit test from main
* refactor (kms): uses migrations that don't include KMS schema changes
Leave cluster listener up when bad data is sent
Removing the ALPN muxer exposed the underlying listener directly to the
gRPC server, which means that a quirk of the ALPN muxer (not surfacing
errors on accept) turned into the gRPC server receiving errors on
accept, and it turns out if the error does not satisfy a special
temporary interface the listener will be closed.
The obviously better thing to do would be to let the listener close
itself if there was truly a fatal error rather than relying on
undocumented interfaces, but here we are.
* Move the server daemons into their own package
With this change the internal/server package now is dedicated to the server domain (workers and controllers) and not to the server daemons.
The hope is that this will make it easier to keep the daemons decoupled from the representation of the servers in the database.
* fix(cmd): Add profiling build imports
It seems like this file has been bitrotted since last used,
and it was missing some imports. This was discovered
by running goimports on the code.
* Add build-memprof Makefile target
* Move the session cleanup job logic into the session package.
This is part of a broader effort to more narrowly define the purpose of the internal/server directory to be for interacting with servers themselves. Future changes will continue to remove things from this directory until the purpose of the directory is clearly working primarily with the boundary servers resource.
When applying the limit and order by, the where clause also needs to be
applied to yield the correct results. This can be seen with an example.
Assuming a data set where there are 100 sessions, with 10 sessions
belonging to a user with id `u_user1` and 90 sessions belonging to
`u_user2`. If the constructed query is like:
select *
from
(select public_id from session limit 10) s,
session_list ss
where
s.public_id = ss.public_id
and s.user_id = 'u_user1'
order by create_time asc;
The initial sub-query with the limit could return sessions only
belonging to `u_user2`, then it would apply the where clause and exclude
all of the rows.
Instead if the query is:
with
session_ids as (
select public_id
from sessions as s
where
s.user_id = 'u_user1'
order by create_time asc
limit 10
)
select *
from session_list
where
session_list.public_id in (select * from session_ids)
order by create_time asc;
The CTE applies the where clause ensuring that only sessions for
`u_user1` are used for the final select from the view.
Note that the CTE is use primarily for readability.
This also adds an index on `session (create_time)` to aide the query
planner in performing the order by.
The list benchmark tests uses pre-created database
dumps to run its benchmarks. This benchmark
will be used to guide optimizations around listing
of sessions.
Fixes https://hashicorp.atlassian.net/browse/ICU-4042
Sometimes when creating a lot of sessions concurrently we
can run into a unique constraint error in the warehouse triggers
in the database. Simply retrying the call will allow the
session creation to succeed.
Our test helper packages were not possible to use
with benchmarks. This refactor switches to testing.TB
to allow both tests and benchmarks to use the helpers.
* Reorder session authz checks
This implements a mechanism for service handlers to pass a repo
satisfying an interface into the scope IDs listing function. The purpose
of this interface is to allow the same function to perform authorization
checking against resources rather than requiring service handlers to
fetch all resources in the determined set of scopes and then perform
this checking. While this could be done in each service handler, this
approach centralizes the logic, which has the nice benefit of removing
some boilerplate from service handler list functions that have adapted
to it.
Tests haven't changed, which is expected -- the function should return
exactly what was being returned before, simply faster.
Previously we were matching what boundary considers an id, but that is determined after requests are routed by grpc-gateway.
This changes the regexp to match any path segment to try to behave the same way that grpc-gateway would when assigning values
to the {id} variable declared in the protobuf.
The wrapper is sometimes unset at the time when the
audit config is validated, so it is impossible for us
to validate that a valid wrapper is configured. The wrapper
is configured by later setup steps because of a dependency
on the eventer in our KMS setup.
Louis Ruch
Michael Gaffney
Damian DebkowskiandJohan Brandhorst-Satzkorn
Todd
Timothy Messier
Jeff Mitchell
Gabriel SantosandJohan Brandhorst-Satzkorn
Jim
Carlos Alcaide Corvo
irenarindos
Zed