diff options
author | Tiger <twatson@gitlab.com> | 2019-06-14 03:18:50 +0300 |
---|---|---|
committer | Tiger <twatson@gitlab.com> | 2019-06-17 14:21:13 +0300 |
commit | ddd271b6027b13bca02416ec3dda17d3ec7fd5be (patch) | |
tree | 4cd5f7e7347d8e94ec4720291083e229af6ec1a5 | |
parent | b05de5a583e35931967dcc70d2f26f568c9cf0db (diff) |
Don't use Kubernetes namespaces with no token
Whenever we are selecting a namespace to use for a
deployment or to query a cluster we want to exclude
Kubernetes namespace records that don't have a token
set as they will not have the required permissions.
However when configuring clusters, we want to
use the original namespace record even if it has no
token, as a namespace has to be unique on a cluster.
5 files changed, 101 insertions, 5 deletions
diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index ccc877fb924..0206ce81c5f 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -193,15 +193,34 @@ module Clusters platform_kubernetes.kubeclient if kubernetes? end + ## + # This is subtly different to #find_or_initialize_kubernetes_namespace_for_project + # below because it will ignore any namespaces that have not got a service account + # token. This provides a guarantee that any namespace selected here can be used + # for cluster operations - a namespace needs to have a service account configured + # before it it can be used. + # + # This is used for selecting a namespace to use when querying a cluster, or + # generating variables to pass to CI. def kubernetes_namespace_for(project) - find_or_initialize_kubernetes_namespace_for_project(project).namespace + find_or_initialize_kubernetes_namespace_for_project( + project, scope: kubernetes_namespaces.has_service_account_token + ).namespace end - def find_or_initialize_kubernetes_namespace_for_project(project) + ## + # This is subtly different to #kubernetes_namespace_for because it will include + # namespaces that have yet to receive a service account token. This allows + # the namespace configuration process to be repeatable - if a namespace has + # already been created without a token we don't need to create another + # record entirely, just set the token on the pre-existing namespace. + # + # This is used for configuring cluster namespaces. + def find_or_initialize_kubernetes_namespace_for_project(project, scope: kubernetes_namespaces) attributes = { project: project } attributes[:cluster_project] = cluster_project if project_type? - kubernetes_namespaces.find_or_initialize_by(attributes).tap do |namespace| + scope.find_or_initialize_by(attributes).tap do |namespace| namespace.set_defaults end end diff --git a/changelogs/unreleased/63079-exclude-k8s-namespaces-with-no-service-account-token.yml b/changelogs/unreleased/63079-exclude-k8s-namespaces-with-no-service-account-token.yml new file mode 100644 index 00000000000..9dc99c8a62f --- /dev/null +++ b/changelogs/unreleased/63079-exclude-k8s-namespaces-with-no-service-account-token.yml @@ -0,0 +1,6 @@ +--- +title: Ensure a Kubernetes namespace is not used for deployments if there is no service + account token associated with it +merge_request: 29643 +author: +type: fixed diff --git a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb index 5387863bd07..b1cfb8e1c62 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb @@ -35,7 +35,7 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do end context 'and a namespace is already created for this project' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, project: build.project) } + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster, project: build.project) } it { is_expected.to be_falsey } end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index f206bb41f45..c302b7a15f4 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -555,6 +555,63 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end + describe '#find_or_initialize_kubernetes_namespace_for_project' do + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:project) { cluster.projects.first } + + subject { cluster.find_or_initialize_kubernetes_namespace_for_project(project) } + + context 'kubernetes namespace exists' do + context 'with no service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, project: project, cluster: cluster) } + + it { is_expected.to eq kubernetes_namespace } + end + + context 'with a service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, project: project, cluster: cluster) } + + it { is_expected.to eq kubernetes_namespace } + end + end + + context 'kubernetes namespace does not exist' do + it 'initializes a new namespace and sets default values' do + expect(subject).to be_new_record + expect(subject.project).to eq project + expect(subject.cluster).to eq cluster + expect(subject.namespace).to be_present + expect(subject.service_account_name).to be_present + end + end + + context 'a custom scope is provided' do + let(:scope) { cluster.kubernetes_namespaces.has_service_account_token } + + subject { cluster.find_or_initialize_kubernetes_namespace_for_project(project, scope: scope) } + + context 'kubernetes namespace exists' do + context 'with no service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, project: project, cluster: cluster) } + + it 'initializes a new namespace and sets default values' do + expect(subject).to be_new_record + expect(subject.project).to eq project + expect(subject.cluster).to eq cluster + expect(subject.namespace).to be_present + expect(subject.service_account_name).to be_present + end + end + + context 'with a service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, project: project, cluster: cluster) } + + it { is_expected.to eq kubernetes_namespace } + end + end + end + end + describe '#predefined_variables' do subject { cluster.predefined_variables } diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index c485850c16e..f4dd457c3d3 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -223,19 +223,33 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let(:namespace) { 'namespace-123' } it { is_expected.to eq(namespace) } + + context 'kubernetes namespace is present but has no service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } + + it { is_expected.to eq(namespace) } + end end context 'with no namespace assigned' do let(:namespace) { nil } context 'when kubernetes namespace is present' do - let(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } + let(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster) } before do kubernetes_namespace end it { is_expected.to eq(kubernetes_namespace.namespace) } + + context 'kubernetes namespace has no service account token' do + before do + kubernetes_namespace.update!(namespace: 'old-namespace', service_account_token: nil) + end + + it { is_expected.to eq("#{project.path}-#{project.id}") } + end end context 'when kubernetes namespace is not present' do |