Commit 5b5f40f1 authored by Thong Kuah's avatar Thong Kuah

Restrict management project to same namespace

When working out the deployment platform for a project, only return
clusters where its namespace is the same as the management project.

This ensures that cluster credentials won't leak out of the namespace.

Allow where managment project is in the hierarchy of the cluster's
namespace as well.

- For group clusters, this is achieved by simply changing the order
clause
- For project clusters, we need to have a new query in our CTE to fetch
clusters where management cluster is in the same namespace as the
project's namespace.

Add specs for new code, alter existing specs to work with new
restrictions.

And finally update documentation!
parent 036ee862
......@@ -117,6 +117,8 @@ module Clusters
scope :default_environment, -> { where(environment_scope: DEFAULT_ENVIRONMENT) }
scope :for_project_namespace, -> (namespace_id) { joins(:projects).where(projects: { namespace_id: namespace_id }) }
def self.ancestor_clusters_for_clusterable(clusterable, hierarchy_order: :asc)
return [] if clusterable.is_a?(Instance)
......
......@@ -20,7 +20,7 @@ module Clusters
.with
.recursive(cte.to_arel)
.from(cte_alias)
.order(DEPTH_COLUMN => :asc)
.order(depth_order_clause)
end
private
......@@ -40,7 +40,7 @@ module Clusters
end
if clusterable.is_a?(::Project) && include_management_project
cte << management_clusters_query
cte << same_namespace_management_clusters_query
end
cte << base_query
......@@ -49,13 +49,42 @@ module Clusters
cte
end
# Returns project-level clusters where the project is the management project
# for the cluster. The management project has to be in the same namespace /
# group as the cluster's project.
#
# Support for management project in sub-groups is planned in
# https://gitlab.com/gitlab-org/gitlab/issues/34650
#
# NB: group_parent_id is un-used but we still need to match the same number of
# columns as other queries in the CTE.
def same_namespace_management_clusters_query
clusterable.management_clusters
.project_type
.select([clusters_star, 'NULL AS group_parent_id', "0 AS #{DEPTH_COLUMN}"])
.for_project_namespace(clusterable.namespace_id)
end
# Management clusters should be first in the hierarchy so we use 0 for the
# depth column.
#
# group_parent_id is un-used but we still need to match the same number of
# columns as other queries in the CTE.
def management_clusters_query
clusterable.management_clusters.select([clusters_star, 'NULL AS group_parent_id', "0 AS #{DEPTH_COLUMN}"])
# Only applicable if the clusterable is a project (most especially when
# requesting project.deployment_platform).
def depth_order_clause
return { DEPTH_COLUMN => :asc } unless clusterable.is_a?(::Project) && include_management_project
order = <<~SQL
(CASE clusters.management_project_id
WHEN :project_id THEN 0
ELSE #{DEPTH_COLUMN}
END) ASC
SQL
values = {
project_id: clusterable.id
}
model.sanitize_sql_array([Arel.sql(order), values])
end
def group_clusters_base_query
......
......@@ -4,7 +4,7 @@ CAUTION: **Warning:**
This is an _alpha_ feature, and it is subject to change at any time without
prior notice.
> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/17866) in GitLab 12.4
> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/32810) in GitLab 12.5
A project can be designated as the management project for a cluster.
A management project can be used to run deployment jobs with
......@@ -22,6 +22,14 @@ This can be useful for:
Only the management project will receive `cluster-admin` privileges. All
other projects will continue to receive [namespace scoped `edit` level privileges](../project/clusters/index.md#rbac-cluster-resources).
Management projects are restricted to the following:
- For project-level clusters, the management project must in the same
namespace (or descendants) as the cluster's project.
- For group-level clusters, the management project must in the same
group (or descendants) as as the cluster's group.
- For instance-level clusters, there are no such restrictions.
## Usage
### Selecting a cluster management project
......
......@@ -152,6 +152,16 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
end
end
describe '.for_project_namespace' do
subject { described_class.for_project_namespace(namespace_id) }
let!(:cluster) { create(:cluster, :project) }
let!(:another_cluster) { create(:cluster, :project) }
let(:namespace_id) { cluster.first_project.namespace_id }
it { is_expected.to contain_exactly(cluster) }
end
describe 'validations' do
subject { cluster.valid? }
......
......@@ -42,6 +42,28 @@ describe Clusters::ClustersHierarchy do
it 'returns clusters for project' do
expect(base_and_ancestors(cluster.project)).to eq([cluster])
end
context 'cluster has management project' do
let(:management_project) { create(:project, namespace: cluster.first_project.namespace) }
before do
cluster.update!(management_project: management_project)
end
context 'management_project is in same namespace as cluster' do
it 'returns cluster for management_project' do
expect(base_and_ancestors(management_project)).to eq([cluster])
end
end
context 'management_project is in a different namespace from cluster' do
let(:management_project) { create(:project) }
it 'returns nothing' do
expect(base_and_ancestors(management_project)).to be_empty
end
end
end
end
context 'cluster has management project' do
......@@ -50,16 +72,12 @@ describe Clusters::ClustersHierarchy do
let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
let(:management_project) { create(:project) }
let(:management_project) { create(:project, group: group) }
it 'returns clusters for management_project' do
expect(base_and_ancestors(management_project)).to eq([group_cluster])
end
it 'returns nothing if include_management_project is false' do
expect(base_and_ancestors(management_project, include_management_project: false)).to be_empty
end
it 'returns clusters for project' do
expect(base_and_ancestors(project)).to eq([project_cluster, group_cluster])
end
......@@ -70,17 +88,21 @@ describe Clusters::ClustersHierarchy do
end
context 'project in nested group with clusters at some levels' do
let!(:child) { create(:cluster, :group, groups: [child_group], management_project: management_project) }
let!(:ancestor) { create(:cluster, :group, groups: [ancestor_group]) }
let!(:child) { create(:cluster, :group, groups: [child_group]) }
let!(:ancestor) { create(:cluster, :group, groups: [ancestor_group], management_project: management_project) }
let(:ancestor_group) { create(:group) }
let(:parent_group) { create(:group, parent: ancestor_group) }
let(:child_group) { create(:group, parent: parent_group) }
let(:project) { create(:project, group: child_group) }
let(:management_project) { create(:project) }
let(:management_project) { create(:project, group: child_group) }
it 'returns clusters for management_project' do
expect(base_and_ancestors(management_project)).to eq([ancestor, child])
end
it 'returns clusters for management_project' do
expect(base_and_ancestors(management_project)).to eq([child])
expect(base_and_ancestors(management_project, include_management_project: false)).to eq([child, ancestor])
end
it 'returns clusters for project' do
......
......@@ -13,7 +13,11 @@ describe DeploymentPlatform do
end
context 'when project is the cluster\'s management project ' do
let!(:cluster_with_management_project) { create(:cluster, :provided_by_user, management_project: project) }
let(:another_project) { create(:project, namespace: project.namespace) }
let!(:cluster_with_management_project) do
create(:cluster, :provided_by_user, projects: [another_project], management_project: project)
end
context 'cluster_management_project feature is enabled' do
it 'returns the cluster with management project' do
......@@ -66,7 +70,11 @@ describe DeploymentPlatform do
end
context 'when project is the cluster\'s management project ' do
let!(:cluster_with_management_project) { create(:cluster, :provided_by_user, management_project: project) }
let(:another_project) { create(:project, namespace: project.namespace) }
let!(:cluster_with_management_project) do
create(:cluster, :provided_by_user, projects: [another_project], management_project: project)
end
context 'cluster_management_project feature is enabled' do
it 'returns the cluster with management project' do
......
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