Merge branch 'clusters_integrations_prometheus' into 'master'

Use a new table/model for Prometheus cluster integration

See merge request gitlab-org/gitlab!59091

app/controllers/clusters/clusters_controller.rb

@@ -61,7 +61,7 @@ class Clusters::ClustersController < Clusters::BaseController
 
   def show
     if params[:tab] == 'integrations'
-      @prometheus_integration = Clusters::IntegrationPresenter.new(@cluster.find_or_build_application(Clusters::Applications::Prometheus))
+      @prometheus_integration = Clusters::IntegrationPresenter.new(@cluster.find_or_build_integration_prometheus)
     end
   end
 

app/models/clusters/cluster.rb

@@ -51,6 +51,8 @@ module Clusters
 
     has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes', inverse_of: :cluster, autosave: true
 
+    has_one :integration_prometheus, class_name: 'Clusters::Integrations::Prometheus', inverse_of: :cluster
+
     def self.has_one_cluster_application(name) # rubocop:disable Naming/PredicateName
       application = APPLICATIONS[name.to_s]
       has_one application.association_name, class_name: application.to_s, inverse_of: :cluster # rubocop:disable Rails/ReflectionClassName
@@ -100,7 +102,6 @@ module Clusters
     delegate :rbac?, to: :platform_kubernetes, prefix: true, allow_nil: true
     delegate :available?, to: :application_helm, prefix: true, allow_nil: true
     delegate :available?, to: :application_ingress, prefix: true, allow_nil: true
-    delegate :available?, to: :application_prometheus, prefix: true, allow_nil: true
     delegate :available?, to: :application_knative, prefix: true, allow_nil: true
     delegate :available?, to: :application_elastic_stack, prefix: true, allow_nil: true
     delegate :external_ip, to: :application_ingress, prefix: true, allow_nil: true
@@ -148,6 +149,9 @@ module Clusters
     scope :with_management_project, -> { where.not(management_project: nil) }
 
     scope :for_project_namespace, -> (namespace_id) { joins(:projects).where(projects: { namespace_id: namespace_id }) }
+
+    # with_application_prometheus scope is deprecated, and scheduled for removal
+    # in %14.0. See https://gitlab.com/groups/gitlab-org/-/epics/4280
     scope :with_application_prometheus, -> { includes(:application_prometheus).joins(:application_prometheus) }
     scope :with_project_http_integrations, -> (project_ids) do
       conditions = { projects: :alert_management_http_integrations }
@@ -276,6 +280,10 @@ module Clusters
       public_send(association_name) || public_send("build_#{association_name}") # rubocop:disable GitlabSecurity/PublicSend
     end
 
+    def find_or_build_integration_prometheus
+      integration_prometheus || build_integration_prometheus
+    end
+
     def provider
       if gcp?
         provider_gcp
@@ -361,8 +369,12 @@ module Clusters
       end
     end
 
+    def application_prometheus_available?
+      integration_prometheus&.available? || application_prometheus&.available?
+    end
+
     def prometheus_adapter
-      application_prometheus
+      integration_prometheus || application_prometheus
     end
 
     private

app/models/clusters/integrations/prometheus.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Integrations
+    class Prometheus < ApplicationRecord
+      include ::Clusters::Concerns::PrometheusClient
+
+      self.table_name = 'clusters_integration_prometheus'
+      self.primary_key = :cluster_id
+
+      belongs_to :cluster, class_name: 'Clusters::Cluster', foreign_key: :cluster_id
+
+      validates :cluster, presence: true
+      validates :enabled, inclusion: { in: [true, false] }
+
+      def available?
+        enabled?
+      end
+    end
+  end
+end

app/presenters/clusters/integration_presenter.rb

@@ -4,16 +4,8 @@ module Clusters
   class IntegrationPresenter < Gitlab::View::Presenter::Delegated
     presents :integration
 
-    def integration?
-      integration.new_record? || integration.externally_installed? || integration.uninstalled?
-    end
-
     def application_type
-      integration.class.application_name
-    end
-
-    def enabled
-      integration.externally_installed?
+      integration.class.name.demodulize.underscore
     end
   end
 end

app/services/clusters/integrations/create_service.rb

@@ -3,8 +3,6 @@
 module Clusters
   module Integrations
     class CreateService < BaseContainerService
-      InvalidApplicationError = Class.new(StandardError)
-
       attr_accessor :cluster
 
       def initialize(container:, cluster:, current_user: nil, params: {})
@@ -16,20 +14,27 @@ module Clusters
       def execute
         return ServiceResponse.error(message: 'Unauthorized') unless authorized?
 
