Refactor Cluster Application classes to pass through a has of config files

This is refactoring in the lead up to passing mutual TLS certs for helm applications. As such we expect all applications to need config files so we can remove the logic about which applications need and do not need this (ie `#config_map?`).

22 files changed, 122 insertions, 128 deletions

diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb
index 58de3448577..06d85a69b29 100644
--- a/app/models/clusters/applications/helm.rb
+++ b/app/models/clusters/applications/helm.rb
@@ -15,7 +15,10 @@ module Clusters
       end
 
       def install_command
-        Gitlab::Kubernetes::Helm::InitCommand.new(name)
+        Gitlab::Kubernetes::Helm::InitCommand.new(
+          name: name,
+          files: {}
+        )
       end
     end
   end
diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb
index 27fc3b85465..64810812531 100644
--- a/app/models/clusters/applications/ingress.rb
+++ b/app/models/clusters/applications/ingress.rb
@@ -32,9 +32,9 @@ module Clusters
 
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
-          name,
+          name: name,
           chart: chart,
-          values: values
+          files: files
         )
       end
 
diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb
index 975d434e1a4..cff5a423acb 100644
--- a/app/models/clusters/applications/jupyter.rb
+++ b/app/models/clusters/applications/jupyter.rb
@@ -35,9 +35,9 @@ module Clusters
 
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
-          name,
+          name: name,
           chart: chart,
-          values: values,
+          files: files,
           repository: repository
         )
       end
diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb
index ea6ec4d6b03..22815cc1219 100644
--- a/app/models/clusters/applications/prometheus.rb
+++ b/app/models/clusters/applications/prometheus.rb
@@ -43,10 +43,10 @@ module Clusters
 
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
-          name,
+          name: name,
           chart: chart,
           version: version,
-          values: values
+          files: files
         )
       end
 
diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb
index e6f795f3e0b..4c894b5376d 100644
--- a/app/models/clusters/applications/runner.rb
+++ b/app/models/clusters/applications/runner.rb
@@ -28,9 +28,9 @@ module Clusters
 
       def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
-          name,
+          name: name,
           chart: chart,
-          values: values,
+          files: files,
           repository: repository
         )
       end
diff --git a/app/models/clusters/concerns/application_data.rb b/app/models/clusters/concerns/application_data.rb
index 96ac757e99e..215a299dd03 100644
--- a/app/models/clusters/concerns/application_data.rb
+++ b/app/models/clusters/concerns/application_data.rb
@@ -12,6 +12,12 @@ module Clusters
           File.read(chart_values_file)
         end
 
+        def files
+          {
+            'values.yaml': values
+          }
+        end
+
         private
 
         def chart_values_file
diff --git a/lib/gitlab/kubernetes/config_map.rb b/lib/gitlab/kubernetes/config_map.rb
index 8a8a59a9cd4..9e55dae137c 100644
--- a/lib/gitlab/kubernetes/config_map.rb
+++ b/lib/gitlab/kubernetes/config_map.rb
@@ -1,15 +1,15 @@
 module Gitlab
   module Kubernetes
     class ConfigMap
-      def initialize(name, values = "")
+      def initialize(name, files)
         @name = name
-        @values = values
+        @files = files
       end
 
       def generate
         resource = ::Kubeclient::Resource.new
         resource.metadata = metadata
-        resource.data = { values: values }
+        resource.data = files
         resource
       end
 
@@ -19,7 +19,7 @@ module Gitlab
 
       private
 
