https://linear.app/unleash/issue/SR-169/ticket-1107-project-feature-flag-limit-is-not-correctly-updatedFixes#5315, an issue where it would not be possible to set an empty
flag limit.
This also fixes the UI behavior: Before, when the flag limit field was
emptied, it would disappear from the UI.
I'm a bit unsure of the original intent of the `(data.defaultStickiness
!== undefined || data.featureLimit !== undefined)` condition. We're in
an update method, triggered by a PUT endpoint - I think it's safe to
assume that we'll always want to set these values to whatever they come
as, we just need to convert them to `null` in case they are not present
(i.e. `undefined`).
This fixes an edge case not caught originally in
https://github.com/Unleash/unleash/pull/5304 - When creating a new
segment on the global level:
- There is no `projectId`, either in the params or body
- The `UPDATE_PROJECT_SEGMENT` is still a part of the permissions
checked on the endpoint
- There is no `id` on the params
This made it so that we would run `segmentStore.get(id)` with an
undefined `id`, causing issues.
The fix was simply checking for the presence of `params.id` before
proceeding.
This PR hooks up the changes introduced in #5301 to the API and puts
them behind a feature flag. A new test has been added and the test setup
has been slightly tweaked to allow this test.
When the flag is enabled, the API will now not let you delete a segment
that's used in any active CRs.
https://linear.app/unleash/issue/SR-164/ticket-1106-user-with-createedit-project-segment-is-not-able-to-edit-a
Fixes a bug where the `UPDATE_PROJECT_SEGMENT` permission is not
respected, both on the UI and on the API. The original intention was
stated
[here](https://github.com/Unleash/unleash/pull/3346#discussion_r1140434517).
This was easy to fix on the UI, since we were simply missing the extra
permission on the button permission checks.
Unfortunately the API can be tricky. Our auth middleware tries to grab
the `project` information from either the params or body object, but our
`DELETE` method does not contain this information. There is no body and
the endpoint looks like `/admin/segments/:id`, only including the
segment id.
This means that, in the rbac middleware when we check the permissions,
we need to figure out if we're in such a scenario and fetch the project
information from the DB, which feels a bit hacky, but it's something
we're seemingly already doing for features, so at least it's somewhat
consistent.
Ideally what we could do is leave this API alone and create a separate
one for project segments, with endpoints where we would have project as
a param, like so:
`http://localhost:4242/api/admin/projects/:projectId/segments/1`.
This PR opts to go with the quick and hacky solution for now since this
is an issue we want to fix quickly, but this is something that we should
be aware of. I'm also unsure if we want to create a new API for project
segments. If we decide that we want a different solution I don't mind
either adapting this PR or creating a follow up.
This test was flaky because it relied on the order of the array
returned. To make it less flaky, we now turn the array into an object
instead and compare that.
This PR adds a way to tell if a specific segment is being used in any
active change requests. It's the first step towards preventing segments
that are being used in change requests from being deleted.
It does that by checking the db for any unclosed CRs and using those CR
ids to look for "addStrategy" and "updateStrategy" events in the cr
events table.
## Upcoming PRs
This only puts in a way to detect it, but doesn't add that to anything.
That'll be in an upcoming iteration.
The `dataPath` was present (but not in the type) in previous versions of
the
error library that we use. But with the recent major upgrade, it's
been removed and the `instancePath` property has finally come into use.
This PR removes all the handling for the previous property and
replaces it with `instancePath`. Because the `dataPath` used full
stops and the `instancePath` uses slashes, we need to change a little
bit of the handling too.
Switch the express-openapi implementation from our internal fork to the
upstream version. We have upstreamed our changes and a new version has
been released, so this should be the last step before we can retire our
fork.
Because some of the dependencies have been updated since our internal
fork, we also need to update some of our error handling to reflect this.
Expose new interface while also getting rid of unneeded compiler ignores
None of the changes should add new security risks, despite this report:
> Code scanning results / CodeQL Failing after 4s — 2 new alerts
including 2 high severity security vulnerabilities
Not sure what that means, maybe a removed ignore...
Sort the items before inserting them into the database in order to
reduce the chance of deadlocks happening when multiple pods are
inserting at the same time.
For a while we ran a diffing algorithm in production to verify that the
results of the refactor did not differ from the previous results. As the
experiment has run it's course and new attributes have been added on top
of the new flow, this will remove the logging and associated code.
`EXECUTE FUNCTION` was introduced in Postgres v11. In Postgres v10 the
syntax was `EXECUTE PROCEDURE`. This fix changes the syntax to `EXECUTE
PROCEDURE`, which is perfectly fine sense our function does not return
anything.
### What
This PR makes the rate limit for user creation and simple login (our
password based login) configurable in the same way you can do
metricsRateLimiting.
### Worth noting
In addition this PR adds a `rate_limit{endpoint, method}` prometheus
gauge, which gets the data from the UnleashConfig.
This PR adds a db table for CR schedules. The table has two columns:
1. `change_request` :: This acts as both a foreign key and as the
primary key for this table.
2. `scheduled_at` :: When the change is scheduled to be applied.
We could use a separate ID column for these rows and put a `unique`
constraint on the `change_request` FK, but I don't think that adds any
more value. However, I'm happy to hear other thoughts around it.
As #4475 says, MD5 is not available in secure places anymore. This PR
swaps out gravatar-url with an inline function using crypto:sha256 which
is FIPS-140-2 compliant. Since we only used this method for generating
avatar URLs the extra customization wasn't needed and we could hard code
the URL parameters.
fixes: Linear
https://linear.app/unleash/issue/SR-112/gh-support-swap-out-gravatar-url-libcloses: #4475
To prepare for 5.6 GA,
I've done a find through both Frontend and Backend here to remove the
usages of the flag. Seems like the flag was only in use in the frontend.
@nunogois can you confirm?
This PR adds a cleanup job that removes unknown feature flags from
last_seen_at_metrics table every 24 hours since we no longer have a
foreign key on the name column in the features table.
## About the changes
This fixes a bug updating a project, when optional data
(defaultStickiness and featureLimit are not part of the payload).
The problem happens due to:
1. ProjectController does not use the type: UpdateProjectSchema for the
request body (will be addressed in another PR in unleash-enterprise)
2. Project Store interface does not match UpdateProjectSchema (but it
relies on accepting `additional properties: true`, which is what we
agreed on for input)
3. Feature limit is not defined in UpdateProjectSchema (also addressed
in the other PR)
https://linear.app/unleash/issue/2-1531/rename-message-banners-to-banners
This renames "message banners" to "banners".
I also added support for external banners coming from a `banner` flag
instead of only `messageBanner` flag, so we can eventually migrate to
the new one in the future if we want.
## About the changes
This makes sure that projects have at least one owner, either a group or
a user. This is to prevent accidentally losing access to a project.
We check this when removing a user/group or when changing the role of a
user/group
**Note**: We can still leave a group empty as the only owner of the
project, but that's okay because we can still add more users to the
group
Sort array items before running compare. Feature flag certain properties
of strategy that were previously not present in the /api/admin/features
endpoint.
### What
The heaviest requests we serve are the register and metrics POSTs from
our SDKs/clients.
This PR adds ratelimiting to /api/client/register, /api/client/metrics,
/api/frontend/register and /api/frontend/metrics with a default set to
6000 requests per minute (or 100 rps) for each of the endpoints.
It will be overrideable by the environment variables documented.
### Points of discussion
@kwasniew already suggested using featuretoggles with variants to
control the rate per clientId. I struggled to see if we could
dynamically update the middleware after initialisation, so this attempt
will need a restart of the pod to update the request limit.
## About the changes
This small improvement aims to help developers when instantiating
services. They need to be constructed without injecting services or
stores created elsewhere so they can be bound to the same transactional
scope.
This suggests that you need to create the services and stores on your
own
This fixes a return type error by changing the logic of
`extractUsernameFromUser` to never return undefined.
In the previous code, `user` could be truthy, but that doesn't mean
`email` or `username` were defined. This assumes we always fallback to
"unknown" in those scenarios.
This PR is the first step in separating the client and admin stores.
Currently our feature toggle services uses the client store to serve
multiple purposes.
Admin API uses the feature toggle service to serve both the feature
toggle list and playground features, while the client API uses the
feature toggle service to serve client features. The admin API can
change often and have very different requirements than the client API,
which changes infrequently and generally keeps the same stable structure
for long periods of time. This architecture is error prone, because when
you need to make changes to the admin API, you can very easily affect
the client API.
I aim to put up a stone wall between the two APIs. Complete separation
between the two APIs, at the cost of some duplication.
In this PR I have created a feature oriented architecture for client
features and disconnected the client API from the feature toggle
service. It now goes through it's own service to it's own store. For
feature toggle service I have duplicated and replaced the functionality
that serves /api/admin/features, I have kept a lot of the ugliness in
the code and haven't removed anything in order to avoid breaking
changes.
Next steps:
* Move playground to admin API
* Remove client-feature-toggle-store from feature-toggle-service
## About the changes
This splits the interfaces for import and export, especially because the
import functionality has to be replaced in enterprise repo.
This is a breaking change because of the service renames, but I'll have
the PR for the other repository ready so we reduce the time to fix. I
intentionally avoided doing it backward compatible because of time.
https://linear.app/unleash/issue/2-1494/re-order-message-banners
- Re-orders message banners to fit into this logic:
>1. Maintenance banner
>2. External message banner(s) - Most likely coming from Unleash
>3. Internal message banner(s)
- Renames the feature flag to better reflect the feature behavior;
- Lays a basic skeleton structure for this new feature;
As part of more telemetry on the usage of Unleash.
This PR adds a new `stat_` prefixed table as well as a trigger on the
events table trigger on each insert to increment a counter per
environment per day.
The trigger will trigger on every insert into the events base, but will
filter and only increment the counter for events that actually have the
environment set. (there are events, like user-created, that does not
relate to a specific environment).
Bit wary on this, but since we truncate down to row per (day,
environment) combo, finding conflict and incrementing shouldn't take too
long here.
@ivarconr was it something like this you were considering?
This PR cleans up and refactors the feature-strategy-store method
getFeatureOverview to join on the new table and attempts to make the
function more readable by extracting some of the logic into separate
functions. Keeping the LastSeenMapper for now in case there is a reason
to use it for the other endpoints.
## About the changes
Segment changes in predata and data columns were both showing the new
segments list
Adds formatting of what's changed with segments to feature strategy
update events, so when a user changes the strategy from using
constraints, to using segments instead, it's communicated in event
updates
results in:
admin updated
[sample-toggle](http://localhost/projects/default/features/sample-toggle)
in project [default](http://localhost/projects/default) by updating
strategy Sample Strategy in development constraints from [userId is one
of (1,2,3)] to empty set of constraints; segments from empty set of
segments to (1)
Closes #
#4912
### Important files
- `src/lib/services/feature-toggle-service.ts` - Segment changes in
preData and data
- `src/lib/addons/feature-event-formatter-md.ts` - Formatting segments
## Discussion points
This is an SR least effort PR - we should plan a task where we look at
how to render this list of segments in a more comprehensible way (it's
just rendering ids now)
Fixes an issue where SSO group sync would delete a syncable group that a
user was manually added to
## Discussion points
Is this the longterm fix for this? Or would we want another column in
the mapping table for future-proofing this?
## About the changes
This transactional implementation decorates a service with a
transactional method that removes the need to start transactions in the
method using the service.
This is a gradual rollout with a feature toggle, just because
transactions are not easy.
