Add EE specific module for Kubernetes API

Overrides:
- optional_params_ee to set environment_scope
- create_cluster_user_params: To add environment_scope

ee/lib/ee/api/project_clusters.rb

+# frozen_string_literal: true
+
+module EE
+  module API
+    module ProjectClusters
+      extend ActiveSupport::Concern
+
+      prepended do
+        helpers do
+          params :create_params_ee do
+            optional :environment_scope, default: '*', type: String, desc: 'The associated environment to the cluster'
+          end
+
+          params :update_params_ee do
+            optional :environment_scope, type: String, desc: 'The associated environment to the cluster'
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/requests/api/project_clusters_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::ProjectClusters do
+  include KubernetesHelpers
+
+  let(:current_user) { create(:user) }
+  let(:project) { create(:project, :repository) }
+
+  shared_context 'kubernetes calls stubbed' do
+    before do
+      stub_kubeclient_discover(api_url)
+      stub_kubeclient_get_namespace(api_url, namespace: namespace)
+      stub_kubeclient_get_service_account(api_url, "#{namespace}-service-account", namespace: namespace)
+      stub_kubeclient_put_service_account(api_url, "#{namespace}-service-account", namespace: namespace)
+
+      stub_kubeclient_get_secret(
+        api_url,
+        {
+          metadata_name: "#{namespace}-token",
+          token: Base64.encode64('sample-token'),
+          namespace: namespace
+        }
+      )
+
+      stub_kubeclient_put_secret(api_url, "#{namespace}-token", namespace: namespace)
+      stub_kubeclient_get_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace)
+      stub_kubeclient_put_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace)
+    end
+  end
+
+  before do
+    project.add_maintainer(current_user)
+  end
+
+  describe 'POST /projects/:id/clusters/user' do
+    include_context 'kubernetes calls stubbed'
+
+    let(:api_url) { 'https://kubernetes.example.com' }
+    let(:namespace) { project.path }
+
+    let(:platform_kubernetes_attributes) do
+      {
+        api_url: api_url,
+        token: 'sample-token',
+        namespace: namespace
+      }
+    end
+
+    let(:cluster_params) do
+      {
+        name: 'test-cluster',
+        environment_scope: 'production/*',
+        platform_kubernetes_attributes: platform_kubernetes_attributes
+      }
+    end
+
+    before do
+      post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params
+    end
+
+    context 'when user sets specific environment scope' do
+      it 'should create a cluster with that specific environment' do
+        expect(json_response['environment_scope']).to eq('production/*')
+      end
+    end
+
+    context 'when does not set an specific environment scope' do
+      let(:cluster_params) do
+        {
+          name: 'test-cluster',
+          platform_kubernetes_attributes: platform_kubernetes_attributes
+        }
+      end
+
+      it 'should set default environment' do
+        expect(json_response['environment_scope']).to eq('*')
+      end
+    end
+  end
+
+  describe 'PUT /projects/:id/clusters/:cluster_id' do
+    include_context 'kubernetes calls stubbed'
+
+    let(:api_url) { 'https://kubernetes.example.com' }
+    let(:namespace) { project.path }
+
+    let(:update_params) do
+      {
+        namespace: namespace,
+        environment_scope: 'test/*'
+      }
+    end
+
+    before do
+      put api("/projects/#{project.id}/clusters/#{cluster.id}", current_user), params: update_params
+
+      cluster.reload
+    end
+
+    context 'With a GCP cluster' do
+      let(:cluster) do
+        create(:cluster, :project, :provided_by_gcp,
+               projects: [project])
+      end
+
+      it 'should update the environment scope' do
+        expect(cluster.environment_scope).to eq('test/*')
+      end
+    end
+
+    context 'With an user cluster' do
+      let(:cluster) do
+        create(:cluster, :project, :provided_by_user,
+               projects: [project])
+      end
+
+      it 'should update the environment scope' do
+        expect(cluster.environment_scope).to eq('test/*')
+      end
+    end
+  end
+end

lib/api/entities.rb

@@ -1592,7 +1592,7 @@ module API
       expose :provider_type, :platform_type, :environment_scope, :cluster_type
       expose :user, using: Entities::UserBasic
       expose :platform_kubernetes, using: Entities::Platform::Kubernetes
-      expose :provider, as: :gcp_provider, using: Entities::Provider::Gcp, if: ->(cluster, _) { cluster.gcp? }
+      expose :provider_gcp, using: Entities::Provider::Gcp
     end
 
     class ClusterProject < Cluster

lib/api/project_clusters.rb

@@ -6,14 +6,13 @@ module API
 
     before { authenticate! }
 
