Merge branch 'zj-kube-service-auto-fill' into 'master'

Don't autofill kubernetes namespace

See merge request !10438

4 files changed, 90 insertions, 48 deletions

diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb
index 02fbd5497fa..9c56518c991 100644
--- a/app/models/project_services/kubernetes_service.rb
+++ b/app/models/project_services/kubernetes_service.rb
@@ -22,22 +22,21 @@ class KubernetesService < DeploymentService
   with_options presence: true, if: :activated? do
     validates :api_url, url: true
     validates :token
-
-    validates :namespace,
-      format: {
-        with: Gitlab::Regex.kubernetes_namespace_regex,
-        message: Gitlab::Regex.kubernetes_namespace_regex_message,
-      },
-      length: 1..63
   end
 
+  validates :namespace,
+    allow_blank: true,
+    length: 1..63,
+    if: :activated?,
+    format: {
+      with: Gitlab::Regex.kubernetes_namespace_regex,
+      message: Gitlab::Regex.kubernetes_namespace_regex_message
+    }
+
   after_save :clear_reactive_cache!
 
   def initialize_properties
-    if properties.nil?
-      self.properties = {}
-      self.namespace = "#{project.path}-#{project.id}" if project.present?
-    end
+    self.properties = {} if properties.nil?
   end
 
   def title
@@ -62,7 +61,7 @@ class KubernetesService < DeploymentService
         { type: 'text',
           name: 'namespace',
           title: 'Kubernetes namespace',
-          placeholder: 'Kubernetes namespace' },
+          placeholder: namespace_placeholder },
         { type: 'text',
           name: 'api_url',
           title: 'API URL',
@@ -92,7 +91,7 @@ class KubernetesService < DeploymentService
     variables = [
       { key: 'KUBE_URL', value: api_url, public: true },
       { key: 'KUBE_TOKEN', value: token, public: false },
-      { key: 'KUBE_NAMESPACE', value: namespace, public: true }
+      { key: 'KUBE_NAMESPACE', value: namespace_variable, public: true }
     ]
 
     if ca_pem.present?
@@ -135,8 +134,26 @@ class KubernetesService < DeploymentService
     { pods: pods }
   end
 
+  TEMPLATE_PLACEHOLDER = 'Kubernetes namespace'.freeze
+
   private
 
+  def namespace_placeholder
+    default_namespace || TEMPLATE_PLACEHOLDER
+  end
+
+  def namespace_variable
+    if namespace.present?
+      namespace
+    else
+      default_namespace
+    end
+  end
+
+  def default_namespace
+    "#{project.path}-#{project.id}" if project.present?
+  end
+
   def build_kubeclient!(api_path: 'api', api_version: 'v1')
     raise "Incomplete settings" unless api_url && namespace && token
 
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index b78de092a60..160345cfaa5 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -238,6 +238,8 @@
           %ul
             %li Be careful. Renaming a project's repository can have unintended side effects.
             %li You will need to update your local repositories to point to the new location.
+            - if @project.deployment_services.any?
+              %li Your deployment services will be broken, you will need to manually fix the services after renaming.
         = f.submit 'Rename project', class: "btn btn-warning"
   - if can?(current_user, :change_namespace, @project)
     %hr
diff --git a/changelogs/unreleased/zj-kube-service-auto-fill.yml b/changelogs/unreleased/zj-kube-service-auto-fill.yml
new file mode 100644
index 00000000000..7a2c7a5085b
--- /dev/null
+++ b/changelogs/unreleased/zj-kube-service-auto-fill.yml
@@ -0,0 +1,4 @@
+---
+title: Don't fill in the default kubernetes namespace
+merge_request:
+author:
diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb
index bf7950ef1c9..e69eb0098dd 100644
--- a/spec/models/project_services/kubernetes_service_spec.rb
+++ b/spec/models/project_services/kubernetes_service_spec.rb
@@ -4,7 +4,7 @@ describe KubernetesService, models: true, caching: true do
   include KubernetesHelpers
   include ReactiveCachingHelpers
 
-  let(:project) { create(:kubernetes_project) }
+  let(:project) { build_stubbed(:kubernetes_project) }
   let(:service) { project.kubernetes_service }
 
   # We use Kubeclient to interactive with the Kubernetes API. It will
@@ -32,7 +32,8 @@ describe KubernetesService, models: true, caching: true do
   describe 'Validations' do
     context 'when service is active' do
       before { subject.active = true }
-      it { is_expected.to validate_presence_of(:namespace) }
+
+      it { is_expected.not_to validate_presence_of(:namespace) }
       it { is_expected.to validate_presence_of(:api_url) }
       it { is_expected.to validate_presence_of(:token) }
 
@@ -55,7 +56,7 @@ describe KubernetesService, models: true, caching: true do
           'a.b' => false,
           'a*b' => false,
         }.each do |namespace, validity|
