Merge branch 'add-alert-manager-token-to-prometheus-cluster-integration' into 'master'

Keep new Prometheus cluster integration in sync with app

See merge request gitlab-org/gitlab!60877

app/models/clusters/applications/prometheus.rb

@@ -25,18 +25,15 @@ module Clusters
         key: Settings.attr_encrypted_db_key_base_32,
         algorithm: 'aes-256-gcm'
 
+      default_value_for(:alert_manager_token) { SecureRandom.hex }
+
       after_destroy do
-        run_after_commit do
-          disable_prometheus_integration
-        end
+        cluster.find_or_build_integration_prometheus.destroy
       end
 
       state_machine :status do
         after_transition any => [:installed, :externally_installed] do |application|
-          application.run_after_commit do
-            Clusters::Applications::ActivateServiceWorker
-              .perform_async(application.cluster_id, ::PrometheusService.to_param) # rubocop:disable CodeReuse/ServiceClass
-          end
+          application.cluster.find_or_build_integration_prometheus.update(enabled: true, alert_manager_token: application.alert_manager_token)
         end
 
         after_transition any => :updating do |application|
@@ -103,23 +100,8 @@ module Clusters
         files.merge('values.yaml': replaced_values)
       end
 
-      def generate_alert_manager_token!
-        unless alert_manager_token.present?
-          update!(alert_manager_token: generate_token)
-        end
-      end
-
       private
 
-      def generate_token
-        SecureRandom.hex
-      end
-
-      def disable_prometheus_integration
-        ::Clusters::Applications::DeactivateServiceWorker
-          .perform_async(cluster_id, ::PrometheusService.to_param) # rubocop:disable CodeReuse/ServiceClass
-      end
-
       def install_knative_metrics
         return [] unless cluster.application_knative_available?
 

app/models/clusters/integrations/prometheus.rb

@@ -4,6 +4,7 @@ module Clusters
   module Integrations
     class Prometheus < ApplicationRecord
       include ::Clusters::Concerns::PrometheusClient
+      include AfterCommitQueue
 
       self.table_name = 'clusters_integration_prometheus'
       self.primary_key = :cluster_id
@@ -13,9 +14,46 @@ module Clusters
       validates :cluster, presence: true
       validates :enabled, inclusion: { in: [true, false] }
 
+      attr_encrypted :alert_manager_token,
+        mode: :per_attribute_iv,
+        key: Settings.attr_encrypted_db_key_base_32,
+        algorithm: 'aes-256-gcm'
+
+      default_value_for(:alert_manager_token) { SecureRandom.hex }
+
+      after_destroy do
+        run_after_commit do
+          deactivate_project_services
+        end
+      end
+
+      after_save do
+        next unless enabled_before_last_save != enabled
+
+        run_after_commit do
+          if enabled
+            activate_project_services
+          else
+            deactivate_project_services
+          end
+        end
+      end
+
       def available?
         enabled?
       end
+
+      private
+
+      def activate_project_services
+        ::Clusters::Applications::ActivateServiceWorker
+          .perform_async(cluster_id, ::PrometheusService.to_param) # rubocop:disable CodeReuse/ServiceClass
+      end
+
+      def deactivate_project_services
+        ::Clusters::Applications::DeactivateServiceWorker
+          .perform_async(cluster_id, ::PrometheusService.to_param) # rubocop:disable CodeReuse/ServiceClass
+      end
     end
   end
 end

app/services/clusters/applications/prometheus_config_service.rb

@@ -96,8 +96,6 @@ module Clusters
       end
 
       def alert_manager_token
-        app.generate_alert_manager_token!
-
         app.alert_manager_token
       end
 

app/services/projects/prometheus/alerts/notify_service.rb

@@ -63,7 +63,7 @@ module Projects
         def valid_alert_manager_token?(token, integration)
           valid_for_manual?(token) ||
             valid_for_alerts_endpoint?(token, integration) ||
-            valid_for_managed?(token)
+            valid_for_cluster?(token)
         end
 
         def valid_for_manual?(token)
@@ -83,18 +83,20 @@ module Projects
           compare_token(token, integration.token)
         end
 
-        def valid_for_managed?(token)
-          prometheus_application = available_prometheus_application(project)
-          return false unless prometheus_application
+        def valid_for_cluster?(token)
+          cluster_integration = find_cluster_integration(project)
+          return false unless cluster_integration
+
+          cluster_integration_token = cluster_integration.alert_manager_token
 
           if token
-            compare_token(token, prometheus_application.alert_manager_token)
+            compare_token(token, cluster_integration_token)
           else
-            prometheus_application.alert_manager_token.nil?
+            cluster_integration_token.nil?
           end
         end
 
-        def available_prometheus_application(project)
+        def find_cluster_integration(project)
           alert_id = gitlab_alert_id
           return unless alert_id
 
@@ -105,7 +107,7 @@ module Projects
           return unless cluster&.enabled?
           return unless cluster.application_prometheus_available?
 
-          cluster.application_prometheus
+          cluster.application_prometheus || cluster.integration_prometheus
         end
 
         def find_alert(project, metric)