+    # EE::API::ProjectClusters will
+    # override these methods
     helpers do
-      params :optional_add_params_ee do
-        # EE::API::ProjectClusters would override this
+      params :create_params_ee do
       end
 
-      # EE::API::ProjectClusters would override this
-      def environment_scope
-        '*'
+      params :update_params_ee do
       end
     end
 
@@ -55,25 +54,27 @@ module API
       end
       params do
         requires :name, type: String, desc: 'Cluster name'
-        requires :api_url, type: String, desc: 'URL to access the Kubernetes API'
-        requires :token, type: String, desc: 'Token to authenticate against Kubernetes'
-        optional :ca_cert, type: String, desc: 'TLS certificate (needed if API is using a self-signed TLS certificate)'
-        optional :namespace, type: String, desc: 'Unique namespace related to Project'
-        optional :rbac_enabled, type: Boolean, default: true, desc: 'Enable RBAC authorization type, defaults to true'
-        use :optional_add_params_ee
-      end
-      post ':id/add_cluster' do
+        optional :enabled, type: Boolean, default: true, desc: 'Determines if cluster is active or not, defaults to true'
+        requires :platform_kubernetes_attributes, type: Hash, desc: %q(Platform Kubernetes data) do
+          requires :api_url, type: String, allow_blank: false, desc: 'URL to access the Kubernetes API'
+          requires :token, type: String, desc: 'Token to authenticate against Kubernetes'
+          optional :ca_cert, type: String, desc: 'TLS certificate (needed if API is using a self-signed TLS certificate)'
+          optional :namespace, type: String, desc: 'Unique namespace related to Project'
+          optional :authorization_type, type: String, values: Clusters::Platforms::Kubernetes.authorization_types.keys, default: 'rbac', desc: 'Cluster authorization type, defaults to RBAC'
+        end
+        use :create_params_ee
+      end
+      post ':id/clusters/user' do
         authorize! :create_cluster, user_project
 
-        new_cluster = ::Clusters::CreateService
-          .new(current_user, create_cluster_params)
-          .execute(access_token: token_in_session)
-          .present(current_user: current_user)
+        user_cluster = ::Clusters::CreateService
+          .new(current_user, create_cluster_user_params)
+          .execute
 
-        if new_cluster.persisted?
-          present new_cluster, with: Entities::ClusterProject
+        if user_cluster.persisted?
+          present user_cluster, with: Entities::ClusterProject
         else
-          render_validation_error!(new_cluster)
+          render_validation_error!(user_cluster)
         end
       end
 
@@ -84,10 +85,13 @@ module API
       params do
         requires :cluster_id, type: Integer, desc: 'The cluster ID'
         optional :name, type: String, desc: 'Cluster name'
-        optional :api_url, type: String, desc: 'URL to access the Kubernetes API'
-        optional :token, type: String, desc: 'Token to authenticate against Kubernetes'
-        optional :ca_cert, type: String, desc: 'TLS certificate (needed if API is using a self-signed TLS certificate)'
-        optional :namespace, type: String, desc: 'Unique namespace related to Project'
+        optional :platform_kubernetes_attributes, type: Hash, desc: %q(Platform Kubernetes data) do
+          optional :api_url, type: String, desc: 'URL to access the Kubernetes API'
+          optional :token, type: String, desc: 'Token to authenticate against Kubernetes'
+          optional :ca_cert, type: String, desc: 'TLS certificate (needed if API is using a self-signed TLS certificate)'
+          optional :namespace, type: String, desc: 'Unique namespace related to Project'
+        end
+        use :update_params_ee
       end
       put ':id/clusters/:cluster_id' do
         authorize! :update_cluster, cluster
@@ -124,57 +128,16 @@ module API
         @cluster ||= clusters_for_current_user.find(params[:cluster_id])
       end
 
-      def token_in_session
-        session[GoogleApi::CloudPlatform::Client.session_key_for_token]
-      end
-
-      def create_cluster_params
-        {
-          name: declared_params[:name],
-          enabled: true,
-          environment_scope: environment_scope,
+      def create_cluster_user_params
+        declared_params.merge({
           provider_type: :user,
           platform_type: :kubernetes,
-          cluster_type: :project,
-          clusterable: user_project,
-          platform_kubernetes_attributes: create_platform_kubernetes_params
-        }
-      end
-
-      def create_platform_kubernetes_params
-        kubernetes_params = { authorization_type: kubernetes_authorization_type }
-        permitted_params = platform_kubernetes_params + [:authorization_type]
-
-        kubernetes_params.merge(declared_params.slice(*permitted_params))
+          clusterable: user_project
+        })
       end
 
       def update_cluster_params