-          it "should validate #{namespace} as #{validity ? 'valid' : 'invalid'}" do
+          it "validates #{namespace} as #{validity ? 'valid' : 'invalid'}" do
             subject.namespace = namespace
 
             expect(subject.valid?).to eq(validity)
@@ -66,24 +67,40 @@ describe KubernetesService, models: true, caching: true do
 
     context 'when service is inactive' do
       before { subject.active = false }
-      it { is_expected.not_to validate_presence_of(:namespace) }
+
       it { is_expected.not_to validate_presence_of(:api_url) }
       it { is_expected.not_to validate_presence_of(:token) }
     end
   end
 
   describe '#initialize_properties' do
-    context 'with a project' do
-      let(:namespace_name) { "#{project.path}-#{project.id}" }
+    context 'without a project' do
+      it 'leaves the namespace unset' do
+        expect(described_class.new.namespace).to be_nil
+      end
+    end
+  end
+
+  describe '#fields' do
+    let(:kube_namespace) do
+      subject.fields.find { |h| h[:name] == 'namespace' }
+    end
+
+    context 'as template' do
+      before { subject.template = true }
 
-      it 'defaults to the project name with ID' do
-        expect(described_class.new(project: project).namespace).to eq(namespace_name)
+      it 'sets the namespace to the default' do
+        expect(kube_namespace).not_to be_nil
+        expect(kube_namespace[:placeholder]).to eq(subject.class::TEMPLATE_PLACEHOLDER)
       end
     end
 
-    context 'without a project' do
-      it 'leaves the namespace unset' do
-        expect(described_class.new.namespace).to be_nil
+    context 'with associated project' do
+      before { subject.project = project }
+
+      it 'sets the namespace to the default' do
+        expect(kube_namespace).not_to be_nil
+        expect(kube_namespace[:placeholder]).to match(/\A#{Gitlab::Regex::PATH_REGEX_STR}-\d+\z/)
       end
     end
   end
@@ -138,38 +155,40 @@ describe KubernetesService, models: true, caching: true do
     before do
       subject.api_url = 'https://kube.domain.com'
       subject.token = 'token'
-      subject.namespace = 'my-project'
       subject.ca_pem = 'CA PEM DATA'
+      subject.project = project
     end
 
-    it 'sets KUBE_URL' do
-      expect(subject.predefined_variables).to include(
-        { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true }
-      )
-    end
+    context 'namespace is provided' do
+      before { subject.namespace = 'my-project' }
 
-    it 'sets KUBE_TOKEN' do
-      expect(subject.predefined_variables).to include(
-        { key: 'KUBE_TOKEN', value: 'token', public: false }
-      )
+      it 'sets the variables' do
+        expect(subject.predefined_variables).to include(
+          { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true },
+          { key: 'KUBE_TOKEN', value: 'token', public: false },
+          { key: 'KUBE_NAMESPACE', value: 'my-project', public: true },
+          { key: 'KUBE_CA_PEM', value: 'CA PEM DATA', public: true },
+          { key: 'KUBE_CA_PEM_FILE', value: 'CA PEM DATA', public: true, file: true },
+        )
+      end
     end
 
-    it 'sets KUBE_NAMESPACE' do
-      expect(subject.predefined_variables).to include(
-        { key: 'KUBE_NAMESPACE', value: 'my-project', public: true }
-      )
-    end
+    context 'no namespace provided' do
+      it 'sets the variables' do
+        expect(subject.predefined_variables).to include(
+          { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true },
+          { key: 'KUBE_TOKEN', value: 'token', public: false },
+          { key: 'KUBE_CA_PEM', value: 'CA PEM DATA', public: true },
+          { key: 'KUBE_CA_PEM_FILE', value: 'CA PEM DATA', public: true, file: true },
+        )
+      end
 
-    it 'sets KUBE_CA_PEM' do
-      expect(subject.predefined_variables).to include(
-        { key: 'KUBE_CA_PEM', value: 'CA PEM DATA', public: true }
-      )
-    end
+      it 'sets the KUBE_NAMESPACE' do
+        kube_namespace = subject.predefined_variables.find { |h| h[:key] == 'KUBE_NAMESPACE' }
 
-    it 'sets KUBE_CA_PEM_FILE' do
-      expect(subject.predefined_variables).to include(
-        { key: 'KUBE_CA_PEM_FILE', value: 'CA PEM DATA', public: true, file: true }
-      )
+        expect(kube_namespace).not_to be_nil
+        expect(kube_namespace[:value]).to match(/\A#{Gitlab::Regex::PATH_REGEX_STR}-\d+\z/)
+      end
     end
   end