Update Audit Logging for Feature Flags

Audit logging wasn't updated to audit log changes to feature flag
strategies, but needs to be now that legacy feature flags are in the
process of being removed.

Changelog: fixed

app/services/feature_flags/base_service.rb

@@ -48,10 +48,12 @@ module FeatureFlags
       end
     end
 
-    def created_scope_message(scope)
-      "Created rule #{scope.environment_scope} "\
-      "and set it as #{scope.active ? "active" : "inactive"} "\
-      "with strategies #{scope.strategies}."
+    def created_strategy_message(strategy)
+      scopes = strategy.scopes
+                 .map { |scope| %Q("#{scope.environment_scope}") }
+                 .join(', ')
+      %Q(Created strategy \"#{strategy.name}\" )\
+      "with scopes #{scopes}."
     end
 
     def feature_flag_by_name

app/services/feature_flags/create_service.rb

@@ -24,8 +24,8 @@ module FeatureFlags
     def audit_message(feature_flag)
       message_parts = ["Created feature flag #{feature_flag.name} with description \"#{feature_flag.description}\"."]
 
-      message_parts += feature_flag.scopes.map do |scope|
-        created_scope_message(scope)
+      message_parts += feature_flag.strategies.map do |strategy|
+        created_strategy_message(strategy)
       end
 
       message_parts.join(" ")

app/services/feature_flags/update_service.rb

@@ -2,10 +2,9 @@
 
 module FeatureFlags
   class UpdateService < FeatureFlags::BaseService
-    AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES = {
-      'active' => 'active state',
-      'environment_scope' => 'environment scope',
-      'strategies' => 'strategies'
+    AUDITABLE_STRATEGY_ATTRIBUTES_HUMAN_NAMES = {
+      'scopes' => 'environment scopes',
+      'parameters' => 'parameters'
     }.freeze
 
     def execute(feature_flag)
@@ -41,7 +40,7 @@ module FeatureFlags
 
     def audit_message(feature_flag)
       changes = changed_attributes_messages(feature_flag)
-      changes += changed_scopes_messages(feature_flag)
+      changes += changed_strategies_messages(feature_flag)
 
       return if changes.empty?
 
@@ -56,29 +55,30 @@ module FeatureFlags
       end
     end
 
-    def changed_scopes_messages(feature_flag)
-      feature_flag.scopes.map do |scope|
-        if scope.new_record?
-          created_scope_message(scope)
-        elsif scope.marked_for_destruction?
-          deleted_scope_message(scope)
+    def changed_strategies_messages(feature_flag)
+      feature_flag.strategies.map do |strategy|
+        if strategy.new_record?
+          created_strategy_message(strategy)
+        elsif strategy.marked_for_destruction?
+          deleted_strategy_message(strategy)
         else
-          updated_scope_message(scope)
+          updated_strategy_message(strategy)
         end
-      end.compact # updated_scope_message can return nil if nothing has been changed
+      end.compact # updated_strategy_message can return nil if nothing has been changed
     end
 
-    def deleted_scope_message(scope)
-      "Deleted rule #{scope.environment_scope}."
+    def deleted_strategy_message(strategy)
+      scopes = strategy.scopes.map { |scope| scope.environment_scope }.join(', ')
+      "Deleted strategy #{strategy.name} with environment scopes #{scopes}."
     end
 
-    def updated_scope_message(scope)
-      changes = scope.changes.slice(*AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES.keys)
+    def updated_strategy_message(strategy)
+      changes = strategy.changes.slice(*AUDITABLE_STRATEGY_ATTRIBUTES_HUMAN_NAMES.keys)
       return if changes.empty?
 
-      message = "Updated rule #{scope.environment_scope} "
+      message = "Updated strategy #{strategy.name} "
       message += changes.map do |attribute_name, change|
-        name = AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES[attribute_name]
+        name = AUDITABLE_STRATEGY_ATTRIBUTES_HUMAN_NAMES[attribute_name]
         "#{name} from #{change.first} to #{change.second}"
       end.join(' ')
 

doc/administration/audit_events.md

@@ -126,6 +126,7 @@ From there, you can see the following actions:
 - User password required for approvals was updated ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/336211) in GitLab 14.2)
 - Permission to modify merge requests approval rules in merge requests was updated ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/336211) in GitLab 14.2)
 - New approvals requirement when new commits are added to an MR was updated ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/336211) in GitLab 14.2)
+- When [strategies for feature flags](../operations/feature_flags.md#feature-flag-strategies) are changed ([introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68408) in GitLab 14.3)
 
 Project events can also be accessed via the [Project Audit Events API](../api/audit_events.md#project-audit-events).
 

spec/services/feature_flags/create_service_spec.rb

@@ -48,8 +48,9 @@ RSpec.describe FeatureFlags::CreateService do
         {
           name: 'feature_flag',
           description: 'description',
-          scopes_attributes: [{ environment_scope: '*', active: true },
-                              { environment_scope: 'production', active: false }]
+          version: 'new_version_flag',
+          strategies_attributes: [{ name: 'default', scopes_attributes: [{ environment_scope: '*' }], parameters: {} },
+                                  { name: 'default', parameters: {}, scopes_attributes: [{ environment_scope: 'production' }] }]
         }
       end
 
@@ -70,10 +71,10 @@ RSpec.describe FeatureFlags::CreateService do
       it 'creates audit event' do
         expected_message = 'Created feature flag feature_flag '\
                            'with description "description". '\
-                           'Created rule * and set it as active '\
-                           'with strategies [{"name"=>"default", "parameters"=>{}}]. '\
-                           'Created rule production and set it as inactive '\
-                           'with strategies [{"name"=>"default", "parameters"=>{}}].'
+                           'Created strategy "default" with scopes '\
+                           '"*". '\
+                           'Created strategy "default" with scopes '\
+                           '"production".'
 
         expect { subject }.to change { AuditEvent.count }.by(1)
         expect(AuditEvent.last.details[:custom_message]).to eq(expected_message)