Commit cd9057e1 authored by Thong Kuah's avatar Thong Kuah Committed by Ash McKenzie

Revert "Handle forbidden error when checking for knative"

This reverts commit f0631507.
parent efe84ce0
# frozen_string_literal: true
module Clusters
class KnativeServingNamespaceFinder
attr_reader :cluster
def initialize(cluster)
@cluster = cluster
end
def execute
cluster.kubeclient&.get_namespace(Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE)
rescue Kubeclient::ResourceNotFoundError
nil
rescue Kubeclient::HttpError => e
# If the kubernetes auth engine is enabled, it will return 403
if e.error_code == 403
Gitlab::ErrorTracking.track_exception(e)
nil
else
raise
end
end
end
end
# frozen_string_literal: true
module Clusters
class KnativeVersionRoleBindingFinder
attr_reader :cluster
def initialize(cluster)
@cluster = cluster
end
def execute
cluster.kubeclient&.get_cluster_role_binding(Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME)
rescue Kubeclient::ResourceNotFoundError
nil
end
end
end
......@@ -12,8 +12,5 @@ module Clusters
GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME = 'gitlab-knative-serving-rolebinding'
GITLAB_CROSSPLANE_DATABASE_ROLE_NAME = 'gitlab-crossplane-database-role'
GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME = 'gitlab-crossplane-database-rolebinding'
GITLAB_KNATIVE_VERSION_ROLE_NAME = 'gitlab-knative-version-role'
GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME = 'gitlab-knative-version-rolebinding'
KNATIVE_SERVING_NAMESPACE = 'knative-serving'
end
end
......@@ -49,14 +49,8 @@ module Clusters
create_or_update_knative_serving_role
create_or_update_knative_serving_role_binding
create_or_update_crossplane_database_role
create_or_update_crossplane_database_role_binding
return unless knative_serving_namespace
create_or_update_knative_version_role
create_or_update_knative_version_role_binding
end
private
......@@ -70,12 +64,6 @@ module Clusters
).ensure_exists!
end
def knative_serving_namespace
kubeclient.get_namespace(Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE)
rescue Kubeclient::ResourceNotFoundError
nil
end
def create_role_or_cluster_role_binding
if namespace_creator
kubeclient.create_or_update_role_binding(role_binding_resource)
......@@ -100,14 +88,6 @@ module Clusters
kubeclient.update_role_binding(crossplane_database_role_binding_resource)
end
def create_or_update_knative_version_role
kubeclient.update_cluster_role(knative_version_role_resource)
end
def create_or_update_knative_version_role_binding
kubeclient.update_cluster_role_binding(knative_version_role_binding_resource)
end
def service_account_resource
Gitlab::Kubernetes::ServiceAccount.new(
service_account_name,
......@@ -186,27 +166,6 @@ module Clusters
service_account_name: service_account_name
).generate
end
def knative_version_role_resource
Gitlab::Kubernetes::ClusterRole.new(
name: Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_NAME,
rules: [{
apiGroups: %w(apps),
resources: %w(deployments),
verbs: %w(list get)
}]
).generate
end
def knative_version_role_binding_resource
subjects = [{ kind: 'ServiceAccount', name: service_account_name, namespace: service_account_namespace }]
Gitlab::Kubernetes::ClusterRoleBinding.new(
Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME,
Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_NAME,
subjects
).generate
end
end
end
end
---
title: Reverts Add RBAC permissions for getting knative version
merge_request: 22560
author:
type: fixed
......@@ -118,8 +118,7 @@ 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. It will also
need list access to the deployments in the `knative-serving` namespace.
provided can manage resources in the `serving.knative.dev` API group.
- For a GitLab managed cluster, if you added the cluster in [GitLab 12.1 or later](https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/30235),
then GitLab will already have the required access and you can proceed to the next step.
......@@ -156,19 +155,6 @@ You must do the following:
- delete
- patch
- watch
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: gitlab-knative-version-role
rules:
- apiGroups:
- apps
resources:
- deployments
verbs:
- list
- get
```
Then run the following command:
......
......@@ -8,7 +8,7 @@ module Gitlab
def unmet?
deployment_cluster.present? &&
deployment_cluster.managed? &&
(missing_namespace? || need_knative_version_role_binding?)
missing_namespace?
end
def complete!
......@@ -23,10 +23,6 @@ module Gitlab
kubernetes_namespace.nil? || kubernetes_namespace.service_account_token.blank?
end
def need_knative_version_role_binding?
!knative_serving_namespace.nil? && knative_version_role_binding.nil?
end
def deployment_cluster
build.deployment&.cluster
end
......@@ -35,22 +31,6 @@ module Gitlab
build.deployment.environment
end
def knative_serving_namespace
strong_memoize(:knative_serving_namespace) do
Clusters::KnativeServingNamespaceFinder.new(
deployment_cluster
).execute
end
end
def knative_version_role_binding
strong_memoize(:knative_version_role_binding) do
Clusters::KnativeVersionRoleBindingFinder.new(
deployment_cluster
).execute
end
end
def kubernetes_namespace
strong_memoize(:kubernetes_namespace) do
Clusters::KubernetesNamespaceFinder.new(
......
# frozen_string_literal: true
module Gitlab
module Kubernetes
class ClusterRole
attr_reader :name, :rules
def initialize(name:, rules:)
@name = name
@rules = rules
end
def generate
::Kubeclient::Resource.new(
metadata: metadata,
rules: rules
)
end
private
def metadata
{
name: name
}
end
end
end
end
......@@ -57,7 +57,6 @@ module Gitlab
# group client
delegate :create_cluster_role_binding,
:get_cluster_role_binding,
:get_cluster_role_bindings,
:update_cluster_role_binding,
to: :rbac_client
......@@ -68,13 +67,6 @@ module Gitlab
:update_role,
to: :rbac_client
# RBAC methods delegates to the apis/rbac.authorization.k8s.io api
# group client
delegate :create_cluster_role,
:get_cluster_role,
:update_cluster_role,
to: :rbac_client
# RBAC methods delegates to the apis/rbac.authorization.k8s.io api
# group client
delegate :create_role_binding,
......
# frozen_string_literal: true
require 'spec_helper'
describe Clusters::KnativeServingNamespaceFinder do
include KubernetesHelpers
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:service) { environment.deployment_platform }
let(:project) { cluster.cluster_project.project }
let(:environment) { create(:environment, project: project) }
subject { Clusters::KnativeServingNamespaceFinder.new(cluster) }
before do
stub_kubeclient_discover(service.api_url)
end
it 'finds the namespace in a cluster where it exists' do
stub_kubeclient_get_namespace(service.api_url, namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE)
expect(subject.execute).to be_a Kubeclient::Resource
end
it 'returns nil in a cluster where it does not' do
stub_kubeclient_get_namespace(
service.api_url,
namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE,
response: {
status: [404, "Resource Not Found"]
}
)
expect(subject.execute).to be nil
end
it 'returns nil in a cluster where the lookup results in a 403 as it will in some versions of kubernetes' do
stub_kubeclient_get_namespace(
service.api_url,
namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE,
response: {
status: [403, "Resource Not Found"]
}
)
expect(subject.execute).to be nil
end
it 'raises an error if error code is not 404 or 403' do
stub_kubeclient_get_namespace(
service.api_url,
namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE,
response: {
status: [500, "Internal Server Error"]
}
)
expect { subject.execute }.to raise_error(Kubeclient::HttpError)
end
end
......@@ -38,44 +38,12 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do
.and_return(double(execute: kubernetes_namespace))
end
context 'and the knative-serving namespace is missing' do
before do
allow(Clusters::KnativeServingNamespaceFinder).to receive(:new)
.and_return(double(execute: false))
end
it { is_expected.to be_truthy }
end
context 'and the knative-serving namespace exists' do
before do
allow(Clusters::KnativeServingNamespaceFinder).to receive(:new)
.and_return(double(execute: true))
end
context 'and the knative version role binding is missing' do
before do
allow(Clusters::KnativeVersionRoleBindingFinder).to receive(:new)
.and_return(double(execute: nil))
end
it { is_expected.to be_truthy }
end
context 'and the knative version role binding already exists' do
before do
allow(Clusters::KnativeVersionRoleBindingFinder).to receive(:new)
.and_return(double(execute: true))
end
it { is_expected.to be_falsey }
it { is_expected.to be_falsey }
context 'and the service_account_token is blank' do
let(:kubernetes_namespace) { instance_double(Clusters::KubernetesNamespace, service_account_token: nil) }
context 'and the service_account_token is blank' do
let(:kubernetes_namespace) { instance_double(Clusters::KubernetesNamespace, service_account_token: nil) }
it { is_expected.to be_truthy }
end
end
it { is_expected.to be_truthy }
end
end
end
......@@ -188,24 +156,6 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do
subject
end
end
context 'knative version role binding is missing' do
before do
allow(Clusters::KubernetesNamespaceFinder).to receive(:new)
.and_return(double(execute: kubernetes_namespace))
allow(Clusters::KnativeVersionRoleBindingFinder).to receive(:new)
.and_return(double(execute: nil))
end
it 'creates the knative version role binding' do
expect(Clusters::Kubernetes::CreateOrUpdateNamespaceService)
.to receive(:new)
.with(cluster: cluster, kubernetes_namespace: kubernetes_namespace)
.and_return(service)
subject
end
end
end
context 'completion is not required' do
......
......@@ -22,6 +22,7 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do
before do
stub_kubeclient_discover(api_url)
stub_kubeclient_get_namespace(api_url)
stub_kubeclient_get_service_account_error(api_url, 'gitlab')
stub_kubeclient_create_service_account(api_url)
stub_kubeclient_get_secret_error(api_url, 'gitlab-token')
......@@ -30,7 +31,6 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do
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_namespace(api_url, namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE)
stub_kubeclient_get_service_account_error(api_url, "#{namespace}-service-account", namespace: namespace)
stub_kubeclient_create_service_account(api_url, namespace: namespace)
stub_kubeclient_create_secret(api_url, namespace: namespace)
......@@ -39,8 +39,6 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do
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)
stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME, namespace: namespace)
stub_kubeclient_put_cluster_role(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_NAME)
stub_kubeclient_put_cluster_role_binding(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME)
stub_kubeclient_get_secret(
api_url,
......
......@@ -141,15 +141,12 @@ describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
before do
cluster.platform_kubernetes.rbac!
stub_kubeclient_get_namespace(api_url, namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE)
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::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)
stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME, namespace: namespace)
stub_kubeclient_put_cluster_role(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_NAME)
stub_kubeclient_put_cluster_role_binding(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME)
end
it_behaves_like 'creates service account and token'
......@@ -237,30 +234,6 @@ describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
)
)
end
it 'creates a role and role binding granting the ability to get the version of deployments in knative-serving namespace' do
subject
expect(WebMock).to have_requested(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/#{Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME}").with(
body: hash_including(
metadata: {
name: Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME
},
roleRef: {
apiGroup: "rbac.authorization.k8s.io",
kind: "ClusterRole",
name: Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_NAME
},
subjects: [
{
kind: "ServiceAccount",
name: service_account_name,
namespace: namespace
}
]
)
)
end
end
end
end
......@@ -13,7 +13,4 @@ describe Clusters::Kubernetes do
it { is_expected.to be_const_defined(:GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME) }
it { is_expected.to be_const_defined(:GITLAB_CROSSPLANE_DATABASE_ROLE_NAME) }
it { is_expected.to be_const_defined(:GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME) }
it { is_expected.to be_const_defined(:GITLAB_KNATIVE_VERSION_ROLE_NAME) }
it { is_expected.to be_const_defined(:GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME) }
it { is_expected.to be_const_defined(:KNATIVE_SERVING_NAMESPACE) }
end
......@@ -202,11 +202,6 @@ module KubernetesHelpers
.to_return(kube_response({}))
end
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
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({}))
......@@ -232,18 +227,8 @@ module KubernetesHelpers
.to_return(kube_response({}))
end
def stub_kubeclient_get_namespaces(api_url)
WebMock.stub_request(:get, api_url + '/api/v1/namespaces')
.to_return(kube_response(kube_v1_namespace_list_body))
end
def stub_kubeclient_get_namespace(api_url, namespace: 'default', response: kube_response({}))
def stub_kubeclient_get_namespace(api_url, namespace: 'default')
WebMock.stub_request(:get, api_url + "/api/v1/namespaces/#{namespace}")
.to_return(response)
end
def stub_kubeclient_put_cluster_role(api_url, name)
WebMock.stub_request(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterroles/#{name}")
.to_return(kube_response({}))
end
......@@ -290,20 +275,6 @@ module KubernetesHelpers
}
end
def kube_v1_namespace_list_body
{
"kind" => "NamespaceList",
"apiVersion" => "v1",
"items" => [
{
"metadata" => {
"name" => "knative-serving"
}
}
]
}
end
def kube_v1beta1_discovery_body
{
"kind" => "APIResourceList",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment