From 7e3af9491c4a723091ff24a9b058b7360b457466 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 7 Oct 2019 19:29:42 +0700 Subject: [PATCH] Add public API support for Feature Flag scope This commit adds public API support for FF scope --- ee/lib/api/feature_flag_scopes.rb | 109 ++++++++ ee/lib/api/feature_flags.rb | 6 + ee/lib/ee/api/api.rb | 1 + ee/lib/ee/api/entities.rb | 4 + .../v4/feature_flag_detailed_scopes.json | 22 ++ .../public_api/v4/feature_flag_scopes.json | 9 + .../requests/api/feature_flag_scopes_spec.rb | 249 ++++++++++++++++++ 7 files changed, 400 insertions(+) create mode 100644 ee/lib/api/feature_flag_scopes.rb create mode 100644 ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_detailed_scopes.json create mode 100644 ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_scopes.json 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..e367a77a5dd9b0 --- /dev/null +++ b/ee/lib/api/feature_flag_scopes.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +module API + class FeatureFlagScopes < Grape::API + include PaginationParams + + ENVIRONMENT_SCOPE_ENDPOINT_REQUIREMETS = FeatureFlags::FEATURE_FLAG_ENDPOINT_REQUIREMENTS + .merge(environment_scope: API::NO_SLASH_URL_PART_REGEX) + + before do + not_found! unless Feature.enabled?(:feature_flag_api, user_project) + authorize_read_feature_flags! + end + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource 'projects/:id', requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + resource :feature_flag_scopes do + desc 'Get all effective feature flags under the environment' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' + success EE::API::Entities::FeatureFlag::DetailedScope + end + params do + requires :environment, type: String, desc: 'The environment name' + end + get do + present scopes_for_environment, with: EE::API::Entities::FeatureFlag::DetailedScope + end + end + + params do + requires :name, type: String, desc: 'The name of the feature flag' + end + resource 'feature_flags/:name', requirements: FeatureFlags::FEATURE_FLAG_ENDPOINT_REQUIREMENTS do + resource :scopes do + desc 'Get all scopes of a feature flag' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' + 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 + + params do + requires :environment_scope, type: String, desc: 'URL-encoded environment scope' + end + resource ':environment_scope', requirements: ENVIRONMENT_SCOPE_ENDPOINT_REQUIREMETS do + desc 'Get a scope of a feature flag' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' + success EE::API::Entities::FeatureFlag::Scope + end + get do + present scope, with: EE::API::Entities::FeatureFlag::Scope + end + + desc 'Delete a scope from a feature flag' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' + success EE::API::Entities::FeatureFlag::Scope + end + delete do + authorize_update_feature_flag! + + param = { scopes_attributes: [{ id: scope.id, _destroy: true }] } + + result = ::FeatureFlags::UpdateService + .new(user_project, current_user, param) + .execute(feature_flag) + + if result[:status] == :success + status :no_content + else + render_api_error!(result[:message], result[:http_status]) + end + end + end + end + end + end + + helpers do + def authorize_read_feature_flags! + authorize! :read_feature_flag, user_project + 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_environment_scope!(CGI.unescape(params[:environment_scope])) + end + + def scopes_for_environment + Operations::FeatureFlagScope + .for_unleash_client(user_project, params[:environment]) + end + end + end +end diff --git a/ee/lib/api/feature_flags.rb b/ee/lib/api/feature_flags.rb index 685eaf2fcbc374..1ea4f021125646 100644 --- a/ee/lib/api/feature_flags.rb +++ b/ee/lib/api/feature_flags.rb @@ -18,6 +18,7 @@ class FeatureFlags < Grape::API resource 'projects/:id', requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do resource :feature_flags do desc 'Get all feature flags of a project' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' success EE::API::Entities::FeatureFlag end params do @@ -34,6 +35,7 @@ class FeatureFlags < Grape::API end desc 'Create a new feature flag' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' success EE::API::Entities::FeatureFlag end params do @@ -68,6 +70,7 @@ class FeatureFlags < Grape::API end resource 'feature_flags/:name', requirements: FEATURE_FLAG_ENDPOINT_REQUIREMENTS do desc 'Get a feature flag of a project' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' success EE::API::Entities::FeatureFlag end get do @@ -77,6 +80,7 @@ class FeatureFlags < Grape::API end desc 'Enable a strategy for a feature flag on an environment' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' success EE::API::Entities::FeatureFlag end params do @@ -96,6 +100,7 @@ class FeatureFlags < Grape::API end desc 'Disable a strategy for a feature flag on an environment' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' success EE::API::Entities::FeatureFlag end params do @@ -115,6 +120,7 @@ class FeatureFlags < Grape::API end desc 'Delete a feature flag' do + detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed' success EE::API::Entities::FeatureFlag end delete do diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index 2c8818eb325697..be414c9d5ec176 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -19,6 +19,7 @@ module API mount ::API::EpicLinks mount ::API::Epics mount ::API::FeatureFlags + mount ::API::FeatureFlagScopes 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 f55cc150bc2cc4..632c960a301e72 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -863,6 +863,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/ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_detailed_scopes.json b/ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_detailed_scopes.json new file mode 100644 index 00000000000000..a11ae5705cc223 --- /dev/null +++ b/ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_detailed_scopes.json @@ -0,0 +1,22 @@ +{ + "type": "array", + "items": { + "type": "object", + "required" : [ + "name", + "id", + "environment_scope", + "active" + ], + "properties" : { + "name": { "type": "string" }, + "id": { "type": "integer" }, + "environment_scope": { "type": "string" }, + "active": { "type": "boolean" }, + "created_at": { "type": "date" }, + "updated_at": { "type": "date" }, + "strategies": { "type": "array", "items": { "$ref": "feature_flag_strategy.json" } } + }, + "additionalProperties": false + } +} diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_scopes.json b/ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_scopes.json new file mode 100644 index 00000000000000..b1a7021db8bf47 --- /dev/null +++ b/ee/spec/fixtures/api/schemas/public_api/v4/feature_flag_scopes.json @@ -0,0 +1,9 @@ +{ + "type": "array", + "items": { + "type": "object", + "properties": { + "$ref": "./feature_flag_scope.json" + } + } +} 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..2272c6db77a767 --- /dev/null +++ b/ee/spec/requests/api/feature_flag_scopes_spec.rb @@ -0,0 +1,249 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe API::FeatureFlagScopes do + include FeatureFlagHelpers + + let(:project) { create(:project, :repository) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:user) { developer } + + before do + stub_licensed_features(feature_flags: true) + + project.add_developer(developer) + project.add_reporter(reporter) + end + + shared_examples_for 'check user permission' do + context 'when user is reporter' do + let(:user) { reporter } + + it 'forbids the request' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when license is not sufficient' do + before do + stub_licensed_features(feature_flags: false) + end + + it 'forbids the request' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when feature_flag_api feature flag is disabled' do + before do + stub_feature_flags(feature_flag_api: false) + end + + it 'forbids the request' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + shared_examples_for 'not found' do + it 'returns Not Found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'GET /projects/:id/feature_flag_scopes' do + subject do + get api("/projects/#{project.id}/feature_flag_scopes", user), + params: params + end + + let(:feature_flag_1) { create_flag(project, 'flag_1', false) } + let(:feature_flag_2) { create_flag(project, 'flag_2', false) } + + before do + create_scope(feature_flag_1, 'staging', true) + create_scope(feature_flag_1, 'production', false) + create_scope(feature_flag_2, 'review/*', true) + end + + context 'when environment is production' do + let(:params) { { environment: 'production' } } + + it_behaves_like 'check user permission' + + it 'returns all effective feature flags under the environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag_detailed_scopes', dir: 'ee') + expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => false }) + expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => false }) + end + end + + context 'when environment is staging' do + let(:params) { { environment: 'staging' } } + + it 'returns all effective feature flags under the environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => true }) + expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => false }) + end + end + + context 'when environment is review/feature X' do + let(:params) { { environment: 'review/feature X' } } + + it 'returns all effective feature flags under the environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.second).to include({ 'name' => 'flag_1', 'active' => false }) + expect(json_response.first).to include({ 'name' => 'flag_2', 'active' => true }) + end + end + end + + describe 'GET /projects/:id/feature_flags/:name/scopes' do + subject do + get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes", user) + end + + context 'when there are two scopes' do + let(:feature_flag) { create_flag(project, 'test') } + let!(:additional_scope) { create_scope(feature_flag, 'production', false) } + + it_behaves_like 'check user permission' + + it 'returns scopes of the feature flag' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag_scopes', dir: 'ee') + 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 + + context 'when there are no feature flags' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + + it_behaves_like 'not found' + end + end + + describe 'GET /projects/:id/feature_flags/:name/scopes/:environment_scope' do + subject do + get api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{environment_scope}", + user) + end + + let(:environment_scope) { scope.environment_scope } + + shared_examples_for 'successful response' do + it 'returns a scope' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/feature_flag_scope', dir: 'ee') + expect(json_response['id']).to eq(scope.id) + expect(json_response['active']).to eq(scope.active) + expect(json_response['environment_scope']).to eq(scope.environment_scope) + end + end + + context 'when there is a feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + let(:scope) { feature_flag.default_scope } + + it_behaves_like 'check user permission' + it_behaves_like 'successful response' + + context 'when environment scope includes slash' do + let!(:scope) { create_scope(feature_flag, 'review/*', false) } + + it_behaves_like 'not found' + + context 'when URL-encoding the environment scope parameter' do + let(:environment_scope) { CGI.escape(scope.environment_scope) } + + it_behaves_like 'successful response' + end + end + end + + context 'when there are no feature flags' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + let(:scope) { double(:feature_flag_scope, environment_scope: 'prd') } + + it_behaves_like 'not found' + end + end + + describe 'DELETE /projects/:id/feature_flags/:name/scopes/:environment_scope' do + subject do + delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}/scopes/#{environment_scope}", + user) + end + + let(:environment_scope) { scope.environment_scope } + + shared_examples_for 'successful response' do + it 'destroys the scope' do + expect { subject } + .to change { Operations::FeatureFlagScope.exists?(environment_scope: scope.environment_scope) } + .from(true).to(false) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when there is a feature flag' do + let!(:feature_flag) { create(:operations_feature_flag, project: project) } + + context 'when there is a targeted scope' do + let!(:scope) { create_scope(feature_flag, 'production', false) } + + it_behaves_like 'check user permission' + it_behaves_like 'successful response' + + context 'when environment scope includes slash' do + let!(:scope) { create_scope(feature_flag, 'review/*', false) } + + it_behaves_like 'not found' + + context 'when URL-encoding the environment scope parameter' do + let(:environment_scope) { CGI.escape(scope.environment_scope) } + + it_behaves_like 'successful response' + end + end + end + + context 'when there are no targeted scopes' do + let!(:scope) { double(:feature_flag_scope, environment_scope: 'production') } + + it_behaves_like 'not found' + end + end + + context 'when there are no feature flags' do + let(:feature_flag) { double(:feature_flag, name: 'test') } + let(:scope) { double(:feature_flag_scope, environment_scope: 'prd') } + + it_behaves_like 'not found' + end + end +end -- GitLab