Read about our upcoming Code of Conduct on this issue

Commit 93a03cd9 authored by Nick Thomas's avatar Nick Thomas
Browse files

Add an environment slug

parent 83edda002779
class Environment < ActiveRecord::Base
# Used to generate random suffixes for the slug
NUMBERS = '0'..'9'
SUFFIX_CHARS = ('a'..'z').to_a + NUMBERS.to_a
belongs_to :project, required: true, validate: true
has_many :deployments
before_validation :nullify_external_url
before_validation :generate_slug, if: ->(env) { env.slug.blank? }
before_save :set_environment_type
validates :name,
......@@ -13,6 +19,13 @@ class Environment < ActiveRecord::Base
format: { with: Gitlab::Regex.environment_name_regex,
message: Gitlab::Regex.environment_name_regex_message }
validates :slug,
presence: true,
uniqueness: { scope: :project_id },
length: { maximum: 24 },
format: { with: Gitlab::Regex.environment_slug_regex,
message: Gitlab::Regex.environment_slug_regex_message }
validates :external_url,
uniqueness: { scope: :project_id },
length: { maximum: 255 },
......@@ -107,4 +120,41 @@ def actions_for(environment)
action.expanded_environment_name == environment
end
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 if NUMBERS.cover?(slugified[0])
# 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] == "-"
# 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
self.slug = slugified
end
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
end
end
---
title: Add a slug to environments
merge_request: 7983
author:
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddEnvironmentSlug < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = true
DOWNTIME_REASON = 'Adding NOT NULL column environments.slug with dependent data'
# Used to generate random suffixes for the slug
NUMBERS = '0'..'9'
SUFFIX_CHARS = ('a'..'z').to_a + NUMBERS.to_a
def up
environments = Arel::Table.new(:environments)
add_column :environments, :slug, :string
finder = environments.project(:id, :name)
connection.exec_query(finder.to_sql).rows.each do |id, name|
updater = Arel::UpdateManager.new(ActiveRecord::Base).
table(environments).
set(environments[:slug] => generate_slug(name)).
where(environments[:id].eq(id))
connection.exec_update(updater.to_sql, self.class.name, [])
end
change_column_null :environments, :slug, false
end
def down
remove_column :environments, :slug
end
# Copy of the Environment#generate_slug implementation
def generate_slug(name)
# Lowercase letters and numbers only
slugified = name.to_s.downcase.gsub(/[^a-z0-9]/, '-')
# Must start with a letter
slugified = "env-" + slugified if NUMBERS.cover?(slugified[0])
# 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] == "-"
# 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
slugified
end
def random_suffix
(0..5).map { SUFFIX_CHARS.sample }.join
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddUniqueIndexForEnvironmentSlug < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = true
DOWNTIME_REASON = 'Adding a *unique* index to environments.slug'
disable_ddl_transaction!
def change
add_concurrent_index :environments, [:project_id, :slug], unique: true
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20161207231621) do
ActiveRecord::Schema.define(version: 20161212142807) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -98,14 +98,14 @@
t.text "help_page_text_html"
t.text "shared_runners_text_html"
t.text "after_sign_up_text_html"
t.boolean "sidekiq_throttling_enabled", default: false
t.string "sidekiq_throttling_queues"
t.decimal "sidekiq_throttling_factor"
t.boolean "housekeeping_enabled", default: true, null: false
t.boolean "housekeeping_bitmaps_enabled", default: true, null: false
t.integer "housekeeping_incremental_repack_period", default: 10, null: false
t.integer "housekeeping_full_repack_period", default: 50, null: false
t.integer "housekeeping_gc_period", default: 200, null: false
t.boolean "sidekiq_throttling_enabled", default: false
t.string "sidekiq_throttling_queues"
t.decimal "sidekiq_throttling_factor"
t.boolean "html_emails_enabled", default: true
end
......@@ -428,9 +428,11 @@
t.string "external_url"
t.string "environment_type"
t.string "state", default: "available", null: false
t.string "slug", null: false
end
add_index "environments", ["project_id", "name"], name: "index_environments_on_project_id_and_name", unique: true, using: :btree
add_index "environments", ["project_id", "slug"], name: "index_environments_on_project_id_and_slug", unique: true, using: :btree
create_table "events", force: :cascade do |t|
t.string "target_type"
......@@ -737,8 +739,8 @@
t.integer "visibility_level", default: 20, null: false
t.boolean "request_access_enabled", default: false, null: false
t.datetime "deleted_at"
t.text "description_html"
t.boolean "lfs_enabled"
t.text "description_html"
t.integer "parent_id"
end
......@@ -1219,8 +1221,8 @@
t.datetime "otp_grace_period_started_at"
t.boolean "ldap_email", default: false, null: false
t.boolean "external", default: false
t.string "incoming_email_token"
t.string "organization"
t.string "incoming_email_token"
t.boolean "authorized_projects_populated"
end
......
......@@ -22,8 +22,9 @@ Example response:
[
{
"id": 1,
"name": "Env1",
"external_url": "https://env1.example.gitlab.com"
"name": "review/fix-foo",
"slug": "review-fix-foo-dfjre3",
"external_url": "https://review-fix-foo-dfjre3.example.gitlab.com"
}
]
```
......@@ -54,6 +55,7 @@ Example response:
{
"id": 1,
"name": "deploy",
"slug": "deploy",
"external_url": "https://deploy.example.gitlab.com"
}
```
......@@ -85,6 +87,7 @@ Example response:
{
"id": 1,
"name": "staging",
"slug": "staging",
"external_url": "https://staging.example.gitlab.com"
}
```
......@@ -112,6 +115,7 @@ Example response:
{
"id": 1,
"name": "deploy",
"slug": "deploy",
"external_url": "https://deploy.example.gitlab.com"
}
```
......@@ -629,7 +629,7 @@ class Pipeline < PipelineBasic
end
class EnvironmentBasic < Grape::Entity
expose :id, :name, :external_url
expose :id, :name, :slug, :external_url
end
class Environment < EnvironmentBasic
......
module API
# Environments RESTfull API endpoints
class Environments < Grape::API
include ::API::Helpers::CustomValidators
include PaginationParams
before { authenticate! }
......@@ -29,6 +30,7 @@ class Environments < Grape::API
params do
requires :name, type: String, desc: 'The name of the environment to be created'
optional :external_url, type: String, desc: 'URL on which this deployment is viewable'
optional :slug, absence: { message: "is automatically generated and cannot be changed" }
end
post ':id/environments' do
authorize! :create_environment, user_project
......@@ -50,6 +52,7 @@ class Environments < Grape::API
requires :environment_id, type: Integer, desc: 'The environment ID'
optional :name, type: String, desc: 'The new environment name'
optional :external_url, type: String, desc: 'The new URL on which this deployment is viewable'
optional :slug, absence: { message: "is automatically generated and cannot be changed" }
end
put ':id/environments/:environment_id' do
authorize! :update_environment, user_project
......
module API
module Helpers
module CustomValidators
class Absence < Grape::Validations::Base
def validate_param!(attr_name, params)
return if params.respond_to?(:key?) && !params.key?(attr_name)
raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:absence)
end
end
end
end
end
Grape::Validations.register_validator(:absence, ::API::Helpers::CustomValidators::Absence)
......@@ -131,5 +131,14 @@ def kubernetes_namespace_regex
def kubernetes_namespace_regex_message
"can contain only letters, digits or '-', and cannot start or end with '-'"
end
def environment_slug_regex
@environment_slug_regex ||= /\A[a-z]([a-z0-9-]*[a-z0-9])?\z/.freeze
end
def environment_slug_regex_message
"can contain only lowercase letters, digits, and '-'. " \
"Must start with a letter, and cannot end with '-'"
end
end
end
......@@ -29,4 +29,20 @@
describe 'file path regex' do
it { expect('foo@/bar').to match(Gitlab::Regex.file_path_regex) }
end
describe 'environment slug regex' do
def be_matched
match(Gitlab::Regex.environment_slug_regex)
end
it { expect('foo').to be_matched }
it { expect('foo-1').to be_matched }
it { expect('FOO').not_to be_matched }
it { expect('foo/1').not_to be_matched }
it { expect('foo.1').not_to be_matched }
it { expect('foo*1').not_to be_matched }
it { expect('9foo').not_to be_matched }
it { expect('foo-').not_to be_matched }
end
end
require 'spec_helper'
describe Environment, models: true do
let(:environment) { create(:environment) }
subject(:environment) { create(:environment) }
it { is_expected.to belong_to(:project) }
it { is_expected.to have_many(:deployments) }
......@@ -15,15 +15,11 @@
it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) }
it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_length_of(:external_url).is_at_most(255) }
# To circumvent a not null violation of the name column:
# https://github.com/thoughtbot/shoulda-matchers/issues/336
it 'validates uniqueness of :external_url' do
create(:environment)
it { is_expected.to validate_uniqueness_of(:slug).scoped_to(:project_id) }
it { is_expected.to validate_length_of(:slug).is_at_most(24) }
is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id)
end
it { is_expected.to validate_length_of(:external_url).is_at_most(255) }
it { is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) }
describe '#nullify_external_url' do
it 'replaces a blank url with nil' do
......@@ -199,4 +195,38 @@
expect(environment.actions_for('review/master')).to contain_exactly(review_action)
end
end
describe '#slug' do
it "is automatically generated" do
expect(environment.slug).not_to be_nil
end
it "is not regenerated if name changes" do
original_slug = environment.slug
environment.update_attributes!(name: environment.name.reverse)
expect(environment.slug).to eq(original_slug)
end
end
describe '#generate_slug' do
SUFFIX = "-[a-z0-9]{6}"
{
"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,
}.each do |name, matcher|
it "returns a slug matching #{matcher}, given #{name}" do
slug = described_class.new(name: name).generate_clean_name
expect(slug).to match(/\A#{matcher}\z/)
end
end
end
end
......@@ -46,6 +46,7 @@
expect(response).to have_http_status(201)
expect(json_response['name']).to eq('mepmep')
expect(json_response['slug']).to eq('mepmep')
expect(json_response['external']).to be nil
end
......@@ -60,6 +61,13 @@
expect(response).to have_http_status(400)
end
it 'returns a 400 if slug is specified' do
post api("/projects/#{project.id}/environments", user), name: "foo", slug: "foo"
expect(response).to have_http_status(400)
expect(json_response["error"]).to eq("slug is automatically generated and cannot be changed")
end
end
context 'a non member' do
......@@ -86,6 +94,15 @@
expect(json_response['external_url']).to eq(url)
end
it "won't allow slug to be changed" do
slug = environment.slug
api_url = api("/projects/#{project.id}/environments/#{environment.id}", user)
put api_url, slug: slug + "-foo"
expect(response).to have_http_status(400)
expect(json_response["error"]).to eq("slug is automatically generated and cannot be changed")
end
it "won't update the external_url if only the name is passed" do
url = environment.external_url
put api("/projects/#{project.id}/environments/#{environment.id}", user),
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment