Merge branch 'local_tiller_backend' into 'master'

Local tiller backend fixes

See merge request gitlab-org/gitlab!25340

app/models/clusters/concerns/application_core.rb

@@ -15,7 +15,7 @@ module Clusters
         def set_initial_status
           return unless not_installable?
 
-          self.status = status_states[:installable] if cluster&.application_helm_available? || Feature.enabled?(:managed_apps_local_tiller)
+          self.status = status_states[:installable] if cluster&.application_helm_available? || ::Gitlab::Kubernetes::Helm.local_tiller_enabled?
         end
 
         def can_uninstall?

app/models/clusters/concerns/application_data.rb

@@ -23,7 +23,7 @@ module Clusters
         @files ||= begin
           files = { 'values.yaml': values }
 
-          files.merge!(certificate_files) if cluster.application_helm.has_ssl?
+          files.merge!(certificate_files) if use_tiller_ssl?
 
           files
         end
@@ -31,6 +31,12 @@ module Clusters
 
       private
 
+      def use_tiller_ssl?
+        return false if ::Gitlab::Kubernetes::Helm.local_tiller_enabled?
+
+        cluster.application_helm.has_ssl?
+      end
+
       def certificate_files
         {
           'ca.pem': ca_cert,

app/models/clusters/concerns/application_status.rb

@@ -92,7 +92,10 @@ module Clusters
             # When installing any application we are also performing an update
             # of tiller (see Gitlab::Kubernetes::Helm::ClientCommand) so
             # therefore we need to reflect that in the database.
-            application.cluster.application_helm.update!(version: Gitlab::Kubernetes::Helm::HELM_VERSION)
+
+            unless ::Gitlab::Kubernetes::Helm.local_tiller_enabled?
+              application.cluster.application_helm.update!(version: Gitlab::Kubernetes::Helm::HELM_VERSION)
+            end
           end
 
           after_transition any => [:uninstalling], :use_transactions => false do |application, _|

lib/gitlab/kubernetes/helm.rb

@@ -10,6 +10,12 @@ module Gitlab
       SERVICE_ACCOUNT = 'tiller'
       CLUSTER_ROLE_BINDING = 'tiller-admin'
       CLUSTER_ROLE = 'cluster-admin'
+
+      MANAGED_APPS_LOCAL_TILLER_FEATURE_FLAG = :managed_apps_local_tiller
+
+      def self.local_tiller_enabled?
+        Feature.enabled?(MANAGED_APPS_LOCAL_TILLER_FEATURE_FLAG)
+      end
     end
   end
 end

lib/gitlab/kubernetes/helm/client_command.rb

@@ -59,7 +59,7 @@ module Gitlab
         end
 
         def local_tiller_enabled?
-          Feature.enabled?(:managed_apps_local_tiller)
+          ::Gitlab::Kubernetes::Helm.local_tiller_enabled?
         end
       end
     end

spec/factories/clusters/applications/helm.rb

@@ -73,39 +73,71 @@ FactoryBot.define do
     factory :clusters_applications_ingress, class: 'Clusters::Applications::Ingress' do
       modsecurity_enabled { false }
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
 
     factory :clusters_applications_cert_manager, class: 'Clusters::Applications::CertManager' do
       email { 'admin@example.com' }
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
 
     factory :clusters_applications_elastic_stack, class: 'Clusters::Applications::ElasticStack' do
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
 
     factory :clusters_applications_crossplane, class: 'Clusters::Applications::Crossplane' do
       stack { 'gcp' }
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
 
     factory :clusters_applications_prometheus, class: 'Clusters::Applications::Prometheus' do
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
 
     factory :clusters_applications_runner, class: 'Clusters::Applications::Runner' do
       runner factory: %i(ci_runner)
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
 
     factory :clusters_applications_knative, class: 'Clusters::Applications::Knative' do
       hostname { 'example.com' }
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
 
     factory :clusters_applications_jupyter, class: 'Clusters::Applications::Jupyter' do
       oauth_application factory: :oauth_application
       cluster factory: %i(cluster with_installed_helm provided_by_gcp project)
+
+      trait :no_helm_installed do
+        cluster factory: %i(cluster provided_by_gcp)
+      end
     end
   end
 end

spec/models/clusters/applications/prometheus_spec.rb

@@ -274,7 +274,8 @@ describe Clusters::Applications::Prometheus do
     subject { application.files_with_replaced_values({ hello: :world }) }
 
     it 'does not modify #files' do
-      expect(subject[:'values.yaml']).not_to eq(files)
+      expect(subject[:'values.yaml']).not_to eq(files[:'values.yaml'])
+
       expect(files[:'values.yaml']).to eq(application.values)
     end
 
@@ -282,27 +283,17 @@ describe Clusters::Applications::Prometheus do
       expect(subject[:'values.yaml']).to eq({ hello: :world })
     end
 
-    it 'includes cert files' do
-      expect(subject[:'ca.pem']).to be_present
-      expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert)
-
-      expect(subject[:'cert.pem']).to be_present
-      expect(subject[:'key.pem']).to be_present
-
-      cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem'])
-      expect(cert.not_after).to be < 60.minutes.from_now
-    end
-
-    context 'when the helm application does not have a ca_cert' do
-      before do
-        application.cluster.application_helm.ca_cert = nil
-      end
-
-      it 'does not include cert files' do
-        expect(subject[:'ca.pem']).not_to be_present
-        expect(subject[:'cert.pem']).not_to be_present
-        expect(subject[:'key.pem']).not_to be_present
-      end
+    it 'uses values from #files, except for values.yaml' do
+      allow(application).to receive(:files).and_return({
+        'values.yaml': 'some value specific to files',
+        'file_a.txt': 'file_a',
+        'file_b.txt': 'file_b'
+      })
+
+      expect(subject.except(:'values.yaml')).to eq({
+        'file_a.txt': 'file_a',
+        'file_b.txt': 'file_b'
+      })
     end
   end
 

