## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
Adds projects user and group -usage information to the dialog shown when
user wants to delete a project role
<img width="670" alt="Skjermbilde 2023-08-10 kl 08 28 40"
src="https://github.com/Unleash/unleash/assets/707867/a1df961b-2d0f-419d-b9bf-fedef896a84e">
---------
Co-authored-by: Nuno Góis <github@nunogois.com>
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
Fixes an issue where project role deletion validation didn't validate
against project roles being connected to groups
https://linear.app/unleash/issue/2-1311/add-a-new-prometheus-metric-with-custom-root-roles-in-use
As a follow-up to https://github.com/Unleash/unleash/pull/4435, this PR
adds a metric for total custom root roles in use by at least one entity:
users, service accounts, groups.
`custom_root_roles_in_use_total`
Output from `http://localhost:4242/internal-backstage/prometheus`:
```
# HELP process_cpu_user_seconds_total Total user CPU time spent in seconds.
# TYPE process_cpu_user_seconds_total counter
process_cpu_user_seconds_total 0.060755
# HELP process_cpu_system_seconds_total Total system CPU time spent in seconds.
# TYPE process_cpu_system_seconds_total counter
process_cpu_system_seconds_total 0.01666
# HELP process_cpu_seconds_total Total user and system CPU time spent in seconds.
# TYPE process_cpu_seconds_total counter
process_cpu_seconds_total 0.077415
# HELP process_start_time_seconds Start time of the process since unix epoch in seconds.
# TYPE process_start_time_seconds gauge
process_start_time_seconds 1691420275
# HELP process_resident_memory_bytes Resident memory size in bytes.
# TYPE process_resident_memory_bytes gauge
process_resident_memory_bytes 199196672
# HELP nodejs_eventloop_lag_seconds Lag of event loop in seconds.
# TYPE nodejs_eventloop_lag_seconds gauge
nodejs_eventloop_lag_seconds 0
# HELP nodejs_eventloop_lag_min_seconds The minimum recorded event loop delay.
# TYPE nodejs_eventloop_lag_min_seconds gauge
nodejs_eventloop_lag_min_seconds 0.009076736
# HELP nodejs_eventloop_lag_max_seconds The maximum recorded event loop delay.
# TYPE nodejs_eventloop_lag_max_seconds gauge
nodejs_eventloop_lag_max_seconds 0.037683199
# HELP nodejs_eventloop_lag_mean_seconds The mean of the recorded event loop delays.
# TYPE nodejs_eventloop_lag_mean_seconds gauge
nodejs_eventloop_lag_mean_seconds 0.011063251638989169
# HELP nodejs_eventloop_lag_stddev_seconds The standard deviation of the recorded event loop delays.
# TYPE nodejs_eventloop_lag_stddev_seconds gauge
nodejs_eventloop_lag_stddev_seconds 0.0013618102764025837
# HELP nodejs_eventloop_lag_p50_seconds The 50th percentile of the recorded event loop delays.
# TYPE nodejs_eventloop_lag_p50_seconds gauge
nodejs_eventloop_lag_p50_seconds 0.011051007
# HELP nodejs_eventloop_lag_p90_seconds The 90th percentile of the recorded event loop delays.
# TYPE nodejs_eventloop_lag_p90_seconds gauge
nodejs_eventloop_lag_p90_seconds 0.011321343
# HELP nodejs_eventloop_lag_p99_seconds The 99th percentile of the recorded event loop delays.
# TYPE nodejs_eventloop_lag_p99_seconds gauge
nodejs_eventloop_lag_p99_seconds 0.013688831
# HELP nodejs_active_resources Number of active resources that are currently keeping the event loop alive, grouped by async resource type.
# TYPE nodejs_active_resources gauge
nodejs_active_resources{type="FSReqCallback"} 1
nodejs_active_resources{type="TTYWrap"} 3
nodejs_active_resources{type="TCPSocketWrap"} 5
nodejs_active_resources{type="TCPServerWrap"} 1
nodejs_active_resources{type="Timeout"} 1
nodejs_active_resources{type="Immediate"} 1
# HELP nodejs_active_resources_total Total number of active resources.
# TYPE nodejs_active_resources_total gauge
nodejs_active_resources_total 12
# HELP nodejs_active_handles Number of active libuv handles grouped by handle type. Every handle type is C++ class name.
# TYPE nodejs_active_handles gauge
nodejs_active_handles{type="WriteStream"} 2
nodejs_active_handles{type="ReadStream"} 1
nodejs_active_handles{type="Socket"} 5
nodejs_active_handles{type="Server"} 1
# HELP nodejs_active_handles_total Total number of active handles.
# TYPE nodejs_active_handles_total gauge
nodejs_active_handles_total 9
# HELP nodejs_active_requests Number of active libuv requests grouped by request type. Every request type is C++ class name.
# TYPE nodejs_active_requests gauge
nodejs_active_requests{type="FSReqCallback"} 1
# HELP nodejs_active_requests_total Total number of active requests.
# TYPE nodejs_active_requests_total gauge
nodejs_active_requests_total 1
# HELP nodejs_heap_size_total_bytes Process heap size from Node.js in bytes.
# TYPE nodejs_heap_size_total_bytes gauge
nodejs_heap_size_total_bytes 118587392
# HELP nodejs_heap_size_used_bytes Process heap size used from Node.js in bytes.
# TYPE nodejs_heap_size_used_bytes gauge
nodejs_heap_size_used_bytes 89642552
# HELP nodejs_external_memory_bytes Node.js external memory size in bytes.
# TYPE nodejs_external_memory_bytes gauge
nodejs_external_memory_bytes 1601594
# HELP nodejs_heap_space_size_total_bytes Process heap space size total from Node.js in bytes.
# TYPE nodejs_heap_space_size_total_bytes gauge
nodejs_heap_space_size_total_bytes{space="read_only"} 0
nodejs_heap_space_size_total_bytes{space="old"} 70139904
nodejs_heap_space_size_total_bytes{space="code"} 3588096
nodejs_heap_space_size_total_bytes{space="map"} 2899968
nodejs_heap_space_size_total_bytes{space="large_object"} 7258112
nodejs_heap_space_size_total_bytes{space="code_large_object"} 1146880
nodejs_heap_space_size_total_bytes{space="new_large_object"} 0
nodejs_heap_space_size_total_bytes{space="new"} 33554432
# HELP nodejs_heap_space_size_used_bytes Process heap space size used from Node.js in bytes.
# TYPE nodejs_heap_space_size_used_bytes gauge
nodejs_heap_space_size_used_bytes{space="read_only"} 0
nodejs_heap_space_size_used_bytes{space="old"} 66992120
nodejs_heap_space_size_used_bytes{space="code"} 2892640
nodejs_heap_space_size_used_bytes{space="map"} 2519280
nodejs_heap_space_size_used_bytes{space="large_object"} 7026824
nodejs_heap_space_size_used_bytes{space="code_large_object"} 983200
nodejs_heap_space_size_used_bytes{space="new_large_object"} 0
nodejs_heap_space_size_used_bytes{space="new"} 9236136
# HELP nodejs_heap_space_size_available_bytes Process heap space size available from Node.js in bytes.
# TYPE nodejs_heap_space_size_available_bytes gauge
nodejs_heap_space_size_available_bytes{space="read_only"} 0
nodejs_heap_space_size_available_bytes{space="old"} 1898360
nodejs_heap_space_size_available_bytes{space="code"} 7328
nodejs_heap_space_size_available_bytes{space="map"} 327888
nodejs_heap_space_size_available_bytes{space="large_object"} 0
nodejs_heap_space_size_available_bytes{space="code_large_object"} 0
nodejs_heap_space_size_available_bytes{space="new_large_object"} 16495616
nodejs_heap_space_size_available_bytes{space="new"} 7259480
# HELP nodejs_version_info Node.js version info.
# TYPE nodejs_version_info gauge
nodejs_version_info{version="v18.16.0",major="18",minor="16",patch="0"} 1
# HELP nodejs_gc_duration_seconds Garbage collection duration by kind, one of major, minor, incremental or weakcb.
# TYPE nodejs_gc_duration_seconds histogram
# HELP http_request_duration_milliseconds App response time
# TYPE http_request_duration_milliseconds summary
# HELP db_query_duration_seconds DB query duration time
# TYPE db_query_duration_seconds summary
db_query_duration_seconds{quantile="0.1",store="api-tokens",action="getAllActive"} 0.03091475
db_query_duration_seconds{quantile="0.5",store="api-tokens",action="getAllActive"} 0.03091475
db_query_duration_seconds{quantile="0.9",store="api-tokens",action="getAllActive"} 0.03091475
db_query_duration_seconds{quantile="0.95",store="api-tokens",action="getAllActive"} 0.03091475
db_query_duration_seconds{quantile="0.99",store="api-tokens",action="getAllActive"} 0.03091475
db_query_duration_seconds_sum{store="api-tokens",action="getAllActive"} 0.03091475
db_query_duration_seconds_count{store="api-tokens",action="getAllActive"} 1
# HELP feature_toggle_update_total Number of times a toggle has been updated. Environment label would be "n/a" when it is not available, e.g. when a feature toggle is created.
# TYPE feature_toggle_update_total counter
# HELP feature_toggle_usage_total Number of times a feature toggle has been used
# TYPE feature_toggle_usage_total counter
# HELP feature_toggles_total Number of feature toggles
# TYPE feature_toggles_total gauge
feature_toggles_total{version="5.3.0"} 31
# HELP users_total Number of users
# TYPE users_total gauge
users_total 1011
# HELP projects_total Number of projects
# TYPE projects_total gauge
projects_total 4
# HELP environments_total Number of environments
# TYPE environments_total gauge
environments_total 10
# HELP groups_total Number of groups
# TYPE groups_total gauge
groups_total 5
# HELP roles_total Number of roles
# TYPE roles_total gauge
roles_total 11
# HELP custom_root_roles_total Number of custom root roles
# TYPE custom_root_roles_total gauge
custom_root_roles_total 3
# HELP custom_root_roles_in_use_total Number of custom root roles in use
# TYPE custom_root_roles_in_use_total gauge
custom_root_roles_in_use_total 2
# HELP segments_total Number of segments
# TYPE segments_total gauge
segments_total 5
# HELP context_total Number of context
# TYPE context_total gauge
context_total 7
# HELP strategies_total Number of strategies
# TYPE strategies_total gauge
strategies_total 5
# HELP client_apps_total Number of registered client apps aggregated by range by last seen
# TYPE client_apps_total gauge
client_apps_total{range="allTime"} 0
client_apps_total{range="30d"} 0
client_apps_total{range="7d"} 0
# HELP saml_enabled Whether SAML is enabled
# TYPE saml_enabled gauge
saml_enabled 1
# HELP oidc_enabled Whether OIDC is enabled
# TYPE oidc_enabled gauge
oidc_enabled 0
# HELP client_sdk_versions Which sdk versions are being used
# TYPE client_sdk_versions counter
# HELP optimal_304_diffing Count the Optimal 304 diffing with status
# TYPE optimal_304_diffing counter
# HELP db_pool_min Minimum DB pool size
# TYPE db_pool_min gauge
db_pool_min 0
# HELP db_pool_max Maximum DB pool size
# TYPE db_pool_max gauge
db_pool_max 4
# HELP db_pool_free Current free connections in DB pool
# TYPE db_pool_free gauge
db_pool_free 0
# HELP db_pool_used Current connections in use in DB pool
# TYPE db_pool_used gauge
db_pool_used 4
# HELP db_pool_pending_creates how many asynchronous create calls are running in DB pool
# TYPE db_pool_pending_creates gauge
db_pool_pending_creates 0
# HELP db_pool_pending_acquires how many acquires are waiting for a resource to be released in DB pool
# TYPE db_pool_pending_acquires gauge
db_pool_pending_acquires 24
```
This PR does **one** thing:
it changes the events for potentially stale to:
- Only being emitted when potentially stale gets turned on
- In doing so, it also simplifies the event that's getting emitted,
removing the `data` property.
- The event is also renamed to better match the existing
`feature-stale-on` and `...-off` events.
The addon listening was broken out into a separate PR (#4279)
## Old description
This change lets all addons listen for events when features get marked
or unmarked as potentially stale.
### Discussion
#### All addons?
Should this be available to all addons? I can't see a reason why it
shouldn't be available to all addons, but I might be missing
something.
**Update**: spoke to a couple people. Can see no reason why this isn't
okay.
#### Should it be behind a flag?
The feature is still behind a flag, but the event type is not. Should
we gate the event being available until we actually emit the event?
That would require some more code, but could yield less potential
confusion.
Open to hearing your thoughts.
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
wrap reorder event creation to strategy variant feature
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
<!-- Does it close an issue? Multiple? -->
Closes #
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
Fixes a bug around createStrategy
Fix: Create/Store strategy reorder event only when feature is on
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
<!-- Does it close an issue? Multiple? -->
Closes #
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
This PR updates the feature type service by adding a new
`updateLifetime` method. This method handles the connection between the
API (#4256) and the store (#4252).
I've also added some new e2e tests to ensure that the API behaves as
expected.
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
When reordering strategies for a feature environment:
- Adds stop when CR are enabled
- Emits an event
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
<!-- Does it close an issue? Multiple? -->
Closes #
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
This PR activates the event emission that was prepared for in
https://github.com/Unleash/unleash/pull/4239.
It emits events (behind a flag) when something is marked as potentially
stale or the opposite.
It takes the features returned from the store and creates events out of
them.
The events only contain data, no preData. This is because the preData
can easily be inferred and because it gives a nicer event in the event
log.
Here is an image of the difference. The top event uses only data, so it
shows the name of the feature and the new potentiallyStale status. The
bottom event uses both preData and data, so it only shows the new
potentiallyStale status and not the feature name (unless you show the
raw event):
Should not be merged before #4239. Merge that and then rebase this off
main or cherry the commit.
## Discussion
### `preData`
Should we also use preData or is it enough to use only data? It seems
unnecessary in this event, but I'm open to hearing your thoughts.
### event author: `createdBy`
I've set `unleash-system` as the `createdBy` property on these events
because they are generated by the system. I found the same string used
some other places. However, it may be that there we want to use a
different author.
This PR adds updates the potentially stale status change events whenever
the potentially stale update function is run.
No events are emitted yet. While the emission is only a few lines of
code, I'd like to do that in a separate PR so that we can give it the
attention it deserves in the form of tests, etc.
This PR also moves the potentially stale update functionality from the
`update` method to only being done in the
`updatePotentiallyStaleFeatures` method. This keeps all functionality
related to marking `potentiallyStale` in one place.
The emission implementation was removed in
4fb7cbde03
## The update queries
While it would be possible to do the state updates in a single query
instead of three separate ones, wrangling this into knex proved to be
troublesome (and would also probably be harder to understand and reason
about). The current solution uses three smaller queries (one select, two
updates), as Jaanus suggested in a private slack thread.
This reverts commit 16e3799b9a.
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
<!-- Does it close an issue? Multiple? -->
Closes #
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
https://linear.app/unleash/issue/2-1232/implement-first-iteration-of-the-new-slack-app-addon
This PR implements the first iteration of the new Slack App addon.
Unlike the old Slack addon, this one uses a Slack App (bot) that is
installed to Slack workspaces in order to post messages. This uses
`@slack/web-api`, which internally uses the latest Slack API endpoints
like `postMessage`.
This is currently behind a flag: `slackAppAddon`.
The current flow is that the Unleash Slack App is installed from
whatever source:
- Unleash addons page;
- Direct link;
- https://unleash-slack-app.vercel.app/ (temporary URL);
- Slack App Directory (in the future);
- Etc;
After installed, we resolve the authorization to an `access_token` that
the user can paste into the Unleash Slack App addon configuration form.
https://github.com/Unleash/unleash/assets/14320932/6a6621b9-5b8a-4921-a279-30668be6d46c
Co-authored by: @daveleek
---------
Co-authored-by: David Leek <david@getunleash.io>
This PR lays most of the groundwork required for emitting events when
features are marked as potentially stale by Unleash. It does **not**
emit any events just yet. The summary is:
- periodically look for features that are potentially stale and mark
them (set to run every 10 seconds for now; can be changed)
- when features are updated, if the update data contains changes to the
feature's type or createdAt date, also update the potentially stale
status.
It is currently about 220 lines of tests and about 100 lines of
application code (primarily db migration and two new methods on the
IFeatureToggleStore interface).
The reason I wanted to put this into a single PR (instead of just the db
migration, then just the potentially stale marking, then the update
logic) is:
If users get the db migration first, but not the rest of the update
logic until the events are fired, then they could get a bunch of new
events for features that should have been marked as potentially stale
several days/weeks/months ago. That seemed undesirable to me, so I
decided to bunch those changes together. Of course, I'd be happy to
break it into smaller parts.
## Rules
A toggle will be marked as potentially stale iff:
- it is not already stale
- its createdAt date is older than its feature type's expected lifetime
would dictate
## Migration
The migration adds a new `potentially_stale` column to the features
table and sets this to true for any toggles that have exceeded their
expected lifetime and that have not already been marked as `stale`.
## Discussion
### The `currentTime` parameter of `markPotentiallyStaleFeatures`
The `markPotentiallyStaleFetaures` method takes an optional
`currentTime` parameter. This was added to make it easier to test (so
you can test "into the future"), but it's not used in the application.
We can rewrite the tests to instead update feature toggles manually, but
that wouldn't test the actual marking method. Happy to discuss.
## About the changes
Fix un-awaited promise on batch variant update - reduce function allowed
TS to skip Promise type.
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
Wraps the whole `registerClientMetrics` function with try/catch to
return 400 on error
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
<!-- Does it close an issue? Multiple? -->
Closes #
[1-1037](https://linear.app/unleash/issue/1-1037/return-4xx-error-for-incorrect-metrics-input)
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
## About the changes
- Adding descriptions and examples to tag and tag types schemas
- Adding standard errors, summaries, and descriptions to tag and tag
types endpoints
- Some improvements on compilation errors
---------
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
In some of the places we used `NoAccessError` for permissions, other
places we used it for a more generic 403 error with a different
message. This refactoring splits the error type into two distinct
types instead to make the error messages more consistent.
This PR fixes an issue where events generated during a db transaction
would get published before the transaction was complete. This caused
errors in some of our services that expected the data to be stored
before the transaction had been commited. Refer to [linear issue
1-1049](https://linear.app/unleash/issue/1-1049/event-emitter-should-emit-events-after-db-transaction-is-commited-not)
for more info.
Fixes 1-1049.
## Changes
The most important change here is that the `eventStore` no longer emits
events when they happen (because that can be in the middle of a
transaction). Instead, events are stored with a new `announced` column.
The new event announcer service runs on a schedule (every second) and
publishes any new events that have not been published.
Parts of the code have largely been lifted from the
`client-application-store`, which uses a similar logic.
I have kept the emitting of the event within the event store because a
lot of other services listen to events from this store, so removing that
would require a large rewrite. It's something we could look into down
the line, but it seems like too much of a change to do right now.
## Discussion
### Terminology:
Published vs announced? We should settle on one or the other. Announced
is consistent with the client-application store, but published sounds
more fitting for events.
### Publishing and marking events as published
The current implementation fetches all events that haven't been marked
as announced, sets them as announced, and then emits them. It's possible
that Unleash would crash in the interim or something else might happen,
causing the events not to get published. Maybe it would make sense to
just fetch the events and only mark them as published after the
announcement? On the other hand, that might get us into other problems.
Any thoughts on this would be much appreciated.
## What
This adds openapi documentation for the Auth tagged operations and
connected schemas.
## Discussion points
Our user schema seems to be exposing quite a bit of internal fields, I
flagged the isApi field as deprecated, I can imagine quite a few of
these fields also being deprecated to prepare for removal in next major
version, but I was unsure which ones were safe to do so with.
## Observation
We have some technical debt around the shape of the schema we're
claiming we're returning and what we actually are returning. I believe
@gastonfournier also observed this when we turned on validation for our
endpoints.
---------
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
This removes the experimental feature flag that defaulted to turn off
telemetry collection
This PR adds strategy titles as an optional bit of data added to client
features. It's only added when prompted.
## Discussion points:
### getPlaygroundFeatures
The optional `includeStrategyId` parameter has been replaced by a
`getPlaygroundFeatures` in the service (and in the underlying store).
The playground was the only place that used this specific include, so
instead of adding more and making the interface for that method more
complex, I created a new method that deals specifically with the
playground.
The underlying store still uses an `optionalIncludes` parameter,
however. I have a plan to make that interface more fluid, but I'd like
to propose that in a follow-up PR.
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
Remove strategy improvements flag
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
<!-- Does it close an issue? Multiple? -->
Closes #
[1-1048](https://linear.app/unleash/issue/1-1048/remove-strategyimprovements-flag)
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
## About the changes
`getUserRootRoles` should also consider custom root roles
This introduces test cases that unveiled a dependency between stores
(this happens actually at the DB layer having access-service access
tables from two different stores but skipping the store layer).
https://linear.app/unleash/issue/2-1161/a-user-with-custom-root-role-and-permission-to-create-client-api
---------
Co-authored-by: Nuno Góis <github@nunogois.com>
## What
As part of the move to enable custom-root-roles, our permissions model
was found to not be granular enough to allow service accounts to only be
allowed to create read-only tokens (client, frontend), but not be
allowed to create admin tokens to avoid opening up a path for privilege
escalation.
## How
This PR adds 12 new roles, a CRUD set for each of the three token types
(admin, client, frontend). To access the `/api/admin/api-tokens`
endpoints you will still need the existing permission (CREATE_API_TOKEN,
DELETE_API_TOKEN, READ_API_TOKEN, UPDATE_API_TOKEN). Once this PR has
been merged the token type you're modifying will also be checked, so if
you're trying to create a CLIENT api-token, you will need
`CREATE_API_TOKEN` and `CREATE_CLIENT_API_TOKEN` permissions. If the
user performing the create call does not have these two permissions or
the `ADMIN` permission, the creation will be rejected with a `403 -
FORBIDDEN` status.
### Discussion points
The test suite tests all operations using a token with
operation_CLIENT_API_TOKEN permission and verifies that it fails trying
to do any of the operations against FRONTEND and ADMIN tokens. During
development the operation_FRONTEND_API_TOKEN and
operation_ADMIN_API_TOKEN permission has also been tested in the same
way. I wonder if it's worth it to re-add these tests in order to verify
that the permission checker works for all operations, or if this is
enough. Since we're running them using e2e tests, I've removed them for
now, to avoid hogging too much processing time.
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
Adds an environment variable for switching off feature telemetry in
version check
## About the changes
Implements custom root roles, encompassing a lot of different areas of
the project, and slightly refactoring the current roles logic. It
includes quite a clean up.
This feature itself is behind a flag: `customRootRoles`
This feature covers root roles in:
- Users;
- Service Accounts;
- Groups;
Apologies in advance. I may have gotten a bit carried away 🙈
### Roles
We now have a new admin tab called "Roles" where we can see all root
roles and manage custom ones. We are not allowed to edit or remove
*predefined* roles.
This meant slightly pushing away the existing roles to `project-roles`
instead. One idea we want to explore in the future is to unify both
types of roles in the UI instead of having 2 separate tabs. This
includes modernizing project roles to fit more into our current design
and decisions.
Hovering the permissions cell expands detailed information about the
role:
### Create and edit role
Here's how the role form looks like (create / edit):
Here I categorized permissions so it's easier to visualize and manage
from a UX perspective.
I'm using the same endpoint as before. I tried to unify the logic and
get rid of the `projectRole` specific hooks. What distinguishes custom
root roles from custom project roles is the extra `root-custom` type we
see on the payload. By default we assume `custom` (custom project role)
instead, which should help in terms of backwards compatibility.
### Delete role
When we delete a custom role we try to help the end user make an
informed decision by listing all the entities which currently use this
custom root role:
~~As mentioned in the screenshot, when deleting a custom role, we demote
all entities associated with it to the predefined `Viewer` role.~~
**EDIT**: Apparently we currently block this from the API
(access-service deleteRole) with a message:
What should the correct behavior be?
### Role selector
I added a new easy-to-use role selector component that is present in:
- Users
- Service Accounts
- Groups
### Role description
I also added a new role description component that you can see below the
dropdown in the selector component, but it's also used to better
describe each role in the respective tables:
I'm not listing all the permissions of predefined roles. Those simply
show the description in the tooltip:
### Role badge
Groups is a bit different, since it uses a list of cards, so I added yet
another component - Role badge:
I'm using this same component on the profile tab:
## Discussion points
- Are we being defensive enough with the use of the flag? Should we
cover more?
- Are we breaking backwards compatibility in any way?
- What should we do when removing a role? Block or demote?
- Maybe some existing permission-related issues will surface with this
change: Are we being specific enough with our permissions? A lot of
places are simply checking for `ADMIN`;
- We may want to get rid of the API roles coupling we have with the
users and SAs and instead use the new hooks (e.g. `useRoles`)
explicitly;
- We should update the docs;
- Maybe we could allow the user to add a custom role directly from the
role selector component;
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
Adds feature usage info and custom strategy counters to the version
check object.
<!-- Does it close an issue? Multiple? -->
Closes #
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
