Merge branch '64295-predictable-environment-slugs' into 'master'

Use predictable environment slugs

See merge request gitlab-org/gitlab-ce!30551

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

changelogs/unreleased/64295-predictable-environment-slugs.yml

+---
+title: Use predictable environment slugs
+merge_request: 30551
+author:
+type: added

lib/gitlab/slug/environment.rb

+# 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

spec/lib/gitlab/slug/environment_spec.rb

+# 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

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')