Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/registry/components/collapsible_container.vue

@@ -31,6 +31,7 @@ export default {
   data() {
     return {
       isOpen: false,
+      modalId: `confirm-repo-deletion-modal-${this.repo.id}`,
     };
   },
   computed: {
@@ -80,7 +81,7 @@ export default {
         <gl-button
           v-if="repo.canDelete"
           v-gl-tooltip
-          v-gl-modal="'confirm-repo-deletion-modal'"
+          v-gl-modal="modalId"
           :title="s__('ContainerRegistry|Remove repository')"
           :aria-label="s__('ContainerRegistry|Remove repository')"
           class="js-remove-repo"
@@ -100,12 +101,7 @@ export default {
         {{ s__('ContainerRegistry|No tags in Container Registry for this container image.') }}
       </div>
     </div>
-
-    <gl-modal
-      modal-id="confirm-repo-deletion-modal"
-      ok-variant="danger"
-      @ok="handleDeleteRepository"
-    >
+    <gl-modal :modal-id="modalId" ok-variant="danger" @ok="handleDeleteRepository">
       <template v-slot:modal-title>{{ s__('ContainerRegistry|Remove repository') }}</template>
       <p
         v-html="

app/assets/javascripts/registry/components/table_registry.vue

@@ -32,6 +32,7 @@ export default {
   data() {
     return {
       itemToBeDeleted: null,
+      modalId: `confirm-image-deletion-modal-${this.repo.id}`,
     };
   },
   computed: {
@@ -114,7 +115,7 @@ export default {
             <gl-button
               v-if="item.canDelete"
               v-gl-tooltip
-              v-gl-modal="'confirm-image-deletion-modal'"
+              v-gl-modal="modalId"
               :title="s__('ContainerRegistry|Remove image')"
               :aria-label="s__('ContainerRegistry|Remove image')"
               variant="danger"
@@ -134,11 +135,7 @@ export default {
       :page-info="repo.pagination"
     />
 
-    <gl-modal
-      modal-id="confirm-image-deletion-modal"
-      ok-variant="danger"
-      @ok="handleDeleteRegistry"
-    >
+    <gl-modal :modal-id="modalId" ok-variant="danger" @ok="handleDeleteRegistry">
       <template v-slot:modal-title>{{ s__('ContainerRegistry|Remove image') }}</template>
       <template v-slot:modal-ok>{{ s__('ContainerRegistry|Remove image and tags') }}</template>
       <p

app/services/clusters/gcp/kubernetes.rb

@@ -9,6 +9,8 @@ module Clusters
       GITLAB_CLUSTER_ROLE_BINDING_NAME = 'gitlab-admin'
       GITLAB_CLUSTER_ROLE_NAME = 'cluster-admin'
       PROJECT_CLUSTER_ROLE_NAME = 'edit'
+      GITLAB_KNATIVE_SERVING_ROLE_NAME = 'gitlab-knative-serving-role'
+      GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME = 'gitlab-knative-serving-rolebinding'
     end
   end
 end

app/services/clusters/gcp/kubernetes/create_or_update_service_account_service.rb

@@ -41,7 +41,15 @@ module Clusters
 
           kubeclient.create_or_update_service_account(service_account_resource)
           kubeclient.create_or_update_secret(service_account_token_resource)
-          create_role_or_cluster_role_binding if rbac
+
+          return unless rbac
+
+          create_role_or_cluster_role_binding
+
+          return unless namespace_creator
+
+          create_or_update_knative_serving_role
+          create_or_update_knative_serving_role_binding
         end
 
         private
@@ -63,6 +71,14 @@ module Clusters
           end
         end
 
+        def create_or_update_knative_serving_role
+          kubeclient.update_role(knative_serving_role_resource)
+        end
+
+        def create_or_update_knative_serving_role_binding
+          kubeclient.update_role_binding(knative_serving_role_binding_resource)
+        end
+
         def service_account_resource
           Gitlab::Kubernetes::ServiceAccount.new(
             service_account_name,
@@ -92,6 +108,29 @@ module Clusters
           Gitlab::Kubernetes::RoleBinding.new(
             name: role_binding_name,
             role_name: Clusters::Gcp::Kubernetes::PROJECT_CLUSTER_ROLE_NAME,
+            role_kind: :ClusterRole,
+            namespace: service_account_namespace,
+            service_account_name: service_account_name
+          ).generate
+        end
+
+        def knative_serving_role_resource
+          Gitlab::Kubernetes::Role.new(
+            name: Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME,
+            namespace: service_account_namespace,
+            rules: [{
+              apiGroups: %w(serving.knative.dev),
+              resources: %w(configurations configurationgenerations routes revisions revisionuids autoscalers services),
+              verbs: %w(get list create update delete patch watch)
+            }]
+          ).generate
+        end
+
+        def knative_serving_role_binding_resource
+          Gitlab::Kubernetes::RoleBinding.new(
+            name: Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME,
+            role_name: Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME,
+            role_kind: :Role,
             namespace: service_account_namespace,
             service_account_name: service_account_name
           ).generate

changelogs/unreleased/hfy-apply-knative-cluster-role-on-service-account-creation.yml

+---
+title: Create Knative role and binding with service account
+merge_request: 30235
+author:
+type: changed

changelogs/unreleased/registry-fix-multi-delete-modal.yml

+---
+title: Prevent multiple confirmation modals from opening when deleting a repository
+merge_request: 30532
+author:
+type: fixed

doc/user/project/clusters/serverless/index.md

@@ -102,12 +102,15 @@ You must do the following:
 1. Ensure GitLab can manage Knative:
     - For a non-GitLab managed cluster, ensure that the service account for the token
       provided can manage resources in the `serving.knative.dev` API group.
-    - For a GitLab managed cluster,
-      GitLab uses a service account with the `edit` cluster role. This account needs
-      the ability to manage resources in the `serving.knative.dev` API group.
-      We suggest you do this with an [aggregated ClusterRole](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#aggregated-clusterroles)
-      adding rules to the default `edit` cluster role:
-      First, save the following YAML as `knative-serving-only-role.yaml`:
+    - For a GitLab managed cluster, if you added the cluster in [GitLab 12.1 or later](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/30235),
+      then GitLab will already have the required access and you can proceed to the next step.
+
+      Otherwise, you need to manually grant GitLab's service account the ability to manage
+      resources in the `serving.knative.dev` API group. Since every GitLab service account
+      has the `edit` cluster role, the simplest way to do this is with an
+      [aggregated ClusterRole](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#aggregated-clusterroles)
+      adding rules to the default `edit` cluster role: First, save the following YAML as
+      `knative-serving-only-role.yaml`:
 
       ```yaml
       apiVersion: rbac.authorization.k8s.io/v1
@@ -143,6 +146,9 @@ You must do the following:
         kubectl apply -f knative-serving-only-role.yaml
         ```
 
+        If you would rather grant permissions on a per service account basis, you can do this
+        using a `Role` and `RoleBinding` specific to the service account and namespace.
+
 1. Follow the steps to deploy [functions](#deploying-functions)
    or [serverless applications](#deploying-serverless-applications) onto your
    cluster.
@@ -376,13 +382,13 @@ cluster.
 By default, a GitLab serverless deployment will be served over `http`. In order to serve over `https` you
 must manually obtain and install TLS certificates.
 
-The simplest way to accomplish this is to 
+The simplest way to accomplish this is to
 use [Certbot to manually obtain Let's Encrypt certificates](https://knative.dev/docs/serving/using-a-tls-cert/#using-certbot-to-manually-obtain-let-s-encrypt-certificates). Certbot is a free, open source software tool for automatically using Let’s Encrypt certificates on manually-administrated websites to enable HTTPS.
 
 NOTE: **Note:**
 The instructions below relate to installing and running Certbot on a Linux server and may not work on other operating systems.
 
-1. Install Certbot by running the 
+1. Install Certbot by running the
    [`certbot-auto` wrapper script](https://certbot.eff.org/docs/install.html#certbot-auto).
    On the command line of your server, run the following commands:
 
@@ -594,7 +600,7 @@ The instructions below relate to installing and running Certbot on a Linux serve
     Where `cert.pem` and `cert.pk` are your certificate and private key files. Note that the `istio-ingressgateway-certs` secret name is required.
 
 1. Configure Knative to use the new secret that you created for HTTPS
-   connections. Run the 
+   connections. Run the
   following command to open the Knative shared `gateway` in edit mode:
 
     ```sh
@@ -641,4 +647,4 @@ The instructions below relate to installing and running Certbot on a Linux serve
 
       After your changes are running on your Knative cluster, you can begin using the HTTPS protocol for secure access your deployed Knative services.
       In the event a mistake is made during this process and you need to update the cert, you will need to edit the gateway `knative-ingress-gateway`
-      to switch back to `PASSTHROUGH` mode. Once corrections are made, edit the file again so the gateway will use the new certificates.
+      to switch back to `PASSTHROUGH` mode. Once corrections are made, edit the file again so the gateway will use the new certificates.
\ No newline at end of file

lib/gitlab/kubernetes/kube_client.rb

@@ -57,6 +57,13 @@ module Gitlab
         :update_cluster_role_binding,
         to: :rbac_client
 
+      # RBAC methods delegates to the apis/rbac.authorization.k8s.io api
+      # group client
+      delegate :create_role,
+      :get_role,
+      :update_role,
+      to: :rbac_client
+
       # RBAC methods delegates to the apis/rbac.authorization.k8s.io api
       # group client
       delegate :create_role_binding,

lib/gitlab/kubernetes/role.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Kubernetes
+    class Role
+      def initialize(name:, namespace:, rules:)
+        @name = name
+        @namespace = namespace
+        @rules = rules
+      end
+
+      def generate
+        ::Kubeclient::Resource.new(
+          metadata: { name: name, namespace: namespace },
+          rules: rules
+        )
+      end
+
+      private
+
+      attr_reader :name, :namespace, :rules
+    end
+  end
+end

lib/gitlab/kubernetes/role_binding.rb

@@ -3,9 +3,10 @@
 module Gitlab
   module Kubernetes
     class RoleBinding
-      def initialize(name:, role_name:, namespace:, service_account_name:)
+      def initialize(name:, role_name:, role_kind:, namespace:, service_account_name:)
         @name = name
         @role_name = role_name
+        @role_kind = role_kind
         @namespace = namespace
         @service_account_name = service_account_name
       end
@@ -20,7 +21,7 @@ module Gitlab
 
       private
 
-      attr_reader :name, :role_name, :namespace, :service_account_name
+      attr_reader :name, :role_name, :role_kind, :namespace, :service_account_name
 
       def metadata
         { name: name, namespace: namespace }
@@ -29,7 +30,7 @@ module Gitlab
       def role_ref
         {
           apiGroup: 'rbac.authorization.k8s.io',
-          kind: 'ClusterRole',
+          kind: role_kind,
           name: role_name
         }
       end

spec/factories/services.rb

@@ -100,4 +100,16 @@ FactoryBot.define do
     type 'HipchatService'
     token 'test_token'
   end
+
+  trait :without_properties_callback do
+    after(:build) do |service|
+      allow(service).to receive(:handle_properties)
+    end
+
+    after(:create) do |service|
+      # we have to remove the stub because the behaviour of
+      # handle_properties method is tested after the creation
+      allow(service).to receive(:handle_properties).and_call_original
+    end
+  end
 end

spec/javascripts/registry/components/collapsible_container_spec.js

@@ -77,7 +77,7 @@ describe('collapsible registry container', () => {
       spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve());
 
       Vue.nextTick(() => {
-        document.querySelector('#confirm-repo-deletion-modal .btn-danger').click();
+        document.querySelector(`#${vm.modalId} .btn-danger`).click();
 
         expect(vm.deleteItem).toHaveBeenCalledWith(vm.repo);
         done();

spec/javascripts/registry/components/table_registry_spec.js

@@ -51,7 +51,7 @@ describe('table registry', () => {
       spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve());
 
       Vue.nextTick(() => {
-        document.querySelector('#confirm-image-deletion-modal .btn-danger').click();
+        document.querySelector(`#${vm.modalId} .btn-danger`).click();
 
         expect(vm.deleteItem).toHaveBeenCalledWith(firstImage);
         expect(vm.itemToBeDeleted).toBeNull();

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -176,6 +176,9 @@ describe Gitlab::Kubernetes::KubeClient do
     let(:rbac_client) { client.rbac_client }
 
     [
+      :create_role,
+      :get_role,
+      :update_role,
       :create_cluster_role_binding,
       :get_cluster_role_binding,
       :update_cluster_role_binding

spec/lib/gitlab/kubernetes/role_binding_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe Gitlab::Kubernetes::RoleBinding, '#generate' do
   let(:role_name) { 'edit' }
+  let(:role_kind) { 'ClusterRole' }
   let(:namespace) { 'my-namespace' }
   let(:service_account_name) { 'my-service-account' }
 
@@ -20,7 +21,7 @@ describe Gitlab::Kubernetes::RoleBinding, '#generate' do
   let(:role_ref) do
     {
       apiGroup: 'rbac.authorization.k8s.io',
-      kind: 'ClusterRole',
+      kind: role_kind,
       name: role_name
     }
   end
@@ -37,6 +38,7 @@ describe Gitlab::Kubernetes::RoleBinding, '#generate' do
     described_class.new(
       name: "gitlab-#{namespace}",
       role_name: role_name,
+      role_kind: role_kind,
       namespace: namespace,
       service_account_name: service_account_name
     ).generate

spec/lib/gitlab/kubernetes/role_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Kubernetes::Role do
+  let(:role) { described_class.new(name: name, namespace: namespace, rules: rules) }
+  let(:name) { 'example-name' }
+  let(:namespace) { 'example-namespace' }
+
+  let(:rules) do
+    [{
+      apiGroups: %w(hello.world),
+      resources: %w(oil diamonds coffee),
+      verbs: %w(say do walk run)
+    }]
+  end
+
+  describe '#generate' do
+    subject { role.generate }
+
+    let(:resource) do
+      ::Kubeclient::Resource.new(
+        metadata: { name: name, namespace: namespace },
+        rules: rules
+      )
+    end
+
+    it { is_expected.to eq(resource) }
+  end
+end

spec/models/project_services/bugzilla_service_spec.rb

@@ -44,7 +44,9 @@ describe BugzillaService do
     # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
     context 'when data are stored in properties' do
       let(:properties) { access_params.merge(title: title, description: description) }
-      let(:service) { create(:bugzilla_service, properties: properties) }
+      let(:service) do
+        create(:bugzilla_service, :without_properties_callback, properties: properties)
+      end
 
       include_examples 'issue tracker fields'
     end
@@ -60,7 +62,7 @@ describe BugzillaService do
     context 'when data are stored in both properties and separated fields' do
       let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
       let(:service) do
-        create(:bugzilla_service, title: title, description: description, properties: properties)
+        create(:bugzilla_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
       include_examples 'issue tracker fields'

spec/models/project_services/custom_issue_tracker_service_spec.rb

@@ -58,7 +58,9 @@ describe CustomIssueTrackerService do
     # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
     context 'when data are stored in properties' do
       let(:properties) { access_params.merge(title: title, description: description) }
-      let(:service) { create(:custom_issue_tracker_service, properties: properties) }
+      let(:service) do
+        create(:custom_issue_tracker_service, :without_properties_callback, properties: properties)
+      end
 
       include_examples 'issue tracker fields'
     end
@@ -74,7 +76,7 @@ describe CustomIssueTrackerService do
     context 'when data are stored in both properties and separated fields' do
       let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
       let(:service) do
-        create(:custom_issue_tracker_service, title: title, description: description, properties: properties)
+        create(:custom_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
       include_examples 'issue tracker fields'

spec/models/project_services/gitlab_issue_tracker_service_spec.rb

@@ -61,7 +61,9 @@ describe GitlabIssueTrackerService do
     # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
     context 'when data are stored in properties' do
       let(:properties) { access_params.merge(title: title, description: description) }
-      let(:service) { create(:gitlab_issue_tracker_service, properties: properties) }
+      let(:service) do
+        create(:gitlab_issue_tracker_service, :without_properties_callback, properties: properties)
+      end
 
       include_examples 'issue tracker fields'
     end
@@ -77,7 +79,7 @@ describe GitlabIssueTrackerService do
     context 'when data are stored in both properties and separated fields' do
       let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
       let(:service) do
-        create(:gitlab_issue_tracker_service, title: title, description: description, properties: properties)
+        create(:gitlab_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
       include_examples 'issue tracker fields'

spec/models/project_services/jira_service_spec.rb

@@ -145,7 +145,9 @@ describe JiraService do
     # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
     context 'when data are stored in properties' do
       let(:properties) { access_params.merge(title: title, description: description) }
-      let(:service) { create(:jira_service, properties: properties) }
+      let(:service) do
+        create(:jira_service, :without_properties_callback, properties: properties)
+      end
 
       include_examples 'issue tracker fields'
     end
@@ -161,7 +163,7 @@ describe JiraService do
     context 'when data are stored in both properties and separated fields' do
       let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
       let(:service) do
-        create(:jira_service, title: title, description: description, properties: properties)
+        create(:jira_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
       include_examples 'issue tracker fields'

spec/models/project_services/redmine_service_spec.rb

@@ -50,7 +50,9 @@ describe RedmineService do
     # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
     context 'when data are stored in properties' do
       let(:properties) { access_params.merge(title: title, description: description) }
-      let(:service) { create(:redmine_service, properties: properties) }
+      let(:service) do
+        create(:redmine_service, :without_properties_callback, properties: properties)
+      end
 
       include_examples 'issue tracker fields'
     end
@@ -66,7 +68,7 @@ describe RedmineService do
     context 'when data are stored in both properties and separated fields' do
       let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
       let(:service) do
-        create(:redmine_service, title: title, description: description, properties: properties)
+        create(:redmine_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
       include_examples 'issue tracker fields'

spec/models/project_services/youtrack_service_spec.rb

@@ -47,7 +47,9 @@ describe YoutrackService do
     # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
     context 'when data are stored in properties' do
       let(:properties) { access_params.merge(title: title, description: description) }
-      let(:service) { create(:youtrack_service, properties: properties) }
+      let(:service) do
+        create(:youtrack_service, :without_properties_callback, properties: properties)
+      end
 
       include_examples 'issue tracker fields'
     end
@@ -63,7 +65,7 @@ describe YoutrackService do
     context 'when data are stored in both properties and separated fields' do
       let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
       let(:service) do
-        create(:youtrack_service, title: title, description: description, properties: properties)
+        create(:youtrack_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
       include_examples 'issue tracker fields'

spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb

@@ -34,6 +34,8 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' d
     stub_kubeclient_create_service_account(api_url, namespace: namespace)
     stub_kubeclient_create_secret(api_url, namespace: namespace)
     stub_kubeclient_put_secret(api_url, "#{namespace}-token", namespace: namespace)
+    stub_kubeclient_put_role(api_url, Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME, namespace: namespace)
+    stub_kubeclient_put_role_binding(api_url, Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME, namespace: namespace)
 
     stub_kubeclient_get_secret(
       api_url,

spec/services/clusters/gcp/kubernetes/create_or_update_service_account_service_spec.rb

@@ -143,6 +143,8 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateServiceAccountService do
 
         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(api_url, Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME, namespace: namespace)
+        stub_kubeclient_put_role_binding(api_url, Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME, namespace: namespace)
       end
 
       it_behaves_like 'creates service account and token'
@@ -169,6 +171,24 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateServiceAccountService do
           )
         )
       end
+
+      it 'creates a role and role binding granting knative serving permissions to the service account' do
+        subject
+
+        expect(WebMock).to have_requested(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/roles/#{Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME}").with(
+          body: hash_including(
+            metadata: {
+              name: Clusters::Gcp::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME,
+              namespace: namespace
+            },
+            rules: [{
+              apiGroups: %w(serving.knative.dev),
+              resources: %w(configurations configurationgenerations routes revisions revisionuids autoscalers services),
+              verbs: %w(get list create update delete patch watch)
+            }]
+          )
+        )
+      end
     end
   end
 end

spec/support/helpers/kubernetes_helpers.rb

@@ -199,6 +199,11 @@ module KubernetesHelpers
       .to_return(kube_response({}))
   end
 
+  def stub_kubeclient_put_role(api_url, name, namespace: 'default')
+    WebMock.stub_request(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/roles/#{name}")
+      .to_return(kube_response({}))
+  end
+
   def kube_v1_secret_body(**options)
     {
       "kind" => "SecretList",