Commit cb8d4dae authored by Allison Browne's avatar Allison Browne Committed by Mayra Cabrera

Increase limit for ci instance level variables

Increase application and database level constraint
parent 540e5aa1
...@@ -14,12 +14,14 @@ module Ci ...@@ -14,12 +14,14 @@ module Ci
alias_attribute :secret_value, :value alias_attribute :secret_value, :value
validates :key, uniqueness: { validates :key, uniqueness: {
message: "(%{value}) has already been taken" message: -> (object, data) { _("(%{value}) has already been taken") }
} }
validates :encrypted_value, length: { validates :value, length: {
maximum: 1024, maximum: 10_000,
too_long: 'The encrypted value of the provided variable exceeds %{count} bytes. Variables over 700 characters risk exceeding the limit.' too_long: -> (object, data) do
_('The value of the provided variable exceeds the %{count} character limit')
end
} }
scope :unprotected, -> { where(protected: false) } scope :unprotected, -> { where(protected: false) }
......
---
title: Increase CI instance variable value limit
merge_request: 35063
author:
type: changed
# frozen_string_literal: true
class IncreaseSizeOnInstanceLevelVariableValues < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
existing_constraint_name = text_limit_name(:ci_instance_variables, :encrypted_value)
new_constraint_name = check_constraint_name(:ci_instance_variables, :encrypted_value, :char_length_updated)
add_text_limit(:ci_instance_variables, :encrypted_value, 13_579, constraint_name: new_constraint_name)
remove_check_constraint(:ci_instance_variables, existing_constraint_name)
end
def down
# no-op
end
end
e691c94223b44a5866f34aab96cc9a8c5857a6302115f977542bacb36e7c010e
\ No newline at end of file
...@@ -9990,7 +9990,7 @@ CREATE TABLE public.ci_instance_variables ( ...@@ -9990,7 +9990,7 @@ CREATE TABLE public.ci_instance_variables (
encrypted_value_iv text, encrypted_value_iv text,
CONSTRAINT check_07a45a5bcb CHECK ((char_length(encrypted_value_iv) <= 255)), CONSTRAINT check_07a45a5bcb CHECK ((char_length(encrypted_value_iv) <= 255)),
CONSTRAINT check_5aede12208 CHECK ((char_length(key) <= 255)), CONSTRAINT check_5aede12208 CHECK ((char_length(key) <= 255)),
CONSTRAINT check_5ebd0515a0 CHECK ((char_length(encrypted_value) <= 1024)) CONSTRAINT check_956afd70f1 CHECK ((char_length(encrypted_value) <= 13579))
); );
CREATE SEQUENCE public.ci_instance_variables_id_seq CREATE SEQUENCE public.ci_instance_variables_id_seq
......
...@@ -73,7 +73,7 @@ POST /admin/ci/variables ...@@ -73,7 +73,7 @@ POST /admin/ci/variables
| Attribute | Type | required | Description | | Attribute | Type | required | Description |
|-----------------|---------|----------|-----------------------| |-----------------|---------|----------|-----------------------|
| `key` | string | yes | The `key` of a variable. Max 255 characters, only `A-Z`, `a-z`, `0-9`, and `_` are allowed. | | `key` | string | yes | The `key` of a variable. Max 255 characters, only `A-Z`, `a-z`, `0-9`, and `_` are allowed. |
| `value` | string | yes | The `value` of a variable. Around 700 characters allowed. | | `value` | string | yes | The `value` of a variable. 10,000 characters allowed. [Since GitLab 13.3] |
| `variable_type` | string | no | The type of a variable. Available types are: `env_var` (default) and `file`. | | `variable_type` | string | no | The type of a variable. Available types are: `env_var` (default) and `file`. |
| `protected` | boolean | no | Whether the variable is protected. | | `protected` | boolean | no | Whether the variable is protected. |
| `masked` | boolean | no | Whether the variable is masked. | | `masked` | boolean | no | Whether the variable is masked. |
...@@ -103,7 +103,7 @@ PUT /admin/ci/variables/:key ...@@ -103,7 +103,7 @@ PUT /admin/ci/variables/:key
| Attribute | Type | required | Description | | Attribute | Type | required | Description |
|-----------------|---------|----------|-------------------------| |-----------------|---------|----------|-------------------------|
| `key` | string | yes | The `key` of a variable. | | `key` | string | yes | The `key` of a variable. |
| `value` | string | yes | The `value` of a variable. | | `value` | string | yes | The `value` of a variable. [Since GitLab 13.3](https://gitlab.com/gitlab-org/gitlab/-/issues/220028), around 10,000 characters allowed. Previously 700 characters. |
| `variable_type` | string | no | The type of a variable. Available types are: `env_var` (default) and `file`. | | `variable_type` | string | no | The type of a variable. Available types are: `env_var` (default) and `file`. |
| `protected` | boolean | no | Whether the variable is protected. | | `protected` | boolean | no | Whether the variable is protected. |
| `masked` | boolean | no | Whether the variable is masked. | | `masked` | boolean | no | Whether the variable is masked. |
......
...@@ -444,11 +444,11 @@ To add an instance-level variable: ...@@ -444,11 +444,11 @@ To add an instance-level variable:
1. Navigate to your admin area's **Settings > CI/CD** and expand the **Variables** section. 1. Navigate to your admin area's **Settings > CI/CD** and expand the **Variables** section.
1. Click the **Add variable** button, and fill in the details: 1. Click the **Add variable** button, and fill in the details:
- **Key**: Must be one line, using only letters, numbers, or `_` (underscore), with no spaces. - **Key**: Must be one line, using only letters, numbers, or `_` (underscore), with no spaces.
- **Value**: 700 characters allowed. - **Value**: [Since GitLab 13.3](https://gitlab.com/gitlab-org/gitlab/-/issues/220028), 10,000 characters allowed. This is also bounded by the limits of the selected Runner operating system. In GitLab 13.0 to 13.2, 700 characters allowed.
- **Type**: `File` or `Variable`. - **Type**: `File` or `Variable`.
- **Protect variable** (Optional): If selected, the variable will only be available in pipelines that run on protected branches or tags. - **Protect variable** (Optional): If selected, the variable will only be available in pipelines that run on protected branches or tags.
- **Mask variable** (Optional): If selected, the variable's **Value** will not be shown in job logs. The variable will not be saved if the value does not meet the [masking requirements](#masked-variable-requirements). - **Mask variable** (Optional): If selected, the variable's **Value** will not be shown in job logs. The variable will not be saved if the value does not meet the [masking requirements](#masked-variable-requirements).
After a variable is created, you can update any of the details by clicking the **{pencil}** **Edit** button. After a variable is created, you can update any of the details by clicking the **{pencil}** **Edit** button.
......
...@@ -785,6 +785,9 @@ msgstr[1] "" ...@@ -785,6 +785,9 @@ msgstr[1] ""
msgid "(%{mrCount} merged)" msgid "(%{mrCount} merged)"
msgstr "" msgstr ""
msgid "(%{value}) has already been taken"
msgstr ""
msgid "(No changes)" msgid "(No changes)"
msgstr "" msgstr ""
...@@ -23870,6 +23873,9 @@ msgstr "" ...@@ -23870,6 +23873,9 @@ msgstr ""
msgid "The value lying at the midpoint of a series of observed values. E.g., between 3, 5, 9, the median is 5. Between 3, 5, 7, 8, the median is (5+7)/2 = 6." msgid "The value lying at the midpoint of a series of observed values. E.g., between 3, 5, 9, the median is 5. Between 3, 5, 7, 8, the median is (5+7)/2 = 6."
msgstr "" msgstr ""
msgid "The value of the provided variable exceeds the %{count} character limit"
msgstr ""
msgid "The vulnerability is no longer detected. Verify the vulnerability has been fixed or removed before changing its status." msgid "The vulnerability is no longer detected. Verify the vulnerability has been fixed or removed before changing its status."
msgstr "" msgstr ""
......
...@@ -9,12 +9,39 @@ RSpec.describe Ci::InstanceVariable do ...@@ -9,12 +9,39 @@ RSpec.describe Ci::InstanceVariable do
it { is_expected.to include_module(Ci::Maskable) } it { is_expected.to include_module(Ci::Maskable) }
it { is_expected.to validate_uniqueness_of(:key).with_message(/\(\w+\) has already been taken/) } it { is_expected.to validate_uniqueness_of(:key).with_message(/\(\w+\) has already been taken/) }
it { is_expected.to validate_length_of(:encrypted_value).is_at_most(1024).with_message(/Variables over 700 characters risk exceeding the limit/) } it { is_expected.to validate_length_of(:value).is_at_most(10_000).with_message(/The value of the provided variable exceeds the 10000 character limit/) }
it_behaves_like 'includes Limitable concern' do it_behaves_like 'includes Limitable concern' do
subject { build(:ci_instance_variable) } subject { build(:ci_instance_variable) }
end end
describe '#value' do
context 'without application limit' do
# Ensures breakage if encryption algorithm changes
let(:variable) { build(:ci_instance_variable, key: 'too_long', value: value) }
before do
allow(variable).to receive(:valid?).and_return(true)
end
context 'when value is over the limit' do
let(:value) { SecureRandom.alphanumeric(10_002) }
it 'raises a database level error' do
expect { variable.save }.to raise_error(ActiveRecord::StatementInvalid)
end
end
context 'when value is under the limit' do
let(:value) { SecureRandom.alphanumeric(10_000) }
it 'does not raise database level error' do
expect { variable.save }.not_to raise_error
end
end
end
end
describe '.unprotected' do describe '.unprotected' do
subject { described_class.unprotected } subject { described_class.unprotected }
......
...@@ -110,20 +110,19 @@ RSpec.describe ::API::Admin::Ci::Variables do ...@@ -110,20 +110,19 @@ RSpec.describe ::API::Admin::Ci::Variables do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
it 'does not allow values above 700 characters' do it 'does not allow values above 10,000 characters' do
too_long_message = <<~MESSAGE.strip too_long_message = <<~MESSAGE.strip
The encrypted value of the provided variable exceeds 1024 bytes. \ The value of the provided variable exceeds the 10000 character limit
Variables over 700 characters risk exceeding the limit.
MESSAGE MESSAGE
expect do expect do
post api('/admin/ci/variables', admin), post api('/admin/ci/variables', admin),
params: { key: 'too_long', value: SecureRandom.hex(701) } params: { key: 'too_long', value: SecureRandom.hex(10_001) }
end.not_to change { ::Ci::InstanceVariable.count } end.not_to change { ::Ci::InstanceVariable.count }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response).to match('message' => expect(json_response).to match('message' =>
a_hash_including('encrypted_value' => [too_long_message])) a_hash_including('value' => [too_long_message]))
end end
end end
......
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