Skip to content

Display No Environments for Feature Flag Strategies without Scopes

Jason Goodman requested to merge display-no-ff-scopes into master

What does this MR do?

This fixes two related issues.

  • Removing all scopes from a strategy displays All environments in the UI, but actually saves zero scopes for the strategy, resulting in the strategy being applied to no environments. Fixes this issue so that removing all scopes from a strategy adds an all environments ('*') scope so that the strategy will be applied to all environments.

  • A strategy with no scopes is displayed as applying to All environments. While the UI does not allow a strategy with zero scopes, the API does. So the UI should be able to display this properly. This is a separate issue from the point above, but exacerbates that problem.

There is still an issue even after this update. If a strategy has no scopes, it will display as No environments in the UI. This is good, as the UI accurately represents the strategy scopes. However, the ensuing user experience may be counter-intuitive. It is unclear how to change this to All environments, as this cannot actually be selected in the UI. The user can change the scope from No environments to All environments by adding any scope, such as "production", and then removing that scope. Then the strategy will be applied to All environments.

Screenshots

The database state is displayed in a table that results from the following query:

SELECT
  f.name AS flag_name,
  s.name AS strategy_name,
  c.environment_scope
FROM operations_feature_flags AS f JOIN operations_strategies AS s ON f.id = s.feature_flag_id
LEFT JOIN operations_scopes AS c ON s.id = c.strategy_id
WHERE f.project_id = 30 and f.name = 'clouds_v2';

Before

  1. Start with a feature flag with a single strategy with a single scope, staging.

Screen_Shot_2020-07-16_at_11.18.39_AM

flag_name strategy_name environment_scope
clouds_v2 default staging
  1. Remove the staging scope.

Screen_Shot_2020-07-16_at_11.18.51_AM

  1. Click save. Zero scopes are saved in the database for the strategy.
flag_name strategy_name environment_scope
clouds_v2 default NULL
  1. Open the flag again. Observe the flag strategy says it applies to All environments. However, the database has no scopes for the strategy. The flag is not sent to clients, so it is disabled for all environments.

Screen_Shot_2020-07-16_at_11.18.51_AM

flag_name strategy_name environment_scope
clouds_v2 default NULL
  1. Save the flag again. This changes the database state and saves an all environments scope (*).

Screen_Shot_2020-07-16_at_11.18.51_AM

flag_name strategy_name environment_scope
clouds_v2 default *

After

  1. Start with a feature flag with a single strategy with a single scope, staging.

Screen_Shot_2020-07-16_at_11.18.39_AM

flag_name strategy_name environment_scope
clouds_v2 default staging
  1. Remove the staging scope.

Screen_Shot_2020-07-16_at_11.18.51_AM

  1. Click save. A single all environments (*) scope is saved in the database. The strategy is sent to all environments.
flag_name strategy_name environment_scope
clouds_v2 default *
  1. Open the flag again. Observe the flag strategy says it applies to All environments. This matches the database state.

Screen_Shot_2020-07-16_at_11.18.51_AM

flag_name strategy_name environment_scope
clouds_v2 default *
  1. Delete the all environments scope in the database directly by using SQL. The API could be used to do this.
flag_name strategy_name environment_scope
clouds_v2 default NULL
  1. Reload the feature flag edit page. Observe the strategy applies to No environments.

Screen_Shot_2020-07-16_at_11.31.00_AM

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Jason Goodman

Merge request reports