From 10402cfc256e95cbc7334fdb0f889d6feb0c6595 Mon Sep 17 00:00:00 2001
From: Vladimir Shushlin <v.shushlin@gmail.com>
Date: Mon, 16 Aug 2021 15:46:48 +0300
Subject: [PATCH] Fix creating environment after renaming

When an environment is renamed, the slug stays the same.
This prevents a new environment with the old name from being creted
because the slug is deterministically generated from the name
and should be unique.

This commit adds a random suffix to all slugs fixing this issue.

Changelog: fixed
---
 lib/gitlab/slug/environment.rb           | 31 +++++++++----------
 spec/lib/gitlab/slug/environment_spec.rb | 39 +++++++++++-------------
 2 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/lib/gitlab/slug/environment.rb b/lib/gitlab/slug/environment.rb
index fd70def8e7cf74..36549338720da9 100644
--- a/lib/gitlab/slug/environment.rb
+++ b/lib/gitlab/slug/environment.rb
@@ -12,6 +12,10 @@ module Slug
     class Environment
       attr_reader :name
 
+      LETTERS = ('a'..'z').freeze
+      NUMBERS = ('0'..'9').freeze
+      SUFFIX_CHARS = LETTERS.to_a + NUMBERS.to_a
+
       def initialize(name)
         @name = name
       end
@@ -26,29 +30,22 @@ def generate
         # Repeated dashes are invalid (OpenShift limitation)
         slugified.squeeze!('-')
 
-        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 = slugified[0..16]
+        slugified << '-' unless slugified.end_with?('-')
+        slugified << random_suffix
+
+        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,
+      # based on name (which must be unique).
+      # Also when environment is renamed slug isn't being changed, which can lead to collisions.
+      # To compensate, we add a random 6-byte suffix. 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)
+      def random_suffix
+        (0..5).map { SUFFIX_CHARS.sample }.join
       end
     end
   end
diff --git a/spec/lib/gitlab/slug/environment_spec.rb b/spec/lib/gitlab/slug/environment_spec.rb
index f516322b937ab1..3bfa86bf84eb70 100644
--- a/spec/lib/gitlab/slug/environment_spec.rb
+++ b/spec/lib/gitlab/slug/environment_spec.rb
@@ -5,33 +5,28 @@
 RSpec.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",
+      "staging-12345678901234567" => "staging-123456789",
+      "9-staging-123456789012345" => "env-9-staging-123",
+      "staging-1234567890123456"  => "staging-123456789",
+      "staging-1234567890123456-" => "staging-123456789",
       "production"                => "production",
-      "PRODUCTION"                => "production-q517sa",
-      "review/1-foo"              => "review-1-foo-q517sa",
-      "1-foo"                     => "env-1-foo-q517sa",
-      "1/foo"                     => "env-1-foo-q517sa",
+      "PRODUCTION"                => "production",
+      "review/1-foo"              => "review-1-foo",
+      "1-foo"                     => "env-1-foo",
+      "1/foo"                     => "env-1-foo",
       "foo-"                      => "foo",
-      "foo--bar"                  => "foo-bar-q517sa",
-      "foo**bar"                  => "foo-bar-q517sa",
-      "*-foo"                     => "env-foo-q517sa",
+      "foo--bar"                  => "foo-bar",
+      "foo**bar"                  => "foo-bar",
+      "*-foo"                     => "env-foo",
       "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
+      "staging-12345678-01234567" => "staging-12345678",
+      ""                          => "env",
+      nil                         => "env"
+    }.each do |name, expected|
+      it "returns a slug matching #{expected}-[a-z0-9]{6}, given #{name}" do
         slug = described_class.new(name).generate
 
-        expect(slug).to match(/\A#{matcher}\z/)
+        expect(slug).to match(/\A#{expected}-[a-z0-9]{6}\z/)
       end
     end
   end
-- 
GitLab