-      attr_reader :name, :values
+      attr_reader :name, :files
 
       def metadata
         {
diff --git a/lib/gitlab/kubernetes/helm/api.rb b/lib/gitlab/kubernetes/helm/api.rb
index c4de9a398cc..d65374cc23b 100644
--- a/lib/gitlab/kubernetes/helm/api.rb
+++ b/lib/gitlab/kubernetes/helm/api.rb
@@ -9,7 +9,7 @@ module Gitlab
 
         def install(command)
           namespace.ensure_exists!
-          create_config_map(command) if command.config_map?
+          create_config_map(command)
           kubeclient.create_pod(command.pod_resource)
         end
 
diff --git a/lib/gitlab/kubernetes/helm/base_command.rb b/lib/gitlab/kubernetes/helm/base_command.rb
index f9ebe53d6af..8decddcd92f 100644
--- a/lib/gitlab/kubernetes/helm/base_command.rb
+++ b/lib/gitlab/kubernetes/helm/base_command.rb
@@ -1,13 +1,7 @@
 module Gitlab
   module Kubernetes
     module Helm
-      class BaseCommand
-        attr_reader :name
-
-        def initialize(name)
-          @name = name
-        end
-
+      module BaseCommand
         def pod_resource
           Gitlab::Kubernetes::Helm::Pod.new(self, namespace).generate
         end
@@ -24,14 +18,22 @@ module Gitlab
           HEREDOC
         end
 
-        def config_map?
-          false
-        end
-
         def pod_name
           "install-#{name}"
         end
 
+        def config_map_resource
+          Gitlab::Kubernetes::ConfigMap.new(name, files).generate
+        end
+
+        def name
+          raise "Not implemented"
+        end
+
+        def files
+          raise "Not implemented"
+        end
+
         private
 
         def namespace
diff --git a/lib/gitlab/kubernetes/helm/init_command.rb b/lib/gitlab/kubernetes/helm/init_command.rb
index a02e64561f6..2f2b3e930ae 100644
--- a/lib/gitlab/kubernetes/helm/init_command.rb
+++ b/lib/gitlab/kubernetes/helm/init_command.rb
@@ -1,7 +1,16 @@
 module Gitlab
   module Kubernetes
     module Helm
-      class InitCommand < BaseCommand
+      class InitCommand
+        include BaseCommand
+
+        attr_reader :name, :files
+
+        def initialize(name:, files:)
+          @name = name
+          @files = files
+        end
+
         def generate_script
           super + [
             init_helm_command
diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb
index d2133a6d65b..5008724f17d 100644
--- a/lib/gitlab/kubernetes/helm/install_command.rb
+++ b/lib/gitlab/kubernetes/helm/install_command.rb
@@ -1,14 +1,17 @@
 module Gitlab
   module Kubernetes
     module Helm
-      class InstallCommand < BaseCommand
-        attr_reader :name, :chart, :version, :repository, :values
+      class InstallCommand
+        include BaseCommand
 
-        def initialize(name, chart:, values:, version: nil, repository: nil)
+        attr_reader :name, :files
+        attr_reader :chart, :version, :repository
+
+        def initialize(name:, chart:, files:, version: nil, repository: nil)
           @name = name
           @chart = chart
           @version = version
-          @values = values
+          @files = files
           @repository = repository
         end
 
@@ -20,14 +23,6 @@ module Gitlab
           ].compact.join("\n")
         end
 
-        def config_map?
-          true
-        end
-
-        def config_map_resource
-          Gitlab::Kubernetes::ConfigMap.new(name, values).generate
-        end
-
         private
 
         def init_command
diff --git a/lib/gitlab/kubernetes/helm/pod.rb b/lib/gitlab/kubernetes/helm/pod.rb
index 1e12299eefd..862664ff543 100644
--- a/lib/gitlab/kubernetes/helm/pod.rb
+++ b/lib/gitlab/kubernetes/helm/pod.rb
@@ -10,10 +10,8 @@ module Gitlab
         def generate
           spec = { containers: [container_specification], restartPolicy: 'Never' }
 
-          if command.config_map?
-            spec[:volumes] = volumes_specification
-            spec[:containers][0][:volumeMounts] = volume_mounts_specification
-          end
+          spec[:volumes] = volumes_specification
+          spec[:containers][0][:volumeMounts] = volume_mounts_specification
 
           ::Kubeclient::Resource.new(metadata: metadata, spec: spec)
         end
@@ -61,7 +59,7 @@ module Gitlab
               name: 'configuration-volume',
               configMap: {
                 name: "values-content-configuration-#{command.name}",
-                items: [{ key: 'values', path: 'values.yaml' }]
+                items: command.files.map { |name, _| { key: name, path: name } }
               }
             }
           ]