spec/support/shared_examples/models/cluster_application_helm_cert_shared_examples.rb

@@ -28,22 +28,46 @@ RSpec.shared_examples 'cluster application helm specs' do |application_name|
   describe '#files' do
     subject { application.files }
 
-    context 'when the helm application does not have a ca_cert' do
+    context 'managed_apps_local_tiller feature flag is disabled' do
       before do
-        application.cluster.application_helm.ca_cert = nil
+        stub_feature_flags(managed_apps_local_tiller: false)
       end
 
-      it 'does not include cert files when there is no ca_cert entry' do
-        expect(subject).not_to include(:'ca.pem', :'cert.pem', :'key.pem')
+      context 'when the helm application does not have a ca_cert' do
+        before do
+          application.cluster.application_helm.ca_cert = nil
+        end
+
+        it 'does not include cert files when there is no ca_cert entry' do
+          expect(subject).not_to include(:'ca.pem', :'cert.pem', :'key.pem')
+        end
+      end
+
+      it 'includes cert files when there is a ca_cert entry' do
+        expect(subject).to include(:'ca.pem', :'cert.pem', :'key.pem')
+        expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert)
+
+        cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem'])
+        expect(cert.not_after).to be < 60.minutes.from_now
       end
     end
 
-    it 'includes cert files when there is a ca_cert entry' do
-      expect(subject).to include(:'ca.pem', :'cert.pem', :'key.pem')
-      expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert)
+    context 'managed_apps_local_tiller feature flag is enabled' do
+      before do
+        stub_feature_flags(managed_apps_local_tiller: true)
+      end
+
+      it 'does not include cert files' do
+        expect(subject).not_to include(:'ca.pem', :'cert.pem', :'key.pem')
+      end
+
+      context 'when cluster does not have helm installed' do
+        let(:application) { create(application_name, :no_helm_installed) }
 
-      cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem'])
-      expect(cert.not_after).to be < 60.minutes.from_now
+        it 'does not include cert files' do
+          expect(subject).not_to include(:'ca.pem', :'cert.pem', :'key.pem')
+        end
+      end
     end
   end
 end

spec/support/shared_examples/models/cluster_application_status_shared_examples.rb

@@ -48,14 +48,44 @@ RSpec.shared_examples 'cluster application status specs' do |application_name|
         expect(subject).to be_installed
       end
 
-      it 'updates helm version' do
-        subject.cluster.application_helm.update!(version: '1.2.3')
+      context 'managed_apps_local_tiller feature flag disabled' do
+        before do
+          stub_feature_flags(managed_apps_local_tiller: false)
+        end
 
-        subject.make_installed!
+        it 'updates helm version' do
+          subject.cluster.application_helm.update!(version: '1.2.3')
 
-        subject.cluster.application_helm.reload
+          subject.make_installed!
 
-        expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION)
+          subject.cluster.application_helm.reload
+
+          expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION)
+        end
+      end
+
+      context 'managed_apps_local_tiller feature flag enabled' do
+        before do
+          stub_feature_flags(managed_apps_local_tiller: true)
+        end
+
+        it 'does not update the helm version' do
+          subject.cluster.application_helm.update!(version: '1.2.3')
+
+          expect do
+            subject.make_installed!
+
+            subject.cluster.application_helm.reload
+          end.not_to change { subject.cluster.application_helm.version }
+        end
+
+        context 'the cluster has no helm installed' do
+          subject { create(application_name, :installing, :no_helm_installed) }
+
+          it 'runs without errors' do
+            expect { subject.make_installed! }.not_to raise_error
+          end
+        end
       end
 
       it 'sets the correct version of the application' do
@@ -77,14 +107,44 @@ RSpec.shared_examples 'cluster application status specs' do |application_name|
           expect(subject).to be_updated
         end
 
-        it 'updates helm version' do
-          subject.cluster.application_helm.update!(version: '1.2.3')
+        context 'managed_apps_local_tiller feature flag disabled' do
+          before do
+            stub_feature_flags(managed_apps_local_tiller: false)
+          end
 
-          subject.make_installed!
+          it 'updates helm version' do
+            subject.cluster.application_helm.update!(version: '1.2.3')
 
-          subject.cluster.application_helm.reload
+            subject.make_installed!
 
-          expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION)
+            subject.cluster.application_helm.reload
+
+            expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION)
+          end
+        end
+
+        context 'managed_apps_local_tiller feature flag enabled' do
+          before do
+            stub_feature_flags(managed_apps_local_tiller: true)
+          end
+
+          it 'does not update the helm version' do
+            subject.cluster.application_helm.update!(version: '1.2.3')
+
+            expect do
+              subject.make_installed!
+
+              subject.cluster.application_helm.reload
+            end.not_to change { subject.cluster.application_helm.version }
+          end
+
+          context 'the cluster has no helm installed' do
+            subject { create(application_name, :updating, :no_helm_installed) }
+
+            it 'runs without errors' do
+              expect { subject.make_installed! }.not_to raise_error
+            end
+          end
         end
 
         it 'updates the version of the application' do