diff --git a/db/migrate/20221225010101_create_workspaces_table.rb b/db/migrate/20221225010101_create_workspaces_table.rb index 90480b7f9c840e27f2645e4da2d4d05890de68b3..73491c7a92ca7bfef96622d0c21f06c2e8d3c8e9 100644 --- a/db/migrate/20221225010101_create_workspaces_table.rb +++ b/db/migrate/20221225010101_create_workspaces_table.rb @@ -10,15 +10,20 @@ def up t.references :user, null: false, index: true, foreign_key: { on_delete: :cascade } t.references :namespace, null: false, foreign_key: { on_delete: :cascade } t.references :cluster_agent, null: false, foreign_key: { on_delete: :cascade } - t.references :project, null: true, foreign_key: { on_delete: :cascade } - t.text :name, limit: 63, null: false, index: { unique: true } - t.text :namespace, limit: 63, null: false + t.references :project, null: false, foreign_key: { on_delete: :cascade } + t.text :name, limit: 64, null: false, index: { unique: true } + t.text :namespace, limit: 64, null: false t.text :desired_state, limit: 32, null: false t.text :actual_state, limit: 32, null: false - t.text :editor, limit: 255, null: false - t.text :url, limit: 256 + t.text :editor, limit: 256, null: false + t.text :devfile_ref, limit: 256, null: false + t.text :devfile_path, limit: 2048, null: false + # NOTE: THe limit on the devfile field is arbitrary, and only added to avoid + # rubocop Migration/AddLimitToTextColumns error. We don't know what the upper bound to expect is, + # but 65535 allows for a YAML file with over 800 lines of an average 80-character length. + t.text :devfile, limit: 65535 + t.text :url, limit: 1024, null: false t.integer :deployment_resource_version - t.text :devfile # rubocop:disable Migration/AddLimitToTextColumns end # rubocop:enable Migration/CreateTableWithForeignKeys end diff --git a/db/structure.sql b/db/structure.sql index 8c375a9159b348f4d78c4f29caa6537bc8935042..5d40acde28c1789226cc65a63a7069e860a43a67 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24297,21 +24297,26 @@ CREATE TABLE workspaces ( user_id bigint NOT NULL, namespace_id bigint NOT NULL, cluster_agent_id bigint NOT NULL, - project_id bigint, + project_id bigint NOT NULL, name text NOT NULL, namespace text NOT NULL, desired_state text NOT NULL, actual_state text NOT NULL, editor text NOT NULL, - url text, - deployment_resource_version integer, + devfile_ref text NOT NULL, + devfile_path text NOT NULL, devfile text, - CONSTRAINT check_15543fb0fa CHECK ((char_length(name) <= 63)), - CONSTRAINT check_157d5f955c CHECK ((char_length(namespace) <= 63)), + url text NOT NULL, + deployment_resource_version integer, + CONSTRAINT check_15543fb0fa CHECK ((char_length(name) <= 64)), + CONSTRAINT check_157d5f955c CHECK ((char_length(namespace) <= 64)), + CONSTRAINT check_8e363ee3ad CHECK ((char_length(devfile_ref) <= 256)), CONSTRAINT check_8e4db5ffc2 CHECK ((char_length(actual_state) <= 32)), - CONSTRAINT check_9e42558c35 CHECK ((char_length(url) <= 256)), + CONSTRAINT check_9e42558c35 CHECK ((char_length(url) <= 1024)), CONSTRAINT check_b70eddcbc1 CHECK ((char_length(desired_state) <= 32)), - CONSTRAINT check_d7ed376e49 CHECK ((char_length(editor) <= 255)) + CONSTRAINT check_d7ed376e49 CHECK ((char_length(editor) <= 256)), + CONSTRAINT check_dc58d56169 CHECK ((char_length(devfile_path) <= 2048)), + CONSTRAINT check_eb32879a3d CHECK ((char_length(devfile) <= 65535)) ); CREATE SEQUENCE workspaces_id_seq diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 9ea8f700cbbe3dba36436c277ca4c26250baeeaf..c8425f19091fc6c30393c4189e16b97bc16a7a0e 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -6699,6 +6699,8 @@ Input type: `WorkspaceCreateInput` | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `clusterAgentId` | [`ClustersAgentID!`](#clustersagentid) | ID of the cluster agent the created workspace will be associated with. | | `desiredState` | [`String!`](#string) | Desired state of the created workspace. | +| `devfilePath` | [`String!`](#string) | Project repo git path containing the devfile used to configure the workspace. | +| `devfileRef` | [`String!`](#string) | Project repo git ref containing the devfile used to configure the workspace. | | `editor` | [`String!`](#string) | Editor to inject into the created workspace. Must match a configured template. | | `groupPath` | [`ID!`](#id) | Group (namespace) full path the created workspace will be associated with. | | `projectId` | [`ProjectID!`](#projectid) | ID of the project that will provide the Devfile for the created workspace. | @@ -6720,7 +6722,7 @@ Input type: `WorkspaceUpdateInput` | Name | Type | Description | | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `desiredState` | [`String`](#string) | Desired state of the created workspace. | +| `desiredState` | [`String!`](#string) | Desired state of the created workspace. | | `id` | [`RemoteDevelopmentWorkspaceID!`](#remotedevelopmentworkspaceid) | Global ID of the workspace. | #### Fields @@ -22609,15 +22611,17 @@ Represents a remote development workspace. | `clusterAgent` | [`ClusterAgent!`](#clusteragent) | Kubernetes Agent associated with the workspace. | | `deploymentResourceVersion` | [`Int`](#int) | ResourceVersion of the Deployment resource for the workspace. | | `desiredState` | [`String!`](#string) | Desired state of the workspace. | -| `devfile` | [`String!`](#string) | Devfile used to configure the workspace. | +| `devfile` | [`String!`](#string) | Source YAML of the devfile used to configure the workspace. | +| `devfilePath` | [`String!`](#string) | Project repo git path containing the devfile used to configure the workspace. | +| `devfileRef` | [`String!`](#string) | Project repo git ref containing the devfile used to configure the workspace. | | `displayedState` | [`String!`](#string) | Displayed state of the workspace. Calculated from desired_state+actual_state. | | `editor` | [`String!`](#string) | Editor used to configure the workspace. Must match a configured template. | | `group` | [`Group!`](#group) | Group associated with the workspace. | | `id` | [`RemoteDevelopmentWorkspaceID!`](#remotedevelopmentworkspaceid) | Global ID of the workspace. | | `name` | [`String!`](#string) | Name of the workspace in Kubernetes. | | `namespace` | [`String!`](#string) | Namespace of the workspace in Kubernetes. | -| `project` | [`Project`](#project) | Project providing the Devfile for the workspace. | -| `url` | [`String`](#string) | URL of the workspace. | +| `project` | [`Project!`](#project) | Project providing the Devfile for the workspace. | +| `url` | [`String!`](#string) | URL of the workspace. | ### `X509Certificate` diff --git a/ee/app/assets/javascripts/remote_development/components/workspace_form.vue b/ee/app/assets/javascripts/remote_development/components/workspace_form.vue index 2f41b872d3fffd8e18f55dcaa88cb826b0d73da6..584cd4398939830c09fd0cb2b34e5dd91ed9d606 100644 --- a/ee/app/assets/javascripts/remote_development/components/workspace_form.vue +++ b/ee/app/assets/javascripts/remote_development/components/workspace_form.vue @@ -40,6 +40,8 @@ export default { clusterAgentId: s__('Workspaces|Cluster agent id'), desiredState: s__('Workspaces|Desired state'), editor: s__('Workspaces|Editor'), + devfileRef: s__('Workspaces|Devfile Ref'), + devfilePath: s__('Workspaces|Devfile Path'), devfile: s__('Workspaces|Devfile'), devfileProject: s__('Workspaces|Devfile project'), }, @@ -85,6 +87,8 @@ export default { this.desiredState = workspace.desiredState; this.actualState = workspace.actualState; this.editor = workspace.editor; + this.devfileRef = workspace.devfileRef; + this.devfilePath = workspace.devfilePath; this.devfile = workspace.devfile; this.projectId = workspace.project.id; @@ -155,6 +159,8 @@ export default { desiredState: this.validDesiredStates[0], projects: [], projectId: null, + devfileRef: null, + devfilePath: null, editor: null, devfile: null, }; @@ -190,6 +196,8 @@ export default { clusterAgentId: this.clusterAgentId, projectId: this.projectId, desiredState: this.desiredState, + devfileRef: this.devfileRef, + devfilePath: this.devfilePath, editor: this.editor, }; }, @@ -202,6 +210,8 @@ export default { }, async mounted() { if (!this.isEditing) { + this.devfileRef = 'main'; + this.devfilePath = '.devfile.yml'; this.editor = 'webide'; } return true; @@ -355,6 +365,7 @@ export default { > @@ -374,6 +385,26 @@ export default { /> + + + + + +
example_devfile } + files = { devfile_path => example_devfile } create(:project, :custom_repo, files: files, namespace: group, creator: user) end @@ -49,6 +50,8 @@ # CREATE WORKSPACE click_link 'New workspace' + fill_in 'Devfile Ref', with: project.default_branch + fill_in 'Devfile Path', with: devfile_path wait_for_requests click_button 'Create workspace' wait_for_requests diff --git a/ee/spec/graphql/types/remote_development/workspace_type_spec.rb b/ee/spec/graphql/types/remote_development/workspace_type_spec.rb index 80c8bd0c0e615391ee36ae2c3c4909c3ca4bc8a2..a5a80d25cbab27a3eaefc0c1940083b844b5d85b 100644 --- a/ee/spec/graphql/types/remote_development/workspace_type_spec.rb +++ b/ee/spec/graphql/types/remote_development/workspace_type_spec.rb @@ -5,8 +5,8 @@ RSpec.describe GitlabSchema.types['Workspace'], feature_category: :remote_development do let(:fields) do %i[ - id cluster_agent group project name namespace desired_state actual_state displayed_state url editor devfile - deployment_resource_version + id cluster_agent group project name namespace desired_state actual_state displayed_state url editor + devfile_ref devfile_path devfile deployment_resource_version ] end diff --git a/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/create_spec.rb b/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/create_spec.rb index 05544daabc5288c48442c3de01e086043ea5d862..1bfe535659b4093d401bd25df8cc9c3cef0bb423 100644 --- a/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/create_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/create_spec.rb @@ -6,26 +6,43 @@ include GraphqlHelpers let_it_be(:user) { create(:user) } + let_it_be(:current_user) { user } # NOTE: Some graphql spec helper methods rely on current_user to be set let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository) } let_it_be(:agent) { create(:cluster_agent) } let(:desired_state) { RemoteDevelopment::States::RUNNING } - let(:attributes) do + let(:all_mutation_args) do { desired_state: desired_state, editor: 'webide', group_path: group.full_path, cluster_agent_id: agent.to_global_id.to_s, - project_id: project.to_global_id.to_s + project_id: project.to_global_id.to_s, + devfile_ref: 'main', + devfile_path: '.devfile.yml' } end - let(:params) { attributes } + let(:mutation_args) { all_mutation_args } let(:mutation) do - graphql_mutation(:workspace_create, params) + graphql_mutation(:workspace_create, mutation_args) + end + + let(:expected_service_params) do + params = all_mutation_args.except(:group_path, :cluster_agent_id, :project_id) + params[:agent] = agent + params[:project] = project + params + end + + let_it_be(:created_workspace, refind: true) { create(:workspace, group: group, user: user) } + + let(:stub_service_payload) { { workspace: created_workspace } } + let(:stub_service_response) do + ServiceResponse.success(payload: stub_service_payload) end def mutation_response @@ -35,8 +52,14 @@ def mutation_response before do group.add_developer(user) agent.project.add_developer(user) - allow_next_instance_of(Repository) do |repository_instance| - allow(repository_instance).to receive_message_chain(:blob_at_branch, :data) { '---' } + allow_next_instance_of( + ::RemoteDevelopment::Workspaces::CreateService, group: group, current_user: user + ) do |service_instance| + allow(service_instance).to receive(:execute).with( + params: expected_service_params, override_namespace_to_default: false + ) do + stub_service_response + end end end @@ -45,36 +68,22 @@ def mutation_response expect_graphql_errors_to_be_empty - workspace_hash = mutation_response.fetch('workspace') - aggregate_failures do - expect(workspace_hash.fetch('desiredState')).to eq('Running') - end + expect(mutation_response.fetch('workspace')['name']).to eq(created_workspace['name']) end - context 'when there are ActiveRecord validation errors' do - let(:desired_state) { 'InvalidDesiredState' } - - # TODO: this fails because the desired state validation has "Internal server error" as the error message - # it_behaves_like 'a mutation that returns errors in the response', errors: ['some error'] + context 'when there are service errors' do + let(:stub_service_response) { ::ServiceResponse.error(message: 'some error', reason: :bad_request) } - it 'does not create the workspace' do - expect { post_graphql_mutation(mutation, current_user: user) } - .not_to change { RemoteDevelopment::Workspace.count } - end + it_behaves_like 'a mutation that returns errors in the response', errors: ['some error'] end context 'when required arguments are missing' do - let(:params) { attributes.except(:desired_state) } + let(:mutation_args) { all_mutation_args.except(:desired_state) } it 'returns error about required argument' do post_graphql_mutation(mutation, current_user: user) expect_graphql_errors_to_include(/was provided invalid value for desiredState \(Expected value to not be null\)/) end - - it 'does not create the workspace' do - expect { post_graphql_mutation(mutation, current_user: user) } - .not_to change { RemoteDevelopment::Workspace.count } - end end end diff --git a/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/update_spec.rb b/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/update_spec.rb index 490577ec2d4abdf68be4379c9074fd02b824704c..a399679977f71f20e35b532e3a5fc7ea331f493f 100644 --- a/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/update_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/remote_development/workspaces/update_spec.rb @@ -6,71 +6,73 @@ include GraphqlHelpers let_it_be(:user) { create(:user) } + let_it_be(:current_user) { user } # NOTE: Some graphql spec helper methods rely on current_user to be set let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository) } + let_it_be(:agent) { create(:cluster_agent) } let_it_be(:workspace, refind: true) do create( :workspace, group: group, + agent: agent, user: user, desired_state: RemoteDevelopment::States::RUNNING, editor: 'old_editor' ) end - let(:attributes) do + let(:all_mutation_args) do { id: workspace.to_global_id.to_s, desired_state: RemoteDevelopment::States::STOPPED } end - let(:params) { attributes } - - let(:mutation) do - graphql_mutation(:workspace_update, params) + let(:mutation_args) { { id: global_id_of(workspace), desired_state: RemoteDevelopment::States::STOPPED } } + let(:mutation) { graphql_mutation(:workspace_update, mutation_args) } + let(:expected_service_params) { all_mutation_args.except(:id) } + let(:stub_service_payload) { { workspace: workspace } } + let(:stub_service_response) do + ServiceResponse.success(payload: stub_service_payload) end def mutation_response graphql_mutation_response(:workspace_update) end - it 'updates the workspace', :aggregate_failures do + before do + group.add_developer(user) + agent.project.add_developer(user) + allow_next_instance_of( + ::RemoteDevelopment::Workspaces::UpdateService, workspace: workspace, current_user: user + ) do |service_instance| + allow(service_instance).to receive(:execute).with(params: expected_service_params) do + stub_service_response + end + end + end + + it 'updates the workspace' do post_graphql_mutation(mutation, current_user: user) expect_graphql_errors_to_be_empty - workspace_hash = mutation_response['workspace'] - expect(workspace_hash['desiredState']).to eq(RemoteDevelopment::States::STOPPED) - - workspace.reload - expect(workspace.desired_state).to eq(RemoteDevelopment::States::STOPPED) + expect(mutation_response.fetch('workspace')['name']).to eq(workspace['name']) end - context 'when there are ActiveRecord validation errors' do - let(:params) { attributes.merge({ desired_state: "InvalidDesiredState" }) } + context 'when there are service errors' do + let(:stub_service_response) { ::ServiceResponse.error(message: 'some error', reason: :bad_request) } - # TODO: this fails because the desired state validation has "Internal server error" as the error message - # it_behaves_like 'a mutation that returns errors in the response', errors: ['some error'] - - it 'does not update the workspace' do - expect do - post_graphql_mutation(mutation, current_user: user) - workspace.reload - end - .not_to change { workspace.desired_state } - end + it_behaves_like 'a mutation that returns errors in the response', errors: ['some error'] end context 'when some required arguments are missing' do - let(:params) { attributes.except(:desired_state) } + let(:mutation_args) { all_mutation_args.except(:desired_state) } - it 'does not update the workspace' do - expect do - post_graphql_mutation(mutation, current_user: user) - workspace.reload - end - .not_to change { workspace.desired_state } + it 'returns error about required argument' do + post_graphql_mutation(mutation, current_user: user) + + expect_graphql_errors_to_include(/was provided invalid value for desiredState \(Expected value to not be null\)/) end end end diff --git a/ee/spec/services/remote_development/workspaces/create_service_spec.rb b/ee/spec/services/remote_development/workspaces/create_service_spec.rb index 67a1b2fbfd8eb9388802dddbfa292ce6c1928977..a61a3216321df60d125e00fabcaa4bbf3505c02d 100644 --- a/ee/spec/services/remote_development/workspaces/create_service_spec.rb +++ b/ee/spec/services/remote_development/workspaces/create_service_spec.rb @@ -29,7 +29,8 @@ project: project, editor: 'webide', desired_state: RemoteDevelopment::States::RUNNING, - devfile: fake_devfile + devfile_ref: 'main', + devfile_path: '.devfile.yml' } service_response = subject.execute(params: params) expect(service_response).to be_success diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3a385e4fd372299291112edddde08a0f9439c473..ea4cf56131d19a23d95de7728181bc89e73338d3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -49791,6 +49791,12 @@ msgstr "" msgid "Workspaces|Devfile" msgstr "" +msgid "Workspaces|Devfile Path" +msgstr "" + +msgid "Workspaces|Devfile Ref" +msgstr "" + msgid "Workspaces|Devfile project" msgstr "" diff --git a/scripts/remote_development/reset-remote-dev-db-tables.sh b/scripts/remote_development/reset-remote-dev-db-tables.sh index 5a8d562ee2b19349f060f8927bb42700d75e6e72..3b8c9ed4a022a0b02ac79bfe58848e527ddb061f 100755 --- a/scripts/remote_development/reset-remote-dev-db-tables.sh +++ b/scripts/remote_development/reset-remote-dev-db-tables.sh @@ -1,5 +1,7 @@ # Resets all db schema related to remote development environment, so we can # keep updating a single migration for the spike branch. +# All ongoing migration code will be added to the 20221225010101 migration while we are still +# on the branch. # shellcheck disable=SC2089 CMD1=("DROP TABLE workspaces") diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 191e5192a610ff23e8465b4e8c7ebf50336ef327..012d27c46520b4321b7c2a1778d4cc467c10df0a 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -633,7 +633,11 @@ def expect_graphql_errors_to_include(regexes_to_match) end def expect_graphql_errors_to_be_empty - expect(flattened_errors).to be_empty + # TODO: using eq([]) instead of be_empty makes it print out the full error message including the + # raisedAt key which contains the full stacktrace. This is necessary to know where the + # unexpected error occurred during tests. + # This or an equivalent fix should be added in a separate MR on master. + expect(flattened_errors).to eq([]) end # Helps migrate to the new GraphQL interpreter,