Make RBAC enabled default for new clusters53696-make-rbac-default

Many changes were also made to tests that expected this to default to
false.

18 files changed, 90 insertions, 108 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 893ed3e3f64..0f6407f193b 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -161,16 +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
 
-    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
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)