summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTiger <twatson@gitlab.com>2019-07-10 13:55:32 +1000
committerTiger <twatson@gitlab.com>2019-07-19 11:33:07 +1000
commit41fc4d1e449dedbc417b3ee57ca9e1e8e77bd18c (patch)
tree48a089eec308c2e3f7fe2c87d0e7bd747565209c
parent9578b7e9926db5900b5be07f5699aac04e47c0c6 (diff)
downloadgitlab-ce-64295-predictable-environment-slugs.tar.gz
Introduce predictable environment slugs64295-predictable-environment-slugs
If an environment slug is predictable given only the environment name, we can use the environment slug earlier in the CI variable evaluation process as we don't have to wait for the environment record itself to be persisted.
-rw-r--r--app/models/environment.rb47
-rw-r--r--changelogs/unreleased/64295-predictable-environment-slugs.yml5
-rw-r--r--lib/gitlab/slug/environment.rb58
-rw-r--r--spec/lib/gitlab/slug/environment_spec.rb38
-rw-r--r--spec/models/environment_spec.rb26
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')