Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: add limits to feature flags #7536

Merged
merged 8 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/lib/__snapshots__/create-config.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ exports[`should create default config 1`] = `
"constraintValues": 250,
"environments": 50,
"featureEnvironmentStrategies": 30,
"flags": 250,
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
"projects": 500,
"segmentValues": 1000,
"segments": 300,
Expand Down
7 changes: 7 additions & 0 deletions src/lib/create-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,13 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig {
0,
parseEnvVarNumber(process.env.UNLEASH_SEGMENTS_LIMIT, 300),
),
flags: Math.max(
1,
parseEnvVarNumber(
process.env.UNLEASH_FLAGS_LIMIT,
options?.resourceLimits?.flags || 250,
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
),
),
};

return {
Expand Down
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -219,5 +219,5 @@ export const createFakeFeatureToggleService = (config: IUnleashConfig) => {
dependentFeaturesService,
featureLifecycleReadModel,
);
return { featureToggleService, featureToggleStore };
return { featureToggleService, featureToggleStore, projectStore };
};
22 changes: 13 additions & 9 deletions src/lib/features/feature-toggle/fakes/fake-feature-toggle-store.ts
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,10 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore {
throw new Error('Method not implemented.');
}

async count(query: Partial<IFeatureToggleStoreQuery>): Promise<number> {
return this.features.filter(this.getFilterQuery(query)).length;
async count(
query: Partial<IFeatureToggleStoreQuery> = { archived: false },
): Promise<number> {
return this.getAll(query).then((features) => features.length);
}

async getAllByNames(names: string[]): Promise<FeatureToggle[]> {
Expand All @@ -92,16 +94,16 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore {
private getFilterQuery(query: Partial<IFeatureToggleStoreQuery>) {
return (f) => {
let projectMatch = true;
if (query.project) {
if ('project' in query) {
projectMatch = f.project === query.project;
}
let archiveMatch = true;
if (query.archived) {
archiveMatch = f.archived === query.archived;
if ('archived' in query) {
archiveMatch = (f.archived ?? false) === query.archived;
}
let staleMatch = true;
if (query.stale) {
staleMatch = f.stale === query.stale;
if ('stale' in query) {
staleMatch = (f.stale ?? false) === query.stale;
}
return projectMatch && archiveMatch && staleMatch;
};
Expand Down Expand Up @@ -141,8 +143,10 @@ export default class FakeFeatureToggleStore implements IFeatureToggleStore {
throw new NotFoundError(`Could not find feature with name ${key}`);
}

async getAll(): Promise<FeatureToggle[]> {
return this.features.filter((f) => !f.archived);
async getAll(
query: Partial<IFeatureToggleStoreQuery> = { archived: false },
): Promise<FeatureToggle[]> {
return this.features.filter(this.getFilterQuery(query));
}

async getFeatureMetadata(name: string): Promise<FeatureToggle> {
Expand Down
13 changes: 13 additions & 0 deletions src/lib/features/feature-toggle/feature-toggle-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,16 @@ class FeatureToggleService {
);
}

private async validateFeatureFlagLimit() {
if (this.flagResolver.isEnabled('resourceLimits')) {
const currentFlagCount = await this.featureToggleStore.count();
const limit = this.resourceLimits.flags;
if (currentFlagCount >= limit) {
throw new ExceedsLimitError('feature flag', limit);
}
}
}

async createFeatureToggle(
projectId: string,
value: FeatureToggleDTO,
Expand All @@ -1190,6 +1200,9 @@ class FeatureToggleService {
'You have reached the maximum number of feature flags for this project.',
);
}

await this.validateFeatureFlagLimit();

if (exists) {
let featureData: FeatureToggleInsert;
if (isValidated) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
IFlagResolver,
IStrategyConfig,
IUnleashConfig,
IUser,
} from '../../../types';
import getLogger from '../../../../test/fixtures/no-logger';

Expand All @@ -14,71 +15,145 @@ const alwaysOnFlagResolver = {
},
} as unknown as IFlagResolver;

test('Should not allow to exceed strategy limit', async () => {
const LIMIT = 3;
const { featureToggleService, featureToggleStore } =
createFakeFeatureToggleService({
getLogger,
flagResolver: alwaysOnFlagResolver,
resourceLimits: {
featureEnvironmentStrategies: LIMIT,
},
} as unknown as IUnleashConfig);

const addStrategy = () =>
featureToggleService.unprotectedCreateStrategy(
{ name: 'default', featureName: 'feature' } as IStrategyConfig,
{ projectId: 'default', featureName: 'feature' } as any,
{} as IAuditUser,
describe('Strategy limits', () => {
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
test('Should not allow to exceed strategy limit', async () => {
const LIMIT = 3;
const { featureToggleService, featureToggleStore } =
createFakeFeatureToggleService({
getLogger,
flagResolver: alwaysOnFlagResolver,
resourceLimits: {
featureEnvironmentStrategies: LIMIT,
},
} as unknown as IUnleashConfig);

const addStrategy = () =>
featureToggleService.unprotectedCreateStrategy(
{ name: 'default', featureName: 'feature' } as IStrategyConfig,
{ projectId: 'default', featureName: 'feature' } as any,
{} as IAuditUser,
);
await featureToggleStore.create('default', {
name: 'feature',
createdByUserId: 1,
});

for (let i = 0; i < LIMIT; i++) {
await addStrategy();
}

await expect(addStrategy()).rejects.toThrow(
"Failed to create strategy. You can't create more than the established limit of 3",
);
await featureToggleStore.create('default', {
name: 'feature',
createdByUserId: 1,
});

for (let i = 0; i < LIMIT; i++) {
await addStrategy();
}
test('Should not allow to exceed constraint values limit', async () => {
const LIMIT = 3;
const { featureToggleService, featureToggleStore } =
createFakeFeatureToggleService({
getLogger,
flagResolver: alwaysOnFlagResolver,
resourceLimits: {
constraintValues: LIMIT,
},
} as unknown as IUnleashConfig);

await expect(addStrategy()).rejects.toThrow(
"Failed to create strategy. You can't create more than the established limit of 3",
);
const addStrategyWithConstraints = (constraints: IConstraint[]) =>
featureToggleService.unprotectedCreateStrategy(
{
name: 'default',
featureName: 'feature',
constraints,
} as IStrategyConfig,
{ projectId: 'default', featureName: 'feature' } as any,
{} as IAuditUser,
);
await featureToggleStore.create('default', {
name: 'feature',
createdByUserId: 1,
});
await expect(() =>
addStrategyWithConstraints([
{
contextName: 'userId',
operator: 'IN',
values: ['1', '2', '3', '4'],
},
]),
).rejects.toThrow(
"Failed to create content values for userId. You can't create more than the established limit of 3",
);
});
});

test('Should not allow to exceed constraint values limit', async () => {
const LIMIT = 3;
const { featureToggleService, featureToggleStore } =
createFakeFeatureToggleService({
getLogger,
flagResolver: alwaysOnFlagResolver,
resourceLimits: {
constraintValues: LIMIT,
},
} as unknown as IUnleashConfig);

const addStrategyWithConstraints = (constraints: IConstraint[]) =>
featureToggleService.unprotectedCreateStrategy(
{
name: 'default',
featureName: 'feature',
constraints,
} as IStrategyConfig,
{ projectId: 'default', featureName: 'feature' } as any,
describe('Flag limits', () => {
test('Should not allow you to exceed the flag limit', async () => {
const LIMIT = 3;
const { featureToggleService, projectStore } =
createFakeFeatureToggleService({
getLogger,
flagResolver: alwaysOnFlagResolver,
resourceLimits: {
flags: LIMIT,
},
} as unknown as IUnleashConfig);

await projectStore.create({
name: 'default',
description: 'default',
id: 'default',
});

const createFlag = (name: string) =>
featureToggleService.createFeatureToggle(
'default',
{ name },
{} as IAuditUser,
);

for (let i = 0; i < LIMIT; i++) {
await createFlag(`feature-${i}`);
}

await expect(createFlag('excessive')).rejects.toThrow(
"Failed to create feature flag. You can't create more than the established limit of 3",
);
});

test('Archived flags do not count towards the total', async () => {
const LIMIT = 1;
const { featureToggleService, projectStore } =
createFakeFeatureToggleService({
getLogger,
flagResolver: alwaysOnFlagResolver,
resourceLimits: {
flags: LIMIT,
},
} as unknown as IUnleashConfig);

await projectStore.create({
name: 'default',
description: 'default',
id: 'default',
});

const createFlag = (name: string) =>
featureToggleService.createFeatureToggle(
'default',
{ name },
{} as IAuditUser,
);

await createFlag('to-be-archived');

await featureToggleService.archiveToggle(
'to-be-archived',
{} as IUser,
{} as IAuditUser,
);
await featureToggleStore.create('default', {
name: 'feature',
createdByUserId: 1,

await expect(createFlag('should-be-okay')).resolves.toMatchObject({
name: 'should-be-okay',
});
});
await expect(() =>
addStrategyWithConstraints([
{
contextName: 'userId',
operator: 'IN',
values: ['1', '2', '3', '4'],
},
]),
).rejects.toThrow(
"Failed to create content values for userId. You can't create more than the established limit of 3",
);
});
8 changes: 8 additions & 0 deletions src/lib/openapi/spec/resource-limits-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const resourceLimitsSchema = {
'projects',
'apiTokens',
'segments',
'flags',
],
additionalProperties: false,
properties: {
Expand Down Expand Up @@ -103,6 +104,13 @@ export const resourceLimitsSchema = {
example: 300,
description: 'The maximum number of segments allowed.',
},
flags: {
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
type: 'integer',
minimum: 1,
example: 5000,
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
description:
'The maximum number of feature flags you can have at the same time. Archived flags do not count towards this limit.',
},
},
components: {},
} as const;
Expand Down
4 changes: 3 additions & 1 deletion src/lib/types/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ export interface IUnleashOptions {
metricsRateLimiting?: Partial<IMetricsRateLimiting>;
dailyMetricsStorageDays?: number;
rateLimiting?: Partial<IRateLimiting>;
resourceLimits?: Partial<Pick<ResourceLimitsSchema, 'constraintValues'>>;
resourceLimits?: Partial<
Pick<ResourceLimitsSchema, 'constraintValues' | 'flags'>
>;
}

export interface IEmailOption {
Expand Down
Loading