diff options
18 files changed, 95 insertions, 113 deletions
diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 0dc0c4f80d6..1cc170c8c4d 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -65,6 +65,8 @@ module Clusters abac: 2 } + default_value_for :authorization_type, :rbac + def actual_namespace if namespace.present? namespace diff --git a/changelogs/unreleased/53696-make-rbac-default.yml b/changelogs/unreleased/53696-make-rbac-default.yml new file mode 100644 index 00000000000..4f1326cd874 --- /dev/null +++ b/changelogs/unreleased/53696-make-rbac-default.yml @@ -0,0 +1,5 @@ +--- +title: Make RBAC enabled default for new clusters +merge_request: 24119 +author: +type: changed diff --git a/db/migrate/20190103140724_make_legacy_false_default.rb b/db/migrate/20190103140724_make_legacy_false_default.rb new file mode 100644 index 00000000000..154035f76cd --- /dev/null +++ b/db/migrate/20190103140724_make_legacy_false_default.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class MakeLegacyFalseDefault < ActiveRecord::Migration[5.0] + DOWNTIME = false + + def change + change_column_default :cluster_providers_gcp, :legacy_abac, from: true, to: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 0af185409a9..8dc1260d177 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20181219145520) do +ActiveRecord::Schema.define(version: 20190103140724) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -630,7 +630,7 @@ ActiveRecord::Schema.define(version: 20181219145520) do t.string "endpoint" t.text "encrypted_access_token" t.string "encrypted_access_token_iv" - t.boolean "legacy_abac", default: true, null: false + t.boolean "legacy_abac", default: false, null: false t.index ["cluster_id"], name: "index_cluster_providers_gcp_on_cluster_id", unique: true, using: :btree end diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index 5788dceaaae..0a48f4c0e7f 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -92,6 +92,10 @@ module QA find_element(name).set(true) end + def uncheck_element(name) + find_element(name).set(false) + end + def click_element(name) find_element(name).click end diff --git a/qa/qa/page/project/operations/kubernetes/add_existing.rb b/qa/qa/page/project/operations/kubernetes/add_existing.rb index f3ab636ecc1..ffd5b36e1ae 100644 --- a/qa/qa/page/project/operations/kubernetes/add_existing.rb +++ b/qa/qa/page/project/operations/kubernetes/add_existing.rb @@ -33,8 +33,8 @@ module QA click_on 'Add Kubernetes cluster' end - def check_rbac! - check_element :rbac_checkbox + def uncheck_rbac! + uncheck_element :rbac_checkbox end end end diff --git a/qa/qa/resource/kubernetes_cluster.rb b/qa/qa/resource/kubernetes_cluster.rb index 96c8843fb99..d67e5f6da20 100644 --- a/qa/qa/resource/kubernetes_cluster.rb +++ b/qa/qa/resource/kubernetes_cluster.rb @@ -29,7 +29,7 @@ module QA page.set_api_url(@cluster.api_url) page.set_ca_certificate(@cluster.ca_certificate) page.set_token(@cluster.token) - page.check_rbac! if @cluster.rbac + page.uncheck_rbac! unless @cluster.rbac page.add_cluster! end diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 06e30571336..9322e29d744 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -33,32 +33,6 @@ describe 'Gcp Cluster', :js do context 'when user filled form with valid parameters' do subject { click_button 'Create Kubernetes cluster' } - shared_examples 'valid cluster gcp form' do - it 'users sees a form with the GCP token' do - expect(page).to have_selector(:css, 'form[data-token="token"]') - end - - it 'user sees a cluster details page and creation status' do - subject - - expect(page).to have_content('Kubernetes cluster is being created on Google Kubernetes Engine...') - - Clusters::Cluster.last.provider.make_created! - - expect(page).to have_content('Kubernetes cluster was successfully created on Google Kubernetes Engine') - end - - it 'user sees a error if something wrong during creation' do - subject - - expect(page).to have_content('Kubernetes cluster is being created on Google Kubernetes Engine...') - - Clusters::Cluster.last.provider.make_errored!('Something wrong!') - - expect(page).to have_content('Something wrong!') - end - end - before do allow_any_instance_of(GoogleApi::CloudPlatform::Client) .to receive(:projects_zones_clusters_create) do @@ -82,14 +56,32 @@ describe 'Gcp Cluster', :js do fill_in 'cluster[provider_gcp_attributes][machine_type]', with: 'n1-standard-2' end - it_behaves_like 'valid cluster gcp form' + it 'users sees a form with the GCP token' do + expect(page).to have_selector(:css, 'form[data-token="token"]') + end - context 'RBAC is enabled for the cluster' do - before do - check 'cluster_provider_gcp_attributes_legacy_abac' - end + it 'user sees a cluster details page and creation status' do + subject + + expect(page).to have_content('Kubernetes cluster is being created on Google Kubernetes Engine...') + + Clusters::Cluster.last.provider.make_created! + + expect(page).to have_content('Kubernetes cluster was successfully created on Google Kubernetes Engine') + end + + it 'user sees a error if something wrong during creation' do + subject + + expect(page).to have_content('Kubernetes cluster is being created on Google Kubernetes Engine...') + + Clusters::Cluster.last.provider.make_errored!('Something wrong!') + + expect(page).to have_content('Something wrong!') + end - it_behaves_like 'valid cluster gcp form' + it 'user sees RBAC is enabled by default' do + expect(page).to have_checked_field('RBAC-enabled cluster') end end diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb index 250c964cc32..1f2f7592d8b 100644 --- a/spec/features/projects/clusters/user_spec.rb +++ b/spec/features/projects/clusters/user_spec.rb @@ -23,19 +23,6 @@ describe 'User Cluster', :js do end context 'when user filled form with valid parameters' do - shared_examples 'valid cluster user form' do - it 'user sees a cluster details page' do - subject - - expect(page).to have_content('Kubernetes cluster integration') - expect(page.find_field('cluster[name]').value).to eq('dev-cluster') - expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) - .to have_content('http://example.com') - expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value) - .to have_content('my-token') - end - end - before do fill_in 'cluster_name', with: 'dev-cluster' fill_in 'cluster_platform_kubernetes_attributes_api_url', with: 'http://example.com' @@ -44,20 +31,19 @@ describe 'User Cluster', :js do subject { click_button 'Add Kubernetes cluster' } - it_behaves_like 'valid cluster user form' - - context 'RBAC is enabled for the cluster' do - before do - check 'cluster_platform_kubernetes_attributes_authorization_type' - end - - it_behaves_like 'valid cluster user form' + it 'user sees a cluster details page' do + subject - it 'user sees a cluster details page with RBAC enabled' do - subject + expect(page).to have_content('Kubernetes cluster integration') + expect(page.find_field('cluster[name]').value).to eq('dev-cluster') + expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) + .to have_content('http://example.com') + expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value) + .to have_content('my-token') + end - expect(page.find_field('cluster[platform_kubernetes_attributes][authorization_type]', disabled: true)).to be_checked - end + it 'user sees RBAC is enabled by default' do + expect(page).to have_checked_field('RBAC-enabled cluster') end end diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb index e825f3e2392..8e14abe098d 100644 --- a/spec/models/clusters/applications/cert_manager_spec.rb +++ b/spec/models/clusters/applications/cert_manager_spec.rb @@ -29,7 +29,7 @@ describe Clusters::Applications::CertManager do expect(subject.name).to eq('certmanager') expect(subject.chart).to eq('stable/cert-manager') expect(subject.version).to eq('v0.5.2') - expect(subject).not_to be_rbac + expect(subject).to be_rbac expect(subject.files).to eq(cert_manager.files.merge(cluster_issuer_file)) expect(subject.postinstall).to eq(['/usr/bin/kubectl create -f /data/helm/certmanager/config/cluster_issuer.yaml']) end @@ -45,12 +45,12 @@ describe Clusters::Applications::CertManager do end end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - cert_manager.cluster.platform_kubernetes.rbac! + cert_manager.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 2c37cd20ecc..64f6d9c8bb4 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -49,16 +49,16 @@ describe Clusters::Applications::Helm do end describe 'rbac' do - context 'non rbac cluster' do - it { expect(subject).not_to be_rbac } + context 'rbac cluster' do + it { expect(subject).to be_rbac } end - context 'rbac cluster' do + context 'non rbac cluster' do before do - helm.cluster.platform_kubernetes.rbac! + helm.cluster.platform_kubernetes.abac! end - it { expect(subject).to be_rbac } + it { expect(subject).not_to be_rbac } end end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index cd28f1fe9c6..de313a8ca36 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -91,16 +91,16 @@ 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).not_to be_rbac + expect(subject).to be_rbac expect(subject.files).to eq(ingress.files) end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - ingress.cluster.platform_kubernetes.rbac! + ingress.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index a40edbf267b..391e5425384 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -52,17 +52,17 @@ describe Clusters::Applications::Jupyter do expect(subject.name).to eq('jupyter') expect(subject.chart).to eq('jupyter/jupyterhub') expect(subject.version).to eq('v0.6') - expect(subject).not_to be_rbac + expect(subject).to be_rbac expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') expect(subject.files).to eq(jupyter.files) end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - jupyter.cluster.platform_kubernetes.rbac! + jupyter.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 27143f29350..de6b844023a 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -161,20 +161,16 @@ describe Clusters::Applications::Prometheus do expect(subject.name).to eq('prometheus') expect(subject.chart).to eq('stable/prometheus') expect(subject.version).to eq('6.7.3') - expect(subject).not_to be_rbac + expect(subject).to be_rbac expect(subject.files).to eq(prometheus.files) end - it 'should not install knative metrics' do - expect(subject.postinstall).to be_nil - end - - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - prometheus.cluster.platform_kubernetes.rbac! + prometheus.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do @@ -185,13 +181,17 @@ describe Clusters::Applications::Prometheus do end end + it 'should not install knative metrics' do + expect(subject.postinstall).to be_nil + end + context 'with knative installed' do let(:knative) { create(:clusters_applications_knative, :installed ) } let(:prometheus) { create(:clusters_applications_prometheus, cluster: knative.cluster) } subject { prometheus.install_command } - it 'should install metrics' do + it 'should install knative metrics' do expect(subject.postinstall).to include("kubectl apply -f #{Clusters::Applications::Knative::METRICS_CONFIG}") end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index cae23daac8c..3d0735c6d0b 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -47,17 +47,17 @@ describe Clusters::Applications::Runner do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') expect(subject.version).to eq('0.1.43') - expect(subject).not_to be_rbac + expect(subject).to be_rbac expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.files).to eq(gitlab_runner.files) end - context 'on a rbac enabled cluster' do + context 'on a non rbac enabled cluster' do before do - gitlab_runner.cluster.platform_kubernetes.rbac! + gitlab_runner.cluster.platform_kubernetes.abac! end - it { is_expected.to be_rbac } + it { is_expected.not_to be_rbac } end context 'application failed to install previously' do diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index e6b076adc76..6c8a223092e 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -154,19 +154,11 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end describe '#rbac?' do - subject { kubernetes.rbac? } - let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) } - context 'when authorization type is rbac' do - let(:kubernetes) { build(:cluster_platform_kubernetes, :rbac_enabled, :configured) } - - it { is_expected.to be_truthy } - end + subject { kubernetes.rbac? } - context 'when authorization type is nil' do - it { is_expected.to be_falsey } - end + it { is_expected.to be_truthy } end describe '#actual_namespace' do diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index d134608b538..5012e6f15c6 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -79,17 +79,7 @@ describe Clusters::Providers::Gcp do subject { gcp } - it 'should default to true' do - is_expected.to be_legacy_abac - end - - context 'legacy_abac is set to false' do - let(:gcp) { build(:cluster_provider_gcp, legacy_abac: false) } - - it 'is false' do - is_expected.not_to be_legacy_abac - end - end + it { is_expected.not_to be_legacy_abac } end describe '#state_machine' do diff --git a/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb index fe785735fef..18f218fc236 100644 --- a/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb +++ b/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb @@ -27,6 +27,8 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' d stub_kubeclient_get_secret_error(api_url, 'gitlab-token') stub_kubeclient_create_secret(api_url) + stub_kubeclient_get_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace) + stub_kubeclient_put_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace) stub_kubeclient_get_namespace(api_url, namespace: namespace) stub_kubeclient_get_service_account_error(api_url, "#{namespace}-service-account", namespace: namespace) stub_kubeclient_create_service_account(api_url, namespace: namespace) |