-        application_class = Clusters::Cluster::APPLICATIONS[params[:application_type]]
-        application = cluster.find_or_build_application(application_class)
+        integration.enabled = params[:enabled]
+        integration.save!
 
-        if params[:enabled]
-          application.make_externally_installed!
-          ServiceResponse.success(message: s_('ClusterIntegration|Integration enabled'), payload: { application: application })
+        if integration.enabled?
+          ServiceResponse.success(message: s_('ClusterIntegration|Integration enabled'), payload: { integration: integration })
         else
-          application.make_externally_uninstalled!
-          ServiceResponse.success(message: s_('ClusterIntegration|Integration disabled'), payload: { application: application })
+          ServiceResponse.success(message: s_('ClusterIntegration|Integration disabled'), payload: { integration: integration })
         end
       end
 
       private
 
+      def integration
+        case params[:application_type]
+        when 'prometheus'
+          cluster.find_or_build_integration_prometheus
+        else
+          raise ArgumentError, "invalid application_type: #{params[:application_type]}"
+        end
+      end
+
       def authorized?
         Ability.allowed?(current_user, :admin_cluster, cluster)
       end

app/views/clusters/clusters/_integrations.html.haml

@@ -3,13 +3,13 @@
     = s_('ClusterIntegration|Integrations enable you to integrate your cluster as part of your GitLab workflow.')
     = link_to _('Learn more'), help_page_path('user/clusters/integrations.md'), target: '_blank'
   .settings-content#advanced-settings-section
-    - if can?(current_user, :admin_cluster, @cluster) && @prometheus_integration.integration?
+    - if can?(current_user, :admin_cluster, @cluster)
       .sub-section.form-group
         = form_for @prometheus_integration, url: @cluster.integrations_path, as: :integration, method: :post, html: { class: 'js-cluster-integrations-form' } do |form|
           = form.hidden_field :application_type
           .form-group
             .gl-form-checkbox.custom-control.custom-checkbox
-              = form.check_box :enabled, { class: 'custom-control-input'}, true, false
+              = form.check_box :enabled, { class: 'custom-control-input'}
               = form.label :enabled, s_('ClusterIntegration|Enable Prometheus integration'), class: 'custom-control-label'
             .gl-form-group
               .form-text.text-gl-muted

changelogs/unreleased/clusters_integrations_prometheus.yml

+---
+title: Adds new clusters_integrations_prometheus table and model for Prometheus Cluster
+  Integration
+merge_request: 59091
+author:
+type: changed

db/migrate/20210411212813_add_clusters_integrations_prometheus.rb

+# frozen_string_literal: true
+
+class AddClustersIntegrationsPrometheus < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  def up
+    with_lock_retries do
+      create_table :clusters_integration_prometheus, id: false do |t|
+        t.timestamps_with_timezone null: false
+        t.references :cluster, primary_key: true, default: nil, index: false, foreign_key: { on_delete: :cascade }
+        t.boolean :enabled, null: false, default: false
+      end
+    end
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :clusters_integration_prometheus
+    end
+  end
+end

db/schema_migrations/20210411212813

+47c1d8d699a18f4c52178dd5de6434f9997166e05acd70bdc40ff85a1572a797
\ No newline at end of file

db/structure.sql

@@ -11572,6 +11572,13 @@ CREATE SEQUENCE clusters_id_seq
 
 ALTER SEQUENCE clusters_id_seq OWNED BY clusters.id;
 
