From 05bc71c856c735fa364e15db62ba5d40cfd7c5ef Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 14 Dec 2016 22:38:40 +0800 Subject: [PATCH 01/10] Convert CI YAML variables keys into strings So that this would be more consistent with the other variables, which all of them are string based. Closes #25554 --- app/models/ci/build.rb | 2 +- changelogs/unreleased/fix-yaml-variables.yml | 4 +++ lib/ci/gitlab_ci_yaml_processor.rb | 2 +- lib/gitlab/serialize/yaml_variables.rb | 30 +++++++++++++++++++ spec/factories/ci/builds.rb | 2 +- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 24 +++++++-------- .../gitlab/serialize/yaml_variables_spec.rb | 19 ++++++++++++ spec/models/build_spec.rb | 2 +- 8 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 changelogs/unreleased/fix-yaml-variables.yml create mode 100644 lib/gitlab/serialize/yaml_variables.rb create mode 100644 spec/lib/gitlab/serialize/yaml_variables_spec.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index e7cf606a7ae..99f3f4711aa 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -10,7 +10,7 @@ module Ci has_many :deployments, as: :deployable serialize :options - serialize :yaml_variables + serialize :yaml_variables, Gitlab::Serialize::YamlVariables validates :coverage, numericality: true, allow_blank: true validates_presence_of :ref diff --git a/changelogs/unreleased/fix-yaml-variables.yml b/changelogs/unreleased/fix-yaml-variables.yml new file mode 100644 index 00000000000..3abff1e3b08 --- /dev/null +++ b/changelogs/unreleased/fix-yaml-variables.yml @@ -0,0 +1,4 @@ +--- +title: Convert CI YAML variables keys into strings +merge_request: 8088 +author: diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index fef652cb975..8806a506ffa 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -118,7 +118,7 @@ module Ci .merge(job_variables(name)) variables.map do |key, value| - { key: key, value: value, public: true } + { key: key.to_s, value: value.to_s, public: true } end end diff --git a/lib/gitlab/serialize/yaml_variables.rb b/lib/gitlab/serialize/yaml_variables.rb new file mode 100644 index 00000000000..ca44acbd906 --- /dev/null +++ b/lib/gitlab/serialize/yaml_variables.rb @@ -0,0 +1,30 @@ + +module Gitlab + module Serialize + # This serializer could make sure our YAML variables' keys and values + # are always strings. This is more for legacy build data because + # from now on we convert them into strings before saving to database. + module YamlVariables + extend self + + def load(string) + return unless string + + YAML.load(string). + map(&YamlVariables.method(:convert_key_value_to_string)) + end + + def dump(object) + YAML.dump(object) + end + + private + + def convert_key_value_to_string(variable) + variable[:key] = variable[:key].to_s + variable[:value] = variable[:value].to_s + variable + end + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 62466c06194..0397d5d4001 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -22,7 +22,7 @@ FactoryGirl.define do yaml_variables do [ - { key: :DB_NAME, value: 'postgres', public: true } + { key: 'DB_NAME', value: 'postgres', public: true } ] end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index ff5dcc06ab3..62d68721574 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -483,7 +483,7 @@ module Ci context 'when global variables are defined' do let(:variables) do - { VAR1: 'value1', VAR2: 'value2' } + { 'VAR1' => 'value1', 'VAR2' => 'value2' } end let(:config) do { @@ -495,18 +495,18 @@ module Ci it 'returns global variables' do expect(subject).to contain_exactly( - { key: :VAR1, value: 'value1', public: true }, - { key: :VAR2, value: 'value2', public: true } + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } ) end end context 'when job and global variables are defined' do let(:global_variables) do - { VAR1: 'global1', VAR3: 'global3' } + { 'VAR1' => 'global1', 'VAR3' => 'global3' } end let(:job_variables) do - { VAR1: 'value1', VAR2: 'value2' } + { 'VAR1' => 'value1', 'VAR2' => 'value2' } end let(:config) do { @@ -518,9 +518,9 @@ module Ci it 'returns all unique variables' do expect(subject).to contain_exactly( - { key: :VAR3, value: 'global3', public: true }, - { key: :VAR1, value: 'value1', public: true }, - { key: :VAR2, value: 'value2', public: true } + { key: 'VAR3', value: 'global3', public: true }, + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } ) end end @@ -535,13 +535,13 @@ module Ci context 'when syntax is correct' do let(:variables) do - { VAR1: 'value1', VAR2: 'value2' } + { 'VAR1' => 'value1', 'VAR2' => 'value2' } end it 'returns job variables' do expect(subject).to contain_exactly( - { key: :VAR1, value: 'value1', public: true }, - { key: :VAR2, value: 'value2', public: true } + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } ) end end @@ -549,7 +549,7 @@ module Ci context 'when syntax is incorrect' do context 'when variables defined but invalid' do let(:variables) do - [ :VAR1, 'value1', :VAR2, 'value2' ] + [ 'VAR1', 'value1', 'VAR2', 'value2' ] end it 'raises error' do diff --git a/spec/lib/gitlab/serialize/yaml_variables_spec.rb b/spec/lib/gitlab/serialize/yaml_variables_spec.rb new file mode 100644 index 00000000000..6d74f8c44d6 --- /dev/null +++ b/spec/lib/gitlab/serialize/yaml_variables_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::Serialize::YamlVariables do + subject do + Gitlab::Serialize::YamlVariables.load( + Gitlab::Serialize::YamlVariables.dump(object)) + end + + let(:object) do + [{ key: :key, value: 'value', public: true }, + { key: 'wee', value: 1, public: false }] + end + + it 'converts key and values into strings' do + is_expected.to eq([ + { key: 'key', value: 'value', public: true }, + { key: 'wee', value: '1', public: false }]) + end +end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 7f39aff7639..13928695ccb 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -403,7 +403,7 @@ describe Ci::Build, models: true do }) end let(:variables) do - [{ key: :KEY, value: 'value', public: true }] + [{ key: 'KEY', value: 'value', public: true }] end it { is_expected.to eq(predefined_variables + variables) } From e682e2f888fd84deefc7b4b028d00a55bdd1c3a5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 15 Dec 2016 22:55:23 +0800 Subject: [PATCH 02/10] Strictly check the type loaded from YAML Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20062470 Eventually we should move to SafeYAML, but requiring that would impact all other `YAML.load` which is bad. For this particular case, I think we could just check it strictly. --- lib/gitlab/serialize/yaml_variables.rb | 27 ++++++++++++++++--- .../gitlab/serialize/yaml_variables_spec.rb | 21 +++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/serialize/yaml_variables.rb b/lib/gitlab/serialize/yaml_variables.rb index ca44acbd906..68ca50ed60e 100644 --- a/lib/gitlab/serialize/yaml_variables.rb +++ b/lib/gitlab/serialize/yaml_variables.rb @@ -10,15 +10,36 @@ module Gitlab def load(string) return unless string - YAML.load(string). - map(&YamlVariables.method(:convert_key_value_to_string)) + object = YAML.load(string) + + # We don't need to verify the object once we're using SafeYAML + if YamlVariables.verify_object(object) + YamlVariables.convert_object(object) + else + [] + end end def dump(object) YAML.dump(object) end - private + def verify_object(object) + YamlVariables.verify_type(object, Array) && + object.all? { |obj| YamlVariables.verify_type(obj, Hash) } + end + + # We use three ways to check if the class is exactly the one we want, + # rather than some subclass or duck typing class. + def verify_type(object, klass) + object.kind_of?(klass) && + object.class == klass && + klass === object + end + + def convert_object(object) + object.map(&YamlVariables.method(:convert_key_value_to_string)) + end def convert_key_value_to_string(variable) variable[:key] = variable[:key].to_s diff --git a/spec/lib/gitlab/serialize/yaml_variables_spec.rb b/spec/lib/gitlab/serialize/yaml_variables_spec.rb index 6d74f8c44d6..41aea95dfdb 100644 --- a/spec/lib/gitlab/serialize/yaml_variables_spec.rb +++ b/spec/lib/gitlab/serialize/yaml_variables_spec.rb @@ -16,4 +16,25 @@ describe Gitlab::Serialize::YamlVariables do { key: 'key', value: 'value', public: true }, { key: 'wee', value: '1', public: false }]) end + + context 'with a subclass of Array' do + let(:object) do + Kaminari::PaginatableArray.new << 'I am evil' + end + + it 'ignores it' do + is_expected.to eq([]) + end + end + + context 'with the array containing subclasses of Hash' do + let(:object) do + [ActiveSupport::OrderedOptions.new( + key: 'key', value: 'value', public: true)] + end + + it 'ignores it' do + is_expected.to eq([]) + end + end end From bcc09ca76098e8e12b0a42454920e1d4df6434c2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 15 Dec 2016 23:37:22 +0800 Subject: [PATCH 03/10] Just use YAML.safe_load and assume the format should be correct since it's already passing the validation anyway. Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20076187 --- lib/gitlab/serialize/yaml_variables.rb | 26 +++---------------- .../gitlab/serialize/yaml_variables_spec.rb | 21 --------------- 2 files changed, 3 insertions(+), 44 deletions(-) diff --git a/lib/gitlab/serialize/yaml_variables.rb b/lib/gitlab/serialize/yaml_variables.rb index 68ca50ed60e..db1e7641c74 100644 --- a/lib/gitlab/serialize/yaml_variables.rb +++ b/lib/gitlab/serialize/yaml_variables.rb @@ -10,36 +10,16 @@ module Gitlab def load(string) return unless string - object = YAML.load(string) + object = YAML.safe_load(string, [Symbol]) - # We don't need to verify the object once we're using SafeYAML - if YamlVariables.verify_object(object) - YamlVariables.convert_object(object) - else - [] - end + object.map(&YamlVariables.method(:convert_key_value_to_string)) end def dump(object) YAML.dump(object) end - def verify_object(object) - YamlVariables.verify_type(object, Array) && - object.all? { |obj| YamlVariables.verify_type(obj, Hash) } - end - - # We use three ways to check if the class is exactly the one we want, - # rather than some subclass or duck typing class. - def verify_type(object, klass) - object.kind_of?(klass) && - object.class == klass && - klass === object - end - - def convert_object(object) - object.map(&YamlVariables.method(:convert_key_value_to_string)) - end + private def convert_key_value_to_string(variable) variable[:key] = variable[:key].to_s diff --git a/spec/lib/gitlab/serialize/yaml_variables_spec.rb b/spec/lib/gitlab/serialize/yaml_variables_spec.rb index 41aea95dfdb..6d74f8c44d6 100644 --- a/spec/lib/gitlab/serialize/yaml_variables_spec.rb +++ b/spec/lib/gitlab/serialize/yaml_variables_spec.rb @@ -16,25 +16,4 @@ describe Gitlab::Serialize::YamlVariables do { key: 'key', value: 'value', public: true }, { key: 'wee', value: '1', public: false }]) end - - context 'with a subclass of Array' do - let(:object) do - Kaminari::PaginatableArray.new << 'I am evil' - end - - it 'ignores it' do - is_expected.to eq([]) - end - end - - context 'with the array containing subclasses of Hash' do - let(:object) do - [ActiveSupport::OrderedOptions.new( - key: 'key', value: 'value', public: true)] - end - - it 'ignores it' do - is_expected.to eq([]) - end - end end From cc1eb7fec5a225342992377495e4969fdf8adf45 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 16 Dec 2016 22:49:33 +0800 Subject: [PATCH 04/10] Use described_class Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20133505 --- spec/lib/gitlab/serialize/yaml_variables_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/lib/gitlab/serialize/yaml_variables_spec.rb b/spec/lib/gitlab/serialize/yaml_variables_spec.rb index 6d74f8c44d6..2691c3dc22f 100644 --- a/spec/lib/gitlab/serialize/yaml_variables_spec.rb +++ b/spec/lib/gitlab/serialize/yaml_variables_spec.rb @@ -2,8 +2,7 @@ require 'spec_helper' describe Gitlab::Serialize::YamlVariables do subject do - Gitlab::Serialize::YamlVariables.load( - Gitlab::Serialize::YamlVariables.dump(object)) + described_class.load(described_class.dump(object)) end let(:object) do From 1b4a244dbc8d1e5b79feb4f111ec6183afa1b413 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 16 Dec 2016 23:04:01 +0800 Subject: [PATCH 05/10] Rename to Gitlab::Serialize::Ci::Variables Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20133176 --- app/models/ci/build.rb | 2 +- lib/gitlab/serialize/ci/variables.rb | 32 +++++++++++++++++++ lib/gitlab/serialize/yaml_variables.rb | 31 ------------------ .../variables_spec.rb} | 2 +- 4 files changed, 34 insertions(+), 33 deletions(-) create mode 100644 lib/gitlab/serialize/ci/variables.rb delete mode 100644 lib/gitlab/serialize/yaml_variables.rb rename spec/lib/gitlab/serialize/{yaml_variables_spec.rb => ci/variables_spec.rb} (89%) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 99f3f4711aa..0e30755b870 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -10,7 +10,7 @@ module Ci has_many :deployments, as: :deployable serialize :options - serialize :yaml_variables, Gitlab::Serialize::YamlVariables + serialize :yaml_variables, Gitlab::Serialize::Ci::Variables validates :coverage, numericality: true, allow_blank: true validates_presence_of :ref diff --git a/lib/gitlab/serialize/ci/variables.rb b/lib/gitlab/serialize/ci/variables.rb new file mode 100644 index 00000000000..0ca060cd95e --- /dev/null +++ b/lib/gitlab/serialize/ci/variables.rb @@ -0,0 +1,32 @@ +module Gitlab + module Serialize + module Ci + # This serializer could make sure our YAML variables' keys and values + # are always strings. This is more for legacy build data because + # from now on we convert them into strings before saving to database. + module Variables + extend self + + def load(string) + return unless string + + object = YAML.safe_load(string, [Symbol]) + + object.map(&Variables.method(:convert_key_value_to_string)) + end + + def dump(object) + YAML.dump(object) + end + + private + + def convert_key_value_to_string(variable) + variable[:key] = variable[:key].to_s + variable[:value] = variable[:value].to_s + variable + end + end + end + end +end diff --git a/lib/gitlab/serialize/yaml_variables.rb b/lib/gitlab/serialize/yaml_variables.rb deleted file mode 100644 index db1e7641c74..00000000000 --- a/lib/gitlab/serialize/yaml_variables.rb +++ /dev/null @@ -1,31 +0,0 @@ - -module Gitlab - module Serialize - # This serializer could make sure our YAML variables' keys and values - # are always strings. This is more for legacy build data because - # from now on we convert them into strings before saving to database. - module YamlVariables - extend self - - def load(string) - return unless string - - object = YAML.safe_load(string, [Symbol]) - - object.map(&YamlVariables.method(:convert_key_value_to_string)) - end - - def dump(object) - YAML.dump(object) - end - - private - - def convert_key_value_to_string(variable) - variable[:key] = variable[:key].to_s - variable[:value] = variable[:value].to_s - variable - end - end - end -end diff --git a/spec/lib/gitlab/serialize/yaml_variables_spec.rb b/spec/lib/gitlab/serialize/ci/variables_spec.rb similarity index 89% rename from spec/lib/gitlab/serialize/yaml_variables_spec.rb rename to spec/lib/gitlab/serialize/ci/variables_spec.rb index 2691c3dc22f..797baef640c 100644 --- a/spec/lib/gitlab/serialize/yaml_variables_spec.rb +++ b/spec/lib/gitlab/serialize/ci/variables_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Serialize::YamlVariables do +describe Gitlab::Serialize::Ci::Variables do subject do described_class.load(described_class.dump(object)) end From 8607383e1dcdde62a2143890f02a72141dd44af1 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 19 Dec 2016 18:11:18 +0800 Subject: [PATCH 06/10] Just implement it in the block Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20223109 --- lib/gitlab/serialize/ci/variables.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/gitlab/serialize/ci/variables.rb b/lib/gitlab/serialize/ci/variables.rb index 0ca060cd95e..8919f0ccd00 100644 --- a/lib/gitlab/serialize/ci/variables.rb +++ b/lib/gitlab/serialize/ci/variables.rb @@ -12,20 +12,16 @@ module Gitlab object = YAML.safe_load(string, [Symbol]) - object.map(&Variables.method(:convert_key_value_to_string)) + object.map do |variable| + variable[:key] = variable[:key].to_s + variable[:value] = variable[:value].to_s + variable + end end def dump(object) YAML.dump(object) end - - private - - def convert_key_value_to_string(variable) - variable[:key] = variable[:key].to_s - variable[:value] = variable[:value].to_s - variable - end end end end From f2afdc92b8029f628f6f8ed2d5b9dc1323fd3295 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 19 Dec 2016 21:09:40 +0800 Subject: [PATCH 07/10] Test if expanded_environment_name could expand var with symbols. Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20234245 --- spec/models/build_spec.rb | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 5a47e7ddf0d..e97f6ae3cea 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1306,11 +1306,25 @@ describe Ci::Build, models: true do describe '#expanded_environment_name' do subject { build.expanded_environment_name } - context 'when environment uses variables' do - let(:build) { create(:ci_build, ref: 'master', environment: 'review/$CI_BUILD_REF_NAME') } + context 'when environment uses $CI_BUILD_REF_NAME' do + let(:build) do + create(:ci_build, + ref: 'master', + environment: 'review/$CI_BUILD_REF_NAME') + end it { is_expected.to eq('review/master') } end + + context 'when environment uses yaml_variables containing symbol keys' do + let(:build) do + create(:ci_build, + yaml_variables: [{key: :APP_HOST, value: 'host'}], + environment: 'review/$APP_HOST') + end + + it { is_expected.to eq('review/host') } + end end describe '#detailed_status' do From 3ea8d983adc467c64c91b2cad91486555678c958 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 19 Dec 2016 21:15:47 +0800 Subject: [PATCH 08/10] Keep the value type for YAML variables Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20235080 --- lib/ci/gitlab_ci_yaml_processor.rb | 2 +- lib/gitlab/serialize/ci/variables.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 8806a506ffa..7463bd719d5 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -118,7 +118,7 @@ module Ci .merge(job_variables(name)) variables.map do |key, value| - { key: key.to_s, value: value.to_s, public: true } + { key: key.to_s, value: value, public: true } end end diff --git a/lib/gitlab/serialize/ci/variables.rb b/lib/gitlab/serialize/ci/variables.rb index 8919f0ccd00..3a9443bfcd9 100644 --- a/lib/gitlab/serialize/ci/variables.rb +++ b/lib/gitlab/serialize/ci/variables.rb @@ -14,7 +14,6 @@ module Gitlab object.map do |variable| variable[:key] = variable[:key].to_s - variable[:value] = variable[:value].to_s variable end end From 8e2ea26cc1fff5787c0cf0c4b0160e1815ee1344 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 19 Dec 2016 22:22:57 +0800 Subject: [PATCH 09/10] Spaces for literal hash --- spec/models/build_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index e97f6ae3cea..cd3b6d51545 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1319,7 +1319,7 @@ describe Ci::Build, models: true do context 'when environment uses yaml_variables containing symbol keys' do let(:build) do create(:ci_build, - yaml_variables: [{key: :APP_HOST, value: 'host'}], + yaml_variables: [{ key: :APP_HOST, value: 'host' }], environment: 'review/$APP_HOST') end From 4d8a2bc987dbdd417f7ae971c884b084a8982f0a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 19 Dec 2016 23:32:09 +0800 Subject: [PATCH 10/10] Fix tests because now we don't convert values --- spec/lib/gitlab/serialize/ci/variables_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/serialize/ci/variables_spec.rb b/spec/lib/gitlab/serialize/ci/variables_spec.rb index 797baef640c..7ea74da5252 100644 --- a/spec/lib/gitlab/serialize/ci/variables_spec.rb +++ b/spec/lib/gitlab/serialize/ci/variables_spec.rb @@ -10,9 +10,9 @@ describe Gitlab::Serialize::Ci::Variables do { key: 'wee', value: 1, public: false }] end - it 'converts key and values into strings' do + it 'converts keys into strings' do is_expected.to eq([ { key: 'key', value: 'value', public: true }, - { key: 'wee', value: '1', public: false }]) + { key: 'wee', value: 1, public: false }]) end end