Commit 15718065 authored by Hordur Freyr Yngvason's avatar Hordur Freyr Yngvason Committed by Rémy Coutable

Find namespace by env name instead of slug

The problem with using slugs is that we already have persisted
environments matching generic branch names but different slugs.
Environments are uniquely identified by by `(project_id, name)`, so this
should be just as safe.

See https://gitlab.com/gitlab-org/gitlab/issues/32341
parent 94f3df64
...@@ -2,12 +2,12 @@ ...@@ -2,12 +2,12 @@
module Clusters module Clusters
class KubernetesNamespaceFinder class KubernetesNamespaceFinder
attr_reader :cluster, :project, :environment_slug attr_reader :cluster, :project, :environment_name
def initialize(cluster, project:, environment_slug:, allow_blank_token: false) def initialize(cluster, project:, environment_name:, allow_blank_token: false)
@cluster = cluster @cluster = cluster
@project = project @project = project
@environment_slug = environment_slug @environment_name = environment_name
@allow_blank_token = allow_blank_token @allow_blank_token = allow_blank_token
end end
...@@ -20,7 +20,11 @@ module Clusters ...@@ -20,7 +20,11 @@ module Clusters
attr_reader :allow_blank_token attr_reader :allow_blank_token
def find_namespace(with_environment:) def find_namespace(with_environment:)
relation = with_environment ? namespaces.with_environment_slug(environment_slug) : namespaces relation = if with_environment
namespaces.with_environment_name(environment_name)
else
namespaces
end
relation.find_by_project_id(project.id) relation.find_by_project_id(project.id)
end end
......
...@@ -172,7 +172,7 @@ module Clusters ...@@ -172,7 +172,7 @@ module Clusters
persisted_namespace = Clusters::KubernetesNamespaceFinder.new( persisted_namespace = Clusters::KubernetesNamespaceFinder.new(
self, self,
project: project, project: project,
environment_slug: environment.slug environment_name: environment.name
).execute ).execute
persisted_namespace&.namespace || Gitlab::Kubernetes::DefaultNamespace.new(self, project: project).from_environment_slug(environment.slug) persisted_namespace&.namespace || Gitlab::Kubernetes::DefaultNamespace.new(self, project: project).from_environment_slug(environment.slug)
......
...@@ -27,7 +27,7 @@ module Clusters ...@@ -27,7 +27,7 @@ module Clusters
algorithm: 'aes-256-cbc' algorithm: 'aes-256-cbc'
scope :has_service_account_token, -> { where.not(encrypted_service_account_token: nil) } scope :has_service_account_token, -> { where.not(encrypted_service_account_token: nil) }
scope :with_environment_slug, -> (slug) { joins(:environment).where(environments: { slug: slug }) } scope :with_environment_name, -> (name) { joins(:environment).where(environments: { name: name }) }
def token_name def token_name
"#{namespace}-token" "#{namespace}-token"
......
...@@ -105,19 +105,11 @@ module Clusters ...@@ -105,19 +105,11 @@ module Clusters
private private
##
# Environment slug can be predicted given an environment
# name, so even if the environment isn't persisted yet we
# still know what to look for.
def environment_slug(name)
Gitlab::Slug::Environment.new(name).generate
end
def find_persisted_namespace(project, environment_name:) def find_persisted_namespace(project, environment_name:)
Clusters::KubernetesNamespaceFinder.new( Clusters::KubernetesNamespaceFinder.new(
cluster, cluster,
project: project, project: project,
environment_slug: environment_slug(environment_name) environment_name: environment_name
).execute ).execute
end end
......
...@@ -36,7 +36,7 @@ module Gitlab ...@@ -36,7 +36,7 @@ module Gitlab
Clusters::KubernetesNamespaceFinder.new( Clusters::KubernetesNamespaceFinder.new(
deployment_cluster, deployment_cluster,
project: environment.project, project: environment.project,
environment_slug: environment.slug, environment_name: environment.name,
allow_blank_token: true allow_blank_token: true
).execute ).execute
end end
......
...@@ -7,7 +7,7 @@ RSpec.describe Clusters::KubernetesNamespaceFinder do ...@@ -7,7 +7,7 @@ RSpec.describe Clusters::KubernetesNamespaceFinder do
described_class.new( described_class.new(
cluster, cluster,
project: project, project: project,
environment_slug: 'production', environment_name: 'production',
allow_blank_token: allow_blank_token allow_blank_token: allow_blank_token
) )
end end
...@@ -22,8 +22,8 @@ RSpec.describe Clusters::KubernetesNamespaceFinder do ...@@ -22,8 +22,8 @@ RSpec.describe Clusters::KubernetesNamespaceFinder do
end end
describe '#execute' do describe '#execute' do
let(:production) { create(:environment, project: project, slug: 'production') } let(:production) { create(:environment, project: project, name: 'production') }
let(:staging) { create(:environment, project: project, slug: 'staging') } let(:staging) { create(:environment, project: project, name: 'staging') }
let(:cluster) { create(:cluster, :group, :provided_by_user) } let(:cluster) { create(:cluster, :group, :provided_by_user) }
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -546,7 +546,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do ...@@ -546,7 +546,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
before do before do
expect(Clusters::KubernetesNamespaceFinder).to receive(:new) expect(Clusters::KubernetesNamespaceFinder).to receive(:new)
.with(cluster, project: environment.project, environment_slug: environment.slug) .with(cluster, project: environment.project, environment_name: environment.name)
.and_return(double(execute: persisted_namespace)) .and_return(double(execute: persisted_namespace))
end end
......
...@@ -24,13 +24,13 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do ...@@ -24,13 +24,13 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do
end end
end end
describe '.with_environment_slug' do describe '.with_environment_name' do
let(:cluster) { create(:cluster, :group) } let(:cluster) { create(:cluster, :group) }
let(:environment) { create(:environment, slug: slug) } let(:environment) { create(:environment, name: name) }
let(:slug) { 'production' } let(:name) { 'production' }
subject { described_class.with_environment_slug(slug) } subject { described_class.with_environment_name(name) }
context 'there is no associated environment' do context 'there is no associated environment' do
let!(:namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, project: environment.project) } let!(:namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, project: environment.project) }
...@@ -48,12 +48,12 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do ...@@ -48,12 +48,12 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do
) )
end end
context 'with a matching slug' do context 'with a matching name' do
it { is_expected.to eq [namespace] } it { is_expected.to eq [namespace] }
end end
context 'without a matching slug' do context 'without a matching name' do
let(:environment) { create(:environment, slug: 'staging') } let(:environment) { create(:environment, name: 'staging') }
it { is_expected.to be_empty } it { is_expected.to be_empty }
end end
......
...@@ -218,7 +218,7 @@ describe Clusters::Platforms::Kubernetes do ...@@ -218,7 +218,7 @@ describe Clusters::Platforms::Kubernetes do
before do before do
allow(Clusters::KubernetesNamespaceFinder).to receive(:new) allow(Clusters::KubernetesNamespaceFinder).to receive(:new)
.with(cluster, project: project, environment_slug: environment_slug) .with(cluster, project: project, environment_name: environment_name)
.and_return(double(execute: persisted_namespace)) .and_return(double(execute: persisted_namespace))
end end
......
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