diff --git a/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb b/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb new file mode 100644 index 0000000000000000000000000000000000000000..1aa34861e08aeb221782344cbf110e2df57b2186 --- /dev/null +++ b/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class UpdateWorkspacesDevfilePathNullable < Gitlab::Database::Migration[2.2] + milestone '17.8' + disable_ddl_transaction! + + def up + change_column_null :workspaces, :devfile_path, true + end + + def down + change_column_null :workspaces, :devfile_path, false + end +end diff --git a/db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb b/db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb new file mode 100644 index 0000000000000000000000000000000000000000..d8d8eea02534afd823d9d5d551186471bf62bbe6 --- /dev/null +++ b/db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class RenameWorkspacesDevfileRefToProjectRef < Gitlab::Database::Migration[2.2] + milestone '17.8' + disable_ddl_transaction! + + def up + rename_column_concurrently :workspaces, :devfile_ref, :project_ref + end + + def down + undo_rename_column_concurrently :workspaces, :devfile_ref, :project_ref + end +end diff --git a/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb b/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb new file mode 100644 index 0000000000000000000000000000000000000000..217f723f4bc281bccddfa3df0dcbb93e4d8c1527 --- /dev/null +++ b/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CleanupWorkspacesDevfileRefRename < Gitlab::Database::Migration[2.2] + milestone '17.8' + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :workspaces, :devfile_ref, :project_ref + end + + def down + undo_cleanup_concurrent_column_rename :workspaces, :devfile_ref, :project_ref + end +end diff --git a/db/schema_migrations/20241118061025 b/db/schema_migrations/20241118061025 new file mode 100644 index 0000000000000000000000000000000000000000..742a591c8ecd2ac2d7f810fa99e3f5bb22c5738c --- /dev/null +++ b/db/schema_migrations/20241118061025 @@ -0,0 +1 @@ +2ab125f0a48dbece1a6e98b698f2f44734f07976e34909a502f7cfbb937d9c55 \ No newline at end of file diff --git a/db/schema_migrations/20241118061030 b/db/schema_migrations/20241118061030 new file mode 100644 index 0000000000000000000000000000000000000000..2dba8d5e77a89c6084d730c308764ceb0c0b306d --- /dev/null +++ b/db/schema_migrations/20241118061030 @@ -0,0 +1 @@ +3de77a23770fa6eb0bfff996189e1f3b9748299fed8d01c0929423eba4570978 \ No newline at end of file diff --git a/db/schema_migrations/20241118061130 b/db/schema_migrations/20241118061130 new file mode 100644 index 0000000000000000000000000000000000000000..a0981989edec9fe95cd5a5462bdeae95ba062fa4 --- /dev/null +++ b/db/schema_migrations/20241118061130 @@ -0,0 +1 @@ +32e2325b1442e77a35af0544cdc9a6ea7fab37d4f9185264923dcdc574da758e \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e9049f5db8aa1ad9bee27263d590306bfad8443a..729a5d4bca4cfb36ab7e7f8f7deb18b23dbf0951 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22720,8 +22720,7 @@ CREATE TABLE workspaces ( namespace text NOT NULL, desired_state text NOT NULL, actual_state text NOT NULL, - devfile_ref text NOT NULL, - devfile_path text NOT NULL, + devfile_path text, devfile text, processed_devfile text, url text, @@ -22732,15 +22731,17 @@ CREATE TABLE workspaces ( url_query_string text, workspaces_agent_config_version integer NOT NULL, desired_config_generator_version integer, + project_ref text, 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)), CONSTRAINT check_35e31ca320 CHECK ((desired_config_generator_version IS NOT NULL)), + CONSTRAINT check_72fee08424 CHECK ((char_length(project_ref) <= 256)), CONSTRAINT check_77d1a2ff50 CHECK ((char_length(processed_devfile) <= 65535)), CONSTRAINT check_8a0ab61b6b CHECK ((char_length(url_query_string) <= 256)), - 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) <= 1024)), + CONSTRAINT check_a758efdc89 CHECK ((project_ref IS NOT NULL)), CONSTRAINT check_b70eddcbc1 CHECK ((char_length(desired_state) <= 32)), CONSTRAINT check_dc58d56169 CHECK ((char_length(devfile_path) <= 2048)), CONSTRAINT check_eb32879a3d CHECK ((char_length(devfile) <= 65535)), diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 040894605d67c18a4a8969fa6ad6487dfb2e2e0e..b5ba699d441b9c88e5d5a6fa95951628e83900b5 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11613,11 +11613,12 @@ Input type: `WorkspaceCreateInput` | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `clusterAgentId` | [`ClustersAgentID!`](#clustersagentid) | GlobalID 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. | +| `devfilePath` | [`String`](#string) | Project path containing the devfile used to configure the workspace. If not provided, the GitLab default devfile is used. | +| `devfileRef` **{warning-solid}** | [`String`](#string) | **Deprecated:** Argument is renamed to project_ref. Deprecated in GitLab 17.8. | | `editor` **{warning-solid}** | [`String`](#string) | **Deprecated:** Argument is not used. Deprecated in GitLab 17.5. | | `maxHoursBeforeTermination` | [`Int!`](#int) | Maximum hours the workspace can exist before it is automatically terminated. | | `projectId` | [`ProjectID!`](#projectid) | ID of the project that will provide the Devfile for the created workspace. | +| `projectRef` | [`String`](#string) | Project repo git ref. | | `variables` | [`[WorkspaceVariableInput!]`](#workspacevariableinput) | Variables to inject into the workspace. | #### Fields @@ -37985,9 +37986,9 @@ Represents a remote development workspace. | `desiredState` | [`String!`](#string) | Desired state of the workspace. | | `desiredStateUpdatedAt` | [`Time!`](#time) | Timestamp of the last update to the desired state. | | `devfile` | [`String!`](#string) | Source YAML of the devfile used to configure the workspace. | -| `devfilePath` | [`String!`](#string) | Path to the devfile used to configure the workspace. | -| `devfileRef` | [`String!`](#string) | Git reference that contains the devfile used to configure the workspace. | -| `devfileWebUrl` | [`String!`](#string) | Web URL of the devfile used to configure the workspace. | +| `devfilePath` | [`String`](#string) | Path to the devfile used to configure the workspace. | +| `devfileRef` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.8. Field is renamed to project_ref. | +| `devfileWebUrl` **{warning-solid}** | [`String`](#string) | **Deprecated** in GitLab 17.8. Field is not used. | | `editor` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.5. Field is not used. | | `forceIncludeAllResources` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in GitLab 17.6. **Status**: Experiment. Forces all resources to be included for the workspaceduring the next reconciliation with the agent. | | `id` | [`RemoteDevelopmentWorkspaceID!`](#remotedevelopmentworkspaceid) | Global ID of the workspace. | @@ -37996,6 +37997,7 @@ Represents a remote development workspace. | `namespace` | [`String!`](#string) | Namespace of the workspace in Kubernetes. | | `processedDevfile` | [`String!`](#string) | Processed YAML of the devfile used to configure the workspace. | | `projectId` | [`ID!`](#id) | ID of the project that contains the devfile for the workspace. | +| `projectRef` | [`String!`](#string) | Git reference that contains the devfile used to configure the workspace, and that will be cloned into the workspace. | | `respondedToAgentAt` | [`Time`](#time) | Timestamp of the last response sent to the GitLab agent for Kubernetes for the workspace. | | `updatedAt` | [`Time!`](#time) | Timestamp of the last update to any mutable workspace property. | | `url` | [`String!`](#string) | URL of the workspace. | diff --git a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb index ccf1f7de65ca514205153c472947493448ca8736..f9944cf78f2b7e1b98b5e6e356427036e4db2037 100644 --- a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb +++ b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb @@ -43,15 +43,23 @@ class Create < BaseMutation required: true, description: 'ID of the project that will provide the Devfile for the created workspace.' + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/510078 - Remove in 19.0 argument :devfile_ref, GraphQL::Types::String, - required: true, - description: 'Project repo git ref containing the devfile used to configure the workspace.' + required: false, + description: 'Project repo git ref containing the devfile used to configure the workspace.', + deprecated: { reason: 'Argument is renamed to project_ref', milestone: '17.8' } + + argument :project_ref, + GraphQL::Types::String, + required: false, + description: 'Project repo git ref.' argument :devfile_path, GraphQL::Types::String, - required: true, - description: 'Project repo git path containing the devfile used to configure the workspace.' + required: false, + description: 'Project path containing the devfile used to configure the workspace. ' \ + 'If not provided, the GitLab default devfile is used.' argument :variables, [::Types::RemoteDevelopment::WorkspaceVariableInput], required: false, @@ -70,6 +78,18 @@ def resolve(args) cluster_agent_id = args.delete(:cluster_agent_id) + # Ensure that at least one of 'devfile_ref' or 'project_ref' is provided, + # raising an error if neither is present. + unless args[:devfile_ref] || args[:project_ref] + raise ::Gitlab::Graphql::Errors::ArgumentError, + "Either 'project_ref' or deprecated 'devfile_ref' must be provided." + end + + # Remove 'devfile_ref' from the arguments. + # If 'project_ref' is not specified, assign 'devfile_ref' to 'project_ref' for backward compatibility. + devfile_ref = args.delete(:devfile_ref) + args[:project_ref] = devfile_ref if args[:project_ref].nil? + # NOTE: What the following line actually does - the agent is delegating to the project to check that the user # has the :create_workspace ability on the _agent's_ project, which will be true if the user is a developer # on the agent's project. diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 4900890db3c25341893ea9ee8861ee870ec0b7ad..503ec2459d63864d7a2c0cead4c0417c243e646c 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -55,14 +55,22 @@ class WorkspaceType < ::Types::BaseObject field :max_hours_before_termination, GraphQL::Types::Int, null: false, description: 'Number of hours until the workspace automatically terminates.' + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/510078 - Remove in 19.0 field :devfile_ref, GraphQL::Types::String, - null: false, description: 'Git reference that contains the devfile used to configure the workspace.' + null: false, description: 'Git reference that contains the devfile used to configure the workspace.', + deprecated: { reason: 'Field is renamed to project_ref', milestone: '17.8' }, method: :project_ref + + field :project_ref, GraphQL::Types::String, # rubocop:disable GraphQL/ExtractType -- We don't want to extract this to a type + null: false, description: 'Git reference that contains the devfile used to configure the workspace, ' \ + 'and that will be cloned into the workspace' field :devfile_path, GraphQL::Types::String, - null: false, description: 'Path to the devfile used to configure the workspace.' + null: true, description: 'Path to the devfile used to configure the workspace.' + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/503465 - Remove in 19.0 field :devfile_web_url, GraphQL::Types::String, # rubocop:disable GraphQL/ExtractType -- We don't want to extract this to a type, it would cause confusion with the devfile field - null: false, description: 'Web URL of the devfile used to configure the workspace.' + null: true, description: 'Web URL of the devfile used to configure the workspace.', + deprecated: { reason: 'Field is not used', milestone: '17.8' } field :devfile, GraphQL::Types::String, null: false, description: 'Source YAML of the devfile used to configure the workspace.' diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 0647d0f69da75aaecf95b8cea48cc718274b557c..1acb86e716c7d1e9b126e379d2ce82a8dec0e63f 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -9,6 +9,8 @@ class Workspace < ApplicationRecord columns_changing_default :desired_config_generator_version + ignore_column :devfile_ref, remove_with: '17.8', remove_after: '2025-01-08' + 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 @@ -153,8 +155,9 @@ def url query: url_query_string).to_s end + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/503465 - Remove in 19.0 def devfile_web_url - project.http_url_to_repo.gsub(/\.git$/, "/-/blob/#{devfile_ref}/#{devfile_path}") + devfile_path.nil? ? nil : project.http_url_to_repo.gsub(/\.git\Z/, "/-/blob/#{project_ref}/#{devfile_path}") end private diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 3adf0f976d75656e99ecc7d2928c57bbb186cbb1..af15367246d8d029cb8321750a5aa3b4f313a3c9 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -5,6 +5,24 @@ module Settings class DefaultSettings UNDEFINED = nil + # When updating DEFAULT_DEVFILE_YAML, update the user facing doc as well. + # https://docs.gitlab.com/ee/user/workspace/#devfault-devfile + # + # The container image is pinned to linux/amd64 digest, instead of the tag digest. + # This is to prevent Rancher Desktop from pulling the linux/arm64 architecture of the image + # which will disrupt local development since gitlab-workspaces-tools does not support + # that architecture yet and thus the workspace won't start. + # This will be fixed in https://gitlab.com/gitlab-org/workspaces/gitlab-workspaces-tools/-/issues/12 + DEFAULT_DEVFILE_YAML = <<~DEVFILE + schemaVersion: 2.2.0 + components: + - name: development-environment + attributes: + gl/inject-editor: true + container: + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04:golang-1.23@sha256:3bf9ab30623b621b2d09bd3f1eae91996eec6f2264d1f0cb7072141baa0b42a1" + DEVFILE + # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. # See ../README.md for more details. # @return [Hash] @@ -16,6 +34,7 @@ def self.default_settings # the logic for reading settings from ::Gitlab::CurrentSettings. It can be replaced when there is an # actual Remote Development entry in ::Gitlab::CurrentSettings. default_branch_name: [UNDEFINED, String], + default_devfile_yaml: [DEFAULT_DEVFILE_YAML, String], default_max_hours_before_termination: [24, Integer], default_resources_per_workspace_container: [{}, Hash], default_runtime_class: ["", String], diff --git a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb index 113043a7349ab89ef4a30585cca91641f9a6136a..673dc80e7486e2d22f4448834ed0b8f4a2ee7894 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb @@ -12,12 +12,20 @@ class DevfileFetcher # @param [Hash] context # @return [Gitlab::Fp::Result] def self.fetch(context) - context => { params: Hash => params } - params => { - agent: Clusters::Agent => agent, - project: Project => project, - devfile_ref: String => devfile_ref, - devfile_path: String => devfile_path + Gitlab::Fp::Result.ok(context) + .and_then(method(:validate_agent_config_exists)) + .map(method(:use_default_devfile_yaml_if_devfile_path_is_not_provided)) + .and_then(method(:read_devfile_yaml_from_repo_if_devfile_path_is_provided)) + .and_then(method(:parse_devfile_yaml)) + end + + # @param [Hash] context + # @return [Gitlab::Fp::Result] + def self.validate_agent_config_exists(context) + context => { + params: { + agent: Clusters::Agent => agent + } } unless agent.unversioned_latest_workspaces_agent_config @@ -26,24 +34,67 @@ def self.fetch(context) )) end - repository = project.repository + Gitlab::Fp::Result.ok(context) + end + + # @param [Hash] context + # @return [Hash] context + def self.use_default_devfile_yaml_if_devfile_path_is_not_provided(context) + context => { + params: { + devfile_path: String | nil => devfile_path + }, + settings: { + default_devfile_yaml: String => default_devfile_yaml + } + } - devfile_blob = repository.blob_at_branch(devfile_ref, devfile_path) + context[:devfile_yaml] = default_devfile_yaml unless devfile_path + + context + end + + # @param [Hash] context + # @return [Gitlab::Fp::Result] + def self.read_devfile_yaml_from_repo_if_devfile_path_is_provided(context) + context => { + params: { + project: Project => project, + project_ref: String => project_ref, + devfile_path: String | nil => devfile_path + } + } + + return Gitlab::Fp::Result.ok(context) unless devfile_path + + repository = project.repository + devfile_blob = repository.blob_at_branch(project_ref, devfile_path) unless devfile_blob return Gitlab::Fp::Result.err(WorkspaceCreateDevfileLoadFailed.new( - details: "Devfile path '#{devfile_path}' at ref '#{devfile_ref}' does not exist in project repository" + details: "Devfile path '#{devfile_path}' at ref '#{project_ref}' " \ + "does not exist in the project repository" )) end - devfile_yaml = devfile_blob.data - - unless devfile_yaml.present? + unless devfile_blob.data.present? return Gitlab::Fp::Result.err( WorkspaceCreateDevfileLoadFailed.new(details: "Devfile could not be loaded from project") ) end + context[:devfile_yaml] = devfile_blob.data + + Gitlab::Fp::Result.ok(context) + end + + # @param [Hash] context + # @return [Gitlab::Fp::Result] + def self.parse_devfile_yaml(context) + context => { + devfile_yaml: String => devfile_yaml + } + begin # load YAML, convert YAML to JSON and load it again to remove YAML vulnerabilities devfile_stringified = YAML.safe_load(YAML.safe_load(devfile_yaml).to_json) @@ -58,10 +109,12 @@ def self.fetch(context) Gitlab::Fp::Result.ok(context.merge({ # NOTE: The devfile_yaml should only be used for storing it in the database and not in any other # later step in the chain. - devfile_yaml: devfile_yaml, devfile: devfile })) end + + private_class_method :validate_agent_config_exists, :use_default_devfile_yaml_if_devfile_path_is_not_provided, + :read_devfile_yaml_from_repo_if_devfile_path_is_provided, :parse_devfile_yaml end end end diff --git a/ee/lib/remote_development/workspace_operations/create/project_cloner_component_inserter.rb b/ee/lib/remote_development/workspace_operations/create/project_cloner_component_inserter.rb index d8088778601ba47b8e45567756fdc8e5c6a1ec92..625eb1e86aa4ad3448debb763c5effc1f173f77e 100644 --- a/ee/lib/remote_development/workspace_operations/create/project_cloner_component_inserter.rb +++ b/ee/lib/remote_development/workspace_operations/create/project_cloner_component_inserter.rb @@ -19,7 +19,7 @@ def self.insert(context) data_volume => { path: String => volume_path } params => { project: project, - devfile_ref: String => devfile_ref, + project_ref: String => project_ref, } settings => { project_cloner_image: String => image, @@ -52,7 +52,7 @@ def self.insert(context) rm -rf "#{Shellwords.shellescape(clone_dir)}"; fi echo "Cloning project"; - git clone --branch "#{Shellwords.shellescape(devfile_ref)}" "#{Shellwords.shellescape(project_url)}" "#{Shellwords.shellescape(clone_dir)}"; + git clone --branch "#{Shellwords.shellescape(project_ref)}" "#{Shellwords.shellescape(project_url)}" "#{Shellwords.shellescape(clone_dir)}"; exit_code=$? if [ "${exit_code}" -eq 0 ]; then diff --git a/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb b/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb index aac3029f1917d19fa427b91999282cbcb20a57d0..75ae6bbac32ebc6d68b02bd0d1b982cb475ed8c6 100644 --- a/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb +++ b/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb @@ -28,8 +28,8 @@ def self.create(context) params => { desired_state: String => desired_state, max_hours_before_termination: Integer => max_hours_before_termination, - devfile_ref: String => devfile_ref, - devfile_path: String => devfile_path, + project_ref: String => project_ref, + devfile_path: String | nil => devfile_path, agent: Clusters::Agent => agent, user: User => user, project: Project => project, @@ -42,7 +42,7 @@ def self.create(context) workspace.desired_state = desired_state workspace.actual_state = CREATION_REQUESTED workspace.max_hours_before_termination = max_hours_before_termination - workspace.devfile_ref = devfile_ref + workspace.project_ref = project_ref workspace.devfile_path = devfile_path workspace.devfile = devfile_yaml workspace.processed_devfile = YAML.dump(processed_devfile.deep_stringify_keys) diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index 84b77851fac3709d123d48f1b647a14909da9309..6b9842bb0c1ef81799acb530b6c507bd0983003f 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -19,7 +19,7 @@ deployment_resource_version { 2 } max_hours_before_termination { 24 } - devfile_ref { 'main' } + project_ref { 'main' } devfile_path { '.devfile.yaml' } devfile do 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 b883ab10efbbe5e8164eb711fde022888ef28ebb..973a9109514834d4503614075d04e7f7e0144695 100644 --- a/ee/spec/graphql/types/remote_development/workspace_type_spec.rb +++ b/ee/spec/graphql/types/remote_development/workspace_type_spec.rb @@ -8,7 +8,7 @@ id cluster_agent project_id user name namespace max_hours_before_termination desired_state desired_state_updated_at actual_state responded_to_agent_at url editor devfile_ref devfile_path devfile_web_url devfile processed_devfile - deployment_resource_version desired_config_generator_version + project_ref deployment_resource_version desired_config_generator_version workspaces_agent_config_version force_include_all_resources created_at updated_at ] end diff --git a/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb b/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb index 056c1a99afb69ee363e4afcd5a45c9fe34351927..ac3ba4a0ce1104c3312c5e4e35da36a9ad0ae168 100644 --- a/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb +++ b/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb @@ -10,6 +10,8 @@ { requested_setting_names: requested_setting_names } end + let(:default_devfile_yaml) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE_YAML } + subject(:returned_value) do described_class.init(context) end @@ -24,6 +26,7 @@ :allow_privilege_escalation, :annotations, :default_branch_name, + :default_devfile_yaml, :default_max_hours_before_termination, :default_resources_per_workspace_container, :default_runtime_class, @@ -48,6 +51,7 @@ allow_privilege_escalation: false, annotations: {}, default_branch_name: nil, + default_devfile_yaml: default_devfile_yaml, default_max_hours_before_termination: 24, default_resources_per_workspace_container: {}, default_runtime_class: "", @@ -75,6 +79,7 @@ allow_privilege_escalation: :Boolean, annotations: Hash, default_branch_name: String, + default_devfile_yaml: String, default_max_hours_before_termination: Integer, default_resources_per_workspace_container: Hash, default_runtime_class: String, diff --git a/ee/spec/lib/remote_development/workspace_operations/create/devfile_fetcher_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/devfile_fetcher_spec.rb index dc4fc43559e4257fac2ce7ca8249e1192d622c6c..f7049b8a2971fe561d819ebc4f00ea864e863253 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/devfile_fetcher_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/devfile_fetcher_spec.rb @@ -15,10 +15,7 @@ let_it_be(:project) { create(:project, :in_group, :repository) } let_it_be(:agent) { create(:ee_cluster_agent, :with_existing_workspaces_agent_config) } let(:random_string) { 'abcdef' } - let(:devfile_ref) { 'main' } - let(:devfile_path) { '.devfile.yaml' } - let(:devfile_fixture_name) { 'example.devfile.yaml' } - let(:devfile_yaml) { read_devfile_yaml(devfile_fixture_name) } + let(:project_ref) { 'main' } let(:workspace_root) { '/projects' } let(:params) do { @@ -27,12 +24,21 @@ project: project, max_hours_before_termination: 24, desired_state: RemoteDevelopment::WorkspaceOperations::States::RUNNING, - devfile_ref: devfile_ref, + project_ref: project_ref, devfile_path: devfile_path } end - let(:context) { { params: params } } + let(:default_devfile_yaml) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE_YAML } + + let(:context) do + { + params: params, + settings: { + default_devfile_yaml: default_devfile_yaml + } + } + end before do allow(project.repository).to receive_message_chain(:blob_at_branch, :data) { devfile_yaml } @@ -41,19 +47,50 @@ context 'when params are valid' do let(:devfile) { yaml_safe_load_symbolized(devfile_yaml) } - it 'returns an ok Result containing the original params and the devfile_yaml_string' do - expect(result).to eq( - Gitlab::Fp::Result.ok({ - params: params, - devfile_yaml: devfile_yaml, - devfile: devfile - }) - ) + context 'when devfile_path is points to an existing file' do + let(:devfile_path) { '.devfile.yaml' } + let(:devfile_fixture_name) { 'example.devfile.yaml' } + let(:devfile_yaml) { read_devfile_yaml(devfile_fixture_name) } + + it 'returns an ok Result containing the original params and the devfile_yaml' do + expect(result).to eq( + Gitlab::Fp::Result.ok({ + params: params, + devfile_yaml: devfile_yaml, + devfile: devfile, + settings: { + default_devfile_yaml: default_devfile_yaml + } + }) + ) + end + end + + context 'when devfile_path is nil' do + let(:devfile_path) { nil } + let(:devfile_yaml) { default_devfile_yaml } + + it 'returns an ok Result containing the original params and the default devfile_yaml_string' do + expect(project.repository).not_to receive(:blob_at_branch) + + expect(result).to eq( + Gitlab::Fp::Result.ok({ + params: params, + devfile_yaml: devfile_yaml, + devfile: devfile, + settings: { + default_devfile_yaml: default_devfile_yaml + } + }) + ) + end end end context 'when params are invalid' do context 'when agent has no associated config' do + let(:devfile_path) { '.devfile.yaml' } + let_it_be(:agent) { create(:cluster_agent) } it 'returns an err Result containing error details' do @@ -80,12 +117,31 @@ expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceCreateDevfileLoadFailed) message.content => { details: String => error_details } expect(error_details) - .to eq("Devfile path '#{devfile_path}' at ref '#{devfile_ref}' does not exist in project repository") + .to eq("Devfile path '#{devfile_path}' at ref '#{project_ref}' does not exist in the project repository") + end + end + end + + context 'when devfile_path is empty string' do + let(:devfile_path) { '' } + + before do + allow(project.repository).to receive(:blob_at_branch).and_return(nil) + end + + it 'returns an err Result containing error details' do + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceCreateDevfileLoadFailed) + message.content => { details: String => error_details } + expect(error_details) + .to eq("Devfile path '#{devfile_path}' at ref '#{project_ref}' does not exist in the project repository") end end end context 'when devfile blob data could not be loaded' do + let(:devfile_path) { '.devfile.yaml' } + before do allow(project.repository).to receive_message_chain(:blob_at_branch, :data) { '' } end @@ -100,6 +156,7 @@ end context 'when devfile YAML cannot be loaded' do + let(:devfile_path) { '.devfile.yaml' } let(:devfile_yaml) { "invalid: yaml: boom" } it 'returns an err Result containing error details' do @@ -112,6 +169,7 @@ end context 'when devfile YAML is valid but is invalid JSON' do + let(:devfile_path) { '.devfile.yaml' } let(:devfile_yaml) { "!binary key: value" } it 'returns an err Result containing error details' do diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb index 4cf08dc967b8658002983d1b5cf7d53b1b58150f..0e594b08ee0cb414cd1bf4496290fdb39ef76b03 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb @@ -13,7 +13,7 @@ let(:user) { create(:user) } let(:group) { create(:group, name: 'test-group', developers: user) } let(:random_string) { 'abcdef' } - let(:devfile_ref) { 'master' } + let(:project_ref) { 'master' } let(:devfile_path) { '.devfile.yaml' } let(:devfile_fixture_name) { 'example.devfile.yaml' } let(:devfile_yaml) { read_devfile_yaml(devfile_fixture_name) } @@ -27,8 +27,10 @@ ] end + let(:default_devfile_yaml) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE_YAML } + let(:project) do - files = { devfile_path => devfile_yaml } + files = devfile_path.nil? ? {} : { devfile_path => devfile_yaml } create(:project, :in_group, :custom_repo, path: 'test-project', files: files, namespace: group) end @@ -45,7 +47,7 @@ project: project, max_hours_before_termination: 24, desired_state: RemoteDevelopment::WorkspaceOperations::States::RUNNING, - devfile_ref: devfile_ref, + project_ref: project_ref, devfile_path: devfile_path, variables: variables } @@ -66,7 +68,8 @@ let(:settings) do { project_cloner_image: 'alpine/git:2.45.2', - tools_injector_image: tools_injector_image_from_settings + tools_injector_image: tools_injector_image_from_settings, + default_devfile_yaml: default_devfile_yaml } end @@ -89,15 +92,12 @@ context 'when params are valid' do before do allow(project.repository).to receive_message_chain(:blob_at_branch, :data) { devfile_yaml } + allow(SecureRandom).to receive(:alphanumeric) { random_string } end context 'when devfile is valid' do let(:expected_workspaces_agent_config_version) { 1 } - before do - allow(SecureRandom).to receive(:alphanumeric) { random_string } - end - it 'creates a new workspace and returns success', :aggregate_failures do # NOTE: This example is structured and ordered to give useful and informative error messages in case of failures expect { response }.to change { RemoteDevelopment::Workspace.count }.by(1) @@ -155,6 +155,16 @@ end end + context 'when devfile_path is nil' do + let(:devfile_path) { nil } + + it 'creates a new workspace using default_devfile_yaml from settings' do + workspace = response.fetch(:payload).fetch(:workspace) + + expect(workspace.devfile).to eq(default_devfile_yaml) + end + end + context 'when devfile is not valid', :aggregate_failures do let(:devfile_fixture_name) { 'example.invalid-components-entry-missing-devfile.yaml' } @@ -184,8 +194,8 @@ expect(response).to eq({ status: :error, message: - "Workspace create devfile load failed: Devfile path '#{devfile_path}' at ref '#{devfile_ref}' " \ - "does not exist in project repository", # rubocop:disable Layout/LineEndStringConcatenationIndentation -- RubyMine formatting conflict. See https://gitlab.com/gitlab-org/gitlab/-/issues/442626 + "Workspace create devfile load failed: Devfile path '#{devfile_path}' at ref '#{project_ref}' " \ + "does not exist in the project repository", # rubocop:disable Layout/LineEndStringConcatenationIndentation -- RubyMine formatting conflict. See https://gitlab.com/gitlab-org/gitlab/-/issues/442626 reason: :bad_request }) end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/project_cloner_component_inserter_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/project_cloner_component_inserter_spec.rb index 21d3aa0eef50f420ddef6f9789f5c70d8c566111..0604753d97bbaa13f420dc0840975c3419db1747 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/project_cloner_component_inserter_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/project_cloner_component_inserter_spec.rb @@ -22,7 +22,7 @@ { params: { project: project, - devfile_ref: "master" + project_ref: "master" }, processed_devfile: input_processed_devfile, volume_mounts: { diff --git a/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb index 6ae4e4c7773b9e62b7caf631441c82bdbad475bf..730efa029d480bd02e21302c7d728e5def01da08 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/workspace_creator_spec.rb @@ -25,7 +25,7 @@ project: project, max_hours_before_termination: 24, desired_state: desired_state, - devfile_ref: 'main', + project_ref: 'main', devfile_path: devfile_path } end diff --git a/ee/spec/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index ffc035c78bdfde8199bb6f998a790622482c3c2e..2e8c2e3a5c5046fa8bb255c33efd2be6fba55be7 100644 --- a/ee/spec/models/remote_development/workspace_spec.rb +++ b/ee/spec/models/remote_development/workspace_spec.rb @@ -108,6 +108,16 @@ # noinspection HttpUrlsUsage - suppress RubyMine warning for insecure http link. expect(workspace.devfile_web_url).to eq("http://#{Gitlab.config.gitlab.host}/#{workspace.project.path_with_namespace}/-/blob/main/.devfile.yaml") end + + context 'when devfile_path is nil' do + before do + workspace.devfile_path = nil + end + + it 'returns nil as devfile_web_url' do + expect(workspace.devfile_web_url).to eq(nil) + end + end end describe '.before_save' do diff --git a/ee/spec/requests/api/graphql/mutations/remote_development/workspace_operations/create_spec.rb b/ee/spec/requests/api/graphql/mutations/remote_development/workspace_operations/create_spec.rb index f840680673ed9c3ca36dbf06428b2d0602b841d0..b86b8d572868aacc83f3c875bc1157b592bb7982 100644 --- a/ee/spec/requests/api/graphql/mutations/remote_development/workspace_operations/create_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/remote_development/workspace_operations/create_spec.rb @@ -44,7 +44,7 @@ max_hours_before_termination: 24, cluster_agent_id: agent.to_global_id.to_s, project_id: workspace_project.to_global_id.to_s, - devfile_ref: 'main', + project_ref: 'main', devfile_path: '.devfile.yaml', variables: [ { key: 'VAR1', value: 'value 1', type: 'ENVIRONMENT' }, @@ -176,12 +176,24 @@ def mutation_response end context 'when required arguments are missing' do - let(:mutation_args) { all_mutation_args.except(:desired_state) } + context 'when validates against GraphQL not allow null behaviour' do + let(:mutation_args) { all_mutation_args.except(:desired_state) } - it 'returns error about required argument' do - post_graphql_mutation(mutation, current_user: user) + it 'returns error about required argument' do + post_graphql_mutation(mutation, current_user: user) + + expect_graphql_errors_to_include(/provided invalid value for desiredState \(Expected value to not be null\)/) + end + end - expect_graphql_errors_to_include(/provided invalid value for desiredState \(Expected value to not be null\)/) + context 'when both project_ref and devfile_ref not present' do + let(:mutation_args) { all_mutation_args.except(:project_ref, :devfile_ref) } + + it 'returns error about required argument' do + post_graphql_mutation(mutation, current_user: user) + + expect_graphql_errors_to_include(/Either 'project_ref' or deprecated 'devfile_ref' must be provided./) + end end end diff --git a/ee/spec/requests/remote_development/integration_spec.rb b/ee/spec/requests/remote_development/integration_spec.rb index b0c4a7f26fbad8bce348e84311d6f70a1e3f210e..184059b152b59441970248f5461c82f9d0f22b08 100644 --- a/ee/spec/requests/remote_development/integration_spec.rb +++ b/ee/spec/requests/remote_development/integration_spec.rb @@ -60,7 +60,7 @@ let(:workspace_project_name) { "workspace-project" } let(:workspace_namespace_path) { "#{common_parent_namespace_name}/#{workspace_project_namespace_name}" } let(:random_string) { "abcdef" } - let(:devfile_ref) { "master" } + let(:project_ref) { "master" } let(:devfile_path) { ".devfile.yaml" } let(:devfile_fixture_name) { "example.devfile.yaml" } let(:devfile_yaml) do @@ -272,7 +272,7 @@ def workspace_create_mutation_args(cluster_agent_gid) max_hours_before_termination: 24, cluster_agent_id: cluster_agent_gid, project_id: workspace_project.to_global_id.to_s, - devfile_ref: devfile_ref, + project_ref: project_ref, devfile_path: devfile_path, variables: user_provided_variables.each_with_object([]) do |variable, arr| arr << variable.merge(type: variable[:type].to_s.upcase) diff --git a/spec/migrations/cap_workspaces_max_termination_to_one_year_spec.rb b/spec/migrations/cap_workspaces_max_termination_to_one_year_spec.rb index 57db4ab1188bb2e0977d4a08af184f875c422a02..52c80b41132f6e23b6f99449fbd6352d7ad5b778 100644 --- a/spec/migrations/cap_workspaces_max_termination_to_one_year_spec.rb +++ b/spec/migrations/cap_workspaces_max_termination_to_one_year_spec.rb @@ -45,7 +45,7 @@ desired_state: 'Terminated', actual_state: 'Terminated', editor: 'vs-code', - devfile_ref: 'devfile-ref', + project_ref: 'devfile-ref', devfile_path: 'devfile-path', devfile: 'devfile', processed_devfile: 'processed_dev_file', @@ -68,7 +68,7 @@ desired_state: 'Running', actual_state: 'Running', editor: 'vs-code', - devfile_ref: 'devfile-ref', + project_ref: 'devfile-ref', devfile_path: 'devfile-path', devfile: 'devfile', processed_devfile: 'processed_dev_file',