From 0cebb841090f7473387707b7a4deddd314b7a545 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 4 Nov 2024 13:37:11 +1100 Subject: [PATCH 01/24] Use default devfile with golden image to create workspace --- ...update_workspaces_devfile_path_nullable.rb | 14 +++++++ ...e_workspaces_devfile_ref_to_project_ref.rb | 14 +++++++ ...3_cleanup_workspaces_devfile_ref_rename.rb | 14 +++++++ db/schema_migrations/20241031053532 | 1 + db/schema_migrations/20241031053638 | 1 + db/schema_migrations/20241031053743 | 1 + db/structure.sql | 6 +-- doc/api/graphql/reference/index.md | 10 +++-- .../workspace_operations/create.rb | 22 +++++++++- .../remote_development/workspace_type.rb | 6 ++- ee/app/models/remote_development/workspace.rb | 5 ++- .../settings/default_settings.rb | 14 ++++++- .../create/devfile_fetcher.rb | 42 ++++++++++++------- .../project_cloner_component_inserter.rb | 2 +- .../create/workspace_creator.rb | 6 +-- .../remote_development/workspaces.rb | 2 +- .../remote_development/workspace_type_spec.rb | 2 +- .../settings/settings_initializer_spec.rb | 21 ++++++++-- .../create/devfile_fetcher_spec.rb | 32 ++++++++++++-- .../create/main_integration_spec.rb | 23 +++++++--- .../project_cloner_component_inserter_spec.rb | 2 +- .../create/workspace_creator_spec.rb | 2 +- .../workspace_operations/create_spec.rb | 2 +- .../remote_development/integration_spec.rb | 4 +- ...spaces_max_termination_to_one_year_spec.rb | 4 +- 25 files changed, 199 insertions(+), 53 deletions(-) create mode 100644 db/migrate/20241031053532_update_workspaces_devfile_path_nullable.rb create mode 100644 db/migrate/20241031053638_rename_workspaces_devfile_ref_to_project_ref.rb create mode 100644 db/post_migrate/20241031053743_cleanup_workspaces_devfile_ref_rename.rb create mode 100644 db/schema_migrations/20241031053532 create mode 100644 db/schema_migrations/20241031053638 create mode 100644 db/schema_migrations/20241031053743 diff --git a/db/migrate/20241031053532_update_workspaces_devfile_path_nullable.rb b/db/migrate/20241031053532_update_workspaces_devfile_path_nullable.rb new file mode 100644 index 00000000000000..bfb3ebc66ea672 --- /dev/null +++ b/db/migrate/20241031053532_update_workspaces_devfile_path_nullable.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class UpdateWorkspacesDevfilePathNullable < Gitlab::Database::Migration[2.2] + milestone '17.6' + 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/20241031053638_rename_workspaces_devfile_ref_to_project_ref.rb b/db/migrate/20241031053638_rename_workspaces_devfile_ref_to_project_ref.rb new file mode 100644 index 00000000000000..f7c9952756b7e3 --- /dev/null +++ b/db/migrate/20241031053638_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.6' + 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/20241031053743_cleanup_workspaces_devfile_ref_rename.rb b/db/post_migrate/20241031053743_cleanup_workspaces_devfile_ref_rename.rb new file mode 100644 index 00000000000000..2555202a197be6 --- /dev/null +++ b/db/post_migrate/20241031053743_cleanup_workspaces_devfile_ref_rename.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CleanupWorkspacesDevfileRefRename < Gitlab::Database::Migration[2.2] + milestone '17.6' + 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/20241031053532 b/db/schema_migrations/20241031053532 new file mode 100644 index 00000000000000..fc23633fb84709 --- /dev/null +++ b/db/schema_migrations/20241031053532 @@ -0,0 +1 @@ +0abef21ebab05bf8b4ce3d658c19a4d9585d656089fe09a479e78e026d47614f \ No newline at end of file diff --git a/db/schema_migrations/20241031053638 b/db/schema_migrations/20241031053638 new file mode 100644 index 00000000000000..01b8a5fb4e1c1d --- /dev/null +++ b/db/schema_migrations/20241031053638 @@ -0,0 +1 @@ +16824767cac64dbe34b4f6533aa3def15515faffc61e35a06431d9dbc087c45e \ No newline at end of file diff --git a/db/schema_migrations/20241031053743 b/db/schema_migrations/20241031053743 new file mode 100644 index 00000000000000..9c9477834cfe46 --- /dev/null +++ b/db/schema_migrations/20241031053743 @@ -0,0 +1 @@ +ca36c483ec7be0d58690dcf8376bbaaf356de06e9d7222203a75db9261ba6b61 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e9049f5db8aa1a..4a292969dc4447 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, @@ -22736,11 +22735,12 @@ CREATE TABLE workspaces ( 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 040894605d67c1..a9eb4c4ade8663 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 repo git path containing the devfile used to configure the workspace. | +| `devfileRef` **{warning-solid}** | [`String`](#string) | **Deprecated:** Argument is renamed to project_ref. Deprecated in GitLab 17.6. | | `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 containing the devfile used to configure the workspace. | | `variables` | [`[WorkspaceVariableInput!]`](#workspacevariableinput) | Variables to inject into the workspace. | #### Fields @@ -37985,8 +37986,8 @@ 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. | +| `devfilePath` | [`String`](#string) | Path to the devfile used to configure the workspace. | +| `devfileRef` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.6. Field is renamed to project_ref. | | `devfileWebUrl` | [`String!`](#string) | Web URL of the devfile used to configure the workspace. | | `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. | @@ -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. | | `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 ccf1f7de65ca51..4446470cc5f622 100644 --- a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb +++ b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb @@ -45,12 +45,18 @@ class Create < BaseMutation argument :devfile_ref, GraphQL::Types::String, - required: true, + 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.6' } + + argument :project_ref, + GraphQL::Types::String, + required: false, description: 'Project repo git ref containing the devfile used to configure the workspace.' argument :devfile_path, GraphQL::Types::String, - required: true, + required: false, description: 'Project repo git path containing the devfile used to configure the workspace.' argument :variables, [::Types::RemoteDevelopment::WorkspaceVariableInput], @@ -70,6 +76,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 'devfile_ref' or 'project_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 4900890db3c253..018d02703c1425 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -56,10 +56,14 @@ class WorkspaceType < ::Types::BaseObject null: false, description: 'Number of hours until the workspace automatically terminates.' field :devfile_ref, GraphQL::Types::String, + null: false, description: 'Git reference that contains the devfile used to configure the workspace.', + deprecated: { reason: 'Field is renamed to project_ref', milestone: '17.6' }, 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.' 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.' 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.' diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 0647d0f69da75a..f7e153342b48ba 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.7', remove_after: '2024-11-21' + 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 UPDATE HERE def devfile_web_url - project.http_url_to_repo.gsub(/\.git$/, "/-/blob/#{devfile_ref}/#{devfile_path}") + project.http_url_to_repo.gsub(/\.git$/, "/-/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 3adf0f976d7565..e4c7b8620f532f 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -5,6 +5,17 @@ module Settings class DefaultSettings UNDEFINED = nil + # TODO - UPDATE IMAGE HERE + DEFAULT_DEVFILE = <<~DEVFILE + schemaVersion: 2.2.0 + components: + - name: development-environment + attributes: + gl/inject-editor: true + container: + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspace/ubuntu-22.04:go-1.22" + DEVFILE + # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. # See ../README.md for more details. # @return [Hash] @@ -39,7 +50,8 @@ def self.default_settings ], use_kubernetes_user_namespaces: [false, :Boolean], workspaces_per_user_quota: [-1, Integer], - workspaces_quota: [-1, Integer] + workspaces_quota: [-1, Integer], + default_devfile: [DEFAULT_DEVFILE, String] } end end 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 113043a7349ab8..db0f7d40527422 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,17 @@ class DevfileFetcher # @param [Hash] context # @return [Gitlab::Fp::Result] def self.fetch(context) - context => { params: Hash => params } + context => { + params: Hash => params, + settings: { + default_devfile: String => default_devfile + } + } params => { agent: Clusters::Agent => agent, project: Project => project, - devfile_ref: String => devfile_ref, - devfile_path: String => devfile_path + project_ref: String => project_ref, + devfile_path: String | nil => devfile_path } unless agent.unversioned_latest_workspaces_agent_config @@ -26,23 +31,28 @@ def self.fetch(context) )) end - repository = project.repository + devfile_yaml = if devfile_path.nil? + default_devfile + else + repository = project.repository - devfile_blob = repository.blob_at_branch(devfile_ref, devfile_path) + 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" - )) - end + unless devfile_blob + return Gitlab::Fp::Result.err(WorkspaceCreateDevfileLoadFailed.new( + details: "Devfile path '#{devfile_path}' at ref '#{project_ref}' " \ + "does not exist in the project repository" + )) + end - devfile_yaml = devfile_blob.data + unless devfile_blob.data.present? + return Gitlab::Fp::Result.err( + WorkspaceCreateDevfileLoadFailed.new(details: "Devfile could not be loaded from project") + ) + end - unless devfile_yaml.present? - return Gitlab::Fp::Result.err( - WorkspaceCreateDevfileLoadFailed.new(details: "Devfile could not be loaded from project") - ) - end + devfile_blob.data + end begin # load YAML, convert YAML to JSON and load it again to remove YAML vulnerabilities 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 d8088778601ba4..e007bc399b5af6 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 @@ -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 aac3029f1917d1..75ae6bbac32ebc 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 84b77851fac370..6b9842bb0c1ef8 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 b883ab10efbbe5..973a9109514834 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 056c1a99afb69e..a79c89aaa9c74b 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,18 @@ { requested_setting_names: requested_setting_names } end + let(:default_devfile_text) do + <<~DEVFILE + schemaVersion: 2.2.0 + components: + - name: development-environment + attributes: + gl/inject-editor: true + container: + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspace/ubuntu-22.04:go-1.22" + DEVFILE + end + subject(:returned_value) do described_class.init(context) end @@ -42,7 +54,8 @@ :tools_injector_image, :use_kubernetes_user_namespaces, :workspaces_per_user_quota, - :workspaces_quota + :workspaces_quota, + :default_devfile ].sort, settings: { allow_privilege_escalation: false, @@ -69,7 +82,8 @@ tools_injector_image: "registry.gitlab.com/gitlab-org/remote-development/gitlab-workspaces-tools:2.0.0", use_kubernetes_user_namespaces: false, workspaces_per_user_quota: -1, - workspaces_quota: -1 + workspaces_quota: -1, + default_devfile: default_devfile_text }, setting_types: { allow_privilege_escalation: :Boolean, @@ -93,7 +107,8 @@ tools_injector_image: String, use_kubernetes_user_namespaces: :Boolean, workspaces_per_user_quota: Integer, - workspaces_quota: Integer + workspaces_quota: Integer, + default_devfile: String }, env_var_prefix: "GITLAB_REMOTE_DEVELOPMENT", env_var_failed_message_class: RemoteDevelopment::Settings::Messages::SettingsEnvironmentVariableOverrideFailed 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 dc4fc43559e425..9153075b52c9ee 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,7 +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(:project_ref) { 'main' } let(:devfile_path) { '.devfile.yaml' } let(:devfile_fixture_name) { 'example.devfile.yaml' } let(:devfile_yaml) { read_devfile_yaml(devfile_fixture_name) } @@ -27,12 +27,19 @@ 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(:context) do + { + params: params, + settings: { + default_devfile: default_devfile + } + } + end before do allow(project.repository).to receive_message_chain(:blob_at_branch, :data) { devfile_yaml } @@ -80,7 +87,24 @@ 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 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 4cf08dc967b865..c14009ac49bd98 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,6 +27,18 @@ ] end + let(:default_devfile_text) do + <<~DEVFILE + schemaVersion: 2.2.0 + components: + - name: development-environment + attributes: + gl/inject-editor: true + container: + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspace/ubuntu-22.04:go-1.22" + DEVFILE + end + let(:project) do files = { devfile_path => devfile_yaml } create(:project, :in_group, :custom_repo, path: 'test-project', files: files, namespace: group) @@ -45,7 +57,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 +78,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: default_devfile_text } end @@ -184,8 +197,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 21d3aa0eef50f4..0604753d97bbaa 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 6ae4e4c7773b9e..730efa029d480b 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/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 f840680673ed9c..dff4112cd3f08d 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' }, diff --git a/ee/spec/requests/remote_development/integration_spec.rb b/ee/spec/requests/remote_development/integration_spec.rb index b0c4a7f26fbad8..184059b152b594 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 57db4ab1188bb2..52c80b41132f6e 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', -- GitLab From 4093741aaa54fac10782d4ebfb904fc5abbc0525 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 4 Nov 2024 14:46:48 +1100 Subject: [PATCH 02/24] update devfile_web_url in model --- ee/app/models/remote_development/workspace.rb | 3 +-- .../create/main_integration_spec.rb | 17 ++++++++++++----- .../models/remote_development/workspace_spec.rb | 10 ++++++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index f7e153342b48ba..656a92ac101349 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -155,9 +155,8 @@ def url query: url_query_string).to_s end - # TODO UPDATE HERE def devfile_web_url - project.http_url_to_repo.gsub(/\.git$/, "/-/blob/#{project_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/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 c14009ac49bd98..ca4b00f7964c3f 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 @@ -40,7 +40,7 @@ end 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 @@ -102,15 +102,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) @@ -168,6 +165,16 @@ end end + context 'when devfile_path is nil' do + let(:devfile_path) { nil } + + it 'creates a new workspace using default_devfile from settings' do + workspace = response.fetch(:payload).fetch(:workspace) + + expect(workspace.devfile).to eq(default_devfile_text) + end + end + context 'when devfile is not valid', :aggregate_failures do let(:devfile_fixture_name) { 'example.invalid-components-entry-missing-devfile.yaml' } diff --git a/ee/spec/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index ffc035c78bdfde..2e8c2e3a5c5046 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 -- GitLab From e306769f42765dbc1c23e6a26cf98378ec5d8e06 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 4 Nov 2024 20:03:39 +1100 Subject: [PATCH 03/24] update devfile_web_url graphql nullable --- doc/api/graphql/reference/index.md | 2 +- ee/app/graphql/types/remote_development/workspace_type.rb | 2 +- ee/lib/remote_development/settings/default_settings.rb | 2 +- .../remote_development/settings/settings_initializer_spec.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index a9eb4c4ade8663..0e0e70fedd5dda 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -37988,7 +37988,7 @@ Represents a remote development workspace. | `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` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.6. Field is renamed to project_ref. | -| `devfileWebUrl` | [`String!`](#string) | Web URL of the devfile used to configure the workspace. | +| `devfileWebUrl` | [`String`](#string) | Web URL of the devfile used to configure the workspace. | | `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. | diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 018d02703c1425..7cd0008df7dfd1 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -66,7 +66,7 @@ class WorkspaceType < ::Types::BaseObject null: true, description: 'Path to the devfile used to configure the workspace.' 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.' field :devfile, GraphQL::Types::String, null: false, description: 'Source YAML of the devfile used to configure the workspace.' diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index e4c7b8620f532f..03c2e501bd9d2f 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -13,7 +13,7 @@ class DefaultSettings attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspace/ubuntu-22.04:go-1.22" + image: "registry.gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/ubuntu:22.04" DEVFILE # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. 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 a79c89aaa9c74b..54a2df166d791f 100644 --- a/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb +++ b/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb @@ -18,7 +18,7 @@ attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspace/ubuntu-22.04:go-1.22" + image: "registry.gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/ubuntu:22.04" DEVFILE end -- GitLab From d5ea2355841da09989bfa62f685051d413d99918 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Fri, 8 Nov 2024 14:43:00 +1100 Subject: [PATCH 04/24] address comments --- doc/api/graphql/reference/index.md | 6 +++--- .../remote_development/workspace_operations/create.rb | 4 ++-- ee/app/graphql/types/remote_development/workspace_type.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 0e0e70fedd5dda..8b6023e4da7a8d 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11613,12 +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. | +| `devfilePath` | [`String`](#string) | Project path containing the devfile used to configure the workspace. | | `devfileRef` **{warning-solid}** | [`String`](#string) | **Deprecated:** Argument is renamed to project_ref. Deprecated in GitLab 17.6. | | `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 containing the devfile used to configure the workspace. | +| `projectRef` | [`String`](#string) | Project repo git ref. | | `variables` | [`[WorkspaceVariableInput!]`](#workspacevariableinput) | Variables to inject into the workspace. | #### Fields @@ -37997,7 +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. | +| `projectRef` | [`String!`](#string) | Project repo git ref. | | `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 4446470cc5f622..4424b29a1b0b65 100644 --- a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb +++ b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb @@ -52,12 +52,12 @@ class Create < BaseMutation argument :project_ref, GraphQL::Types::String, required: false, - description: 'Project repo git ref containing the devfile used to configure the workspace.' + description: 'Project repo git ref.' argument :devfile_path, GraphQL::Types::String, required: false, - description: 'Project repo git path containing the devfile used to configure the workspace.' + description: 'Project path containing the devfile used to configure the workspace.' argument :variables, [::Types::RemoteDevelopment::WorkspaceVariableInput], required: false, diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 7cd0008df7dfd1..88bfd4c342d63b 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -60,7 +60,7 @@ class WorkspaceType < ::Types::BaseObject deprecated: { reason: 'Field is renamed to project_ref', milestone: '17.6' }, 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.' + null: false, description: 'Project repo git ref.' field :devfile_path, GraphQL::Types::String, null: true, description: 'Path to the devfile used to configure the workspace.' -- GitLab From 9cbf4379fa85a82c845974181800866a6656f301 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 11 Nov 2024 12:52:41 +1100 Subject: [PATCH 05/24] address comments --- doc/api/graphql/reference/index.md | 2 +- .../workspace_operations/create.rb | 2 +- .../remote_development/workspace_type.rb | 4 +++- ee/app/models/remote_development/workspace.rb | 1 + .../settings/default_settings.rb | 1 + .../workspace_operations/create_spec.rb | 20 +++++++++++++++---- 6 files changed, 23 insertions(+), 7 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 8b6023e4da7a8d..a6c5381acbc1aa 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -37988,7 +37988,7 @@ Represents a remote development workspace. | `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` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.6. Field is renamed to project_ref. | -| `devfileWebUrl` | [`String`](#string) | Web URL of the devfile used to configure the workspace. | +| `devfileWebUrl` **{warning-solid}** | [`String`](#string) | **Deprecated** in GitLab 17.6. 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. | 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 4424b29a1b0b65..156681e0df69c9 100644 --- a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb +++ b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb @@ -80,7 +80,7 @@ def resolve(args) # raising an error if neither is present. unless args[:devfile_ref] || args[:project_ref] raise ::Gitlab::Graphql::Errors::ArgumentError, - "Either 'devfile_ref' or 'project_ref' must be provided." + "Either 'project_ref' or deprecated 'devfile_ref' must be provided." end # Remove 'devfile_ref' from the arguments. diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 88bfd4c342d63b..26f1055647d8d4 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -65,8 +65,10 @@ class WorkspaceType < ::Types::BaseObject field :devfile_path, GraphQL::Types::String, 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: true, 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.6' } 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 656a92ac101349..c2adec75f4cb59 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -155,6 +155,7 @@ 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 devfile_path.nil? ? nil : project.http_url_to_repo.gsub(/\.git\Z/, "/-/blob/#{project_ref}/#{devfile_path}") end diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 03c2e501bd9d2f..dc2c29e4736bfa 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -6,6 +6,7 @@ class DefaultSettings UNDEFINED = nil # TODO - UPDATE IMAGE HERE + # When updating DEFAULT_DEVFILE, pls also update the user facing doc _LINK_ DEFAULT_DEVFILE = <<~DEVFILE schemaVersion: 2.2.0 components: 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 dff4112cd3f08d..b86b8d572868aa 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 @@ -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 -- GitLab From 78adafd45a69bd340a1110307631012d2115aeca Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Thu, 14 Nov 2024 15:31:10 +1100 Subject: [PATCH 06/24] fix failing rspec --- .../remote_development/settings/default_settings.rb | 4 ++-- .../settings/settings_initializer_spec.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index dc2c29e4736bfa..350a6ad583e24e 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -28,6 +28,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: [DEFAULT_DEVFILE, String], default_max_hours_before_termination: [24, Integer], default_resources_per_workspace_container: [{}, Hash], default_runtime_class: ["", String], @@ -51,8 +52,7 @@ def self.default_settings ], use_kubernetes_user_namespaces: [false, :Boolean], workspaces_per_user_quota: [-1, Integer], - workspaces_quota: [-1, Integer], - default_devfile: [DEFAULT_DEVFILE, String] + workspaces_quota: [-1, Integer] } end 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 54a2df166d791f..c75597921c6231 100644 --- a/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb +++ b/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb @@ -36,6 +36,7 @@ :allow_privilege_escalation, :annotations, :default_branch_name, + :default_devfile, :default_max_hours_before_termination, :default_resources_per_workspace_container, :default_runtime_class, @@ -54,13 +55,13 @@ :tools_injector_image, :use_kubernetes_user_namespaces, :workspaces_per_user_quota, - :workspaces_quota, - :default_devfile + :workspaces_quota ].sort, settings: { allow_privilege_escalation: false, annotations: {}, default_branch_name: nil, + default_devfile: default_devfile_text, default_max_hours_before_termination: 24, default_resources_per_workspace_container: {}, default_runtime_class: "", @@ -82,13 +83,13 @@ tools_injector_image: "registry.gitlab.com/gitlab-org/remote-development/gitlab-workspaces-tools:2.0.0", use_kubernetes_user_namespaces: false, workspaces_per_user_quota: -1, - workspaces_quota: -1, - default_devfile: default_devfile_text + workspaces_quota: -1 }, setting_types: { allow_privilege_escalation: :Boolean, annotations: Hash, default_branch_name: String, + default_devfile: String, default_max_hours_before_termination: Integer, default_resources_per_workspace_container: Hash, default_runtime_class: String, @@ -107,8 +108,7 @@ tools_injector_image: String, use_kubernetes_user_namespaces: :Boolean, workspaces_per_user_quota: Integer, - workspaces_quota: Integer, - default_devfile: String + workspaces_quota: Integer }, env_var_prefix: "GITLAB_REMOTE_DEVELOPMENT", env_var_failed_message_class: RemoteDevelopment::Settings::Messages::SettingsEnvironmentVariableOverrideFailed -- GitLab From ae84472bc3abb3df4ecb82a0ba9aeac747083df3 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 18 Nov 2024 17:17:13 +1100 Subject: [PATCH 07/24] update milestone to 17.7 --- ...241118061025_update_workspaces_devfile_path_nullable.rb} | 2 +- ...8061030_rename_workspaces_devfile_ref_to_project_ref.rb} | 2 +- ...20241118061130_cleanup_workspaces_devfile_ref_rename.rb} | 2 +- db/schema_migrations/20241031053532 | 1 - db/schema_migrations/20241031053638 | 1 - db/schema_migrations/20241031053743 | 1 - db/schema_migrations/20241118061025 | 1 + db/schema_migrations/20241118061030 | 1 + db/schema_migrations/20241118061130 | 1 + doc/api/graphql/reference/index.md | 6 +++--- .../remote_development/workspace_operations/create.rb | 2 +- ee/app/graphql/types/remote_development/workspace_type.rb | 4 ++-- 12 files changed, 12 insertions(+), 12 deletions(-) rename db/migrate/{20241031053532_update_workspaces_devfile_path_nullable.rb => 20241118061025_update_workspaces_devfile_path_nullable.rb} (93%) rename db/migrate/{20241031053638_rename_workspaces_devfile_ref_to_project_ref.rb => 20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb} (94%) rename db/post_migrate/{20241031053743_cleanup_workspaces_devfile_ref_rename.rb => 20241118061130_cleanup_workspaces_devfile_ref_rename.rb} (94%) delete mode 100644 db/schema_migrations/20241031053532 delete mode 100644 db/schema_migrations/20241031053638 delete mode 100644 db/schema_migrations/20241031053743 create mode 100644 db/schema_migrations/20241118061025 create mode 100644 db/schema_migrations/20241118061030 create mode 100644 db/schema_migrations/20241118061130 diff --git a/db/migrate/20241031053532_update_workspaces_devfile_path_nullable.rb b/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb similarity index 93% rename from db/migrate/20241031053532_update_workspaces_devfile_path_nullable.rb rename to db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb index bfb3ebc66ea672..29701db5fd88c0 100644 --- a/db/migrate/20241031053532_update_workspaces_devfile_path_nullable.rb +++ b/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class UpdateWorkspacesDevfilePathNullable < Gitlab::Database::Migration[2.2] - milestone '17.6' + milestone '17.7' disable_ddl_transaction! def up diff --git a/db/migrate/20241031053638_rename_workspaces_devfile_ref_to_project_ref.rb b/db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb similarity index 94% rename from db/migrate/20241031053638_rename_workspaces_devfile_ref_to_project_ref.rb rename to db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb index f7c9952756b7e3..e6a89f70486214 100644 --- a/db/migrate/20241031053638_rename_workspaces_devfile_ref_to_project_ref.rb +++ b/db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class RenameWorkspacesDevfileRefToProjectRef < Gitlab::Database::Migration[2.2] - milestone '17.6' + milestone '17.7' disable_ddl_transaction! def up diff --git a/db/post_migrate/20241031053743_cleanup_workspaces_devfile_ref_rename.rb b/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb similarity index 94% rename from db/post_migrate/20241031053743_cleanup_workspaces_devfile_ref_rename.rb rename to db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb index 2555202a197be6..fe1cd46d3f8fee 100644 --- a/db/post_migrate/20241031053743_cleanup_workspaces_devfile_ref_rename.rb +++ b/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class CleanupWorkspacesDevfileRefRename < Gitlab::Database::Migration[2.2] - milestone '17.6' + milestone '17.7' disable_ddl_transaction! def up diff --git a/db/schema_migrations/20241031053532 b/db/schema_migrations/20241031053532 deleted file mode 100644 index fc23633fb84709..00000000000000 --- a/db/schema_migrations/20241031053532 +++ /dev/null @@ -1 +0,0 @@ -0abef21ebab05bf8b4ce3d658c19a4d9585d656089fe09a479e78e026d47614f \ No newline at end of file diff --git a/db/schema_migrations/20241031053638 b/db/schema_migrations/20241031053638 deleted file mode 100644 index 01b8a5fb4e1c1d..00000000000000 --- a/db/schema_migrations/20241031053638 +++ /dev/null @@ -1 +0,0 @@ -16824767cac64dbe34b4f6533aa3def15515faffc61e35a06431d9dbc087c45e \ No newline at end of file diff --git a/db/schema_migrations/20241031053743 b/db/schema_migrations/20241031053743 deleted file mode 100644 index 9c9477834cfe46..00000000000000 --- a/db/schema_migrations/20241031053743 +++ /dev/null @@ -1 +0,0 @@ -ca36c483ec7be0d58690dcf8376bbaaf356de06e9d7222203a75db9261ba6b61 \ No newline at end of file diff --git a/db/schema_migrations/20241118061025 b/db/schema_migrations/20241118061025 new file mode 100644 index 00000000000000..742a591c8ecd2a --- /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 00000000000000..2dba8d5e77a89c --- /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 00000000000000..a0981989edec9f --- /dev/null +++ b/db/schema_migrations/20241118061130 @@ -0,0 +1 @@ +32e2325b1442e77a35af0544cdc9a6ea7fab37d4f9185264923dcdc574da758e \ No newline at end of file diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index a6c5381acbc1aa..111ee634f1a6bc 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11614,7 +11614,7 @@ Input type: `WorkspaceCreateInput` | `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 path containing the devfile used to configure the workspace. | -| `devfileRef` **{warning-solid}** | [`String`](#string) | **Deprecated:** Argument is renamed to project_ref. Deprecated in GitLab 17.6. | +| `devfileRef` **{warning-solid}** | [`String`](#string) | **Deprecated:** Argument is renamed to project_ref. Deprecated in GitLab 17.7. | | `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. | @@ -37987,8 +37987,8 @@ Represents a remote development 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` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.6. Field is renamed to project_ref. | -| `devfileWebUrl` **{warning-solid}** | [`String`](#string) | **Deprecated** in GitLab 17.6. Field is not used. | +| `devfileRef` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.7. Field is renamed to project_ref. | +| `devfileWebUrl` **{warning-solid}** | [`String`](#string) | **Deprecated** in GitLab 17.7. 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. | 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 156681e0df69c9..7768ff06449606 100644 --- a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb +++ b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb @@ -47,7 +47,7 @@ class Create < BaseMutation GraphQL::Types::String, 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.6' } + deprecated: { reason: 'Argument is renamed to project_ref', milestone: '17.7' } argument :project_ref, GraphQL::Types::String, diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 26f1055647d8d4..3021225f223421 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -57,7 +57,7 @@ class WorkspaceType < ::Types::BaseObject field :devfile_ref, GraphQL::Types::String, null: false, description: 'Git reference that contains the devfile used to configure the workspace.', - deprecated: { reason: 'Field is renamed to project_ref', milestone: '17.6' }, method: :project_ref + deprecated: { reason: 'Field is renamed to project_ref', milestone: '17.7' }, 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: 'Project repo git ref.' @@ -68,7 +68,7 @@ class WorkspaceType < ::Types::BaseObject # 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: true, description: 'Web URL of the devfile used to configure the workspace.', - deprecated: { reason: 'Field is not used', milestone: '17.6' } + deprecated: { reason: 'Field is not used', milestone: '17.7' } field :devfile, GraphQL::Types::String, null: false, description: 'Source YAML of the devfile used to configure the workspace.' -- GitLab From 35eaa189080c98e94b133d0eff13007d32781965 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Wed, 20 Nov 2024 10:52:17 +1100 Subject: [PATCH 08/24] address comment --- ee/app/models/remote_development/workspace.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index c2adec75f4cb59..693dcd61a65c32 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -157,7 +157,7 @@ def url # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/503465 - Remove in 19.0 def devfile_web_url - devfile_path.nil? ? nil : project.http_url_to_repo.gsub(/\.git\Z/, "/-/blob/#{project_ref}/#{devfile_path}") + devfile_path.nil? ? nil : project.http_url_to_repo.gsub(/\.git$/, "/-/blob/#{project_ref}/#{devfile_path}") end private -- GitLab From c273e0c45f6ea96c6ada432a3a8ab8b29d08eb98 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 25 Nov 2024 12:11:00 +1100 Subject: [PATCH 09/24] Address comments --- doc/api/graphql/reference/index.md | 2 +- .../workspace_operations/create.rb | 3 ++- .../remote_development/settings/default_settings.rb | 3 ++- .../workspace_operations/create/devfile_fetcher.rb | 7 +++++++ .../settings/settings_initializer_spec.rb | 12 +----------- .../create/main_integration_spec.rb | 12 +----------- 6 files changed, 14 insertions(+), 25 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 111ee634f1a6bc..c43196a2bb4963 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11613,7 +11613,7 @@ 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 path 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.7. | | `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. | 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 7768ff06449606..926e6b93852ea1 100644 --- a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb +++ b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb @@ -57,7 +57,8 @@ class Create < BaseMutation argument :devfile_path, GraphQL::Types::String, required: false, - description: 'Project path containing the devfile used to configure the workspace.' + 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, diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 350a6ad583e24e..a22d5fc12e3593 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -6,7 +6,8 @@ class DefaultSettings UNDEFINED = nil # TODO - UPDATE IMAGE HERE - # When updating DEFAULT_DEVFILE, pls also update the user facing doc _LINK_ + # When updating DEFAULT_DEVFILE, pls also update the user facing doc + # https://docs.gitlab.com/ee/user/workspace/#devfault-devfile DEFAULT_DEVFILE = <<~DEVFILE schemaVersion: 2.2.0 components: 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 db0f7d40527422..5b8f0fc19cf5e8 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb @@ -12,6 +12,13 @@ class DevfileFetcher # @param [Hash] context # @return [Gitlab::Fp::Result] def self.fetch(context) + Gitlab::Fp::Result.ok(context) + .and_then(method(:fetch_devfile)) + end + + # @param [Hash] context + # @return [Gitlab::Fp::Result] + def self.fetch_devfile(context) context => { params: Hash => params, settings: { 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 c75597921c6231..1a81f7453f2fdf 100644 --- a/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb +++ b/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb @@ -10,17 +10,7 @@ { requested_setting_names: requested_setting_names } end - let(:default_devfile_text) do - <<~DEVFILE - schemaVersion: 2.2.0 - components: - - name: development-environment - attributes: - gl/inject-editor: true - container: - image: "registry.gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/ubuntu:22.04" - DEVFILE - end + let(:default_devfile_text) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE } subject(:returned_value) do described_class.init(context) 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 ca4b00f7964c3f..55c43c5c174009 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 @@ -27,17 +27,7 @@ ] end - let(:default_devfile_text) do - <<~DEVFILE - schemaVersion: 2.2.0 - components: - - name: development-environment - attributes: - gl/inject-editor: true - container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspace/ubuntu-22.04:go-1.22" - DEVFILE - end + let(:default_devfile_text) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE } let(:project) do files = devfile_path.nil? ? {} : { devfile_path => devfile_yaml } -- GitLab From ada762d734b761ea8f46752f86bb38879f96c9cd Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 25 Nov 2024 14:09:19 +1100 Subject: [PATCH 10/24] Fix failing rspec --- .../workspace_operations/create/devfile_fetcher.rb | 1 + 1 file changed, 1 insertion(+) 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 5b8f0fc19cf5e8..8d2e8c8ed70f6d 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb @@ -79,6 +79,7 @@ def self.fetch_devfile(context) devfile: devfile })) end + private_class_method :fetch_devfile end end end -- GitLab From 3d1d760334a4ee8928042f25c8ad88e4dd6a9575 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Tue, 3 Dec 2024 21:22:32 +1100 Subject: [PATCH 11/24] update default devfile --- ee/app/models/remote_development/workspace.rb | 2 +- ee/lib/remote_development/settings/default_settings.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 693dcd61a65c32..c2adec75f4cb59 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -157,7 +157,7 @@ def url # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/503465 - Remove in 19.0 def devfile_web_url - devfile_path.nil? ? nil : project.http_url_to_repo.gsub(/\.git$/, "/-/blob/#{project_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 a22d5fc12e3593..095926c650c91a 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -5,7 +5,6 @@ module Settings class DefaultSettings UNDEFINED = nil - # TODO - UPDATE IMAGE HERE # When updating DEFAULT_DEVFILE, pls also update the user facing doc # https://docs.gitlab.com/ee/user/workspace/#devfault-devfile DEFAULT_DEVFILE = <<~DEVFILE @@ -15,7 +14,7 @@ class DefaultSettings attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/ubuntu:22.04" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04:golang-1.23" DEVFILE # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. -- GitLab From 4c22f1bc188da4cbcbdddd3b4b696781db555139 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Wed, 11 Dec 2024 15:26:00 +1100 Subject: [PATCH 12/24] update --- .../create/project_cloner_component_inserter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e007bc399b5af6..625eb1e86aa4ad 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, -- GitLab From 9ec37dd478aad7afd40c03ae81eebf0001ed020b Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Thu, 12 Dec 2024 09:35:58 +1100 Subject: [PATCH 13/24] fix rebase error --- db/structure.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/db/structure.sql b/db/structure.sql index 4a292969dc4447..729a5d4bca4cfb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22731,6 +22731,7 @@ 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)), -- GitLab From 92951f75aa020a3577094b84011a41f1ad064454 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Thu, 12 Dec 2024 23:18:26 +1100 Subject: [PATCH 14/24] update image to digest based --- ee/lib/remote_development/settings/default_settings.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 095926c650c91a..8db81b556bd1ba 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -14,7 +14,7 @@ class DefaultSettings attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04:golang-1.23" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" DEVFILE # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. -- GitLab From 9851dd56889ebb55c0e2b2d85e15a0a639a726fa Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Thu, 12 Dec 2024 23:28:07 +1100 Subject: [PATCH 15/24] Update image with tag and digest --- ee/lib/remote_development/settings/default_settings.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 8db81b556bd1ba..25f946c7d545c0 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -7,6 +7,11 @@ class DefaultSettings # When updating DEFAULT_DEVFILE, pls also update the user facing doc # https://docs.gitlab.com/ee/user/workspace/#devfault-devfile + # For image we use, it is pinned to linux/amd64 digest, instead of the tag digest. + # Rancher Desktop will pull the lniux/arm64 version. + # But there is not compatible architecture for gitlab-workspaces-tools yet and thus the workspace won't start. + # This would be temporary fix, and we will work on the proper solution soon. + # https://gitlab.com/gitlab-org/workspaces/gitlab-workspaces-tools/-/issues/12 DEFAULT_DEVFILE = <<~DEVFILE schemaVersion: 2.2.0 components: @@ -14,7 +19,7 @@ class DefaultSettings attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04:golang-1.23@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" DEVFILE # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. -- GitLab From 7043b2cd9a9d29598b6e85f9046c7dfb2cd17f0a Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 16 Dec 2024 23:42:26 +1100 Subject: [PATCH 16/24] Address Comment --- .../remote_development/settings/default_settings.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 25f946c7d545c0..6982cb9ff6c34e 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -5,13 +5,14 @@ module Settings class DefaultSettings UNDEFINED = nil - # When updating DEFAULT_DEVFILE, pls also update the user facing doc + # When updating DEFAULT_DEVFILE, update the user facing doc as well. # https://docs.gitlab.com/ee/user/workspace/#devfault-devfile - # For image we use, it is pinned to linux/amd64 digest, instead of the tag digest. - # Rancher Desktop will pull the lniux/arm64 version. - # But there is not compatible architecture for gitlab-workspaces-tools yet and thus the workspace won't start. - # This would be temporary fix, and we will work on the proper solution soon. - # https://gitlab.com/gitlab-org/workspaces/gitlab-workspaces-tools/-/issues/12 + # + # 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 = <<~DEVFILE schemaVersion: 2.2.0 components: -- GitLab From 280f5784e29b3907f966c71cee5b4e152a937660 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Mon, 16 Dec 2024 23:44:17 +1100 Subject: [PATCH 17/24] Update DB milestone to 17.8 --- .../20241118061025_update_workspaces_devfile_path_nullable.rb | 2 +- ...241118061030_rename_workspaces_devfile_ref_to_project_ref.rb | 2 +- .../20241118061130_cleanup_workspaces_devfile_ref_rename.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb b/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb index 29701db5fd88c0..1aa34861e08aeb 100644 --- a/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb +++ b/db/migrate/20241118061025_update_workspaces_devfile_path_nullable.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class UpdateWorkspacesDevfilePathNullable < Gitlab::Database::Migration[2.2] - milestone '17.7' + milestone '17.8' disable_ddl_transaction! def up 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 index e6a89f70486214..d8d8eea02534af 100644 --- a/db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb +++ b/db/migrate/20241118061030_rename_workspaces_devfile_ref_to_project_ref.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class RenameWorkspacesDevfileRefToProjectRef < Gitlab::Database::Migration[2.2] - milestone '17.7' + milestone '17.8' disable_ddl_transaction! def up diff --git a/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb b/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb index fe1cd46d3f8fee..217f723f4bc281 100644 --- a/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb +++ b/db/post_migrate/20241118061130_cleanup_workspaces_devfile_ref_rename.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class CleanupWorkspacesDevfileRefRename < Gitlab::Database::Migration[2.2] - milestone '17.7' + milestone '17.8' disable_ddl_transaction! def up -- GitLab From b21c3aee023cdd2c1767dd4f9839f486188fd673 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Wed, 18 Dec 2024 16:16:25 +1100 Subject: [PATCH 18/24] Address comment and update image url --- doc/api/graphql/reference/index.md | 6 +++--- .../remote_development/workspace_operations/create.rb | 3 ++- ee/app/graphql/types/remote_development/workspace_type.rb | 5 +++-- ee/lib/remote_development/settings/default_settings.rb | 6 +++++- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index c43196a2bb4963..fe09cb34a0a212 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11614,7 +11614,7 @@ Input type: `WorkspaceCreateInput` | `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 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.7. | +| `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. | @@ -37987,8 +37987,8 @@ Represents a remote development 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` **{warning-solid}** | [`String!`](#string) | **Deprecated** in GitLab 17.7. Field is renamed to project_ref. | -| `devfileWebUrl` **{warning-solid}** | [`String`](#string) | **Deprecated** in GitLab 17.7. Field is not used. | +| `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. | 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 926e6b93852ea1..f9944cf78f2b7e 100644 --- a/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb +++ b/ee/app/graphql/mutations/remote_development/workspace_operations/create.rb @@ -43,11 +43,12 @@ 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: false, description: 'Project repo git ref containing the devfile used to configure the workspace.', - deprecated: { reason: 'Argument is renamed to project_ref', milestone: '17.7' } + deprecated: { reason: 'Argument is renamed to project_ref', milestone: '17.8' } argument :project_ref, GraphQL::Types::String, diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 3021225f223421..494d9dde60cacd 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -55,9 +55,10 @@ 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.', - deprecated: { reason: 'Field is renamed to project_ref', milestone: '17.7' }, method: :project_ref + 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: 'Project repo git ref.' @@ -68,7 +69,7 @@ class WorkspaceType < ::Types::BaseObject # 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: true, description: 'Web URL of the devfile used to configure the workspace.', - deprecated: { reason: 'Field is not used', milestone: '17.7' } + 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/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 6982cb9ff6c34e..f504b0f1dc88df 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -8,6 +8,10 @@ class DefaultSettings # When updating DEFAULT_DEVFILE, update the user facing doc as well. # https://docs.gitlab.com/ee/user/workspace/#devfault-devfile # + # Woekspace Golden Image is published in the repo gitlab-build-images. + # https://gitlab.com/gitlab-org/gitlab-build-images/container_registry/8089199 + # Tag golang-1.23 is tag used, and its digest is pinned below. + # # 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 @@ -20,7 +24,7 @@ class DefaultSettings attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04:golang-1.23@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" DEVFILE # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. -- GitLab From 2cceaa56da8025fc827eb8eaffb34bd003acb447 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Wed, 18 Dec 2024 16:39:12 +1100 Subject: [PATCH 19/24] Revert image tag into image reference --- ee/lib/remote_development/settings/default_settings.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index f504b0f1dc88df..6982cb9ff6c34e 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -8,10 +8,6 @@ class DefaultSettings # When updating DEFAULT_DEVFILE, update the user facing doc as well. # https://docs.gitlab.com/ee/user/workspace/#devfault-devfile # - # Woekspace Golden Image is published in the repo gitlab-build-images. - # https://gitlab.com/gitlab-org/gitlab-build-images/container_registry/8089199 - # Tag golang-1.23 is tag used, and its digest is pinned below. - # # 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 @@ -24,7 +20,7 @@ class DefaultSettings attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04:golang-1.23@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" DEVFILE # ALL REMOTE DEVELOPMENT SETTINGS MUST BE DECLARED HERE. -- GitLab From b3e1b4e281216ffbba2d49de41d01a1b4a0397a1 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Tue, 7 Jan 2025 19:09:38 +1100 Subject: [PATCH 20/24] update default image digest to latest --- ee/lib/remote_development/settings/default_settings.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index 6982cb9ff6c34e..ff25fd5c0f97b2 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -20,7 +20,7 @@ class DefaultSettings attributes: gl/inject-editor: true container: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images/workspaces/ubuntu-24.04:golang-1.23@sha256:fb6e3c2136772506b374f8c6abdf38873cc41d3e988df30b1edabe3943a159ed" + 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. -- GitLab From 6b1df5254c2d8890bdff1f5059a52294c330d17c Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Tue, 7 Jan 2025 08:13:53 +0000 Subject: [PATCH 21/24] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Chad Woolley --- ee/app/graphql/types/remote_development/workspace_type.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 494d9dde60cacd..e2994103340292 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -61,7 +61,7 @@ class WorkspaceType < ::Types::BaseObject 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: 'Project repo git ref.' + null: false, description: 'Project repository git reference that will be cloned into the workspace.' field :devfile_path, GraphQL::Types::String, null: true, description: 'Path to the devfile used to configure the workspace.' -- GitLab From a7df8ef295157e992ab12edbf25c707a2d039534 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Tue, 7 Jan 2025 19:16:59 +1100 Subject: [PATCH 22/24] update text --- doc/api/graphql/reference/index.md | 2 +- ee/app/graphql/types/remote_development/workspace_type.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index fe09cb34a0a212..b5ba699d441b9c 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -37997,7 +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) | Project repo git ref. | +| `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/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index e2994103340292..503ec2459d6386 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -61,7 +61,8 @@ class WorkspaceType < ::Types::BaseObject 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: 'Project repository git reference that will be cloned into the workspace.' + 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: true, description: 'Path to the devfile used to configure the workspace.' -- GitLab From 4ae027325b90f0f98aac844fa7eaceab26f762a8 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Wed, 8 Jan 2025 10:29:43 +1100 Subject: [PATCH 23/24] fix failing rspec --- ee/app/models/remote_development/workspace.rb | 2 +- .../workspace_operations/create/devfile_fetcher_spec.rb | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index c2adec75f4cb59..1acb86e716c7d1 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -9,7 +9,7 @@ class Workspace < ApplicationRecord columns_changing_default :desired_config_generator_version - ignore_column :devfile_ref, remove_with: '17.7', remove_after: '2024-11-21' + 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 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 9153075b52c9ee..1f48479e6fd88f 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 @@ -32,11 +32,13 @@ } end + let(:default_devfile_text) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE } + let(:context) do { params: params, settings: { - default_devfile: default_devfile + default_devfile: default_devfile_text } } end @@ -53,7 +55,10 @@ Gitlab::Fp::Result.ok({ params: params, devfile_yaml: devfile_yaml, - devfile: devfile + devfile: devfile, + settings: { + default_devfile: default_devfile_text + } }) ) end -- GitLab From 844de7004d08d9309489cb37b2ec268f74af1105 Mon Sep 17 00:00:00 2001 From: Zhaochen Li Date: Wed, 8 Jan 2025 18:45:39 +1100 Subject: [PATCH 24/24] apply pitch & address comments --- .../settings/default_settings.rb | 6 +- .../create/devfile_fetcher.rb | 97 +++++++++++++------ .../settings/settings_initializer_spec.rb | 8 +- .../create/devfile_fetcher_spec.rb | 61 +++++++++--- .../create/main_integration_spec.rb | 8 +- 5 files changed, 122 insertions(+), 58 deletions(-) diff --git a/ee/lib/remote_development/settings/default_settings.rb b/ee/lib/remote_development/settings/default_settings.rb index ff25fd5c0f97b2..af15367246d8d0 100644 --- a/ee/lib/remote_development/settings/default_settings.rb +++ b/ee/lib/remote_development/settings/default_settings.rb @@ -5,7 +5,7 @@ module Settings class DefaultSettings UNDEFINED = nil - # When updating DEFAULT_DEVFILE, update the user facing doc as well. + # 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. @@ -13,7 +13,7 @@ class DefaultSettings # 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 = <<~DEVFILE + DEFAULT_DEVFILE_YAML = <<~DEVFILE schemaVersion: 2.2.0 components: - name: development-environment @@ -34,7 +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: [DEFAULT_DEVFILE, 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 8d2e8c8ed70f6d..673dc80e7486e2 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb @@ -13,24 +13,20 @@ class DevfileFetcher # @return [Gitlab::Fp::Result] def self.fetch(context) Gitlab::Fp::Result.ok(context) - .and_then(method(:fetch_devfile)) + .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.fetch_devfile(context) + def self.validate_agent_config_exists(context) context => { - params: Hash => params, - settings: { - default_devfile: String => default_devfile + params: { + agent: Clusters::Agent => agent } } - params => { - agent: Clusters::Agent => agent, - project: Project => project, - project_ref: String => project_ref, - devfile_path: String | nil => devfile_path - } unless agent.unversioned_latest_workspaces_agent_config return Gitlab::Fp::Result.err(WorkspaceCreateParamsValidationFailed.new( @@ -38,28 +34,66 @@ def self.fetch_devfile(context) )) end - devfile_yaml = if devfile_path.nil? - default_devfile - else - 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 + } + } + + 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 '#{project_ref}' " \ + "does not exist in the project repository" + )) + end - devfile_blob = repository.blob_at_branch(project_ref, devfile_path) + unless devfile_blob.data.present? + return Gitlab::Fp::Result.err( + WorkspaceCreateDevfileLoadFailed.new(details: "Devfile could not be loaded from project") + ) + end - unless devfile_blob - return Gitlab::Fp::Result.err(WorkspaceCreateDevfileLoadFailed.new( - details: "Devfile path '#{devfile_path}' at ref '#{project_ref}' " \ - "does not exist in the project repository" - )) - end + context[:devfile_yaml] = devfile_blob.data - unless devfile_blob.data.present? - return Gitlab::Fp::Result.err( - WorkspaceCreateDevfileLoadFailed.new(details: "Devfile could not be loaded from project") - ) - end + Gitlab::Fp::Result.ok(context) + end - devfile_blob.data - 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 @@ -75,11 +109,12 @@ def self.fetch_devfile(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 :fetch_devfile + + 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/spec/lib/remote_development/settings/settings_initializer_spec.rb b/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb index 1a81f7453f2fdf..ac3ba4a0ce1104 100644 --- a/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb +++ b/ee/spec/lib/remote_development/settings/settings_initializer_spec.rb @@ -10,7 +10,7 @@ { requested_setting_names: requested_setting_names } end - let(:default_devfile_text) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE } + let(:default_devfile_yaml) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE_YAML } subject(:returned_value) do described_class.init(context) @@ -26,7 +26,7 @@ :allow_privilege_escalation, :annotations, :default_branch_name, - :default_devfile, + :default_devfile_yaml, :default_max_hours_before_termination, :default_resources_per_workspace_container, :default_runtime_class, @@ -51,7 +51,7 @@ allow_privilege_escalation: false, annotations: {}, default_branch_name: nil, - default_devfile: default_devfile_text, + default_devfile_yaml: default_devfile_yaml, default_max_hours_before_termination: 24, default_resources_per_workspace_container: {}, default_runtime_class: "", @@ -79,7 +79,7 @@ allow_privilege_escalation: :Boolean, annotations: Hash, default_branch_name: String, - default_devfile: 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 1f48479e6fd88f..f7049b8a2971fe 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 @@ -16,9 +16,6 @@ let_it_be(:agent) { create(:ee_cluster_agent, :with_existing_workspaces_agent_config) } let(:random_string) { 'abcdef' } let(:project_ref) { 'main' } - let(:devfile_path) { '.devfile.yaml' } - let(:devfile_fixture_name) { 'example.devfile.yaml' } - let(:devfile_yaml) { read_devfile_yaml(devfile_fixture_name) } let(:workspace_root) { '/projects' } let(:params) do { @@ -32,13 +29,13 @@ } end - let(:default_devfile_text) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE } + let(:default_devfile_yaml) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE_YAML } let(:context) do { params: params, settings: { - default_devfile: default_devfile_text + default_devfile_yaml: default_devfile_yaml } } end @@ -50,22 +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, - settings: { - default_devfile: default_devfile_text - } - }) - ) + 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 @@ -115,6 +140,8 @@ 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 @@ -129,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 @@ -141,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 55c43c5c174009..0e594b08ee0cb4 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 @@ -27,7 +27,7 @@ ] end - let(:default_devfile_text) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE } + let(:default_devfile_yaml) { ::RemoteDevelopment::Settings::DefaultSettings::DEFAULT_DEVFILE_YAML } let(:project) do files = devfile_path.nil? ? {} : { devfile_path => devfile_yaml } @@ -69,7 +69,7 @@ { project_cloner_image: 'alpine/git:2.45.2', tools_injector_image: tools_injector_image_from_settings, - default_devfile: default_devfile_text + default_devfile_yaml: default_devfile_yaml } end @@ -158,10 +158,10 @@ context 'when devfile_path is nil' do let(:devfile_path) { nil } - it 'creates a new workspace using default_devfile from settings' do + 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_text) + expect(workspace.devfile).to eq(default_devfile_yaml) end end -- GitLab