Merge branch 'kubeclient-create-or-update' into 'master'

Uses Kubernetes API conventions to create or update a resource

Closes #29959

See merge request gitlab-org/gitlab!29010

changelogs/unreleased/kubeclient-create-or-update.yml

+---
+title: Uses Kubernetes API conventions to create or update a resource leandrogs
+merge_request: 29010
+author: Leandro Silva
+type: performance

ee/spec/requests/api/project_clusters_spec.rb

@@ -25,7 +25,6 @@ describe API::ProjectClusters do
       )
 
       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

lib/gitlab/kubernetes/helm/api.rb

@@ -99,11 +99,7 @@ module Gitlab
           command.cluster_role_binding_resource.tap do |cluster_role_binding_resource|
             break unless cluster_role_binding_resource
 
-            if cluster_role_binding_exists?(cluster_role_binding_resource)
-              kubeclient.update_cluster_role_binding(cluster_role_binding_resource)
-            else
-              kubeclient.create_cluster_role_binding(cluster_role_binding_resource)
-            end
+            kubeclient.update_cluster_role_binding(cluster_role_binding_resource)
           end
         end
 

lib/gitlab/kubernetes/kube_client.rb

@@ -57,9 +57,7 @@ module Gitlab
 
       # RBAC methods delegates to the apis/rbac.authorization.k8s.io api
       # group client
-      delegate :create_cluster_role_binding,
-        :get_cluster_role_binding,
-        :update_cluster_role_binding,
+      delegate :update_cluster_role_binding,
         to: :rbac_client
 
       # RBAC methods delegates to the apis/rbac.authorization.k8s.io api
@@ -71,9 +69,7 @@ module Gitlab
 
       # RBAC methods delegates to the apis/rbac.authorization.k8s.io api
       # group client
-      delegate :create_role_binding,
-        :get_role_binding,
-        :update_role_binding,
+      delegate :update_role_binding,
         to: :rbac_client
 
       # non-entity methods that can only work with the core client
@@ -134,19 +130,11 @@ module Gitlab
       end
 
       def create_or_update_cluster_role_binding(resource)
-        if cluster_role_binding_exists?(resource)
-          update_cluster_role_binding(resource)
-        else
-          create_cluster_role_binding(resource)
-        end
+        update_cluster_role_binding(resource)
       end
 
       def create_or_update_role_binding(resource)
-        if role_binding_exists?(resource)
-          update_role_binding(resource)
-        else
-          create_role_binding(resource)
-        end
+        update_role_binding(resource)
       end
 
       def create_or_update_service_account(resource)
@@ -173,18 +161,6 @@ module Gitlab
         Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false)
       end
 
-      def cluster_role_binding_exists?(resource)
-        get_cluster_role_binding(resource.metadata.name)
-      rescue ::Kubeclient::ResourceNotFoundError
-        false
-      end
-
-      def role_binding_exists?(resource)
-        get_role_binding(resource.metadata.name, resource.metadata.namespace)
-      rescue ::Kubeclient::ResourceNotFoundError
-        false
-      end
-
       def service_account_exists?(resource)
         get_service_account(resource.metadata.name, resource.metadata.namespace)
       rescue ::Kubeclient::ResourceNotFoundError

spec/lib/gitlab/kubernetes/helm/api_spec.rb

@@ -92,7 +92,6 @@ describe Gitlab::Kubernetes::Helm::API do
       allow(client).to receive(:get_config_map).and_return(nil)
       allow(client).to receive(:create_config_map).and_return(nil)
       allow(client).to receive(:create_service_account).and_return(nil)
-      allow(client).to receive(:create_cluster_role_binding).and_return(nil)
       allow(client).to receive(:delete_pod).and_return(nil)
       allow(namespace).to receive(:ensure_exists!).once
     end
@@ -136,7 +135,7 @@ describe Gitlab::Kubernetes::Helm::API do
     context 'without a service account' do
       it 'does not create a service account on kubeclient' do
         expect(client).not_to receive(:create_service_account)
-        expect(client).not_to receive(:create_cluster_role_binding)
+        expect(client).not_to receive(:update_cluster_role_binding)
 
         subject.install(command)
       end
@@ -160,15 +159,14 @@ describe Gitlab::Kubernetes::Helm::API do
           )
         end
 
-        context 'service account and cluster role binding does not exist' do
+        context 'service account does not exist' do
           before do
             expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
-            expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
           end
 
           it 'creates a service account, followed the cluster role binding on kubeclient' do
             expect(client).to receive(:create_service_account).with(service_account_resource).once.ordered
-            expect(client).to receive(:create_cluster_role_binding).with(cluster_role_binding_resource).once.ordered
+            expect(client).to receive(:update_cluster_role_binding).with(cluster_role_binding_resource).once.ordered
 
             subject.install(command)
           end
@@ -177,21 +175,6 @@ describe Gitlab::Kubernetes::Helm::API do
         context 'service account already exists' do
           before do
             expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_return(service_account_resource)
-            expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
-          end
-
-          it 'updates the service account, followed by creating the cluster role binding' do
-            expect(client).to receive(:update_service_account).with(service_account_resource).once.ordered
-            expect(client).to receive(:create_cluster_role_binding).with(cluster_role_binding_resource).once.ordered
-
-            subject.install(command)
-          end
-        end
-
-        context 'service account and cluster role binding already exists' do
-          before do
-            expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_return(service_account_resource)
-            expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_return(cluster_role_binding_resource)
           end
 
           it 'updates the service account, followed by creating the cluster role binding' do
@@ -216,7 +199,7 @@ describe Gitlab::Kubernetes::Helm::API do
       context 'legacy abac cluster' do
         it 'does not create a service account on kubeclient' do
           expect(client).not_to receive(:create_service_account)
-          expect(client).not_to receive(:create_cluster_role_binding)
+          expect(client).not_to receive(:update_cluster_role_binding)
 
           subject.install(command)
         end

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -234,8 +234,6 @@ describe Gitlab::Kubernetes::KubeClient do
       :create_role,
       :get_role,
       :update_role,
-      :create_cluster_role_binding,
-      :get_cluster_role_binding,
       :update_cluster_role_binding
     ].each do |method|
       describe "##{method}" do
@@ -354,6 +352,16 @@ describe Gitlab::Kubernetes::KubeClient do
     end
   end
 
+  shared_examples 'create_or_update method using put' do
+    let(:update_method) { "update_#{resource_type}" }
+
+    it 'calls the update method' do
+      expect(client).to receive(update_method).with(resource)
+
+      subject
+    end
+  end
+
   shared_examples 'create_or_update method' do
     let(:get_method) { "get_#{resource_type}" }
     let(:update_method) { "update_#{resource_type}" }
@@ -393,7 +401,7 @@ describe Gitlab::Kubernetes::KubeClient do
 
     subject { client.create_or_update_cluster_role_binding(resource) }
 
-    it_behaves_like 'create_or_update method'
+    it_behaves_like 'create_or_update method using put'
   end
 
   describe '#create_or_update_role_binding' do
@@ -405,7 +413,7 @@ describe Gitlab::Kubernetes::KubeClient do
 
     subject { client.create_or_update_role_binding(resource) }
 
-    it_behaves_like 'create_or_update method'
+    it_behaves_like 'create_or_update method using put'
   end
 
   describe '#create_or_update_service_account' do

spec/services/clusters/gcp/finalize_creation_service_spec.rb

@@ -108,8 +108,7 @@ describe Clusters::Gcp::FinalizeCreationService, '#execute' do
         }
       )
 
-      stub_kubeclient_get_cluster_role_binding_error(api_url, 'gitlab-admin')
-      stub_kubeclient_create_cluster_role_binding(api_url)
+      stub_kubeclient_put_cluster_role_binding(api_url, 'gitlab-admin')
     end
   end
 

spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb

@@ -28,7 +28,6 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do
     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)

spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb

@@ -83,8 +83,7 @@ describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
       before do
         cluster.platform_kubernetes.rbac!
 
-        stub_kubeclient_get_cluster_role_binding_error(api_url, cluster_role_binding_name)
-        stub_kubeclient_create_cluster_role_binding(api_url)
+        stub_kubeclient_put_cluster_role_binding(api_url, cluster_role_binding_name)
       end
 
       it_behaves_like 'creates service account and token'
@@ -92,9 +91,8 @@ describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
       it 'creates a cluster role binding with cluster-admin access' do
         subject
 
-        expect(WebMock).to have_requested(:post, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings").with(
+        expect(WebMock).to have_requested(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/gitlab-admin").with(
           body: hash_including(
-            kind: 'ClusterRoleBinding',
             metadata: { name: 'gitlab-admin' },
             roleRef: {
               apiGroup: 'rbac.authorization.k8s.io',
@@ -143,8 +141,7 @@ describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
       before do
         cluster.platform_kubernetes.rbac!
 
-        stub_kubeclient_get_role_binding_error(api_url, role_binding_name, namespace: namespace)
-        stub_kubeclient_create_role_binding(api_url, namespace: namespace)
+        stub_kubeclient_put_role_binding(api_url, role_binding_name, namespace: namespace)
         stub_kubeclient_put_role(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME, namespace: namespace)
         stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME, namespace: namespace)
         stub_kubeclient_put_role(api_url, Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_NAME, namespace: namespace)
@@ -166,9 +163,8 @@ describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
       it 'creates a namespaced role binding with edit access' do
         subject
 
-        expect(WebMock).to have_requested(:post, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings").with(
+        expect(WebMock).to have_requested(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings/#{role_binding_name}").with(
           body: hash_including(
-            kind: 'RoleBinding',
             metadata: { name: "gitlab-#{namespace}", namespace: "#{namespace}" },
             roleRef: {
               apiGroup: 'rbac.authorization.k8s.io',

spec/support/helpers/kubernetes_helpers.rb

@@ -201,28 +201,8 @@ module KubernetesHelpers
       .to_return(kube_response({}))
   end
 
-  def stub_kubeclient_get_cluster_role_binding_error(api_url, name, status: 404)
-    WebMock.stub_request(:get, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/#{name}")
-      .to_return(status: [status, "Internal Server Error"])
-  end
-
-  def stub_kubeclient_create_cluster_role_binding(api_url)
-    WebMock.stub_request(:post, api_url + '/apis/rbac.authorization.k8s.io/v1/clusterrolebindings')
-      .to_return(kube_response({}))
-  end
-
-  def stub_kubeclient_get_role_binding(api_url, name, namespace: 'default')
-    WebMock.stub_request(:get, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings/#{name}")
-      .to_return(kube_response({}))
-  end
-
-  def stub_kubeclient_get_role_binding_error(api_url, name, namespace: 'default', status: 404)
-    WebMock.stub_request(:get, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings/#{name}")
-      .to_return(status: [status, "Internal Server Error"])
-  end
-
-  def stub_kubeclient_create_role_binding(api_url, namespace: 'default')
-    WebMock.stub_request(:post, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings")
+  def stub_kubeclient_put_cluster_role_binding(api_url, name)
+    WebMock.stub_request(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/#{name}")
       .to_return(kube_response({}))
   end