+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
+);
+
 CREATE TABLE clusters_kubernetes_namespaces (
     id bigint NOT NULL,
     cluster_id integer NOT NULL,
@@ -20447,6 +20454,9 @@ ALTER TABLE ONLY clusters_applications_prometheus
 ALTER TABLE ONLY clusters_applications_runners
     ADD CONSTRAINT clusters_applications_runners_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY clusters_integration_prometheus
+    ADD CONSTRAINT clusters_integration_prometheus_pkey PRIMARY KEY (cluster_id);
+
 ALTER TABLE ONLY clusters_kubernetes_namespaces
     ADD CONSTRAINT clusters_kubernetes_namespaces_pkey PRIMARY KEY (id);
 
@@ -26677,6 +26687,9 @@ ALTER TABLE ONLY ci_builds_metadata
 ALTER TABLE ONLY vulnerability_finding_evidences
     ADD CONSTRAINT fk_rails_e3205a0c65 FOREIGN KEY (vulnerability_occurrence_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY clusters_integration_prometheus
+    ADD CONSTRAINT fk_rails_e44472034c FOREIGN KEY (cluster_id) REFERENCES clusters(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY vulnerability_occurrence_identifiers
     ADD CONSTRAINT fk_rails_e4ef6d027c FOREIGN KEY (occurrence_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE;
 

lib/gitlab/prometheus/adapter.rb

@@ -19,6 +19,10 @@ module Gitlab
       end
 
       def cluster_prometheus_adapter
+        if cluster&.integration_prometheus
+          return cluster.integration_prometheus
+        end
+
         application = cluster&.application_prometheus
 
         application if application&.available?

spec/factories/clusters/integrations/prometheus.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :clusters_integrations_prometheus, class: 'Clusters::Integrations::Prometheus' do
+    cluster factory: %i(cluster provided_by_gcp)
+    enabled { true }
+
+    trait :disabled do
+      enabled { false }
+    end
+  end
+end

spec/lib/gitlab/prometheus/adapter_spec.rb

@@ -32,6 +32,14 @@ RSpec.describe Gitlab::Prometheus::Adapter do
     context "prometheus service can't execute queries" do
       let(:prometheus_service) { double(:prometheus_service, can_query?: false) }
 
+      context 'with cluster with prometheus integration' do
+        let!(:prometheus_integration) { create(:clusters_integrations_prometheus, cluster: cluster) }
+
+        it 'returns the integration' do
+          expect(subject.prometheus_adapter).to eq(prometheus_integration)
+        end
+      end
+
       context 'with cluster with prometheus not available' do
         let!(:prometheus) { create(:clusters_applications_prometheus, :installable, cluster: cluster) }
 
@@ -46,6 +54,14 @@ RSpec.describe Gitlab::Prometheus::Adapter do
         it 'returns application handling all environments' do
           expect(subject.prometheus_adapter).to eq(prometheus)
         end
+
+        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)
+          end
+        end
       end
 
       context 'with cluster without prometheus installed' do

spec/models/clusters/cluster_spec.rb

@@ -21,6 +21,7 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
   it { is_expected.to have_one(:provider_gcp) }
   it { is_expected.to have_one(:provider_aws) }
   it { is_expected.to have_one(:platform_kubernetes) }
+  it { is_expected.to have_one(:integration_prometheus) }
   it { is_expected.to have_one(:application_helm) }
   it { is_expected.to have_one(:application_ingress) }
   it { is_expected.to have_one(:application_prometheus) }
@@ -40,7 +41,6 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
   it { is_expected.to delegate_method(:rbac?).to(:platform_kubernetes).with_prefix }
   it { is_expected.to delegate_method(:available?).to(:application_helm).with_prefix }
   it { is_expected.to delegate_method(:available?).to(:application_ingress).with_prefix }
-  it { is_expected.to delegate_method(:available?).to(:application_prometheus).with_prefix }
   it { is_expected.to delegate_method(:available?).to(:application_knative).with_prefix }
   it { is_expected.to delegate_method(:available?).to(:application_elastic_stack).with_prefix }
   it { is_expected.to delegate_method(:external_ip).to(:application_ingress).with_prefix }
@@ -1349,6 +1349,80 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '#application_prometheus_available?' do
+    let_it_be_with_reload(:cluster) { create(:cluster, :project) }
+
+    subject { cluster.application_prometheus_available? }
+
+    it { is_expected.to be_falsey }
+
+    context 'has a integration_prometheus' do
+      let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) }
+
+      it { is_expected.to be_truthy }
+
+      context 'disabled' do
+        before do
+          cluster.integration_prometheus.enabled = false
+        end
+
+        it { is_expected.to be_falsey }
+      end
+    end
+
+    context 'has a application_prometheus' do
+      let_it_be(:application) { create(:clusters_applications_prometheus, :installed, :no_helm_installed, cluster: cluster) }
+
+      it { is_expected.to be_truthy }
+
+      context 'errored' do
+        before do
+          cluster.application_prometheus.status = Clusters::Applications::Prometheus.state_machines[:status].states[:errored]
+        end
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'also has a integration_prometheus' do
+        let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) }
+
+        it { is_expected.to be_truthy }
+      end
+    end
+  end
+
+  describe '#prometheus_adapter' do
+    let_it_be_with_reload(:cluster) { create(:cluster, :project) }
+
+    it 'returns nothing' do
+      expect(cluster.prometheus_adapter).to be_nil
+    end
+
+    context 'has a integration_prometheus' do
+      let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) }
+
+      it 'returns the integration' do
+        expect(cluster.prometheus_adapter).to eq(integration)
+      end
+    end
+
+    context 'has a application_prometheus' do
+      let_it_be(:application) { create(:clusters_applications_prometheus, :no_helm_installed, cluster: cluster) }
+
+      it 'returns the application' do
+        expect(cluster.prometheus_adapter).to eq(application)
+      end
+
+      context 'also has a integration_prometheus' do
+        let_it_be(:integration) { create(:clusters_integrations_prometheus, cluster: cluster) }
+
+        it 'returns the integration' do
+          expect(cluster.prometheus_adapter).to eq(integration)
+        end
+      end
+    end
+  end
+
   describe '#delete_cached_resources!' do
     let!(:cluster) { create(:cluster, :project) }
     let!(:staging_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, namespace: 'staging') }

