If the kill switch is enabled unleash returns 404 and a json body explaining why a 404 was given, encouraging users to upgrade to the most recent version of Edge.
## About the changes
Creating an incoming webhook with an admin token means we can't
correlate the action with a real user. In this case we should support
null.
Was having some trouble running these migration tests locally due to
`dbm` not correctly picking up the passed in config. This fixes it by
setting the custom config property after it has been initialized, always
overriding any wrong values.
PS: I think I found the issue. `dbm` was prioritizing my `DATABASE_URL`
for some reason, as I started having issues when it was set, and stopped
having issues when I unset it.
I still think this is a good change, as it prevents similar
hard-to-debug issues in the future.
To help clarify this, running this locally:
- `export
DATABASE_URL=postgres://unleash_user:passord@localhost:5432/unleash`
- `yarn test dedupe-permissions`
Fails on `main`, but passes on this branch. For some reason the `dbm`
instance prioritizes whatever is set in `DATABASE_URL` instead of the
options that are passed in `getInstance`.
We've had a couple of misunderstandings from people surprised that
Unleash allows posts against the `/edge/validate` endpoint without an
API key. It is intentional that this endpoint does not require an
Authorization header, so this PR updates our OpenAPI spec to clarify
that there is no security required for `/edge/validate`
## About the changes
Migrations for:
- Adds column is_system to users
- Inserts unleash_system_user id -1337 to users
includes `is_system: false` in the activeUsers and activeAccounts where filter
Tested by running:
`
select * into users_pre_check from users where id > -1;
delete from users where id > -1;
`
before starting unleash, then inspecting users table after unleash has
started and verifying that an 'admin' user has been created.
---------
Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>
This backwards compatible change allows us to specify a schema `id`
(full path) which to me feels a bit better than specifying the schema
name as a string, since a literal string is prone to typos.
### Before
```ts
requestBody: createRequestSchema(
'createResourceSchema',
),
responses: {
...getStandardResponses(400, 401, 403, 415),
201: resourceCreatedResponseSchema(
'resourceSchema',
),
},
```
### After
```ts
requestBody: createRequestSchema(
createResourceSchema.$id,
),
responses: {
...getStandardResponses(400, 401, 403, 415),
201: resourceCreatedResponseSchema(
resourceSchema.$id,
),
},
```
With the recent changes it's common that we'll need both the id and
processed username from the auth user in the request, so this PR
provides some helper methods to simplify this.
## About the changes
Adds the new nullable column created_by_user_id to the data used by
feature-tag-store and feature-tag-service. Also updates openapi schemas.
## About the changes
Replaces #5616
Renamed newly added `created_by` columns to `created_by_user_id` for
these tables:
features
feature_tag
feature_strategies
feature_types
role_permission
role_user
roles
users
api_tokens
Two changes were needed to sort better
1. Since we are still using `last seen` from `features` table for
backwards compatibility, we needed to add it to sort condition.
2. Nulls break the order, so now sorting nulls as last.
I noticed I was getting warnings logged in my local instance when
visiting the users page (`/admin/users`)
```json
{
"schema": "#/components/schemas/publicSignupTokensSchema",
"errors": [
{
"instancePath": "/tokens/0/users/0/username",
"schemaPath": "#/components/schemas/userSchema/properties/username/type",
"keyword": "type",
"params": {
"type": "string"
},
"message": "must be string"
}
]
}
```
It was complaining because one of my users doesn't have a username, so
the value returned from the API was:
```json
{
"users": [
{
"id": 2,
"name": "2mas",
"username": null
}
]
}
```
This adjustment fixes that oversight by allowing `null` values for the
username.
## Why
Currently AWS API Gateway doesn't have compression enabled by default,
this PR will make it easier to for example deploy Unleash over to AWS
Lambda without further configuration in API Gateway, frameworks like
Serverless requires a bit more work to set up compression and some times
one might not need compression at all.
## How
Create a new config flag called `disableCompression` which will not
include `compression` middleware in express' instance when set as true.
## About the changes
This adds a Makefile to make it easy to test migrations from one version
of Unleash to another.
The script depends on [docker compose
V2](https://docs.docker.com/compose/migrate/)
**Before starting**: make sure you're inside test-migrations folder and
run `make clean` to be in a clean state.
We can run 2 versions of Unleash side by side with a shared database
(the second version will apply migrations to the DB):
```shell
UNLEASH_DOCKER_IMAGE=unleashorg/unleash-server:5.6.10 make start-unleash # defaults to port 4242
UNLEASH_DOCKER_IMAGE=unleashorg/unleash-server:latest make start-another-unleash # defaults to port 4243
make test # run basic UI tests against port 4242 (first image)
EXPOSED_PORT=4243 make test # run basic UI tests against port 4243
```
This also enables us to test our local repository with our code of
Unleash server running at port 4244 (`EXPOSE_PORT=4444 make run-current`
if you want to change it):
```shell
UNLEASH_DOCKER_IMAGE=unleashorg/unleash-server:5.6.10 make start-unleash # defaults to port 4242
make run-current # exposes the current backend at 4244
```
You can also connect the latest UI to any of the ports specified above,
starting the UI at port 3000:
```shell
EXPOSED_PORT=4242 make run-current-ui # exposed port defaults to 4244 which is the port of the current backend
```
### What
Adds `createdByUserId` to all events exposed by unleash. In addition
this PR updates all tests and usages of the methods in this codebase to
include the required number.
## About the changes
Adds the column `created_by_user_id` to `events` table and adds index
for it
---------
Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>
This PR fixes the issue discussed in SR-234, where you would get a 200
OK response even if your POST request to
`/api/admin/projects/<project-name>/access` contains invalid data (and
nothing is persisted).
Today we include a lot of "secutiry headers" for all API calls. Quite a
lot of them are only relevent when we return a HTML document for the
browser.
This PR removes and simplify these headers for API calls, so that we do
not include unecessary data in the HTTP headers.
Each header have been carfully examied by following best practices from
these source:
-
https://cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html
- https://owasp.org/www-project-secure-headers/
This feature is protected with feature flag named 'stripHeadersOnAPI'.
As it says on the tin. In an attempt to make all operations in Unleash
traceable to an originator. This PR adds created_by to role_permission,
which will show which user assigned a permission to a role.
This change adds a property to the segmentStrategiesSchema to make sure
that change request strategies are listed in the openapi spec
It also renames the files that contains that schema and its tests from
`admin-strategies-schema` to `segment-strategies-schema`.
Adding new project overview endpoint and deprecating the old one.
The new one has extra info about feature types, but does not have
features anymore, because features are coming from search endpoint.
## About the changes
Add user ids to group changes. This also modifies the payload of group created to include only the user id and creates events for SSO sync functionality
This adds more data to the setting events, so that its possible to see
what has changed
Used to look like:
```
{
"id": "maintenance.mode"
}
```
Now it looks like this:
```
{
"id": "maintenance.mode",
"enabled": false
}
```
because this is setting events, the default behaviour is to hide the content.
This PR checks that the unleash instance is an enterprise instance
before fetching change request data. This is to prevent Change Request
usage from preventing OSS users from deleting segments (when they don't
have access to change requests).
This PR also does a little bit of refactoring (which we can remove if
you want)
This PR updates the returned value about segments to also include the CR
title and to be one list item per strategy per change request. This
means that if the same strategy is used multiple times in multiple
change requests, they each get their own line (as has been discussed
with Nicolae).
Because of this, this pr removes a collection step in the query and
fixes some test cases.
The previous check would return `false` if the value was 0, causing a
bug where the usage data wouldn't be included.
This also adds tests to ensure that usage data for CR segments is
propagated correctly because that's where I first encountered the issue.
Before this fix, if the values were 0, the data would display like the
bottom element in the screenshot:
- Create 2 new events to replace the SCHEDULED_CHANGE_REQUEST_EXECUTED
event
- Handle the 3 events in slack-app and webhook addon definitions
3 events handled:
- CHANGE_REQUEST_SCHEDULED
- CHANGE_REQUEST_SCHEDULED_APPLICATION_SUCCESS
- CHANGE_REQUEST_SCHEDULED_APPLICATION_FAILURE
Closes #
[1-1555](https://linear.app/unleash/issue/1-1555/update-change-request-scheduled-and-scheduled-change-request-executed)
Note: SCHEDULED_CHANGE_REQUEST_EXECUTED will be removed in follow up PR
not to break current enterprise build
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
This PR addresses some cleanup related to removing the
useLastSeenRefactor flag:
* Added fallback last seen to the feature table last_seen_at column
* Remove foreign key on environment since we can not guarantee that we
will get valid data in this field
* Add environments to cleanup function
* Add test for cleanup environments
This PR changes the behavior of the API a little bit. Instead of
removing any strategies from `changeRequestStrategies` that are also
in `strategies`, we keep them in instead.
The reason for this is that the overview of where a segment is used is
incomplete if it shows only strategies but not CRs. Imagine this:
You want to delete a segment, but you're told it's only used in strategy
S.
So you go and remove it from strategy S, but then you're told it's
suddenly used in CRs A, B, and C. This is now a two-step operation
with a bad surprise. Instead, we could show you immediately that this
segment is used in strategy S and CRs A, B, and C.
Otherwise, we might accidentally display CR data to open source users.
But more importantly, it might keep them from being able to delete a
segment that's in use by a CR in their database that they can't touch.
So by checking that they're on an enterprise instance, we avoid this
potential blocker.
I've added the `includeChangeRequestUsageData` parameter as a boolean
now, but I'm open to other suggestions.
This PR handles the case where a single strategy is used in multiple
change requests. Instead of listing the strategy several times in the
output, we consolidate the entries and add a new `changeRequestIds`
property. This is a non-empty list that points to all the change
requests it is used in.
This is required for us to be able to link back to the change requests
from the UI overview.
This change is just a refactor, removing code that's no longer used. Instead of
checking just whether a segment is in use, we now extract the list of
strategies that use this segment. This is slightly more costly,
perhaps, but it will be necessary for the upcoming implementation.
This PR changes the payload of the strategiesBySegment endpoint when the
flag is active. In addition to returning just the strategies, the object
will also contain a new property, called `changeRequestStrategies`
containing the strategies that are used in change requests.
This PR does not update the schema. That can be done later when the
changes go into beta. This also allows us some time to iterate on the
payload without changing the public API.
## Discussion points:
Should `strategies` and `changeRequestStrategies` ever contain
duplicates? Take this scenario:
- Strategy S uses segment T.
- There is an open change request that updates the list of segments for
S to T and a new segment U.
- In this case, strategy S would show up both in `strategies` _and_ in
`changeRequestStrategies`.
We have two options:
1. Filter the list of change request strategies, so that they don't
contain any duplicates (this is currently how it's implemented)
2. Ignore the duplicates and just send both lists as is.
We're doing option 2 for now.
Removing a user from a project was impossible if you only had 1 owner.
It worked fine when having more than an owner. This should fix it and
we'll add tests later
In short the issue is that after our last seen improvements, we did not
update where we are getting last_seen field. It was still using features
table, which is not the source of last seen anymore.
## PR Description
https://linear.app/unleash/issue/2-1645/address-post-mortem-action-point-all-flags-should-be-runtime
Refactor with the goal of ensuring that flags are runtime controllable,
mostly focused on the current scheduler logic.
This includes the following changes:
- Moves scheduler into its own "scheduler" feature folder
- Reverts dependency: SchedulerService takes in the MaintenanceService,
not the other way around
- Scheduler now evaluates maintenance mode at runtime instead of relying
only on its mode state (active / paused)
- Favors flag checks to happen inside the scheduled methods, instead of
controlling whether the method is scheduled at all (favor runtime over
startup)
- Moves "account last seen update" to scheduler
- Updates tests accordingly
- Boyscouting
Here's a manual test showing this behavior, where my local instance was
controlled by a remote instance. Whenever I toggle `maintenanceMode`
through a flag remotely, my scheduled functions stop running:
https://github.com/Unleash/unleash/assets/14320932/ae0a7fa9-5165-4c0b-9b0b-53b9fb20de72
Had a look through all of our current flags and it *seems to me* that
they are all used in a runtime controllable way, but would still feel
more comfortable if this was double checked, since it can be complex to
ensure this.
The only exception to this was `migrationLock`, which I believe is OK,
since the migration only happens at the start anyways.
## Discussion / Questions
~~Scheduler `mode` (active / paused) is currently not *really* being
used, along with its respective methods, except in tests. I think this
could be a potential footgun. Should we remove it in favor of only
controlling the scheduler state through maintenance mode?~~ Addressed in
7c52e3f638
~~The config property `disableScheduler` is still a startup
configuration, but perhaps that makes sense to leave as is?~~
[Answered](https://github.com/Unleash/unleash/pull/5363#issuecomment-1819005445)
by @FredrikOseberg, leaving as is.
Are there any other tests we should add?
Is there anything I missed?
Identified some `setInterval` and `setTimeout` that may make sense to
leave as is instead of moving over to the scheduler service:
- ~~`src/lib/metrics` - This is currently considered a `MetricsMonitor`.
Should this be refactored to a service instead and adapt these
setIntervals to use the scheduler instead? Is there anything special
with this we need to take into account? @chriswk @ivarconr~~
[Answered](https://github.com/Unleash/unleash/pull/5363#issuecomment-1820501511)
by @ivarconr, leaving as is.
- ~~`src/lib/proxy/proxy-repository.ts` - This seems to have a complex
and specific logic currently. Perhaps we should leave it alone for now?
@FredrikOseberg~~
[Answered](https://github.com/Unleash/unleash/pull/5363#issuecomment-1819005445)
by @FredrikOseberg, leaving as is.
- `src/lib/services/user-service.ts` - This one also seems to be a bit
more specific, where we generate new timeouts for each receiver id.
Might not belong in the scheduler service. @Tymek
This PR adds the ability to detect which strategies use a specific
segment in active change requests.
It does not wire this functionality up to anything just yet. Follow-up
PRs will integrate this with the segment service and eventually with the
front end.
The issue was that we all features were created exactly in same time,
and our feature counter waas expecting time to be unique to feature,
which was not the case.
Instead of throwing an error when the project doesn't exist, we say that
the names are valid, because we have nothing to say that they're not.
Presumably there is already something in place to prevent you from
importing into a non-existent project.
## About the changes
This feature allows our Enterprise customers to configure banners to be
displayed on their Unleash instance for all their users to see and
interact with. Previously known as "internal message banners".
