v5 - Breaking changes - DRAFT

[ivarconr]

This issue is used to highlight important changes for v5. We have not decided on when we will release v5 of Unleash, but this issue will allow us to collect all breaking changes we want to do.

Please not that this is not a commitment on our side, and more an indication of items we are currently discussing and possible impacts for that.

• DROP "/api/admin/features" API (link) - Does not support environments, custom permission, etc. Replaced with the new new features api, which supports environments (link)
• DROP express and migrate to fastify. - faster, more secure, and will allow us to automatically generate Open API specifications.
• DROP eventHook - its out-dated, does not support all event types etc. You should use web-hook or the event-store instead.
• Default to node.js v16 and drop support for anything below v14
• DROP direct support for Auth With Google (SSO) - Not needed as Sign-In with google supports OIDC out of the box. ... more to come
• Migrate to ES modules. This will break CommonJS (require...) support
• Change default password for Unleash DB connections from passord to password to avoid confusion

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[thomasheartman]

I'd like to propose a few things:

1. Align APIs on how you PUT vs POST vs PATCH and what the different methods do. That means that PUT should do a complete replace, and that a PATCH is what you do for partial updates.
2. Reconsider the PATCH format we used. We discussed this during the PUT discussions previously. PATCHing is hard because the JSON patch syntax is honestly a bit of a hassle. Instead, we should use an alternative format. A suggestion was to simply send pure JSON data and do a regular merge, leaving untouched fields unchanged, and overwriting any fields you pass in.
3. Do more API-side validation of values passed in instead of blindly accepting them: this includes things such as numbers that are invalid in a given context, strings that don't match some casing, etc. The most recent example I can think of was an issue about passing invalid weights to the variants API.
4. Check the various APIs against other REST standards we might want to follow. OpenAPI already forces us to do some of this, but it means aligning on return status codes and what they contain (what's a 200 vs a 201, for instance).

[ivarconr]

Migrate to ES modules. This will break CommonJS (require...) support

We need to validate that this will work with jest. In the past we have had some issues with this.

[gardleopard]

openapi validation errors can maybe be fixed?

[FredrikOseberg]

Things to think about:

• Today we have a default project, this has proven to create some confusion for our users. Should we think about changing it in the next version
• We added the default environment for migration from v3 -> v4. There is an opportunity to mark it as deprecated and remove it in the next major version, basically enforcing those who want to be on the new version to migrate to environment specific strategies and tokens.

[thomasheartman]

Another point: I think we should rename the "username" property of API tokens (and probably make it unique too). It's a token name and has nothing to do with users. This has been a point of confusion for our users.

[thomasheartman]

Follow-up on the comment around PUT. I've checked all (well, except one) of the PUT endpoints in the open source version. Here's what I found:

/api/admin/stategies/{strategy}

• PUT works more like patch. Does not update fields you don't mention.
• PUT requires the "name" property do be present and it must match the name you used in the request URL. You cannot change the name in any way.

/api/admin/contextField/{field}

• PUT requires the "name" property do be present, but its value is ignored.
• PUT works more like patch. Updates some fields, but not all. For instance, "description" is ignored if it's missing. "legalValues" is not.

/api/admin/tag-types/{name}

• crashes (500!) When you send a payload with only "name" or empty. "name" does not have to match the URL.
• works as a patch. Only touches fields you send in. Set a value to null to clear it

/addons/{id}

• POST endpoint crashes (500) on invalid data ("unknown addon provider string")
• POST gives a 400 with an empty message if the payload has a provider but is missing required parameters: {"details":"","isJoi":true}
• PUT seems to work correctly.

api-tokens/{id}

• • Seems to work correctly a

user-admin/{id}

• The API spec say we accept the whole user object, but not all fields can be updated.
• Works like a PATCH. Doesn't replace the object.

feedback/{ id }

• Not sure why we can put AND post here. Can't GET the resource anyway, so 🤷

projects/{projectId}/features/{featureName}/environments/{environment}/strategies/{strategyId}

• Does not remove "parameters" if they're not provided in the put
• Does not validate that the name of the strategy is an existing one
• Probably does not validate that the config is valid

projects/{projectId}/features/{featureName}

• works like PATCH
• requires "name" property, although it cannot be updated and is ignored.

projects/{projectId}/features/{featureName}/variants

• seems to work as expected

environments/sort-order

• … Should … Probably not be a PUT? Doesn't actually refer to a specific resource that's accessible anywhere 🤔 POST?

invite-link/tokens/token

• … Haven't been able to toggle on.

[thomasheartman]

Couple more things:

1. Remove the environment context field included in the Unleash context. While this is more up to each SDK, I'm sure there's places in Unleash proper where we also present this or expect it to be present.
2. Make summary (and possibly description) required properties for the OpenApiService.validPath method. You shouldn't be able to create an API endpoint that doesn't have a description or at least a summary. I'd potentially argue that the same goes for the API schemas, so that all payloads must have full descriptions.

[nunogois]

We should have a look at things we're keeping only for legacy / non-back-compatibility-breaking purposes. We identified we should clean up the old variants endpoint after we implemented the new variants per environment feature, but maybe it would make sense to have a larger sweep to identify and handle similar things.

[FredrikOseberg]

Deprecation discussion:

• Custom strategies - Is there any use case that couldn't be handled elegantly with constraints?
• Builtin strategies - Everything should now be customizable with a single strategy and constraints combinations. No reasons to have multiple anymore.

Expanding on that, we could do away with the entire strategy concept and just have constraints and rollout percentage. When you create a feature toggle, the rollout percentage would automatically start at 0 and you would have no constraints. Turning the feature on would no longer require a strategy, since every toggle would have a default set of settings in all environments.

EDIT: Maybe we can also eventually remove explicitly turning it off / on in an environment, perhaps this can be controlled only by the rollout %.

[thomasheartman]

Interesting points!

Builtin strategies - Everything should now be customizable with a single strategy and constraints combinations. No reasons to have multiple anymore.

I wanna say yes to this. It could take one element out of the equation, simplifying it. However:

Expanding on that, we could do away with the entire strategy concept and just have constraints and rollout percentage. When you create a feature toggle, the rollout percentage would automatically start at 0 and you would have no constraints. Turning the feature on would no longer require a strategy, since every toggle would have a default set of settings.

What about cases where you have multiple strategies that target different user groups? Constraints limit a target group, but what about when you need to expand it? What replaces the OR functionality of strategies?

Custom strategies - Is there any use case that couldn't be handled elegantly with constraints?

I don't know of any, but I'd also want to think through this very thoroughly before removing them completely. There's always someone (ref this XKCD: workflow)

[FredrikOseberg]

Interesting points!

Builtin strategies - Everything should now be customizable with a single strategy and constraints combinations. No reasons to have multiple anymore.

I wanna say yes to this. It could take one element out of the equation, simplifying it. However:

Expanding on that, we could do away with the entire strategy concept and just have constraints and rollout percentage. When you create a feature toggle, the rollout percentage would automatically start at 0 and you would have no constraints. Turning the feature on would no longer require a strategy, since every toggle would have a default set of settings.

What about cases where you have multiple strategies that target different user groups? Constraints limit a target group, but what about when you need to expand it? What replaces the OR functionality of strategies?

Custom strategies - Is there any use case that couldn't be handled elegantly with constraints?

I don't know of any, but I'd also want to think through this very thoroughly before removing them completely. There's always someone (ref this XKCD: workflow)

You're right. We'll need to preserve that concept somehow, maybe keeping only one strategy that is added by default is enough.

[gastonfournier]

The main branch is migrated to node 18 and we've opened a support branch for v4: https://github.com/Unleash/unleash/tree/v4

The intent of the support branch is just to backport security issues and bug fixes but not for new feature development.