From 71691df62fbbf5d0d96700dcf594bfd51a240dfd Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 2 Sep 2019 14:55:56 +0700 Subject: [PATCH 01/11] Generalize Feature Flag system and implement unleash adapter This commit abstracts the current Feature implementation and make it adoptable with any feature flag systems. --- app/views/projects/_home_panel.html.haml | 5 +- config/gitlab.yml.example | 7 + config/initializers/1_settings.rb | 9 ++ config/initializers/flipper.rb | 2 +- config/initializers/sidekiq.rb | 4 +- config/initializers/unleash.rb | 4 + ee/app/finders/feature_flags_finder.rb | 3 +- ee/lib/api/feature_flags.rb | 161 +++++++++++++++++++++++ ee/lib/ee/api/entities.rb | 19 +++ lib/feature.rb | 99 ++------------ lib/feature/gitaly.rb | 2 +- lib/feature_flag/adapters/flipper.rb | 110 ++++++++++++++++ lib/feature_flag/adapters/unleash.rb | 99 ++++++++++++++ lib/gitlab/unleash/logger.rb | 19 +++ lib/unleash/strategy/user_with_id.rb | 36 +++++ 15 files changed, 487 insertions(+), 92 deletions(-) create mode 100644 config/initializers/unleash.rb create mode 100644 ee/lib/api/feature_flags.rb create mode 100644 lib/feature_flag/adapters/flipper.rb create mode 100644 lib/feature_flag/adapters/unleash.rb create mode 100644 lib/gitlab/unleash/logger.rb create mode 100644 lib/unleash/strategy/user_with_id.rb diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 4783b10cf6da0c..8506b86a54fda2 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -11,7 +11,10 @@ .d-flex.flex-column.flex-wrap.align-items-baseline .d-inline-flex.align-items-baseline %h1.home-panel-title.prepend-top-8.append-bottom-5.qa-project-name - = @project.name + - if Feature.enabled?(:test_project_name, @project) + = "The feature is on this project!" + - else + = @project.name %span.visibility-icon.text-secondary.prepend-left-4.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@project) } = visibility_level_icon(@project.visibility_level, fw: false, options: {class: 'icon'}) .home-panel-metadata.d-flex.flex-wrap.text-secondary diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 814ea551e19b63..f3393eb51b8b4e 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -468,6 +468,13 @@ production: &base # enabled: true # primary_api_url: http://localhost:5000/ # internal address to the primary registry, will be used by GitLab to directly communicate with primary registry API + ## Feature Flag https://docs.gitlab.com/ee/user/project/operations/feature_flags.html + unleash: + # enabled: false + # url: https://gitlab.com/api/v4/feature_flags/unleash/ + # app_name: gitlab.com # Environment name of your GitLab instance + # instance_id: INSTNACE_ID + # personal_access_token: PAT # Used for controlling flags on the project # # 2. GitLab CI settings diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index fbe6c21e53dfa5..1332d425373e51 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -310,6 +310,15 @@ Settings.geo.registry_replication['enabled'] ||= false end +# +# Unleash +# +Settings['unleash'] ||= Settingslogic.new({}) +Settings.unleash['enabled'] = false if Settings.unleash['enabled'].nil? +Settings.unleash['app_name'] = Rails.env if Settings.unleash['app_name'].nil? +Settings.unleash['instance_id'] = ENV['UNLEASH_INSTANCE_ID'] if Settings.unleash['instance_id'].nil? +Settings.unleash['personal_access_token'] = ENV['UNLEASH_PERSONAL_ACCESS_TOKEN'] if Settings.unleash['personal_access_token'].nil? + # # External merge request diffs # diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index 80cab7273e5ccd..73e0681081880c 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -1 +1 @@ -Feature.register_feature_groups +FeatureFlag::Adapters::Flipper.register_feature_groups diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index e3505579204b9d..024f9ee19d7616 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -1,13 +1,13 @@ require 'sidekiq/web' def enable_reliable_fetch? - return true unless Feature::FlipperFeature.table_exists? + return true unless Feature.table_exists? Feature.enabled?(:gitlab_sidekiq_reliable_fetcher, default_enabled: true) end def enable_semi_reliable_fetch_mode? - return true unless Feature::FlipperFeature.table_exists? + return true unless Feature.table_exists? Feature.enabled?(:gitlab_sidekiq_enable_semi_reliable_fetcher, default_enabled: true) end diff --git a/config/initializers/unleash.rb b/config/initializers/unleash.rb new file mode 100644 index 00000000000000..f968da0aa9e691 --- /dev/null +++ b/config/initializers/unleash.rb @@ -0,0 +1,4 @@ +return unless Gitlab.config.unleash.enabled && defined?(::Unicorn) + +FeatureFlag::Adapters::Unleash.configure +UNLEASH = Unleash::Client.new diff --git a/ee/app/finders/feature_flags_finder.rb b/ee/app/finders/feature_flags_finder.rb index 60a2ffac6ded46..7a02b48e70945b 100644 --- a/ee/app/finders/feature_flags_finder.rb +++ b/ee/app/finders/feature_flags_finder.rb @@ -10,7 +10,7 @@ def initialize(project, current_user, params = {}) @params = params end - def execute + def execute(preload: false) unless Ability.allowed?(current_user, :read_feature_flag, project) return Operations::FeatureFlag.none end @@ -19,6 +19,7 @@ def execute items = by_scope(items) items = for_list(items) + items.preload_relations if preload items.ordered end diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb new file mode 100644 index 00000000000000..714fbf626d6a98 --- /dev/null +++ b/ee/lib/api/feature_flags.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +module API + class FeatureFlags < Grape::API + include PaginationParams + + FEATURE_FLAG_ENDPOINT_REQUIREMETS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS + .merge(name: API::NO_SLASH_URL_PART_REGEX) + + before { authorize_read_feature_flags! } + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc 'Get all feature flags of a project' do + detail 'This feature was introduced in GitLab 12.4.' + success Entities::FeatureFlag + end + params do + optional :scope, type: String, desc: 'The scope ', values: %w[enabled disabled] + use :pagination + end + get ':id/feature_flags' do + feature_flags = ::FeatureFlagsFinder + .new(user_project, current_user, scope: params[:scope]) + .execute(preload: true) + + present paginate(feature_flags), with: Entities::FeatureFlag + end + + desc 'Get a feature flag of a project' do + detail 'This feature was introduced in GitLab 12.4.' + success Entities::FeatureFlag + end + params do + requires :name, type: String, desc: 'The name of the feature flag' + end + get ':id/feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + present feature_flag, with: Entities::FeatureFlag + end + + desc 'Create a new feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success Entities::FeatureFlag + end + params do + requires :name, type: String, desc: 'The name of the tag' + requires :description, type: String, desc: 'The name of the release' + requires :active, type: Boolean, desc: 'The name of the release' + optional :scopes, type: Hash do + requires :environment_scope, type: String + requires :active, type: Boolean + optional :strategies, type: Hash do + requires :name, type: String + requires :parameters, type: Hash do + requires :groupId, type: String + requires :percentage, type: String + requires :userIds, type: String + end + end + end + end + post ':id/feature_flags' do + authorize_create_feature_flag! + + result = FeatureFlags::CreateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute + + if result[:status] == :success + present result[:feature_flag], with: Entities::FeatureFlag + else + render_api_error!(result[:message], result[:http_status]) + end + end + + desc 'Update a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success Entities::FeatureFlag + end + params do + optional :name, type: String, desc: 'The name of the tag' + optional :description, type: String, desc: 'The name of the release' + optional :active, type: Boolean, desc: 'The name of the release' + optional :scopes, type: Hash do + requires :environment_scope, type: String + requires :active, type: Boolean + optional :strategies, type: Hash do + requires :name, type: String + requires :parameters, type: Hash do + requires :groupId, type: String + requires :percentage, type: String + requires :userIds, type: String + end + end + end + end + put ':id/feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + authorize_update_feature_flag! + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute(feature_flag) + + if result[:status] == :success + present result[:feature_flag], with: Entities::FeatureFlag + else + render_api_error!(result[:message], result[:http_status]) + end + end + + desc 'Delete a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success Entities::FeatureFlag + end + params do + optional :name, type: String, desc: 'The name of the feature flag' + end + delete ':id/feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + authorize_destroy_release! + + result = ::FeatureFlags::DestroyService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute(feature_flag) + + if result[:status] == :success + present result[:feature_flag], with: Entities::FeatureFlag + else + render_api_error!(result[:message], result[:http_status]) + end + end + end + + helpers do + def authorize_read_feature_flags! + authorize! :read_feature_flag, user_project + end + + def authorize_read_feature_flag! + authorize! :read_feature_flag, feature_flag + end + + def authorize_create_feature_flag! + authorize! :create_feature_flag, user_project + end + + def authorize_update_feature_flag! + authorize! :update_feature_flag, feature_flag + end + + def authorize_destroy_feature_flag! + authorize! :destroy_feature_flag, feature_flag + end + + def feature_flag + @feature_flag ||= user_project.operations_feature_flags.find_by_name!(params[:name]) + end + end + end +end diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index c3da875234ee1c..50c1feebb168b2 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -789,6 +789,25 @@ def can_read_vulnerabilities?(user, project) Ability.allowed?(user, :read_project_security_dashboard, project) end end + + class FeatureFlag < Grape::Entity + expose :id + expose :name + expose :description + expose :active + expose :created_at + expose :updated_at + expose :scopes, using: Entities::FeatureFlagScope + end + + class FeatureFlagScope < Grape::Entity + expose :id + expose :active + expose :environment_scope + expose :strategies + expose :created_at + expose :updated_at + end end end end diff --git a/lib/feature.rb b/lib/feature.rb index 88b0d871c3adc2..c2608d5fce82ca 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -1,64 +1,31 @@ # frozen_string_literal: true -require 'flipper/adapters/active_record' -require 'flipper/adapters/active_support_cache_store' - class Feature prepend_if_ee('EE::Feature') # rubocop: disable Cop/InjectEnterpriseEditionModule - # Classes to override flipper table names - class FlipperFeature < Flipper::Adapters::ActiveRecord::Feature - # Using `self.table_name` won't work. ActiveRecord bug? - superclass.table_name = 'features' - - def self.feature_names - pluck(:key) - end - end - - class FlipperGate < Flipper::Adapters::ActiveRecord::Gate - superclass.table_name = 'feature_gates' - end + SUPPORTED_FEATURE_FLAG_ADAPTERS = %w[unleash flipper] class << self - delegate :group, to: :flipper + delegate :all, :get, :group, :persisted?, :table_exists?, to: :adapter - def all - flipper.features.to_a - end - - def get(key) - flipper.feature(key) - end - - def persisted_names - Gitlab::SafeRequestStore[:flipper_persisted_names] ||= - begin - # We saw on GitLab.com, this database request was called 2300 - # times/s. Let's cache it for a minute to avoid that load. - Gitlab::ThreadMemoryCache.cache_backend.fetch('flipper:persisted_names', expires_in: 1.minute) do - FlipperFeature.feature_names - end + def adapter + @adapter ||= + SUPPORTED_FEATURE_FLAG_ADAPTERS.find do |type| + adapter = get_adapter(type) + break adapter if adapter.available? end end - def persisted?(feature) - # Flipper creates on-memory features when asked for a not-yet-created one. - # If we want to check if a feature has been actually set, we look for it - # on the persisted features list. - persisted_names.include?(feature.name.to_s) - end - # use `default_enabled: true` to default the flag to being `enabled` # unless set explicitly. The default is `disabled` def enabled?(key, thing = nil, default_enabled: false) - feature = Feature.get(key) + feature = get(key) # If we're not default enabling the flag or the feature has been set, always evaluate. # `persisted?` can potentially generate DB queries and also checks for inclusion # in an array of feature names (177 at last count), possibly reducing performance by half. # So we only perform the `persisted` check if `default_enabled: true` - !default_enabled || Feature.persisted?(feature) ? feature.enabled?(thing) : true + !default_enabled || persisted?(feature) ? feature.enabled?(thing) : true end def disabled?(key, thing = nil, default_enabled: false) @@ -89,50 +56,10 @@ def remove(key) feature.remove end - def flipper - if Gitlab::SafeRequestStore.active? - Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance - else - @flipper ||= build_flipper_instance - end - end - - def build_flipper_instance - Flipper.new(flipper_adapter).tap { |flip| flip.memoize = true } - end - - # This method is called from config/initializers/flipper.rb and can be used - # to register Flipper groups. - # See https://docs.gitlab.com/ee/development/feature_flags.html#feature-groups - def register_feature_groups - end - - def flipper_adapter - active_record_adapter = Flipper::Adapters::ActiveRecord.new( - feature_class: FlipperFeature, - gate_class: FlipperGate) - - # Redis L2 cache - redis_cache_adapter = - Flipper::Adapters::ActiveSupportCacheStore.new( - active_record_adapter, - l2_cache_backend, - expires_in: 1.hour) - - # Thread-local L1 cache: use a short timeout since we don't have a - # way to expire this cache all at once - Flipper::Adapters::ActiveSupportCacheStore.new( - redis_cache_adapter, - l1_cache_backend, - expires_in: 1.minute) - end - - def l1_cache_backend - Gitlab::ThreadMemoryCache.cache_backend - end - - def l2_cache_backend - Rails.cache + private + + def get_adapter(type) + "FeatureFlag::Adapters::#{type.camelize}".constantize end end diff --git a/lib/feature/gitaly.rb b/lib/feature/gitaly.rb index 81f8ba5c8c3dd0..5351ccffe519a1 100644 --- a/lib/feature/gitaly.rb +++ b/lib/feature/gitaly.rb @@ -16,7 +16,7 @@ class Gitaly class << self def enabled?(feature_flag) - return false unless Feature::FlipperFeature.table_exists? + return false unless Feature.table_exists? default_on = DEFAULT_ON_FLAGS.include?(feature_flag) Feature.enabled?("gitaly_#{feature_flag}", default_enabled: default_on) diff --git a/lib/feature_flag/adapters/flipper.rb b/lib/feature_flag/adapters/flipper.rb new file mode 100644 index 00000000000000..1f2d7fca3743b6 --- /dev/null +++ b/lib/feature_flag/adapters/flipper.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +require 'flipper/adapters/active_record' +require 'flipper/adapters/active_support_cache_store' + +module FeatureFlag + module Adapters + class Flipper + # Classes to override flipper table names + class FlipperFeature < ::Flipper::Adapters::ActiveRecord::Feature + # Using `self.table_name` won't work. ActiveRecord bug? + superclass.table_name = 'features' + + def self.feature_names + pluck(:key) + end + end + + class FlipperGate < ::Flipper::Adapters::ActiveRecord::Gate + superclass.table_name = 'feature_gates' + end + + class << self + delegate :group, to: :flipper + + def available? + true + end + + def all + flipper.features.to_a + end + + def get(key) + flipper.feature(key) + end + + # This method is called from config/initializers/flipper.rb and can be used + # to register Flipper groups. + # See https://docs.gitlab.com/ee/development/feature_flags.html#feature-groups + def register_feature_groups + end + + def persisted?(feature) + # Flipper creates on-memory features when asked for a not-yet-created one. + # If we want to check if a feature has been actually set, we look for it + # on the persisted features list. + persisted_names.include?(feature.name.to_s) + end + + def table_exists? + FlipperFeature.table_exists? + end + + private + + def flipper + if Gitlab::SafeRequestStore.active? + Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance + else + @flipper ||= build_flipper_instance + end + end + + def persisted_names + Gitlab::SafeRequestStore[:flipper_persisted_names] ||= + begin + # We saw on GitLab.com, this database request was called 2300 + # times/s. Let's cache it for a minute to avoid that load. + Gitlab::ThreadMemoryCache.cache_backend.fetch('flipper:persisted_names', expires_in: 1.minute) do + FlipperFeature.feature_names + end + end + end + + def build_flipper_instance + ::Flipper.new(flipper_adapter).tap { |flip| flip.memoize = true } + end + + def flipper_adapter + active_record_adapter = ::Flipper::Adapters::ActiveRecord.new( + feature_class: FlipperFeature, + gate_class: FlipperGate) + + # Redis L2 cache + redis_cache_adapter = + ::Flipper::Adapters::ActiveSupportCacheStore.new( + active_record_adapter, + l2_cache_backend, + expires_in: 1.hour) + + # Thread-local L1 cache: use a short timeout since we don't have a + # way to expire this cache all at once + ::Flipper::Adapters::ActiveSupportCacheStore.new( + redis_cache_adapter, + l1_cache_backend, + expires_in: 1.minute) + end + + def l1_cache_backend + Gitlab::ThreadMemoryCache.cache_backend + end + + def l2_cache_backend + Rails.cache + end + end + end + end +end diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb new file mode 100644 index 00000000000000..60941765b22b50 --- /dev/null +++ b/lib/feature_flag/adapters/unleash.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module FeatureFlag + module Adapters + class Unleash + class Feature + attr_reader :key + + def initialize(key) + @key = key.to_s + end + + def enabled?(thing = nil) + client.is_enabled?(key, context(thing)) + end + + def off?(thing = nil) + !enabled?(thing) + end + + def enable(thing = true) + # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + end + + def disable(thing = false) + # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + end + + def enable_group(group) + # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + end + + def disable_group(group) + # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + end + + def remove + # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + end + + private + + def client + FeatureFlag::Adapters::Unleash.client + end + + def context(thing) + ::Unleash::Context.new(properties: { thing: thing }) + end + end + + class << self + def available? + Gitlab.config.unleash.enabled + end + + def all + Unleash::ToggleFetcher.toggle_cache + end + + def get(key) + Feature.new(key) + end + + def persisted?(feature) + true # TODO: Fix + end + + def table_exists? + true + end + + def configure + ::Unleash.configure do |config| + config.url = Gitlab.config.unleash.url + config.app_name = Gitlab.config.unleash.app_name + config.instance_id = Gitlab.config.unleash.instance_id + config.logger = Gitlab::Unleash::Logger + end + end + + def client + # TODO: Fix + @client ||= if defined?(UNLEASH) + UNLEASH + elsif defined?(Rails.configuration.unleash) + Rails.configuration.unleash + else + ::Unleash::Client.new( + url: Gitlab.config.unleash.url, + app_name: Gitlab.config.unleash.app_name, + instance_id: Gitlab.config.unleash.instance_id, + logger: Gitlab::Unleash::Logger) + end + end + end + end + end +end diff --git a/lib/gitlab/unleash/logger.rb b/lib/gitlab/unleash/logger.rb new file mode 100644 index 00000000000000..5de41d9abde4ee --- /dev/null +++ b/lib/gitlab/unleash/logger.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module Unleash + class Logger < ::Gitlab::JsonLogger + def self.file_name_noext + 'unleash' + end + + def self.level=(level) + # no-op, otherwise `Unleash.logger.level` causes NotImplementedError. + end + + def self.build + super.tap { |logger| logger.level = Rails.logger.level } # rubocop:disable Gitlab/RailsLogger + end + end + end +end diff --git a/lib/unleash/strategy/user_with_id.rb b/lib/unleash/strategy/user_with_id.rb new file mode 100644 index 00000000000000..8f1e09a64c1526 --- /dev/null +++ b/lib/unleash/strategy/user_with_id.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +## +# We override the existing userWithId strategy to make it compatible with ActiveRecord +# objects such as Project, Group, User, etc. +# +# To properly gate a feature per target, you have to define userWithId strategy +# params in the following convention. +# +# ":" +# +# For example, if you want to enable a featue on the project (id: 123), you need +# to define the value "Project:123". +module Unleash + module Strategy + class UserWithId < Base + def name + 'userWithId' + end + + # requires: params['userIds'], context.user_id, + def is_enabled?(params = {}, context = nil) + return false unless params.is_a?(Hash) && params.has_key?(PARAM) + return false unless params.fetch(PARAM, nil).is_a? String + return false unless context.class.name == 'Unleash::Context' + + target = context.properties[:thing].yield_self do |thing| + thing = thing.__getobj__ if thing.respond_to?(:__getobj__) # Resolve SimpleDelegator + "#{thing.class.name}:#{thing.id}" + end + + params[PARAM].split(",").map(&:strip).any? { |allowed| allowed == target } + end + end + end +end -- GitLab From 981b34037bfecf5babc35f198896114eb7d7c7ed Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 5 Sep 2019 16:01:19 +0700 Subject: [PATCH 02/11] add some tests --- ee/lib/api/feature_flags.rb | 22 +- ee/lib/ee/api/api.rb | 1 + ee/lib/ee/api/entities.rb | 14 +- ee/spec/requests/api/feature_flags_spec.rb | 763 +++++++++++++++++++++ 4 files changed, 782 insertions(+), 18 deletions(-) create mode 100644 ee/spec/requests/api/feature_flags_spec.rb diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb index 714fbf626d6a98..ad889d7657b733 100644 --- a/ee/lib/api/feature_flags.rb +++ b/ee/lib/api/feature_flags.rb @@ -15,10 +15,10 @@ class FeatureFlags < Grape::API resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do desc 'Get all feature flags of a project' do detail 'This feature was introduced in GitLab 12.4.' - success Entities::FeatureFlag + success EE::API::Entities::FeatureFlag end params do - optional :scope, type: String, desc: 'The scope ', values: %w[enabled disabled] + optional :scope, type: String, desc: 'The scope', values: %w[enabled disabled] use :pagination end get ':id/feature_flags' do @@ -26,23 +26,23 @@ class FeatureFlags < Grape::API .new(user_project, current_user, scope: params[:scope]) .execute(preload: true) - present paginate(feature_flags), with: Entities::FeatureFlag + present paginate(feature_flags), with: EE::API::Entities::FeatureFlag end desc 'Get a feature flag of a project' do detail 'This feature was introduced in GitLab 12.4.' - success Entities::FeatureFlag + success EE::API::Entities::FeatureFlag end params do requires :name, type: String, desc: 'The name of the feature flag' end get ':id/feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do - present feature_flag, with: Entities::FeatureFlag + present feature_flag, with: EE::API::Entities::FeatureFlag end desc 'Create a new feature flag' do detail 'This feature was introduced in GitLab 12.4.' - success Entities::FeatureFlag + success EE::API::Entities::FeatureFlag end params do requires :name, type: String, desc: 'The name of the tag' @@ -69,7 +69,7 @@ class FeatureFlags < Grape::API .execute if result[:status] == :success - present result[:feature_flag], with: Entities::FeatureFlag + present result[:feature_flag], with: EE::API::Entities::FeatureFlag else render_api_error!(result[:message], result[:http_status]) end @@ -77,7 +77,7 @@ class FeatureFlags < Grape::API desc 'Update a feature flag' do detail 'This feature was introduced in GitLab 12.4.' - success Entities::FeatureFlag + success EE::API::Entities::FeatureFlag end params do optional :name, type: String, desc: 'The name of the tag' @@ -104,7 +104,7 @@ class FeatureFlags < Grape::API .execute(feature_flag) if result[:status] == :success - present result[:feature_flag], with: Entities::FeatureFlag + present result[:feature_flag], with: EE::API::Entities::FeatureFlag else render_api_error!(result[:message], result[:http_status]) end @@ -112,7 +112,7 @@ class FeatureFlags < Grape::API desc 'Delete a feature flag' do detail 'This feature was introduced in GitLab 12.4.' - success Entities::FeatureFlag + success EE::API::Entities::FeatureFlag end params do optional :name, type: String, desc: 'The name of the feature flag' @@ -125,7 +125,7 @@ class FeatureFlags < Grape::API .execute(feature_flag) if result[:status] == :success - present result[:feature_flag], with: Entities::FeatureFlag + present result[:feature_flag], with: EE::API::Entities::FeatureFlag else render_api_error!(result[:message], result[:http_status]) end diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index 91563ca6ec41bd..b78d291564070b 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -17,6 +17,7 @@ module API mount ::API::EpicIssues mount ::API::EpicLinks mount ::API::Epics + mount ::API::FeatureFlags mount ::API::ContainerRegistryEvent mount ::API::Geo mount ::API::GeoNodes diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index 50c1feebb168b2..dc4aa1cb2c692a 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -790,23 +790,23 @@ def can_read_vulnerabilities?(user, project) end end - class FeatureFlag < Grape::Entity + class FeatureFlagScope < Grape::Entity expose :id - expose :name - expose :description expose :active + expose :environment_scope + expose :strategies expose :created_at expose :updated_at - expose :scopes, using: Entities::FeatureFlagScope end - class FeatureFlagScope < Grape::Entity + class FeatureFlag < Grape::Entity expose :id + expose :name + expose :description expose :active - expose :environment_scope - expose :strategies expose :created_at expose :updated_at + expose :scopes, using: API::Entities::FeatureFlagScope end end end diff --git a/ee/spec/requests/api/feature_flags_spec.rb b/ee/spec/requests/api/feature_flags_spec.rb new file mode 100644 index 00000000000000..cb9943371e66d3 --- /dev/null +++ b/ee/spec/requests/api/feature_flags_spec.rb @@ -0,0 +1,763 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe API::FeatureFlags do + include FeatureFlagHelpers + + let(:project) { create(:project, :repository) } + let(:developer) { create(:user) } + let(:non_project_member) { create(:user) } + + before do + stub_licensed_features(feature_flags: true) + + project.add_developer(developer) + end + + describe 'GET /projects/:id/feature_flags' do + context 'when there are two feature flags' do + let!(:feature_flag_1) do + create(:operations_feature_flag, project: project) + end + + let!(:release_2) do + create(:operations_feature_flag, project: project) + end + + it 'returns 200 HTTP status' do + get api("/projects/#{project.id}/feature_flags", developer) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns feature flags' do + get api("/projects/#{project.id}/feature_flags", developer) + + expect(json_response.count).to eq(2) + expect(json_response.first['name']).to eq(feature_flag_1.name) + expect(json_response.second['name']).to eq(feature_flag_2.name) + end + + # it 'matches response schema' do + # get api("/projects/#{project.id}/releases", maintainer) + + # expect(response).to match_response_schema('public_api/v4/releases') + # end + end + + it 'returns an upcoming_release status for a future release' do + tomorrow = Time.now.utc + 1.day + create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: tomorrow) + + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.first['upcoming_release']).to eq(true) + end + + it 'returns an upcoming_release status for a past release' do + yesterday = Time.now.utc - 1.day + create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: yesterday) + + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.first['upcoming_release']).to eq(false) + end + + context 'when tag does not exist in git repository' do + let!(:release) { create(:release, project: project, tag: 'v1.1.5') } + + it 'returns the tag' do + get api("/projects/#{project.id}/releases", maintainer) + + expect(json_response.count).to eq(1) + expect(json_response.first['tag_name']).to eq('v1.1.5') + expect(release).to be_tag_missing + end + end + + context 'when user is a guest' do + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + author: maintainer, + created_at: 2.days.ago) + end + + it 'responds 200 OK' do + get api("/projects/#{project.id}/releases", guest) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "does not expose tag, commit and source code" do + get api("/projects/#{project.id}/releases", guest) + + expect(response).to match_response_schema('public_api/v4/release/releases_for_guest') + expect(json_response[0]['assets']['count']).to eq(release.links.count) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'responds 200 OK' do + get api("/projects/#{project.id}/releases", guest) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "exposes tag, commit and source code" do + get api("/projects/#{project.id}/releases", guest) + + expect(response).to match_response_schema('public_api/v4/releases') + expect(json_response[0]['assets']['count']).to eq(release.links.count + release.sources.count) + end + end + end + + context 'when user is not a project member' do + it 'cannot find the project' do + get api("/projects/#{project.id}/releases", non_project_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'allows the request' do + get api("/projects/#{project.id}/releases", non_project_member) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + + describe 'GET /projects/:id/releases/:tag_name' do + context 'when there is a release' do + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + sha: commit.id, + author: maintainer, + description: 'This is v0.1') + end + + it 'returns 200 HTTP status' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns a release entry' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['tag_name']).to eq(release.tag) + expect(json_response['description']).to eq('This is v0.1') + expect(json_response['author']['name']).to eq(maintainer.name) + expect(json_response['commit']['id']).to eq(commit.id) + expect(json_response['assets']['count']).to eq(4) + end + + it 'matches response schema' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to match_response_schema('public_api/v4/release') + end + + it 'contains source information as assets' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['assets']['sources'].map { |h| h['format'] }) + .to match_array(release.sources.map(&:format)) + expect(json_response['assets']['sources'].map { |h| h['url'] }) + .to match_array(release.sources.map(&:url)) + end + + context "when release description contains confidential issue's link" do + let(:confidential_issue) do + create(:issue, + :confidential, + project: project, + title: 'A vulnerability') + end + + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + sha: commit.id, + author: maintainer, + description: "This is confidential #{confidential_issue.to_reference}") + end + + it "does not expose confidential issue's title" do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['description_html']).to include(confidential_issue.to_reference) + expect(json_response['description_html']).not_to include('A vulnerability') + end + end + + context 'when release has link asset' do + let!(:link) do + create(:release_link, + release: release, + name: 'release-18.04.dmg', + url: url) + end + + let(:url) { 'https://my-external-hosting.example.com/scrambled-url/app.zip' } + + it 'contains link information as assets' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['assets']['links'].count).to eq(1) + expect(json_response['assets']['links'].first['id']).to eq(link.id) + expect(json_response['assets']['links'].first['name']) + .to eq('release-18.04.dmg') + expect(json_response['assets']['links'].first['url']) + .to eq('https://my-external-hosting.example.com/scrambled-url/app.zip') + expect(json_response['assets']['links'].first['external']) + .to be_truthy + end + + context 'when link is internal' do + let(:url) do + "#{project.web_url}/-/jobs/artifacts/v11.6.0-rc4/download?" \ + "job=rspec-mysql+41%2F50" + end + + it 'has external false' do + get api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(json_response['assets']['links'].first['external']) + .to be_falsy + end + end + end + + context 'when user is a guest' do + it 'responds 403 Forbidden' do + get api("/projects/#{project.id}/releases/v0.1", guest) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'responds 200 OK' do + get api("/projects/#{project.id}/releases/v0.1", guest) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "exposes tag and commit" do + create(:release, + project: project, + tag: 'v0.1', + author: maintainer, + created_at: 2.days.ago) + get api("/projects/#{project.id}/releases/v0.1", guest) + + expect(response).to match_response_schema('public_api/v4/release') + end + end + end + end + + context 'when specified tag is not found in the project' do + it 'cannot find the release entry' do + get api("/projects/#{project.id}/releases/non_exist_tag", maintainer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + let!(:release) { create(:release, tag: 'v0.1', project: project) } + + it 'cannot find the project' do + get api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'allows the request' do + get api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + + describe 'POST /projects/:id/releases' do + let(:params) do + { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release' + } + end + + it 'accepts the request' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:created) + end + + it 'creates a new release' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Release.count }.by(1) + + expect(project.releases.last.name).to eq('New release') + expect(project.releases.last.tag).to eq('v0.1') + expect(project.releases.last.description).to eq('Super nice release') + end + + it 'sets the released_at to the current time if the released_at parameter is not provided' do + now = Time.zone.parse('2015-08-25 06:00:00Z') + Timecop.freeze(now) do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq(now) + end + end + + it 'sets the released_at to the value in the parameters if specified' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-20T10:00:00Z' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-20T10:00:00Z') + end + + it 'assumes the utc timezone for released_at if the timezone is not provided' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-25 10:00:00' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-25T10:00:00Z') + end + + it 'allows specifying a released_at with a local time zone' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-25T10:00:00+09:00' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-25T01:00:00Z') + end + + context 'when description is empty' do + let(:params) do + { + name: 'New release', + tag_name: 'v0.1', + description: '' + } + end + + it 'returns an error as validation failure' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.not_to change { Release.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']) + .to eq("Validation failed: Description can't be blank") + end + end + + it 'matches response schema' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to match_response_schema('public_api/v4/release') + end + + it 'does not create a new tag' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.not_to change { Project.find_by_id(project.id).repository.tag_count } + end + + context 'when user is a reporter' do + it 'forbids the request' do + post api("/projects/#{project.id}/releases", reporter), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'forbids the request' do + post api("/projects/#{project.id}/releases", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'forbids the request' do + post api("/projects/#{project.id}/releases", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when create assets altogether' do + let(:base_params) do + { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release' + } + end + + context 'when create one asset' do + let(:params) do + base_params.merge({ + assets: { + links: [{ name: 'beta', url: 'https://dosuken.example.com/inspection.exe' }] + } + }) + end + + it 'accepts the request' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:created) + end + + it 'creates an asset with specified parameters' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(json_response['assets']['links'].count).to eq(1) + expect(json_response['assets']['links'].first['name']).to eq('beta') + expect(json_response['assets']['links'].first['url']) + .to eq('https://dosuken.example.com/inspection.exe') + end + + it 'matches response schema' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to match_response_schema('public_api/v4/release') + end + end + + context 'when create two assets' do + let(:params) do + base_params.merge({ + assets: { + links: [ + { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' }, + { name: 'beta', url: 'https://dosuken.example.com/beta.exe' } + ] + } + }) + end + + it 'creates two assets with specified parameters' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(json_response['assets']['links'].count).to eq(2) + expect(json_response['assets']['links'].map { |h| h['name'] }) + .to match_array(%w[alpha beta]) + expect(json_response['assets']['links'].map { |h| h['url'] }) + .to match_array(%w[https://dosuken.example.com/alpha.exe + https://dosuken.example.com/beta.exe]) + end + + context 'when link names are duplicates' do + let(:params) do + base_params.merge({ + assets: { + links: [ + { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' }, + { name: 'alpha', url: 'https://dosuken.example.com/beta.exe' } + ] + } + }) + end + + it 'recognizes as a bad request' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + end + end + + context 'when tag does not exist in git repository' do + let(:params) do + { + name: 'Android ~ Ice Cream Sandwich ~', + tag_name: tag_name, + description: 'Android 4.0–4.0.4 "Ice Cream Sandwich" is the ninth' \ + 'version of the Android mobile operating system developed' \ + 'by Google.', + ref: 'master' + } + end + + let(:tag_name) { 'v4.0' } + + it 'creates a new tag' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Project.find_by_id(project.id).repository.tag_count }.by(1) + + expect(project.repository.find_tag('v4.0').dereferenced_target.id) + .to eq(project.repository.commit('master').id) + end + + it 'creates a new release' do + expect do + post api("/projects/#{project.id}/releases", maintainer), params: params + end.to change { Release.count }.by(1) + + expect(project.releases.last.name).to eq('Android ~ Ice Cream Sandwich ~') + expect(project.releases.last.tag).to eq('v4.0') + expect(project.releases.last.description).to eq( + 'Android 4.0–4.0.4 "Ice Cream Sandwich" is the ninth' \ + 'version of the Android mobile operating system developed' \ + 'by Google.') + end + + context 'when tag name is HEAD' do + let(:tag_name) { 'HEAD' } + + it 'returns an error as failure on tag creation' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['message']).to eq('Tag name invalid') + end + end + + context 'when tag name is empty' do + let(:tag_name) { '' } + + it 'returns an error as failure on tag creation' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['message']).to eq('Tag name invalid') + end + end + end + + context 'when release already exists' do + before do + create(:release, project: project, tag: 'v0.1', name: 'New release') + end + + it 'returns an error as conflicted request' do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(response).to have_gitlab_http_status(:conflict) + end + end + end + + describe 'PUT /projects/:id/releases/:tag_name' do + let(:params) { { description: 'Best release ever!' } } + + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + name: 'New release', + released_at: '2018-03-01T22:00:00Z', + description: 'Super nice release') + end + + it 'accepts the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'updates the description' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(project.releases.last.description).to eq('Best release ever!') + end + + it 'does not change other attributes' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(project.releases.last.tag).to eq('v0.1') + expect(project.releases.last.name).to eq('New release') + expect(project.releases.last.released_at).to eq('2018-03-01T22:00:00Z') + end + + it 'matches response schema' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to match_response_schema('public_api/v4/release') + end + + it 'updates released_at' do + params = { released_at: '2015-10-10T05:00:00Z' } + + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2015-10-10T05:00:00Z') + end + + context 'when user tries to update sha' do + let(:params) { { sha: 'xxx' } } + + it 'does not allow the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when params is empty' do + let(:params) { {} } + + it 'does not allow the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when there are no corresponding releases' do + let!(:release) { } + + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is a reporter' do + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", reporter), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'forbids the request' do + put api("/projects/#{project.id}/releases/v0.1", non_project_member), + params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + describe 'DELETE /projects/:id/releases/:tag_name' do + let!(:release) do + create(:release, + project: project, + tag: 'v0.1', + name: 'New release', + description: 'Super nice release') + end + + it 'accepts the request' do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'destroys the release' do + expect do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + end.to change { Release.count }.by(-1) + end + + it 'does not remove a tag in repository' do + expect do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + end.not_to change { Project.find_by_id(project.id).repository.tag_count } + end + + it 'matches response schema' do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to match_response_schema('public_api/v4/release') + end + + context 'when there are no corresponding releases' do + let!(:release) { } + + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", maintainer) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is a reporter' do + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", reporter) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'forbids the request' do + delete api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end +end -- GitLab From 074204c3cba1f2c1667c0d7431400524366042ee Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 5 Sep 2019 20:42:09 +0700 Subject: [PATCH 03/11] Support scope API --- ee/lib/api/feature_flag/scopes.rb | 133 ++++ ee/lib/api/feature_flags.rb | 38 +- ee/lib/ee/api/api.rb | 1 + ee/lib/ee/api/entities.rb | 22 +- .../requests/api/feature_flag/scopes_spec.rb | 103 +++ ee/spec/requests/api/feature_flags_spec.rb | 748 ++---------------- .../support/helpers/feature_flag_helpers.rb | 2 +- 7 files changed, 314 insertions(+), 733 deletions(-) create mode 100644 ee/lib/api/feature_flag/scopes.rb create mode 100644 ee/spec/requests/api/feature_flag/scopes_spec.rb diff --git a/ee/lib/api/feature_flag/scopes.rb b/ee/lib/api/feature_flag/scopes.rb new file mode 100644 index 00000000000000..edd67a38b14d46 --- /dev/null +++ b/ee/lib/api/feature_flag/scopes.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +module API + module FeatureFlag + class Scopes < Grape::API + include PaginationParams + + FEATURE_FLAG_ENDPOINT_REQUIREMETS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS + .merge(name: API::NO_SLASH_URL_PART_REGEX) + + before { authorize_read_feature_flag! } + + params do + requires :id, type: String, desc: 'The ID of a project' + requires :name, type: String, desc: 'The name of the feature flag' + end + resource 'projects/:id/feature_flags/:name/scopes', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + desc 'Get all scopes of a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + use :pagination + end + get do + present paginate(feature_flag.scopes), with: EE::API::Entities::FeatureFlag::Scope + end + + desc 'Get a scope of a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + requires :scope_id, type: String + end + get ':scope_id' do + present scope, with: EE::API::Entities::FeatureFlag::Scope + end + + desc 'Create a new scope for a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + requires :environment_scope, type: String + requires :active, type: Boolean + requires :strategies, type: JSON + end + post do + puts "#{self.class.name} - #{__callee__}: 1" + authorize_update_feature_flag! + + param = { scopes_attributes: [declared_params(include_missing: false)] } + + puts "#{self.class.name} - #{__callee__}: param: #{param}" + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, param) + .execute(feature_flag) + + if result[:status] == :success + present result[:feature_flag].scopes.last, with: EE::API::Entities::FeatureFlag::Scope + else + render_api_error!(result[:message], result[:http_status]) + end + end + + desc 'Update a scope of a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + requires :scope_id, type: String + optional :environment_scope, type: String + optional :active, type: Boolean + optional :strategies, type: JSON + end + put ':scope_id' do + authorize_update_feature_flag! + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute(feature_flag) + + if result[:status] == :success + present result[:feature_flag], with: EE::API::Entities::FeatureFlag::Scope + else + render_api_error!(result[:message], result[:http_status]) + end + end + + desc 'Delete a scope from a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + optional :scope_id, type: String, desc: 'The name of the feature flag' + end + delete ':scope_id' do + authorize_update_feature_flag! + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute(feature_flag) + + if result[:status] == :success + present result[:feature_flag], with: EE::API::Entities::FeatureFlag::Scope + else + render_api_error!(result[:message], result[:http_status]) + end + end + end + + helpers do + def authorize_read_feature_flag! + authorize! :read_feature_flag, feature_flag + end + + def authorize_update_feature_flag! + authorize! :update_feature_flag, feature_flag + end + + def feature_flag + @feature_flag ||= user_project.operations_feature_flags.find_by_name!(params[:name]) + end + + def scope + @scope ||= feature_flag.scopes.find_by_id!(params[:scope_id]) + end + end + end + end +end diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb index ad889d7657b733..ce07ebfeb84995 100644 --- a/ee/lib/api/feature_flags.rb +++ b/ee/lib/api/feature_flags.rb @@ -45,26 +45,18 @@ class FeatureFlags < Grape::API success EE::API::Entities::FeatureFlag end params do - requires :name, type: String, desc: 'The name of the tag' - requires :description, type: String, desc: 'The name of the release' - requires :active, type: Boolean, desc: 'The name of the release' - optional :scopes, type: Hash do + requires :name, type: String + optional :description, type: String + optional :scopes_attributes, type: Array do requires :environment_scope, type: String requires :active, type: Boolean - optional :strategies, type: Hash do - requires :name, type: String - requires :parameters, type: Hash do - requires :groupId, type: String - requires :percentage, type: String - requires :userIds, type: String - end - end + requires :strategies, type: JSON end end post ':id/feature_flags' do authorize_create_feature_flag! - result = FeatureFlags::CreateService + result = ::FeatureFlags::CreateService .new(user_project, current_user, declared_params(include_missing: false)) .execute @@ -80,21 +72,9 @@ class FeatureFlags < Grape::API success EE::API::Entities::FeatureFlag end params do - optional :name, type: String, desc: 'The name of the tag' - optional :description, type: String, desc: 'The name of the release' - optional :active, type: Boolean, desc: 'The name of the release' - optional :scopes, type: Hash do - requires :environment_scope, type: String - requires :active, type: Boolean - optional :strategies, type: Hash do - requires :name, type: String - requires :parameters, type: Hash do - requires :groupId, type: String - requires :percentage, type: String - requires :userIds, type: String - end - end - end + optional :new_name, type: String + optional :description, type: String + at_least_one_of :new_name, :description end put ':id/feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do authorize_update_feature_flag! @@ -118,7 +98,7 @@ class FeatureFlags < Grape::API optional :name, type: String, desc: 'The name of the feature flag' end delete ':id/feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do - authorize_destroy_release! + authorize_destroy_feature_flag! result = ::FeatureFlags::DestroyService .new(user_project, current_user, declared_params(include_missing: false)) diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index b78d291564070b..b0a1d89cfb36f2 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -18,6 +18,7 @@ module API mount ::API::EpicLinks mount ::API::Epics mount ::API::FeatureFlags + mount ::API::FeatureFlag::Scopes mount ::API::ContainerRegistryEvent mount ::API::Geo mount ::API::GeoNodes diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index dc4aa1cb2c692a..e996ad91d31ffb 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -790,23 +790,21 @@ def can_read_vulnerabilities?(user, project) end end - class FeatureFlagScope < Grape::Entity - expose :id - expose :active - expose :environment_scope - expose :strategies - expose :created_at - expose :updated_at - end - class FeatureFlag < Grape::Entity - expose :id + class Scope < Grape::Entity + expose :id + expose :active + expose :environment_scope + expose :strategies + expose :created_at + expose :updated_at + end + expose :name expose :description - expose :active expose :created_at expose :updated_at - expose :scopes, using: API::Entities::FeatureFlagScope + expose :scopes, using: Scope end end end diff --git a/ee/spec/requests/api/feature_flag/scopes_spec.rb b/ee/spec/requests/api/feature_flag/scopes_spec.rb new file mode 100644 index 00000000000000..f7c2ac46ff7b3e --- /dev/null +++ b/ee/spec/requests/api/feature_flag/scopes_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe API::FeatureFlag::Scopes do + include FeatureFlagHelpers + + let(:project) { create(:project, :repository) } + let(:developer) { create(:user) } + + before do + stub_licensed_features(feature_flags: true) + + project.add_developer(developer) + end + + describe 'GET /projects/:id/feature_flags/:name/scopes' do + context 'when there are two scopes' do + let!(:feature_flag) { create_flag(project, 'test') } + let!(:additional_scope) { create_scope(feature_flag, 'production', false) } + + it 'returns scopes' do + get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes", developer) + + puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.count).to eq(2) + expect(json_response.first['environment_scope']).to eq(feature_flag.scopes[0].environment_scope) + expect(json_response.second['environment_scope']).to eq(feature_flag.scopes[1].environment_scope) + end + end + end + + describe 'GET /projects/:id/feature_flags/:name/scopes/:scope_id' do + context 'when there is a feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + let(:default_scope) { feature_flag.default_scope } + + it 'returns a scope' do + get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{default_scope.id}", developer) + + puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(default_scope.id) + expect(json_response['active']).to eq(default_scope.active) + expect(json_response['environment_scope']).to eq(default_scope.environment_scope) + end + end + end + + describe 'POST /projects/:id/feature_flags/:name/scopes' do + let(:params) do + { + environment_scope: 'staging', + active: false, + strategies: [{ + name: 'userWithId', + parameters: { userIds: 'user:1,project:1,group:1' } + }].to_json + } + end + + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + + it 'creates a new scope' do + post api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes", developer), params: params + + expect(response).to have_gitlab_http_status(:created) + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(scope.active).to eq(params[:active]) + expect(scope.strategies).to eq(JSON.parse(params[:strategies])) + end + end + + describe 'PUT /projects/:id/feature_flags/:name' do + let(:params) { { description: 'bbbb' } } + + let!(:feature_flag) do + create(:operations_feature_flag, project: project, description: 'aaaaa') + end + + it 'updates the name' do + put api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(project.operations_feature_flags.last.description).to eq('bbbb') + end + end + + describe 'DELETE /projects/:id/feature_flags/:name' do + let!(:feature_flag) do + create(:operations_feature_flag, project: project) + end + + it 'destroys the release' do + expect do + delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer) + end.to change { Operations::FeatureFlag.count }.by(-1) + + expect(response).to have_gitlab_http_status(:ok) + end + end +end diff --git a/ee/spec/requests/api/feature_flags_spec.rb b/ee/spec/requests/api/feature_flags_spec.rb index cb9943371e66d3..02332de59db554 100644 --- a/ee/spec/requests/api/feature_flags_spec.rb +++ b/ee/spec/requests/api/feature_flags_spec.rb @@ -20,744 +20,110 @@ create(:operations_feature_flag, project: project) end - let!(:release_2) do + let!(:feature_flag_2) do create(:operations_feature_flag, project: project) end - it 'returns 200 HTTP status' do - get api("/projects/#{project.id}/feature_flags", developer) - - expect(response).to have_gitlab_http_status(:ok) - end - it 'returns feature flags' do get api("/projects/#{project.id}/feature_flags", developer) + puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" + expect(response).to have_gitlab_http_status(:ok) expect(json_response.count).to eq(2) expect(json_response.first['name']).to eq(feature_flag_1.name) expect(json_response.second['name']).to eq(feature_flag_2.name) end - - # it 'matches response schema' do - # get api("/projects/#{project.id}/releases", maintainer) - - # expect(response).to match_response_schema('public_api/v4/releases') - # end - end - - it 'returns an upcoming_release status for a future release' do - tomorrow = Time.now.utc + 1.day - create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: tomorrow) - - get api("/projects/#{project.id}/releases", maintainer) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.first['upcoming_release']).to eq(true) - end - - it 'returns an upcoming_release status for a past release' do - yesterday = Time.now.utc - 1.day - create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: yesterday) - - get api("/projects/#{project.id}/releases", maintainer) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.first['upcoming_release']).to eq(false) - end - - context 'when tag does not exist in git repository' do - let!(:release) { create(:release, project: project, tag: 'v1.1.5') } - - it 'returns the tag' do - get api("/projects/#{project.id}/releases", maintainer) - - expect(json_response.count).to eq(1) - expect(json_response.first['tag_name']).to eq('v1.1.5') - expect(release).to be_tag_missing - end - end - - context 'when user is a guest' do - let!(:release) do - create(:release, - project: project, - tag: 'v0.1', - author: maintainer, - created_at: 2.days.ago) - end - - it 'responds 200 OK' do - get api("/projects/#{project.id}/releases", guest) - - expect(response).to have_gitlab_http_status(:ok) - end - - it "does not expose tag, commit and source code" do - get api("/projects/#{project.id}/releases", guest) - - expect(response).to match_response_schema('public_api/v4/release/releases_for_guest') - expect(json_response[0]['assets']['count']).to eq(release.links.count) - end - - context 'when project is public' do - let(:project) { create(:project, :repository, :public) } - - it 'responds 200 OK' do - get api("/projects/#{project.id}/releases", guest) - - expect(response).to have_gitlab_http_status(:ok) - end - - it "exposes tag, commit and source code" do - get api("/projects/#{project.id}/releases", guest) - - expect(response).to match_response_schema('public_api/v4/releases') - expect(json_response[0]['assets']['count']).to eq(release.links.count + release.sources.count) - end - end - end - - context 'when user is not a project member' do - it 'cannot find the project' do - get api("/projects/#{project.id}/releases", non_project_member) - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'when project is public' do - let(:project) { create(:project, :repository, :public) } - - it 'allows the request' do - get api("/projects/#{project.id}/releases", non_project_member) - - expect(response).to have_gitlab_http_status(:ok) - end - end end end - describe 'GET /projects/:id/releases/:tag_name' do - context 'when there is a release' do - let!(:release) do - create(:release, - project: project, - tag: 'v0.1', - sha: commit.id, - author: maintainer, - description: 'This is v0.1') + describe 'GET /projects/:id/feature_flags/:name' do + context 'when there is a feature flag' do + let!(:feature_flag) do + create(:operations_feature_flag, project: project) end - it 'returns 200 HTTP status' do - get api("/projects/#{project.id}/releases/v0.1", maintainer) + it 'returns a feature flag entry' do + get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer) + puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" expect(response).to have_gitlab_http_status(:ok) - end - - it 'returns a release entry' do - get api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(json_response['tag_name']).to eq(release.tag) - expect(json_response['description']).to eq('This is v0.1') - expect(json_response['author']['name']).to eq(maintainer.name) - expect(json_response['commit']['id']).to eq(commit.id) - expect(json_response['assets']['count']).to eq(4) - end - - it 'matches response schema' do - get api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(response).to match_response_schema('public_api/v4/release') - end - - it 'contains source information as assets' do - get api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(json_response['assets']['sources'].map { |h| h['format'] }) - .to match_array(release.sources.map(&:format)) - expect(json_response['assets']['sources'].map { |h| h['url'] }) - .to match_array(release.sources.map(&:url)) - end - - context "when release description contains confidential issue's link" do - let(:confidential_issue) do - create(:issue, - :confidential, - project: project, - title: 'A vulnerability') - end - - let!(:release) do - create(:release, - project: project, - tag: 'v0.1', - sha: commit.id, - author: maintainer, - description: "This is confidential #{confidential_issue.to_reference}") - end - - it "does not expose confidential issue's title" do - get api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(json_response['description_html']).to include(confidential_issue.to_reference) - expect(json_response['description_html']).not_to include('A vulnerability') - end - end - - context 'when release has link asset' do - let!(:link) do - create(:release_link, - release: release, - name: 'release-18.04.dmg', - url: url) - end - - let(:url) { 'https://my-external-hosting.example.com/scrambled-url/app.zip' } - - it 'contains link information as assets' do - get api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(json_response['assets']['links'].count).to eq(1) - expect(json_response['assets']['links'].first['id']).to eq(link.id) - expect(json_response['assets']['links'].first['name']) - .to eq('release-18.04.dmg') - expect(json_response['assets']['links'].first['url']) - .to eq('https://my-external-hosting.example.com/scrambled-url/app.zip') - expect(json_response['assets']['links'].first['external']) - .to be_truthy - end - - context 'when link is internal' do - let(:url) do - "#{project.web_url}/-/jobs/artifacts/v11.6.0-rc4/download?" \ - "job=rspec-mysql+41%2F50" - end - - it 'has external false' do - get api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(json_response['assets']['links'].first['external']) - .to be_falsy - end - end - end - - context 'when user is a guest' do - it 'responds 403 Forbidden' do - get api("/projects/#{project.id}/releases/v0.1", guest) - - expect(response).to have_gitlab_http_status(:forbidden) - end - - context 'when project is public' do - let(:project) { create(:project, :repository, :public) } - - it 'responds 200 OK' do - get api("/projects/#{project.id}/releases/v0.1", guest) - - expect(response).to have_gitlab_http_status(:ok) - end - - it "exposes tag and commit" do - create(:release, - project: project, - tag: 'v0.1', - author: maintainer, - created_at: 2.days.ago) - get api("/projects/#{project.id}/releases/v0.1", guest) - - expect(response).to match_response_schema('public_api/v4/release') - end - end - end - end - - context 'when specified tag is not found in the project' do - it 'cannot find the release entry' do - get api("/projects/#{project.id}/releases/non_exist_tag", maintainer) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user is not a project member' do - let!(:release) { create(:release, tag: 'v0.1', project: project) } - - it 'cannot find the project' do - get api("/projects/#{project.id}/releases/v0.1", non_project_member) - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'when project is public' do - let(:project) { create(:project, :repository, :public) } - - it 'allows the request' do - get api("/projects/#{project.id}/releases/v0.1", non_project_member) - - expect(response).to have_gitlab_http_status(:ok) - end + expect(json_response['id']).to eq(feature_flag.id) + expect(json_response['name']).to eq(feature_flag.name) + expect(json_response['description']).to eq(feature_flag.description) end end end - describe 'POST /projects/:id/releases' do + describe 'POST /projects/:id/feature_flags' do let(:params) do { - name: 'New release', - tag_name: 'v0.1', - description: 'Super nice release' - } - end - - it 'accepts the request' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(response).to have_gitlab_http_status(:created) - end - - it 'creates a new release' do - expect do - post api("/projects/#{project.id}/releases", maintainer), params: params - end.to change { Release.count }.by(1) - - expect(project.releases.last.name).to eq('New release') - expect(project.releases.last.tag).to eq('v0.1') - expect(project.releases.last.description).to eq('Super nice release') - end - - it 'sets the released_at to the current time if the released_at parameter is not provided' do - now = Time.zone.parse('2015-08-25 06:00:00Z') - Timecop.freeze(now) do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(project.releases.last.released_at).to eq(now) - end - end - - it 'sets the released_at to the value in the parameters if specified' do - params = { - name: 'New release', - tag_name: 'v0.1', - description: 'Super nice release', - released_at: '2019-03-20T10:00:00Z' - } - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(project.releases.last.released_at).to eq('2019-03-20T10:00:00Z') - end - - it 'assumes the utc timezone for released_at if the timezone is not provided' do - params = { - name: 'New release', - tag_name: 'v0.1', - description: 'Super nice release', - released_at: '2019-03-25 10:00:00' - } - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(project.releases.last.released_at).to eq('2019-03-25T10:00:00Z') - end - - it 'allows specifying a released_at with a local time zone' do - params = { - name: 'New release', - tag_name: 'v0.1', - description: 'Super nice release', - released_at: '2019-03-25T10:00:00+09:00' - } - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(project.releases.last.released_at).to eq('2019-03-25T01:00:00Z') - end - - context 'when description is empty' do - let(:params) do - { - name: 'New release', - tag_name: 'v0.1', - description: '' - } - end - - it 'returns an error as validation failure' do - expect do - post api("/projects/#{project.id}/releases", maintainer), params: params - end.not_to change { Release.count } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']) - .to eq("Validation failed: Description can't be blank") - end - end - - it 'matches response schema' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(response).to match_response_schema('public_api/v4/release') - end - - it 'does not create a new tag' do - expect do - post api("/projects/#{project.id}/releases", maintainer), params: params - end.not_to change { Project.find_by_id(project.id).repository.tag_count } - end - - context 'when user is a reporter' do - it 'forbids the request' do - post api("/projects/#{project.id}/releases", reporter), params: params - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user is not a project member' do - it 'forbids the request' do - post api("/projects/#{project.id}/releases", non_project_member), - params: params - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'when project is public' do - let(:project) { create(:project, :repository, :public) } - - it 'forbids the request' do - post api("/projects/#{project.id}/releases", non_project_member), - params: params - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when create assets altogether' do - let(:base_params) do + name: 'awesome-feature', + description: 'aaaaaaaa', + scopes_attributes: [ { - name: 'New release', - tag_name: 'v0.1', - description: 'Super nice release' - } - end - - context 'when create one asset' do - let(:params) do - base_params.merge({ - assets: { - links: [{ name: 'beta', url: 'https://dosuken.example.com/inspection.exe' }] - } - }) - end - - it 'accepts the request' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(response).to have_gitlab_http_status(:created) - end - - it 'creates an asset with specified parameters' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(json_response['assets']['links'].count).to eq(1) - expect(json_response['assets']['links'].first['name']).to eq('beta') - expect(json_response['assets']['links'].first['url']) - .to eq('https://dosuken.example.com/inspection.exe') - end - - it 'matches response schema' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(response).to match_response_schema('public_api/v4/release') - end - end - - context 'when create two assets' do - let(:params) do - base_params.merge({ - assets: { - links: [ - { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' }, - { name: 'beta', url: 'https://dosuken.example.com/beta.exe' } - ] + environment_scope: '*', + active: true, + strategies: [{ + name: 'default', + parameters: {} + }].to_json + }, + { + environment_scope: 'production', + active: true, + strategies: [{ + name: 'userWithId', + parameters: { + userIds: 'user:1' } - }) - end - - it 'creates two assets with specified parameters' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(json_response['assets']['links'].count).to eq(2) - expect(json_response['assets']['links'].map { |h| h['name'] }) - .to match_array(%w[alpha beta]) - expect(json_response['assets']['links'].map { |h| h['url'] }) - .to match_array(%w[https://dosuken.example.com/alpha.exe - https://dosuken.example.com/beta.exe]) - end - - context 'when link names are duplicates' do - let(:params) do - base_params.merge({ - assets: { - links: [ - { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' }, - { name: 'alpha', url: 'https://dosuken.example.com/beta.exe' } - ] - } - }) - end - - it 'recognizes as a bad request' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end - end + }].to_json + }] + } end - context 'when tag does not exist in git repository' do - let(:params) do - { - name: 'Android ~ Ice Cream Sandwich ~', - tag_name: tag_name, - description: 'Android 4.0–4.0.4 "Ice Cream Sandwich" is the ninth' \ - 'version of the Android mobile operating system developed' \ - 'by Google.', - ref: 'master' - } - end - - let(:tag_name) { 'v4.0' } - - it 'creates a new tag' do - expect do - post api("/projects/#{project.id}/releases", maintainer), params: params - end.to change { Project.find_by_id(project.id).repository.tag_count }.by(1) - - expect(project.repository.find_tag('v4.0').dereferenced_target.id) - .to eq(project.repository.commit('master').id) - end - - it 'creates a new release' do - expect do - post api("/projects/#{project.id}/releases", maintainer), params: params - end.to change { Release.count }.by(1) - - expect(project.releases.last.name).to eq('Android ~ Ice Cream Sandwich ~') - expect(project.releases.last.tag).to eq('v4.0') - expect(project.releases.last.description).to eq( - 'Android 4.0–4.0.4 "Ice Cream Sandwich" is the ninth' \ - 'version of the Android mobile operating system developed' \ - 'by Google.') - end - - context 'when tag name is HEAD' do - let(:tag_name) { 'HEAD' } - - it 'returns an error as failure on tag creation' do - post api("/projects/#{project.id}/releases", maintainer), params: params + it 'creates a new feature flag' do + post api("/projects/#{project.id}/feature_flags", developer), params: params - expect(response).to have_gitlab_http_status(:internal_server_error) - expect(json_response['message']).to eq('Tag name invalid') - end - end - - context 'when tag name is empty' do - let(:tag_name) { '' } - - it 'returns an error as failure on tag creation' do - post api("/projects/#{project.id}/releases", maintainer), params: params - - expect(response).to have_gitlab_http_status(:internal_server_error) - expect(json_response['message']).to eq('Tag name invalid') - end - end - end - - context 'when release already exists' do - before do - create(:release, project: project, tag: 'v0.1', name: 'New release') - end + expect(response).to have_gitlab_http_status(:created) - it 'returns an error as conflicted request' do - post api("/projects/#{project.id}/releases", maintainer), params: params + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + expect(feature_flag.description).to eq(params[:description]) - expect(response).to have_gitlab_http_status(:conflict) + feature_flag.scopes.each_with_index do |scope, index| + expect(scope.environment_scope).to eq(params[:scopes_attributes][index][:environment_scope]) + expect(scope.active).to eq(params[:scopes_attributes][index][:active]) + expect(scope.strategies).to eq(JSON.parse(params[:scopes_attributes][index][:strategies])) end end end - describe 'PUT /projects/:id/releases/:tag_name' do - let(:params) { { description: 'Best release ever!' } } + describe 'PUT /projects/:id/feature_flags/:name' do + let(:params) { { description: 'bbbb' } } - let!(:release) do - create(:release, - project: project, - tag: 'v0.1', - name: 'New release', - released_at: '2018-03-01T22:00:00Z', - description: 'Super nice release') + let!(:feature_flag) do + create(:operations_feature_flag, project: project, description: 'aaaaa') end - it 'accepts the request' do - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + it 'updates the name' do + put api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer), params: params expect(response).to have_gitlab_http_status(:ok) - end - - it 'updates the description' do - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params - - expect(project.releases.last.description).to eq('Best release ever!') - end - - it 'does not change other attributes' do - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params - - expect(project.releases.last.tag).to eq('v0.1') - expect(project.releases.last.name).to eq('New release') - expect(project.releases.last.released_at).to eq('2018-03-01T22:00:00Z') - end - - it 'matches response schema' do - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params - - expect(response).to match_response_schema('public_api/v4/release') - end - - it 'updates released_at' do - params = { released_at: '2015-10-10T05:00:00Z' } - - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params - - expect(project.releases.last.released_at).to eq('2015-10-10T05:00:00Z') - end - - context 'when user tries to update sha' do - let(:params) { { sha: 'xxx' } } - - it 'does not allow the request' do - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - - context 'when params is empty' do - let(:params) { {} } - - it 'does not allow the request' do - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - - context 'when there are no corresponding releases' do - let!(:release) { } - - it 'forbids the request' do - put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user is a reporter' do - it 'forbids the request' do - put api("/projects/#{project.id}/releases/v0.1", reporter), params: params - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user is not a project member' do - it 'forbids the request' do - put api("/projects/#{project.id}/releases/v0.1", non_project_member), - params: params - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'when project is public' do - let(:project) { create(:project, :repository, :public) } - - it 'forbids the request' do - put api("/projects/#{project.id}/releases/v0.1", non_project_member), - params: params - - expect(response).to have_gitlab_http_status(:forbidden) - end - end + expect(project.operations_feature_flags.last.description).to eq('bbbb') end end - describe 'DELETE /projects/:id/releases/:tag_name' do - let!(:release) do - create(:release, - project: project, - tag: 'v0.1', - name: 'New release', - description: 'Super nice release') - end - - it 'accepts the request' do - delete api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(response).to have_gitlab_http_status(:ok) + describe 'DELETE /projects/:id/feature_flags/:name' do + let!(:feature_flag) do + create(:operations_feature_flag, project: project) end it 'destroys the release' do expect do - delete api("/projects/#{project.id}/releases/v0.1", maintainer) - end.to change { Release.count }.by(-1) - end - - it 'does not remove a tag in repository' do - expect do - delete api("/projects/#{project.id}/releases/v0.1", maintainer) - end.not_to change { Project.find_by_id(project.id).repository.tag_count } - end - - it 'matches response schema' do - delete api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(response).to match_response_schema('public_api/v4/release') - end - - context 'when there are no corresponding releases' do - let!(:release) { } + delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer) + end.to change { Operations::FeatureFlag.count }.by(-1) - it 'forbids the request' do - delete api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user is a reporter' do - it 'forbids the request' do - delete api("/projects/#{project.id}/releases/v0.1", reporter) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when user is not a project member' do - it 'forbids the request' do - delete api("/projects/#{project.id}/releases/v0.1", non_project_member) - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'when project is public' do - let(:project) { create(:project, :repository, :public) } - - it 'forbids the request' do - delete api("/projects/#{project.id}/releases/v0.1", non_project_member) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end + expect(response).to have_gitlab_http_status(:ok) end end end diff --git a/ee/spec/support/helpers/feature_flag_helpers.rb b/ee/spec/support/helpers/feature_flag_helpers.rb index e8b7cd3e58c6c8..ec1907d0adcd6b 100644 --- a/ee/spec/support/helpers/feature_flag_helpers.rb +++ b/ee/spec/support/helpers/feature_flag_helpers.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module FeatureFlagHelpers - def create_flag(project, name, active, description: nil) + def create_flag(project, name, active = true, description: nil) create(:operations_feature_flag, name: name, active: active, description: description, project: project) end -- GitLab From db602ac34637ff50d394283e30113d1e46779520 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 6 Sep 2019 15:57:49 +0700 Subject: [PATCH 04/11] Finish spec --- ee/lib/api/feature_flag/scopes.rb | 16 +++++++---- .../requests/api/feature_flag/scopes_spec.rb | 28 +++++++++---------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/ee/lib/api/feature_flag/scopes.rb b/ee/lib/api/feature_flag/scopes.rb index edd67a38b14d46..8aa615e8e4e038 100644 --- a/ee/lib/api/feature_flag/scopes.rb +++ b/ee/lib/api/feature_flag/scopes.rb @@ -78,12 +78,16 @@ class Scopes < Grape::API put ':scope_id' do authorize_update_feature_flag! + param = declared_params(include_missing: false) + param[:id] = param.delete(:scope_id) + param = { scopes_attributes: [param] } + result = ::FeatureFlags::UpdateService - .new(user_project, current_user, declared_params(include_missing: false)) + .new(user_project, current_user, param) .execute(feature_flag) if result[:status] == :success - present result[:feature_flag], with: EE::API::Entities::FeatureFlag::Scope + present scope.reload, with: EE::API::Entities::FeatureFlag::Scope else render_api_error!(result[:message], result[:http_status]) end @@ -94,17 +98,19 @@ class Scopes < Grape::API success EE::API::Entities::FeatureFlag::Scope end params do - optional :scope_id, type: String, desc: 'The name of the feature flag' + optional :scope_id, type: String, desc: 'The scope' end delete ':scope_id' do authorize_update_feature_flag! + param = { scopes_attributes: [{ id: scope.id, _destroy: 1 }] } + result = ::FeatureFlags::UpdateService - .new(user_project, current_user, declared_params(include_missing: false)) + .new(user_project, current_user, param) .execute(feature_flag) if result[:status] == :success - present result[:feature_flag], with: EE::API::Entities::FeatureFlag::Scope + present scope, with: EE::API::Entities::FeatureFlag::Scope else render_api_error!(result[:message], result[:http_status]) end diff --git a/ee/spec/requests/api/feature_flag/scopes_spec.rb b/ee/spec/requests/api/feature_flag/scopes_spec.rb index f7c2ac46ff7b3e..5d54bfceb3cb8f 100644 --- a/ee/spec/requests/api/feature_flag/scopes_spec.rb +++ b/ee/spec/requests/api/feature_flag/scopes_spec.rb @@ -72,30 +72,30 @@ end end - describe 'PUT /projects/:id/feature_flags/:name' do - let(:params) { { description: 'bbbb' } } + describe 'PUT /projects/:id/feature_flags/:name/scopes/:scope_id' do + let(:params) { { active: true } } - let!(:feature_flag) do - create(:operations_feature_flag, project: project, description: 'aaaaa') - end + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + let!(:production_scope) { create_scope(feature_flag, 'production', false) } it 'updates the name' do - put api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer), params: params + put api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{production_scope.id}", developer), params: params expect(response).to have_gitlab_http_status(:ok) - expect(project.operations_feature_flags.last.description).to eq('bbbb') + + production_scope.reload + expect(production_scope.active).to eq(true) end end - describe 'DELETE /projects/:id/feature_flags/:name' do - let!(:feature_flag) do - create(:operations_feature_flag, project: project) - end + describe 'DELETE /projects/:id/feature_flags/:name/scopes/:scope_id' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + let!(:production_scope) { create_scope(feature_flag, 'production', false) } - it 'destroys the release' do + it 'destroys the scope' do expect do - delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer) - end.to change { Operations::FeatureFlag.count }.by(-1) + delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{production_scope.id}", developer) + end.to change { Operations::FeatureFlagScope.count }.by(-1) expect(response).to have_gitlab_http_status(:ok) end -- GitLab From f487fefff0b6fc62abad1794e33e494ffb86ab66 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 6 Sep 2019 19:51:42 +0700 Subject: [PATCH 05/11] Implementing enable --- ee/lib/api/feature_flag/scopes.rb | 3 - .../requests/api/feature_flag/scopes_spec.rb | 2 - lib/feature_flag/adapters/unleash.rb | 72 +++++++++++++++++-- lib/unleash/strategy/user_with_id.rb | 5 +- 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/ee/lib/api/feature_flag/scopes.rb b/ee/lib/api/feature_flag/scopes.rb index 8aa615e8e4e038..e0e2a4029f1326 100644 --- a/ee/lib/api/feature_flag/scopes.rb +++ b/ee/lib/api/feature_flag/scopes.rb @@ -47,13 +47,10 @@ class Scopes < Grape::API requires :strategies, type: JSON end post do - puts "#{self.class.name} - #{__callee__}: 1" authorize_update_feature_flag! param = { scopes_attributes: [declared_params(include_missing: false)] } - puts "#{self.class.name} - #{__callee__}: param: #{param}" - result = ::FeatureFlags::UpdateService .new(user_project, current_user, param) .execute(feature_flag) diff --git a/ee/spec/requests/api/feature_flag/scopes_spec.rb b/ee/spec/requests/api/feature_flag/scopes_spec.rb index 5d54bfceb3cb8f..e1f7abac826b43 100644 --- a/ee/spec/requests/api/feature_flag/scopes_spec.rb +++ b/ee/spec/requests/api/feature_flag/scopes_spec.rb @@ -21,7 +21,6 @@ it 'returns scopes' do get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes", developer) - puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" expect(response).to have_gitlab_http_status(:ok) expect(json_response.count).to eq(2) expect(json_response.first['environment_scope']).to eq(feature_flag.scopes[0].environment_scope) @@ -38,7 +37,6 @@ it 'returns a scope' do get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{default_scope.id}", developer) - puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(default_scope.id) expect(json_response['active']).to eq(default_scope.active) diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb index 60941765b22b50..f94ef53d31516e 100644 --- a/lib/feature_flag/adapters/unleash.rb +++ b/lib/feature_flag/adapters/unleash.rb @@ -19,7 +19,32 @@ def off?(thing = nil) end def enable(thing = true) - # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + # TODO: How to control flags? + if persisted? + + else + + end + + strategies = if thing == true + { name: 'default', parameters: {} } + else + { name: 'userWithId', parameters: { userIds: sanitized(thing) } } + end + + responce = HTTParty.post(Unleash.create_feature_flag_url, + headers: Unleash.request_headers, + body: { name: @key, + scopes_attributes: [{ + environment_scope: Gitlab.config.unleash.app_name, + active: true, + strategies: strategies}]}.to_json) + + responce.map do |feature_flag| + feature = Feature.new(feature_flag[:name]) + feature.state = + feature + end end def disable(thing = false) @@ -38,24 +63,43 @@ def remove # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) end + def persisted? + Unleash.toggles.select{ |toggle| toggle['name'] == @key } + end + private + def enable(thing = true) + + end + def client FeatureFlag::Adapters::Unleash.client end def context(thing) - ::Unleash::Context.new(properties: { thing: thing }) + ::Unleash::Context.new(properties: { thing: sanitized(thing) }) + end + + def sanitized(thing) + thing = thing.__getobj__ if thing.respond_to?(:__getobj__) # Resolve SimpleDelegator + + return thing unless thing.is_a?(ActiveRecord::Base) + + "#{thing.class.name}:#{thing.id}" end end class << self + include Gitlab::Utils::StrongMemoize + def available? Gitlab.config.unleash.enabled end def all - Unleash::ToggleFetcher.toggle_cache + # TODO: Wrap in Feature + Unleash.toggles end def get(key) @@ -63,7 +107,7 @@ def get(key) end def persisted?(feature) - true # TODO: Fix + get(key).persisted? end def table_exists? @@ -79,6 +123,26 @@ def configure end end + def create_feature_flag_url + "#{api_endpoint}/" + end + + def api_endpoint + strong_memoize(:api_endpoint) do + api_url, project_id = Gitlab.config.unleash.url + .scan( %r{(https?://.*/api/v4/)feature_flags/unleash/(\d+)} ) + .first + + "#{api_url}/projects/#{project_id}/feature_flags" + end + end + + def request_headers + strong_memoize(:request_headers) do + { 'Private-Token': Gitlab.config.unleash.personal_access_token } + end + end + def client # TODO: Fix @client ||= if defined?(UNLEASH) diff --git a/lib/unleash/strategy/user_with_id.rb b/lib/unleash/strategy/user_with_id.rb index 8f1e09a64c1526..c9790f58e2d798 100644 --- a/lib/unleash/strategy/user_with_id.rb +++ b/lib/unleash/strategy/user_with_id.rb @@ -24,10 +24,7 @@ def is_enabled?(params = {}, context = nil) return false unless params.fetch(PARAM, nil).is_a? String return false unless context.class.name == 'Unleash::Context' - target = context.properties[:thing].yield_self do |thing| - thing = thing.__getobj__ if thing.respond_to?(:__getobj__) # Resolve SimpleDelegator - "#{thing.class.name}:#{thing.id}" - end + target = context.properties[:thing] params[PARAM].split(",").map(&:strip).any? { |allowed| allowed == target } end -- GitLab From eb008189a9a9314972ea4d60b00c293d8e67a9f7 Mon Sep 17 00:00:00 2001 From: Jason Goodman Date: Wed, 11 Sep 2019 18:20:22 -0400 Subject: [PATCH 06/11] Fixup unleash adapter --- lib/feature_flag/adapters/unleash.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb index f94ef53d31516e..37d96ac02a136a 100644 --- a/lib/feature_flag/adapters/unleash.rb +++ b/lib/feature_flag/adapters/unleash.rb @@ -64,7 +64,8 @@ def remove end def persisted? - Unleash.toggles.select{ |toggle| toggle['name'] == @key } + toggles = ::Unleash.toggles + toggles.present? && toggles.select { |toggle| toggle['name'] == @key } end private @@ -107,7 +108,8 @@ def get(key) end def persisted?(feature) - get(key).persisted? + # get(key).persisted? + feature.persisted? end def table_exists? -- GitLab From 3fb51b373039ad7826ed2057aa4f7b134d6f7775 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 17 Sep 2019 18:38:07 +0700 Subject: [PATCH 07/11] Introduce addtiona API enable/disable for controlling flags --- ee/lib/api/feature_flags.rb | 120 +++++++++++++++++++ ee/spec/requests/api/feature_flags_spec.rb | 133 ++++++++++++++++++++- lib/feature_flag/adapters/unleash.rb | 60 +++++----- 3 files changed, 281 insertions(+), 32 deletions(-) diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb index ce07ebfeb84995..2d9744cdcb597b 100644 --- a/ee/lib/api/feature_flags.rb +++ b/ee/lib/api/feature_flags.rb @@ -90,6 +90,126 @@ class FeatureFlags < Grape::API end end + desc 'Enable a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag + end + params do + requires :name, type: String + requires :environment_scope, type: String + requires :strategy, type: JSON + end + post ':id/feature_flags/enable', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + authorize_create_feature_flag! + + result = nil + + if feature_flag = user_project.operations_feature_flags.find_by_name(params[:name]) + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + + update_params = unless scope + { + scopes_attributes:[{ + active: true, + environment_scope: params[:environment_scope], + strategies: [params[:strategy]] + }] + } + else + { + scopes_attributes:[{ + id: scope.id, + active: true, + strategies: scope.strategies.push(params[:strategy]) + }] + } + end + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, update_params) + .execute(feature_flag) + else + create_params = { + name: params[:name], + scopes_attributes:[{ + active: false, + environment_scope: '*' + },{ + active: true, + environment_scope: params[:environment_scope], + strategies: [params[:strategy]] + }] + } + + result = ::FeatureFlags::CreateService + .new(user_project, current_user, create_params) + .execute + end + + if result.nil? + render_api_error!('Bad request', 400) + elsif result[:status] == :success + present result[:feature_flag], with: EE::API::Entities::FeatureFlag + else + render_api_error!(result[:message], result[:http_status]) + end + end + + desc 'Disable a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag + end + params do + requires :name, type: String + requires :environment_scope, type: String + requires :strategy, type: JSON + end + post ':id/feature_flags/disable', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + authorize_create_feature_flag! + + result = nil + + if feature_flag = user_project.operations_feature_flags.find_by_name(params[:name]) + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + + not_modified! unless scope + + remained_strategy = scope.strategies.reject { |str| str['name'] == params[:strategy]['name'] && str['parameters'] == params[:strategy]['parameters'] } + + not_modified! if scope.strategies == remained_strategy + + update_params = if remained_strategy.empty? + { + scopes_attributes:[{ + id: scope.id, + active: false + }] + } + else + { + scopes_attributes:[{ + id: scope.id, + strategies: remained_strategy + }] + } + end + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, update_params) + .execute(feature_flag) + else + not_modified! + end + + if result.nil? + render_api_error!('Bad request', 400) + elsif result[:status] == :success + present result[:feature_flag], with: EE::API::Entities::FeatureFlag + else + render_api_error!(result[:message], result[:http_status]) + end + end + desc 'Delete a feature flag' do detail 'This feature was introduced in GitLab 12.4.' success EE::API::Entities::FeatureFlag diff --git a/ee/spec/requests/api/feature_flags_spec.rb b/ee/spec/requests/api/feature_flags_spec.rb index 02332de59db554..d72745f994942b 100644 --- a/ee/spec/requests/api/feature_flags_spec.rb +++ b/ee/spec/requests/api/feature_flags_spec.rb @@ -27,7 +27,6 @@ it 'returns feature flags' do get api("/projects/#{project.id}/feature_flags", developer) - puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" expect(response).to have_gitlab_http_status(:ok) expect(json_response.count).to eq(2) expect(json_response.first['name']).to eq(feature_flag_1.name) @@ -45,9 +44,7 @@ it 'returns a feature flag entry' do get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", developer) - puts "#{self.class.name} - #{__callee__}: json_response: #{JSON.pretty_generate(json_response)}" expect(response).to have_gitlab_http_status(:ok) - expect(json_response['id']).to eq(feature_flag.id) expect(json_response['name']).to eq(feature_flag.name) expect(json_response['description']).to eq(feature_flag.description) end @@ -113,6 +110,136 @@ end end + describe 'POST /projects/:id/feature_flags/enable' do + let(:params) do + { + name: 'awesome-feature', + environment_scope: 'production', + strategy: { name: 'userWithId', parameters: { userIds: 'Project:1' } }.to_json + } + end + + context 'when feature flag & scope do not exist yet' do + it 'creates a new feature flag and scope' do + post api("/projects/#{project.id}/feature_flags/enable", developer), params: params + + expect(response).to have_gitlab_http_status(:created) + + feature_flag = project.operations_feature_flags.last + expect(feature_flag.name).to eq(params[:name]) + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(scope.strategies).to eq([JSON.parse(params[:strategy])]) + end + end + + context 'when feature flag exists already' do + let!(:feature_flag) { create_flag(project, params[:name]) } + + context 'when environment scope does not exist yet' do + it 'creates a new scope' do + post api("/projects/#{project.id}/feature_flags/enable", developer), params: params + + expect(response).to have_gitlab_http_status(:created) + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(scope.strategies).to eq([JSON.parse(params[:strategy])]) + end + end + + context 'when scope exists already' do + let(:defined_strategy) { { name: 'userWithId', parameters: { userIds: 'Project:2' }} } + + before do + create_scope(feature_flag, params[:environment_scope], true, [defined_strategy]) + end + + it 'adds an additional strategy param' do + post api("/projects/#{project.id}/feature_flags/enable", developer), params: params + + expect(response).to have_gitlab_http_status(:created) + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(scope.strategies).to eq([defined_strategy.deep_stringify_keys, JSON.parse(params[:strategy])]) + end + end + end + end + + describe 'POST /projects/:id/feature_flags/disable' do + let(:params) do + { + name: 'awesome-feature', + environment_scope: 'production', + strategy: { name: 'userWithId', parameters: { userIds: 'Project:1' } }.to_json + } + end + + context 'when feature flag & scope do not exist yet' do + it 'returns not modified' do + post api("/projects/#{project.id}/feature_flags/disable", developer), params: params + + expect(response).to have_gitlab_http_status(:not_modified) + end + end + + context 'when feature flag exists already' do + let!(:feature_flag) { create_flag(project, params[:name]) } + + context 'when environment scope does not exist yet' do + it 'returns not modified' do + post api("/projects/#{project.id}/feature_flags/disable", developer), params: params + + expect(response).to have_gitlab_http_status(:not_modified) + end + end + + context 'when scope exists already and can find the corresponding one' do + let(:defined_strategies) { [{ name: 'userWithId', parameters: { userIds: 'Project:1' }}, { name: 'userWithId', parameters: { userIds: 'Project:2' }}] } + + before do + create_scope(feature_flag, params[:environment_scope], true, defined_strategies) + end + + it 'removes the strategy from the scope' do + post api("/projects/#{project.id}/feature_flags/disable", developer), params: params + + expect(response).to have_gitlab_http_status(:created) + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(scope.strategies).to eq([{ name: 'userWithId', parameters: { userIds: 'Project:2' }}.deep_stringify_keys]) + end + + context 'when strategies become empty array afterward' do + let(:defined_strategies) { [{ name: 'userWithId', parameters: { userIds: 'Project:1' }}] } + + it 'deactivates the scope' do + post api("/projects/#{project.id}/feature_flags/disable", developer), params: params + + expect(response).to have_gitlab_http_status(:created) + + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + expect(scope.active).to eq(false) + end + end + end + + context 'when scope exists already but cannot find the corresponding one' do + let(:defined_strategy) { { name: 'userWithId', parameters: { userIds: 'Project:2' }} } + + before do + create_scope(feature_flag, params[:environment_scope], true, [defined_strategy]) + end + + it 'returns not modified' do + post api("/projects/#{project.id}/feature_flags/disable", developer), params: params + + expect(response).to have_gitlab_http_status(:not_modified) + end + end + end + end + describe 'DELETE /projects/:id/feature_flags/:name' do let!(:feature_flag) do create(:operations_feature_flag, project: project) diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb index 37d96ac02a136a..aeb46aaaa7ae59 100644 --- a/lib/feature_flag/adapters/unleash.rb +++ b/lib/feature_flag/adapters/unleash.rb @@ -19,36 +19,30 @@ def off?(thing = nil) end def enable(thing = true) - # TODO: How to control flags? - if persisted? - - else + puts "#{self.class.name} - #{__callee__}: 1" + response = HTTParty.post(Unleash.enable_feature_flag_url, + headers: Unleash.request_headers, + body: { name: @key, + environment_scope: Gitlab.config.unleash.app_name, + strategy: strategy_for(thing).to_json }) - end + puts "#{self.class.name} - #{__callee__}: 2" + Sidekiq.logger.debug("#{self.class.name} - #{__callee__}: response: #{response}") - strategies = if thing == true - { name: 'default', parameters: {} } - else - { name: 'userWithId', parameters: { userIds: sanitized(thing) } } - end + puts "#{self.class.name} - #{__callee__}: 3" + response + end - responce = HTTParty.post(Unleash.create_feature_flag_url, + def disable(thing = false) + response = HTTParty.post(Unleash.disable_feature_flag_url, headers: Unleash.request_headers, body: { name: @key, - scopes_attributes: [{ - environment_scope: Gitlab.config.unleash.app_name, - active: true, - strategies: strategies}]}.to_json) - - responce.map do |feature_flag| - feature = Feature.new(feature_flag[:name]) - feature.state = - feature - end - end - - def disable(thing = false) - # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + environment_scope: Gitlab.config.unleash.app_name, + strategy: strategy_for(thing).to_json }) + + Sidekiq.logger.debug("#{self.class.name} - #{__callee__}: response: #{response}") + + response end def enable_group(group) @@ -70,8 +64,12 @@ def persisted? private - def enable(thing = true) - + def strategy_for(thing) + if thing == true + { name: 'default', parameters: {} } + else + { name: 'userWithId', parameters: { userIds: sanitized(thing) } } + end end def client @@ -125,8 +123,12 @@ def configure end end - def create_feature_flag_url - "#{api_endpoint}/" + def enable_feature_flag_url + "#{api_endpoint}/enable" + end + + def disable_feature_flag_url + "#{api_endpoint}/disable" end def api_endpoint -- GitLab From fdced634d855523a2237a52ce7487811963ff2df Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 19 Sep 2019 19:18:51 +0700 Subject: [PATCH 08/11] Fix persisted? --- lib/feature_flag/adapters/unleash.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb index aeb46aaaa7ae59..b5612c5fcbe71e 100644 --- a/lib/feature_flag/adapters/unleash.rb +++ b/lib/feature_flag/adapters/unleash.rb @@ -59,7 +59,7 @@ def remove def persisted? toggles = ::Unleash.toggles - toggles.present? && toggles.select { |toggle| toggle['name'] == @key } + toggles.present? && toggles.any? { |toggle| toggle['name'] == @key } end private @@ -106,7 +106,6 @@ def get(key) end def persisted?(feature) - # get(key).persisted? feature.persisted? end -- GitLab From fa40cb84db28a4adf039aa038e6dcbec90561543 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 20 Sep 2019 10:38:43 +0700 Subject: [PATCH 09/11] Fix Feature.enable/disable --- ee/app/models/operations/feature_flag_scope.rb | 2 +- ee/lib/api/feature_flags.rb | 4 ++-- lib/feature_flag/adapters/unleash.rb | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ee/app/models/operations/feature_flag_scope.rb b/ee/app/models/operations/feature_flag_scope.rb index b8eb9e1950da69..08f938c9a73d1e 100644 --- a/ee/app/models/operations/feature_flag_scope.rb +++ b/ee/app/models/operations/feature_flag_scope.rb @@ -34,7 +34,7 @@ def userwithid_strategy def self.with_name_and_description joins(:feature_flag) - .select(FeatureFlag.arel_table[:name], FeatureFlag.arel_table[:description]) + .select(::Operations::FeatureFlag.arel_table[:name], ::Operations::FeatureFlag.arel_table[:description]) end def self.for_unleash_client(project, environment) diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb index 2d9744cdcb597b..9ac512d88b9782 100644 --- a/ee/lib/api/feature_flags.rb +++ b/ee/lib/api/feature_flags.rb @@ -120,7 +120,7 @@ class FeatureFlags < Grape::API scopes_attributes:[{ id: scope.id, active: true, - strategies: scope.strategies.push(params[:strategy]) + strategies: scope.strategies.push(params[:strategy]).uniq }] } end @@ -182,7 +182,7 @@ class FeatureFlags < Grape::API { scopes_attributes:[{ id: scope.id, - active: false + _destroy: 1 }] } else diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb index b5612c5fcbe71e..f58bdb9560c36a 100644 --- a/lib/feature_flag/adapters/unleash.rb +++ b/lib/feature_flag/adapters/unleash.rb @@ -65,7 +65,7 @@ def persisted? private def strategy_for(thing) - if thing == true + if thing.in?([true, false]) { name: 'default', parameters: {} } else { name: 'userWithId', parameters: { userIds: sanitized(thing) } } @@ -157,7 +157,8 @@ def client url: Gitlab.config.unleash.url, app_name: Gitlab.config.unleash.app_name, instance_id: Gitlab.config.unleash.instance_id, - logger: Gitlab::Unleash::Logger) + logger: Gitlab::Unleash::Logger, + disable_metrics: true) end end end -- GitLab From 275826feda7c0c92e0f73969195af4fc9447c4e1 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 20 Sep 2019 20:35:23 +0700 Subject: [PATCH 10/11] Support Feature.all --- ee/lib/api/feature_flag/scopes.rb | 197 +++++++++++++++------------ ee/lib/api/feature_flags.rb | 10 +- ee/lib/ee/api/entities.rb | 4 + lib/feature_flag/adapters/unleash.rb | 75 ++++++---- 4 files changed, 164 insertions(+), 122 deletions(-) diff --git a/ee/lib/api/feature_flag/scopes.rb b/ee/lib/api/feature_flag/scopes.rb index e0e2a4029f1326..e7f9108c4fc689 100644 --- a/ee/lib/api/feature_flag/scopes.rb +++ b/ee/lib/api/feature_flag/scopes.rb @@ -8,115 +8,130 @@ class Scopes < Grape::API FEATURE_FLAG_ENDPOINT_REQUIREMETS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS .merge(name: API::NO_SLASH_URL_PART_REGEX) - before { authorize_read_feature_flag! } + before { authorize_read_feature_flags! } params do requires :id, type: String, desc: 'The ID of a project' - requires :name, type: String, desc: 'The name of the feature flag' end - resource 'projects/:id/feature_flags/:name/scopes', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do - desc 'Get all scopes of a feature flag' do + resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc 'Get all effective scopes under the project' do detail 'This feature was introduced in GitLab 12.4.' - success EE::API::Entities::FeatureFlag::Scope + success EE::API::Entities::FeatureFlag::DetailedScope end params do - use :pagination + requires :environment_scope, type: String, desc: 'The environment scope' end - get do - present paginate(feature_flag.scopes), with: EE::API::Entities::FeatureFlag::Scope + get ':id/feature_flag_scopes' do + present scopes_for_environment, with: EE::API::Entities::FeatureFlag::DetailedScope end - desc 'Get a scope of a feature flag' do - detail 'This feature was introduced in GitLab 12.4.' - success EE::API::Entities::FeatureFlag::Scope - end params do - requires :scope_id, type: String - end - get ':scope_id' do - present scope, with: EE::API::Entities::FeatureFlag::Scope - end - - desc 'Create a new scope for a feature flag' do - detail 'This feature was introduced in GitLab 12.4.' - success EE::API::Entities::FeatureFlag::Scope + requires :name, type: String, desc: 'The name of the feature flag' end - params do - requires :environment_scope, type: String - requires :active, type: Boolean - requires :strategies, type: JSON - end - post do - authorize_update_feature_flag! - - param = { scopes_attributes: [declared_params(include_missing: false)] } - - result = ::FeatureFlags::UpdateService - .new(user_project, current_user, param) - .execute(feature_flag) - - if result[:status] == :success - present result[:feature_flag].scopes.last, with: EE::API::Entities::FeatureFlag::Scope - else - render_api_error!(result[:message], result[:http_status]) + resource ':id/feature_flags/:name/scopes', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + desc 'Get all scopes of a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope end - end - - desc 'Update a scope of a feature flag' do - detail 'This feature was introduced in GitLab 12.4.' - success EE::API::Entities::FeatureFlag::Scope - end - params do - requires :scope_id, type: String - optional :environment_scope, type: String - optional :active, type: Boolean - optional :strategies, type: JSON - end - put ':scope_id' do - authorize_update_feature_flag! - - param = declared_params(include_missing: false) - param[:id] = param.delete(:scope_id) - param = { scopes_attributes: [param] } - - result = ::FeatureFlags::UpdateService - .new(user_project, current_user, param) - .execute(feature_flag) - - if result[:status] == :success - present scope.reload, with: EE::API::Entities::FeatureFlag::Scope - else - render_api_error!(result[:message], result[:http_status]) + params do + use :pagination end - end - - desc 'Delete a scope from a feature flag' do - detail 'This feature was introduced in GitLab 12.4.' - success EE::API::Entities::FeatureFlag::Scope - end - params do - optional :scope_id, type: String, desc: 'The scope' - end - delete ':scope_id' do - authorize_update_feature_flag! - - param = { scopes_attributes: [{ id: scope.id, _destroy: 1 }] } - - result = ::FeatureFlags::UpdateService - .new(user_project, current_user, param) - .execute(feature_flag) - - if result[:status] == :success + get do + present paginate(feature_flag.scopes), with: EE::API::Entities::FeatureFlag::Scope + end + + desc 'Get a scope of a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + requires :scope_id, type: String + end + get ':scope_id' do present scope, with: EE::API::Entities::FeatureFlag::Scope - else - render_api_error!(result[:message], result[:http_status]) + end + + desc 'Create a new scope for a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + requires :environment_scope, type: String + requires :active, type: Boolean + requires :strategies, type: JSON + end + post do + authorize_update_feature_flag! + + param = { scopes_attributes: [declared_params(include_missing: false)] } + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, param) + .execute(feature_flag) + + if result[:status] == :success + present result[:feature_flag].scopes.last, with: EE::API::Entities::FeatureFlag::Scope + else + render_api_error!(result[:message], result[:http_status]) + end + end + + desc 'Update a scope of a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + requires :scope_id, type: String + optional :environment_scope, type: String + optional :active, type: Boolean + optional :strategies, type: JSON + end + put ':scope_id' do + authorize_update_feature_flag! + + param = declared_params(include_missing: false) + param[:id] = param.delete(:scope_id) + param = { scopes_attributes: [param] } + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, param) + .execute(feature_flag) + + if result[:status] == :success + present scope.reload, with: EE::API::Entities::FeatureFlag::Scope + else + render_api_error!(result[:message], result[:http_status]) + end + end + + desc 'Delete a scope from a feature flag' do + detail 'This feature was introduced in GitLab 12.4.' + success EE::API::Entities::FeatureFlag::Scope + end + params do + optional :scope_id, type: String, desc: 'The scope' + end + delete ':scope_id' do + authorize_update_feature_flag! + + param = { scopes_attributes: [{ id: scope.id, _destroy: 1 }] } + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, param) + .execute(feature_flag) + + if result[:status] == :success + present scope, with: EE::API::Entities::FeatureFlag::Scope + else + render_api_error!(result[:message], result[:http_status]) + end end end end helpers do - def authorize_read_feature_flag! - authorize! :read_feature_flag, feature_flag + def authorize_read_feature_flags! + authorize! :read_feature_flag, user_project end def authorize_update_feature_flag! @@ -130,6 +145,10 @@ def feature_flag def scope @scope ||= feature_flag.scopes.find_by_id!(params[:scope_id]) end + + def scopes_for_environment + Operations::FeatureFlagScope.for_unleash_client(user_project, params[:environment_scope]) + end end end end diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb index 9ac512d88b9782..d3a451814d5736 100644 --- a/ee/lib/api/feature_flags.rb +++ b/ee/lib/api/feature_flags.rb @@ -23,7 +23,7 @@ class FeatureFlags < Grape::API end get ':id/feature_flags' do feature_flags = ::FeatureFlagsFinder - .new(user_project, current_user, scope: params[:scope]) + .new(user_project, current_user, params) .execute(preload: true) present paginate(feature_flags), with: EE::API::Entities::FeatureFlag @@ -90,7 +90,7 @@ class FeatureFlags < Grape::API end end - desc 'Enable a feature flag' do + desc 'Enable a feature flag for an environment with strategy' do detail 'This feature was introduced in GitLab 12.4.' success EE::API::Entities::FeatureFlag end @@ -99,7 +99,7 @@ class FeatureFlags < Grape::API requires :environment_scope, type: String requires :strategy, type: JSON end - post ':id/feature_flags/enable', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + post ':id/feature_flags/:name/enable', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do authorize_create_feature_flag! result = nil @@ -155,7 +155,7 @@ class FeatureFlags < Grape::API end end - desc 'Disable a feature flag' do + desc 'Disable a feature flag for an environment with strategy' do detail 'This feature was introduced in GitLab 12.4.' success EE::API::Entities::FeatureFlag end @@ -164,7 +164,7 @@ class FeatureFlags < Grape::API requires :environment_scope, type: String requires :strategy, type: JSON end - post ':id/feature_flags/disable', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do + post ':id/feature_flags/:name/disable', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do authorize_create_feature_flag! result = nil diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index e996ad91d31ffb..8b2ec554d9c8f4 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -800,6 +800,10 @@ class Scope < Grape::Entity expose :updated_at end + class DetailedScope < Scope + expose :name + end + expose :name expose :description expose :created_at diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb index f58bdb9560c36a..f01d78b1585bcb 100644 --- a/lib/feature_flag/adapters/unleash.rb +++ b/lib/feature_flag/adapters/unleash.rb @@ -4,14 +4,20 @@ module FeatureFlag module Adapters class Unleash class Feature - attr_reader :key + attr_accessor :active + attr_accessor :strategies + attr_reader :name - def initialize(key) - @key = key.to_s + alias_attribute :state, :active + + Gate = Struct.new(:key, :value) + + def initialize(name) + @name = name.to_s end def enabled?(thing = nil) - client.is_enabled?(key, context(thing)) + client.is_enabled?(name, context(thing)) end def off?(thing = nil) @@ -19,30 +25,19 @@ def off?(thing = nil) end def enable(thing = true) - puts "#{self.class.name} - #{__callee__}: 1" - response = HTTParty.post(Unleash.enable_feature_flag_url, + HTTParty.post(Unleash.enable_feature_flag_url(name), headers: Unleash.request_headers, - body: { name: @key, + body: { name: name, environment_scope: Gitlab.config.unleash.app_name, strategy: strategy_for(thing).to_json }) - - puts "#{self.class.name} - #{__callee__}: 2" - Sidekiq.logger.debug("#{self.class.name} - #{__callee__}: response: #{response}") - - puts "#{self.class.name} - #{__callee__}: 3" - response end def disable(thing = false) - response = HTTParty.post(Unleash.disable_feature_flag_url, + HTTParty.post(Unleash.disable_feature_flag_url(name), headers: Unleash.request_headers, - body: { name: @key, + body: { name: name, environment_scope: Gitlab.config.unleash.app_name, strategy: strategy_for(thing).to_json }) - - Sidekiq.logger.debug("#{self.class.name} - #{__callee__}: response: #{response}") - - response end def enable_group(group) @@ -59,7 +54,18 @@ def remove def persisted? toggles = ::Unleash.toggles - toggles.present? && toggles.any? { |toggle| toggle['name'] == @key } + toggles.present? && toggles.any? { |toggle| toggle['name'] == name } + end + + def gates + @gates ||= strategies.map { |strategy| Gate.new(strategy['name'], strategy['parameters']) } + end + + def gate_values + @gate_values ||= strategies.inject({}) do |hash, strategy| + hash[strategy['name']] = strategy['parameters'].to_s + hash + end end private @@ -97,8 +103,17 @@ def available? end def all - # TODO: Wrap in Feature - Unleash.toggles + response = HTTParty.get(get_feature_flag_scopes_url, + headers: request_headers, + query: { environment_scope: Gitlab.config.unleash.app_name } + ) + + response.map do |scope| + feature = Feature.new(scope['name']) + feature.active = scope['active'] + feature.strategies = scope['strategies'] + feature + end end def get(key) @@ -122,21 +137,25 @@ def configure end end - def enable_feature_flag_url - "#{api_endpoint}/enable" + def get_feature_flag_scopes_url + "#{api_endpoint}/feature_flag_scopes" + end + + def enable_feature_flag_url(key) + "#{api_endpoint}/feature_flags/#{key}/enable" end - def disable_feature_flag_url - "#{api_endpoint}/disable" + def disable_feature_flag_url(key) + "#{api_endpoint}/feature_flags/#{key}/disable" end def api_endpoint strong_memoize(:api_endpoint) do api_url, project_id = Gitlab.config.unleash.url - .scan( %r{(https?://.*/api/v4/)feature_flags/unleash/(\d+)} ) + .scan( %r{(https?://.*/api/v4)/feature_flags/unleash/(\d+)} ) .first - "#{api_url}/projects/#{project_id}/feature_flags" + "#{api_url}/projects/#{project_id}/" end end -- GitLab From bcb5990e7ce28f766eb77061eebdcbabbe9d0136 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 23 Sep 2019 15:35:42 +0700 Subject: [PATCH 11/11] Support Feature.remove --- ee/lib/api/feature_flags.rb | 78 +++++++++++++++++----------- lib/feature_flag/adapters/unleash.rb | 9 +++- 2 files changed, 56 insertions(+), 31 deletions(-) diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb index d3a451814d5736..d30198751068a4 100644 --- a/ee/lib/api/feature_flags.rb +++ b/ee/lib/api/feature_flags.rb @@ -169,38 +169,34 @@ class FeatureFlags < Grape::API result = nil - if feature_flag = user_project.operations_feature_flags.find_by_name(params[:name]) - scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) - - not_modified! unless scope + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) - remained_strategy = scope.strategies.reject { |str| str['name'] == params[:strategy]['name'] && str['parameters'] == params[:strategy]['parameters'] } + not_found! unless scope - not_modified! if scope.strategies == remained_strategy + remained_strategy = scope.strategies.reject { |str| str['name'] == params[:strategy]['name'] && str['parameters'] == params[:strategy]['parameters'] } - update_params = if remained_strategy.empty? - { - scopes_attributes:[{ - id: scope.id, - _destroy: 1 - }] - } - else - { - scopes_attributes:[{ - id: scope.id, - strategies: remained_strategy - }] - } - end + not_modified! if scope.strategies == remained_strategy - result = ::FeatureFlags::UpdateService - .new(user_project, current_user, update_params) - .execute(feature_flag) + update_params = if remained_strategy.empty? + { + scopes_attributes:[{ + id: scope.id, + _destroy: 1 + }] + } else - not_modified! + { + scopes_attributes:[{ + id: scope.id, + strategies: remained_strategy + }] + } end + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, update_params) + .execute(feature_flag) + if result.nil? render_api_error!('Bad request', 400) elsif result[:status] == :success @@ -215,16 +211,38 @@ class FeatureFlags < Grape::API success EE::API::Entities::FeatureFlag end params do - optional :name, type: String, desc: 'The name of the feature flag' + requires :name, type: String, desc: 'The name of the feature flag' + optional :environment_scope, type: String, desc: 'The environment scope' end delete ':id/feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMETS do authorize_destroy_feature_flag! - result = ::FeatureFlags::DestroyService - .new(user_project, current_user, declared_params(include_missing: false)) - .execute(feature_flag) + result = nil - if result[:status] == :success + if params[:environment_scope] + scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope]) + + not_found! unless scope + + update_params = { + scopes_attributes:[{ + id: scope.id, + _destroy: 1 + }] + } + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, update_params) + .execute(feature_flag) + else + result = ::FeatureFlags::DestroyService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute(feature_flag) + end + + if result.nil? + render_api_error!('Bad request', 400) + elsif result[:status] == :success present result[:feature_flag], with: EE::API::Entities::FeatureFlag else render_api_error!(result[:message], result[:http_status]) diff --git a/lib/feature_flag/adapters/unleash.rb b/lib/feature_flag/adapters/unleash.rb index f01d78b1585bcb..2d31fc3653658f 100644 --- a/lib/feature_flag/adapters/unleash.rb +++ b/lib/feature_flag/adapters/unleash.rb @@ -49,7 +49,10 @@ def disable_group(group) end def remove - # Not Supported yet (See https://gitlab.com/gitlab-org/gitlab-ee/issues/9566) + HTTParty.post(Unleash.delete_feature_flag_url(name), + headers: Unleash.request_headers, + body: { name: name, + environment_scope: Gitlab.config.unleash.app_name }) end def persisted? @@ -149,6 +152,10 @@ def disable_feature_flag_url(key) "#{api_endpoint}/feature_flags/#{key}/disable" end + def delete_feature_flag_url(key) + "#{api_endpoint}/feature_flags/#{key}" + end + def api_endpoint strong_memoize(:api_endpoint) do api_url, project_id = Gitlab.config.unleash.url -- GitLab