Merge branch '55447-validate-k8s-credentials' into 'master'

Resolve "Validate k8s credentials provided at cluster creation"

Closes #55447

See merge request gitlab-org/gitlab-ce!27403

app/assets/javascripts/clusters/clusters_bundle.js

@@ -1,5 +1,6 @@
 import Visibility from 'visibilityjs';
 import Vue from 'vue';
+import AccessorUtilities from '~/lib/utils/accessor';
 import { GlToast } from '@gitlab/ui';
 import PersistentUserCallout from '../persistent_user_callout';
 import { s__, sprintf } from '../locale';
@@ -43,8 +44,10 @@ export default class Clusters {
       helpPath,
       ingressHelpPath,
       ingressDnsHelpPath,
+      clusterId,
     } = document.querySelector('.js-edit-cluster-form').dataset;
 
+    this.clusterId = clusterId;
     this.store = new ClustersStore();
     this.store.setHelpPaths(helpPath, ingressHelpPath, ingressDnsHelpPath);
     this.store.setManagePrometheusPath(managePrometheusPath);
@@ -69,6 +72,10 @@ export default class Clusters {
     this.errorContainer = document.querySelector('.js-cluster-error');
     this.successContainer = document.querySelector('.js-cluster-success');
     this.creatingContainer = document.querySelector('.js-cluster-creating');
+    this.unreachableContainer = document.querySelector('.js-cluster-api-unreachable');
+    this.authenticationFailureContainer = document.querySelector(
+      '.js-cluster-authentication-failure',
+    );
     this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason');
     this.successApplicationContainer = document.querySelector('.js-cluster-application-notice');
     this.showTokenButton = document.querySelector('.js-show-cluster-token');
@@ -125,6 +132,13 @@ export default class Clusters {
     PersistentUserCallout.factory(callout);
   }
 
+  addBannerCloseHandler(el, status) {
+    el.querySelector('.js-close-banner').addEventListener('click', () => {
+      el.classList.add('hidden');
+      this.setBannerDismissedState(status, true);
+    });
+  }
+
   addListeners() {
     if (this.showTokenButton) this.showTokenButton.addEventListener('click', this.showToken);
     eventHub.$on('installApplication', this.installApplication);
@@ -133,6 +147,9 @@ export default class Clusters {
     eventHub.$on('saveKnativeDomain', data => this.saveKnativeDomain(data));
     eventHub.$on('setKnativeHostname', data => this.setKnativeHostname(data));
     eventHub.$on('uninstallApplication', data => this.uninstallApplication(data));
+    // Add event listener to all the banner close buttons
+    this.addBannerCloseHandler(this.unreachableContainer, 'unreachable');
+    this.addBannerCloseHandler(this.authenticationFailureContainer, 'authentication_failure');
   }
 
   removeListeners() {
@@ -205,6 +222,8 @@ export default class Clusters {
     this.errorContainer.classList.add('hidden');
     this.successContainer.classList.add('hidden');
     this.creatingContainer.classList.add('hidden');
+    this.unreachableContainer.classList.add('hidden');
+    this.authenticationFailureContainer.classList.add('hidden');
   }
 
   checkForNewInstalls(prevApplicationMap, newApplicationMap) {
@@ -228,9 +247,32 @@ export default class Clusters {
     }
   }
 
+  setBannerDismissedState(status, isDismissed) {
+    if (AccessorUtilities.isLocalStorageAccessSafe()) {
+      window.localStorage.setItem(
+        `cluster_${this.clusterId}_banner_dismissed`,
+        `${status}_${isDismissed}`,
+      );
+    }
+  }
+
+  isBannerDismissed(status) {
+    let bannerState;
+    if (AccessorUtilities.isLocalStorageAccessSafe()) {
+      bannerState = window.localStorage.getItem(`cluster_${this.clusterId}_banner_dismissed`);
+    }
+
+    return bannerState === `${status}_true`;
+  }
+
   updateContainer(prevStatus, status, error) {
     this.hideAll();
 
+    if (this.isBannerDismissed(status)) {
+      return;
+    }
+    this.setBannerDismissedState(status, false);
+
     // We poll all the time but only want the `created` banner to show when newly created
     if (this.store.state.status !== 'created' || prevStatus !== this.store.state.status) {
       switch (status) {
@@ -241,6 +283,12 @@ export default class Clusters {
           this.errorContainer.classList.remove('hidden');
           this.errorReasonContainer.textContent = error;
           break;
+        case 'unreachable':
+          this.unreachableContainer.classList.remove('hidden');
+          break;
+        case 'authentication_failure':
+          this.authenticationFailureContainer.classList.remove('hidden');
+          break;
         case 'scheduled':
         case 'creating':
           this.creatingContainer.classList.remove('hidden');

app/assets/stylesheets/pages/clusters.scss

@@ -69,6 +69,8 @@
   align-self: flex-start;
   font-weight: 500;
   font-size: 20px;
+  color: $orange-900;
+  opacity: 1;
   margin: $gl-padding-8 14px 0 0;
 }
 

app/models/clusters/cluster.rb

@@ -5,8 +5,10 @@ module Clusters
     include Presentable
     include Gitlab::Utils::StrongMemoize
     include FromUnion
+    include ReactiveCaching
 
     self.table_name = 'clusters'
+    self.reactive_cache_key = -> (cluster) { [cluster.class.model_name.singular, cluster.id] }
 
     PROJECT_ONLY_APPLICATIONS = {
       Applications::Jupyter.application_name => Applications::Jupyter,
@@ -57,6 +59,8 @@ module Clusters
     validate :no_groups, unless: :group_type?
     validate :no_projects, unless: :project_type?
 
+    after_save :clear_reactive_cache!
+
     delegate :status, to: :provider, allow_nil: true
     delegate :status_reason, to: :provider, allow_nil: true
     delegate :on_creation?, to: :provider, allow_nil: true
@@ -123,15 +127,19 @@ module Clusters
     end
 
     def status_name
-      if provider
-        provider.status_name
-      else
-        :created
+      provider&.status_name || connection_status.presence || :created
+    end
+
+    def connection_status
+      with_reactive_cache do |data|
+        data[:connection_status]
       end
     end
 
-    def created?
-      status_name == :created
+    def calculate_reactive_cache
+      return unless enabled?
+
+      { connection_status: retrieve_connection_status }
     end
 
     def applications
@@ -221,6 +229,51 @@ module Clusters
       @instance_domain ||= Gitlab::CurrentSettings.auto_devops_domain
     end
 
+    def retrieve_connection_status
+      kubeclient.core_client.discover
+    rescue *Gitlab::Kubernetes::Errors::CONNECTION
+      :unreachable
+    rescue *Gitlab::Kubernetes::Errors::AUTHENTICATION
+      :authentication_failure
+    rescue Kubeclient::HttpError => e
+      kubeclient_error_status(e.message)
+    rescue => e
+      Gitlab::Sentry.track_acceptable_exception(e, extra: { cluster_id: id })
+
+      :unknown_failure
+    else
+      :connected
+    end
+
+    # KubeClient uses the same error class
+    # For connection errors (eg. timeout) and
+    # for Kubernetes errors.
+    def kubeclient_error_status(message)
+      if message&.match?(/timed out|timeout/i)
+        :unreachable
+      else
+        :authentication_failure
+      end
+    end
+
+    # To keep backward compatibility with AUTO_DEVOPS_DOMAIN
+    # environment variable, we need to ensure KUBE_INGRESS_BASE_DOMAIN
+    # is set if AUTO_DEVOPS_DOMAIN is set on any of the following options:
+    # ProjectAutoDevops#Domain, project variables or group variables,
+    # as the AUTO_DEVOPS_DOMAIN is needed for CI_ENVIRONMENT_URL
+    #
+    # This method should is scheduled to be removed on
+    # https://gitlab.com/gitlab-org/gitlab-ce/issues/56959
+    def legacy_auto_devops_domain
+      if project_type?
+        project&.auto_devops&.domain.presence ||
+          project.variables.find_by(key: 'AUTO_DEVOPS_DOMAIN')&.value.presence ||
+          project.group&.variables&.find_by(key: 'AUTO_DEVOPS_DOMAIN')&.value.presence
+      elsif group_type?
+        group.variables.find_by(key: 'AUTO_DEVOPS_DOMAIN')&.value.presence
+      end
+    end
+
     def restrict_modification
       if provider&.on_creation?
         errors.add(:base, "cannot modify during creation")

app/presenters/clusters/cluster_presenter.rb

@@ -22,10 +22,6 @@ module Clusters
       "https://console.cloud.google.com/kubernetes/clusters/details/#{provider.zone}/#{name}" if gcp?
     end
 
-    def can_toggle_cluster?
-      can?(current_user, :update_cluster, cluster) && created?
-    end
-
     def can_read_cluster?
       can?(current_user, :read_cluster, cluster)
     end

app/views/clusters/clusters/_banner.html.haml

@@ -5,5 +5,17 @@
 .hidden.js-cluster-creating.bs-callout.bs-callout-info{ role: 'alert' }
   = s_('ClusterIntegration|Kubernetes cluster is being created on Google Kubernetes Engine...')
 
+.hidden.row.js-cluster-api-unreachable.bs-callout.bs-callout-warning{ role: 'alert' }
+  .col-11
+    = s_('ClusterIntegration|Your cluster API is unreachable. Please ensure your API URL is correct.')
+  .col-1.p-0
+    %button.js-close-banner.close.cluster-application-banner-close.h-100.m-0= "×"
+
+.hidden.js-cluster-authentication-failure.row.js-cluster-api-unreachable.bs-callout.bs-callout-warning{ role: 'alert' }
+  .col-11
+    = s_('ClusterIntegration|There was a problem authenticating with your cluster. Please ensure your CA Certificate and Token are valid.')
+  .col-1.p-0
+    %button.js-close-banner.close.cluster-application-banner-close.h-100.m-0= "×"
+
 .hidden.js-cluster-success.bs-callout.bs-callout-success{ role: 'alert' }
   = s_("ClusterIntegration|Kubernetes cluster was successfully created on Google Kubernetes Engine. Refresh the page to see Kubernetes cluster's details")

app/views/clusters/clusters/show.html.haml

@@ -24,7 +24,8 @@
   help_path: help_page_path('user/project/clusters/index.md', anchor: 'installing-applications'),
   ingress_help_path: help_page_path('user/project/clusters/index.md', anchor: 'getting-the-external-endpoint'),
   ingress_dns_help_path: help_page_path('user/project/clusters/index.md', anchor: 'manually-determining-the-external-endpoint'),
-  manage_prometheus_path: manage_prometheus_path } }
+  manage_prometheus_path: manage_prometheus_path,
+  cluster_id: @cluster.id } }
 
   .js-cluster-application-notice
     .flash-container

changelogs/unreleased/55447-validate-k8s-credentials.yml

@@ -0,0 +1,5 @@
+---
+title: Validate Kubernetes credentials at cluster creation
+merge_request: 27403
+author:
+type: added

lib/gitlab/kubernetes/errors.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Kubernetes
+    module Errors
+      CONNECTION = [
+        SocketError,
+        OpenSSL::SSL::SSLError,
+        Errno::ECONNRESET,
+        Errno::ENETUNREACH,
+        Errno::ECONNREFUSED,
+        Errno::EHOSTUNREACH,
+        Net::OpenTimeout,
+        Net::ReadTimeout,
+        IPAddr::InvalidAddressError
+      ].freeze
+
+      AUTHENTICATION = [
+        OpenSSL::X509::CertificateError
+      ].freeze
+    end
+  end
+end

locale/gitlab.pot

@@ -2505,6 +2505,9 @@ msgstr ""
 msgid "ClusterIntegration|The endpoint is in the process of being assigned. Please check your Kubernetes cluster or Quotas on Google Kubernetes Engine if it takes a long time."
 msgstr ""
 
+msgid "ClusterIntegration|There was a problem authenticating with your cluster. Please ensure your CA Certificate and Token are valid."
+msgstr ""
+
 msgid "ClusterIntegration|This account must have permissions to create a Kubernetes cluster in the %{link_to_container_project} specified below"
 msgstr ""
 
@@ -2559,6 +2562,9 @@ msgstr ""
 msgid "ClusterIntegration|Your account must have %{link_to_kubernetes_engine}"
 msgstr ""
 
+msgid "ClusterIntegration|Your cluster API is unreachable. Please ensure your API URL is correct."
+msgstr ""
+
 msgid "ClusterIntegration|Zone"
 msgstr ""
 

spec/features/clusters/cluster_detail_page_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe 'Clusterable > Show page' do
+  include KubernetesHelpers
+
   let(:current_user) { create(:user) }
   let(:cluster_ingress_help_text_selector) { '.js-ingress-domain-help-text' }
   let(:hide_modifier_selector) { '.hide' }
@@ -83,6 +85,7 @@ describe 'Clusterable > Show page' do
 
   shared_examples 'editing a user-provided cluster' do
     before do
+      stub_kubeclient_discover(cluster.platform.api_url)
       clusterable.add_maintainer(current_user)
       visit cluster_path
     end

spec/features/groups/clusters/user_spec.rb

@@ -14,6 +14,7 @@ describe 'User Cluster', :js do
 
     allow(Groups::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
     allow_any_instance_of(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).to receive(:execute)
+    allow_any_instance_of(Clusters::Cluster).to receive(:retrieve_connection_status).and_return(:connected)
   end
 
   context 'when user does not have a cluster and visits cluster index page' do

spec/features/projects/clusters/user_spec.rb

@@ -12,6 +12,7 @@ describe 'User Cluster', :js do
 
     allow(Projects::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
     allow_any_instance_of(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).to receive(:execute)
+    allow_any_instance_of(Clusters::Cluster).to receive(:retrieve_connection_status).and_return(:connected)
   end
 
   context 'when user does not have a cluster and visits cluster index page' do

spec/frontend/clusters/clusters_bundle_spec.js

@@ -209,6 +209,22 @@ describe('Clusters', () => {
         expect(cluster.errorContainer.classList.contains('hidden')).toBeFalsy();
       });
     });
+
+    describe('when cluster is unreachable', () => {
+      it('should show the unreachable warning container', () => {
+        cluster.updateContainer(null, 'unreachable');
+
+        expect(cluster.unreachableContainer.classList.contains('hidden')).toBe(false);
+      });
+    });
+
+    describe('when cluster has an authentication failure', () => {
+      it('should show the authentication failure warning container', () => {
+        cluster.updateContainer(null, 'authentication_failure');
+
+        expect(cluster.authenticationFailureContainer.classList.contains('hidden')).toBe(false);
+      });
+    });
   });
 
   describe('installApplication', () => {

spec/models/clusters/cluster_spec.rb

@@ -2,7 +2,10 @@
 
 require 'spec_helper'
 
-describe Clusters::Cluster do
+describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
+  include ReactiveCachingHelpers
+  include KubernetesHelpers
+
   it_behaves_like 'having unique enum values'
 
   subject { build(:cluster) }
@@ -23,7 +26,6 @@ describe Clusters::Cluster do
 
   it { is_expected.to delegate_method(:status).to(:provider) }
   it { is_expected.to delegate_method(:status_reason).to(:provider) }
-  it { is_expected.to delegate_method(:status_name).to(:provider) }
   it { is_expected.to delegate_method(:on_creation?).to(:provider) }
   it { is_expected.to delegate_method(:active?).to(:platform_kubernetes).with_prefix }
   it { is_expected.to delegate_method(:rbac?).to(:platform_kubernetes).with_prefix }
@@ -501,28 +503,6 @@ describe Clusters::Cluster do
     end
   end
 
-  describe '#created?' do
-    let(:cluster) { create(:cluster, :provided_by_gcp) }
-
-    subject { cluster.created? }
-
-    context 'when status_name is :created' do
-      before do
-        allow(cluster).to receive_message_chain(:provider, :status_name).and_return(:created)
-      end
-
-      it { is_expected.to eq(true) }
-    end
-
-    context 'when status_name is not :created' do
-      before do
-        allow(cluster).to receive_message_chain(:provider, :status_name).and_return(:creating)
-      end
-
-      it { is_expected.to eq(false) }
-    end
-  end
-
   describe '#allow_user_defined_namespace?' do
     let(:cluster) { create(:cluster, :provided_by_gcp) }
 
@@ -617,4 +597,139 @@ describe Clusters::Cluster do
       it { is_expected.to be_truthy }
     end
   end
+
+  describe '#status_name' do
+    subject { cluster.status_name }
+
+    context 'the cluster has a provider' do
+      let(:cluster) { create(:cluster, :provided_by_gcp) }
+
+      before do
+        cluster.provider.make_errored!
+      end
+
+      it { is_expected.to eq :errored }
+    end
+
+    context 'there is a cached connection status' do
+      let(:cluster) { create(:cluster, :provided_by_user) }
+
+      before do
+        allow(cluster).to receive(:connection_status).and_return(:connected)
+      end
+
+      it { is_expected.to eq :connected }
+    end
+
+    context 'there is no connection status in the cache' do
+      let(:cluster) { create(:cluster, :provided_by_user) }
+
+      before do
+        allow(cluster).to receive(:connection_status).and_return(nil)
+      end
+
+      it { is_expected.to eq :created }
+    end
+  end
+
+  describe '#connection_status' do
+    let(:cluster) { create(:cluster) }
+    let(:status) { :connected }
+
+    subject { cluster.connection_status }
+
+    it { is_expected.to be_nil }
+
+    context 'with a cached status' do
+      before do
+        stub_reactive_cache(cluster, connection_status: status)
+      end
+
+      it { is_expected.to eq(status) }
+    end
+  end
+
+  describe '#calculate_reactive_cache' do
+    subject { cluster.calculate_reactive_cache }
+
+    context 'cluster is disabled' do
+      let(:cluster) { create(:cluster, :disabled) }
+
+      it 'does not populate the cache' do
+        expect(cluster).not_to receive(:retrieve_connection_status)
+
+        is_expected.to be_nil
+      end
+    end
+
+    context 'cluster is enabled' do
+      let(:cluster) { create(:cluster, :provided_by_user, :group) }
+
+      context 'connection to the cluster is successful' do
+        before do
+          stub_kubeclient_discover(cluster.platform.api_url)
+        end
+
+        it { is_expected.to eq(connection_status: :connected) }
+      end
+
+      context 'cluster cannot be reached' do
+        before do
+          allow(cluster.kubeclient.core_client).to receive(:discover)
+            .and_raise(SocketError)
+        end
+
+        it { is_expected.to eq(connection_status: :unreachable) }
+      end
+
+      context 'cluster cannot be authenticated to' do
+        before do
+          allow(cluster.kubeclient.core_client).to receive(:discover)
+            .and_raise(OpenSSL::X509::CertificateError.new("Certificate error"))
+        end
+
+        it { is_expected.to eq(connection_status: :authentication_failure) }
+      end
+
+      describe 'Kubeclient::HttpError' do
+        let(:error_code) { 403 }
+        let(:error_message) { "Forbidden" }
+
+        before do
+          allow(cluster.kubeclient.core_client).to receive(:discover)
+            .and_raise(Kubeclient::HttpError.new(error_code, error_message, nil))
+        end
+
+        it { is_expected.to eq(connection_status: :authentication_failure) }
+
+        context 'generic timeout' do
+          let(:error_message) { 'Timed out connecting to server'}
+
+          it { is_expected.to eq(connection_status: :unreachable) }
+        end
+
+        context 'gateway timeout' do
+          let(:error_message) { '504 Gateway Timeout for GET https://kubernetes.example.com/api/v1'}
+
+          it { is_expected.to eq(connection_status: :unreachable) }
+        end
+      end
+
+      context 'an uncategorised error is raised' do
+        before do
+          allow(cluster.kubeclient.core_client).to receive(:discover)
+            .and_raise(StandardError)
+        end
+
+        it { is_expected.to eq(connection_status: :unknown_failure) }
+
+        it 'notifies Sentry' do
+          expect(Gitlab::Sentry).to receive(:track_acceptable_exception)
+            .with(instance_of(StandardError), hash_including(extra: { cluster_id: cluster.id }))
+
+          subject
+        end
+      end
+    end
+  end
 end

spec/presenters/clusters/cluster_presenter_spec.rb

@@ -158,46 +158,6 @@ describe Clusters::ClusterPresenter do
     it { is_expected.to include(cluster.name) }
   end
 
-  describe '#can_toggle_cluster' do
-    let(:user) { create(:user) }
-
-    before do
-      allow(cluster).to receive(:current_user).and_return(user)
-    end
-
-    subject { described_class.new(cluster).can_toggle_cluster? }
-
-    context 'when user can update' do
-      before do
-        allow_any_instance_of(described_class).to receive(:can?).with(user, :update_cluster, cluster).and_return(true)
-      end
-
-      context 'when cluster is created' do
-        before do
-          allow(cluster).to receive(:created?).and_return(true)
-        end
-
-        it { is_expected.to eq(true) }
-      end
-
-      context 'when cluster is not created' do
-        before do
-          allow(cluster).to receive(:created?).and_return(false)
-        end
-
-        it { is_expected.to eq(false) }
-      end
-    end
-
-    context 'when user can not update' do
-      before do
-        allow_any_instance_of(described_class).to receive(:can?).with(user, :update_cluster, cluster).and_return(false)
-      end
-
-      it { is_expected.to eq(false) }
-    end
-  end
-
   describe '#cluster_type_description' do
     subject { described_class.new(cluster).cluster_type_description }