Merge branch 'add-feature-flag-type-licensed' into 'master'

Add feature flag type licensed (RUN AS-IF-FOSS)

Closes #221055

See merge request gitlab-org/gitlab!34275

app/controllers/concerns/issuable_actions.rb

@@ -9,7 +9,7 @@ module IssuableActions
     before_action :check_destroy_confirmation!, only: :destroy
     before_action :authorize_admin_issuable!, only: :bulk_update
     before_action only: :show do
-      push_frontend_feature_flag(:scoped_labels, default_enabled: true)
+      push_frontend_feature_flag(:scoped_labels, type: :licensed, default_enabled: true)
     end
     before_action do
       push_frontend_feature_flag(:not_issuable_queries, @project, default_enabled: true)

app/models/clusters/instance.rb

@@ -7,7 +7,7 @@ module Clusters
     end
 
     def feature_available?(feature)
-      ::Feature.enabled?(feature, default_enabled: true)
+      ::Feature.enabled?(feature, type: :licensed, default_enabled: true)
     end
 
     def flipper_id

bin/feature-flag

@@ -181,6 +181,10 @@ class FeatureFlagOptionParser
         $stderr.puts "URL needs to start with https://"
       end
     end
+
+    def read_default_enabled(options)
+      TYPES.dig(options.type, :default_enabled)
+    end
   end
 end
 
@@ -226,7 +230,7 @@ class FeatureFlagCreator
       'rollout_issue_url' => options.rollout_issue_url,
       'group'             => options.group.to_s,
       'type'              => options.type.to_s,
-      'default_enabled'   => false
+      'default_enabled'   => FeatureFlagOptionParser.read_default_enabled(options)
     ).strip
   end
 

doc/development/feature_flags/development.md

