diff options
-rw-r--r-- | app/models/ci/build.rb | 2 | ||||
-rw-r--r-- | app/services/create_deployment_service.rb | 16 | ||||
-rw-r--r-- | lib/expand_variables.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/environment.rb | 16 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/job.rb | 10 | ||||
-rw-r--r-- | spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 14 | ||||
-rw-r--r-- | spec/services/create_deployment_service_spec.rb | 21 |
7 files changed, 49 insertions, 32 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 47dedef38d0..d5724af4cce 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -86,7 +86,7 @@ module Ci ref: build.ref, tag: build.tag, options: build.options[:environment], - variables: variables) + variables: build.variables) service.execute(build) end end diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index 577ba731583..e6667132e27 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -2,13 +2,7 @@ require_relative 'base_service' class CreateDeploymentService < BaseService def execute(deployable = nil) - environment = project.environments.find_or_create_by( - name: expanded_name - ) - - if expanded_url - environment.external_url = expanded_url - end + environment = find_or_create_environment project.deployments.create( environment: environment, @@ -22,6 +16,12 @@ class CreateDeploymentService < BaseService private + def find_or_create_environment + project.environments.find_or_create_by(name: expanded_name) do |environment| + environment.external_url = expanded_url + end + end + def expanded_name ExpandVariables.expand(name, variables) end @@ -41,7 +41,7 @@ class CreateDeploymentService < BaseService end def options - params[:environment] || {} + params[:options] || {} end def variables diff --git a/lib/expand_variables.rb b/lib/expand_variables.rb index 669735cc56c..7b1533d0d32 100644 --- a/lib/expand_variables.rb +++ b/lib/expand_variables.rb @@ -1,6 +1,6 @@ module ExpandVariables class << self - def expand_variables(value, variables) + def expand(value, variables) # Convert hash array to variables if variables.is_a?(Array) variables = variables.reduce({}) do |hash, variable| diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index 629c17e6250..85302589ce6 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -8,7 +8,11 @@ module Gitlab class Environment < Entry include Validatable + ALLOWED_KEYS = %i[name url] + validations do + validates :config, allowed_keys: ALLOWED_KEYS, if: :hash? + validates :name, presence: true validates :url, @@ -32,9 +36,9 @@ module Gitlab end def name - case - when string? then @config - when hash? then @config[:name] + case @config.type + when String then @config + when Hash then @config[:name] end end @@ -43,9 +47,9 @@ module Gitlab end def value - case - when string? then { name: @config } - when hash? then @config + case @config.type + when String then { name: @config } + when Hash then @config end end end diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb index e90e80171a4..3ab34d23d37 100644 --- a/lib/gitlab/ci/config/node/job.rb +++ b/lib/gitlab/ci/config/node/job.rb @@ -29,14 +29,6 @@ module Gitlab inclusion: { in: %w[on_success on_failure always manual], message: 'should be on_success, on_failure, ' \ 'always or manual' } - validates :environment, - type: { - with: String, - message: Gitlab::Regex.environment_name_regex_message } - validates :environment, - format: { - with: Gitlab::Regex.environment_name_regex, - message: Gitlab::Regex.environment_name_regex_message } validates :dependencies, array_of_strings: true end @@ -83,7 +75,7 @@ module Gitlab helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, - :artifacts, :commands + :artifacts, :commands, :environment def compose!(deps = nil) super do diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index af192664b33..2ad33007b8a 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -754,6 +754,20 @@ module Ci it 'does return production' do expect(builds.size).to eq(1) expect(builds.first[:environment]).to eq(environment) + expect(builds.first[:options]).to include(environment: { name: environment }) + end + end + + context 'when hash is specified' do + let(:environment) do + { name: 'production', + url: 'http://production.gitlab.com' } + end + + it 'does return production and URL' do + expect(builds.size).to eq(1) + expect(builds.first[:environment]).to eq(environment[:name]) + expect(builds.first[:options]).to include(environment) end end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index c8c741c0e88..7ebfddc45d5 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -41,7 +41,7 @@ describe CreateDeploymentService, services: true do context 'for environment with invalid name' do let(:params) do - { environment: 'name with spaces', + { environment: '..', ref: 'master', tag: false, sha: '97de212e80737a608d939f648d959671fb0a0142', @@ -64,10 +64,8 @@ describe CreateDeploymentService, services: true do tag: false, sha: '97de212e80737a608d939f648d959671fb0a0142', options: { - environment: { - name: 'review-apps/$CI_BUILD_REF_NAME', - url: 'http://$CI_BUILD_REF_NAME.review-apps.gitlab.com' - } + name: 'review-apps/$CI_BUILD_REF_NAME', + url: 'http://$CI_BUILD_REF_NAME.review-apps.gitlab.com' }, variables: [ { key: 'CI_BUILD_REF_NAME', value: 'feature-review-apps' } @@ -83,7 +81,7 @@ describe CreateDeploymentService, services: true do end it 'does create a new deployment' do - expect(subject).not_to be_persisted + expect(subject).to be_persisted end end end @@ -125,6 +123,12 @@ describe CreateDeploymentService, services: true do expect(Deployment.last.deployable).to eq(deployable) end + + it 'create environment has URL set' do + subject + + expect(Deployment.last.environment.external_url).not_to be_nil + end end context 'without environment specified' do @@ -137,7 +141,10 @@ describe CreateDeploymentService, services: true do context 'when environment is specified' do let(:pipeline) { create(:ci_pipeline, project: project) } - let(:build) { create(:ci_build, pipeline: pipeline, environment: 'production') } + let(:build) { create(:ci_build, pipeline: pipeline, environment: 'production', options: options) } + let(:options) do + { environment: { name: 'production', url: 'http://gitlab.com' } } + end context 'when build succeeds' do it_behaves_like 'does create environment and deployment' do |