changelogs/unreleased/add-alert-manager-token-to-prometheus-cluster-integration.yml

+---
+title: Keep new prometheus cluster integration in sync with old cluster application
+merge_request: 60877
+author:
+type: changed

db/migrate/20210430034712_add_alert_manager_token_to_clusters_integration_prometheus.rb

+# frozen_string_literal: true
+
+class AddAlertManagerTokenToClustersIntegrationPrometheus < ActiveRecord::Migration[6.0]
+  def change
+    change_table :clusters_integration_prometheus do |t|
+      t.text :encrypted_alert_manager_token
+      t.text :encrypted_alert_manager_token_iv
+    end
+  end
+end

db/schema_migrations/20210430034712

+6b7436d7712e31ca116204d37270435ccc059ca75a128750e5c39fdddfa020e3
\ No newline at end of file

db/structure.sql

@@ -11701,7 +11701,9 @@ CREATE TABLE clusters_integration_prometheus (
     created_at timestamp with time zone NOT NULL,
     updated_at timestamp with time zone NOT NULL,
     cluster_id bigint NOT NULL,
-    enabled boolean DEFAULT false NOT NULL
+    enabled boolean DEFAULT false NOT NULL,
+    encrypted_alert_manager_token text,
+    encrypted_alert_manager_token_iv text
 );
 
 CREATE TABLE clusters_kubernetes_namespaces (

lib/gitlab/prometheus/adapter.rb

@@ -19,13 +19,11 @@ module Gitlab
       end
 
       def cluster_prometheus_adapter
-        if cluster&.integration_prometheus
-          return cluster.integration_prometheus
-        end
-
         application = cluster&.application_prometheus
+        return application if application&.available?
 
-        application if application&.available?
+        integration = cluster&.integration_prometheus
+        integration if integration&.available?
       end
 
       private

spec/deprecation_toolkit_env.rb

@@ -59,6 +59,7 @@ module DeprecationToolkitEnv
       activesupport-6.0.3.6/lib/active_support/cache.rb
       activerecord-6.0.3.6/lib/active_record/relation.rb
       asciidoctor-2.0.12/lib/asciidoctor/extensions.rb
+      attr_encrypted-3.1.0/lib/attr_encrypted/adapters/active_record.rb
     ]
   end
 

spec/lib/gitlab/prometheus/adapter_spec.rb

@@ -58,8 +58,8 @@ RSpec.describe Gitlab::Prometheus::Adapter do
         context 'with cluster with prometheus integration' do
           let!(:prometheus_integration) { create(:clusters_integrations_prometheus, cluster: cluster) }
 
-          it 'returns the integration instead' do
-            expect(subject.prometheus_adapter).to eq(prometheus_integration)
+          it 'returns the application' do
+            expect(subject.prometheus_adapter).to eq(prometheus)
           end
         end
       end

spec/models/clusters/applications/prometheus_spec.rb

@@ -13,16 +13,13 @@ RSpec.describe Clusters::Applications::Prometheus do
   include_examples 'cluster application initial status specs'
 
   describe 'after_destroy' do
