Merge branch 'ci-platform-metrics-invalid-bucket' into 'master'

Updates CiPlatformMetrics to ignore invalids

See merge request gitlab-org/gitlab!42116

app/models/ci_platform_metric.rb

@@ -14,19 +14,25 @@ class CiPlatformMetric < ApplicationRecord
     numericality: { only_integer: true, greater_than: 0 }
 
   CI_VARIABLE_KEY = 'AUTO_DEVOPS_PLATFORM_TARGET'
+  ALLOWED_TARGETS = %w[ECS FARGATE].freeze
 
   def self.insert_auto_devops_platform_targets!
+    recorded_at = Time.zone.now
+
     # This work can NOT be done in-database because value is encrypted.
     # However, for 'AUTO_DEVOPS_PLATFORM_TARGET', these values are only
     # encrypted as a matter of course, rather than as a need for secrecy.
     # So this is not a security risk, but exposing other keys possibly could be.
     variables = Ci::Variable.by_key(CI_VARIABLE_KEY)
-    recorded_at = Time.zone.now
+
     counts = variables.group_by(&:value).map do |value, variables|
-      target = value.truncate(PLATFORM_TARGET_MAX_LENGTH, separator: '', omission: '')
+      # While this value is, in theory, not secret. A user could accidentally
+      # put a secret in here so we need to make sure we filter invalid values.
+      next unless ALLOWED_TARGETS.include?(value)
+
       count = variables.count
-      self.new(recorded_at: recorded_at, platform_target: target, count: count)
-    end
+      self.new(recorded_at: recorded_at, platform_target: value, count: count)
+    end.compact
 
     bulk_insert!(counts, validate: true)
   end

changelogs/unreleased/ci-platform-metrics-invalid-bucket.yml

+---
+title: Filter the values for deployment platform metrics
+merge_request: 42116
+author:
+type: changed

spec/models/ci_platform_metric_spec.rb

@@ -46,44 +46,45 @@ RSpec.describe CiPlatformMetric do
 
       it 'inserts platform target counts for that day' do
         Timecop.freeze(today) do
-          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'aws')
-          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'aws')
-          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'fargate')
-          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'fargate')
-          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'fargate')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'ECS')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'ECS')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FARGATE')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FARGATE')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FARGATE')
           described_class.insert_auto_devops_platform_targets!
         end
         Timecop.freeze(tomorrow) do
-          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'fargate')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FARGATE')
           described_class.insert_auto_devops_platform_targets!
         end
 
         expect(platform_target_counts_by_day).to eq({
-          today.to_date => { 'aws' => 2, 'fargate' => 3 },
-          tomorrow.to_date => { 'aws' => 2, 'fargate' => 4 }
+          today.to_date => { 'ECS' => 2, 'FARGATE' => 3 },
+          tomorrow.to_date => { 'ECS' => 2, 'FARGATE' => 4 }
         })
       end
     end
 
-    context 'when there are ci variable values too long for platform_target' do
+    context 'when there are invalid ci variable values for platform_target' do
       let(:today) { Time.zone.local(1982, 4, 24) }
 
-      it 'truncates those values' do
-        max = described_class::PLATFORM_TARGET_MAX_LENGTH
+      it 'ignores those values' do
         Timecop.freeze(today) do
-          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'F' * (max + 1))
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'ECS')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FOO')
+          create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'BAR')
           described_class.insert_auto_devops_platform_targets!
         end
 
         expect(platform_target_counts_by_day).to eq({
-          today.to_date => { 'F' * max => 1 }
+          today.to_date => { 'ECS' => 1 }
         })
       end
     end
 
     context 'when there are no platform target variables' do
       it 'does not generate any new platform metrics' do
-        create(:ci_variable, key: 'KEY_WHATEVER', value: 'aws')
+        create(:ci_variable, key: 'KEY_WHATEVER', value: 'ECS')
         described_class.insert_auto_devops_platform_targets!
 
         expect(platform_target_counts_by_day).to eq({})