From 89a1594a244face6071ad5173ff86df13e446794 Mon Sep 17 00:00:00 2001 From: Tiger Date: Fri, 31 May 2019 12:44:59 -0500 Subject: Pass environment when finding deployment variables This will enable looking up Kubernetes namespaces based on the environment being deployed to, instead of using a single namespace across all environments. --- app/models/clusters/platforms/kubernetes.rb | 4 +++- app/models/concerns/ci/contextable.rb | 6 +++++- app/models/project.rb | 4 ++-- app/models/project_services/kubernetes_service.rb | 6 +++--- app/models/project_services/mock_deployment_service.rb | 2 +- spec/models/clusters/platforms/kubernetes_spec.rb | 7 +++++-- spec/models/project_services/kubernetes_service_spec.rb | 4 ++-- spec/models/project_spec.rb | 14 ++++++++++++-- 8 files changed, 33 insertions(+), 14 deletions(-) diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 2afe471d1cc..0f592a7b131 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -70,7 +70,9 @@ module Clusters default_value_for :authorization_type, :rbac - def predefined_variables(project:) + def predefined_variables(environment:) + project = environment.project + Gitlab::Ci::Variables::Collection.new.tap do |variables| variables.append(key: 'KUBE_URL', value: api_url) diff --git a/app/models/concerns/ci/contextable.rb b/app/models/concerns/ci/contextable.rb index e1d5ce7f7d4..a7e68ce7403 100644 --- a/app/models/concerns/ci/contextable.rb +++ b/app/models/concerns/ci/contextable.rb @@ -15,7 +15,7 @@ module Ci variables.concat(project.predefined_variables) variables.concat(pipeline.predefined_variables) variables.concat(runner.predefined_variables) if runnable? && runner - variables.concat(project.deployment_variables(environment: environment)) if environment + variables.concat(deployment_variables(environment_name: environment)) if environment variables.concat(yaml_variables) variables.concat(user_variables) variables.concat(secret_group_variables) @@ -104,5 +104,9 @@ module Ci def secret_project_variables(environment: persisted_environment) project.ci_variables_for(ref: git_ref, environment: environment) end + + def deployment_variables(environment_name:) + project.deployment_variables(environment_name: environment_name, persisted_environment: persisted_environment) + end end end diff --git a/app/models/project.rb b/app/models/project.rb index 7851f37116c..ff66de80089 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1860,8 +1860,8 @@ class Project < ApplicationRecord end end - def deployment_variables(environment: nil) - deployment_platform(environment: environment)&.predefined_variables(project: self) || [] + def deployment_variables(environment_name: nil, persisted_environment: nil) + deployment_platform(environment: environment_name)&.predefined_variables(environment: persisted_environment) || [] end def auto_devops_variables diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index edf7e886e77..fa29b3265bf 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -94,7 +94,7 @@ class KubernetesService < Service ] end - def kubernetes_namespace_for(project) + def kubernetes_namespace_for(_) if namespace.present? namespace else @@ -117,12 +117,12 @@ class KubernetesService < Service # as a way to keep this service compatible with # Clusters::Platforms::Kubernetes, it won't be used on this method # as it's only needed for Clusters::Cluster. - def predefined_variables(project:) + def predefined_variables(environment:) Gitlab::Ci::Variables::Collection.new.tap do |variables| variables .append(key: 'KUBE_URL', value: api_url) .append(key: 'KUBE_TOKEN', value: token, public: false, masked: true) - .append(key: 'KUBE_NAMESPACE', value: kubernetes_namespace_for(project)) + .append(key: 'KUBE_NAMESPACE', value: kubernetes_namespace_for(nil)) .append(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true) if ca_pem.present? diff --git a/app/models/project_services/mock_deployment_service.rb b/app/models/project_services/mock_deployment_service.rb index 1103cb11e73..fea5e35e69a 100644 --- a/app/models/project_services/mock_deployment_service.rb +++ b/app/models/project_services/mock_deployment_service.rb @@ -24,7 +24,7 @@ class MockDeploymentService < Service %w() end - def predefined_variables(project:) + def predefined_variables(environment:) [] end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 0fa5d031736..753f367ee95 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -263,8 +263,9 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let(:kubernetes) { create(:cluster_platform_kubernetes, api_url: api_url, ca_cert: ca_pem) } let(:api_url) { 'https://kube.domain.com' } let(:ca_pem) { File.read(Rails.root.join('spec/fixtures/clusters/sample_cert.pem')) } + let(:environment) { create(:environment, project: cluster.project) } - subject { kubernetes.predefined_variables(project: cluster.project) } + subject { kubernetes.predefined_variables(environment: environment) } shared_examples 'setting variables' do it 'sets the variables' do @@ -344,8 +345,9 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let!(:cluster) { create(:cluster, :group, platform_kubernetes: kubernetes) } let(:project) { create(:project, group: cluster.group) } + let(:environment) { create(:environment, project: project) } - subject { kubernetes.predefined_variables(project: project) } + subject { kubernetes.predefined_variables(environment: environment) } context 'no kubernetes namespace for the project' do it_behaves_like 'setting variables' @@ -383,6 +385,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end context 'with a domain' do + let(:environment) { create(:environment) } let!(:cluster) do create(:cluster, :provided_by_gcp, :with_domain, platform_kubernetes: kubernetes) diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 34ee1eafd5c..ed29d84fc02 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -257,7 +257,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do shared_examples 'setting variables' do it 'sets the variables' do - expect(subject.predefined_variables(project: project)).to include( + expect(subject.predefined_variables(environment: double)).to include( { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true }, { key: 'KUBE_TOKEN', value: 'token', public: false, masked: true }, { key: 'KUBE_NAMESPACE', value: namespace, public: true }, @@ -284,7 +284,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do it_behaves_like 'setting variables' it 'sets the KUBE_NAMESPACE' do - kube_namespace = subject.predefined_variables(project: project).find { |h| h[:key] == 'KUBE_NAMESPACE' } + kube_namespace = subject.predefined_variables(environment: double).find { |h| h[:key] == 'KUBE_NAMESPACE' } expect(kube_namespace).not_to be_nil expect(kube_namespace[:value]).to match(/\A#{Gitlab::PathRegex::PATH_REGEX_STR}-\d+\z/) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cc0f5002a1e..4537d44d1fd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2655,9 +2655,14 @@ describe Project do context 'when user configured kubernetes from CI/CD > Clusters and KubernetesNamespace migration has not been executed' do let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:project) { cluster.project } + let!(:environment) { create(:environment, project: project) } + + let(:deployment_variables) do + project.deployment_variables(environment_name: environment.name, persisted_environment: environment) + end it 'does not return variables from this service' do - expect(project.deployment_variables).not_to include( + expect(deployment_variables).not_to include( { key: 'KUBE_TOKEN', value: project.deployment_platform.token, public: false, masked: true } ) end @@ -2667,9 +2672,14 @@ describe Project do let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token) } let!(:cluster) { kubernetes_namespace.cluster } let(:project) { kubernetes_namespace.project } + let!(:environment) { create(:environment, project: project) } + + let(:deployment_variables) do + project.deployment_variables(environment_name: environment.name, persisted_environment: environment) + end it 'returns token from kubernetes namespace' do - expect(project.deployment_variables).to include( + expect(deployment_variables).to include( { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } ) end -- cgit v1.2.1