-        {
-          platform_kubernetes_attributes: update_platform_kubernetes_params
-        }.merge(cluster.managed? ? {} : { name: declared_params[:name] })
-      end
-
-      def update_platform_kubernetes_params
-        permitted_params = if cluster.managed?
-                             [:namespace]
-                           else
-                             platform_kubernetes_params
-                           end
-
-        declared_params.slice(*permitted_params)
-      end
-
-      def platform_kubernetes_params
-        [:api_url, :token, :ca_cert, :namespace]
-      end
-
-      def kubernetes_authorization_type
-        rbac_enabled = declared_params.fetch(:rbac_enabled, true)
-        rbac_enabled ? authorization_types[:rbac] : authorization_types[:abac]
-      end
-
-      def authorization_types
-        Clusters::Platforms::Kubernetes.authorization_types
+        declared_params(include_missing: false).without(:cluster_id)
       end
     end
   end

spec/requests/api/project_clusters_spec.rb

@@ -114,7 +114,7 @@ describe API::ProjectClusters do
       end
 
       it 'returns GCP provider information' do
-        gcp_provider = json_response['gcp_provider']
+        gcp_provider = json_response['provider_gcp']
 
         expect(gcp_provider['cluster_id']).to eq(cluster.id)
         expect(gcp_provider['status_name']).to eq('created')
@@ -132,11 +132,11 @@ describe API::ProjectClusters do
         end
 
         it 'should not include GCP provider info' do
-          expect(json_response['gcp_provider']).not_to be_present
+          expect(json_response['provider_gcp']).not_to be_present
         end
       end
 
-      context 'for non existing cluster' do
+      context 'with non-existing cluster' do
         let(:cluster_id) { 123 }
 
         it 'returns 404' do
@@ -168,24 +168,32 @@ describe API::ProjectClusters do
     end
   end
 
-  describe 'POST /projects/:id/add_cluster' do
+  describe 'POST /projects/:id/clusters/user' do
     include_context 'kubernetes calls stubbed'
 
     let(:api_url) { 'https://kubernetes.example.com' }
     let(:namespace) { project.path }
+    let(:authorization_type) { 'rbac' }
 
-    let(:base_params) do
+    let(:platform_kubernetes_attributes) do
       {
-        name: 'test-cluster',
         api_url: api_url,
         token: 'sample-token',
-        namespace: namespace
+        namespace: namespace,
+        authorization_type: authorization_type
+      }
+    end
+
+    let(:cluster_params) do
+      {
+        name: 'test-cluster',
+        platform_kubernetes_attributes: platform_kubernetes_attributes
       }
     end
 
     context 'non-authorized user' do
       it 'should respond with 404' do
-        post api("/projects/#{project.id}/add_cluster", non_member), params: base_params
+        post api("/projects/#{project.id}/clusters/user", non_member), params: cluster_params
 
         expect(response).to have_gitlab_http_status(404)
       end
@@ -193,12 +201,10 @@ describe API::ProjectClusters do
 
     context 'authorized user' do
       before do
-        post api("/projects/#{project.id}/add_cluster", current_user), params: cluster_params
+        post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params
       end
 
       context 'with valid params' do
-        let(:cluster_params) { base_params }
-
         it 'should respond with 201' do
           expect(response).to have_gitlab_http_status(201)
         end
@@ -218,18 +224,34 @@ describe API::ProjectClusters do
         end
       end
 
-      context 'when user disables RBAC' do
-        let(:cluster_params) { base_params.merge(rbac_enabled: false) }
+      context 'when user does not indicate authorization type' do
+        let(:platform_kubernetes_attributes) do
+          {
+            api_url: api_url,
+            token: 'sample-token',
+            namespace: namespace
+          }
+        end
+
+        it 'defaults to RBAC' do
+          cluster_result = Clusters::Cluster.find(json_response['id'])
+
+          expect(cluster_result.platform_kubernetes.rbac?).to be_truthy
+        end
+      end
+
+      context 'when user sets authorization type as ABAC' do
+        let(:authorization_type) { 'abac' }
 
         it 'should create an ABAC cluster' do
-          cluster_result = Clusters::Cluster.find(json_response["id"])
+          cluster_result = Clusters::Cluster.find(json_response['id'])
 
           expect(cluster_result.platform.abac?).to be_truthy
         end
       end
 
       context 'with invalid params' do
-        let(:cluster_params) { base_params.merge(name: '') }
+        let(:namespace) { 'invalid_namespace' }
 
         it 'should respond with 400' do
           expect(response).to have_gitlab_http_status(400)
@@ -240,8 +262,7 @@ describe API::ProjectClusters do
         end
 
         it 'should return validation errors' do
