Commit 97441486 authored by Chris Baumbauer's avatar Chris Baumbauer

Fix issue with missing knative cluster role binding, and cleanup tests

parent 1435fe60
...@@ -18,21 +18,18 @@ module Clusters ...@@ -18,21 +18,18 @@ module Clusters
include ::Clusters::Concerns::ApplicationData include ::Clusters::Concerns::ApplicationData
default_value_for :version, VERSION default_value_for :version, VERSION
default_value_for :hostname, nil
validates :hostname, presence: true
def chart def chart
'knative/knative' 'knative/knative'
end end
def values def values
content_values.to_yaml { domain: hostname }.to_yaml
end end
def install_command def install_command
if hostname.nil?
raise 'Hostname is required'
end
Gitlab::Kubernetes::Helm::InstallCommand.new( Gitlab::Kubernetes::Helm::InstallCommand.new(
name: name, name: name,
version: VERSION, version: VERSION,
...@@ -40,25 +37,60 @@ module Clusters ...@@ -40,25 +37,60 @@ module Clusters
chart: chart, chart: chart,
files: files, files: files,
repository: REPOSITORY, repository: REPOSITORY,
script: install_script preinstall: install_script,
postinstall: setup_knative_role
) )
end end
private
def install_script def install_script
['/usr/bin/kubectl', 'apply', '-f', ISTIO_CRDS] ["/usr/bin/kubectl apply -f #{ISTIO_CRDS} >/dev/null"]
end end
private def setup_knative_role
if !cluster.kubernetes_namespace.nil?
def content_values [
YAML.load_file(chart_values_file).deep_merge!(knative_configs) "echo \'#{create_rolebinding.to_yaml}\' > /tmp/rolebinding.yaml\n",
"/usr/bin/kubectl apply -f /tmp/rolebinding.yaml > /dev/null"
]
else
nil
end
end end
def knative_configs def create_rolebinding
{ {
"domain" => hostname "apiVersion" => "rbac.authorization.k8s.io/v1",
"kind" => "ClusterRoleBinding",
"metadata" => {
"name" => create_role_binding_name,
"namespace" => namespace
},
"roleRef" => {
"apiGroup" => "rbac.authorization.k8s.io",
"kind" => "ClusterRole",
"name" => "knative-serving-admin"
},
"subjects" => role_subject
} }
end end
def create_role_binding_name
"#{namespace}-knative-binding"
end
def service_account_name
cluster.kubernetes_namespace.service_account_name
end
def role_subject
[{ "kind" => 'ServiceAccount', "name" => service_account_name, "namespace" => namespace }]
end
def namespace
cluster.kubernetes_namespace.namespace
end
end end
end end
end end
...@@ -4,16 +4,17 @@ module Gitlab ...@@ -4,16 +4,17 @@ module Gitlab
class InstallCommand class InstallCommand
include BaseCommand include BaseCommand
attr_reader :name, :files, :chart, :version, :repository, :script attr_reader :name, :files, :chart, :version, :repository, :preinstall, :postinstall
def initialize(name:, chart:, files:, rbac:, version: nil, repository: nil, script: nil) def initialize(name:, chart:, files:, rbac:, version: nil, repository: nil, preinstall: nil, postinstall: nil)
@name = name @name = name
@chart = chart @chart = chart
@version = version @version = version
@rbac = rbac @rbac = rbac
@files = files @files = files
@repository = repository @repository = repository
@script = script @preinstall = preinstall
@postinstall = postinstall
end end
def generate_script def generate_script
...@@ -21,8 +22,9 @@ module Gitlab ...@@ -21,8 +22,9 @@ module Gitlab
init_command, init_command,
repository_command, repository_command,
repository_update_command, repository_update_command,
script_command, preinstall_command,
install_command install_command,
postinstall_command
].compact.join("\n") ].compact.join("\n")
end end
...@@ -50,9 +52,15 @@ module Gitlab ...@@ -50,9 +52,15 @@ module Gitlab
command.shelljoin + " >/dev/null\n" command.shelljoin + " >/dev/null\n"
end end
def script_command def preinstall_command
unless script.nil? unless preinstall.nil?
script.shelljoin + " >/dev/null\n" preinstall.join("\n")
end
end
def postinstall_command
unless postinstall.nil?
postinstall.join("\n")
end end
end end
......
...@@ -58,6 +58,7 @@ FactoryBot.define do ...@@ -58,6 +58,7 @@ FactoryBot.define do
end end
factory :clusters_applications_knative, class: Clusters::Applications::Knative do factory :clusters_applications_knative, class: Clusters::Applications::Knative do
hostname 'example.com'
cluster factory: %i(cluster with_installed_helm provided_by_gcp) cluster factory: %i(cluster with_installed_helm provided_by_gcp)
end end
......
...@@ -5,6 +5,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do ...@@ -5,6 +5,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do
let(:repository) { 'https://repository.example.com' } let(:repository) { 'https://repository.example.com' }
let(:rbac) { false } let(:rbac) { false }
let(:version) { '1.2.3' } let(:version) { '1.2.3' }
let(:preinstall) { nil }
let(:postinstall) { nil }
let(:install_command) do let(:install_command) do
described_class.new( described_class.new(
...@@ -13,7 +15,9 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do ...@@ -13,7 +15,9 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do
rbac: rbac, rbac: rbac,
files: files, files: files,
version: version, version: version,
repository: repository repository: repository,
preinstall: preinstall,
postinstall: postinstall
) )
end end
...@@ -101,6 +105,53 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do ...@@ -101,6 +105,53 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do
end end
end end
context 'when there is a pre-install script' do
let(:preinstall) { ['/bin/date', '/bin/true'] }
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 repo update >/dev/null
#{helm_install_command}
EOS
end
let(:helm_install_command) do
<<~EOS.strip
/bin/date
/bin/true
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 'when there is a post-install script' do
let(:postinstall) { ['/bin/date', "/bin/false\n"] }
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 repo update >/dev/null
#{helm_install_command}
EOS
end
let(:helm_install_command) do
<<~EOS.strip
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
/bin/date
/bin/false
EOS
end
end
end
context 'when there is no ca.pem file' do context 'when there is no ca.pem file' do
let(:files) { { 'file.txt': 'some content' } } let(:files) { { 'file.txt': 'some content' } }
......
require 'rails_helper' require 'rails_helper'
describe Clusters::Applications::Knative do describe Clusters::Applications::Knative do
let(:knative) { create(:clusters_applications_knative, hostname: 'example.com') } let(:knative) { create(:clusters_applications_knative) }
include_examples 'cluster application core specs', :clusters_applications_knative include_examples 'cluster application core specs', :clusters_applications_knative
include_examples 'cluster application status specs', :clusters_applications_knative include_examples 'cluster application status specs', :clusters_applications_knative
...@@ -47,7 +47,9 @@ describe Clusters::Applications::Knative do ...@@ -47,7 +47,9 @@ describe Clusters::Applications::Knative do
describe '#install_command' do describe '#install_command' do
subject { knative.install_command } subject { knative.install_command }
it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::Helm::InstallCommand) } it 'should be an instance of Helm::InstallCommand' do
expect(subject).to be_an_instance_of(Gitlab::Kubernetes::Helm::InstallCommand)
end
it 'should be initialized with knative arguments' do it 'should be initialized with knative arguments' do
expect(subject.name).to eq('knative') expect(subject.name).to eq('knative')
...@@ -55,14 +57,6 @@ describe Clusters::Applications::Knative do ...@@ -55,14 +57,6 @@ describe Clusters::Applications::Knative do
expect(subject.version).to eq('0.1.3') expect(subject.version).to eq('0.1.3')
expect(subject.files).to eq(knative.files) expect(subject.files).to eq(knative.files)
end end
context 'application failed to install previously' do
let(:knative) { create(:clusters_applications_knative, :errored, version: 'knative', hostname: 'example.com') }
it 'should be initialized with the locked version' do
expect(subject.version).to eq('0.1.3')
end
end
end end
describe '#files' do describe '#files' do
...@@ -71,7 +65,7 @@ describe Clusters::Applications::Knative do ...@@ -71,7 +65,7 @@ describe Clusters::Applications::Knative do
subject { application.files } subject { application.files }
it 'should include knative valid keys in values' do it 'should include knative specific keys in the values.yaml file' do
expect(values).to include('domain') expect(values).to include('domain')
end end
...@@ -80,20 +74,15 @@ describe Clusters::Applications::Knative do ...@@ -80,20 +74,15 @@ describe Clusters::Applications::Knative do
application.cluster.application_helm.ca_cert = nil application.cluster.application_helm.ca_cert = nil
end end
it 'should not include cert files' do it 'should not include cert files when there is no ca_cert entry' do
expect(subject[:'ca.pem']).not_to be_present expect(subject).not_to include(:'ca.pem', :'cert.pem', :'key.pem')
expect(subject[:'cert.pem']).not_to be_present
expect(subject[:'key.pem']).not_to be_present
end end
end end
it 'should include cert files' do it 'should include cert files when there is a ca_cert entry' do
expect(subject[:'ca.pem']).to be_present expect(subject).to include(:'ca.pem', :'cert.pem', :'key.pem')
expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert) 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']) cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem'])
expect(cert.not_after).to be < 60.minutes.from_now expect(cert.not_after).to be < 60.minutes.from_now
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment