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?`).

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

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
 

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

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
 

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

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

lib/gitlab/kubernetes/config_map.rb

 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
         {

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
 

lib/gitlab/kubernetes/helm/base_command.rb

 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

lib/gitlab/kubernetes/helm/init_command.rb

 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

lib/gitlab/kubernetes/helm/install_command.rb

 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

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 } }
               }
             }
           ]

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

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

spec/lib/gitlab/kubernetes/helm/base_command_spec.rb

 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

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

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 }
 

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

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

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

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

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