From 5027979b9ba0ebfb6376cfa78114e942f5054c5c Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Fri, 2 Aug 2019 19:02:57 +0000 Subject: Implement Helm ResetCommand for removing Tiller Also creates specs Only allow Helm to be uninstalled if it's the only app - Remove Tiller leftovers after reser command - Fixes specs and offenses Adds changelog file Fix reset_command specs --- .../uninstall_application_confirmation_modal.vue | 5 +- app/models/clusters/applications/helm.rb | 28 ++++++++-- changelogs/unreleased/60516-uninstall-tiller.yml | 5 ++ doc/user/clusters/applications.md | 1 + lib/gitlab/kubernetes/helm/delete_command.rb | 11 ---- lib/gitlab/kubernetes/helm/reset_command.rb | 54 ++++++++++++++++++ locale/gitlab.pot | 3 + .../projects/clusters/applications_spec.rb | 3 +- .../gitlab/kubernetes/helm/reset_command_spec.rb | 65 ++++++++++++++++++++++ spec/models/clusters/applications/helm_spec.rb | 59 +++++++++++++++++++- .../serializers/cluster_application_entity_spec.rb | 2 +- 11 files changed, 215 insertions(+), 21 deletions(-) create mode 100644 changelogs/unreleased/60516-uninstall-tiller.yml create mode 100644 lib/gitlab/kubernetes/helm/reset_command.rb create mode 100644 spec/lib/gitlab/kubernetes/helm/reset_command_spec.rb diff --git a/app/assets/javascripts/clusters/components/uninstall_application_confirmation_modal.vue b/app/assets/javascripts/clusters/components/uninstall_application_confirmation_modal.vue index 2ff6d5e32e2..e067eb13c54 100644 --- a/app/assets/javascripts/clusters/components/uninstall_application_confirmation_modal.vue +++ b/app/assets/javascripts/clusters/components/uninstall_application_confirmation_modal.vue @@ -2,9 +2,12 @@ import { GlModal } from '@gitlab/ui'; import { sprintf, s__ } from '~/locale'; import trackUninstallButtonClickMixin from 'ee_else_ce/clusters/mixins/track_uninstall_button_click'; -import { INGRESS, CERT_MANAGER, PROMETHEUS, RUNNER, KNATIVE, JUPYTER } from '../constants'; +import { HELM, INGRESS, CERT_MANAGER, PROMETHEUS, RUNNER, KNATIVE, JUPYTER } from '../constants'; const CUSTOM_APP_WARNING_TEXT = { + [HELM]: s__( + 'ClusterIntegration|The associated Tiller pod will be deleted and cannot be restored.', + ), [INGRESS]: s__( 'ClusterIntegration|The associated load balancer and IP will be deleted and cannot be restored.', ), diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index a83d06c4b00..3a175fec148 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -14,6 +14,7 @@ module Clusters include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Gitlab::Utils::StrongMemoize default_value_for :version, Gitlab::Kubernetes::Helm::HELM_VERSION @@ -29,11 +30,22 @@ module Clusters self.status = 'installable' if cluster&.platform_kubernetes_active? end - # We will implement this in future MRs. - # Basically we need to check all other applications are not installed - # first. + # It can only be uninstalled if there are no other applications installed + # or with intermitent installation statuses in the database. def allowed_to_uninstall? - false + strong_memoize(:allowed_to_uninstall) do + applications = nil + + Clusters::Cluster::APPLICATIONS.each do |application_name, klass| + next if application_name == 'helm' + + extra_apps = Clusters::Applications::Helm.where('EXISTS (?)', klass.select(1).where(cluster_id: cluster_id)) + + applications = applications.present? ? applications.or(extra_apps) : extra_apps + end + + !applications.exists? + end end def install_command @@ -44,6 +56,14 @@ module Clusters ) end + def uninstall_command + Gitlab::Kubernetes::Helm::ResetCommand.new( + name: name, + files: files, + rbac: cluster.platform_kubernetes_rbac? + ) + end + def has_ssl? ca_key.present? && ca_cert.present? end diff --git a/changelogs/unreleased/60516-uninstall-tiller.yml b/changelogs/unreleased/60516-uninstall-tiller.yml new file mode 100644 index 00000000000..db25e7b3338 --- /dev/null +++ b/changelogs/unreleased/60516-uninstall-tiller.yml @@ -0,0 +1,5 @@ +--- +title: Allow Helm to be uninstalled from the UI +merge_request: 27359 +author: +type: added diff --git a/doc/user/clusters/applications.md b/doc/user/clusters/applications.md index 7c24f6db86f..a29df76f4b7 100644 --- a/doc/user/clusters/applications.md +++ b/doc/user/clusters/applications.md @@ -253,6 +253,7 @@ The applications below can be uninstalled. | Application | GitLab version | Notes | | ----------- | -------------- | ----- | | GitLab Runner | 12.2+ | Any running pipelines will be canceled. | +| Helm | 12.2+ | The associated Tiller pod will be deleted and cannot be restored. | | Ingress | 12.1+ | The associated load balancer and IP will be deleted and cannot be restored. Furthermore, it can only be uninstalled if JupyterHub is not installed. | | JupyterHub | 12.1+ | All data not committed to GitLab will be deleted and cannot be restored. | | Knative | 12.1+ | The associated IP will be deleted and cannot be restored. | diff --git a/lib/gitlab/kubernetes/helm/delete_command.rb b/lib/gitlab/kubernetes/helm/delete_command.rb index aeba4a54b6d..dcf22e7abb6 100644 --- a/lib/gitlab/kubernetes/helm/delete_command.rb +++ b/lib/gitlab/kubernetes/helm/delete_command.rb @@ -43,17 +43,6 @@ module Gitlab command.shelljoin end - - def optional_tls_flags - return [] unless files.key?(:'ca.pem') - - [ - '--tls', - '--tls-ca-cert', "#{files_dir}/ca.pem", - '--tls-cert', "#{files_dir}/cert.pem", - '--tls-key', "#{files_dir}/key.pem" - ] - end end end end diff --git a/lib/gitlab/kubernetes/helm/reset_command.rb b/lib/gitlab/kubernetes/helm/reset_command.rb new file mode 100644 index 00000000000..37e1d8573ab --- /dev/null +++ b/lib/gitlab/kubernetes/helm/reset_command.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Gitlab + module Kubernetes + module Helm + class ResetCommand + include BaseCommand + include ClientCommand + + attr_reader :name, :files + + def initialize(name:, rbac:, files:) + @name = name + @files = files + @rbac = rbac + end + + def generate_script + super + [ + reset_helm_command, + delete_tiller_replicaset + ].join("\n") + end + + def rbac? + @rbac + end + + def pod_name + "uninstall-#{name}" + end + + private + + # This method can be delete once we upgrade Helm to > 12.13.0 + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27096#note_159695900 + # + # Tracking this method to be removed here: + # https://gitlab.com/gitlab-org/gitlab-ce/issues/52791#note_199374155 + def delete_tiller_replicaset + command = %w[kubectl delete replicaset -n gitlab-managed-apps -l name=tiller] + + command.shelljoin + end + + def reset_helm_command + command = %w[helm reset] + optional_tls_flags + + command.shelljoin + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5e9e371a5fc..45a73004954 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2818,6 +2818,9 @@ msgstr "" msgid "ClusterIntegration|The associated IP and all deployed services will be deleted and cannot be restored. Uninstalling Knative will also remove Istio from your cluster. This will not effect any other applications." msgstr "" +msgid "ClusterIntegration|The associated Tiller pod will be deleted and cannot be restored." +msgstr "" + msgid "ClusterIntegration|The associated certifcate will be deleted and cannot be restored." msgstr "" diff --git a/spec/features/projects/clusters/applications_spec.rb b/spec/features/projects/clusters/applications_spec.rb index de97c8a8bc0..217bf1f5506 100644 --- a/spec/features/projects/clusters/applications_spec.rb +++ b/spec/features/projects/clusters/applications_spec.rb @@ -63,7 +63,8 @@ describe 'Clusters Applications', :js do Clusters::Cluster.last.application_helm.make_installed! - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installed') + expect(page).not_to have_css('.js-cluster-application-install-button') + expect(page).to have_css('.js-cluster-application-uninstall-button:not([disabled])', exact_text: 'Uninstall') end expect(page).to have_content('Helm Tiller was successfully installed on your Kubernetes cluster') diff --git a/spec/lib/gitlab/kubernetes/helm/reset_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/reset_command_spec.rb new file mode 100644 index 00000000000..d49d4779735 --- /dev/null +++ b/spec/lib/gitlab/kubernetes/helm/reset_command_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Kubernetes::Helm::ResetCommand do + let(:rbac) { true } + let(:name) { 'helm' } + let(:files) { {} } + let(:reset_command) { described_class.new(name: name, rbac: rbac, files: files) } + + subject { reset_command } + + it_behaves_like 'helm commands' do + let(:commands) do + <<~EOS + helm reset + kubectl delete replicaset -n gitlab-managed-apps -l name\\=tiller + EOS + end + end + + context 'when there is a ca.pem file' do + let(:files) { { 'ca.pem': 'some file content' } } + + it_behaves_like 'helm commands' do + let(:commands) do + <<~EOS1.squish + "\n" + <<~EOS2 + helm reset + --tls + --tls-ca-cert /data/helm/helm/config/ca.pem + --tls-cert /data/helm/helm/config/cert.pem + --tls-key /data/helm/helm/config/key.pem + EOS1 + kubectl delete replicaset -n gitlab-managed-apps -l name\\=tiller + EOS2 + end + end + end + + describe '#pod_resource' do + subject { reset_command.pod_resource } + + context 'rbac is enabled' do + let(:rbac) { true } + + it 'generates a pod that uses the tiller serviceAccountName' do + expect(subject.spec.serviceAccountName).to eq('tiller') + end + end + + context 'rbac is not enabled' do + let(:rbac) { false } + + it 'generates a pod that uses the default serviceAccountName' do + expect(subject.spec.serviceAcccountName).to be_nil + end + end + end + + describe '#pod_name' do + subject { reset_command.pod_name } + + it { is_expected.to eq('uninstall-helm') } + end +end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 6ea6c110d62..d4f8b552088 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -19,11 +19,27 @@ describe Clusters::Applications::Helm do end describe '#can_uninstall?' do - let(:helm) { create(:clusters_applications_helm) } + context "with other existing applications" do + Clusters::Cluster::APPLICATIONS.keys.each do |application_name| + next if application_name == 'helm' + + it do + cluster_application = create("clusters_applications_#{application_name}".to_sym) + + helm = cluster_application.cluster.application_helm - subject { helm.can_uninstall? } + expect(helm.allowed_to_uninstall?).to be_falsy + end + end + end - it { is_expected.to be_falsey } + context "without other existing applications" do + subject { helm.can_uninstall? } + + let(:helm) { create(:clusters_applications_helm) } + + it { is_expected.to be_truthy } + end end describe '#issue_client_cert' do @@ -73,4 +89,41 @@ describe Clusters::Applications::Helm do end end end + + describe '#uninstall_command' do + let(:helm) { create(:clusters_applications_helm) } + + subject { helm.uninstall_command } + + it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::ResetCommand) } + + it 'has name' do + expect(subject.name).to eq('helm') + end + + it 'has cert files' do + expect(subject.files[:'ca.pem']).to be_present + expect(subject.files[:'ca.pem']).to eq(helm.ca_cert) + + expect(subject.files[:'cert.pem']).to be_present + expect(subject.files[:'key.pem']).to be_present + + cert = OpenSSL::X509::Certificate.new(subject.files[:'cert.pem']) + expect(cert.not_after).to be > 999.years.from_now + end + + describe 'rbac' do + context 'rbac cluster' do + it { expect(subject).to be_rbac } + end + + context 'non rbac cluster' do + before do + helm.cluster.platform_kubernetes.abac! + end + + it { expect(subject).not_to be_rbac } + end + end + end end diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb index f38a18fcf59..76ecca06522 100644 --- a/spec/serializers/cluster_application_entity_spec.rb +++ b/spec/serializers/cluster_application_entity_spec.rb @@ -22,7 +22,7 @@ describe ClusterApplicationEntity do end it 'has can_uninstall' do - expect(subject[:can_uninstall]).to be_falsey + expect(subject[:can_uninstall]).to be_truthy end context 'non-helm application' do -- cgit v1.2.1