diff options
author | Mayra Cabrera <mcabrera@gitlab.com> | 2018-08-07 12:39:38 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2018-08-07 12:39:38 +0000 |
commit | fc134096370c94bc1312060c42ed69b2665f0f95 (patch) | |
tree | 7e2a8764e590ae7128058cad67165f8ff1c66722 | |
parent | b3deca7a2606a6b2cef464ed08417be4ffb0cb6b (diff) | |
download | gitlab-ce-fc134096370c94bc1312060c42ed69b2665f0f95.tar.gz |
Resolve "Mutual SSL Auth For Helm TIller"
34 files changed, 538 insertions, 178 deletions
diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 61df6174c86..55bbf7cae7e 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -1,15 +1,28 @@ # frozen_string_literal: true +require 'openssl' + module Clusters module Applications class Helm < ActiveRecord::Base self.table_name = 'clusters_applications_helm' + attr_encrypted :ca_key, + mode: :per_attribute_iv, + key: Settings.attr_encrypted_db_key_base_truncated, + algorithm: 'aes-256-cbc' + include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus default_value_for :version, Gitlab::Kubernetes::Helm::HELM_VERSION + before_create :create_keys_and_certs + + def issue_client_cert + ca_cert_obj.issue + end + def set_initial_status return unless not_installable? @@ -17,7 +30,41 @@ module Clusters end def install_command - Gitlab::Kubernetes::Helm::InitCommand.new(name) + Gitlab::Kubernetes::Helm::InitCommand.new( + name: name, + files: files + ) + end + + def has_ssl? + ca_key.present? && ca_cert.present? + end + + private + + def files + { + 'ca.pem': ca_cert, + 'cert.pem': tiller_cert.cert_string, + 'key.pem': tiller_cert.key_string + } + end + + def create_keys_and_certs + ca_cert = Gitlab::Kubernetes::Helm::Certificate.generate_root + self.ca_key = ca_cert.key_string + self.ca_cert = ca_cert.cert_string + end + + def tiller_cert + @tiller_cert ||= ca_cert_obj.issue(expires_in: Gitlab::Kubernetes::Helm::Certificate::INFINITE_EXPIRY) + end + + def ca_cert_obj + return unless has_ssl? + + Gitlab::Kubernetes::Helm::Certificate + .from_strings(ca_key, ca_cert) end end end diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 2440efe76ab..93f654e0638 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -37,10 +37,10 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, version: VERSION, chart: chart, - values: values + files: files ) end diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb index 33d54ba86fe..ef1c76c03bd 100644 --- a/app/models/clusters/applications/jupyter.rb +++ b/app/models/clusters/applications/jupyter.rb @@ -38,10 +38,10 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, version: VERSION, 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 ccb415b3fe2..88399dbbb95 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -46,10 +46,10 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, version: VERSION, chart: chart, - values: values + files: files ) end diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index 426aed91089..bde255723c8 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -31,10 +31,10 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, version: VERSION, 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 14e004b9a57..52498f123ff 100644 --- a/app/models/clusters/concerns/application_data.rb +++ b/app/models/clusters/concerns/application_data.rb @@ -14,8 +14,34 @@ module Clusters File.read(chart_values_file) end + def files + @files ||= begin + files = { 'values.yaml': values } + + files.merge!(certificate_files) if cluster.application_helm.has_ssl? + + files + end + end + private + def certificate_files + { + 'ca.pem': ca_cert, + 'cert.pem': helm_cert.cert_string, + 'key.pem': helm_cert.key_string + } + end + + def ca_cert + cluster.application_helm.ca_cert + end + + def helm_cert + @helm_cert ||= cluster.application_helm.issue_client_cert + end + def chart_values_file "#{Rails.root}/vendor/#{name}/values.yaml" end diff --git a/changelogs/unreleased/48098-mutual-auth-cluster-applications.yml b/changelogs/unreleased/48098-mutual-auth-cluster-applications.yml new file mode 100644 index 00000000000..43125ef25c4 --- /dev/null +++ b/changelogs/unreleased/48098-mutual-auth-cluster-applications.yml @@ -0,0 +1,6 @@ +--- +title: Ensure installed Helm Tiller For GitLab Managed Apps Is protected by mutual + auth +merge_request: 20928 +author: +type: changed diff --git a/db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb b/db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb new file mode 100644 index 00000000000..57cea18abcd --- /dev/null +++ b/db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true +class AddColumnsForHelmTillerCertificates < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :clusters_applications_helm, :encrypted_ca_key, :text + add_column :clusters_applications_helm, :encrypted_ca_key_iv, :text + add_column :clusters_applications_helm, :ca_cert, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 709c7a9c121..30b8147a474 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -637,6 +637,9 @@ ActiveRecord::Schema.define(version: 20180726172057) do t.integer "status", null: false t.string "version", null: false t.text "status_reason" + t.text "encrypted_ca_key" + t.text "encrypted_ca_key_iv" + t.text "ca_cert" end create_table "clusters_applications_ingress", force: :cascade do |t| 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..afcfd109de0 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,16 +18,32 @@ 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 file_names + files.keys + end + + def name + raise "Not implemented" + end + + def files + raise "Not implemented" + end + private + def files_dir + "/data/helm/#{name}/config" + end + def namespace Gitlab::Kubernetes::Helm::NAMESPACE end diff --git a/lib/gitlab/kubernetes/helm/certificate.rb b/lib/gitlab/kubernetes/helm/certificate.rb new file mode 100644 index 00000000000..598714e0874 --- /dev/null +++ b/lib/gitlab/kubernetes/helm/certificate.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true +module Gitlab + module Kubernetes + module Helm + class Certificate + INFINITE_EXPIRY = 1000.years + SHORT_EXPIRY = 30.minutes + + attr_reader :key, :cert + + def key_string + @key.to_s + end + + def cert_string + @cert.to_pem + end + + def self.from_strings(key_string, cert_string) + key = OpenSSL::PKey::RSA.new(key_string) + cert = OpenSSL::X509::Certificate.new(cert_string) + new(key, cert) + end + + def self.generate_root + _issue(signed_by: nil, expires_in: INFINITE_EXPIRY, certificate_authority: true) + end + + def issue(expires_in: SHORT_EXPIRY) + self.class._issue(signed_by: self, expires_in: expires_in, certificate_authority: false) + end + + private + + def self._issue(signed_by:, expires_in:, certificate_authority:) + key = OpenSSL::PKey::RSA.new(4096) + public_key = key.public_key + + subject = OpenSSL::X509::Name.parse("/C=US") + + cert = OpenSSL::X509::Certificate.new + cert.subject = subject + + cert.issuer = signed_by&.cert&.subject || subject + + cert.not_before = Time.now + cert.not_after = expires_in.from_now + cert.public_key = public_key + cert.serial = 0x0 + cert.version = 2 + + if certificate_authority + extension_factory = OpenSSL::X509::ExtensionFactory.new + extension_factory.subject_certificate = cert + extension_factory.issuer_certificate = cert + cert.add_extension(extension_factory.create_extension('subjectKeyIdentifier', 'hash')) + cert.add_extension(extension_factory.create_extension('basicConstraints', 'CA:TRUE', true)) + cert.add_extension(extension_factory.create_extension('keyUsage', 'cRLSign,keyCertSign', true)) + end + + cert.sign(signed_by&.key || key, OpenSSL::Digest::SHA256.new) + + new(key, cert) + end + + def initialize(key, cert) + @key = key + @cert = cert + end + end + end + end +end diff --git a/lib/gitlab/kubernetes/helm/init_command.rb b/lib/gitlab/kubernetes/helm/init_command.rb index a02e64561f6..a4546509515 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 @@ -11,7 +20,12 @@ module Gitlab private def init_helm_command - "helm init >/dev/null" + tls_flags = "--tiller-tls" \ + " --tiller-tls-verify --tls-ca-cert #{files_dir}/ca.pem" \ + " --tiller-tls-cert #{files_dir}/cert.pem" \ + " --tiller-tls-key #{files_dir}/key.pem" + + "helm init #{tls_flags} >/dev/null" end end end diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb index d2133a6d65b..9672f80687e 100644 --- a/lib/gitlab/kubernetes/helm/install_command.rb +++ b/lib/gitlab/kubernetes/helm/install_command.rb @@ -1,14 +1,16 @@ 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, :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 +22,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 @@ -39,14 +33,25 @@ module Gitlab end def script_command - <<~HEREDOC - helm install #{chart} --name #{name}#{optional_version_flag} --namespace #{Gitlab::Kubernetes::Helm::NAMESPACE} -f /data/helm/#{name}/config/values.yaml >/dev/null - HEREDOC + init_flags = "--name #{name}#{optional_tls_flags}#{optional_version_flag}" \ + " --namespace #{Gitlab::Kubernetes::Helm::NAMESPACE}" \ + " -f /data/helm/#{name}/config/values.yaml" + + "helm install #{chart} #{init_flags} >/dev/null\n" end def optional_version_flag " --version #{version}" if version 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/pod.rb b/lib/gitlab/kubernetes/helm/pod.rb index 1e12299eefd..6e5d3388405 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.file_names.map { |name| { key: name, path: name } } } } ] diff --git a/qa/qa/factory/resource/kubernetes_cluster.rb b/qa/qa/factory/resource/kubernetes_cluster.rb index 1c9e5f94b22..ef2ea72b170 100644 --- a/qa/qa/factory/resource/kubernetes_cluster.rb +++ b/qa/qa/factory/resource/kubernetes_cluster.rb @@ -44,10 +44,11 @@ module QA page.await_installed(:helm) page.install!(:ingress) if @install_ingress - page.await_installed(:ingress) if @install_ingress page.install!(:prometheus) if @install_prometheus - page.await_installed(:prometheus) if @install_prometheus page.install!(:runner) if @install_runner + + page.await_installed(:ingress) if @install_ingress + page.await_installed(:prometheus) if @install_prometheus page.await_installed(:runner) if @install_runner end end diff --git a/qa/qa/page/project/operations/kubernetes/show.rb b/qa/qa/page/project/operations/kubernetes/show.rb index 4923304133e..e831edeb89e 100644 --- a/qa/qa/page/project/operations/kubernetes/show.rb +++ b/qa/qa/page/project/operations/kubernetes/show.rb @@ -16,6 +16,7 @@ module QA def install!(application_name) within(".js-cluster-application-row-#{application_name}") do + page.has_button?('Install', wait: 30) click_on 'Install' end end diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb index 3e4277e4ba6..7c4a440b9a9 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -32,11 +32,21 @@ FactoryBot.define do updated_at ClusterWaitForAppInstallationWorker::TIMEOUT.ago end - factory :clusters_applications_ingress, class: Clusters::Applications::Ingress - factory :clusters_applications_prometheus, class: Clusters::Applications::Prometheus - factory :clusters_applications_runner, class: Clusters::Applications::Runner + factory :clusters_applications_ingress, class: Clusters::Applications::Ingress do + cluster factory: %i(cluster with_installed_helm provided_by_gcp) + end + + factory :clusters_applications_prometheus, class: Clusters::Applications::Prometheus do + cluster factory: %i(cluster with_installed_helm provided_by_gcp) + end + + factory :clusters_applications_runner, class: Clusters::Applications::Runner do + cluster factory: %i(cluster with_installed_helm provided_by_gcp) + 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) end end end diff --git a/spec/factories/clusters/clusters.rb b/spec/factories/clusters/clusters.rb index 0430762c1ff..bbeba8ce8b9 100644 --- a/spec/factories/clusters/clusters.rb +++ b/spec/factories/clusters/clusters.rb @@ -36,5 +36,9 @@ FactoryBot.define do trait :production_environment do sequence(:environment_scope) { |n| "production#{n}/*" } end + + trait :with_installed_helm do + application_helm factory: %i(clusters_applications_helm installed) + end end end diff --git a/spec/features/projects/clusters/applications_spec.rb b/spec/features/projects/clusters/applications_spec.rb index a65ca662350..71d715237f5 100644 --- a/spec/features/projects/clusters/applications_spec.rb +++ b/spec/features/projects/clusters/applications_spec.rb @@ -46,12 +46,14 @@ describe 'Clusters Applications', :js do end end - it 'he sees status transition' do + it 'they see status transition' do page.within('.js-cluster-application-row-helm') do # FE sends request and gets the response, then the buttons is "Install" expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') + wait_until_helm_created! + Clusters::Cluster.last.application_helm.make_installing! # FE starts polling and update the buttons to "Installing" @@ -83,7 +85,7 @@ describe 'Clusters Applications', :js do end end - it 'he sees status transition' do + it 'they see status transition' do page.within('.js-cluster-application-row-ingress') do # FE sends request and gets the response, then the buttons is "Install" expect(page).to have_css('.js-cluster-application-install-button[disabled]') @@ -116,4 +118,14 @@ describe 'Clusters Applications', :js do end end end + + def wait_until_helm_created! + retries = 0 + + while Clusters::Cluster.last.application_helm.nil? + raise "Timed out waiting for helm application to be created in DB" if (retries += 1) > 3 + + sleep(1) + end + end end 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..d50616e95e8 100644 --- a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb @@ -2,7 +2,25 @@ require 'spec_helper' describe Gitlab::Kubernetes::Helm::BaseCommand do let(:application) { create(:clusters_applications_helm) } - let(:base_command) { described_class.new(application.name) } + let(:test_class) do + Class.new do + include Gitlab::Kubernetes::Helm::BaseCommand + + def name + "test-class-name" + end + + def files + { + some: 'value' + } + end + end + end + + let(:base_command) do + test_class.new + end subject { base_command } @@ -18,15 +36,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/certificate_spec.rb b/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb new file mode 100644 index 00000000000..167bee22fc3 --- /dev/null +++ b/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::Kubernetes::Helm::Certificate do + describe '.generate_root' do + subject { described_class.generate_root } + + it 'should generate a root CA that expires a long way in the future' do + expect(subject.cert.not_after).to be > 999.years.from_now + end + end + + describe '#issue' do + subject { described_class.generate_root.issue } + + it 'should generate a cert that expires soon' do + expect(subject.cert.not_after).to be < 60.minutes.from_now + end + + context 'passing in INFINITE_EXPIRY' do + subject { described_class.generate_root.issue(expires_in: described_class::INFINITE_EXPIRY) } + + it 'should generate a cert that expires a long way in the future' do + expect(subject.cert.not_after).to be > 999.years.from_now + end + end + 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..dcbc046cf00 100644 --- a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' describe Gitlab::Kubernetes::Helm::InitCommand do let(:application) { create(:clusters_applications_helm) } - let(:commands) { 'helm init >/dev/null' } + let(:commands) { 'helm init --tiller-tls --tiller-tls-verify --tls-ca-cert /data/helm/helm/config/ca.pem --tiller-tls-cert /data/helm/helm/config/cert.pem --tiller-tls-key /data/helm/helm/config/key.pem >/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 cd456a45287..982e2f41043 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -1,83 +1,82 @@ require 'rails_helper' describe Gitlab::Kubernetes::Helm::InstallCommand do - let(:application) { create(:clusters_applications_prometheus) } - let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } - let(:install_command) { application.install_command } + let(:files) { { 'ca.pem': 'some file content' } } + let(:repository) { 'https://repository.example.com' } + let(:version) { '1.2.3' } + + let(:install_command) do + described_class.new( + name: 'app-name', + chart: 'chart-name', + files: files, + version: version, repository: repository + ) + end subject { install_command } - context 'for ingress' do - let(:application) { create(:clusters_applications_ingress) } - - it_behaves_like 'helm commands' do - let(:commands) do - <<~EOS - helm init --client-only >/dev/null - helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null - EOS - end + it_behaves_like 'helm commands' do + let(:commands) do + <<~EOS + helm init --client-only >/dev/null + helm repo add app-name https://repository.example.com + helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + EOS end end - context 'for prometheus' do - let(:application) { create(:clusters_applications_prometheus) } + context 'when there is no repository' do + let(:repository) { nil } it_behaves_like 'helm commands' do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null EOS end end end - context 'for runner' do - let(:ci_runner) { create(:ci_runner) } - let(:application) { create(:clusters_applications_runner, runner: ci_runner) } + context 'when there is no ca.pem file' do + let(:files) { { 'file.txt': 'some content' } } it_behaves_like 'helm commands' do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm repo add #{application.name} #{application.repository} - helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm repo add app-name https://repository.example.com + helm install chart-name --name app-name --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null EOS end end end - context 'for jupyter' do - let(:application) { create(:clusters_applications_jupyter) } + context 'when there is no version' do + let(:version) { nil } it_behaves_like 'helm commands' do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm repo add #{application.name} #{application.repository} - helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm repo add app-name https://repository.example.com + helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null EOS end 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 { - name: "values-content-configuration-#{application.name}", - namespace: namespace, - labels: { name: "values-content-configuration-#{application.name}" } + name: "values-content-configuration-app-name", + namespace: 'gitlab-managed-apps', + labels: { name: "values-content-configuration-app-name" } } end - let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) } + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: 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..ec64193c0b2 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -2,14 +2,13 @@ require 'rails_helper' describe Gitlab::Kubernetes::Helm::Pod do describe '#generate' do - let(:cluster) { create(:cluster) } - let(:app) { create(:clusters_applications_prometheus, cluster: cluster) } + let(:app) { create(:clusters_applications_prometheus) } let(:command) { app.install_command } let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } 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 +40,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 +55,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/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 0eb1e3876e2..e5b2bdc8a4e 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -6,13 +6,24 @@ describe Clusters::Applications::Helm do describe '.installed' do subject { described_class.installed } - let!(:cluster) { create(:clusters_applications_helm, :installed) } + let!(:installed_cluster) { create(:clusters_applications_helm, :installed) } before do create(:clusters_applications_helm, :errored) end - it { is_expected.to contain_exactly(cluster) } + it { is_expected.to contain_exactly(installed_cluster) } + end + + describe '#issue_client_cert' do + let(:application) { create(:clusters_applications_helm) } + subject { application.issue_client_cert } + + it 'returns a new cert' do + is_expected.to be_kind_of(Gitlab::Kubernetes::Helm::Certificate) + expect(subject.cert_string).not_to eq(application.ca_cert) + expect(subject.key_string).not_to eq(application.ca_key) + end end describe '#install_command' do @@ -25,5 +36,16 @@ describe Clusters::Applications::Helm do it 'should be initialized with 1 arguments' do expect(subject.name).to eq('helm') end + + it 'should have 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 end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index d378248d5d6..21f75ced8c3 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -88,7 +88,7 @@ describe Clusters::Applications::Ingress do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') expect(subject.version).to eq('0.23.0') - expect(subject.values).to eq(ingress.values) + expect(subject.files).to eq(ingress.files) end context 'application failed to install previously' do @@ -100,14 +100,40 @@ describe Clusters::Applications::Ingress do end end - describe '#values' do - subject { ingress.values } + describe '#files' do + let(:application) { ingress } + let(:values) { subject[:'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') + subject { application.files } + + 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 + + context 'when the helm application does not have a ca_cert' do + before do + application.cluster.application_helm.ca_cert = nil + end + + it 'should 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 + end + + it 'should include 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 end end diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index e0d57ac65f7..027b732681b 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -52,7 +52,7 @@ describe Clusters::Applications::Jupyter do expect(subject.chart).to eq('jupyter/jupyterhub') expect(subject.version).to eq('v0.6') 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 context 'application failed to install previously' do @@ -64,19 +64,43 @@ describe Clusters::Applications::Jupyter do end end - describe '#values' do - let(:jupyter) { create(:clusters_applications_jupyter) } + describe '#files' do + let(:application) { create(:clusters_applications_jupyter) } + let(:values) { subject[:'values.yaml'] } - subject { jupyter.values } + subject { application.files } + + it 'should include 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 'should 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 + end 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: '?#{application.oauth_application.uid}/) + expect(values).to match(/callbackUrl: '?#{application.callback_url}/) end end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 3812c65b3b6..7454be3ab2f 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -167,7 +167,7 @@ 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 context 'application failed to install previously' do @@ -179,17 +179,41 @@ describe Clusters::Applications::Prometheus do end end - describe '#values' do - let(:prometheus) { create(:clusters_applications_prometheus) } + describe '#files' do + let(:application) { create(:clusters_applications_prometheus) } + let(:values) { subject[:'values.yaml'] } + + subject { application.files } + + it 'should include 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 - subject { prometheus.values } + context 'when the helm application does not have a ca_cert' do + before do + application.cluster.application_helm.ca_cert = nil + end + + it 'should 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 + end 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 526300755b5..d84f125e246 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -47,7 +47,7 @@ describe Clusters::Applications::Runner do expect(subject.chart).to eq('runner/gitlab-runner') expect(subject.version).to eq('0.1.31') 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 context 'application failed to install previously' do @@ -59,27 +59,51 @@ describe Clusters::Applications::Runner do end end - describe '#values' do - let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) } + describe '#files' do + let(:application) { create(:clusters_applications_runner, runner: ci_runner) } + let(:values) { subject[:'values.yaml'] } + + subject { application.files } + + it 'should include 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 - subject { gitlab_runner.values } + context 'when the helm application does not have a ca_cert' do + before do + application.cluster.application_helm.ca_cert = nil + end + + it 'should 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 + end 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 let(:project) { create(:project) } - let(:cluster) { create(:cluster, projects: [project]) } - let(:gitlab_runner) { create(:clusters_applications_runner, cluster: cluster) } + let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) } + let(:application) { create(:clusters_applications_runner, cluster: cluster) } it 'creates a runner' do expect do @@ -88,18 +112,18 @@ 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: '?#{application.reload.runner.token}/) end it 'assigns the new runner to runner' do subject - expect(gitlab_runner.reload.runner).to be_project_type + expect(application.reload.runner).to be_project_type end end context 'with duplicated values on vendor/runner/values.yaml' do - let(:values) do + let(:stub_values) do { "concurrent" => 4, "checkInterval" => 3, @@ -118,11 +142,11 @@ describe Clusters::Applications::Runner do end before do - allow(gitlab_runner).to receive(:chart_values).and_return(values) + allow(application).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: '?#{application.privileged}/) end end end diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb index 93199964a0e..a744ec30b65 100644 --- a/spec/services/clusters/applications/install_service_spec.rb +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -47,7 +47,7 @@ describe Clusters::Applications::InstallService do end context 'when application cannot be persisted' do - let(:application) { build(:clusters_applications_helm, :scheduled) } + let(:application) { create(:clusters_applications_helm, :scheduled) } it 'make the application errored' do expect(application).to receive(:make_installing!).once.and_raise(ActiveRecord::RecordInvalid) |