From b21730116ea14fc1ac153478ed058e505bceed17 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Tue, 31 Jul 2018 13:15:18 +0000 Subject: [PATCH] Revert "Merge branch '48098-mutual-auth-cluster-applications' into 'master'" This reverts merge request !20801 --- app/models/clusters/applications/helm.rb | 49 +----------- app/models/clusters/applications/ingress.rb | 4 +- app/models/clusters/applications/jupyter.rb | 4 +- .../clusters/applications/prometheus.rb | 4 +- app/models/clusters/applications/runner.rb | 4 +- .../clusters/concerns/application_data.rb | 26 ------- ...48098-mutual-auth-cluster-applications.yml | 6 -- ...dd_columns_for_helm_tiller_certificates.rb | 11 --- db/schema.rb | 3 - lib/gitlab/kubernetes/config_map.rb | 8 +- lib/gitlab/kubernetes/helm/api.rb | 2 +- lib/gitlab/kubernetes/helm/base_command.rb | 32 +++----- lib/gitlab/kubernetes/helm/certificate.rb | 72 ------------------ lib/gitlab/kubernetes/helm/init_command.rb | 18 +---- lib/gitlab/kubernetes/helm/install_command.rb | 37 ++++----- lib/gitlab/kubernetes/helm/pod.rb | 8 +- qa/qa/factory/resource/kubernetes_cluster.rb | 5 +- .../project/operations/kubernetes/show.rb | 1 - spec/factories/clusters/applications/helm.rb | 16 +--- spec/factories/clusters/clusters.rb | 4 - .../projects/clusters/applications_spec.rb | 16 +--- spec/lib/gitlab/kubernetes/config_map_spec.rb | 4 +- spec/lib/gitlab/kubernetes/helm/api_spec.rb | 2 +- .../kubernetes/helm/base_command_spec.rb | 28 ++----- .../kubernetes/helm/certificate_spec.rb | 27 ------- .../kubernetes/helm/init_command_spec.rb | 4 +- .../kubernetes/helm/install_command_spec.rb | 75 ++++++++++--------- spec/lib/gitlab/kubernetes/helm/pod_spec.rb | 29 ++++++- .../models/clusters/applications/helm_spec.rb | 26 +------ .../clusters/applications/ingress_spec.rb | 41 ++-------- .../clusters/applications/jupyter_spec.rb | 45 +++-------- .../clusters/applications/prometheus_spec.rb | 41 +++------- .../clusters/applications/runner_spec.rb | 64 +++++----------- .../applications/install_service_spec.rb | 2 +- 34 files changed, 181 insertions(+), 537 deletions(-) delete mode 100644 changelogs/unreleased/48098-mutual-auth-cluster-applications.yml delete mode 100644 db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb delete mode 100644 lib/gitlab/kubernetes/helm/certificate.rb delete mode 100644 spec/lib/gitlab/kubernetes/helm/certificate_spec.rb diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index b566edae7bb..58de3448577 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -1,26 +1,13 @@ -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? @@ -28,41 +15,7 @@ module Clusters end def install_command - 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) + Gitlab::Kubernetes::Helm::InitCommand.new(name) end end end diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 64810812531..27fc3b85465 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, - files: files + values: values ) end diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb index cff5a423acb..975d434e1a4 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, - files: files, + values: values, repository: repository ) end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index 22815cc1219..ea6ec4d6b03 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, - files: files + values: values ) end diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index 4c894b5376d..e6f795f3e0b 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, - files: files, + values: values, repository: repository ) end diff --git a/app/models/clusters/concerns/application_data.rb b/app/models/clusters/concerns/application_data.rb index d66f09d48b5..96ac757e99e 100644 --- a/app/models/clusters/concerns/application_data.rb +++ b/app/models/clusters/concerns/application_data.rb @@ -12,34 +12,8 @@ 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 deleted file mode 100644 index 36e39cf31db..00000000000 --- a/changelogs/unreleased/48098-mutual-auth-cluster-applications.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Ensure installed Helm Tiller For GitLab Managed Apps Is protected by mutual - auth -merge_request: 20801 -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 deleted file mode 100644 index d9f15b6b67d..00000000000 --- a/db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb +++ /dev/null @@ -1,11 +0,0 @@ -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 0638de8c6ff..a5f7b8149c6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -636,9 +636,6 @@ ActiveRecord::Schema.define(version: 20180722103201) 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 9e55dae137c..8a8a59a9cd4 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, files) + def initialize(name, values = "") @name = name - @files = files + @values = values end def generate resource = ::Kubeclient::Resource.new resource.metadata = metadata - resource.data = files + resource.data = { values: values } resource end @@ -19,7 +19,7 @@ module Gitlab private - attr_reader :name, :files + attr_reader :name, :values def metadata { diff --git a/lib/gitlab/kubernetes/helm/api.rb b/lib/gitlab/kubernetes/helm/api.rb index d65374cc23b..c4de9a398cc 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) + create_config_map(command) if command.config_map? 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 afcfd109de0..f9ebe53d6af 100644 --- a/lib/gitlab/kubernetes/helm/base_command.rb +++ b/lib/gitlab/kubernetes/helm/base_command.rb @@ -1,7 +1,13 @@ module Gitlab module Kubernetes module Helm - module BaseCommand + class BaseCommand + attr_reader :name + + def initialize(name) + @name = name + end + def pod_resource Gitlab::Kubernetes::Helm::Pod.new(self, namespace).generate end @@ -18,32 +24,16 @@ 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 deleted file mode 100644 index c344add82cd..00000000000 --- a/lib/gitlab/kubernetes/helm/certificate.rb +++ /dev/null @@ -1,72 +0,0 @@ -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 a4546509515..a02e64561f6 100644 --- a/lib/gitlab/kubernetes/helm/init_command.rb +++ b/lib/gitlab/kubernetes/helm/init_command.rb @@ -1,16 +1,7 @@ module Gitlab module Kubernetes module Helm - class InitCommand - include BaseCommand - - attr_reader :name, :files - - def initialize(name:, files:) - @name = name - @files = files - end - + class InitCommand < BaseCommand def generate_script super + [ init_helm_command @@ -20,12 +11,7 @@ module Gitlab private def init_helm_command - 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" + "helm init >/dev/null" end end end diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb index c7d6a9c5b4d..d2133a6d65b 100644 --- a/lib/gitlab/kubernetes/helm/install_command.rb +++ b/lib/gitlab/kubernetes/helm/install_command.rb @@ -1,16 +1,14 @@ module Gitlab module Kubernetes module Helm - class InstallCommand - include BaseCommand + class InstallCommand < BaseCommand + attr_reader :name, :chart, :version, :repository, :values - attr_reader :name, :files, :chart, :version, :repository - - def initialize(name:, chart:, files:, version: nil, repository: nil) + def initialize(name, chart:, values:, version: nil, repository: nil) @name = name @chart = chart @version = version - @files = files + @values = values @repository = repository end @@ -22,6 +20,14 @@ 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 @@ -33,27 +39,14 @@ module Gitlab end def script_command - "helm install" \ - "#{optional_tls_flags} " \ - "#{chart} " \ - "--name #{name}" \ - "#{optional_version_flag} " \ - "--namespace #{Gitlab::Kubernetes::Helm::NAMESPACE} " \ - "-f /data/helm/#{name}/config/values.yaml >/dev/null\n" + <<~HEREDOC + helm install #{chart} --name #{name}#{optional_version_flag} --namespace #{Gitlab::Kubernetes::Helm::NAMESPACE} -f /data/helm/#{name}/config/values.yaml >/dev/null + HEREDOC 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 6e5d3388405..1e12299eefd 100644 --- a/lib/gitlab/kubernetes/helm/pod.rb +++ b/lib/gitlab/kubernetes/helm/pod.rb @@ -10,8 +10,10 @@ module Gitlab def generate spec = { containers: [container_specification], restartPolicy: 'Never' } - spec[:volumes] = volumes_specification - spec[:containers][0][:volumeMounts] = volume_mounts_specification + if command.config_map? + spec[:volumes] = volumes_specification + spec[:containers][0][:volumeMounts] = volume_mounts_specification + end ::Kubeclient::Resource.new(metadata: metadata, spec: spec) end @@ -59,7 +61,7 @@ module Gitlab name: 'configuration-volume', configMap: { name: "values-content-configuration-#{command.name}", - items: command.file_names.map { |name| { key: name, path: name } } + items: [{ key: 'values', path: 'values.yaml' }] } } ] diff --git a/qa/qa/factory/resource/kubernetes_cluster.rb b/qa/qa/factory/resource/kubernetes_cluster.rb index ef2ea72b170..1c9e5f94b22 100644 --- a/qa/qa/factory/resource/kubernetes_cluster.rb +++ b/qa/qa/factory/resource/kubernetes_cluster.rb @@ -44,11 +44,10 @@ module QA page.await_installed(:helm) page.install!(:ingress) if @install_ingress - page.install!(:prometheus) if @install_prometheus - page.install!(:runner) if @install_runner - 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(: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 e831edeb89e..4923304133e 100644 --- a/qa/qa/page/project/operations/kubernetes/show.rb +++ b/qa/qa/page/project/operations/kubernetes/show.rb @@ -16,7 +16,6 @@ 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 7c4a440b9a9..3e4277e4ba6 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -32,21 +32,11 @@ FactoryBot.define do updated_at ClusterWaitForAppInstallationWorker::TIMEOUT.ago end - 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_ingress, class: Clusters::Applications::Ingress + factory :clusters_applications_prometheus, class: Clusters::Applications::Prometheus + factory :clusters_applications_runner, class: Clusters::Applications::Runner 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 bbeba8ce8b9..0430762c1ff 100644 --- a/spec/factories/clusters/clusters.rb +++ b/spec/factories/clusters/clusters.rb @@ -36,9 +36,5 @@ 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 71d715237f5..a65ca662350 100644 --- a/spec/features/projects/clusters/applications_spec.rb +++ b/spec/features/projects/clusters/applications_spec.rb @@ -46,14 +46,12 @@ describe 'Clusters Applications', :js do end end - it 'they see status transition' do + it 'he sees 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" @@ -85,7 +83,7 @@ describe 'Clusters Applications', :js do end end - it 'they see status transition' do + it 'he sees 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]') @@ -118,14 +116,4 @@ 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 fe65d03875f..e253b291277 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.files) } + let(:config_map) { described_class.new(application.name, application.values) } 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: application.files) } + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) } 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 341f71a3e49..6e9b4ca0869 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.files).generate } + let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.values).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 d50616e95e8..7be8be54d5e 100644 --- a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb @@ -2,25 +2,7 @@ require 'spec_helper' describe Gitlab::Kubernetes::Helm::BaseCommand do let(:application) { create(:clusters_applications_helm) } - 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 + let(:base_command) { described_class.new(application.name) } subject { base_command } @@ -36,9 +18,15 @@ 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-test-class-name') } + it { is_expected.to eq('install-helm') } end end diff --git a/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb b/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb deleted file mode 100644 index f8d00c95a36..00000000000 --- a/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -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 dcbc046cf00..89e36a298f8 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 --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' } + let(:commands) { 'helm init >/dev/null' } - subject { described_class.new(name: application.name, files: {}) } + subject { described_class.new(application.name) } 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 51221e54d89..25c6fa3b9a3 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -1,82 +1,83 @@ require 'rails_helper' describe Gitlab::Kubernetes::Helm::InstallCommand do - 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 + let(:application) { create(:clusters_applications_prometheus) } + let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } + let(:install_command) { application.install_command } subject { install_command } - 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 --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 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 - - context 'when there is no repository' do - let(:repository) { nil } + 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 --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 chart-name --name app-name --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end end - context 'when there is no ca.pem file' do - let(:files) { { 'file.txt': 'some content' } } + context 'for prometheus' do + let(:application) { create(:clusters_applications_prometheus) } 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 --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/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 end end - context 'when there is no version' do - let(:version) { nil } + context 'for runner' do + let(:ci_runner) { create(:ci_runner) } + let(:application) { create(:clusters_applications_runner, runner: ci_runner) } 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 --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 chart-name --name app-name --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm repo add #{application.name} #{application.repository} + helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end end + context 'for jupyter' do + let(:application) { create(:clusters_applications_jupyter) } + + 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} --namespace #{namespace} -f /data/helm/#{application.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-app-name", - namespace: 'gitlab-managed-apps', - labels: { name: "values-content-configuration-app-name" } + name: "values-content-configuration-#{application.name}", + namespace: namespace, + labels: { name: "values-content-configuration-#{application.name}" } } end - let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: files) } + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) } 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 ec64193c0b2..43adc80d576 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -2,13 +2,14 @@ require 'rails_helper' describe Gitlab::Kubernetes::Helm::Pod do describe '#generate' do - let(:app) { create(:clusters_applications_prometheus) } + let(:cluster) { create(:cluster) } + let(:app) { create(:clusters_applications_prometheus, cluster: cluster) } let(:command) { app.install_command } let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } subject { described_class.new(command, namespace) } - context 'with a command' do + shared_examples 'helm pod' do it 'should generate a Kubeclient::Resource' do expect(subject.generate).to be_a_kind_of(Kubeclient::Resource) end @@ -40,6 +41,10 @@ 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 @@ -55,8 +60,24 @@ 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.yaml') - expect(volume.configMap['items'].first['path']).to eq(:'values.yaml') + 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 end end end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index e5b2bdc8a4e..0eb1e3876e2 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -6,24 +6,13 @@ describe Clusters::Applications::Helm do describe '.installed' do subject { described_class.installed } - let!(:installed_cluster) { create(:clusters_applications_helm, :installed) } + let!(:cluster) { create(:clusters_applications_helm, :installed) } before do create(:clusters_applications_helm, :errored) end - 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 + it { is_expected.to contain_exactly(cluster) } end describe '#install_command' do @@ -36,16 +25,5 @@ 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 c76aae432f9..bb5b2ef3a47 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -74,43 +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.files).to eq(ingress.files) + expect(subject.values).to eq(ingress.values) end end - describe '#files' do - let(:application) { ingress } - subject { application.files } - let(:values) { subject[:'values.yaml'] } + describe '#values' do + subject { ingress.values } - 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 + 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') end end end diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index a8cf44ac8a8..65750141e65 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -38,46 +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.files).to eq(jupyter.files) + expect(subject.values).to eq(jupyter.values) end end - describe '#files' do - let(:application) { create(:clusters_applications_jupyter) } - subject { application.files } - let(:values) { subject[:'values.yaml'] } + describe '#values' do + let(:jupyter) { create(:clusters_applications_jupyter) } - 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 + subject { jupyter.values } it 'should include valid values' do - 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}/) + 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}") end end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 313bd741f88..e4b61552033 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -153,44 +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.files).to eq(prometheus.files) + expect(command.values).to eq(prometheus.values) end end - describe '#files' do - let(:application) { create(:clusters_applications_prometheus) } - subject { application.files } - let(:values) { subject[:'values.yaml'] } + describe '#values' do + let(:prometheus) { create(:clusters_applications_prometheus) } - 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 + subject { prometheus.values } it 'should include prometheus valid values' do - 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') + 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') end end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 65aaa1ee882..b12500d0acd 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -33,55 +33,31 @@ 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.files).to eq(gitlab_runner.files) + expect(subject.values).to eq(gitlab_runner.values) end end - describe '#files' do - let(:application) { create(:clusters_applications_runner, runner: ci_runner) } + describe '#values' do + let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) } - subject { application.files } - let(:values) { subject[:'values.yaml'] } - - 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 + subject { gitlab_runner.values } it 'should include runner valid values' do - 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}/) + 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}") end context 'without a runner' do let(:project) { create(:project) } - let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) } - let(:application) { create(:clusters_applications_runner, cluster: cluster) } + let(:cluster) { create(:cluster, projects: [project]) } + let(:gitlab_runner) { create(:clusters_applications_runner, cluster: cluster) } it 'creates a runner' do expect do @@ -90,18 +66,18 @@ describe Clusters::Applications::Runner do end it 'uses the new runner token' do - expect(values).to match(/runnerToken: '?#{application.reload.runner.token}/) + expect(subject).to include("runnerToken: #{gitlab_runner.reload.runner.token}") end it 'assigns the new runner to runner' do subject - expect(application.reload.runner).to be_project_type + expect(gitlab_runner.reload.runner).to be_project_type end end context 'with duplicated values on vendor/runner/values.yaml' do - let(:stub_values) do + let(:values) do { "concurrent" => 4, "checkInterval" => 3, @@ -120,11 +96,11 @@ describe Clusters::Applications::Runner do end before do - allow(application).to receive(:chart_values).and_return(stub_values) + allow(gitlab_runner).to receive(:chart_values).and_return(values) end it 'should overwrite values.yaml' do - expect(values).to match(/privileged: '?#{application.privileged}/) + is_expected.to include("privileged: #{gitlab_runner.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 a744ec30b65..93199964a0e 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) { create(:clusters_applications_helm, :scheduled) } + let(:application) { build(:clusters_applications_helm, :scheduled) } it 'make the application errored' do expect(application).to receive(:make_installing!).once.and_raise(ActiveRecord::RecordInvalid)