spec/models/clusters/integrations/prometheus_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Clusters::Integrations::Prometheus do
+  include KubernetesHelpers
+  include StubRequests
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:cluster).class_name('Clusters::Cluster') }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:cluster) }
+    it { is_expected.not_to allow_value(nil).for(:enabled) }
+  end
+
+  describe '#prometheus_client' do
+    include_examples '#prometheus_client shared' do
+      let(:factory) { :clusters_integrations_prometheus }
+    end
+  end
+
+  describe '#configured?' do
+    let(:prometheus) { create(:clusters_integrations_prometheus, cluster: cluster) }
+
+    subject { prometheus.configured? }
+
+    context 'when a kubenetes client is present' do
+      let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+
+      it { is_expected.to be_truthy }
+
+      context 'when it is disabled' do
+        let(:prometheus) { create(:clusters_integrations_prometheus, :disabled, cluster: cluster) }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'when the kubernetes URL is blocked' do
+        before do
+          blocked_ip = '127.0.0.1' # localhost addresses are blocked by default
+
+          stub_all_dns(cluster.platform.api_url, ip_address: blocked_ip)
+        end
+
+        it { is_expected.to be_falsey }
+      end
+    end
+
+    context 'when a kubenetes client is not present' do
+      let(:cluster) { create(:cluster) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+end

spec/services/clusters/integrations/create_service_spec.rb

@@ -17,9 +17,9 @@ RSpec.describe Clusters::Integrations::CreateService, '#execute' do
   it 'creates a new Prometheus instance' do
     expect(service.execute).to be_success
 
-    expect(cluster.application_prometheus).to be_present
-    expect(cluster.application_prometheus).to be_persisted
-    expect(cluster.application_prometheus).to be_externally_installed
+    expect(cluster.integration_prometheus).to be_present
+    expect(cluster.integration_prometheus).to be_persisted
+    expect(cluster.integration_prometheus).to be_enabled
   end
 
   context 'enabled param is false' do
@@ -30,9 +30,9 @@ RSpec.describe Clusters::Integrations::CreateService, '#execute' do
     it 'creates a new uninstalled Prometheus instance' do
       expect(service.execute).to be_success
 
-      expect(cluster.application_prometheus).to be_present
-      expect(cluster.application_prometheus).to be_persisted
-      expect(cluster.application_prometheus).to be_uninstalled
+      expect(cluster.integration_prometheus).to be_present
+      expect(cluster.integration_prometheus).to be_persisted
+      expect(cluster.integration_prometheus).not_to be_enabled
     end
   end
 
@@ -46,21 +46,21 @@ RSpec.describe Clusters::Integrations::CreateService, '#execute' do
     it 'does not create a new Prometheus instance' do
       expect(service.execute).to be_error
 
-      expect(cluster.application_prometheus).to be_nil
+      expect(cluster.integration_prometheus).to be_nil
     end
   end
 
   context 'prometheus record exists' do
     before do
-      create(:clusters_applications_prometheus, cluster: cluster)
+      create(:clusters_integrations_prometheus, cluster: cluster)
     end
 
     it 'updates the Prometheus instance' do
       expect(service.execute).to be_success
 
-      expect(cluster.application_prometheus).to be_present
-      expect(cluster.application_prometheus).to be_persisted
-      expect(cluster.application_prometheus).to be_externally_installed
+      expect(cluster.integration_prometheus).to be_present
+      expect(cluster.integration_prometheus).to be_persisted
+      expect(cluster.integration_prometheus).to be_enabled
     end
 
     context 'enabled param is false' do
@@ -71,9 +71,9 @@ RSpec.describe Clusters::Integrations::CreateService, '#execute' do
       it 'updates the Prometheus instance as uninstalled' do
         expect(service.execute).to be_success
 
-        expect(cluster.application_prometheus).to be_present
-        expect(cluster.application_prometheus).to be_persisted
-        expect(cluster.application_prometheus).to be_uninstalled
+        expect(cluster.integration_prometheus).to be_present
+        expect(cluster.integration_prometheus).to be_persisted
+        expect(cluster.integration_prometheus).not_to be_enabled
       end
     end
   end