An entrypoint for enterprise to register a hook which will be called
before the database and scheduler services are torn down. That way
enterprise can also perform graceful shutdown.
There was no need to join the entire metrics table, as it is a huge
table. We only needed all combinations of app_name, environment, and
feature_name. The new query retrieves all this data, which will then be
joined into the main query.
To check that users do indeed have permissions to update the roles from
project-service, we've been depending on req.user.id.
We had one error on Friday March 8th, where we managed to send
undefined/null to a method that requires a number. This PR assumes that
if we have an API token, and we have admin permissions and userId is not
set we're a legacy admin token.
It uses the util method for extractUserId(req: IAuthRequest | IApiRequest), so if we've passed through the apiTokenMiddleware first, we'll have userId -42, if we haven't, we'll get -1337.
Now frontend API requests will be counted separately under
getAllByfrontend. We are already tracking new FE db calls, so we can
build grafana dashboard.
https://linear.app/unleash/issue/2-2022/improve-actions-validation
Improves our current actions form validation.
Empty actions are now ignored on the payload and we get errors in
actions where any of the required fields are empty.
Also refactored our current actions into a constant map that can be
shared across frontend and backend.
## About the changes
This ports the CRUD store into OSS which is an abstraction to reduce the
amount of boilerplate code we have to write in stores.
By extending CRUDStore, the store becomes simply the type definition:
```typescript
type ActionModel = {
actionSetId: number;
action: string;
executionParams: Record<string, unknown>;
createdByUserId: number;
sortOrder: number;
};
export class ActionStore extends CRUDStore<
ActionModel & { id: number; createdAt: Date },
ActionModel
> {
}
```
And eventually specific mappings between those types can be provided (if
the mapping is more complex than camelCase -> snake_case):
```typescript
toRow: ({ project, name, actor, match, createdByUserId }) => ({
created_by_user_id: createdByUserId,
project,
name,
actor_id: actor,
source: match.source,
source_id: match.sourceId,
payload: match.payload,
}),
fromRow: ({
id,
created_at,
created_by_user_id,
project,
name,
actor_id,
source,
source_id,
payload,
}) => ({
id,
createdAt: created_at,
createdByUserId: created_by_user_id,
project,
name,
actor: actor_id,
match: {
source,
sourceId: source_id,
payload,
},
}),
```
Stores can also be extended to include additional functionality in case
you need to join with another table or do an aggregation, but it
significantly reduces the amount of boilerplate code needed to create a
basic store
We should not try to release the migration lock if where unable to
acquire it. By trying to close it when we have not successfully
connected to the database we end up hanging for a while before the
process is eventually killed.
I did not add a better error-message, as Unleash now gives a better
error stack and crashes immediate if you start without a database
password. We should still consider if you need to specify db credentials
or not. Technically it is possible to have a postgres without a password
(but it is likely not common).
Closes: #6408
## About the changes
We don't have a meaningful error for limits established by the
application. This could be a good starting point.
The error code is 400 cause I couldn't find anything better.
The name of the error was picked from UnleashApiErrorTypes:
2d8e9f87ff/src/lib/error/unleash-error.ts (L4-L34)
This column has not been used for 1.5 years and was replace by
**archived_at** column and people still get confused of why this is not
working as name suggests. Removing this column to remove technical debt.
## About the changes
Some of our metrics are not labeled correctly, one example is
`<base-path>/api/frontend/client/metrics` is labeled as
`/client/metrics`. We can see that in internal-backstage/prometheus:
This issue affects all endpoints that fail to validate the request body.
Also, endpoints that are rejected by the authorization-middleware or the
api-token-middleware are reported as `(hidden)`.
To gain more insights on our api usage but being protective of metrics
cardinality we're prefixing `(hidden)` with some well known base urls:
https://github.com/Unleash/unleash/pull/6400/files#diff-1ed998ca46ffc97c9c0d5d400bfd982dbffdb3004b78a230a8a38e7644eee9b6R17-R33
## How to reproduce:
Make an invalid call to metrics (e.g. stop set to null), then check
/internal-backstage/prometheus and find the 400 error. Expected to be at
`path="/api/client/metrics"` but will have `path=""`:
```shell
curl -H"Authorization: *:development.unleash-insecure-client-api-token" -H'Content-type: application/json' localhost:4242/api/client/metrics -d '{
"appName": "bash-test",
"instanceId": "application-name-dacb1234",
"environment": "development",
"bucket": {
"start": "2023-07-27T11:23:44Z",
"stop": null,
"toggles": {
"myCoolToggle": {
"yes": 25,
"no": 42,
"variants": {
"blue": 6,
"green": 15,
"red": 46
}
},
"myOtherToggle": {
"yes": 0,
"no": 100
}
}
}
}'
```
SCIM synchronizations requires a stable id no matter how many changes
are made to username and email (our other unique fields). In addition,
exposing internal incremented database ids to an external service (our
current id field) feels insecure. Our plan is to create either a uuidv7
or ulid when scim operations are performed against the user, so the
external scim provisioner has a stable globally unique id to use to
refer to the users they're modifying.
In order to stop privilege escalation via
`/api/admin/projects/:project/users/:userId/roles` and
`/api/admin/projects/:project/groups/:groupId/roles` this PR adds the
same check we added to setAccess methods to the methods updating access
for these two methods.
Also adds tests that verify that we throw an exception if you try to
assign roles you do not have.
Thank you @nunogois for spotting this during testing.
So, since our assumption about client instances ended up being wrong (or, less than stable).
This PR moves the EdgeUpgradeBanner to be displayed if the featureflag
displayEdgeBanner is enabled. That way, if customers comes back and says
they have upgraded but still get the banner, we can remove them from the
segment.
## About the changes
When edge is configured to automatically generate tokens, it requires
the token to be present in all unleash instances.
It's behind a flag which enables us to turn it on on a case by case
scenario.
The risk of this implementation is that we'd be adding load to the
database in the middleware that evaluates tokens (which are present in
mostly all our API calls. We only query when the token is missing but
because the /client and /frontend endpoints which will be the affected
ones are high throughput, we want to be extra careful to avoid DDoSing
ourselves
## Alternatives:
One alternative would be that we merge the two endpoints into one.
Currently, Edge does the following:
If the token is not valid, it tries to create a token using a service
account token and /api/admin/create-token endpoint. Then it uses the
token generated (which is returned from the prior endpoint) to query
/api/frontend. What if we could call /api/frontend with the same service
account we use to create the token? It may sound risky but if the same
application holding the service account token with permission to create
a token, can call /api/frontend via the generated token, shouldn't it be
able to call the endpoint directly?
The purpose of the token is authentication and authorization. With the
two tokens we are authenticating the same app with 2 different
authorization scopes, but because it's the same app we are
authenticating, can't we just use one token and assume that the app has
both scopes?
If the service account already has permissions to create a token and
then use that token for further actions, allowing it to directly call
/api/frontend does not necessarily introduce new security risks. The
only risk is allowing the app to generate new tokens. Which leads to the
third alternative: should we just remove this option from edge?
Since we're polling for updates to max revision id every second, and
listening for update events for revision id in the proxy repository then
running a refresh interval of 20secs in the proxy repo refresh seems
excessive.
This PR changes the frequency of the refresh to once per 45mins.
This PR adds a property issues to application schema, and also adds all
the missing features that have been reported by SDK, but do not exist in
Unleash.
Adds a migration to create and fill the `project_metrics_summary_trends`
This table is to be used in enterprise to update the metrics data daily
per project (after the aggregation of the hourly data)
Driving force for this was doing performance testing on the executive
dashboard.
This will allow to remove the expensive query to aggregate the data on
demand and will drop the total response time from 2.5sec to 125ms
(measurements done with 100 Projects, 10000 features and over 1M rows in
`client_metrics_env_daily`
Closes #
[1-2080](https://linear.app/unleash/issue/1-2080/create-the-project-metrics-summary-trends-table)
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
