mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01:00 
			
		
		
		
	fix: auto add stratgy when enabling empty env. (#2137)
This commit is contained in:
		
							parent
							
								
									1d5249eddc
								
							
						
					
					
						commit
						a09c6313b1
					
				
							
								
								
									
										0
									
								
								frontend/yarn
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										0
									
								
								frontend/yarn
									
									
									
									
									
										Normal file
									
								
							| @ -527,6 +527,7 @@ export default class ProjectFeaturesController extends Controller { | ||||
|             environment, | ||||
|             true, | ||||
|             extractUsername(req), | ||||
|             req.user, | ||||
|         ); | ||||
|         res.status(200).end(); | ||||
|     } | ||||
| @ -542,6 +543,7 @@ export default class ProjectFeaturesController extends Controller { | ||||
|             environment, | ||||
|             false, | ||||
|             extractUsername(req), | ||||
|             req.user, | ||||
|         ); | ||||
|         res.status(200).end(); | ||||
|     } | ||||
|  | ||||
| @ -71,6 +71,11 @@ import { IContextFieldStore } from 'lib/types/stores/context-field-store'; | ||||
| import { Saved, Unsaved } from '../types/saved'; | ||||
| import { SegmentService } from './segment-service'; | ||||
| import { SetStrategySortOrderSchema } from 'lib/openapi/spec/set-strategy-sort-order-schema'; | ||||
| import { getDefaultStrategy } from '../util/feature-evaluator/helpers'; | ||||
| import { AccessService } from './access-service'; | ||||
| import { User } from '../server-impl'; | ||||
| import { CREATE_FEATURE_STRATEGY } from '../types/permissions'; | ||||
| import NoAccessError from '../error/no-access-error'; | ||||
| 
 | ||||
