From ee50f86166bb95a23bc1f30a990a57eb97700314 Mon Sep 17 00:00:00 2001 From: Daniyal Arshad Date: Wed, 2 Jul 2025 18:41:30 -0400 Subject: [PATCH 1/7] Add support for preserving container entrypoint cmd EE: true --- .../restrictions_enforcer.rb | 1 - .../remote_development_constants.rb | 1 + .../create/main_component_updater.rb | 20 ++++++++++++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb index ac01916e4ad757..f94b058af0790d 100644 --- a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb +++ b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb @@ -14,7 +14,6 @@ class RestrictionsEnforcer # Devfile standard only allows name/id to be of the format /'^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'/ # Hence, we do no need to restrict the prefix `gl_`. # However, we do that for the 'variables' in the devfile since they do not have any such restriction - RESTRICTED_PREFIX = "gl-" # Currently, we only support 'container' and 'volume' type components. # For container components, ensure no endpoint name starts with restricted_prefix diff --git a/ee/lib/remote_development/remote_development_constants.rb b/ee/lib/remote_development/remote_development_constants.rb index 390c168c3ee88f..a106e23c113f64 100644 --- a/ee/lib/remote_development/remote_development_constants.rb +++ b/ee/lib/remote_development/remote_development_constants.rb @@ -11,5 +11,6 @@ module RemoteDevelopmentConstants # Please keep alphabetized MAIN_COMPONENT_INDICATOR_ATTRIBUTE = "gl/inject-editor" REQUIRED_DEVFILE_SCHEMA_VERSION = "2.2.0" + RESTRICTED_PREFIX = "gl-" end end diff --git a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb index 0b815e6075e33b..f3d5a2a4a6096c 100644 --- a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb +++ b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb @@ -29,6 +29,11 @@ def self.update(context) main_component_container = main_component.fetch(:container) + components_with_override_command_enabled = components.select do |component| + override_command = component.dig(:attributes, :overrideCommand) + override_command != false && !component.fetch(:name)&.start_with?(RESTRICTED_PREFIX) + end + update_env_vars( main_component_container: main_component_container, tools_dir: tools_dir, @@ -44,7 +49,7 @@ def self.update(context) ) override_command_and_args( - main_component_container: main_component_container + components_with_override_command_enabled: components_with_override_command_enabled ) context @@ -109,15 +114,20 @@ def self.update_endpoints(main_component_container:, editor_port:, ssh_port:) nil end - # @param [Hash] main_component_container + # @param [Array] components_with_override_command_enabled # @return [void] - def self.override_command_and_args(main_component_container:) + def self.override_command_and_args(components_with_override_command_enabled:) # This overrides the main container's command # Open issue to support both starting the editor and running the default command: # https://gitlab.com/gitlab-org/gitlab/-/issues/392853 - main_component_container[:command] = %w[/bin/sh -c] - main_component_container[:args] = [MAIN_COMPONENT_UPDATER_CONTAINER_ARGS] + components_with_override_command_enabled.each do |component| + container = component[:container] + if container + container[:command] = %w[/bin/sh -c] + container[:args] = [MAIN_COMPONENT_UPDATER_CONTAINER_ARGS] + end + end nil end -- GitLab From ef60c394e94a5f82e9b8d4a10110af8edf811565 Mon Sep 17 00:00:00 2001 From: Daniyal Arshad Date: Mon, 7 Jul 2025 16:29:57 -0400 Subject: [PATCH 2/7] Add validation and update specs/fixtures --- .../restrictions_enforcer.rb | 12 ++++++ .../create/main_component_updater.rb | 8 ++-- .../example.devfile.yaml.erb | 9 ++++ .../example.flattened-devfile.yaml.erb | 11 +++++ ...ststart-commands-inserted-devfile.yaml.erb | 17 ++++++++ ...with-command-args-present-devfile.yaml.erb | 14 +++++++ ...ntainer-command-processed-devfile.yaml.erb | 20 +++++++++ ...ntainer-command-processed-devfile.yaml.erb | 20 +++++++++ ...le.main-container-updated-devfile.yaml.erb | 17 ++++++++ ...ated-marketplace-disabled-devfile.yaml.erb | 17 ++++++++ .../example.processed-devfile.yaml.erb | 20 +++++++++ ...e.tools-injector-inserted-devfile.yaml.erb | 11 +++++ .../restrictions_enforcer_spec.rb | 1 + .../devfile_parser_getter_spec.rb | 14 +++++++ .../create/main_component_updater_spec.rb | 31 ++++++++++++++ .../tools_injector_component_inserter_spec.rb | 5 ++- .../remote_development_shared_contexts.rb | 42 +++++++++++++++++++ locale/gitlab.pot | 6 +++ 18 files changed, 270 insertions(+), 5 deletions(-) create mode 100644 ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-command-args-present-devfile.yaml.erb diff --git a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb index f94b058af0790d..d8bfccc8c4150a 100644 --- a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb +++ b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb @@ -232,6 +232,18 @@ def self.validate_containers(context) append_err(format(_("Property 'dedicatedPod' of component '%{component}' is not yet supported"), component: component_name), context) end + + if component.dig(:attributes, :overrideCommand) != false && + (container[:command].present? || container[:args].present?) + return err( + format( + _("Properties 'command', 'args' for component '%{name}' can only be specified " \ + "when the 'overrideCommand' attribute is set to false"), + name: component.fetch(:name) + ), + context + ) + end end context diff --git a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb index f3d5a2a4a6096c..a566ccad727b79 100644 --- a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb +++ b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb @@ -123,10 +123,10 @@ def self.override_command_and_args(components_with_override_command_enabled:) components_with_override_command_enabled.each do |component| container = component[:container] - if container - container[:command] = %w[/bin/sh -c] - container[:args] = [MAIN_COMPONENT_UPDATER_CONTAINER_ARGS] - end + next unless container + + container[:command] = %w[/bin/sh -c] + container[:args] = [MAIN_COMPONENT_UPDATER_CONTAINER_ARGS] end nil diff --git a/ee/spec/fixtures/remote_development/example.devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.devfile.yaml.erb index b23ec15cc93a0f..a3674c4ed76200 100644 --- a/ee/spec/fixtures/remote_development/example.devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.devfile.yaml.erb @@ -7,11 +7,20 @@ components: container: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo - name: database-container + attributes: + overrideCommand: true container: image: mysql env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] commands: - id: user-defined-command exec: diff --git a/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb index a1ab14be9569e1..3e8d5011f22e2e 100644 --- a/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb @@ -10,6 +10,8 @@ components: mountSources: true image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo - name: database-container + attributes: + overrideCommand: true container: dedicatedPod: false mountSources: true @@ -17,6 +19,15 @@ components: env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + dedicatedPod: false + mountSources: true + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] commands: - id: user-defined-command exec: diff --git a/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb index 5e6fdf7b4e14bf..a479d957fd0bea 100644 --- a/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb @@ -37,13 +37,30 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: true container: image: mysql + args: + - | + <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + command: + - "/bin/sh" + - "-c" env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" dedicatedPod: false mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + dedicatedPod: false + mountSources: true - name: gl-tools-injector container: image: <%= WORKSPACE_TOOLS_IMAGE %> diff --git a/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-command-args-present-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-command-args-present-devfile.yaml.erb new file mode 100644 index 00000000000000..b25aa6ac468752 --- /dev/null +++ b/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-command-args-present-devfile.yaml.erb @@ -0,0 +1,14 @@ +--- +schemaVersion: 2.2.0 +components: + - name: tooling-container + attributes: + gl/inject-editor: true + overrideCommand: true + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] +commands: [] +events: {} +variables: {} \ No newline at end of file diff --git a/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb index e88d474cc57103..9e48cfd2b5c1b2 100644 --- a/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb @@ -41,8 +41,16 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: true container: image: mysql + args: + - | + <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + command: + - "/bin/sh" + - "-c" volumeMounts: - name: gl-workspace-data path: /projects @@ -51,6 +59,18 @@ components: value: "my-secret-pw" dedicatedPod: false mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + volumeMounts: + - name: gl-workspace-data + path: /projects + dedicatedPod: false + mountSources: true - name: gl-tools-injector container: image: <%= WORKSPACE_TOOLS_IMAGE %> diff --git a/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb index 4dd6a01c23a8da..0cc2b007bdcb7a 100644 --- a/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb @@ -40,8 +40,16 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: true container: image: mysql + args: + - | + <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + command: + - "/bin/sh" + - "-c" volumeMounts: - name: gl-workspace-data path: /projects @@ -50,6 +58,18 @@ components: value: "my-secret-pw" dedicatedPod: false mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + volumeMounts: + - name: gl-workspace-data + path: /projects + dedicatedPod: false + mountSources: true - name: gl-tools-injector container: image: <%= WORKSPACE_TOOLS_IMAGE %> diff --git a/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb index 65f71c915b0908..dd294e21274d64 100644 --- a/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb @@ -37,13 +37,30 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: true container: image: mysql + args: + - | + <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + command: + - "/bin/sh" + - "-c" env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" dedicatedPod: false mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + dedicatedPod: false + mountSources: true - name: gl-tools-injector container: image: <%= WORKSPACE_TOOLS_IMAGE %> diff --git a/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb index 65f71c915b0908..dd294e21274d64 100644 --- a/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb @@ -37,13 +37,30 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: true container: image: mysql + args: + - | + <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + command: + - "/bin/sh" + - "-c" env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" dedicatedPod: false mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + dedicatedPod: false + mountSources: true - name: gl-tools-injector container: image: <%= WORKSPACE_TOOLS_IMAGE %> diff --git a/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb index 6d683c9da64e5a..26cf365e950f8a 100644 --- a/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb @@ -40,8 +40,16 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: true container: image: mysql + args: + - | + <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + command: + - "/bin/sh" + - "-c" volumeMounts: - name: gl-workspace-data path: /projects @@ -50,6 +58,18 @@ components: value: "my-secret-pw" dedicatedPod: false mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + volumeMounts: + - name: gl-workspace-data + path: /projects + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + dedicatedPod: false + mountSources: true - name: gl-tools-injector container: image: <%= WORKSPACE_TOOLS_IMAGE %> diff --git a/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb index 57754eda21c8fc..795c8e261fb82b 100644 --- a/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb @@ -10,6 +10,8 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: true container: image: mysql env: @@ -17,6 +19,15 @@ components: value: "my-secret-pw" dedicatedPod: false mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + dedicatedPod: false + mountSources: true - name: gl-tools-injector container: image: <%= WORKSPACE_TOOLS_IMAGE %> diff --git a/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb b/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb index 49c3ca9d4e0704..c9f98ccc04a908 100644 --- a/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb +++ b/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb @@ -74,6 +74,7 @@ # rubocop:disable Layout/LineLength -- we want single lines for RSpec::Parameterized::TableSyntax where(:input_devfile_name, :error_str) do + "example.invalid-attributes-override-command-with-command-args-present-devfile.yaml.erb" | "Properties 'command', 'args' for component 'tooling-container' can only be specified when the 'overrideCommand' attribute is set to false" "example.invalid-attributes-tools-injector-absent-devfile.yaml.erb" | "No component has '#{main_component_indicator_attribute}' attribute" "example.invalid-attributes-tools-injector-multiple-devfile.yaml.erb" | "Multiple components '[\"tooling-container\", \"tooling-container-2\"]' have '#{main_component_indicator_attribute}' attribute" "example.invalid-component-missing-name.yaml.erb" | "A component must have a 'name'" diff --git a/ee/spec/lib/remote_development/workspace_operations/create/desired_config/devfile_parser_getter_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/desired_config/devfile_parser_getter_spec.rb index 3d6215bd243979..0826979b45f5fc 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/desired_config/devfile_parser_getter_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/desired_config/devfile_parser_getter_spec.rb @@ -84,6 +84,20 @@ imagePullPolicy: Always name: database-container resources: {} + - args: + - -n + - user-defined entrypoint command + command: + - echo + env: + - name: PROJECTS_ROOT + value: /projects + - name: PROJECT_SOURCE + value: /projects + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + imagePullPolicy: Always + name: user-defined-entrypoint-cmd-component + resources: {} status: {} --- apiVersion: v1 diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb index d454ee7ed587de..e30468590bf5fa 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb @@ -37,6 +37,37 @@ expect(actual).to eq(expected) end + it "overrides container default entrypoint command and args only if overrideCommand is not explicitly set to false" do + result = returned_value[:processed_devfile] + components = result[:components] + + component_with_override_not_explicitly_enabled = components.find { |c| c[:name] == "tooling-container" } + component_with_override_explicitly_enabled = components.find { |c| c[:name] == "database-container" } + component_with_override_explicitly_disabled = components.find do |c| + c[:name] == "user-defined-entrypoint-cmd-component" + end + internal_component = components.find { |c| c[:name] == "gl-tools-injector" } + + # When overrideCommand: true - should override with tail command + expect(component_with_override_explicitly_enabled[:container][:command]).to eq(["/bin/sh", "-c"]) + expect(component_with_override_explicitly_enabled[:container][:args]).to eq(["tail -f /dev/null\n"]) + + # When overrideCommand is not provided - should override with tail command (default behavior) + expect(component_with_override_not_explicitly_enabled[:container][:command]).to eq(["/bin/sh", "-c"]) + expect(component_with_override_not_explicitly_enabled[:container][:args]).to eq(["tail -f /dev/null\n"]) + + # When overrideCommand: false - should preserve original command/args if provided + expect(component_with_override_explicitly_disabled[:container][:command]).to eq(["echo"]) + expect(component_with_override_explicitly_disabled[:container][:args]).to eq([ + "-n", + "user-defined entrypoint command" + ]) + + # Does not affect internal components with the restricted gl prefix + expect(internal_component[:container][:command]).not_to eq(["/bin/sh", "-c"]) + expect(internal_component[:container][:args]).not_to eq(["tail -f /dev/null\n"]) + end + context "when vscode_extension_marketplace_metadata Web IDE setting is disabled" do let(:expected_processed_devfile_name) { "example.main-container-updated-marketplace-disabled-devfile.yaml.erb" } let(:vscode_extension_marketplace_metadata_enabled) { false } diff --git a/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb index d8d5b7477a0f4a..adad70263dc20a 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb @@ -47,7 +47,10 @@ let(:tools_injector_image_from_settings) { 'my/awesome/image:42' } it 'uses image override' do - image_from_processed_devfile = returned_value.dig(:processed_devfile, :components, 2, :container, :image) + components = returned_value.dig(:processed_devfile, :components) + gl_tools_component = components&.find { |component| component[:name] == "gl-tools-injector" } + image_from_processed_devfile = gl_tools_component&.dig(:container, :image) + expect(image_from_processed_devfile).to eq(tools_injector_image_from_settings) end end diff --git a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb index 04c2c232335005..cdc60d9bb1c6a4 100644 --- a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb +++ b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb @@ -760,6 +760,48 @@ def workspace_deployment( imagePullPolicy: "Always", name: "database-container", resources: default_resources_per_workspace_container, + args: [files_module::MAIN_COMPONENT_UPDATER_CONTAINER_ARGS], + command: %w[/bin/sh -c], + volumeMounts: [ + { + mountPath: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH, + name: create_constants_module::WORKSPACE_DATA_VOLUME_NAME + }, + { + mountPath: workspace_operations_constants_module::VARIABLES_VOLUME_PATH, + name: workspace_operations_constants_module::VARIABLES_VOLUME_NAME + }, + { + mountPath: create_constants_module::WORKSPACE_SCRIPTS_VOLUME_PATH, + name: create_constants_module::WORKSPACE_SCRIPTS_VOLUME_NAME + } + ], + securityContext: container_security_context, + envFrom: [ + { + secretRef: { + name: "#{workspace_name}-env-var" + } + } + ] + }, + { + env: [ + { + name: "PROJECTS_ROOT", + value: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH + }, + { + name: "PROJECT_SOURCE", + value: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH + } + ], + image: "quay.io/mloriedo/universal-developer-image:ubi8-dw-demo", + imagePullPolicy: "Always", + name: "user-defined-entrypoint-cmd-component", + resources: default_resources_per_workspace_container, + command: ["echo"], + args: ["-n", "user-defined entrypoint command"], volumeMounts: [ { mountPath: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4e9ede9ea06f13..355ea7802bd342 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -50613,6 +50613,12 @@ msgstr "" msgid "Property 'dedicatedPod' of component '%{component}' is not yet supported" msgstr "" +msgid "Properties 'command', 'args' for component '%{name}' can only be specified when the 'overrideCommand' attribute is set to false" +msgstr "" + +msgid "Property 'dedicatedPod' of component '%{name}' is not yet supported" +msgstr "" + msgid "Property 'hotReloadCapable' for exec command '%{command}' must be false when specified" msgstr "" -- GitLab From 3650edf381047922b0eb775b018ccef39cf0927f Mon Sep 17 00:00:00 2001 From: Daniyal Arshad Date: Tue, 8 Jul 2025 17:44:38 -0400 Subject: [PATCH 3/7] Address review feedback --- .../restrictions_enforcer.rb | 22 ++++++++++++++----- .../remote_development_constants.rb | 4 ++++ .../create/main_component_updater.rb | 11 +++++----- .../example.devfile.yaml.erb | 2 -- .../example.flattened-devfile.yaml.erb | 2 -- ...ststart-commands-inserted-devfile.yaml.erb | 2 -- ...nd-with-non-boolean-value-devfile.yaml.erb | 14 ++++++++++++ ...ntainer-command-processed-devfile.yaml.erb | 2 -- ...ntainer-command-processed-devfile.yaml.erb | 2 -- ...le.main-container-updated-devfile.yaml.erb | 2 -- ...ated-marketplace-disabled-devfile.yaml.erb | 2 -- .../example.processed-devfile.yaml.erb | 2 -- ...e.tools-injector-inserted-devfile.yaml.erb | 2 -- .../restrictions_enforcer_spec.rb | 1 + .../create/main_component_updater_spec.rb | 11 +++------- locale/gitlab.pot | 3 +++ 16 files changed, 48 insertions(+), 36 deletions(-) create mode 100644 ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb diff --git a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb index d8bfccc8c4150a..28f394e30d810a 100644 --- a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb +++ b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb @@ -11,9 +11,6 @@ class RestrictionsEnforcer # Since this is called after flattening the devfile, we can safely assume that it has valid syntax # as per devfile standard. If you are validating something that is not available across all devfile versions, # add additional guard clauses. - # Devfile standard only allows name/id to be of the format /'^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'/ - # Hence, we do no need to restrict the prefix `gl_`. - # However, we do that for the 'variables' in the devfile since they do not have any such restriction # Currently, we only support 'container' and 'volume' type components. # For container components, ensure no endpoint name starts with restricted_prefix @@ -31,6 +28,9 @@ class RestrictionsEnforcer # Currently, we only support the default value `false` for the `hotReloadCapable` option SUPPORTED_HOT_RELOAD_VALUE = false + # Override command must be a boolean value + SUPPORTED_OVERRIDE_COMMAND_VALUE = [true, false].freeze + # @param [Hash] parent_context # @return [Gitlab::Fp::Result] def self.enforce(parent_context) @@ -233,8 +233,20 @@ def self.validate_containers(context) component: component_name), context) end - if component.dig(:attributes, :overrideCommand) != false && - (container[:command].present? || container[:args].present?) + attributes = component.fetch(:attributes, {}) + override_command = attributes.fetch(:overrideCommand, true) + + unless SUPPORTED_OVERRIDE_COMMAND_VALUE.include?(override_command) + return err( + format( + _("Property 'overrideCommand' of component '%{name}' must be a boolean (true or false)"), + name: component.fetch(:name) + ), + context + ) + end + + if override_command == true && (container[:command].present? || container[:args].present?) return err( format( _("Properties 'command', 'args' for component '%{name}' can only be specified " \ diff --git a/ee/lib/remote_development/remote_development_constants.rb b/ee/lib/remote_development/remote_development_constants.rb index a106e23c113f64..da45fd401478f4 100644 --- a/ee/lib/remote_development/remote_development_constants.rb +++ b/ee/lib/remote_development/remote_development_constants.rb @@ -11,6 +11,10 @@ module RemoteDevelopmentConstants # Please keep alphabetized MAIN_COMPONENT_INDICATOR_ATTRIBUTE = "gl/inject-editor" REQUIRED_DEVFILE_SCHEMA_VERSION = "2.2.0" + + # Devfile standard only allows name/id to be of the format /'^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'/ + # Hence, we do no need to restrict the prefix `gl_`. + # However, we do that for the 'variables' in the devfile since they do not have any such restriction RESTRICTED_PREFIX = "gl-" end end diff --git a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb index a566ccad727b79..f0a951ab2498ef 100644 --- a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb +++ b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb @@ -30,8 +30,10 @@ def self.update(context) main_component_container = main_component.fetch(:container) components_with_override_command_enabled = components.select do |component| - override_command = component.dig(:attributes, :overrideCommand) - override_command != false && !component.fetch(:name)&.start_with?(RESTRICTED_PREFIX) + attributes = component.fetch(:attributes, {}) + override_command = attributes.fetch(:overrideCommand, true) + + override_command == true && !component.fetch(:name)&.start_with?(RESTRICTED_PREFIX) end update_env_vars( @@ -117,9 +119,8 @@ def self.update_endpoints(main_component_container:, editor_port:, ssh_port:) # @param [Array] components_with_override_command_enabled # @return [void] def self.override_command_and_args(components_with_override_command_enabled:) - # This overrides the main container's command - # Open issue to support both starting the editor and running the default command: - # https://gitlab.com/gitlab-org/gitlab/-/issues/392853 + # This overrides the entrypoint command for all containers that don't have the + # `overrideCommand` attribute set to false in the devfile. components_with_override_command_enabled.each do |component| container = component[:container] diff --git a/ee/spec/fixtures/remote_development/example.devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.devfile.yaml.erb index a3674c4ed76200..89469bb6368525 100644 --- a/ee/spec/fixtures/remote_development/example.devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.devfile.yaml.erb @@ -7,8 +7,6 @@ components: container: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo - name: database-container - attributes: - overrideCommand: true container: image: mysql env: diff --git a/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb index 3e8d5011f22e2e..1d9010790ed544 100644 --- a/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.flattened-devfile.yaml.erb @@ -10,8 +10,6 @@ components: mountSources: true image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo - name: database-container - attributes: - overrideCommand: true container: dedicatedPod: false mountSources: true diff --git a/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb index a479d957fd0bea..a99f3d79e29317 100644 --- a/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb @@ -37,8 +37,6 @@ components: dedicatedPod: false mountSources: true - name: database-container - attributes: - overrideCommand: true container: image: mysql args: diff --git a/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb new file mode 100644 index 00000000000000..f2aa343658f138 --- /dev/null +++ b/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb @@ -0,0 +1,14 @@ +--- +schemaVersion: 2.2.0 +components: + - name: tooling-container + attributes: + gl/inject-editor: true + overrideCommand: foobar + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] +commands: [] +events: {} +variables: {} \ No newline at end of file diff --git a/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb index 9e48cfd2b5c1b2..2cbe720c136911 100644 --- a/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb @@ -41,8 +41,6 @@ components: dedicatedPod: false mountSources: true - name: database-container - attributes: - overrideCommand: true container: image: mysql args: diff --git a/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb index 0cc2b007bdcb7a..d16edac4155796 100644 --- a/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb @@ -40,8 +40,6 @@ components: dedicatedPod: false mountSources: true - name: database-container - attributes: - overrideCommand: true container: image: mysql args: diff --git a/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb index dd294e21274d64..d4cc47e1886296 100644 --- a/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb @@ -37,8 +37,6 @@ components: dedicatedPod: false mountSources: true - name: database-container - attributes: - overrideCommand: true container: image: mysql args: diff --git a/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb index dd294e21274d64..d4cc47e1886296 100644 --- a/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb @@ -37,8 +37,6 @@ components: dedicatedPod: false mountSources: true - name: database-container - attributes: - overrideCommand: true container: image: mysql args: diff --git a/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb index 26cf365e950f8a..bd04329845018c 100644 --- a/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb @@ -40,8 +40,6 @@ components: dedicatedPod: false mountSources: true - name: database-container - attributes: - overrideCommand: true container: image: mysql args: diff --git a/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb index 795c8e261fb82b..d43e844f9768ec 100644 --- a/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.tools-injector-inserted-devfile.yaml.erb @@ -10,8 +10,6 @@ components: dedicatedPod: false mountSources: true - name: database-container - attributes: - overrideCommand: true container: image: mysql env: diff --git a/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb b/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb index c9f98ccc04a908..55cbb3a3c2d837 100644 --- a/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb +++ b/ee/spec/lib/remote_development/devfile_operations/restrictions_enforcer_spec.rb @@ -75,6 +75,7 @@ # rubocop:disable Layout/LineLength -- we want single lines for RSpec::Parameterized::TableSyntax where(:input_devfile_name, :error_str) do "example.invalid-attributes-override-command-with-command-args-present-devfile.yaml.erb" | "Properties 'command', 'args' for component 'tooling-container' can only be specified when the 'overrideCommand' attribute is set to false" + "example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb" | "Property 'overrideCommand' of component 'tooling-container' must be a boolean (true or false)" "example.invalid-attributes-tools-injector-absent-devfile.yaml.erb" | "No component has '#{main_component_indicator_attribute}' attribute" "example.invalid-attributes-tools-injector-multiple-devfile.yaml.erb" | "Multiple components '[\"tooling-container\", \"tooling-container-2\"]' have '#{main_component_indicator_attribute}' attribute" "example.invalid-component-missing-name.yaml.erb" | "A component must have a 'name'" diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb index e30468590bf5fa..65c04958599bf8 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb @@ -41,20 +41,15 @@ result = returned_value[:processed_devfile] components = result[:components] - component_with_override_not_explicitly_enabled = components.find { |c| c[:name] == "tooling-container" } - component_with_override_explicitly_enabled = components.find { |c| c[:name] == "database-container" } + component_with_override_not_explicitly_disabled = components.find { |c| c[:name] == "tooling-container" } component_with_override_explicitly_disabled = components.find do |c| c[:name] == "user-defined-entrypoint-cmd-component" end internal_component = components.find { |c| c[:name] == "gl-tools-injector" } - # When overrideCommand: true - should override with tail command - expect(component_with_override_explicitly_enabled[:container][:command]).to eq(["/bin/sh", "-c"]) - expect(component_with_override_explicitly_enabled[:container][:args]).to eq(["tail -f /dev/null\n"]) - # When overrideCommand is not provided - should override with tail command (default behavior) - expect(component_with_override_not_explicitly_enabled[:container][:command]).to eq(["/bin/sh", "-c"]) - expect(component_with_override_not_explicitly_enabled[:container][:args]).to eq(["tail -f /dev/null\n"]) + expect(component_with_override_not_explicitly_disabled[:container][:command]).to eq(["/bin/sh", "-c"]) + expect(component_with_override_not_explicitly_disabled[:container][:args]).to eq(["tail -f /dev/null\n"]) # When overrideCommand: false - should preserve original command/args if provided expect(component_with_override_explicitly_disabled[:container][:command]).to eq(["echo"]) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 355ea7802bd342..e6ca7537c08a18 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -50622,6 +50622,9 @@ msgstr "" msgid "Property 'hotReloadCapable' for exec command '%{command}' must be false when specified" msgstr "" +msgid "Property 'overrideCommand' of component '%{name}' must be a boolean (true or false)" +msgstr "" + msgid "Protect" msgstr "" -- GitLab From fd927eebe6c6dc34ec4da3eddaf1bbc0c5b7fed0 Mon Sep 17 00:00:00 2001 From: Daniyal Arshad Date: Thu, 10 Jul 2025 14:15:30 -0400 Subject: [PATCH 4/7] Add docs for override command --- doc/user/workspace/_index.md | 57 +++++++++++++++---- .../restrictions_enforcer.rb | 15 ++--- locale/gitlab.pot | 8 +-- 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/doc/user/workspace/_index.md b/doc/user/workspace/_index.md index 963e73ba476753..689c94cc5f6570 100644 --- a/doc/user/workspace/_index.md +++ b/doc/user/workspace/_index.md @@ -214,16 +214,53 @@ You can specify the base image, dependencies, and other settings. The `container` component type supports the following schema properties only: -| Property | Description | -|----------------| -------------------------------------------------------------------------------------------------------------------------------| -| `image` | Name of the container image to use for the workspace. | -| `memoryRequest`| Minimum amount of memory the container can use. | -| `memoryLimit` | Maximum amount of memory the container can use. | -| `cpuRequest` | Minimum amount of CPU the container can use. | -| `cpuLimit` | Maximum amount of CPU the container can use. | -| `env` | Environment variables to use in the container. Names must not start with `gl-`. | -| `endpoints` | Port mappings to expose from the container. Names must not start with `gl-`. | -| `volumeMounts` | Storage volume to mount in the container. | +| Property | Description | +|---------------- | -------------------------------------------------------------------------------------------------------------------------------| +| `image` | Name of the container image to use for the workspace. | +| `memoryRequest` | Minimum amount of memory the container can use. | +| `memoryLimit` | Maximum amount of memory the container can use. | +| `cpuRequest` | Minimum amount of CPU the container can use. | +| `cpuLimit` | Maximum amount of CPU the container can use. | +| `env` | Environment variables to use in the container. Names must not start with `gl-`. | +| `endpoints` | Port mappings to expose from the container. Names must not start with `gl-`. | +| `volumeMounts` | Storage volume to mount in the container. | +| `overrideCommand` | Override the container entrypoint with a keep-alive command. | + +#### `overrideCommand` attribute + +The `overrideCommand` attribute is a boolean that control how Workspaces handle container entrypoints. +This attribute determines whether the container's original entrypoint is preserved or replaced +with a keep-alive command. + +When `true` (default), the container entrypoint is replaced with `tail -f /dev/null` to keep the +container running. +When `false`, the container uses either the devfile component `command`/`args` or the built container +image's `Entrypoint`/`Cmd`. + +The following table shows how `overrideCommand` affects container behavior. For clarity, these terms +are used in the table: + +- Devfile component: The `command` and `args` properties in the devfile component entry. +- Container image: The `Entrypoint` and `Cmd` fields in the OCI container image. + +| `overrideCommand` | Devfile component | Container image | Result | +|-------------------|-------------------|-----------------|--------| +| `true` | Specified | Specified | Validation error: Devfile component `command`/`args` cannot be specified when `overrideCommand` is `true`. | +| `true` | Specified | Not specified | Validation error: Devfile component `command`/`args` cannot be specified when `overrideCommand` is `true`. | +| `true` | Not specified | Specified | Container entrypoint replaced with `tail -f /dev/null`. | +| `true` | Not specified | Not specified | Container entrypoint replaced with `tail -f /dev/null`. | +| `false` | Specified | Specified | Devfile component `command`/`args` used as entrypoint. | +| `false` | Specified | Not specified | Devfile component `command`/`args` used as entrypoint. | +| `false` | Not specified | Specified | Container image `Entrypoint`/`Cmd` used. | +| `false` | Not specified | Not specified | Container exits prematurely (`CrashLoopBackOff`). 1 | + +**Footnotes**: + +1. When you create a workspace, it cannot access container image details, for example, from private +or internal registries. When `overrideCommand` is `false` and the Devfile doesn't specify `command` +or `args`, GitLab does not validate container images or check for required `Entrypoint` or `Cmd` fields. +You must ensure that either the Devfile or container specifies these fields, or the container exits +prematurely and the workspace fails to start. ### User-defined `postStart` events diff --git a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb index 28f394e30d810a..aa4b4ad98bdd37 100644 --- a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb +++ b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb @@ -28,9 +28,6 @@ class RestrictionsEnforcer # Currently, we only support the default value `false` for the `hotReloadCapable` option SUPPORTED_HOT_RELOAD_VALUE = false - # Override command must be a boolean value - SUPPORTED_OVERRIDE_COMMAND_VALUE = [true, false].freeze - # @param [Hash] parent_context # @return [Gitlab::Fp::Result] def self.enforce(parent_context) @@ -234,10 +231,14 @@ def self.validate_containers(context) end attributes = component.fetch(:attributes, {}) - override_command = attributes.fetch(:overrideCommand, true) + unless attributes.is_a?(Hash) + return append_err( + format(_("'attributes' in component '%{component}' must be a Hash"), component: component_name), context) + end - unless SUPPORTED_OVERRIDE_COMMAND_VALUE.include?(override_command) - return err( + override_command = attributes.fetch(:overrideCommand, true) + unless override_command.is_a?(TrueClass) || override_command.is_a?(FalseClass) + return append_err( format( _("Property 'overrideCommand' of component '%{name}' must be a boolean (true or false)"), name: component.fetch(:name) @@ -247,7 +248,7 @@ def self.validate_containers(context) end if override_command == true && (container[:command].present? || container[:args].present?) - return err( + return append_err( format( _("Properties 'command', 'args' for component '%{name}' can only be specified " \ "when the 'overrideCommand' attribute is set to false"), diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e6ca7537c08a18..df98f2c52c88eb 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1744,6 +1744,9 @@ msgstr "" msgid "'attributes' for component '%{component_name}' must be a Hash" msgstr "" +msgid "'attributes' in component '%{component}' must be a Hash" +msgstr "" + msgid "'command' must be a Hash" msgstr "" @@ -50610,13 +50613,10 @@ msgstr "" msgid "Prompt users to upload SSH keys" msgstr "" -msgid "Property 'dedicatedPod' of component '%{component}' is not yet supported" -msgstr "" - msgid "Properties 'command', 'args' for component '%{name}' can only be specified when the 'overrideCommand' attribute is set to false" msgstr "" -msgid "Property 'dedicatedPod' of component '%{name}' is not yet supported" +msgid "Property 'dedicatedPod' of component '%{component}' is not yet supported" msgstr "" msgid "Property 'hotReloadCapable' for exec command '%{command}' must be false when specified" -- GitLab From ad7ab49c7149faae789c31efcf163f8324cba8d1 Mon Sep 17 00:00:00 2001 From: Daniyal Arshad Date: Mon, 28 Jul 2025 22:46:32 -0400 Subject: [PATCH 5/7] Address review suggestions (2) --- ee/lib/remote_development/README.md | 2 +- .../restrictions_enforcer.rb | 1 - ee/lib/remote_development/files.rb | 8 +- .../create/container_command_updater.rb | 49 +++++++++++ ...sh => container_keepalive_command_args.sh} | 0 .../workspace_operations/create/main.rb | 1 + .../create/main_component_updater.rb | 30 +------ ...ontainer-commands-updated-devfile.yaml.erb | 85 +++++++++++++++++++ ...ststart-commands-inserted-devfile.yaml.erb | 13 ++- ...ntainer-command-processed-devfile.yaml.erb | 6 -- ...ntainer-command-processed-devfile.yaml.erb | 8 +- ...le.main-container-updated-devfile.yaml.erb | 12 --- ...ated-marketplace-disabled-devfile.yaml.erb | 12 --- .../example.processed-devfile.yaml.erb | 13 ++- .../create/container_command_updater_spec.rb | 70 +++++++++++++++ ...ternal_poststart_commands_inserter_spec.rb | 2 +- .../create/main_component_updater_spec.rb | 32 ------- .../workspace_operations/create/main_spec.rb | 1 + .../tools_injector_component_inserter_spec.rb | 6 +- .../remote_development_shared_contexts.rb | 32 ++++--- 20 files changed, 245 insertions(+), 138 deletions(-) create mode 100644 ee/lib/remote_development/workspace_operations/create/container_command_updater.rb rename ee/lib/remote_development/workspace_operations/create/{main_component_updater_container_args.sh => container_keepalive_command_args.sh} (100%) create mode 100644 ee/spec/fixtures/remote_development/example.container-commands-updated-devfile.yaml.erb create mode 100644 ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb diff --git a/ee/lib/remote_development/README.md b/ee/lib/remote_development/README.md index 8c312caa13166a..3eff887accf426 100644 --- a/ee/lib/remote_development/README.md +++ b/ee/lib/remote_development/README.md @@ -623,7 +623,7 @@ This approach ensures: The `RemoteDevelopment::Files` module contains constants for all the files (default devfile, shell scripts, script fragments, commands, etc) that are used in the Remote Development domain for various purposes, such as programatically injecting into container `ENTRYPOINT`/`CMD`, etc. -For example: `RemoteDevelopment::Files::MAIN_COMPONENT_UPDATER_CONTAINER_ARGS`. +For example: `RemoteDevelopment::Files::CONTAINER_KEEPALIVE_COMMAND_ARGS`. The contents of these files are pulled out to separate files in the filesystem, instead of being hardcoded via inline Ruby HEREDOC or other means. This allows them to have full support for syntax highlighting and refactoring in IDEs, diff --git a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb index aa4b4ad98bdd37..999c084fb12d0e 100644 --- a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb +++ b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb @@ -13,7 +13,6 @@ class RestrictionsEnforcer # add additional guard clauses. # Currently, we only support 'container' and 'volume' type components. - # For container components, ensure no endpoint name starts with restricted_prefix UNSUPPORTED_COMPONENT_TYPES = %i[kubernetes openshift image].freeze # Currently, we only support 'exec' and 'apply' for validation diff --git a/ee/lib/remote_development/files.rb b/ee/lib/remote_development/files.rb index 38fbdc8bcd360d..7d95e29dfde424 100644 --- a/ee/lib/remote_development/files.rb +++ b/ee/lib/remote_development/files.rb @@ -41,8 +41,8 @@ def self.kubernetes_poststart_hook_command end # @return [String] content of the file - def self.main_component_updater_container_args - read_file("workspace_operations/create/main_component_updater_container_args.sh") + def self.container_keepalive_command_args + read_file("workspace_operations/create/container_keepalive_command_args.sh") end # @return [String] content of the file @@ -87,7 +87,7 @@ def self.internal_poststart_command_clone_unshallow_script INTERNAL_POSTSTART_COMMAND_START_SSHD_SCRIPT = internal_poststart_command_start_sshd_script KUBERNETES_LEGACY_POSTSTART_HOOK_COMMAND = kubernetes_legacy_poststart_hook_command KUBERNETES_POSTSTART_HOOK_COMMAND = kubernetes_poststart_hook_command - MAIN_COMPONENT_UPDATER_CONTAINER_ARGS = main_component_updater_container_args + CONTAINER_KEEPALIVE_COMMAND_ARGS = container_keepalive_command_args # @return [Array] def self.all_expected_file_constants @@ -103,7 +103,7 @@ def self.all_expected_file_constants :INTERNAL_POSTSTART_COMMAND_START_SSHD_SCRIPT, :KUBERNETES_LEGACY_POSTSTART_HOOK_COMMAND, :KUBERNETES_POSTSTART_HOOK_COMMAND, - :MAIN_COMPONENT_UPDATER_CONTAINER_ARGS + :CONTAINER_KEEPALIVE_COMMAND_ARGS ] end diff --git a/ee/lib/remote_development/workspace_operations/create/container_command_updater.rb b/ee/lib/remote_development/workspace_operations/create/container_command_updater.rb new file mode 100644 index 00000000000000..97f283e0c7b8f1 --- /dev/null +++ b/ee/lib/remote_development/workspace_operations/create/container_command_updater.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module WorkspaceOperations + module Create + class ContainerCommandUpdater + include Files + include RemoteDevelopmentConstants + + # @param [Hash] context + # @return [Hash] + def self.update(context) + context => { + processed_devfile: { + components: Array => components + } + } + + # This overrides the entrypoint command for all containers that have the + # `overrideCommand` attribute set to true in the devfile. + + components.each do |component| + attributes = component[:attributes] ||= {} + + # If overrideCommand is not present, set it based on whether it's the main component + # Defaults to true for the main_component and false for all other components + unless attributes.key?(:overrideCommand) + is_main_component = attributes[MAIN_COMPONENT_INDICATOR_ATTRIBUTE.to_sym] + attributes[:overrideCommand] = is_main_component ? true : false + end + + override_enabled = component.dig(:attributes, :overrideCommand) == true + + # Skip if overrideCommand is not enabled or component name starts with restricted prefix + next unless override_enabled + + container = component[:container] + next unless container + + container[:command] = %w[/bin/sh -c] + container[:args] = [CONTAINER_KEEPALIVE_COMMAND_ARGS] + end + + context + end + end + end + end +end diff --git a/ee/lib/remote_development/workspace_operations/create/main_component_updater_container_args.sh b/ee/lib/remote_development/workspace_operations/create/container_keepalive_command_args.sh similarity index 100% rename from ee/lib/remote_development/workspace_operations/create/main_component_updater_container_args.sh rename to ee/lib/remote_development/workspace_operations/create/container_keepalive_command_args.sh diff --git a/ee/lib/remote_development/workspace_operations/create/main.rb b/ee/lib/remote_development/workspace_operations/create/main.rb index 6f2a5743a12399..f94c403cb773fb 100644 --- a/ee/lib/remote_development/workspace_operations/create/main.rb +++ b/ee/lib/remote_development/workspace_operations/create/main.rb @@ -21,6 +21,7 @@ def self.main(context) .map(VolumeDefiner.method(:define)) .map(ToolsInjectorComponentInserter.method(:insert)) .map(MainComponentUpdater.method(:update)) + .map(ContainerCommandUpdater.method(:update)) .map(InternalPoststartCommandsInserter.method(:insert)) .map(VolumeComponentInserter.method(:insert)) .and_then(Creator.method(:create)) diff --git a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb index f0a951ab2498ef..b1912b7bb70fde 100644 --- a/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb +++ b/ee/lib/remote_development/workspace_operations/create/main_component_updater.rb @@ -29,13 +29,6 @@ def self.update(context) main_component_container = main_component.fetch(:container) - components_with_override_command_enabled = components.select do |component| - attributes = component.fetch(:attributes, {}) - override_command = attributes.fetch(:overrideCommand, true) - - override_command == true && !component.fetch(:name)&.start_with?(RESTRICTED_PREFIX) - end - update_env_vars( main_component_container: main_component_container, tools_dir: tools_dir, @@ -50,10 +43,6 @@ def self.update(context) ssh_port: WORKSPACE_SSH_PORT ) - override_command_and_args( - components_with_override_command_enabled: components_with_override_command_enabled - ) - context end @@ -116,24 +105,7 @@ def self.update_endpoints(main_component_container:, editor_port:, ssh_port:) nil end - # @param [Array] components_with_override_command_enabled - # @return [void] - def self.override_command_and_args(components_with_override_command_enabled:) - # This overrides the entrypoint command for all containers that don't have the - # `overrideCommand` attribute set to false in the devfile. - - components_with_override_command_enabled.each do |component| - container = component[:container] - next unless container - - container[:command] = %w[/bin/sh -c] - container[:args] = [MAIN_COMPONENT_UPDATER_CONTAINER_ARGS] - end - - nil - end - - private_class_method :update_env_vars, :update_endpoints, :override_command_and_args + private_class_method :update_env_vars, :update_endpoints end end end diff --git a/ee/spec/fixtures/remote_development/example.container-commands-updated-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.container-commands-updated-devfile.yaml.erb new file mode 100644 index 00000000000000..1538644cea6f43 --- /dev/null +++ b/ee/spec/fixtures/remote_development/example.container-commands-updated-devfile.yaml.erb @@ -0,0 +1,85 @@ +--- +schemaVersion: 2.2.0 +metadata: {} +components: + - name: tooling-container + attributes: + gl/inject-editor: true + overrideCommand: true + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + args: + - | + <%= indent_yaml_literal(CONTAINER_KEEPALIVE_COMMAND_ARGS, 10) %> + command: + - "/bin/sh" + - "-c" + env: + - name: GL_TOOLS_DIR + value: "/projects/.gl-tools" + - name: GL_VSCODE_LOG_LEVEL + value: "info" + - name: GL_VSCODE_PORT + value: "<%= WORKSPACE_EDITOR_PORT %>" + - name: GL_SSH_PORT + value: "<%= WORKSPACE_SSH_PORT %>" + - name: GL_VSCODE_ENABLE_MARKETPLACE + value: "false" + endpoints: + - name: editor-server + targetPort: <%= WORKSPACE_EDITOR_PORT %> + exposure: public + secure: true + protocol: https + - name: ssh-server + targetPort: <%= WORKSPACE_SSH_PORT %> + exposure: internal + secure: true + dedicatedPod: false + mountSources: true + - name: database-container + attributes: + overrideCommand: false + container: + image: mysql + env: + - name: MYSQL_ROOT_PASSWORD + value: "my-secret-pw" + dedicatedPod: false + mountSources: true + - name: user-defined-entrypoint-cmd-component + attributes: + overrideCommand: false + container: + image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo + command: ["echo"] + args: ["-n", "user-defined entrypoint command"] + dedicatedPod: false + mountSources: true + - name: gl-tools-injector + attributes: + overrideCommand: false + container: + image: <%= WORKSPACE_TOOLS_IMAGE %> + env: + - name: GL_TOOLS_DIR + value: "/projects/.gl-tools" + memoryLimit: 512Mi + memoryRequest: 256Mi + cpuLimit: 500m + cpuRequest: 100m +events: + preStart: + - gl-tools-injector-command + postStart: + - user-defined-command +commands: + - id: user-defined-command + exec: + component: tooling-container + commandLine: echo 'user-defined postStart command' + hotReloadCapable: false + - id: gl-tools-injector-command + apply: + component: gl-tools-injector +variables: {} diff --git a/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb index a99f3d79e29317..b168db6c869e44 100644 --- a/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.internal-poststart-commands-inserted-devfile.yaml.erb @@ -5,11 +5,12 @@ components: - name: tooling-container attributes: gl/inject-editor: true + overrideCommand: true container: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo args: - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + <%= indent_yaml_literal(CONTAINER_KEEPALIVE_COMMAND_ARGS, 10) %> command: - "/bin/sh" - "-c" @@ -37,14 +38,10 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: false container: image: mysql - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" @@ -60,6 +57,8 @@ components: dedicatedPod: false mountSources: true - name: gl-tools-injector + attributes: + overrideCommand: false container: image: <%= WORKSPACE_TOOLS_IMAGE %> env: diff --git a/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb index 2cbe720c136911..985524f024d513 100644 --- a/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.legacy-no-poststart-in-container-command-processed-devfile.yaml.erb @@ -43,12 +43,6 @@ components: - name: database-container container: image: mysql - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" volumeMounts: - name: gl-workspace-data path: /projects diff --git a/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb index d16edac4155796..c76197c98c6d98 100644 --- a/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.legacy-poststart-in-container-command-processed-devfile.yaml.erb @@ -9,7 +9,7 @@ components: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo args: - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + <%= indent_yaml_literal(CONTAINER_KEEPALIVE_COMMAND_ARGS, 10) %> command: - "/bin/sh" - "-c" @@ -42,12 +42,6 @@ components: - name: database-container container: image: mysql - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" volumeMounts: - name: gl-workspace-data path: /projects diff --git a/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb index d4cc47e1886296..dee97fca4a270f 100644 --- a/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.main-container-updated-devfile.yaml.erb @@ -7,12 +7,6 @@ components: gl/inject-editor: true container: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" env: - name: GL_TOOLS_DIR value: "/projects/.gl-tools" @@ -39,12 +33,6 @@ components: - name: database-container container: image: mysql - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" diff --git a/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb index d4cc47e1886296..dee97fca4a270f 100644 --- a/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.main-container-updated-marketplace-disabled-devfile.yaml.erb @@ -7,12 +7,6 @@ components: gl/inject-editor: true container: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" env: - name: GL_TOOLS_DIR value: "/projects/.gl-tools" @@ -39,12 +33,6 @@ components: - name: database-container container: image: mysql - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" env: - name: MYSQL_ROOT_PASSWORD value: "my-secret-pw" diff --git a/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb index bd04329845018c..3bca9e092e7832 100644 --- a/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.processed-devfile.yaml.erb @@ -5,11 +5,12 @@ components: - name: tooling-container attributes: gl/inject-editor: true + overrideCommand: true container: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo args: - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> + <%= indent_yaml_literal(CONTAINER_KEEPALIVE_COMMAND_ARGS, 10) %> command: - "/bin/sh" - "-c" @@ -40,14 +41,10 @@ components: dedicatedPod: false mountSources: true - name: database-container + attributes: + overrideCommand: false container: image: mysql - args: - - | - <%= indent_yaml_literal(MAIN_COMPONENT_UPDATER_CONTAINER_ARGS, 10) %> - command: - - "/bin/sh" - - "-c" volumeMounts: - name: gl-workspace-data path: /projects @@ -69,6 +66,8 @@ components: dedicatedPod: false mountSources: true - name: gl-tools-injector + attributes: + overrideCommand: false container: image: <%= WORKSPACE_TOOLS_IMAGE %> volumeMounts: diff --git a/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb new file mode 100644 index 00000000000000..bf60aee9e77613 --- /dev/null +++ b/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require "fast_spec_helper" + +RSpec.describe RemoteDevelopment::WorkspaceOperations::Create::ContainerCommandUpdater, feature_category: :workspaces do + include_context 'with remote development shared fixtures' + + let(:input_processed_devfile) do + read_devfile("example.main-container-updated-devfile.yaml.erb") + end + + let(:expected_processed_devfile_name) { "example.container-commands-updated-devfile.yaml.erb" } + let(:expected_processed_devfile) { read_devfile(expected_processed_devfile_name) } + + let(:vscode_extension_marketplace_metadata_enabled) { false } + + let(:context) do + { + processed_devfile: input_processed_devfile, + tools_dir: "#{workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH}/" \ + "#{create_constants_module::TOOLS_DIR_NAME}", + vscode_extension_marketplace_metadata: { enabled: vscode_extension_marketplace_metadata_enabled } + } + end + + subject(:returned_value) do + described_class.update(context) # rubocop:disable Rails/SaveBang -- Silly rubocop, this isn't an ActiveRecord object + end + + it 'preserves script formatting' do + expected = expected_processed_devfile[:components].first[:container][:args].first + actual = returned_value[:processed_devfile][:components].first[:container][:args].first + expect(actual).to eq(expected) + end + + it "adds the overrideCommand attribute for components when not specified in the devfile" do + result = returned_value[:processed_devfile] + components = result[:components] + + # Find the main component (has gl/inject-editor: true) + main_component = components.find { |c| c[:name] == "tooling-container" } + expect(main_component[:attributes][:overrideCommand]).to be true + + # Find non-main components that don't already have overrideCommand set + database_component = components.find { |c| c[:name] == "database-container" } + expect(database_component[:attributes][:overrideCommand]).to be false + + tools_injector_component = components.find { |c| c[:name] == "gl-tools-injector" } + expect(tools_injector_component[:attributes][:overrideCommand]).to be false + + # Verify that existing overrideCommand values are preserved + user_defined_component = components.find { |c| c[:name] == "user-defined-entrypoint-cmd-component" } + expect(user_defined_component[:attributes][:overrideCommand]).to be false + end + + it "updates container command and args for components with overrideCommand: true" do + result = returned_value[:processed_devfile] + components = result[:components] + + # Main component should have its command and args updated + main_component = components.find { |c| c[:name] == "tooling-container" } + expect(main_component[:container][:command]).to eq(["/bin/sh", "-c"]) + expect(main_component[:container][:args]).to eq([files_module::CONTAINER_KEEPALIVE_COMMAND_ARGS]) + + # Non-main components should not have their command/args updated + database_component = components.find { |c| c[:name] == "database-container" } + expect(database_component[:container]).not_to have_key(:command) + expect(database_component[:container]).not_to have_key(:args) + end +end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/internal_poststart_commands_inserter_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/internal_poststart_commands_inserter_spec.rb index b7fe8073659d93..dc8df8225250d8 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/internal_poststart_commands_inserter_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/internal_poststart_commands_inserter_spec.rb @@ -6,7 +6,7 @@ include_context "with remote development shared fixtures" let(:input_processed_devfile) do - read_devfile("example.main-container-updated-devfile.yaml.erb") + read_devfile("example.container-commands-updated-devfile.yaml.erb") end let(:expected_processed_devfile_name) { "example.internal-poststart-commands-inserted-devfile.yaml.erb" } diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb index 65c04958599bf8..e2a8953cb5c682 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_component_updater_spec.rb @@ -31,38 +31,6 @@ expect(returned_value[:processed_devfile]).to eq(expected_processed_devfile) end - it 'preserves script formatting' do - expected = expected_processed_devfile[:components].first[:container][:args].first - actual = returned_value[:processed_devfile][:components].first[:container][:args].first - expect(actual).to eq(expected) - end - - it "overrides container default entrypoint command and args only if overrideCommand is not explicitly set to false" do - result = returned_value[:processed_devfile] - components = result[:components] - - component_with_override_not_explicitly_disabled = components.find { |c| c[:name] == "tooling-container" } - component_with_override_explicitly_disabled = components.find do |c| - c[:name] == "user-defined-entrypoint-cmd-component" - end - internal_component = components.find { |c| c[:name] == "gl-tools-injector" } - - # When overrideCommand is not provided - should override with tail command (default behavior) - expect(component_with_override_not_explicitly_disabled[:container][:command]).to eq(["/bin/sh", "-c"]) - expect(component_with_override_not_explicitly_disabled[:container][:args]).to eq(["tail -f /dev/null\n"]) - - # When overrideCommand: false - should preserve original command/args if provided - expect(component_with_override_explicitly_disabled[:container][:command]).to eq(["echo"]) - expect(component_with_override_explicitly_disabled[:container][:args]).to eq([ - "-n", - "user-defined entrypoint command" - ]) - - # Does not affect internal components with the restricted gl prefix - expect(internal_component[:container][:command]).not_to eq(["/bin/sh", "-c"]) - expect(internal_component[:container][:args]).not_to eq(["tail -f /dev/null\n"]) - end - context "when vscode_extension_marketplace_metadata Web IDE setting is disabled" do let(:expected_processed_devfile_name) { "example.main-container-updated-marketplace-disabled-devfile.yaml.erb" } let(:vscode_extension_marketplace_metadata_enabled) { false } diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb index 0552c6ba8e7ddb..741b9b686a8538 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb @@ -12,6 +12,7 @@ [RemoteDevelopment::WorkspaceOperations::Create::VolumeDefiner, :map], [RemoteDevelopment::WorkspaceOperations::Create::ToolsInjectorComponentInserter, :map], [RemoteDevelopment::WorkspaceOperations::Create::MainComponentUpdater, :map], + [RemoteDevelopment::WorkspaceOperations::Create::ContainerCommandUpdater, :map], [RemoteDevelopment::WorkspaceOperations::Create::InternalPoststartCommandsInserter, :map], [RemoteDevelopment::WorkspaceOperations::Create::VolumeComponentInserter, :map], [RemoteDevelopment::WorkspaceOperations::Create::Creator, :and_then], diff --git a/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb index adad70263dc20a..f3a07feb480e50 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/tools_injector_component_inserter_spec.rb @@ -48,8 +48,10 @@ it 'uses image override' do components = returned_value.dig(:processed_devfile, :components) - gl_tools_component = components&.find { |component| component[:name] == "gl-tools-injector" } - image_from_processed_devfile = gl_tools_component&.dig(:container, :image) + gl_tools_component = components.find do |component| + component[:name] == create_constants_module::TOOLS_INJECTOR_COMPONENT_NAME + end + image_from_processed_devfile = gl_tools_component.dig(:container, :image) expect(image_from_processed_devfile).to eq(tools_injector_image_from_settings) end diff --git a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb index cdc60d9bb1c6a4..f1cf2b5bc73a66 100644 --- a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb +++ b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb @@ -650,7 +650,7 @@ def workspace_deployment( runtimeClassName: default_runtime_class, containers: [ { - args: [files_module::MAIN_COMPONENT_UPDATER_CONTAINER_ARGS], + args: [files_module::CONTAINER_KEEPALIVE_COMMAND_ARGS], command: %w[/bin/sh -c], env: [ { @@ -760,8 +760,6 @@ def workspace_deployment( imagePullPolicy: "Always", name: "database-container", resources: default_resources_per_workspace_container, - args: [files_module::MAIN_COMPONENT_UPDATER_CONTAINER_ARGS], - command: %w[/bin/sh -c], volumeMounts: [ { mountPath: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH, @@ -786,6 +784,12 @@ def workspace_deployment( ] }, { + image: "quay.io/mloriedo/universal-developer-image:ubi8-dw-demo", + imagePullPolicy: "Always", + name: "user-defined-entrypoint-cmd-component", + resources: default_resources_per_workspace_container, + command: ["echo"], + args: ["-n", "user-defined entrypoint command"], env: [ { name: "PROJECTS_ROOT", @@ -796,12 +800,14 @@ def workspace_deployment( value: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH } ], - image: "quay.io/mloriedo/universal-developer-image:ubi8-dw-demo", - imagePullPolicy: "Always", - name: "user-defined-entrypoint-cmd-component", - resources: default_resources_per_workspace_container, - command: ["echo"], - args: ["-n", "user-defined entrypoint command"], + securityContext: container_security_context, + envFrom: [ + { + secretRef: { + name: "#{workspace_name}-env-var" + } + } + ], volumeMounts: [ { mountPath: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH, @@ -815,14 +821,6 @@ def workspace_deployment( mountPath: create_constants_module::WORKSPACE_SCRIPTS_VOLUME_PATH, name: create_constants_module::WORKSPACE_SCRIPTS_VOLUME_NAME } - ], - securityContext: container_security_context, - envFrom: [ - { - secretRef: { - name: "#{workspace_name}-env-var" - } - } ] } ], -- GitLab From b750e98e2517da43ab65f8e9e08f40a740c53078 Mon Sep 17 00:00:00 2001 From: Daniyal Arshad Date: Tue, 29 Jul 2025 18:08:34 -0400 Subject: [PATCH 6/7] Update docs to include overrideCommand defaults per component type --- doc/user/workspace/_index.md | 11 ++++++++--- .../create/container_command_updater.rb | 4 +--- .../create/container_command_updater_spec.rb | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/doc/user/workspace/_index.md b/doc/user/workspace/_index.md index 689c94cc5f6570..549490b1b5e113 100644 --- a/doc/user/workspace/_index.md +++ b/doc/user/workspace/_index.md @@ -224,15 +224,20 @@ The `container` component type supports the following schema properties only: | `env` | Environment variables to use in the container. Names must not start with `gl-`. | | `endpoints` | Port mappings to expose from the container. Names must not start with `gl-`. | | `volumeMounts` | Storage volume to mount in the container. | -| `overrideCommand` | Override the container entrypoint with a keep-alive command. | +| `overrideCommand` | Override the container entrypoint with a keep-alive command. Defaults vary by component type. | #### `overrideCommand` attribute -The `overrideCommand` attribute is a boolean that control how Workspaces handle container entrypoints. +The `overrideCommand` attribute is a boolean that controls how Workspaces handle container entrypoints. This attribute determines whether the container's original entrypoint is preserved or replaced with a keep-alive command. -When `true` (default), the container entrypoint is replaced with `tail -f /dev/null` to keep the +The default value for `overrideCommand` depends on the component type: + +- Main component with attribute `gl/inject-editor: true`: Defaults to `true` when not specified. +- All other components: Defaults to `false` when not specified. + +When `true`, the container entrypoint is replaced with `tail -f /dev/null` to keep the container running. When `false`, the container uses either the devfile component `command`/`args` or the built container image's `Entrypoint`/`Cmd`. diff --git a/ee/lib/remote_development/workspace_operations/create/container_command_updater.rb b/ee/lib/remote_development/workspace_operations/create/container_command_updater.rb index 97f283e0c7b8f1..d85458060c13a7 100644 --- a/ee/lib/remote_development/workspace_operations/create/container_command_updater.rb +++ b/ee/lib/remote_development/workspace_operations/create/container_command_updater.rb @@ -16,9 +16,6 @@ def self.update(context) } } - # This overrides the entrypoint command for all containers that have the - # `overrideCommand` attribute set to true in the devfile. - components.each do |component| attributes = component[:attributes] ||= {} @@ -37,6 +34,7 @@ def self.update(context) container = component[:container] next unless container + # This overrides the entrypoint command for the container container[:command] = %w[/bin/sh -c] container[:args] = [CONTAINER_KEEPALIVE_COMMAND_ARGS] end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb index bf60aee9e77613..1281ea3be1ee5b 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb @@ -37,7 +37,7 @@ result = returned_value[:processed_devfile] components = result[:components] - # Find the main component (has gl/inject-editor: true) + # Find the main component main_component = components.find { |c| c[:name] == "tooling-container" } expect(main_component[:attributes][:overrideCommand]).to be true -- GitLab From 802fe622dec2bbfaec49f4b815d90d4436285913 Mon Sep 17 00:00:00 2001 From: Daniyal Arshad Date: Thu, 31 Jul 2025 15:11:34 -0400 Subject: [PATCH 7/7] Address review suggestions(3) --- .../devfile_operations/restrictions_enforcer.rb | 5 ++++- ...-override-command-with-non-boolean-value-devfile.yaml.erb | 2 +- .../create/container_command_updater_spec.rb | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb index 999c084fb12d0e..5f73e2ffa619a4 100644 --- a/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb +++ b/ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb @@ -27,6 +27,9 @@ class RestrictionsEnforcer # Currently, we only support the default value `false` for the `hotReloadCapable` option SUPPORTED_HOT_RELOAD_VALUE = false + # Override command must be a boolean value + SUPPORTED_OVERRIDE_COMMAND_VALUE = [true, false].freeze + # @param [Hash] parent_context # @return [Gitlab::Fp::Result] def self.enforce(parent_context) @@ -236,7 +239,7 @@ def self.validate_containers(context) end override_command = attributes.fetch(:overrideCommand, true) - unless override_command.is_a?(TrueClass) || override_command.is_a?(FalseClass) + unless SUPPORTED_OVERRIDE_COMMAND_VALUE.include?(override_command) return append_err( format( _("Property 'overrideCommand' of component '%{name}' must be a boolean (true or false)"), diff --git a/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb b/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb index f2aa343658f138..c44f9ddbc50a9d 100644 --- a/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb +++ b/ee/spec/fixtures/remote_development/example.invalid-attributes-override-command-with-non-boolean-value-devfile.yaml.erb @@ -4,7 +4,7 @@ components: - name: tooling-container attributes: gl/inject-editor: true - overrideCommand: foobar + overrideCommand: "true" container: image: quay.io/mloriedo/universal-developer-image:ubi8-dw-demo command: ["echo"] diff --git a/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb index 1281ea3be1ee5b..99de3c5d69a4c5 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/container_command_updater_spec.rb @@ -59,7 +59,7 @@ # Main component should have its command and args updated main_component = components.find { |c| c[:name] == "tooling-container" } - expect(main_component[:container][:command]).to eq(["/bin/sh", "-c"]) + expect(main_component[:container][:command]).to eq(%w[/bin/sh -c]) expect(main_component[:container][:args]).to eq([files_module::CONTAINER_KEEPALIVE_COMMAND_ARGS]) # Non-main components should not have their command/args updated -- GitLab