diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2019-07-19 08:37:57 +0000 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2019-07-19 08:37:57 +0000 |
commit | f3ce7a37a4aa00c0af614757087b6536c9ef701f (patch) | |
tree | 042c825a9104fc38cf5870ebd28987ff0fefdb4f | |
parent | 77d9bfac2a400e74dd26fb26d83b27c0833821f2 (diff) | |
parent | 41fc4d1e449dedbc417b3ee57ca9e1e8e77bd18c (diff) | |
download | gitlab-ce-f3ce7a37a4aa00c0af614757087b6536c9ef701f.tar.gz |
Merge branch '64295-predictable-environment-slugs' into 'master'
Use predictable environment slugs
See merge request gitlab-org/gitlab-ce!30551
-rw-r--r-- | app/models/environment.rb | 47 | ||||
-rw-r--r-- | changelogs/unreleased/64295-predictable-environment-slugs.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/slug/environment.rb | 58 | ||||
-rw-r--r-- | spec/lib/gitlab/slug/environment_spec.rb | 38 | ||||
-rw-r--r-- | spec/models/environment_spec.rb | 26 |
5 files changed, 103 insertions, 71 deletions
diff --git a/app/models/environment.rb b/app/models/environment.rb index b8ee54c1696..392481ea0cc 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -4,11 +4,6 @@ class Environment < ApplicationRecord include Gitlab::Utils::StrongMemoize include ReactiveCaching - # Used to generate random suffixes for the slug - LETTERS = ('a'..'z').freeze - NUMBERS = ('0'..'9').freeze - SUFFIX_CHARS = LETTERS.to_a + NUMBERS.to_a - belongs_to :project, required: true has_many :deployments, -> { success }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -203,40 +198,6 @@ class Environment < ApplicationRecord super.presence || generate_slug end - # An environment name is not necessarily suitable for use in URLs, DNS - # or other third-party contexts, so provide a slugified version. A slug has - # the following properties: - # * contains only lowercase letters (a-z), numbers (0-9), and '-' - # * begins with a letter - # * has a maximum length of 24 bytes (OpenShift limitation) - # * cannot end with `-` - def generate_slug - # Lowercase letters and numbers only - slugified = +name.to_s.downcase.gsub(/[^a-z0-9]/, '-') - - # Must start with a letter - slugified = 'env-' + slugified unless LETTERS.cover?(slugified[0]) - - # Repeated dashes are invalid (OpenShift limitation) - slugified.gsub!(/\-+/, '-') - - # Maximum length: 24 characters (OpenShift limitation) - slugified = slugified[0..23] - - # Cannot end with a dash (Kubernetes label limitation) - slugified.chop! if slugified.end_with?('-') - - # Add a random suffix, shortening the current string if necessary, if it - # has been slugified. This ensures uniqueness. - if slugified != name - slugified = slugified[0..16] - slugified << '-' unless slugified.end_with?('-') - slugified << random_suffix - end - - self.slug = slugified - end - def external_url_for(path, commit_sha) return unless self.external_url @@ -274,11 +235,7 @@ class Environment < ApplicationRecord private - # Slugifying a name may remove the uniqueness guarantee afforded by it being - # based on name (which must be unique). To compensate, we add a random - # 6-byte suffix in those circumstances. This is not *guaranteed* uniqueness, - # but the chance of collisions is vanishingly small - def random_suffix - (0..5).map { SUFFIX_CHARS.sample }.join + def generate_slug + self.slug = Gitlab::Slug::Environment.new(name).generate end end diff --git a/changelogs/unreleased/64295-predictable-environment-slugs.yml b/changelogs/unreleased/64295-predictable-environment-slugs.yml new file mode 100644 index 00000000000..de581b2b8e1 --- /dev/null +++ b/changelogs/unreleased/64295-predictable-environment-slugs.yml @@ -0,0 +1,5 @@ +--- +title: Use predictable environment slugs +merge_request: 30551 +author: +type: added diff --git a/lib/gitlab/slug/environment.rb b/lib/gitlab/slug/environment.rb new file mode 100644 index 00000000000..1b87d3bb626 --- /dev/null +++ b/lib/gitlab/slug/environment.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +# An environment name is not necessarily suitable for use in URLs, DNS +# or other third-party contexts, so provide a slugified version. A slug has +# the following properties: +# * contains only lowercase letters (a-z), numbers (0-9), and '-' +# * begins with a letter +# * has a maximum length of 24 bytes (OpenShift limitation) +# * cannot end with `-` +module Gitlab + module Slug + class Environment + attr_reader :name + + def initialize(name) + @name = name + end + + def generate + # Lowercase letters and numbers only + slugified = name.to_s.downcase.gsub(/[^a-z0-9]/, '-') + + # Must start with a letter + slugified = 'env-' + slugified unless slugified.match?(/^[a-z]/) + + # Repeated dashes are invalid (OpenShift limitation) + slugified.squeeze!('-') + + slugified = + if slugified.size > 24 || slugified != name + # Maximum length: 24 characters (OpenShift limitation) + shorten_and_add_suffix(slugified) + else + # Cannot end with a dash (Kubernetes label limitation) + slugified.chomp('-') + end + + slugified + end + + private + + def shorten_and_add_suffix(slug) + slug = slug[0..16] + slug << '-' unless slug.ends_with?('-') + slug << suffix + end + + # Slugifying a name may remove the uniqueness guarantee afforded by it being + # based on name (which must be unique). To compensate, we add a predictable + # 6-byte suffix in those circumstances. This is not *guaranteed* uniqueness, + # but the chance of collisions is vanishingly small + def suffix + Digest::SHA2.hexdigest(name.to_s).to_i(16).to_s(36).last(6) + end + end + end +end diff --git a/spec/lib/gitlab/slug/environment_spec.rb b/spec/lib/gitlab/slug/environment_spec.rb new file mode 100644 index 00000000000..7dc583a94b8 --- /dev/null +++ b/spec/lib/gitlab/slug/environment_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Slug::Environment do + describe '#generate' do + { + "staging-12345678901234567" => "staging-123456789-q517sa", + "9-staging-123456789012345" => "env-9-staging-123-q517sa", + "staging-1234567890123456" => "staging-1234567890123456", + "staging-1234567890123456-" => "staging-123456789-q517sa", + "production" => "production", + "PRODUCTION" => "production-q517sa", + "review/1-foo" => "review-1-foo-q517sa", + "1-foo" => "env-1-foo-q517sa", + "1/foo" => "env-1-foo-q517sa", + "foo-" => "foo", + "foo--bar" => "foo-bar-q517sa", + "foo**bar" => "foo-bar-q517sa", + "*-foo" => "env-foo-q517sa", + "staging-12345678-" => "staging-12345678", + "staging-12345678-01234567" => "staging-12345678-q517sa", + "" => "env-q517sa", + nil => "env-q517sa" + }.each do |name, matcher| + before do + # ('a' * 64).to_i(16).to_s(36).last(6) gives 'q517sa' + allow(Digest::SHA2).to receive(:hexdigest).with(name).and_return('a' * 64) + end + + it "returns a slug matching #{matcher}, given #{name}" do + slug = described_class.new(name).generate + + expect(slug).to match(/\A#{matcher}\z/) + end + end + end +end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index fe4d64818b4..ce0681c4331 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -762,32 +762,6 @@ describe Environment, :use_clean_rails_memory_store_caching do end end - describe '#generate_slug' do - SUFFIX = "-[a-z0-9]{6}".freeze - { - "staging-12345678901234567" => "staging-123456789" + SUFFIX, - "9-staging-123456789012345" => "env-9-staging-123" + SUFFIX, - "staging-1234567890123456" => "staging-1234567890123456", - "production" => "production", - "PRODUCTION" => "production" + SUFFIX, - "review/1-foo" => "review-1-foo" + SUFFIX, - "1-foo" => "env-1-foo" + SUFFIX, - "1/foo" => "env-1-foo" + SUFFIX, - "foo-" => "foo" + SUFFIX, - "foo--bar" => "foo-bar" + SUFFIX, - "foo**bar" => "foo-bar" + SUFFIX, - "*-foo" => "env-foo" + SUFFIX, - "staging-12345678-" => "staging-12345678" + SUFFIX, - "staging-12345678-01234567" => "staging-12345678" + SUFFIX - }.each do |name, matcher| - it "returns a slug matching #{matcher}, given #{name}" do - slug = described_class.new(name: name).generate_slug - - expect(slug).to match(/\A#{matcher}\z/) - end - end - end - describe '#ref_path' do subject(:environment) do create(:environment, name: 'staging / review-1') |