-    context 'cluster type is project' do
-      let(:cluster) { create(:cluster, :with_installed_helm) }
-      let(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
+    let(:cluster) { create(:cluster, :with_installed_helm) }
+    let(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
 
-      it 'deactivates prometheus_service after destroy' do
-        expect(Clusters::Applications::DeactivateServiceWorker)
-          .to receive(:perform_async).with(cluster.id, 'prometheus')
+    it 'disables the corresponding integration' do
+      application.destroy!
 
-        application.destroy!
-      end
+      expect(cluster.integration_prometheus).not_to be_enabled
     end
   end
 
@@ -31,11 +28,10 @@ RSpec.describe Clusters::Applications::Prometheus do
     let(:cluster) { create(:cluster, :with_installed_helm) }
     let(:application) { create(:clusters_applications_prometheus, :installing, cluster: cluster) }
 
-    it 'schedules post installation job' do
-      expect(Clusters::Applications::ActivateServiceWorker)
-        .to receive(:perform_async).with(cluster.id, 'prometheus')
-
+    it 'enables the corresponding integration' do
       application.make_installed
+
+      expect(cluster.integration_prometheus).to be_enabled
     end
   end
 
@@ -44,11 +40,10 @@ RSpec.describe Clusters::Applications::Prometheus do
     let(:cluster) { create(:cluster, :with_installed_helm) }
     let(:application) { create(:clusters_applications_prometheus, :installing, cluster: cluster) }
 
-    it 'schedules post installation job' do
-      expect(Clusters::Applications::ActivateServiceWorker)
-        .to receive(:perform_async).with(cluster.id, 'prometheus')
-
+    it 'enables the corresponding integration' do
       application.make_externally_installed!
+
+      expect(cluster.integration_prometheus).to be_enabled
     end
   end
 
@@ -338,42 +333,10 @@ RSpec.describe Clusters::Applications::Prometheus do
   describe 'alert manager token' do
     subject { create(:clusters_applications_prometheus) }
 
-    context 'when not set' do
-      it 'is empty by default' do
-        expect(subject.alert_manager_token).to be_nil
-        expect(subject.encrypted_alert_manager_token).to be_nil
-        expect(subject.encrypted_alert_manager_token_iv).to be_nil
-      end
-
-      describe '#generate_alert_manager_token!' do
-        it 'generates a token' do
-          subject.generate_alert_manager_token!
-
-          expect(subject.alert_manager_token).to match(/\A\h{32}\z/)
-        end
-      end
-    end
-
-    context 'when set' do
-      let(:token) { SecureRandom.hex }
-
-      before do
-        subject.update!(alert_manager_token: token)
-      end
-
-      it 'reads the token' do
-        expect(subject.alert_manager_token).to eq(token)
-        expect(subject.encrypted_alert_manager_token).not_to be_nil
-        expect(subject.encrypted_alert_manager_token_iv).not_to be_nil
-      end
-
-      describe '#generate_alert_manager_token!' do
-        it 'does not re-generate the token' do
-          subject.generate_alert_manager_token!
-
-          expect(subject.alert_manager_token).to eq(token)
-        end
-      end
+    it 'is autogenerated on creation' do
+      expect(subject.alert_manager_token).to match(/\A\h{32}\z/)
+      expect(subject.encrypted_alert_manager_token).not_to be_nil
+      expect(subject.encrypted_alert_manager_token_iv).not_to be_nil
     end
   end
 end

spec/models/clusters/integrations/prometheus_spec.rb

@@ -15,6 +15,62 @@ RSpec.describe Clusters::Integrations::Prometheus do
     it { is_expected.not_to allow_value(nil).for(:enabled) }
   end
 
+  describe 'after_destroy' do
+    subject(:integration) { create(:clusters_integrations_prometheus, cluster: cluster, enabled: true) }
+
+    let(:cluster) { create(:cluster, :with_installed_helm) }
+
+    it 'deactivates prometheus_service' do
+      expect(Clusters::Applications::DeactivateServiceWorker)
+        .to receive(:perform_async).with(cluster.id, 'prometheus')
+
+      integration.destroy!
+    end
+  end
+
+  describe 'after_save' do
+    subject(:integration) { create(:clusters_integrations_prometheus, cluster: cluster, enabled: enabled) }
+
+    let(:cluster) { create(:cluster, :with_installed_helm) }
+    let(:enabled) { true }
+
+    context 'when no change to enabled status' do
+      it 'does not touch project services' do
+        integration # ensure integration exists before we set the expectations
+
+        expect(Clusters::Applications::DeactivateServiceWorker)
+          .not_to receive(:perform_async)
+
+        expect(Clusters::Applications::ActivateServiceWorker)
+          .not_to receive(:perform_async)
+
+        integration.update!(enabled: enabled)
+      end
+    end
+
+    context 'when enabling' do
+      let(:enabled) { false }
+
+      it 'deactivates prometheus_service' do
+        expect(Clusters::Applications::ActivateServiceWorker)
+          .to receive(:perform_async).with(cluster.id, 'prometheus')
+
+        integration.update!(enabled: true)
+      end
+    end
+
+    context 'when disabling' do
+      let(:enabled) { true }
+
+      it 'activates prometheus_service' do
+        expect(Clusters::Applications::DeactivateServiceWorker)
+          .to receive(:perform_async).with(cluster.id, 'prometheus')
+
+        integration.update!(enabled: false)
+      end
+    end
+  end
+
   describe '#prometheus_client' do
     include_examples '#prometheus_client shared' do
       let(:factory) { :clusters_integrations_prometheus }

spec/services/projects/prometheus/alerts/notify_service_spec.rb

@@ -62,7 +62,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
       end
     end
 
-    context 'with project specific cluster' do
+    context 'with project specific cluster using prometheus application' do
       where(:cluster_enabled, :status, :configured_token, :token_input, :result) do
         true  | :installed | token | token | :success
         true  | :installed | nil   | nil   | :success
@@ -97,6 +97,40 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
       end
     end
 
+    context 'with project specific cluster using prometheus integration' do
+      where(:cluster_enabled, :integration_enabled, :configured_token, :token_input, :result) do
+        true  | true  | token | token | :success
+        true  | true  | nil   | nil   | :success
+        true  | true  | token | 'x'   | :failure
+        true  | true  | token | nil   | :failure
+        true  | false | token | token | :failure
+        false | true  | token | token | :failure
+        false | nil   | nil   | token | :failure
+      end
+
+      with_them do
+        before do
+          cluster.update!(enabled: cluster_enabled)
+
+          unless integration_enabled.nil?
+            create(:clusters_integrations_prometheus,
+                   cluster: cluster,
+                   enabled: integration_enabled,
+                   alert_manager_token: configured_token)
+          end
+        end
+
+        case result = params[:result]
+        when :success
+          include_examples 'processes one firing and one resolved prometheus alerts'
+        when :failure
+          it_behaves_like 'alerts service responds with an error and takes no actions', :unauthorized
+        else
+          raise "invalid result: #{result.inspect}"
+        end
+      end
+    end
+
     context 'without project specific cluster' do
       let_it_be(:cluster) { create(:cluster, enabled: true) }