diff --git a/spec/lib/gitlab/kubernetes/config_map_spec.rb b/spec/lib/gitlab/kubernetes/config_map_spec.rb
index e253b291277..fe65d03875f 100644
--- a/spec/lib/gitlab/kubernetes/config_map_spec.rb
+++ b/spec/lib/gitlab/kubernetes/config_map_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Gitlab::Kubernetes::ConfigMap do
   let(:kubeclient) { double('kubernetes client') }
   let(:application) { create(:clusters_applications_prometheus) }
-  let(:config_map) { described_class.new(application.name, application.values) }
+  let(:config_map) { described_class.new(application.name, application.files) }
   let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE }
 
   let(:metadata) do
@@ -15,7 +15,7 @@ describe Gitlab::Kubernetes::ConfigMap do
   end
 
   describe '#generate' do
-    let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) }
+    let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: application.files) }
     subject { config_map.generate }
 
     it 'should build a Kubeclient Resource' do
diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb
index 6e9b4ca0869..341f71a3e49 100644
--- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb
@@ -39,7 +39,7 @@ describe Gitlab::Kubernetes::Helm::Api do
     end
 
     context 'with a ConfigMap' do
-      let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.values).generate }
+      let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.files).generate }
 
       it 'creates a ConfigMap on kubeclient' do
         expect(client).to receive(:create_config_map).with(resource).once
diff --git a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb
index 7be8be54d5e..3dcb48f4869 100644
--- a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb
@@ -1,9 +1,21 @@
 require 'spec_helper'
 
+class TestClass
+  include Gitlab::Kubernetes::Helm::BaseCommand
+  def name
+    "test-class-name"
+  end
+
+  def files
+    {
+      some: 'value'
+    }
+  end
+end
+
 describe Gitlab::Kubernetes::Helm::BaseCommand do
   let(:application) { create(:clusters_applications_helm) }
-  let(:base_command) { described_class.new(application.name) }
-
+  let(:base_command) { TestClass.new }
   subject { base_command }
 
   it_behaves_like 'helm commands' do
@@ -18,15 +30,9 @@ describe Gitlab::Kubernetes::Helm::BaseCommand do
     end
   end
 
-  describe '#config_map?' do
-    subject { base_command.config_map? }
-
-    it { is_expected.to be_falsy }
-  end
-
   describe '#pod_name' do
     subject { base_command.pod_name }
 
-    it { is_expected.to eq('install-helm') }
+    it { is_expected.to eq('install-test-class-name') }
   end
 end
diff --git a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb
index 89e36a298f8..7550e23259b 100644
--- a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb
@@ -4,7 +4,7 @@ describe Gitlab::Kubernetes::Helm::InitCommand do
   let(:application) { create(:clusters_applications_helm) }
   let(:commands) { 'helm init >/dev/null' }
 
-  subject { described_class.new(application.name) }
+  subject { described_class.new(name: application.name, files: {}) }
 
   it_behaves_like 'helm commands'
 end
diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb
index 25c6fa3b9a3..1e8407c8dc3 100644
--- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb
@@ -62,12 +62,6 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do
     end
   end
 
-  describe '#config_map?' do
-    subject { install_command.config_map? }
-
-    it { is_expected.to be_truthy }
-  end
-
   describe '#config_map_resource' do
     let(:metadata) do
       {
@@ -77,7 +71,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do
       }
     end
 
-    let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) }
+    let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: application.files) }
 
     subject { install_command.config_map_resource }
 
diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb
index 43adc80d576..c25978e96da 100644
--- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb
@@ -9,7 +9,7 @@ describe Gitlab::Kubernetes::Helm::Pod do
 
     subject { described_class.new(command, namespace) }
 
-    shared_examples 'helm pod' do
+    context 'with a command' do
       it 'should generate a Kubeclient::Resource' do
         expect(subject.generate).to be_a_kind_of(Kubeclient::Resource)
       end
@@ -41,10 +41,6 @@ describe Gitlab::Kubernetes::Helm::Pod do
         spec = subject.generate.spec
         expect(spec.restartPolicy).to eq('Never')
       end
