mirror of
https://github.com/Unleash/unleash.git
synced 2025-03-18 00:19:49 +01:00
chore: allow you to lower constraint values even when they're above limit (#7624)
This PR allows you to gradually lower constraint values, even if they're above the limits. It does, however, come with a few caveats because of how Unleash deals with constraints: Constraints are just json blobs. They have no IDs or other distinguishing features. Because of this, we can't compare the current and previous state of a specific constraint. What we can do instead, is to allow you to lower the amount of constraint values if and only if the number of constraints hasn't changed. In this case, we assume that you also haven't reordered the constraints (not possible from the UI today). That way, we can compare constraint values between updated and existing constraints based on their index in the constraint list. It's not foolproof, but it's a workaround that you can use. There's a few edge cases that pop up, but that I don't think it's worth trying to cover: Case: If you **both** have too many constraints **and** too many constraint values Result: You won't be allowed to lower the amount of constraints as long as the amount of strategy values is still above the limit. Workaround: First, lower the amount of constraint values until you're under the limit and then lower constraints. OR, set the constraint you want to delete to a constraint that is trivially true (e.g. `currentTime > yesterday` ). That will essentially take that constraint out of the equation, achieving the same end result. Case: You re-order constraints and at least one of them has too many values Result: You won't be allowed to (except for in the edge case where the one with too many values doesn't move or switches places with another one with the exact same amount of values). Workaround: We don't need one. The order of constraints has no effect on the evaluation.
This commit is contained in:
parent
c3a00c07e1
commit
87fa5a2414
@ -396,23 +396,39 @@ class FeatureToggleService {
|
||||
}) {
|
||||
if (!this.flagResolver.isEnabled('resourceLimits')) return;
|
||||
|
||||
const constraintsLimit = this.resourceLimits.constraints;
|
||||
const {
|
||||
constraints: constraintsLimit,
|
||||
constraintValues: constraintValuesLimit,
|
||||
} = this.resourceLimits;
|
||||
|
||||
if (
|
||||
constraints.updated.length > constraintsLimit &&
|
||||
constraints.updated.length > constraints.existing.length
|
||||
) {
|
||||
throwExceedsLimitError(this.eventBus, {
|
||||
resource: `constraints`,
|
||||
resource: 'constraints',
|
||||
limit: constraintsLimit,
|
||||
});
|
||||
}
|
||||
|
||||
const constraintValuesLimit = this.resourceLimits.constraintValues;
|
||||
const isSameLength =
|
||||
constraints.existing.length === constraints.updated.length;
|
||||
const constraintOverLimit = constraints.updated.find(
|
||||
(constraint) =>
|
||||
Array.isArray(constraint.values) &&
|
||||
constraint.values?.length > constraintValuesLimit,
|
||||
(constraint, i) => {
|
||||
const updatedCount = constraint.values?.length ?? 0;
|
||||
const existingCount =
|
||||
constraints.existing[i]?.values?.length ?? 0;
|
||||
|
||||
const isOverLimit =
|
||||
Array.isArray(constraint.values) &&
|
||||
updatedCount > constraintValuesLimit;
|
||||
const allowAnyway =
|
||||
isSameLength && existingCount >= updatedCount;
|
||||
|
||||
return isOverLimit && !allowAnyway;
|
||||
},
|
||||
);
|
||||
|
||||
if (constraintOverLimit) {
|
||||
throwExceedsLimitError(this.eventBus, {
|
||||
resource: `constraint values for ${constraintOverLimit.contextName}`,
|
||||
@ -421,7 +437,6 @@ class FeatureToggleService {
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
async validateStrategyType(
|
||||
strategyName: string | undefined,
|
||||
): Promise<void> {
|
||||
|
@ -195,6 +195,73 @@ describe('Strategy limits', () => {
|
||||
"Failed to create constraint values for userId. You can't create more than the established limit of 3",
|
||||
);
|
||||
});
|
||||
|
||||
test('Should not throw limit exceeded errors for constraint values if the new values are less than or equal to the old values AND there have been no other constraint updates (re-ordering or deleting)', async () => {
|
||||
const LIMIT = 1;
|
||||
const { featureToggleService, featureStrategiesStore } =
|
||||
createFakeFeatureToggleService({
|
||||
getLogger,
|
||||
flagResolver: alwaysOnFlagResolver,
|
||||
resourceLimits: {
|
||||
constraintValues: LIMIT,
|
||||
},
|
||||
} as unknown as IUnleashConfig);
|
||||
|
||||
const constraints = (valueCount: number) =>
|
||||
[
|
||||
{
|
||||
values: Array.from({ length: valueCount }).map((_, i) =>
|
||||
i.toString(),
|
||||
),
|
||||
operator: 'IN',
|
||||
contextName: 'appName',
|
||||
},
|
||||
{
|
||||
values: ['a', 'b', 'c'],
|
||||
operator: 'IN',
|
||||
contextName: 'appName',
|
||||
},
|
||||
] as IConstraint[];
|
||||
|
||||
const flagName = 'feature';
|
||||
|
||||
await featureStrategiesStore.createFeature({
|
||||
name: flagName,
|
||||
createdByUserId: 1,
|
||||
});
|
||||
|
||||
const initialConstraintValueCount = LIMIT + 2;
|
||||
const strategy = await featureStrategiesStore.createStrategyFeatureEnv({
|
||||
parameters: {},
|
||||
strategyName: 'default',
|
||||
featureName: flagName,
|
||||
constraints: constraints(initialConstraintValueCount),
|
||||
projectId: 'default',
|
||||
environment: 'default',
|
||||
});
|
||||
|
||||
const updateStrategy = (valueCount) =>
|
||||
featureToggleService.unprotectedUpdateStrategy(
|
||||
strategy.id,
|
||||
{
|
||||
constraints: constraints(valueCount),
|
||||
},
|
||||
{ projectId: 'default', featureName: 'feature' } as any,
|
||||
{} as IAuditUser,
|
||||
);
|
||||
|
||||
// check that you can save the same amount of constraint values
|
||||
await updateStrategy(initialConstraintValueCount);
|
||||
// check that you can save fewer constraint values but still over the limit
|
||||
await updateStrategy(initialConstraintValueCount - 1);
|
||||
|
||||
// check that you can't save more constraint values
|
||||
await expect(async () =>
|
||||
updateStrategy(initialConstraintValueCount + 1),
|
||||
).rejects.toThrow(
|
||||
new ExceedsLimitError('constraint values for appName', LIMIT),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Flag limits', () => {
|
||||
|
Loading…
Reference in New Issue
Block a user