| interface IFeatureContext { | ||||
|     featureName: string; | ||||
| @ -106,6 +111,8 @@ class FeatureToggleService { | ||||
| 
 | ||||
|     private segmentService: SegmentService; | ||||
| 
 | ||||
|     private accessService: AccessService; | ||||
| 
 | ||||
|     constructor( | ||||
|         { | ||||
|             featureStrategiesStore, | ||||
| @ -129,6 +136,7 @@ class FeatureToggleService { | ||||
|         >, | ||||
|         { getLogger }: Pick<IUnleashConfig, 'getLogger'>, | ||||
|         segmentService: SegmentService, | ||||
|         accessService: AccessService, | ||||
|     ) { | ||||
|         this.logger = getLogger('services/feature-toggle-service.ts'); | ||||
|         this.featureStrategiesStore = featureStrategiesStore; | ||||
| @ -140,6 +148,7 @@ class FeatureToggleService { | ||||
|         this.featureEnvironmentStore = featureEnvironmentStore; | ||||
|         this.contextFieldStore = contextFieldStore; | ||||
|         this.segmentService = segmentService; | ||||
|         this.accessService = accessService; | ||||
|     } | ||||
| 
 | ||||
|     async validateFeatureContext({ | ||||
| @ -834,6 +843,7 @@ class FeatureToggleService { | ||||
|         environment: string, | ||||
|         enabled: boolean, | ||||
|         createdBy: string, | ||||
|         user?: User, | ||||
|     ): Promise<FeatureToggle> { | ||||
|         const hasEnvironment = | ||||
|             await this.featureEnvironmentStore.featureHasEnvironment( | ||||
| @ -849,9 +859,23 @@ class FeatureToggleService { | ||||
|                     environment, | ||||
|                 ); | ||||
|                 if (strategies.length === 0) { | ||||
|                     throw new InvalidOperationError( | ||||
|                         'You can not enable the environment before it has strategies', | ||||
|                     ); | ||||
|                     const canAddStrategies = | ||||
|                         user && | ||||
|                         (await this.accessService.hasPermission( | ||||
|                             user, | ||||
|                             CREATE_FEATURE_STRATEGY, | ||||
|                             project, | ||||
|                             environment, | ||||
|                         )); | ||||
|                     if (canAddStrategies) { | ||||
|                         await this.createStrategy( | ||||
|                             getDefaultStrategy(featureName), | ||||
|                             { environment, projectId: project, featureName }, | ||||
|                             createdBy, | ||||
|                         ); | ||||
|                     } else { | ||||
|                         throw new NoAccessError(CREATE_FEATURE_STRATEGY); | ||||
|                     } | ||||
|                 } | ||||
|             } | ||||
|             const updatedEnvironmentStatus = | ||||
|  | ||||
| @ -71,6 +71,7 @@ export const createServices = ( | ||||
|         stores, | ||||
|         config, | ||||
|         segmentService, | ||||
|         accessService, | ||||
|     ); | ||||
|     const environmentService = new EnvironmentService(stores, config); | ||||
|     const featureTagService = new FeatureTagService(stores, config); | ||||
|  | ||||
| @ -125,6 +125,6 @@ export interface IAccessStore extends Store<IRole, number> { | ||||
|     removePermissionFromRole( | ||||
|         roleId: number, | ||||
|         permission: string, | ||||
|         projectId?: string, | ||||
|         environment?: string, | ||||
|     ): Promise<void>; | ||||
| } | ||||
|  | ||||
| @ -1,3 +1,4 @@ | ||||
| import { IStrategyConfig } from '../../types/model'; | ||||
| import { FeatureStrategiesEvaluationResult } from './client'; | ||||
| import { Context } from './context'; | ||||
| 
 | ||||
| @ -38,3 +39,15 @@ export function resolveContextValue( | ||||
| export function safeName(str: string = ''): string { | ||||
|     return str.replace(/\//g, '_'); | ||||
| } | ||||
| 
 | ||||
| export function getDefaultStrategy(featureName: string): IStrategyConfig { | ||||
|     return { | ||||
|         name: 'flexibleRollout', | ||||
|         constraints: [], | ||||
|         parameters: { | ||||
|             rollout: '100', | ||||
|             stickiness: 'default', | ||||
|             groupId: featureName, | ||||
|         }, | ||||
|     }; | ||||
| } | ||||
|  | ||||
| @ -3,6 +3,7 @@ import { IUnleashTest, setupAppWithAuth } from '../../../helpers/test-helper'; | ||||
| import getLogger from '../../../../fixtures/no-logger'; | ||||
| import { DEFAULT_ENV } from '../../../../../lib/util/constants'; | ||||
| import { RoleName } from '../../../../../lib/server-impl'; | ||||
| import { CREATE_FEATURE_STRATEGY } from '../../../../../lib/types/permissions'; | ||||
| 
 | ||||
| let app: IUnleashTest; | ||||
| let db: ITestDb; | ||||
| @ -76,3 +77,36 @@ test('Should be possible to update feature toggle with permission', async () => | ||||
|         .send({ name, description: 'updated', type: 'kill-switch' }) | ||||
|         .expect(200); | ||||
| }); | ||||
| 
 | ||||
| test('Should not be possible auto-enable feature toggle without CREATE_FEATURE_STRATEGY permission', async () => { | ||||
|     const email = 'user33@mail.com'; | ||||
|     const url = '/api/admin/projects/default/features'; | ||||
|     const name = 'auth.toggle.enable'; | ||||
| 
 | ||||
|     await app.services.featureToggleServiceV2.createFeatureToggle( | ||||
|         'default', | ||||
|         { name }, | ||||
|         'me', | ||||
|         true, | ||||
|     ); | ||||
| 
 | ||||
|     await app.services.userService.createUser({ | ||||
|         email, | ||||
|         rootRole: RoleName.EDITOR, | ||||
|     }); | ||||
| 
 | ||||
|     await app.request.post('/auth/demo/login').send({ | ||||
|         email, | ||||
|     }); | ||||
| 
 | ||||
|     const role = await db.stores.roleStore.getRoleByName(RoleName.EDITOR); | ||||
| 
 | ||||
|     await db.stores.accessStore.removePermissionFromRole( | ||||
|         role.id, | ||||
|         CREATE_FEATURE_STRATEGY, | ||||
|         'default', | ||||
|     ); | ||||
|     await app.request | ||||
|         .post(`${url}/${name}/environments/default/on`) | ||||
|         .expect(403); | ||||
| }); | ||||
|  | ||||
| @ -1166,7 +1166,7 @@ test('Trying to get a non existing feature strategy should yield 404', async () | ||||
|         .expect(404); | ||||
| }); | ||||
| 
 | ||||
| test('Can not enable environment for feature without strategies', async () => { | ||||
| test('Can enable environment for feature without strategies', async () => { | ||||
|     const environment = 'some-env'; | ||||
|     const featureName = 'com.test.enable.environment.disabled'; | ||||
| 
 | ||||
| @ -1194,7 +1194,7 @@ test('Can not enable environment for feature without strategies', async () => { | ||||
|             `/api/admin/projects/default/features/${featureName}/environments/${environment}/on`, | ||||
|         ) | ||||
|         .set('Content-Type', 'application/json') | ||||
|         .expect(403); | ||||
|         .expect(200); | ||||
|     await app.request | ||||
|         .get(`/api/admin/projects/default/features/${featureName}`) | ||||
|         .expect(200) | ||||
| @ -1203,7 +1203,7 @@ test('Can not enable environment for feature without strategies', async () => { | ||||
|             const enabledFeatureEnv = res.body.environments.find( | ||||
|                 (e) => e.name === environment, | ||||
|             ); | ||||
|             expect(enabledFeatureEnv.enabled).toBe(false); | ||||
|             expect(enabledFeatureEnv.enabled).toBe(true); | ||||
|             expect(enabledFeatureEnv.type).toBe('test'); | ||||
|         }); | ||||
| }); | ||||
|  | ||||
| @ -218,6 +218,7 @@ beforeAll(async () => { | ||||
|         stores, | ||||
|         config, | ||||
|         new SegmentService(stores, config), | ||||
|         accessService, | ||||
|     ); | ||||
|     projectService = new ProjectService( | ||||
|         stores, | ||||
|  | ||||
| @ -28,6 +28,7 @@ beforeAll(async () => { | ||||
|         stores, | ||||
|         config, | ||||
|         new SegmentService(stores, config), | ||||
|         accessService, | ||||
|     ); | ||||
|     const project = { | ||||
|         id: 'test-project', | ||||
|  | ||||
| @ -6,6 +6,8 @@ import { SegmentService } from '../../../lib/services/segment-service'; | ||||
| import { FeatureStrategySchema } from '../../../lib/openapi/spec/feature-strategy-schema'; | ||||
| import User from '../../../lib/types/user'; | ||||
| import { IConstraint } from '../../../lib/types/model'; | ||||
| import { AccessService } from '../../../lib/services/access-service'; | ||||
| import { GroupService } from '../../../lib/services/group-service'; | ||||
| 
 | ||||
| let stores; | ||||
| let db; | ||||
| @ -28,7 +30,14 @@ beforeAll(async () => { | ||||
|     ); | ||||
|     stores = db.stores; | ||||
|     segmentService = new SegmentService(stores, config); | ||||
|     service = new FeatureToggleService(stores, config, segmentService); | ||||
|     const groupService = new GroupService(stores, config); | ||||
|     const accessService = new AccessService(stores, config, groupService); | ||||
|     service = new FeatureToggleService( | ||||
|         stores, | ||||
|         config, | ||||
|         segmentService, | ||||
|         accessService, | ||||
|     ); | ||||
| }); | ||||
| 
 | ||||
| afterAll(async () => { | ||||
|  | ||||
| @ -18,6 +18,8 @@ import { SdkContextSchema } from 'lib/openapi/spec/sdk-context-schema'; | ||||
| import { SegmentSchema } from 'lib/openapi/spec/segment-schema'; | ||||
| import { playgroundStrategyEvaluation } from '../../../lib/openapi/spec/playground-strategy-schema'; | ||||
| import { PlaygroundSegmentSchema } from 'lib/openapi/spec/playground-segment-schema'; | ||||
| import { GroupService } from '../../../lib/services/group-service'; | ||||
| import { AccessService } from '../../../lib/services/access-service'; | ||||
| 
 | ||||
| let stores: IUnleashStores; | ||||
| let db: ITestDb; | ||||
| @ -30,10 +32,13 @@ beforeAll(async () => { | ||||
|     db = await dbInit('playground_service_serial', config.getLogger); | ||||
|     stores = db.stores; | ||||
|     segmentService = new SegmentService(stores, config); | ||||
|     const groupService = new GroupService(stores, config); | ||||
|     const accessService = new AccessService(stores, config, groupService); | ||||
|     featureToggleService = new FeatureToggleService( | ||||
|         stores, | ||||
|         config, | ||||
|         segmentService, | ||||
|         accessService, | ||||
|     ); | ||||
|     service = new PlaygroundService(config, { | ||||
|         featureToggleServiceV2: featureToggleService, | ||||
|  | ||||
| @ -33,6 +33,7 @@ beforeAll(async () => { | ||||
|         stores, | ||||
|         config, | ||||
|         new SegmentService(stores, config), | ||||
|         accessService, | ||||
|     ); | ||||
|     projectService = new ProjectService( | ||||
|         stores, | ||||
|  | ||||
| @ -40,6 +40,7 @@ beforeAll(async () => { | ||||
|         stores, | ||||
|         config, | ||||
|         new SegmentService(stores, config), | ||||
|         accessService, | ||||
|     ); | ||||
|     environmentService = new EnvironmentService(stores, config); | ||||
|     projectService = new ProjectService( | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user