diff --git a/db/docs/workspace_variables.yml b/db/docs/workspace_variables.yml new file mode 100644 index 0000000000000000000000000000000000000000..f3eef551225eb425b5556ad2b5a7b08b3cf0c123 --- /dev/null +++ b/db/docs/workspace_variables.yml @@ -0,0 +1,10 @@ +--- +table_name: workspace_variables +classes: +- RemoteDevelopment::WorkspaceVariable +feature_categories: +- remote_development +description: Remote Development Workspace variables +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129688 +milestone: '16.4' +gitlab_schema: gitlab_main diff --git a/db/migrate/20230821000001_create_workspace_variables.rb b/db/migrate/20230821000001_create_workspace_variables.rb new file mode 100644 index 0000000000000000000000000000000000000000..fd23b48c3c3be5fb72626ffcb2e1dea543d8c565 --- /dev/null +++ b/db/migrate/20230821000001_create_workspace_variables.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateWorkspaceVariables < Gitlab::Database::Migration[2.1] + def change + create_table :workspace_variables do |t| + t.references :workspace, index: true, null: false, foreign_key: { on_delete: :cascade } + t.integer :variable_type, null: false, limit: 2 + t.timestamps_with_timezone null: false + t.text :key, null: false, limit: 255 + t.binary :encrypted_value, null: false + t.binary :encrypted_value_iv, null: false + end + end +end diff --git a/db/migrate/20230821000002_add_personal_access_token_id_to_workspaces.rb b/db/migrate/20230821000002_add_personal_access_token_id_to_workspaces.rb new file mode 100644 index 0000000000000000000000000000000000000000..8b4d5f259599b9ade60840a04e3629d3e9df2c8a --- /dev/null +++ b/db/migrate/20230821000002_add_personal_access_token_id_to_workspaces.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class AddPersonalAccessTokenIdToWorkspaces < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = "index_workspaces_on_personal_access_token_id" + + def up + with_lock_retries do + add_column :workspaces, :personal_access_token_id, :bigint + end + + add_concurrent_index :workspaces, :personal_access_token_id, name: INDEX_NAME + + # Personal Access Tokens are revokable and are soft deleted, so the record should never actually be deleted. + # Therefore, `restrict` is the appropriate choice, because if a record ever is attempted to be deleted + # outside of Rails, this should be prevented, because `nullify` would result in an invalid state for the workspace, + # and `cascade` would delete the workspace. + add_concurrent_foreign_key :workspaces, + :personal_access_tokens, + column: :personal_access_token_id, + on_delete: :restrict + end + + def down + remove_concurrent_index_by_name :workspaces, INDEX_NAME + remove_foreign_key_if_exists :workspaces, column: :personal_access_tokens + + with_lock_retries do + remove_column :workspaces, :personal_access_token_id, if_exists: true + end + end +end diff --git a/db/migrate/20230821000003_add_config_version_to_workspaces.rb b/db/migrate/20230821000003_add_config_version_to_workspaces.rb new file mode 100644 index 0000000000000000000000000000000000000000..c9ddf53c785ead88a9a261da5de60fda0247e406 --- /dev/null +++ b/db/migrate/20230821000003_add_config_version_to_workspaces.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddConfigVersionToWorkspaces < Gitlab::Database::Migration[2.1] + def change + add_column :workspaces, :config_version, :integer, default: 1, null: false + end +end diff --git a/db/schema_migrations/20230821000001 b/db/schema_migrations/20230821000001 new file mode 100644 index 0000000000000000000000000000000000000000..335a019ce21b775c9401736dd0d45e1f59479b63 --- /dev/null +++ b/db/schema_migrations/20230821000001 @@ -0,0 +1 @@ +30eb1215fb4411780a722c0d49b7e30316200459dd91f67525f4ae5894aa1acc \ No newline at end of file diff --git a/db/schema_migrations/20230821000002 b/db/schema_migrations/20230821000002 new file mode 100644 index 0000000000000000000000000000000000000000..ed24b64020d86a42b69cb52acc2a031a6527db75 --- /dev/null +++ b/db/schema_migrations/20230821000002 @@ -0,0 +1 @@ +ff4aafeb32b4e09ec8344afa8684fda2fd2131a8d4b8f82806a0ca5341beef59 \ No newline at end of file diff --git a/db/schema_migrations/20230821000003 b/db/schema_migrations/20230821000003 new file mode 100644 index 0000000000000000000000000000000000000000..395186ab4f19c74d133fc5c145b663d0b7766e70 --- /dev/null +++ b/db/schema_migrations/20230821000003 @@ -0,0 +1 @@ +5ece2c99a97204a2888f5951d4cd2b16a75e47e395c8a09fa16f151e7d28e16c \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 01f6c7741039e48c8cf6a2d2e1867e11249506dd..e6812177fc7d587187b671382b136df65d084af7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -25103,6 +25103,27 @@ CREATE SEQUENCE work_item_widget_definitions_id_seq ALTER SEQUENCE work_item_widget_definitions_id_seq OWNED BY work_item_widget_definitions.id; +CREATE TABLE workspace_variables ( + id bigint NOT NULL, + workspace_id bigint NOT NULL, + variable_type smallint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + key text NOT NULL, + encrypted_value bytea NOT NULL, + encrypted_value_iv bytea NOT NULL, + CONSTRAINT check_5545042100 CHECK ((char_length(key) <= 255)) +); + +CREATE SEQUENCE workspace_variables_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE workspace_variables_id_seq OWNED BY workspace_variables.id; + CREATE TABLE workspaces ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, @@ -25124,6 +25145,8 @@ CREATE TABLE workspaces ( processed_devfile text, url text NOT NULL, deployment_resource_version text, + personal_access_token_id bigint, + config_version integer DEFAULT 1 NOT NULL, CONSTRAINT check_15543fb0fa CHECK ((char_length(name) <= 64)), CONSTRAINT check_157d5f955c CHECK ((char_length(namespace) <= 64)), CONSTRAINT check_2b401b0034 CHECK ((char_length(deployment_resource_version) <= 64)), @@ -26421,6 +26444,8 @@ ALTER TABLE ONLY work_item_types ALTER COLUMN id SET DEFAULT nextval('work_item_ ALTER TABLE ONLY work_item_widget_definitions ALTER COLUMN id SET DEFAULT nextval('work_item_widget_definitions_id_seq'::regclass); +ALTER TABLE ONLY workspace_variables ALTER COLUMN id SET DEFAULT nextval('workspace_variables_id_seq'::regclass); + ALTER TABLE ONLY workspaces ALTER COLUMN id SET DEFAULT nextval('workspaces_id_seq'::regclass); ALTER TABLE ONLY x509_certificates ALTER COLUMN id SET DEFAULT nextval('x509_certificates_id_seq'::regclass); @@ -29038,6 +29063,9 @@ ALTER TABLE ONLY work_item_types ALTER TABLE ONLY work_item_widget_definitions ADD CONSTRAINT work_item_widget_definitions_pkey PRIMARY KEY (id); +ALTER TABLE ONLY workspace_variables + ADD CONSTRAINT workspace_variables_pkey PRIMARY KEY (id); + ALTER TABLE ONLY workspaces ADD CONSTRAINT workspaces_pkey PRIMARY KEY (id); @@ -34068,10 +34096,14 @@ CREATE UNIQUE INDEX index_work_item_widget_definitions_on_namespace_type_and_nam CREATE INDEX index_work_item_widget_definitions_on_work_item_type_id ON work_item_widget_definitions USING btree (work_item_type_id); +CREATE INDEX index_workspace_variables_on_workspace_id ON workspace_variables USING btree (workspace_id); + CREATE INDEX index_workspaces_on_cluster_agent_id ON workspaces USING btree (cluster_agent_id); CREATE UNIQUE INDEX index_workspaces_on_name ON workspaces USING btree (name); +CREATE INDEX index_workspaces_on_personal_access_token_id ON workspaces USING btree (personal_access_token_id); + CREATE INDEX index_workspaces_on_project_id ON workspaces USING btree (project_id); CREATE INDEX index_workspaces_on_user_id ON workspaces USING btree (user_id); @@ -36983,6 +37015,9 @@ ALTER TABLE ONLY pages_domains ALTER TABLE ONLY catalog_resource_components ADD CONSTRAINT fk_ec417536da FOREIGN KEY (catalog_resource_id) REFERENCES catalog_resources(id) ON DELETE CASCADE; +ALTER TABLE ONLY workspaces + ADD CONSTRAINT fk_ec70695b2c FOREIGN KEY (personal_access_token_id) REFERENCES personal_access_tokens(id) ON DELETE RESTRICT; + ALTER TABLE ONLY merge_requests_compliance_violations ADD CONSTRAINT fk_ec881c1c6f FOREIGN KEY (violating_user_id) REFERENCES users(id) ON DELETE CASCADE; @@ -37649,6 +37684,9 @@ ALTER TABLE ONLY elastic_group_index_statuses ALTER TABLE ONLY bulk_import_configurations ADD CONSTRAINT fk_rails_536b96bff1 FOREIGN KEY (bulk_import_id) REFERENCES bulk_imports(id) ON DELETE CASCADE; +ALTER TABLE ONLY workspace_variables + ADD CONSTRAINT fk_rails_539844891e FOREIGN KEY (workspace_id) REFERENCES workspaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY x509_commit_signatures ADD CONSTRAINT fk_rails_53fe41188f FOREIGN KEY (x509_certificate_id) REFERENCES x509_certificates(id) ON DELETE CASCADE; diff --git a/ee/app/models/ee/personal_access_token.rb b/ee/app/models/ee/personal_access_token.rb index d20e77071a761c8ae281a823b078c211a028d5f9..36d3869682fd2958bd7faf995f537909fa8514a7 100644 --- a/ee/app/models/ee/personal_access_token.rb +++ b/ee/app/models/ee/personal_access_token.rb @@ -13,6 +13,11 @@ module PersonalAccessToken include ::Gitlab::Utils::StrongMemoize include FromUnion + has_one :workspace, + class_name: 'RemoteDevelopment::Workspace', + inverse_of: :personal_access_token, + foreign_key: :personal_access_token_id + after_create :clear_rotation_notification_cache scope :with_expires_at_after, ->(max_lifetime) { where(revoked: false).where('expires_at > ?', max_lifetime) } diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index b3a5aefe6a8a613bb9b6d70d5807d91d3c4c32c3..9cf39310054c382166f6ce519d6138ed1814a789 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -12,8 +12,11 @@ class Workspace < ApplicationRecord belongs_to :user, inverse_of: :workspaces belongs_to :project, inverse_of: :workspaces belongs_to :agent, class_name: 'Clusters::Agent', foreign_key: 'cluster_agent_id', inverse_of: :workspaces + belongs_to :personal_access_token, inverse_of: :workspace has_one :remote_development_agent_config, through: :agent, source: :remote_development_agent_config + has_many :workspace_variables, class_name: 'RemoteDevelopment::WorkspaceVariable', inverse_of: :workspace + delegate :dns_zone, to: :remote_development_agent_config, prefix: false, allow_nil: false validates :user, presence: true diff --git a/ee/app/models/remote_development/workspace_variable.rb b/ee/app/models/remote_development/workspace_variable.rb new file mode 100644 index 0000000000000000000000000000000000000000..242a77dfece6c8a3d6fd887ef9595587ba6c4628 --- /dev/null +++ b/ee/app/models/remote_development/workspace_variable.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RemoteDevelopment + class WorkspaceVariable < ApplicationRecord + belongs_to :workspace, class_name: 'RemoteDevelopment::Workspace', inverse_of: :workspace_variables + + enum variable_type: { + env_var: 0, + file: 1 + }, _prefix: :variable_type + + validates :variable_type, presence: true + + validates :key, + presence: true, + length: { maximum: 255 } + + attr_encrypted :value, + mode: :per_attribute_iv, + key: Settings.attr_encrypted_db_key_base_32, + algorithm: 'aes-256-gcm' + end +end diff --git a/ee/spec/factories/remote_development/workspace_variables.rb b/ee/spec/factories/remote_development/workspace_variables.rb new file mode 100644 index 0000000000000000000000000000000000000000..4c9bbe81c9201b46f384e0c755b90a1de29ae5af --- /dev/null +++ b/ee/spec/factories/remote_development/workspace_variables.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :workspace_variable, class: 'RemoteDevelopment::WorkspaceVariable' do + workspace + + key { 'my_key' } + value { 'my_value' } + variable_type { RemoteDevelopment::WorkspaceVariable.variable_types[:file] } + end +end diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index 498a7555c3de2340a53297332ac89995832cdfb3..f91d769df24a4f3acd11a4a7cb78857bab8ff9e8 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -5,6 +5,7 @@ project factory: [:project, :public, :in_group] user agent factory: [:ee_cluster_agent, :with_remote_development_agent_config] + personal_access_token name { "workspace-#{agent.id}-#{user.id}-#{random_string}" } diff --git a/ee/spec/models/ee/personal_access_token_spec.rb b/ee/spec/models/ee/personal_access_token_spec.rb index eacda8d4c04e189d33ec6503416d294d41b24516..cde08204b91fdfb909774bc52ea761212b0e69a2 100644 --- a/ee/spec/models/ee/personal_access_token_spec.rb +++ b/ee/spec/models/ee/personal_access_token_spec.rb @@ -3,6 +3,24 @@ require 'spec_helper' RSpec.describe PersonalAccessToken, feature_category: :system_access do + describe 'associations' do + subject { create(:personal_access_token) } + + it do + is_expected + .to have_one(:workspace) + .class_name('RemoteDevelopment::Workspace') + .inverse_of(:personal_access_token) + .with_foreign_key(:personal_access_token_id) + end + + it 'has a bidirectional relationship with a workspace' do + workspace = create(:workspace, personal_access_token: subject) + + expect(workspace.personal_access_token).to eq(subject) + end + end + describe 'scopes' do let_it_be(:expiration_date) { 30.days.from_now } let_it_be(:pat) { create(:personal_access_token, expires_at: expiration_date) } diff --git a/ee/spec/models/factories_spec.rb b/ee/spec/models/factories_spec.rb index b20182af218a4da7e23dc57249fc772081949360..675f175d6a1df577c3538bbaeb0b155b1a2a7249 100644 --- a/ee/spec/models/factories_spec.rb +++ b/ee/spec/models/factories_spec.rb @@ -156,6 +156,7 @@ wiki_page wiki_page_meta workspace + workspace_variable ].to_set.freeze # Some factories and their corresponding models are based on diff --git a/ee/spec/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index 456879240a857674308f822fecc6146bd7d24297..7c3e1cc161e5a6f08c168f10be54fa798bc867e3 100644 --- a/ee/spec/models/remote_development/workspace_spec.rb +++ b/ee/spec/models/remote_development/workspace_spec.rb @@ -6,11 +6,16 @@ let_it_be(:user) { create(:user) } let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } let_it_be(:project) { create(:project, :public, :in_group) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - subject { create(:workspace, user: user, agent: agent, project: project) } + subject do + create(:workspace, user: user, agent: agent, project: project, personal_access_token: personal_access_token) + end describe 'associations' do it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:personal_access_token) } + it { is_expected.to have_many(:workspace_variables) } it { is_expected.to have_one(:remote_development_agent_config) } # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409786 @@ -21,6 +26,7 @@ expect(subject.user).to eq(user) expect(subject.project).to eq(project) expect(subject.agent).to eq(agent) + expect(subject.personal_access_token).to eq(personal_access_token) expect(subject.remote_development_agent_config).to eq(agent.remote_development_agent_config) expect(agent.remote_development_agent_config.workspaces.first).to eq(subject) end diff --git a/ee/spec/models/remote_development/workspace_variable_spec.rb b/ee/spec/models/remote_development/workspace_variable_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..cce2196e4028e48ff480535affc41b0368dc5020 --- /dev/null +++ b/ee/spec/models/remote_development/workspace_variable_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::WorkspaceVariable, feature_category: :remote_development do + let(:variable_type_values) { { env_var: 0, file: 1 } } + let(:key) { 'key_1' } + let(:value) { 'value_1' } + let_it_be(:workspace) do + create(:workspace) + end + + subject do + create(:workspace_variable, workspace: workspace, key: key, value: value) + end + + describe 'associations' do + it { is_expected.to belong_to(:workspace) } + + it 'has correct associations from factory' do + expect(subject.workspace).to eq(workspace) + end + end + + describe 'enums' do + it { is_expected.to define_enum_for(:variable_type).with_values(variable_type_values).with_prefix(:variable_type) } + + it_behaves_like 'having unique enum values' + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:key) } + it { is_expected.to validate_length_of(:key).is_at_most(255) } + it { is_expected.to validate_presence_of(:variable_type) } + end + + describe '#value' do + it 'can be decrypted' do + expect(subject.value).to eq(value) + end + end +end