Commit 65745f52 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'add-ci_variables-environment_scope' into 'master'

Add environment_scope column to ci_variables table

See merge request !12363
parents fd72b4ed 1cecc285
...@@ -5,7 +5,7 @@ module Ci ...@@ -5,7 +5,7 @@ module Ci
belongs_to :project belongs_to :project
validates :key, uniqueness: { scope: :project_id } validates :key, uniqueness: { scope: [:project_id, :environment_scope] }
scope :unprotected, -> { where(protected: false) } scope :unprotected, -> { where(protected: false) }
end end
......
---
title: Rename duplicated variables with the same key for projects. Add environment_scope
column to variables and add unique constraint to make sure that no variables could
be created with the same key within a project
merge_request: 12363
author:
class RenameDuplicatedVariableKey < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
execute(<<~SQL)
UPDATE ci_variables
SET #{key} = CONCAT(#{key}, #{underscore}, id)
WHERE id IN (
SELECT *
FROM ( -- MySQL requires an extra layer
SELECT dup.id
FROM ci_variables dup
INNER JOIN (SELECT max(id) AS id, #{key}, project_id
FROM ci_variables tmp
GROUP BY #{key}, project_id) var
USING (#{key}, project_id) where dup.id <> var.id
) dummy
)
SQL
end
def down
# noop
end
def key
# key needs to be quoted in MySQL
quote_column_name('key')
end
def underscore
quote('_')
end
end
class AddEnvironmentScopeToCiVariables < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:ci_variables, :environment_scope, :string, default: '*')
end
def down
remove_column(:ci_variables, :environment_scope)
end
end
class AddUniqueConstraintToCiVariables < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
unless this_index_exists?
add_concurrent_index(:ci_variables, columns, name: index_name, unique: true)
end
end
def down
if this_index_exists?
if Gitlab::Database.mysql? && !index_exists?(:ci_variables, :project_id)
# Need to add this index for MySQL project_id foreign key constraint
add_concurrent_index(:ci_variables, :project_id)
end
remove_concurrent_index(:ci_variables, columns, name: index_name)
end
end
private
def this_index_exists?
index_exists?(:ci_variables, columns, name: index_name)
end
def columns
@columns ||= [:project_id, :key, :environment_scope]
end
def index_name
'index_ci_variables_on_project_id_and_key_and_environment_scope'
end
end
class RemoveCiVariablesProjectIdIndex < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
if index_exists?(:ci_variables, :project_id)
remove_concurrent_index(:ci_variables, :project_id)
end
end
def down
unless index_exists?(:ci_variables, :project_id)
add_concurrent_index(:ci_variables, :project_id)
end
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20170622162730) do ActiveRecord::Schema.define(version: 20170623080805) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -374,9 +374,10 @@ ActiveRecord::Schema.define(version: 20170622162730) do ...@@ -374,9 +374,10 @@ ActiveRecord::Schema.define(version: 20170622162730) do
t.string "encrypted_value_iv" t.string "encrypted_value_iv"
t.integer "project_id", null: false t.integer "project_id", null: false
t.boolean "protected", default: false, null: false t.boolean "protected", default: false, null: false
t.string "environment_scope", default: "*", null: false
end end
add_index "ci_variables", ["project_id"], name: "index_ci_variables_on_project_id", using: :btree add_index "ci_variables", ["project_id", "key", "environment_scope"], name: "index_ci_variables_on_project_id_and_key_and_environment_scope", unique: true, using: :btree
create_table "container_repositories", force: :cascade do |t| create_table "container_repositories", force: :cascade do |t|
t.integer "project_id", null: false t.integer "project_id", null: false
......
require 'spec_helper'
require Rails.root.join('db', 'migrate', '20170622135451_rename_duplicated_variable_key.rb')
describe RenameDuplicatedVariableKey, :migration do
let(:variables) { table(:ci_variables) }
let(:projects) { table(:projects) }
before do
projects.create!(id: 1)
variables.create!(id: 1, key: 'key1', project_id: 1)
variables.create!(id: 2, key: 'key2', project_id: 1)
variables.create!(id: 3, key: 'keyX', project_id: 1)
variables.create!(id: 4, key: 'keyX', project_id: 1)
variables.create!(id: 5, key: 'keyY', project_id: 1)
variables.create!(id: 6, key: 'keyX', project_id: 1)
variables.create!(id: 7, key: 'key7', project_id: 1)
variables.create!(id: 8, key: 'keyY', project_id: 1)
end
it 'correctly remove duplicated records with smaller id' do
migrate!
expect(variables.pluck(:id, :key)).to contain_exactly(
[1, 'key1'],
[2, 'key2'],
[3, 'keyX_3'],
[4, 'keyX_4'],
[5, 'keyY_5'],
[6, 'keyX'],
[7, 'key7'],
[8, 'keyY']
)
end
end
...@@ -3,8 +3,16 @@ require 'spec_helper' ...@@ -3,8 +3,16 @@ require 'spec_helper'
describe Ci::Variable, models: true do describe Ci::Variable, models: true do
subject { build(:ci_variable) } subject { build(:ci_variable) }
it { is_expected.to include_module(HasVariable) } let(:secret_value) { 'secret' }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id) }
describe 'validations' do
it { is_expected.to include_module(HasVariable) }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) }
it { is_expected.to validate_length_of(:key).is_at_most(255) }
it { is_expected.to allow_value('foo').for(:key) }
it { is_expected.not_to allow_value('foo bar').for(:key) }
it { is_expected.not_to allow_value('foo/bar').for(:key) }
end
describe '.unprotected' do describe '.unprotected' do
subject { described_class.unprotected } subject { described_class.unprotected }
......
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