Allow setting feature categories for API endpoints

This works the same way as for controllers, but we use an API route
rather than an action (method) name.

A feature category can also be defined on the action itself.

app/controllers/application_controller.rb

@@ -22,7 +22,7 @@ class ApplicationController < ActionController::Base
   include Impersonation
   include Gitlab::Logging::CloudflareHelper
   include Gitlab::Utils::StrongMemoize
-  include ControllerWithFeatureCategory
+  include ::Gitlab::WithFeatureCategory
 
   before_action :authenticate_user!, except: [:route_not_found]
   before_action :enforce_terms!, if: :should_enforce_terms?

app/controllers/concerns/controller_with_feature_category.rb

-# frozen_string_literal: true
-
-module ControllerWithFeatureCategory
-  extend ActiveSupport::Concern
-  include Gitlab::ClassAttributes
-
-  class_methods do
-    def feature_category(category, actions = [])
-      feature_category_configuration[category] ||= []
-      feature_category_configuration[category] += actions.map(&:to_s)
-
-      validate_config!(feature_category_configuration)
-    end
-
-    def feature_category_for_action(action)
-      category_config = feature_category_configuration.find do |_, actions|
-        actions.empty? || actions.include?(action)
-      end
-
-      category_config&.first || superclass_feature_category_for_action(action)
-    end
-
-    private
-
-    def validate_config!(config)
-      empty = config.find { |_, actions| actions.empty? }
-      duplicate_actions = config.values.flatten.group_by(&:itself).select { |_, v| v.count > 1 }.keys
-
-      if config.length > 1 && empty
-        raise ArgumentError, "#{empty.first} is defined for all actions, but other categories are set"
-      end
-
-      if duplicate_actions.any?
-        raise ArgumentError, "Actions have multiple feature categories: #{duplicate_actions.join(', ')}"
-      end
-    end
-
-    def feature_category_configuration
-      class_attributes[:feature_category_config] ||= {}
-    end
-
-    def superclass_feature_category_for_action(action)
-      return unless superclass.respond_to?(:feature_category_for_action)
-
-      superclass.feature_category_for_action(action)
-    end
-  end
-end

lib/api/base.rb

@@ -2,5 +2,26 @@
 
 module API
   class Base < Grape::API::Instance # rubocop:disable API/Base
+    include ::Gitlab::WithFeatureCategory
+
+    def self.feature_category_for_app(app)
+      feature_category_for_action(path_for_app(app))
+    end
+
+    def self.path_for_app(app)
+      normalize_path(app.namespace, app.options[:path].first)
+    end
+
+    def self.route(methods, paths = ['/'], route_options = {}, &block)
+      if category = route_options.delete(:feature_category)
+        feature_category(category, Array(paths).map { |path| normalize_path(namespace, path) })
+      end
+
+      super
+    end
+
+    def self.normalize_path(namespace, path)
+      [namespace.presence, path.to_s.chomp('/').presence].compact.join('/')
+    end
   end
 end

lib/api/internal/base.rb

@@ -7,10 +7,14 @@ module API
       before { authenticate_by_gitlab_shell_token! }
 
       before do
+        route_path = route.origin
+        route_class = route.app.options[:for]
+
         Gitlab::ApplicationContext.push(
           user: -> { actor&.user },
           project: -> { project },
-          caller_id: route.origin
+          caller_id: route_path,
+          feature_category: route_class.try(:feature_category_for_action, route_path).to_s
         )
       end
 

lib/api/issues.rb

@@ -6,6 +6,8 @@ module API
     helpers Helpers::IssuesHelpers
     helpers Helpers::RateLimiter
 
+    feature_category :issue_tracking
+
     before { authenticate_non_get! }
 
     helpers do

lib/api/users.rb

@@ -8,6 +8,40 @@ module API
 
     allow_access_with_scope :read_user, if: -> (request) { request.get? }
 
+    feature_category :users, [
+                       '/users/:id/custom_attributes',
+                       '/users/:id/custom_attributes/:key',
+                       '/users/:id/emails',
+                       '/users/:id/emails/:email_id',
+                       '/users/:user_id/memberships',
+                       '/user',
+                       '/user/emails',
+                       '/user/emails/:email_id',
+                       '/user/activities',
+                       '/user/status'
+                     ]
+
+    feature_category :authentication_and_authorization, [
+                       '/users/:id/identities/:provider',
+                       '/users/:id/keys',
+                       '/users/:user_id/keys',
+                       '/users/:id/keys/:key_id',
+                       '/users/:id/gpg_keys',
+                       '/users/:id/gpg_keys/:key_id',
+                       '/users/:id/gpg_keys/:key_id/revoke',
+                       '/users/:id/activate',
+                       '/users/:id/deactivate',
+                       '/users/:id/block',
+                       '/users/:id/unblock',
+                       '/users/:user_id/impersonation_tokens',
+                       '/users/:user_id/impersonation_tokens/:impersonation_token_id',
+                       '/user/keys',
+                       '/user/keys/:key_id',
+                       '/user/gpg_keys',
+                       '/user/gpg_keys/:key_id',
+                       '/user/gpg_keys/:key_id/revoke'
+                     ]
+
     resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do
       include CustomAttributesEndpoints
 
@@ -93,7 +127,7 @@ module API
         use :optional_index_params_ee
       end
       # rubocop: disable CodeReuse/ActiveRecord
-      get do
+      get feature_category: :users do
         authenticated_as_admin! if params[:external].present? || (params[:extern_uid].present? && params[:provider].present?)
 
         unless current_user&.admin?
@@ -134,7 +168,7 @@ module API
         use :with_custom_attributes
       end
       # rubocop: disable CodeReuse/ActiveRecord