-    end
-
-    context 'with a install command' do
-      it_behaves_like 'helm pod'
 
       it 'should include volumes for the container' do
         container = subject.generate.spec.containers.first
@@ -60,24 +56,8 @@ describe Gitlab::Kubernetes::Helm::Pod do
       it 'should mount configMap specification in the volume' do
         volume = subject.generate.spec.volumes.first
         expect(volume.configMap['name']).to eq("values-content-configuration-#{app.name}")
-        expect(volume.configMap['items'].first['key']).to eq('values')
-        expect(volume.configMap['items'].first['path']).to eq('values.yaml')
-      end
-    end
-
-    context 'with a init command' do
-      let(:app) { create(:clusters_applications_helm, cluster: cluster) }
-
-      it_behaves_like 'helm pod'
-
-      it 'should not include volumeMounts inside the container' do
-        container = subject.generate.spec.containers.first
-        expect(container.volumeMounts).to be_nil
-      end
-
-      it 'should not a volume inside the specification' do
-        spec = subject.generate.spec
-        expect(spec.volumes).to be_nil
+        expect(volume.configMap['items'].first['key']).to eq(:'values.yaml')
+        expect(volume.configMap['items'].first['path']).to eq(:'values.yaml')
       end
     end
   end
diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb
index bb5b2ef3a47..fbb3c18319f 100644
--- a/spec/models/clusters/applications/ingress_spec.rb
+++ b/spec/models/clusters/applications/ingress_spec.rb
@@ -74,18 +74,18 @@ describe Clusters::Applications::Ingress do
       expect(subject.name).to eq('ingress')
       expect(subject.chart).to eq('stable/nginx-ingress')
       expect(subject.version).to be_nil
-      expect(subject.values).to eq(ingress.values)
+      expect(subject.files).to eq(ingress.files)
     end
   end
 
-  describe '#values' do
-    subject { ingress.values }
+  describe '#files' do
+    let(:values) { ingress.files[:'values.yaml'] }
 
-    it 'should include ingress valid keys' do
-      is_expected.to include('image')
-      is_expected.to include('repository')
-      is_expected.to include('stats')
-      is_expected.to include('podAnnotations')
+    it 'should include ingress valid keys in values' do
+      expect(values).to include('image')
+      expect(values).to include('repository')
+      expect(values).to include('stats')
+      expect(values).to include('podAnnotations')
     end
   end
 end
diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb
index 65750141e65..0e2847592fc 100644
--- a/spec/models/clusters/applications/jupyter_spec.rb
+++ b/spec/models/clusters/applications/jupyter_spec.rb
@@ -38,23 +38,23 @@ describe Clusters::Applications::Jupyter do
       expect(subject.chart).to eq('jupyter/jupyterhub')
       expect(subject.version).to be_nil
       expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/')
-      expect(subject.values).to eq(jupyter.values)
+      expect(subject.files).to eq(jupyter.files)
     end
   end
 
-  describe '#values' do
+  describe '#files' do
     let(:jupyter) { create(:clusters_applications_jupyter) }
 
-    subject { jupyter.values }
+    let(:values) { jupyter.files[:'values.yaml'] }
 
     it 'should include valid values' do
