From a4638dddf22797f46d72ea7b73c8453ba68645ab Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 13 Sep 2016 14:14:55 +0200 Subject: Add support for dynamic environments Environments that can have a URL with predefined CI variables. --- app/models/ci/build.rb | 4 ++- app/models/environment.rb | 12 ++++++++ app/models/merge_request.rb | 13 ++++++++- app/services/create_deployment_service.rb | 34 +++++++++++++++++++++- ...7131111_add_environment_type_to_environments.rb | 29 ++++++++++++++++++ lib/expand_variables.rb | 15 ++++++++++ lib/gitlab/ci/config/node/environment.rb | 33 +++++++++++++++++++++ lib/gitlab/regex.rb | 4 +-- 8 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20160907131111_add_environment_type_to_environments.rb create mode 100644 lib/expand_variables.rb create mode 100644 lib/gitlab/ci/config/node/environment.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index fb16bc06d71..abdf8c76447 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -83,7 +83,9 @@ module Ci environment: build.environment, sha: build.sha, ref: build.ref, - tag: build.tag) + tag: build.tag, + options: build.options[:environment], + variables: variables) service.execute(build) end end diff --git a/app/models/environment.rb b/app/models/environment.rb index 75e6f869786..33c9abf382a 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -4,6 +4,7 @@ class Environment < ActiveRecord::Base has_many :deployments before_validation :nullify_external_url + before_save :set_environment_type validates :name, presence: true, @@ -26,6 +27,17 @@ class Environment < ActiveRecord::Base self.external_url = nil if self.external_url.blank? end + def set_environment_type + names = name.split('/') + + self.environment_type = + if names.many? + names.first + else + nil + end + end + def includes_commit?(commit) return false unless last_deployment diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 75f48fd4ba5..b215f02e4b7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -744,10 +744,21 @@ class MergeRequest < ActiveRecord::Base @pipeline ||= source_project.pipeline_for(source_branch, diff_head_sha) end + def all_commits_sha + merge_request_diffs.map(&:commits).flatten.map(&:sha).sort.uniq + end + + def latest_pipelines + @latest_pipelines ||= + if diff_head_sha && source_project + source_project.pipelines.order(id: :desc).where(sha: commits_sha, ref: source_branch) + end + end + def all_pipelines @all_pipelines ||= if diff_head_sha && source_project - source_project.pipelines.order(id: :desc).where(sha: commits_sha, ref: source_branch) + source_project.pipelines.order(id: :desc).where(sha: all_commits_sha, ref: source_branch) end end diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index efeb9df9527..efa0798f47d 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -3,9 +3,13 @@ require_relative 'base_service' class CreateDeploymentService < BaseService def execute(deployable = nil) environment = project.environments.find_or_create_by( - name: params[:environment] + name: expanded_name ) + if expanded_url + environment.external_url = expanded_url + end + project.deployments.create( environment: environment, ref: params[:ref], @@ -15,4 +19,32 @@ class CreateDeploymentService < BaseService deployable: deployable ) end + + private + + def expanded_name + name.expand_variables(variables) + end + + def expanded_url + return unless url + + @expanded_url ||= url.expand_variables(variables) + end + + def name + params[:environment] + end + + def url + options[:url] + end + + def options + params[:environment] || {} + end + + def variables + params[:variables] || [] + end end diff --git a/db/migrate/20160907131111_add_environment_type_to_environments.rb b/db/migrate/20160907131111_add_environment_type_to_environments.rb new file mode 100644 index 00000000000..d22b3f4d2d1 --- /dev/null +++ b/db/migrate/20160907131111_add_environment_type_to_environments.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddEnvironmentTypeToEnvironments < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :environments, :environment_type, :string + end +end diff --git a/lib/expand_variables.rb b/lib/expand_variables.rb new file mode 100644 index 00000000000..40bf9483eb5 --- /dev/null +++ b/lib/expand_variables.rb @@ -0,0 +1,15 @@ +String.instance_eval + def expand_variables(variables) + # Convert hash array to variables + if variables.is_a?(Array) + variables = variables.reduce({}) do |hash, variable| + hash[variable[:key]] = variable[:value] + hash + end + end + + self.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do + variables[$1 || $2] + end + end +end diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb new file mode 100644 index 00000000000..6f04180039e --- /dev/null +++ b/lib/gitlab/ci/config/node/environment.rb @@ -0,0 +1,33 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents environment variables. + # + class Environment < Entry + include Validatable + + validations do + include LegacyValidationHelpers + + validate do + unless string_or_array_of_strings?(config) + errors.add(:config, + 'should be a string or an array of strings') + end + end + + def string_or_array_of_strings?(field) + validate_string(field) || validate_array_of_strings(field) + end + end + + def value + Array(@config) + end + end + end + end + end +end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index ffad5e17c78..d1a3e54ccd7 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -96,11 +96,11 @@ module Gitlab end def environment_name_regex - @environment_name_regex ||= /\A[a-zA-Z0-9_-]+\z/.freeze + git_reference_regex end def environment_name_regex_message - "can contain only letters, digits, '-' and '_'." + "be a valid git reference name" end end end -- cgit v1.2.1 From ba5bd3d1d64b1f56c39e4ddd03270de6820b2f7b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 14 Sep 2016 19:52:28 +0200 Subject: Add new CI configuration entry for the environment --- lib/ci/gitlab_ci_yaml_processor.rb | 3 +- lib/gitlab/ci/config/node/environment.rb | 33 ++++-- lib/gitlab/ci/config/node/job.rb | 6 +- spec/lib/gitlab/ci/config/node/environment_spec.rb | 125 +++++++++++++++++++++ 4 files changed, 157 insertions(+), 10 deletions(-) create mode 100644 spec/lib/gitlab/ci/config/node/environment_spec.rb diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index caa815f720f..94a63508f79 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -60,7 +60,7 @@ module Ci name: job[:name].to_s, allow_failure: job[:allow_failure] || false, when: job[:when] || 'on_success', - environment: job[:environment], + environment: job.fetch(:environment, {})[:name], yaml_variables: yaml_variables(name), options: { image: job[:image], @@ -69,6 +69,7 @@ module Ci cache: job[:cache], dependencies: job[:dependencies], after_script: job[:after_script], + environment: job[:environment], }.compact } end diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index 6f04180039e..e2fb1ab131e 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -3,28 +3,45 @@ module Gitlab class Config module Node ## - # Entry that represents environment variables. + # Entry that represents an environment. # class Environment < Entry include Validatable validations do - include LegacyValidationHelpers + validates :name, presence: true validate do - unless string_or_array_of_strings?(config) - errors.add(:config, - 'should be a string or an array of strings') + unless hash? || string? + errors.add(:config, 'should be a hash or a string') end end + end + + def hash? + @config.is_a?(Hash) + end + + def string? + @config.is_a?(String) + end - def string_or_array_of_strings?(field) - validate_string(field) || validate_array_of_strings(field) + def name + case + when string? then @config + when hash? then @config[:name] end end + def url + @config[:url] if hash? + end + def value - Array(@config) + case + when string? then { name: @config } + when hash? then @config + end end end end diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb index 0cbdf7619c0..e90e80171a4 100644 --- a/lib/gitlab/ci/config/node/job.rb +++ b/lib/gitlab/ci/config/node/job.rb @@ -13,7 +13,7 @@ module Gitlab type stage when artifacts cache dependencies before_script after_script variables environment] - attributes :tags, :allow_failure, :when, :environment, :dependencies + attributes :tags, :allow_failure, :when, :dependencies validations do validates :config, allowed_keys: ALLOWED_KEYS @@ -78,6 +78,9 @@ module Gitlab node :artifacts, Artifacts, description: 'Artifacts configuration for this job.' + node :environment, Environment, + description: 'Environment configuration for this job.' + helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, :artifacts, :commands @@ -133,6 +136,7 @@ module Gitlab only: only, except: except, variables: variables_defined? ? variables : nil, + environment: environment_defined? ? environment : nil, artifacts: artifacts, after_script: after_script } end diff --git a/spec/lib/gitlab/ci/config/node/environment_spec.rb b/spec/lib/gitlab/ci/config/node/environment_spec.rb new file mode 100644 index 00000000000..27238f9c887 --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/environment_spec.rb @@ -0,0 +1,125 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Environment do + let(:entry) { described_class.new(config) } + + before { entry.compose! } + + context 'when configuration is a string' do + let(:config) { 'production' } + + describe '#string?' do + it 'is string configuration' do + expect(entry).to be_string + end + end + + describe '#hash?' do + it 'is not hash configuration' do + expect(entry).not_to be_hash + end + end + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + describe '#value' do + it 'returns valid hash' do + expect(entry.value).to eq(name: 'production') + end + end + + describe '#name' do + it 'returns environment name' do + expect(entry.name).to eq 'production' + end + end + + describe '#url' do + it 'returns environment url' do + expect(entry.url).to be_nil + end + end + end + + context 'when configuration is a hash' do + let(:config) do + { name: 'development', url: 'https://example.gitlab.com' } + end + + describe '#string?' do + it 'is not string configuration' do + expect(entry).not_to be_string + end + end + + describe '#hash?' do + it 'is hash configuration' do + expect(entry).to be_hash + end + end + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + describe '#value' do + it 'returns valid hash' do + expect(entry.value).to eq config + end + end + + describe '#name' do + it 'returns environment name' do + expect(entry.name).to eq 'development' + end + end + + describe '#url' do + it 'returns environment url' do + expect(entry.url).to eq 'https://example.gitlab.com' + end + end + end + + context 'when configuration is invalid' do + context 'when configuration is an array' do + let(:config) { ['env'] } + + describe '#valid?' do + it 'is not valid' do + expect(entry).not_to be_valid + end + end + + describe '#errors' do + it 'contains error about invalid type' do + expect(entry.errors) + .to include 'environment config should be a hash or a string' + end + end + end + + context 'when environment name is not present' do + let(:config) { { url: 'https://example.gitlab.com' } } + + describe '#valid?' do + it 'is not valid' do + expect(entry).not_to be_valid + end + end + + describe '#errors?' do + it 'contains error about missing environment name' do + expect(entry.errors) + .to include "environment name can't be blank" + end + end + end + end +end -- cgit v1.2.1 From 08272ec1513cbd565e5db5995a681c25e1f4544f Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 14 Sep 2016 22:14:26 +0200 Subject: Add validation of URL and validation of name --- lib/gitlab/ci/config/node/environment.rb | 5 ++++ spec/lib/gitlab/ci/config/node/environment_spec.rb | 30 ++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index e2fb1ab131e..629c17e6250 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -11,6 +11,11 @@ module Gitlab validations do validates :name, presence: true + validates :url, + length: { maximum: 255 }, + allow_nil: true, + addressable_url: true + validate do unless hash? || string? errors.add(:config, 'should be a hash or a string') diff --git a/spec/lib/gitlab/ci/config/node/environment_spec.rb b/spec/lib/gitlab/ci/config/node/environment_spec.rb index 27238f9c887..df453223da7 100644 --- a/spec/lib/gitlab/ci/config/node/environment_spec.rb +++ b/spec/lib/gitlab/ci/config/node/environment_spec.rb @@ -87,6 +87,19 @@ describe Gitlab::Ci::Config::Node::Environment do end end + context 'when variables are used for environment' do + let(:config) do + { name: 'review/$CI_BUILD_REF_NAME', + url: 'https://$CI_BUILD_REF_NAME.review.gitlab.com' } + end + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + context 'when configuration is invalid' do context 'when configuration is an array' do let(:config) { ['env'] } @@ -121,5 +134,22 @@ describe Gitlab::Ci::Config::Node::Environment do end end end + + context 'when invalid URL is used' do + let(:config) { { name: 'test', url: 'invalid-example.gitlab.com' } } + + describe '#valid?' do + it 'is not valid' do + expect(entry).not_to be_valid + end + end + + describe '#errors?' do + it 'contains error about invalid URL' do + expect(entry.errors) + .to include "environment url must be a valid url" + end + end + end end end -- cgit v1.2.1 From 2cc9a785dfdada5e2976b8341d3c9e6eae8fa66f Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 14 Sep 2016 22:33:56 +0200 Subject: Properly create deployment using all possible options --- app/models/ci/build.rb | 15 ++++++++------- lib/ci/api/entities.rb | 9 +++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index abdf8c76447..47dedef38d0 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -79,13 +79,14 @@ module Ci after_transition any => [:success] do |build| if build.environment.present? - service = CreateDeploymentService.new(build.project, build.user, - environment: build.environment, - sha: build.sha, - ref: build.ref, - tag: build.tag, - options: build.options[:environment], - variables: variables) + service = CreateDeploymentService.new( + build.project, build.user, + environment: build.environment, + sha: build.sha, + ref: build.ref, + tag: build.tag, + options: build.options[:environment], + variables: variables) service.execute(build) end end diff --git a/lib/ci/api/entities.rb b/lib/ci/api/entities.rb index 3f5bdaba3f5..66c05773b68 100644 --- a/lib/ci/api/entities.rb +++ b/lib/ci/api/entities.rb @@ -15,6 +15,15 @@ module Ci expose :filename, :size end + class BuildOptions < Grape::Entity + expose :image + expose :services + expose :artifacts + expose :cache + expose :dependencies + expose :after_script + end + class Build < Grape::Entity expose :id, :ref, :tag, :sha, :status expose :name, :token, :stage -- cgit v1.2.1 From e1b3ab5af290f6d2eeb56c4b72e341324414a6d2 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 14 Sep 2016 22:32:11 +0200 Subject: Verify expandability of variables defined as part of environment --- app/services/create_deployment_service.rb | 4 +- lib/expand_variables.rb | 22 ++++---- spec/lib/expand_variables_spec.rb | 73 +++++++++++++++++++++++++ spec/models/environment_spec.rb | 16 ++++++ spec/services/create_deployment_service_spec.rb | 32 ++++++++++- 5 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 spec/lib/expand_variables_spec.rb diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index efa0798f47d..577ba731583 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -23,13 +23,13 @@ class CreateDeploymentService < BaseService private def expanded_name - name.expand_variables(variables) + ExpandVariables.expand(name, variables) end def expanded_url return unless url - @expanded_url ||= url.expand_variables(variables) + @expanded_url ||= ExpandVariables.expand(url, variables) end def name diff --git a/lib/expand_variables.rb b/lib/expand_variables.rb index 40bf9483eb5..669735cc56c 100644 --- a/lib/expand_variables.rb +++ b/lib/expand_variables.rb @@ -1,15 +1,17 @@ -String.instance_eval - def expand_variables(variables) - # Convert hash array to variables - if variables.is_a?(Array) - variables = variables.reduce({}) do |hash, variable| - hash[variable[:key]] = variable[:value] - hash +module ExpandVariables + class << self + def expand_variables(value, variables) + # Convert hash array to variables + if variables.is_a?(Array) + variables = variables.reduce({}) do |hash, variable| + hash[variable[:key]] = variable[:value] + hash + end end - end - self.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do - variables[$1 || $2] + value.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do + variables[$1 || $2] + end end end end diff --git a/spec/lib/expand_variables_spec.rb b/spec/lib/expand_variables_spec.rb new file mode 100644 index 00000000000..90bc7dad379 --- /dev/null +++ b/spec/lib/expand_variables_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe ExpandVariables do + describe '#expand' do + subject { described_class.expand(value, variables) } + + tests = [ + { value: 'key', + result: 'key', + variables: [] + }, + { value: 'key$variable', + result: 'key', + variables: [] + }, + { value: 'key$variable', + result: 'keyvalue', + variables: [ + { key: 'variable', value: 'value' } + ] + }, + { value: 'key${variable}', + result: 'keyvalue', + variables: [ + { key: 'variable', value: 'value' } + ] + }, + { value: 'key$variable$variable2', + result: 'keyvalueresult', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' }, + ] + }, + { value: 'key${variable}${variable2}', + result: 'keyvalueresult', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' } + ] + }, + { value: 'key$variable2$variable', + result: 'keyresultvalue', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' }, + ] + }, + { value: 'key${variable2}${variable}', + result: 'keyresultvalue', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' } + ] + }, + { value: 'review/$CI_BUILD_REF_NAME', + result: 'review/feature/add-review-apps', + variables: [ + { key: 'CI_BUILD_REF_NAME', value: 'feature/add-review-apps' } + ] + }, + ] + + tests.each do |test| + context "#{test[:value]} resolves to #{test[:result]}" do + let(:value) { test[:value] } + let(:variables) { test[:variables] } + + it { is_expected.to eq(test[:result]) } + end + end + end +end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index c881897926e..7afc7ec5ca1 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -63,4 +63,20 @@ describe Environment, models: true do end end end + + describe '#environment_type' do + subject { environment.environment_type } + + it 'sets a environment type if name has multiple segments' do + environment.update(name: 'production/worker.gitlab.com') + + is_expected.to eq('production') + end + + it 'nullifies a type if it\'s a simple name' do + environment.update(name: 'production') + + is_expected.to be_nil + end + end end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 8da2a2b3c1b..c8c741c0e88 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -56,8 +56,38 @@ describe CreateDeploymentService, services: true do expect(subject).not_to be_persisted end end + + context 'when variables are used' do + let(:params) do + { environment: 'review-apps/$CI_BUILD_REF_NAME', + ref: 'master', + tag: false, + sha: '97de212e80737a608d939f648d959671fb0a0142', + options: { + environment: { + 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' } + ] + } + end + + it 'does create a new environment' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject.environment.name).to eq('review-apps/feature-review-apps') + expect(subject.environment.external_url).to eq('http://feature-review-apps.review-apps.gitlab.com') + end + + it 'does create a new deployment' do + expect(subject).not_to be_persisted + end + end end - + describe 'processing of builds' do let(:environment) { nil } -- cgit v1.2.1 From 6b979687459ad1ab5f1953bf451ee80fdb899b96 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 14 Sep 2016 23:00:15 +0200 Subject: Update support for dynamic environments --- app/models/ci/build.rb | 2 +- app/services/create_deployment_service.rb | 16 ++++++++-------- lib/expand_variables.rb | 2 +- lib/gitlab/ci/config/node/environment.rb | 16 ++++++++++------ lib/gitlab/ci/config/node/job.rb | 10 +--------- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 14 ++++++++++++++ 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 -- cgit v1.2.1 From 274d3d50e5d24bef18098ee16464873e17fa010a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 14 Sep 2016 23:27:54 +0200 Subject: Added missing db/schema changes --- db/schema.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 974f5855cf9..930d41d8bdd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -390,10 +390,11 @@ ActiveRecord::Schema.define(version: 20160913212128) do create_table "environments", force: :cascade do |t| t.integer "project_id" - t.string "name", null: false + t.string "name", null: false t.datetime "created_at" t.datetime "updated_at" t.string "external_url" + t.string "environment_type" end add_index "environments", ["project_id", "name"], name: "index_environments_on_project_id_and_name", using: :btree -- cgit v1.2.1 From 4a5c21728ee4e6c3ef8e1c410ee0f0c9a47634cc Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 14 Sep 2016 23:33:10 +0200 Subject: Added documentation about dynamic environments --- CHANGELOG | 2 ++ doc/ci/yaml/README.md | 29 ++++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index e9445a18a18..3da548ef2af 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -25,6 +25,8 @@ v 8.12.0 (unreleased) - Fix sorting of issues in API - Sort project variables by key. !6275 (Diego Souza) - Ensure specs on sorting of issues in API are deterministic on MySQL + - Added ability to use predefined CI variables for environment name + - Added ability to specify URL in environment configuration in gitlab-ci.yml - Escape search term before passing it to Regexp.new !6241 (winniehell) - Fix pinned sidebar behavior in smaller viewports !6169 - Fix file permissions change when updating a file on the Gitlab UI !5979 diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index ff4c8ddc54b..4772565fac9 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -553,7 +553,7 @@ GitLab. If `environment` is specified and no environment under that name exists, a new one will be created automatically. -The `environment` name must contain only letters, digits, '-' and '_'. Common +The `environment` name must be a valid git reference name. Common names are `qa`, `staging`, and `production`, but you can use whatever name works with your workflow. @@ -571,6 +571,33 @@ deploy to production: The `deploy to production` job will be marked as doing deployment to `production` environment. +#### dynamic environments + +>**Note:** +Introduced in GitLab 8.12. + +`environment` can also represent a configuration hash with `name` and `url`. +These parameters can use any of defined CI variables (including predefined, secure variables and .gitlab-ci.yml variables). + +The common use case is to create a dynamic environments for branches and use them as review apps. + +--- + +**Example configurations** + +``` +deploy as review app: + stage: deploy + script: ... + environment: + name: review-apps/$CI_BUILD_REF_NAME + url: https://$CI_BUILD_REF_NAME.review.example.com/ +``` + +The `deploy to production` job will be marked as doing deployment to +`production` environment. + + ### artifacts >**Notes:** -- cgit v1.2.1 From abfceb1e565490bb75e9a4fba0571cb2390fa5d8 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 15 Sep 2016 21:59:01 +0200 Subject: Cleanup changes --- app/models/merge_request.rb | 13 +------------ ...907131111_add_environment_type_to_environments.rb | 20 -------------------- doc/ci/yaml/README.md | 9 +++++---- lib/gitlab/ci/config/node/environment.rb | 7 ++----- 4 files changed, 8 insertions(+), 41 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b215f02e4b7..75f48fd4ba5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -744,21 +744,10 @@ class MergeRequest < ActiveRecord::Base @pipeline ||= source_project.pipeline_for(source_branch, diff_head_sha) end - def all_commits_sha - merge_request_diffs.map(&:commits).flatten.map(&:sha).sort.uniq - end - - def latest_pipelines - @latest_pipelines ||= - if diff_head_sha && source_project - source_project.pipelines.order(id: :desc).where(sha: commits_sha, ref: source_branch) - end - end - def all_pipelines @all_pipelines ||= if diff_head_sha && source_project - source_project.pipelines.order(id: :desc).where(sha: all_commits_sha, ref: source_branch) + source_project.pipelines.order(id: :desc).where(sha: commits_sha, ref: source_branch) end end diff --git a/db/migrate/20160907131111_add_environment_type_to_environments.rb b/db/migrate/20160907131111_add_environment_type_to_environments.rb index d22b3f4d2d1..fac73753d5b 100644 --- a/db/migrate/20160907131111_add_environment_type_to_environments.rb +++ b/db/migrate/20160907131111_add_environment_type_to_environments.rb @@ -1,28 +1,8 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddEnvironmentTypeToEnvironments < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers - # Set this constant to true if this migration requires downtime. DOWNTIME = false - # When a migration requires downtime you **must** uncomment the following - # constant and define a short and easy to understand explanation as to why the - # migration requires downtime. - # DOWNTIME_REASON = '' - - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - def change add_column :environments, :environment_type, :string end diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 4772565fac9..a4cf0ec468a 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -577,9 +577,9 @@ The `deploy to production` job will be marked as doing deployment to Introduced in GitLab 8.12. `environment` can also represent a configuration hash with `name` and `url`. -These parameters can use any of defined CI variables (including predefined, secure variables and .gitlab-ci.yml variables). +These parameters can use any of the defined CI variables (including predefined, secure variables and `.gitlab-ci.yml` variables). -The common use case is to create a dynamic environments for branches and use them as review apps. +The common use case is to create dynamic environments for branches and use them as review apps. --- @@ -594,9 +594,10 @@ deploy as review app: url: https://$CI_BUILD_REF_NAME.review.example.com/ ``` -The `deploy to production` job will be marked as doing deployment to -`production` environment. +The `deploy as review app` job will be marked as deployment to +dynamically created `review-apps/branch-name` environment. +This environment should be accessible under `https://branch-name.review.example.com/`. ### artifacts diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index 85302589ce6..580fcda7549 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -36,14 +36,11 @@ module Gitlab end def name - case @config.type - when String then @config - when Hash then @config[:name] - end + value[:name] end def url - @config[:url] if hash? + value[:url] end def value -- cgit v1.2.1 From 04b56955906f3fa4a5a41912d1b1234e45c392c6 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Fri, 16 Sep 2016 09:53:08 +0200 Subject: Small refactor of review apps docs --- doc/ci/yaml/README.md | 66 ++++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index a4cf0ec468a..f65340f190e 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -90,8 +90,7 @@ builds, including deploy builds. This can be an array or a multi-line string. ### after_script ->**Note:** -Introduced in GitLab 8.7 and requires Gitlab Runner v1.2 +> Introduced in GitLab 8.7 and requires Gitlab Runner v1.2 `after_script` is used to define the command that will be run after for all builds. This has to be an array or a multi-line string. @@ -135,8 +134,7 @@ Alias for [stages](#stages). ### variables ->**Note:** -Introduced in GitLab Runner v0.5.0. +> Introduced in GitLab Runner v0.5.0. GitLab CI allows you to add variables to `.gitlab-ci.yml` that are set in the build environment. The variables are stored in the Git repository and are meant @@ -158,8 +156,7 @@ Variables can be also defined on [job level](#job-variables). ### cache ->**Note:** -Introduced in GitLab Runner v0.7.0. +> Introduced in GitLab Runner v0.7.0. `cache` is used to specify a list of files and directories which should be cached between builds. @@ -220,8 +217,7 @@ will be always present. For implementation details, please check GitLab Runner. #### cache:key ->**Note:** -Introduced in GitLab Runner v1.0.0. +> Introduced in GitLab Runner v1.0.0. The `key` directive allows you to define the affinity of caching between jobs, allowing to have a single cache for all jobs, @@ -531,8 +527,7 @@ The above script will: #### Manual actions ->**Note:** -Introduced in GitLab 8.10. +> Introduced in GitLab 8.10. Manual actions are a special type of job that are not executed automatically; they need to be explicitly started by a user. Manual actions can be started @@ -543,17 +538,16 @@ An example usage of manual actions is deployment to production. ### environment ->**Note:** -Introduced in GitLab 8.9. +> Introduced in GitLab 8.9. -`environment` is used to define that a job deploys to a specific environment. +`environment` is used to define that a job deploys to a specific [environment]. This allows easy tracking of all deployments to your environments straight from GitLab. If `environment` is specified and no environment under that name exists, a new one will be created automatically. -The `environment` name must be a valid git reference name. Common +The `environment` name must be a valid [Git reference name][gitref]. Common names are `qa`, `staging`, and `production`, but you can use whatever name works with your workflow. @@ -573,13 +567,14 @@ The `deploy to production` job will be marked as doing deployment to #### dynamic environments ->**Note:** -Introduced in GitLab 8.12. +> [Introduced][ce-6323] in GitLab 8.12 and GitLab Runner 1.6. `environment` can also represent a configuration hash with `name` and `url`. -These parameters can use any of the defined CI variables (including predefined, secure variables and `.gitlab-ci.yml` variables). +These parameters can use any of the defined CI [variables](#variables) +(including predefined, secure variables and `.gitlab-ci.yml` variables). -The common use case is to create dynamic environments for branches and use them as review apps. +The common use case is to create dynamic environments for branches and use them +as review apps. --- @@ -589,13 +584,13 @@ The common use case is to create dynamic environments for branches and use them deploy as review app: stage: deploy script: ... - environment: + environment: name: review-apps/$CI_BUILD_REF_NAME url: https://$CI_BUILD_REF_NAME.review.example.com/ ``` -The `deploy as review app` job will be marked as deployment to -dynamically created `review-apps/branch-name` environment. +The `deploy as review app` job will be marked as deployment to dynamically +create the `review-apps/branch-name` environment. This environment should be accessible under `https://branch-name.review.example.com/`. @@ -666,8 +661,7 @@ be available for download in the GitLab UI. #### artifacts:name ->**Note:** -Introduced in GitLab 8.6 and GitLab Runner v1.1.0. +> Introduced in GitLab 8.6 and GitLab Runner v1.1.0. The `name` directive allows you to define the name of the created artifacts archive. That way, you can have a unique name for every archive which could be @@ -730,8 +724,7 @@ job: #### artifacts:when ->**Note:** -Introduced in GitLab 8.9 and GitLab Runner v1.3.0. +> Introduced in GitLab 8.9 and GitLab Runner v1.3.0. `artifacts:when` is used to upload artifacts on build failure or despite the failure. @@ -756,8 +749,7 @@ job: #### artifacts:expire_in ->**Note:** -Introduced in GitLab 8.9 and GitLab Runner v1.3.0. +> Introduced in GitLab 8.9 and GitLab Runner v1.3.0. `artifacts:expire_in` is used to delete uploaded artifacts after the specified time. By default, artifacts are stored on GitLab forever. `expire_in` allows you @@ -792,8 +784,7 @@ job: ### dependencies ->**Note:** -Introduced in GitLab 8.6 and GitLab Runner v1.1.1. +> Introduced in GitLab 8.6 and GitLab Runner v1.1.1. This feature should be used in conjunction with [`artifacts`](#artifacts) and allows you to define the artifacts to pass between different builds. @@ -867,9 +858,8 @@ job: ## Git Strategy ->**Note:** -Introduced in GitLab 8.9 as an experimental feature. May change in future -releases or be removed completely. +> Introduced in GitLab 8.9 as an experimental feature. May change in future + releases or be removed completely. You can set the `GIT_STRATEGY` used for getting recent application code. `clone` is slower, but makes sure you have a clean directory before every build. `fetch` @@ -891,8 +881,7 @@ variables: ## Shallow cloning ->**Note:** -Introduced in GitLab 8.9 as an experimental feature. May change in future +> Introduced in GitLab 8.9 as an experimental feature. May change in future releases or be removed completely. You can specify the depth of fetching and cloning using `GIT_DEPTH`. This allows @@ -922,8 +911,7 @@ variables: ## Hidden keys ->**Note:** -Introduced in GitLab 8.6 and GitLab Runner v1.1.1. +> Introduced in GitLab 8.6 and GitLab Runner v1.1.1. Keys that start with a dot (`.`) will be not processed by GitLab CI. You can use this feature to ignore jobs, or use the @@ -951,8 +939,7 @@ Read more about the various [YAML features](https://learnxinyminutes.com/docs/ya ### Anchors ->**Note:** -Introduced in GitLab 8.6 and GitLab Runner v1.1.1. +> Introduced in GitLab 8.6 and GitLab Runner v1.1.1. YAML also has a handy feature called 'anchors', which let you easily duplicate content across your document. Anchors can be used to duplicate/inherit @@ -1095,3 +1082,6 @@ Visit the [examples README][examples] to see a list of examples using GitLab CI with various languages. [examples]: ../examples/README.md +[ce-6323]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6323 +[gitref]: https://git-scm.com/docs/git-check-ref-format +[environment]: ../environments.md -- cgit v1.2.1 From 3fbfc30f5e131e950564712d6f0d1837cd7605e3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 16 Sep 2016 11:00:13 +0200 Subject: Fix CI job environment configuration entry class --- lib/gitlab/ci/config/node/environment.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index 580fcda7549..d2d00f0ad81 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -11,20 +11,22 @@ module Gitlab ALLOWED_KEYS = %i[name url] validations do - validates :config, allowed_keys: ALLOWED_KEYS, if: :hash? - validates :name, presence: true - validates :url, - length: { maximum: 255 }, - allow_nil: true, - addressable_url: true - validate do unless hash? || string? errors.add(:config, 'should be a hash or a string') end end + + with_options if: :hash? do + validates :config, allowed_keys: ALLOWED_KEYS + + validates :url, + length: { maximum: 255 }, + addressable_url: true, + allow_nil: true + end end def hash? @@ -44,9 +46,10 @@ module Gitlab end def value - case @config.type + case @config when String then { name: @config } when Hash then @config + else {} end end end -- cgit v1.2.1 From 2ad7753d34a963abfcadfa42e5d5b4cd59afc221 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 16 Sep 2016 11:25:37 +0200 Subject: Fix CI job environment configuration attributes --- lib/ci/gitlab_ci_yaml_processor.rb | 2 +- lib/gitlab/ci/config/node/environment.rb | 4 ++-- lib/gitlab/ci/config/node/job.rb | 1 + spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 5 +++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 94a63508f79..0369e80312a 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -60,7 +60,7 @@ module Ci name: job[:name].to_s, allow_failure: job[:allow_failure] || false, when: job[:when] || 'on_success', - environment: job.fetch(:environment, {})[:name], + environment: job[:environment_name], yaml_variables: yaml_variables(name), options: { image: job[:image], diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index d2d00f0ad81..bc153854a8d 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -11,14 +11,14 @@ module Gitlab ALLOWED_KEYS = %i[name url] validations do - validates :name, presence: true - validate do unless hash? || string? errors.add(:config, 'should be a hash or a string') end end + validates :name, presence: true + with_options if: :hash? do validates :config, allowed_keys: ALLOWED_KEYS diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb index 3ab34d23d37..6ecc46200c0 100644 --- a/lib/gitlab/ci/config/node/job.rb +++ b/lib/gitlab/ci/config/node/job.rb @@ -129,6 +129,7 @@ module Gitlab except: except, variables: variables_defined? ? variables : nil, environment: environment_defined? ? environment : nil, + environment_name: environment_defined? ? environment[:name] : nil, artifacts: artifacts, after_script: after_script } end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 2ad33007b8a..c139ef36a9a 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -767,7 +767,7 @@ module Ci 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) + expect(builds.first[:options]).to include(environment: environment) end end @@ -784,7 +784,8 @@ module Ci let(:environment) { 1 } it 'raises error' do - expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}") + expect { builds }.to raise_error( + 'jobs:deploy_to_production:environment config should be a hash or a string') end end -- cgit v1.2.1 From 99f1385ee02a368e8fa7cc0bcaad78b904d1a81d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 16 Sep 2016 11:38:56 +0200 Subject: Restore validation of CI job environment name --- lib/gitlab/ci/config/node/environment.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb index bc153854a8d..d388ab6b879 100644 --- a/lib/gitlab/ci/config/node/environment.rb +++ b/lib/gitlab/ci/config/node/environment.rb @@ -18,6 +18,15 @@ module Gitlab end validates :name, presence: true + validates :name, + type: { + with: String, + message: Gitlab::Regex.environment_name_regex_message } + + validates :name, + format: { + with: Gitlab::Regex.environment_name_regex, + message: Gitlab::Regex.environment_name_regex_message } with_options if: :hash? do validates :config, allowed_keys: ALLOWED_KEYS -- cgit v1.2.1 From 223041fa1bba534d613489f41d6143f1785fd0b4 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 18 Sep 2016 20:31:00 +0200 Subject: Fix environments handling --- doc/ci/yaml/README.md | 3 +-- lib/gitlab/regex.rb | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index f65340f190e..16868554c1f 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -547,7 +547,7 @@ GitLab. If `environment` is specified and no environment under that name exists, a new one will be created automatically. -The `environment` name must be a valid [Git reference name][gitref]. Common +The `environment` name must contain only letters, digits, '-', '_', '/', '$', '{', '}' and spaces. Common names are `qa`, `staging`, and `production`, but you can use whatever name works with your workflow. @@ -1083,5 +1083,4 @@ CI with various languages. [examples]: ../examples/README.md [ce-6323]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6323 -[gitref]: https://git-scm.com/docs/git-check-ref-format [environment]: ../environments.md diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index d1a3e54ccd7..4efd9ae2c1e 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -96,11 +96,11 @@ module Gitlab end def environment_name_regex - git_reference_regex + @environment_name_regex ||= /\A[a-zA-Z0-9_\\\/\${} -]+\z/.freeze end def environment_name_regex_message - "be a valid git reference name" + "can contain only letters, digits, '-', '_', '/', '$', '{', '}' and spaces" end end end -- cgit v1.2.1 From 31e8721a44ceddc4d3578f3af2b6c7a1797be35b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 19 Sep 2016 08:49:48 +0200 Subject: Fix scope of the CI config key nodes in jobs entry --- lib/gitlab/ci/config/node/job.rb | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb index 6ecc46200c0..603334d6793 100644 --- a/lib/gitlab/ci/config/node/job.rb +++ b/lib/gitlab/ci/config/node/job.rb @@ -34,43 +34,43 @@ module Gitlab end end - node :before_script, Script, + node :before_script, Node::Script, description: 'Global before script overridden in this job.' - node :script, Commands, + node :script, Node::Commands, description: 'Commands that will be executed in this job.' - node :stage, Stage, + node :stage, Node::Stage, description: 'Pipeline stage this job will be executed into.' - node :type, Stage, + node :type, Node::Stage, description: 'Deprecated: stage this job will be executed into.' - node :after_script, Script, + node :after_script, Node::Script, description: 'Commands that will be executed when finishing job.' - node :cache, Cache, + node :cache, Node::Cache, description: 'Cache definition for this job.' - node :image, Image, + node :image, Node::Image, description: 'Image that will be used to execute this job.' - node :services, Services, + node :services, Node::Services, description: 'Services that will be used to execute this job.' - node :only, Trigger, + node :only, Node::Trigger, description: 'Refs policy this job will be executed for.' - node :except, Trigger, + node :except, Node::Trigger, description: 'Refs policy this job will be executed for.' - node :variables, Variables, + node :variables, Node::Variables, description: 'Environment variables available for this job.' - node :artifacts, Artifacts, + node :artifacts, Node::Artifacts, description: 'Artifacts configuration for this job.' - node :environment, Environment, + node :environment, Node::Environment, description: 'Environment configuration for this job.' helpers :before_script, :script, :stage, :type, :after_script, -- cgit v1.2.1 From 8fe05d83ac259bf8a0fa4ca344d330a1c0cea8bb Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Sep 2016 09:01:04 +0200 Subject: Fix validation regexs (+1 squashed commit) Squashed commits: [f9a9315] Use : to test invalid environment name --- lib/gitlab/regex.rb | 4 ++-- spec/features/environments_spec.rb | 2 +- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 4 ++-- spec/models/environment_spec.rb | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 4efd9ae2c1e..bc8bbf337f3 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -96,11 +96,11 @@ module Gitlab end def environment_name_regex - @environment_name_regex ||= /\A[a-zA-Z0-9_\\\/\${} -]+\z/.freeze + @environment_name_regex ||= /\A[a-zA-Z0-9_\\\/\${}. -]+\z/.freeze end def environment_name_regex_message - "can contain only letters, digits, '-', '_', '/', '$', '{', '}' and spaces" + "can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.' and spaces" end end end diff --git a/spec/features/environments_spec.rb b/spec/features/environments_spec.rb index fcd41b38413..4309a726917 100644 --- a/spec/features/environments_spec.rb +++ b/spec/features/environments_spec.rb @@ -150,7 +150,7 @@ feature 'Environments', feature: true do context 'for invalid name' do before do - fill_in('Name', with: 'name with spaces') + fill_in('Name', with: 'name,with,commas') click_on 'Save' end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index c139ef36a9a..6dedd25e9d3 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -790,10 +790,10 @@ module Ci end context 'is not a valid string' do - let(:environment) { 'production staging' } + let(:environment) { 'production:staging' } it 'raises error' do - expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}") + expect { builds }.to raise_error("jobs:deploy_to_production:environment name #{Gitlab::Regex.environment_name_regex_message}") end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 7afc7ec5ca1..6b1867a44e1 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -68,13 +68,13 @@ describe Environment, models: true do subject { environment.environment_type } it 'sets a environment type if name has multiple segments' do - environment.update(name: 'production/worker.gitlab.com') + environment.update!(name: 'production/worker.gitlab.com') is_expected.to eq('production') end it 'nullifies a type if it\'s a simple name' do - environment.update(name: 'production') + environment.update!(name: 'production') is_expected.to be_nil end -- cgit v1.2.1 From 4939911e96297aa6ed9fb60f635d7d16a360876f Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Sep 2016 12:44:10 +0200 Subject: Fix specs failures --- spec/services/create_deployment_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 7ebfddc45d5..41b897f36cd 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: '..', + { environment: 'name,with,commas', ref: 'master', tag: false, sha: '97de212e80737a608d939f648d959671fb0a0142', -- cgit v1.2.1