Merge branch '26852-fix-slug-for-openshift' into 'master'

Avoid repeated dashes in $CI_ENVIRONMENT_SLUG

Closes #26852

See merge request !8638

app/models/environment.rb

 class Environment < ActiveRecord::Base
   # Used to generate random suffixes for the slug
+  LETTERS = 'a'..'z'
   NUMBERS = '0'..'9'
-  SUFFIX_CHARS = ('a'..'z').to_a + NUMBERS.to_a
+  SUFFIX_CHARS = LETTERS.to_a + NUMBERS.to_a
 
   belongs_to :project, required: true, validate: true
 
@@ -148,17 +149,24 @@ class Environment < ActiveRecord::Base
     slugified = name.to_s.downcase.gsub(/[^a-z0-9]/, '-')
 
     # Must start with a letter
-    slugified = "env-" + slugified if NUMBERS.cover?(slugified[0])
+    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 "-" character (Kubernetes label limitation)
-    slugified = slugified[0..-2] if slugified[-1] == "-"
+    # 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.
-    slugified = slugified[0..16] + "-" + random_suffix if slugified != name
+    if slugified != name
+      slugified = slugified[0..16]
+      slugified << '-' unless slugified.end_with?('-')
+      slugified << random_suffix
+    end
 
     self.slug = slugified
   end

changelogs/unreleased/26852-fix-slug-for-openshift.yml

+---
+title: Avoid repeated dashes in $CI_ENVIRONMENT_SLUG
+merge_request: 8638
+author:

db/migrate/20161207231626_add_environment_slug.rb

@@ -8,8 +8,9 @@ class AddEnvironmentSlug < ActiveRecord::Migration
   DOWNTIME_REASON = 'Adding NOT NULL column environments.slug with dependent data'
 
   # Used to generate random suffixes for the slug
+  LETTERS = 'a'..'z'
   NUMBERS = '0'..'9'
-  SUFFIX_CHARS = ('a'..'z').to_a + NUMBERS.to_a
+  SUFFIX_CHARS = LETTERS.to_a + NUMBERS.to_a
 
   def up
     environments = Arel::Table.new(:environments)
@@ -39,17 +40,24 @@ class AddEnvironmentSlug < ActiveRecord::Migration
     slugified = name.to_s.downcase.gsub(/[^a-z0-9]/, '-')
 
     # Must start with a letter
-    slugified = "env-" + slugified if NUMBERS.cover?(slugified[0])
+    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 "-" character (Kubernetes label limitation)
-    slugified = slugified[0..-2] if slugified[-1] == "-"
+    # 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.
-    slugified = slugified[0..16] + "-" + random_suffix if slugified != name
+    if slugified != name
+      slugified = slugified[0..16]
+      slugified << '-' unless slugified.end_with?('-')
+      slugified << random_suffix
+    end
 
     slugified
   end

spec/models/environment_spec.rb

@@ -288,6 +288,11 @@ describe Environment, models: true do
       "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