-      is_expected.to include('ingress')
-      is_expected.to include('hub')
-      is_expected.to include('rbac')
-      is_expected.to include('proxy')
-      is_expected.to include('auth')
-      is_expected.to include("clientId: #{jupyter.oauth_application.uid}")
-      is_expected.to include("callbackUrl: #{jupyter.callback_url}")
+      expect(values).to include('ingress')
+      expect(values).to include('hub')
+      expect(values).to include('rbac')
+      expect(values).to include('proxy')
+      expect(values).to include('auth')
+      expect(values).to match(/clientId: '?#{jupyter.oauth_application.uid}/)
+      expect(values).to match(/callbackUrl: '?#{jupyter.callback_url}/)
     end
   end
 end
diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index e4b61552033..013cb8da22b 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -153,21 +153,21 @@ describe Clusters::Applications::Prometheus do
       expect(command.name).to eq('prometheus')
       expect(command.chart).to eq('stable/prometheus')
       expect(command.version).to eq('6.7.3')
-      expect(command.values).to eq(prometheus.values)
+      expect(command.files).to eq(prometheus.files)
     end
   end
 
-  describe '#values' do
+  describe '#files' do
     let(:prometheus) { create(:clusters_applications_prometheus) }
 
-    subject { prometheus.values }
+    let(:values) { prometheus.files[:'values.yaml'] }
 
     it 'should include prometheus valid values' do
-      is_expected.to include('alertmanager')
-      is_expected.to include('kubeStateMetrics')
-      is_expected.to include('nodeExporter')
-      is_expected.to include('pushgateway')
-      is_expected.to include('serverFiles')
+      expect(values).to include('alertmanager')
+      expect(values).to include('kubeStateMetrics')
+      expect(values).to include('nodeExporter')
+      expect(values).to include('pushgateway')
+      expect(values).to include('serverFiles')
     end
   end
 end
diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb
index b12500d0acd..4ac136a6274 100644
--- a/spec/models/clusters/applications/runner_spec.rb
+++ b/spec/models/clusters/applications/runner_spec.rb
@@ -33,25 +33,26 @@ describe Clusters::Applications::Runner do
       expect(subject.chart).to eq('runner/gitlab-runner')
       expect(subject.version).to be_nil
       expect(subject.repository).to eq('https://charts.gitlab.io')
-      expect(subject.values).to eq(gitlab_runner.values)
+      expect(subject.files).to eq(gitlab_runner.files)
     end
   end
 
-  describe '#values' do
+  describe '#files' do
     let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) }
 
-    subject { gitlab_runner.values }
+    subject { gitlab_runner.files }
+    let(:values) { subject[:'values.yaml'] }
 
     it 'should include runner valid values' do
-      is_expected.to include('concurrent')
-      is_expected.to include('checkInterval')
-      is_expected.to include('rbac')
-      is_expected.to include('runners')
-      is_expected.to include('privileged: true')
-      is_expected.to include('image: ubuntu:16.04')
-      is_expected.to include('resources')
-      is_expected.to include("runnerToken: #{ci_runner.token}")
-      is_expected.to include("gitlabUrl: #{Gitlab::Routing.url_helpers.root_url}")
+      expect(values).to include('concurrent')
+      expect(values).to include('checkInterval')
+      expect(values).to include('rbac')
+      expect(values).to include('runners')
+      expect(values).to include('privileged: true')
+      expect(values).to include('image: ubuntu:16.04')
+      expect(values).to include('resources')
+      expect(values).to match(/runnerToken: '?#{ci_runner.token}/)
+      expect(values).to match(/gitlabUrl: '?#{Gitlab::Routing.url_helpers.root_url}/)
     end
 
     context 'without a runner' do
@@ -66,7 +67,7 @@ describe Clusters::Applications::Runner do
       end
 
       it 'uses the new runner token' do
-        expect(subject).to include("runnerToken: #{gitlab_runner.reload.runner.token}")
+        expect(values).to match(/runnerToken: '?#{gitlab_runner.reload.runner.token}/)
       end
 
       it 'assigns the new runner to runner' do
@@ -77,7 +78,7 @@ describe Clusters::Applications::Runner do
     end
 
     context 'with duplicated values on vendor/runner/values.yaml' do
-      let(:values) do
+      let(:stub_values) do
         {
           "concurrent" => 4,
           "checkInterval" => 3,
@@ -96,11 +97,11 @@ describe Clusters::Applications::Runner do
       end
 
       before do
-        allow(gitlab_runner).to receive(:chart_values).and_return(values)
+        allow(gitlab_runner).to receive(:chart_values).and_return(stub_values)
       end
 
       it 'should overwrite values.yaml' do
-        is_expected.to include("privileged: #{gitlab_runner.privileged}")
+        expect(values).to match(/privileged: '?#{gitlab_runner.privileged}/)
       end
     end
   end