-      get ":id" do
+      get ":id", feature_category: :users do
         user = User.find_by(id: params[:id])
         not_found!('User') unless user && can?(current_user, :read_user, user)
 
@@ -149,7 +183,7 @@ module API
       params do
         requires :user_id, type: String, desc: 'The ID or username of the user'
       end
-      get ":user_id/status", requirements: API::USER_REQUIREMENTS do
+      get ":user_id/status", requirements: API::USER_REQUIREMENTS, feature_category: :users do
         user = find_user(params[:user_id])
         not_found!('User') unless user && can?(current_user, :read_user, user)
 

lib/gitlab/with_feature_category.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module WithFeatureCategory
+    extend ActiveSupport::Concern
+    include Gitlab::ClassAttributes
+
+    class_methods do
+      def feature_category(category, actions = [])
+        feature_category_configuration[category] ||= []
+        feature_category_configuration[category] += actions.map(&:to_s)
+
+        validate_config!(feature_category_configuration)
+      end
+
+      def feature_category_for_action(action)
+        category_config = feature_category_configuration.find do |_, actions|
+          actions.empty? || actions.include?(action)
+        end
+
+        category_config&.first || superclass_feature_category_for_action(action)
+      end
+
+      private
+
+      def validate_config!(config)
+        empty = config.find { |_, actions| actions.empty? }
+        duplicate_actions = config.values.flatten.group_by(&:itself).select { |_, v| v.count > 1 }.keys
+
+        if config.length > 1 && empty
+          raise ArgumentError, "#{empty.first} is defined for all actions, but other categories are set"
+        end
+
+        if duplicate_actions.any?
+          raise ArgumentError, "Actions have multiple feature categories: #{duplicate_actions.join(', ')}"
+        end
+      end
+
+      def feature_category_configuration
+        class_attributes[:feature_category_config] ||= {}
+      end
+
+      def superclass_feature_category_for_action(action)
+        return unless superclass.respond_to?(:feature_category_for_action)
+
+        superclass.feature_category_for_action(action)
+      end
+    end
+  end
+end

spec/controllers/every_controller_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe "Every controller" do
         .compact
         .select { |route| route[:controller].present? && route[:action].present? }
         .map { |route| [constantize_controller(route[:controller]), route[:action]] }
-        .select { |(controller, action)| controller&.include?(ControllerWithFeatureCategory) }
+        .select { |(controller, action)| controller&.include?(::Gitlab::WithFeatureCategory) }
         .reject { |(controller, action)| controller == ApplicationController || controller == Devise::UnlocksController }
     end
 

spec/lib/api/every_api_endpoint_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Every API endpoint' do
+  context 'feature categories' do
+    let_it_be(:feature_categories) do
+      YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:to_sym).to_set
+    end
+
+    let_it_be(:api_endpoints) do
+      API::API.routes.map do |route|
+        [route.app.options[:for], API::Base.path_for_app(route.app)]
+      end
+    end
+
+    let_it_be(:routes_without_category) do
+      api_endpoints.map do |(klass, path)|
+        next if klass.try(:feature_category_for_action, path)
+        # We'll add the rest in https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/463
+        next unless klass == ::API::Users || klass == ::API::Issues
+
+        "#{klass}##{path}"
+      end.compact.uniq
+    end
+
+    it "has feature categories" do
+      expect(routes_without_category).to be_empty, "#{routes_without_category} did not have a category"
+    end
+
+    it "recognizes the feature categories" do
+      routes_unknown_category = api_endpoints.map do |(klass, path)|
+        used_category = klass.try(:feature_category_for_action, path)
+        next unless used_category
+        next if used_category == :not_owned
+
+        [path, used_category] unless feature_categories.include?(used_category)
+      end.compact
+
+      expect(routes_unknown_category).to be_empty, "#{routes_unknown_category.first(10)} had an unknown category"
+    end
+
+    # This is required for API::Base.path_for_app to work, as it picks
+    # the first path
+    it "has no routes with multiple paths" do
+      routes_with_multiple_paths = API::API.routes.select { |route| route.app.options[:path].length != 1 }
+      failure_routes = routes_with_multiple_paths.map { |route| "#{route.app.options[:for]}:[#{route.app.options[:path].join(', ')}]" }
+
+      expect(routes_with_multiple_paths).to be_empty, "#{failure_routes} have multiple paths"
+    end
+
+    it "doesn't define or exclude categories on removed actions", :aggregate_failures do
+      api_endpoints.group_by(&:first).each do |klass, paths|
+        existing_paths = paths.map(&:last)
+        used_paths = paths_defined_in_feature_category_config(klass)
+        non_existing_used_paths = used_paths - existing_paths
+
+        expect(non_existing_used_paths).to be_empty,
+                                           "#{klass} used #{non_existing_used_paths} to define feature category, but the route does not exist"
+      end
+    end
+  end
+
+  def paths_defined_in_feature_category_config(klass)
+    (klass.try(:class_attributes) || {}).fetch(:feature_category_config, {})
+      .values
+      .flatten
+      .map(&:to_s)
+  end
+end

spec/controllers/concerns/controller_with_feature_category_spec.rb → spec/lib/gitlab/with_feature_category_spec.rb

 # frozen_string_literal: true
 
 require 'fast_spec_helper'
-require_relative "../../../app/controllers/concerns/controller_with_feature_category"
+require_relative "../../../lib/gitlab/with_feature_category"
 
-RSpec.describe ControllerWithFeatureCategory do
+RSpec.describe Gitlab::WithFeatureCategory do
   describe ".feature_category_for_action" do
     let(:base_controller) do
       Class.new do
-        include ControllerWithFeatureCategory
+        include ::Gitlab::WithFeatureCategory
       end
     end