@@ -61,6 +61,30 @@ Feature.disabled?(:my_ops_flag, project, type: ops)
 push_frontend_feature_flag(:my_ops_flag, project, type: :ops)
 ```
 
+### `licensed` type
+
+`licensed` feature flags are used to temporarily disable licensed features. There
+should be a one-to-one mapping of `licensed` feature flags to licensed features.
+
+`licensed` feature flags must be `default_enabled: true`, because that's the only
+supported option in the current implementation. This is under development as per
+the [related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/218667.
+
+The `licensed` type has a dedicated set of functions to check if a licensed
+feature is available for a project or namespace. This check validates
+if the license is assigned to the namespace and feature flag itself.
+The `licensed` feature flag has the same name as a licensed feature name:
+
+```ruby
+# Good: checks if feature flag is enabled
+project.feature_available?(:my_licensed_feature)
+namespace.feature_available?(:my_licensed_feature)
+
+# Bad: licensed flag must be accessed via `feature_available?`
+Feature.enabled?(:my_licensed_feature, type: :licensed)
+push_frontend_feature_flag(:my_licensed_feature, type: :licensed)
+```
+
 ## Feature flag definition and validation
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/229161) in GitLab 13.3.
@@ -290,13 +314,17 @@ The [`Project#feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc
 [`License.feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/license.rb#L293-300) (EE) methods all implicitly check for
 a by default enabled feature flag with the same name as the provided argument.
 
-You'd still want to use an explicit `Feature.enabled?` check if your new feature
-isn't gated by a License or Plan.
-
 **An important side-effect of the implicit feature flags mentioned above is that
 unless the feature is explicitly disabled or limited to a percentage of users,
 the feature flag check will default to `true`.**
 
+NOTE: **Note:**
+Due to limitations with `feature_available?`, the YAML definition for `licensed` feature
+flags accepts only `default_enabled: true`. This is under development as per the
+[related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/218667).
+
+#### Alpha/beta licensed feature flags
+
 This is relevant when developing the feature using
 [several smaller merge requests](https://about.gitlab.com/handbook/values/#make-small-merge-requests), or when the feature is considered to be an
 [alpha or beta](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga), and
@@ -311,6 +339,24 @@ GitLab.com and self-managed instances, you should use the
 method, according to our [definitions](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga). This ensures the feature is disabled unless the feature flag is
 _explicitly_ enabled.
 
+CAUTION: **Caution:**
+If `alpha_feature_available?` or `beta_feature_available?` is used, the YAML definition
+for the feature flag must use `default_enabled: [false, true]`, because the usage
+of the feature flag is undefined. These methods may change, as per the
+[related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/218667).
+
+The resulting YAML should be similar to:
+
+```yaml
+name: scoped_labels
+group: group::memory
+type: licensed
+# The `default_enabled:` is undefined
+# as `feature_available?` uses `default_enabled: true`
+# as `beta_feature_available?` uses `default_enabled: false`
+default_enabled: [false, true]
+```
+
 ### Feature groups
 
 Feature groups must be defined statically in `lib/feature.rb` (in the

ee/app/controllers/concerns/ee/boards_actions.rb

@@ -12,7 +12,7 @@ module EE
 
     def push_wip_limits
       # This is pushing a licensed Feature to the frontend.
-      push_frontend_feature_flag(:wip_limits, default_enabled: true) if parent.feature_available?(:wip_limits)
+      push_frontend_feature_flag(:wip_limits, type: :licensed, default_enabled: true) if parent.feature_available?(:wip_limits)
     end
   end
 end

ee/app/controllers/groups/analytics/application_controller.rb

@@ -6,7 +6,9 @@ class Groups::Analytics::ApplicationController < ApplicationController
   private
 
   def self.check_feature_flag(flag, *args)
-    before_action(*args) { render_404 unless Feature.enabled?(flag, default_enabled: Gitlab::Analytics.feature_enabled_by_default?(flag)) }
+    before_action(*args) do
+      render_404 unless Gitlab::Analytics.feature_enabled?(flag)
+    end
   end
 
   def self.increment_usage_counter(counter_klass, counter, *args)

ee/app/models/ee/ci/pipeline.rb

@@ -163,8 +163,7 @@ module EE
       private
 
       def project_has_subscriptions?
-        return false unless ::Feature.enabled?(:ci_project_subscriptions, project)
-
+        project.beta_feature_available?(:ci_project_subscriptions) &&
           project.downstream_projects.any?
       end
 

ee/app/models/ee/namespace.rb

@@ -126,7 +126,7 @@ module EE
     #   it. This is the case when we're ready to enable a feature for anyone
     #   with the correct license.
     def beta_feature_available?(feature)
-      ::Feature.enabled?(feature) ? feature_available?(feature) : ::Feature.enabled?(feature, self)
+      ::Feature.enabled?(feature, type: :licensed) ? feature_available?(feature) : ::Feature.enabled?(feature, self, type: :licensed)
     end
     alias_method :alpha_feature_available?, :beta_feature_available?
 
@@ -136,7 +136,7 @@ module EE
     override :feature_available?
     def feature_available?(feature)
       # This feature might not be behind a feature flag at all, so default to true
-      return false unless ::Feature.enabled?(feature, default_enabled: true)
+      return false unless ::Feature.enabled?(feature, type: :licensed, default_enabled: true)
 
       available_features = strong_memoize(:feature_available) do
         Hash.new do |h, f|

ee/app/models/ee/project.rb

@@ -306,7 +306,7 @@ module EE
     #   it. This is the case when we're ready to enable a feature for anyone
     #   with the correct license.
     def beta_feature_available?(feature)
-      ::Feature.enabled?(feature) ? feature_available?(feature) : ::Feature.enabled?(feature, self)
+      ::Feature.enabled?(feature, type: :licensed) ? feature_available?(feature) : ::Feature.enabled?(feature, self, type: :licensed)
     end
     alias_method :alpha_feature_available?, :beta_feature_available?
 
@@ -704,7 +704,7 @@ module EE
 
     def licensed_feature_available?(feature, user = nil)
       # This feature might not be behind a feature flag at all, so default to true
-      return false unless ::Feature.enabled?(feature, user, default_enabled: true)
+      return false unless ::Feature.enabled?(feature, user, type: :licensed, default_enabled: true)
 
       available_features = strong_memoize(:licensed_feature_available) do
         Hash.new do |h, f|

ee/app/models/license.rb

@@ -378,7 +378,7 @@ class License < ApplicationRecord
     return false if trial? && expired?
 
     # This feature might not be behind a feature flag at all, so default to true
-    return false unless ::Feature.enabled?(feature, default_enabled: true)
+    return false unless ::Feature.enabled?(feature, type: :licensed, default_enabled: true)
 
     features.include?(feature)
   end

ee/app/services/personal_access_tokens/revoke_invalid_tokens.rb

@@ -8,7 +8,7 @@ module PersonalAccessTokens
     end
 
     def execute
-      return unless ::Feature.enabled?(:personal_access_token_expiration_policy, default_enabled: true)
+      return unless ::Feature.enabled?(:personal_access_token_expiration_policy, type: :licensed, default_enabled: true)
       return unless PersonalAccessToken.expiration_enforced?
       return unless expiration_date && user_affected?
 

ee/lib/gitlab/analytics.rb

@@ -15,14 +15,27 @@ module Gitlab
       PRODUCTIVITY_ANALYTICS_FEATURE_FLAG
     ].freeze
 
+    # Improve that in https://gitlab.com/gitlab-org/gitlab/-/issues/246768
     FEATURE_FLAG_DEFAULTS = {
       PRODUCTIVITY_ANALYTICS_FEATURE_FLAG => true,
       GROUP_COVERAGE_REPORTS_FEATURE_FLAG => true,
-      CYCLE_ANALYTICS_FEATURE_FLAG => true
+      CYCLE_ANALYTICS_FEATURE_FLAG => true,
+      PROJECT_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG => true
+    }.freeze
+
+    FEATURE_FLAGS_TYPE = {
+      # TODO: it seems that we use a licensed "feature"
+      PRODUCTIVITY_ANALYTICS_FEATURE_FLAG => :licensed,
+      GROUP_COVERAGE_REPORTS_FEATURE_FLAG => :licensed,
+      GROUP_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG => :licensed,
+      CYCLE_ANALYTICS_FEATURE_FLAG => :development,
+      PROJECT_MERGE_REQUEST_ANALYTICS_FEATURE_FLAG => :licensed
     }.freeze
 
     def self.any_features_enabled?
-      FEATURE_FLAGS.any? { |flag| Feature.enabled?(flag, default_enabled: feature_enabled_by_default?(flag)) }
+      FEATURE_FLAGS.any? do |flag|
+        feature_enabled?(flag)
+      end
     end
 
     def self.cycle_analytics_enabled?
@@ -50,7 +63,9 @@ module Gitlab
     end
 
     def self.feature_enabled?(feature)
-      Feature.enabled?(feature, default_enabled: feature_enabled_by_default?(feature))
+      Feature.enabled?(feature,
+        type: FEATURE_FLAGS_TYPE.fetch(feature),
+        default_enabled: feature_enabled_by_default?(feature))
     end
   end
 end

ee/spec/models/ci/pipeline_spec.rb

@@ -389,7 +389,7 @@ RSpec.describe Ci::Pipeline do
 
         context 'when feature is available' do
           before do
-            stub_feature_flags(ci_project_subscriptions: true)
+            stub_licensed_features(ci_project_subscriptions: true)
           end
 
           it 'schedules the trigger downstream subscriptions worker' do

lib/feature/shared.rb

@@ -8,12 +8,14 @@ class Feature
   module Shared
     # optional: defines if a on-disk definition is required for this feature flag type
     # rollout_issue: defines if `bin/feature-flag` asks for rollout issue
+    # default_enabled: defines a default state of a feature flag when created by `bin/feature-flag`
     # example: usage being shown when exception is raised
     TYPES = {
       development: {
         description: 'Short lived, used to enable unfinished code to be deployed',
         optional: true,
         rollout_issue: true,
+        default_enabled: false,
         example: <<-EOS
           Feature.enabled?(:my_feature_flag, project)
           Feature.enabled?(:my_feature_flag, project, type: :development)
@@ -24,10 +26,21 @@ class Feature
         description: "Long-lived feature flags that control operational aspects of GitLab's behavior",
         optional: true,
         rollout_issue: false,
+        default_enabled: false,
         example: <<-EOS
           Feature.enabled?(:my_ops_flag, type: ops)
           push_frontend_feature_flag?(:my_ops_flag, project, type: :ops)
         EOS
+      },
+      licensed: {
+        description: 'Permanent feature flags used to temporarily disable licensed features.',
+        optional: true,
+        rollout_issue: false,
+        default_enabled: true,
+        example: <<-EOS
+          project.feature_available?(:my_licensed_feature)
+          namespace.feature_available?(:my_licensed_feature)
+        EOS
       }
     }.freeze