Improve variables support

This ensures that variables accept only string,
alongside also improves kubernetes_namespace,
improving validation and default value being set.

app/models/ci/build.rb

@@ -464,7 +464,9 @@ module Ci
     end
 
     def repo_url
-      auth = "gitlab-ci-token:#{ensure_token!}@"
+      return unless token
+
+      auth = "gitlab-ci-token:#{token}@"
       project.http_url_to_repo.sub(%r{^https?://}) do |prefix|
         prefix + auth
       end
@@ -725,7 +727,7 @@ module Ci
 
       trace = trace.dup
       Gitlab::Ci::MaskSecret.mask!(trace, project.runners_token) if project
-      Gitlab::Ci::MaskSecret.mask!(trace, token)
+      Gitlab::Ci::MaskSecret.mask!(trace, token) if token
       trace
     end
 
@@ -814,12 +816,12 @@ module Ci
           .concat(pipeline.persisted_variables)
           .append(key: 'CI_JOB_ID', value: id.to_s)
           .append(key: 'CI_JOB_URL', value: Gitlab::Routing.url_helpers.project_job_url(project, self))
-          .append(key: 'CI_JOB_TOKEN', value: token, public: false)
+          .append(key: 'CI_JOB_TOKEN', value: token.to_s, public: false)
           .append(key: 'CI_BUILD_ID', value: id.to_s)
-          .append(key: 'CI_BUILD_TOKEN', value: token, public: false)
+          .append(key: 'CI_BUILD_TOKEN', value: token.to_s, public: false)
           .append(key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER)
-          .append(key: 'CI_REGISTRY_PASSWORD', value: token, public: false)
-          .append(key: 'CI_REPOSITORY_URL', value: repo_url, public: false)
+          .append(key: 'CI_REGISTRY_PASSWORD', value: token.to_s, public: false)
+          .append(key: 'CI_REPOSITORY_URL', value: repo_url.to_s, public: false)
           .concat(deploy_token_variables)
       end
     end
@@ -831,9 +833,9 @@ module Ci
         variables.append(key: 'GITLAB_FEATURES', value: project.licensed_features.join(','))
         variables.append(key: 'CI_SERVER_NAME', value: 'GitLab')
         variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION)
-        variables.append(key: 'CI_SERVER_VERSION_MAJOR', value: gitlab_version_info.major.to_s)
-        variables.append(key: 'CI_SERVER_VERSION_MINOR', value: gitlab_version_info.minor.to_s)
-        variables.append(key: 'CI_SERVER_VERSION_PATCH', value: gitlab_version_info.patch.to_s)
+        variables.append(key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s)
+        variables.append(key: 'CI_SERVER_VERSION_MINOR', value: Gitlab.version_info.minor.to_s)
+        variables.append(key: 'CI_SERVER_VERSION_PATCH', value: Gitlab.version_info.patch.to_s)
         variables.append(key: 'CI_SERVER_REVISION', value: Gitlab.revision)
         variables.append(key: 'CI_JOB_NAME', value: name)
         variables.append(key: 'CI_JOB_STAGE', value: stage)
@@ -850,10 +852,6 @@ module Ci
       end
     end
 
-    def gitlab_version_info
-      @gitlab_version_info ||= Gitlab::VersionInfo.parse(Gitlab::VERSION)
-    end
-
     def legacy_variables
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
         variables.append(key: 'CI_BUILD_REF', value: sha)

app/models/clusters/kubernetes_namespace.rb

@@ -11,9 +11,13 @@ module Clusters
     belongs_to :project, class_name: '::Project'
     has_one :platform_kubernetes, through: :cluster
 
+    before_validation :set_defaults
+
     validates :namespace, presence: true
     validates :namespace, uniqueness: { scope: :cluster_id }
 
+    validates :service_account_name, presence: true
+
     delegate :ca_pem, to: :platform_kubernetes, allow_nil: true
     delegate :api_url, to: :platform_kubernetes, allow_nil: true
 
@@ -28,38 +32,43 @@ module Clusters
       "#{namespace}-token"
     end
 
-    def configure_predefined_credentials
-      self.namespace = kubernetes_or_project_namespace
-      self.service_account_name = default_service_account_name
-    end
-
     def predefined_variables
       config = YAML.dump(kubeconfig)
 
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
         variables
-          .append(key: 'KUBE_SERVICE_ACCOUNT', value: service_account_name)
-          .append(key: 'KUBE_NAMESPACE', value: namespace)
-          .append(key: 'KUBE_TOKEN', value: service_account_token, public: false)
+          .append(key: 'KUBE_SERVICE_ACCOUNT', value: service_account_name.to_s)
+          .append(key: 'KUBE_NAMESPACE', value: namespace.to_s)
+          .append(key: 'KUBE_TOKEN', value: service_account_token.to_s, public: false)
           .append(key: 'KUBECONFIG', value: config, public: false, file: true)
       end
     end
 
-    private
-
-    def kubernetes_or_project_namespace
-      platform_kubernetes&.namespace.presence || project_namespace
+    def set_defaults
+      self.namespace ||= default_platform_kubernetes_namespace
+      self.namespace ||= default_project_namespace
+      self.service_account_name ||= default_service_account_name
     end
 
+    private
+
     def default_service_account_name
+      return unless namespace
+
       "#{namespace}-service-account"
     end
 
-    def project_namespace
-      Gitlab::NamespaceSanitizer.sanitize(project_slug)
+    def default_platform_kubernetes_namespace
+      platform_kubernetes&.namespace.presence
+    end
+
+    def default_project_namespace
+      Gitlab::NamespaceSanitizer.sanitize(project_slug) if project_slug
     end
 
     def project_slug
+      return unless project
+
       "#{project.path}-#{project.id}".downcase
     end
 

app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb

@@ -23,7 +23,7 @@ module Clusters
         attr_reader :cluster, :kubernetes_namespace, :platform
 
         def configure_kubernetes_namespace
-          kubernetes_namespace.configure_predefined_credentials
+          kubernetes_namespace.set_defaults
         end
 
         def create_project_service_account

lib/gitlab.rb

@@ -53,4 +53,8 @@ module Gitlab
   def self.pre_release?
     VERSION.include?('pre')
   end
+
+  def self.version_info
+    Gitlab::VersionInfo.parse(Gitlab::VERSION)
+  end
 end

lib/gitlab/ci/variables/collection/item.rb

@@ -6,8 +6,8 @@ module Gitlab
       class Collection
         class Item
           def initialize(key:, value:, public: true, file: false)
-            raise ArgumentError, "`value` must be of type String, while it was: #{value.class}" unless
-              value.is_a?(String) || value.nil?
+            raise ArgumentError, "`#{key}` must be of type String, while it was: #{value.class}" unless
+              value.is_a?(String)
 
             @variable = {
               key: key, value: value, public: public, file: file

spec/factories/clusters/kubernetes_namespaces.rb

@@ -3,7 +3,6 @@
 FactoryBot.define do
   factory :cluster_kubernetes_namespace, class: Clusters::KubernetesNamespace do
     association :cluster, :project, :provided_by_gcp
-    namespace { |n| "environment#{n}" }
 
     after(:build) do |kubernetes_namespace|
       cluster_project = kubernetes_namespace.cluster.cluster_project

spec/lib/gitlab/ci/variables/collection/item_spec.rb

@@ -36,7 +36,7 @@ describe Gitlab::Ci::Variables::Collection::Item do
     shared_examples 'raises error for invalid type' do
       it do
         expect { described_class.new(key: variable_key, value: variable_value) }
-          .to raise_error ArgumentError, /`value` must be of type String, while it was:/
+          .to raise_error ArgumentError, /`#{variable_key}` must be of type String, while it was:/
       end
     end
 
@@ -46,7 +46,7 @@ describe Gitlab::Ci::Variables::Collection::Item do
       let(:variable_value) { nil }
       let(:expected_value) { nil }
 
-      it_behaves_like 'creates variable'
+      it_behaves_like 'raises error for invalid type'
     end
 
     context "when it's an empty string" do

spec/models/clusters/kubernetes_namespace_spec.rb

@@ -45,14 +45,14 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do
     end
   end
 
-  describe '#configure_predefined_variables' do
+  describe '#set_defaults' do
     let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace) }
     let(:cluster) { kubernetes_namespace.cluster }
     let(:platform) { kubernetes_namespace.platform_kubernetes }
 
-    subject { kubernetes_namespace.configure_predefined_credentials }
+    subject { kubernetes_namespace.set_defaults }
 
-    describe 'namespace' do
+    describe '#namespace' do
       before do
         platform.update_column(:namespace, namespace)
       end
@@ -80,7 +80,7 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do
       end
     end
 
-    describe 'service_account_name' do
+    describe '#service_account_name' do
       let(:service_account_name) { "#{kubernetes_namespace.namespace}-service-account" }
 
       it 'should set a service account name based on namespace' do

spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb

@@ -44,7 +44,7 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' d
     let(:namespace) { "#{project.path}-#{project.id}" }
 
     let(:kubernetes_namespace) do
-      build(:cluster_kubernetes_namespace,
+      create(:cluster_kubernetes_namespace,
             cluster: cluster,
             project: cluster_project.project,
             cluster_project: cluster_project)