Merge branch '53696-make-rbac-default' into 'master'

Make RBAC enabled by default for new clusters

Closes #53696

See merge request gitlab-org/gitlab-ce!24119

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

changelogs/unreleased/53696-make-rbac-default.yml

+---
+title: Make RBAC enabled default for new clusters
+merge_request: 24119
+author:
+type: changed

db/migrate/20190103140724_make_legacy_false_default.rb

+# 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

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
 

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

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

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
 

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
 

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
 

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

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

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

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

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

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

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

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

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)