-          expect(json_response['message']).to be_present
-          expect(json_response['message']['name'].first).to eq(" has to be present")
+          expect(json_response['message']['platform_kubernetes.namespace'].first).to be_present
         end
       end
     end
@@ -252,8 +273,13 @@ describe API::ProjectClusters do
 
     let(:api_url) { 'https://kubernetes.example.com' }
     let(:namespace) { 'new-namespace' }
-    let(:base_params) { { namespace: namespace } }
-    let(:update_params) { base_params }
+    let(:platform_kubernetes_attributes) { { namespace: namespace } }
+
+    let(:update_params) do
+      {
+        platform_kubernetes_attributes: platform_kubernetes_attributes
+      }
+    end
 
     let!(:kubernetes_namespace) do
       create(:cluster_kubernetes_namespace,
@@ -281,11 +307,11 @@ describe API::ProjectClusters do
         cluster.reload
       end
 
-      it 'should respond with 200' do
-        expect(response).to have_gitlab_http_status(200)
-      end
-
       context 'with valid params' do
+        it 'should respond with 200' do
+          expect(response).to have_gitlab_http_status(200)
+        end
+
         it 'should update cluster attributes' do
           expect(cluster.platform_kubernetes.namespace).to eq('new-namespace')
           expect(cluster.kubernetes_namespace.namespace).to eq('new-namespace')
@@ -295,33 +321,44 @@ describe API::ProjectClusters do
       context 'with invalid params' do
         let(:namespace) { 'invalid_namespace' }
 
+        it 'should respond with 400' do
+          expect(response).to have_gitlab_http_status(400)
+        end
+
         it 'should not update cluster attributes' do
           expect(cluster.platform_kubernetes.namespace).not_to eq('invalid_namespace')
           expect(cluster.kubernetes_namespace.namespace).not_to eq('invalid_namespace')
         end
 
         it 'should return validation errors' do
-          expect(json_response['message']).to be_present
           expect(json_response['message']['platform_kubernetes.namespace'].first).to match('can contain only lowercase letters')
         end
       end
 
       context 'with a GCP cluster' do
-        let(:update_params) do
-          base_params.merge(
-            name: 'new-name',
-            api_url: 'https://new-api-url.com',
-            token: 'new-sample-token'
-          )
+        context 'when user tries to change GCP specific fields' do
+          let(:platform_kubernetes_attributes) do
+            {
+              api_url: 'https://new-api-url.com',
+              token: 'new-sample-token'
+            }
+          end
+
+          it 'should respond with 400' do
+            expect(response).to have_gitlab_http_status(400)
+          end
+
+          it 'should return validation error' do
+            expect(json_response['message']['platform_kubernetes.base'].first).to eq('Cannot modify managed Kubernetes cluster')
+          end
         end
 
-        it 'should ignore platform kubernetes specific attributes' do
-          platform_kubernetes = cluster.platform_kubernetes
+        context 'when user tries to change namespace' do
+          let(:namespace) { 'new-namespace' }
 
-          expect(cluster.name).not_to eq('new-name')
-          expect(platform_kubernetes.namespace).to eq('new-namespace')
-          expect(platform_kubernetes.api_url).not_to eq('https://new-api-url.com')
-          expect(platform_kubernetes.token).not_to eq('new-sample-token')
+          it 'should respond with 200' do
+            expect(response).to have_gitlab_http_status(200)
+          end
         end
       end
 
@@ -333,15 +370,26 @@ describe API::ProjectClusters do
                  projects: [project])
         end
 
+        let(:platform_kubernetes_attributes) do
+          {
+            api_url: api_url,
+            namespace: 'new-namespace',
+            token: 'new-sample-token'
+          }
+        end
+
         let(:update_params) do
-          base_params.merge(
+          {
             name: 'new-name',
-            api_url: 'https://new-api-url.com',
-            token: 'new-sample-token'
-          )
+            platform_kubernetes_attributes: platform_kubernetes_attributes
+          }
+        end
+
+        it 'should respond with 200' do
+          expect(response).to have_gitlab_http_status(200)
         end
 
-        it 'should not ignore platform kubernetes specific attributes' do
+        it 'should update platform kubernetes attributes' do
           platform_kubernetes = cluster.platform_kubernetes
 
           expect(cluster.name).to eq('new-name')
@@ -354,7 +402,7 @@ describe API::ProjectClusters do
       context 'with a cluster that does not belong to user' do
         let(:cluster) { create(:cluster, :project, :provided_by_user) }
 
-        it 'should respond with 400' do
+        it 'should respond with 404' do
           expect(response).to have_gitlab_http_status(404)
         end
       end