diff --git a/ee/lib/remote_development/README.md b/ee/lib/remote_development/README.md index ad525aa18eee1d8394bf9a541354b79e1be9ae85..6f0b380ad5995446c8aa0d3f615f761fbb72ee51 100644 --- a/ee/lib/remote_development/README.md +++ b/ee/lib/remote_development/README.md @@ -202,6 +202,169 @@ that the types are correct. See [this comment thread](https://gitlab.com/gitlab- Also note that the destructuring a hash or array, even without the type checks (e.g. `x => {y: i}`), is still a form of type safety, because it will raise a `NoMatchingPatternKeyError` exception if the hash or array does not have the expected structure. +#### Pattern matching with arrays + +This approach can also be used with structures that are a mix of arrays and hashes. In this example, the call to `#find` +to search for a matching hash within an array can be replaced with +an equivalent pattern matching structure which uses the `*_` syntax to match any number of elements. + +```ruby +haystack = [ + { type: :straw, value: :nope }, + { type: :needle, value: "FOUND IT!" }, + { type: :dirt, value: :nope }, +] + +puts haystack.find { |item| item.fetch(:type) == :needle }.fetch(:value) +# => "FOUND IT!" + +haystack => [ + *_, + { type: :needle, value: value }, + *_ +] +puts value +# => "FOUND IT!" + +begin + haystack.find { |item| item.fetch(:type) == :gold }.fetch(:value) +rescue => e + # Less descriptive error message: + p e + # # +end + +begin + haystack => [ + *_, + { type: :missing_gold, value: String => value }, + *_ + ] +rescue => e + # More descriptive error message: + p e + # #:straw, :value=>:nope}, {:type=>:needle, :value=>"FOUND IT!"}, {:type=>:dirt, :value=>:nope}]: [{:type=>:straw, :value=>:nope}, {:type=>:needle, :value=>"FOUND IT!"}, {:type=>:dirt, :value=>:nope}] does not match to find pattern> +end +``` + +This approach can also be used to concisely replace combinations of `#find` and `#dig` for deeply-nested +structures. + +```ruby +array = [ + { key: 1 }, + { + key: 2, + a: { + b: { + value: "The value", + } + } + }, + { key: 3 } +] + +value = array.find { |item| item.fetch(:key) == 2 }.dig(:a, :b, :value) +puts value.length +# => 9 + +array => [ + *_, + { + key: 2, + a: { + b: { + value: value + } + } + }, + *_ +] +puts value.length +# => 9 +``` + +#### Type safety for hash keys: `#dig` vs. pattern matching vs. `#fetch` + +In cases where the expected key is not found, pattern matching will fail with a descriptive error, where +`#dig` will just return `nil`: + +```ruby +array = [ + { key: 1 }, + { + key: 2, + a: { + b: { + value: "The value", + } + } + }, + { key: 3 } +] + +# With #dig, an incorrect key will allow nil to be returned: +value = array.find { |item| item.fetch(:key) == 2 }.dig(:MISSING, :b, :value) +begin + puts value.length +rescue => e + # No null-safety, less descriptive error message: + p e + # # +end + +# With pattern matching, an incorrect key will raise a descriptive error: +begin + array => [ + *_, + { + key: 2, + MISSING: { + b: { + value: value + } + } + }, + *_ + ] +rescue => e + # More descriptive error message: + p e + # #1}, {:key=>2, :a=>{:b=>{:value=>"The value"}}}, {:key=>3}]: [{:key=>1}, {:key=>2, :a=>{:b=>{:value=>"The value"}}}, {:key=>3}] does not match to find pattern> +end +``` + +You can also get null-safety if the `#dig` is replaced with multiple chained `#fetch` calls. + +This gives a more descriptive error message than dig, but it is different than pattern matching. +`#fetch` will tell you the missing key, but won't print out the full data structure for you to look +at. Pattern matching doesn't tell you the missing key, but will print out the full data structure. + +```ruby +array => [ + *_, + { + key: 2, + a: { + b: { + value: value + } + } + }, + *_ +] + +# You can get nil safety by using #fetch instead of #dig +begin + array.find { |item| item.fetch(:key) == 2 }.fetch(:a).fetch(:MISSING).fetch(:value) +rescue => e + p e + # # +end +``` + +You can choose which is more appropriate for your use case. + #### Do not use type safety on unvalidated user-provided values We do not want to use type safety on values which come directly from user input, and have not yet been validated. diff --git a/ee/lib/remote_development/settings/network_policy_egress_validator.rb b/ee/lib/remote_development/settings/network_policy_egress_validator.rb index 38198caacc232ec8822e4372f29e4957010e81a6..dbffedd9df201918f3c4929e1c38cb3fa648e8ff 100644 --- a/ee/lib/remote_development/settings/network_policy_egress_validator.rb +++ b/ee/lib/remote_development/settings/network_policy_egress_validator.rb @@ -5,6 +5,8 @@ module Settings class NetworkPolicyEgressValidator include Messages + # @param [Hash] context + # @return [Gitlab::Fp::Result] def self.validate(context) unless context.fetch(:requested_setting_names).include?(:network_policy_egress) return Gitlab::Fp::Result.ok(context) @@ -15,12 +17,13 @@ def self.validate(context) network_policy_egress: Array => network_policy_egress, } } - network_policy_egress = network_policy_egress.map do |element| - next element unless element.is_a?(Hash) - element.deep_stringify_keys - end - errors = validate_against_schema(network_policy_egress) + # NOTE: We deep_stringify_keys here because even though they will be strings in a real request, + # we use symbols during tests. JSON schema validators are the only place where keys need + # to be strings. All other internal logic uses symbols. + network_policy_egress_stringified_keys = network_policy_egress.map(&:deep_stringify_keys) + + errors = validate_against_schema(network_policy_egress_stringified_keys) if errors.none? Gitlab::Fp::Result.ok(context) 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 673dc80e7486e2d22f4448834ed0b8f4a2ee7894..f85ea15bddf40f8ce344ca908a14a7908608387f 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb @@ -97,9 +97,9 @@ def self.parse_devfile_yaml(context) begin # load YAML, convert YAML to JSON and load it again to remove YAML vulnerabilities - devfile_stringified = YAML.safe_load(YAML.safe_load(devfile_yaml).to_json) + devfile_to_json_and_back_to_yaml = YAML.safe_load(YAML.safe_load(devfile_yaml).to_json) # symbolize keys for domain logic processing of devfile (to_h is to avoid nil dereference error in RubyMine) - devfile = devfile_stringified.to_h.deep_symbolize_keys + devfile = devfile_to_json_and_back_to_yaml.to_h.deep_symbolize_keys rescue RuntimeError, JSON::GeneratorError => e return Gitlab::Fp::Result.err(WorkspaceCreateDevfileYamlParseFailed.new( details: "Devfile YAML could not be parsed: #{e.message}" diff --git a/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb b/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb index 31bb7c6917e3731132f823b65054267b3445bd9d..2a91efc35d435a46fd9f276d8c007bdf568259c9 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb @@ -12,7 +12,8 @@ def self.flatten(context) context => { devfile: Hash => devfile } begin - flattened_devfile_yaml = Devfile::Parser.flatten(YAML.dump(devfile.deep_stringify_keys)) + devfile_yaml = YAML.dump(devfile.deep_stringify_keys) + flattened_devfile_yaml = Devfile::Parser.flatten(devfile_yaml) rescue Devfile::CliError => e return Gitlab::Fp::Result.err(WorkspaceCreateDevfileFlattenFailed.new(details: e.message)) end @@ -21,10 +22,7 @@ def self.flatten(context) # Devfile::Parser gem will not produce invalid YAML. We own the gem, and will fix and add any regression # tests in the gem itself. No need to make this domain code more complex, more coupled, and less # cohesive by unnecessarily adding defensive coding against other code we also own. - processed_devfile_stringified = YAML.safe_load(flattened_devfile_yaml) - - # symbolize keys for domain logic processing of devfile (to_h is to avoid nil dereference error in RubyMine) - processed_devfile = processed_devfile_stringified.to_h.deep_symbolize_keys + processed_devfile = YAML.safe_load(flattened_devfile_yaml).to_h.deep_symbolize_keys processed_devfile[:components] ||= [] processed_devfile[:commands] ||= [] 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 f472c33892ea393699234091a9a10126aca5c853..f0dcf6e2433535d9956f2642d920dd6720412a98 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 @@ -13,17 +13,20 @@ class MainComponentUpdater # @return [Hash] def self.update(context) context => { - processed_devfile: Hash => processed_devfile, + processed_devfile: { + components: Array => components + }, tools_dir: String => tools_dir, vscode_extension_marketplace_metadata: Hash => vscode_extension_marketplace_metadata } # NOTE: We will always have exactly one main_component found, because we have already # validated this in devfile_validator.rb - main_component = - processed_devfile - .fetch(:components) - .find { |component| component.dig(:attributes, MAIN_COMPONENT_INDICATOR_ATTRIBUTE.to_sym) } + main_component = components.find do |component| + # NOTE: We can't use pattern matching here, because constants can't be used in pattern matching. + # Otherwise, we could do this all in a single pattern match. + component.dig(:attributes, MAIN_COMPONENT_INDICATOR_ATTRIBUTE.to_sym) + end container = main_component.fetch(:container) diff --git a/ee/lib/remote_development/workspace_operations/create/workspace_variables.rb b/ee/lib/remote_development/workspace_operations/create/workspace_variables.rb index 45320acc0aefe7b52def7523ae80fa5c107f48a1..01006681d12482eab9eb79e9b9384e640dbc96b0 100644 --- a/ee/lib/remote_development/workspace_operations/create/workspace_variables.rb +++ b/ee/lib/remote_development/workspace_operations/create/workspace_variables.rb @@ -40,92 +40,92 @@ def self.variables( workspace_id: workspace_id }, { - key: 'GIT_CONFIG_COUNT', - value: '3', + key: "GIT_CONFIG_COUNT", + value: "3", variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GIT_CONFIG_KEY_0', + key: "GIT_CONFIG_KEY_0", value: "credential.helper", variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GIT_CONFIG_VALUE_0', + key: "GIT_CONFIG_VALUE_0", value: GIT_CREDENTIAL_STORE_SCRIPT_FILE, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GIT_CONFIG_KEY_1', + key: "GIT_CONFIG_KEY_1", value: "user.name", variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GIT_CONFIG_VALUE_1', + key: "GIT_CONFIG_VALUE_1", value: user_name, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GIT_CONFIG_KEY_2', + key: "GIT_CONFIG_KEY_2", value: "user.email", variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GIT_CONFIG_VALUE_2', + key: "GIT_CONFIG_VALUE_2", value: user_email, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GL_GIT_CREDENTIAL_STORE_SCRIPT_FILE', + key: "GL_GIT_CREDENTIAL_STORE_SCRIPT_FILE", value: GIT_CREDENTIAL_STORE_SCRIPT_FILE, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GL_TOKEN_FILE_PATH', + key: "GL_TOKEN_FILE_PATH", value: TOKEN_FILE, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GL_WORKSPACE_DOMAIN_TEMPLATE', + key: "GL_WORKSPACE_DOMAIN_TEMPLATE", value: "${PORT}-#{name}.#{dns_zone}", variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GL_EDITOR_EXTENSIONS_GALLERY_SERVICE_URL', + key: "GL_EDITOR_EXTENSIONS_GALLERY_SERVICE_URL", value: vscode_extensions_gallery_service_url, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GL_EDITOR_EXTENSIONS_GALLERY_ITEM_URL', + key: "GL_EDITOR_EXTENSIONS_GALLERY_ITEM_URL", value: vscode_extensions_gallery_item_url, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GL_EDITOR_EXTENSIONS_GALLERY_RESOURCE_URL_TEMPLATE', + key: "GL_EDITOR_EXTENSIONS_GALLERY_RESOURCE_URL_TEMPLATE", value: vscode_extensions_gallery_resource_url_template, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, # variables with prefix `GITLAB_WORKFLOW_` are used for configured GitLab Workflow extension for VS Code { - key: 'GITLAB_WORKFLOW_INSTANCE_URL', + key: "GITLAB_WORKFLOW_INSTANCE_URL", value: Gitlab::Routing.url_helpers.root_url, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GITLAB_WORKFLOW_TOKEN_FILE', + key: "GITLAB_WORKFLOW_TOKEN_FILE", value: TOKEN_FILE, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id diff --git a/ee/lib/remote_development/workspace_operations/reconcile/input/params_validator.rb b/ee/lib/remote_development/workspace_operations/reconcile/input/params_validator.rb index 82396ca62507d61a58f36cb0f1c050897088e2a0..9911d8a4ccc7c6e583f6704a271e55f17f740b1a 100644 --- a/ee/lib/remote_development/workspace_operations/reconcile/input/params_validator.rb +++ b/ee/lib/remote_development/workspace_operations/reconcile/input/params_validator.rb @@ -13,10 +13,9 @@ class ParamsValidator def self.validate(context) context => { original_params: Hash => original_params } - # NOTE: We deep_stringify_keys here, because even though they will be strings in the a real request, - # so we can still pass keys as symbols during tests, and not have to worry about passing string - # keys in tests. This is the only place where keys need to be strings, because of the JSON schema - # validation, all other places we convert and work with the keys as symbols. + # NOTE: We deep_stringify_keys here because even though they will be strings in a real request, + # we use symbols during tests. JSON schema validators are the only place where keys need + # to be strings. All other internal logic uses symbols. errors = validate_original_params_against_schema(original_params.deep_stringify_keys) if errors.none? diff --git a/ee/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator.rb b/ee/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator.rb index 8c1fcdeee06ef6d81634aa021e65bef98a9cd59f..968cef0d3b41631e5dac1d013f443d41783b6b55 100644 --- a/ee/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator.rb +++ b/ee/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator.rb @@ -61,12 +61,12 @@ def self.generate_desired_config(workspace:, include_all_resources:, logger:) default_runtime_class: workspaces_agent_config.default_runtime_class } - # TODO: https://gitlab.com/groups/gitlab-org/-/epics/12225 - handle error k8s_resources_for_workspace_core = DevfileParser.get_all( processed_devfile: workspace.processed_devfile, k8s_resources_params: k8s_resources_params, logger: logger ) + # If we got no resources back from the devfile parser, this indicates some error was encountered in parsing # the processed_devfile. So we return an empty array which will result in no updates being applied by the # agent. We should not continue on and try to add anything else to the resources, as this would result @@ -193,7 +193,7 @@ def self.get_k8s_resources_for_secrets( data_for_environment = workspace.workspace_variables.with_variable_type_environment data_for_environment = data_for_environment.each_with_object({}) do |workspace_variable, hash| - hash[workspace_variable.key] = workspace_variable.value + hash[workspace_variable.key.to_sym] = workspace_variable.value end k8s_secret_for_environment = get_secret( name: env_secret_name, @@ -205,7 +205,7 @@ def self.get_k8s_resources_for_secrets( data_for_file = workspace.workspace_variables.with_variable_type_file data_for_file = data_for_file.each_with_object({}) do |workspace_variable, hash| - hash[workspace_variable.key] = workspace_variable.value + hash[workspace_variable.key.to_sym] = workspace_variable.value end k8s_secret_for_file = get_secret( name: file_secret_name, @@ -237,8 +237,8 @@ def self.get_workspace_replicas(desired_state:) # @return [Hash] def self.get_inventory_config_map(name:, namespace:, agent_labels:, agent_annotations:, agent_id:) extra_labels = { - 'cli-utils.sigs.k8s.io/inventory-id' => name, - 'agent.gitlab.com/id' => agent_id.to_s + 'cli-utils.sigs.k8s.io/inventory-id': name, + 'agent.gitlab.com/id': agent_id.to_s } labels = agent_labels.merge(extra_labels) { @@ -250,7 +250,7 @@ def self.get_inventory_config_map(name:, namespace:, agent_labels:, agent_annota labels: labels, annotations: agent_annotations } - }.deep_stringify_keys.to_h + } end # @param [Hash] agent_labels @@ -272,14 +272,14 @@ def self.get_merged_labels_and_annotations( max_resources_per_workspace: ) extra_labels = { - 'agent.gitlab.com/id' => agent_id.to_s + 'agent.gitlab.com/id': agent_id.to_s } labels = agent_labels.merge(extra_labels) extra_annotations = { - 'config.k8s.io/owning-inventory' => owning_inventory.to_s, - 'workspaces.gitlab.com/host-template' => domain_template.to_s, - 'workspaces.gitlab.com/id' => workspace_id.to_s, - 'workspaces.gitlab.com/max-resources-per-workspace-sha256' => + 'config.k8s.io/owning-inventory': owning_inventory.to_s, + 'workspaces.gitlab.com/host-template': domain_template.to_s, + 'workspaces.gitlab.com/id': workspace_id.to_s, + 'workspaces.gitlab.com/max-resources-per-workspace-sha256': OpenSSL::Digest::SHA256.hexdigest(max_resources_per_workspace.sort.to_h.to_s) } annotations = agent_annotations.merge(extra_annotations) @@ -303,7 +303,7 @@ def self.get_secret(name:, namespace:, labels:, annotations:, data:) annotations: annotations }, data: data.transform_values { |v| Base64.strict_encode64(v) } - }.deep_stringify_keys.to_h + } end # @param [String] name @@ -378,7 +378,7 @@ def self.get_network_policy( podSelector: {}, policyTypes: policy_types } - }.deep_stringify_keys.to_h + } end # @param [String] name @@ -394,6 +394,17 @@ def self.get_resource_quota( annotations:, max_resources_per_workspace: ) + max_resources_per_workspace => { + limits: { + cpu: limits_cpu, + memory: limits_memory + }, + requests: { + cpu: requests_cpu, + memory: requests_memory + } + } + { apiVersion: "v1", kind: "ResourceQuota", @@ -405,13 +416,13 @@ def self.get_resource_quota( }, spec: { hard: { - "limits.cpu": max_resources_per_workspace.dig(:limits, :cpu), - "limits.memory": max_resources_per_workspace.dig(:limits, :memory), - "requests.cpu": max_resources_per_workspace.dig(:requests, :cpu), - "requests.memory": max_resources_per_workspace.dig(:requests, :memory) + "limits.cpu": limits_cpu, + "limits.memory": limits_memory, + "requests.cpu": requests_cpu, + "requests.memory": requests_memory } } - }.deep_stringify_keys.to_h + } end # @param [String] name @@ -433,7 +444,7 @@ def self.get_image_pull_secrets_service_account(name:, namespace:, labels:, anno }, automountServiceAccountToken: false, imagePullSecrets: image_pull_secrets_names - }.deep_stringify_keys.to_h + } end end end diff --git a/ee/lib/remote_development/workspace_operations/reconcile/output/devfile_parser.rb b/ee/lib/remote_development/workspace_operations/reconcile/output/devfile_parser.rb index f77d3a3840f9f966f66a0d6a33c51aeaf370bd2e..ec40c7eb169c7ca7dcf9faa6cafdee0602328259 100644 --- a/ee/lib/remote_development/workspace_operations/reconcile/output/devfile_parser.rb +++ b/ee/lib/remote_development/workspace_operations/reconcile/output/devfile_parser.rb @@ -70,7 +70,8 @@ def self.get_all(processed_devfile:, k8s_resources_params:, logger:) return [] end - workspace_resources = YAML.load_stream(workspace_resources_yaml) + workspace_resources = YAML.load_stream(workspace_resources_yaml).map(&:deep_symbolize_keys) + workspace_resources = set_host_users( workspace_resources: workspace_resources, use_kubernetes_user_namespaces: use_kubernetes_user_namespaces @@ -86,7 +87,7 @@ def self.get_all(processed_devfile:, k8s_resources_params:, logger:) workspace_resources = patch_default_resources( workspace_resources: workspace_resources, default_resources_per_workspace_container: - default_resources_per_workspace_container.deep_stringify_keys.to_h + default_resources_per_workspace_container ) workspace_resources = inject_secrets( workspace_resources: workspace_resources, @@ -110,9 +111,9 @@ def self.set_host_users(workspace_resources:, use_kubernetes_user_namespaces:) return workspace_resources unless use_kubernetes_user_namespaces workspace_resources.each do |workspace_resource| - next unless workspace_resource['kind'] == 'Deployment' + next unless workspace_resource[:kind] == 'Deployment' - workspace_resource['spec']['template']['spec']['hostUsers'] = use_kubernetes_user_namespaces + workspace_resource[:spec][:template][:spec][:hostUsers] = use_kubernetes_user_namespaces end workspace_resources end @@ -127,9 +128,9 @@ def self.set_runtime_class(workspace_resources:, runtime_class_name:) return workspace_resources if runtime_class_name.empty? workspace_resources.each do |workspace_resource| - next unless workspace_resource['kind'] == 'Deployment' + next unless workspace_resource[:kind] == 'Deployment' - workspace_resource['spec']['template']['spec']['runtimeClassName'] = runtime_class_name + workspace_resource[:spec][:template][:spec][:runtimeClassName] = runtime_class_name end workspace_resources end @@ -151,31 +152,31 @@ def self.set_security_context( allow_privilege_escalation: ) workspace_resources.each do |workspace_resource| - next unless workspace_resource['kind'] == 'Deployment' + next unless workspace_resource[:kind] == 'Deployment' pod_security_context = { - 'runAsNonRoot' => true, - 'runAsUser' => RUN_AS_USER, - 'fsGroup' => 0, - 'fsGroupChangePolicy' => 'OnRootMismatch' + runAsNonRoot: true, + runAsUser: RUN_AS_USER, + fsGroup: 0, + fsGroupChangePolicy: 'OnRootMismatch' } container_security_context = { - 'allowPrivilegeEscalation' => allow_privilege_escalation, - 'privileged' => false, - 'runAsNonRoot' => true, - 'runAsUser' => RUN_AS_USER + allowPrivilegeEscalation: allow_privilege_escalation, + privileged: false, + runAsNonRoot: true, + runAsUser: RUN_AS_USER } - pod_spec = workspace_resource['spec']['template']['spec'] + pod_spec = workspace_resource[:spec][:template][:spec] # Explicitly set security context for the pod - pod_spec['securityContext'] = pod_security_context + pod_spec[:securityContext] = pod_security_context # Explicitly set security context for all containers - pod_spec['containers'].each do |container| - container['securityContext'] = container_security_context + pod_spec[:containers].each do |container| + container[:securityContext] = container_security_context end # Explicitly set security context for all init containers - pod_spec['initContainers'].each do |init_container| - init_container['securityContext'] = container_security_context + pod_spec[:initContainers].each do |init_container| + init_container[:securityContext] = container_security_context end end workspace_resources @@ -186,22 +187,19 @@ def self.set_security_context( # @return [Array] def self.patch_default_resources(workspace_resources:, default_resources_per_workspace_container:) workspace_resources.each do |workspace_resource| - next unless workspace_resource.fetch('kind') == 'Deployment' - - pod_spec = workspace_resource.fetch('spec').fetch('template').fetch('spec') + next unless workspace_resource.fetch(:kind) == 'Deployment' - # the purpose of this deep_merge (and the one below) is to ensure - # the values from the devfile override any defaults defined at the agent - pod_spec.fetch('initContainers').each do |init_container| - init_container - .fetch('resources', {}) - .deep_merge!(default_resources_per_workspace_container) { |_, val, _| val } - end + pod_spec = workspace_resource.fetch(:spec).fetch(:template).fetch(:spec) - pod_spec.fetch('containers').each do |container| - container - .fetch('resources', {}) - .deep_merge!(default_resources_per_workspace_container) { |_, val, _| val } + container_types = [:initContainers, :containers] + container_types.each do |container_type| + # the purpose of this deep_merge is to ensure + # the values from the devfile override any defaults defined at the agent + pod_spec.fetch(container_type).each do |container| + container + .fetch(:resources, {}) + .deep_merge!(default_resources_per_workspace_container) { |_, val, _| val } + end end end workspace_resources @@ -213,37 +211,37 @@ def self.patch_default_resources(workspace_resources:, default_resources_per_wor # @return [Array] def self.inject_secrets(workspace_resources:, env_secret_names:, file_secret_names:) workspace_resources.each do |workspace_resource| - next unless workspace_resource.fetch('kind') == 'Deployment' + next unless workspace_resource.fetch(:kind) == 'Deployment' volume_name = 'gl-workspace-variables' volumes = [ { - 'name' => volume_name, - 'projected' => { - 'defaultMode' => 0o774, - 'sources' => file_secret_names.map { |v| { 'secret' => { 'name' => v } } } + name: volume_name, + projected: { + defaultMode: 0o774, + sources: file_secret_names.map { |v| { secret: { name: v } } } } } ] volume_mounts = [ { - 'name' => volume_name, - 'mountPath' => VARIABLES_FILE_DIR + name: volume_name, + mountPath: VARIABLES_FILE_DIR } ] - env_from = env_secret_names.map { |v| { 'secretRef' => { 'name' => v } } } + env_from = env_secret_names.map { |v| { secretRef: { name: v } } } - pod_spec = workspace_resource.fetch('spec').fetch('template').fetch('spec') - pod_spec.fetch('volumes').concat(volumes) unless file_secret_names.empty? + pod_spec = workspace_resource.fetch(:spec).fetch(:template).fetch(:spec) + pod_spec.fetch(:volumes).concat(volumes) unless file_secret_names.empty? - pod_spec.fetch('initContainers').each do |init_container| - init_container.fetch('volumeMounts').concat(volume_mounts) unless file_secret_names.empty? - init_container['envFrom'] = env_from unless env_secret_names.empty? + pod_spec.fetch(:initContainers).each do |init_container| + init_container.fetch(:volumeMounts).concat(volume_mounts) unless file_secret_names.empty? + init_container[:envFrom] = env_from unless env_secret_names.empty? end - pod_spec.fetch('containers').each do |container| - container.fetch('volumeMounts').concat(volume_mounts) unless file_secret_names.empty? - container['envFrom'] = env_from unless env_secret_names.empty? + pod_spec.fetch(:containers).each do |container| + container.fetch(:volumeMounts).concat(volume_mounts) unless file_secret_names.empty? + container[:envFrom] = env_from unless env_secret_names.empty? end end workspace_resources @@ -254,9 +252,9 @@ def self.inject_secrets(workspace_resources:, env_secret_names:, file_secret_nam # @return [Array] def self.set_service_account(workspace_resources:, service_account_name:) workspace_resources.each do |workspace_resource| - next unless workspace_resource.fetch('kind') == 'Deployment' + next unless workspace_resource.fetch(:kind) == 'Deployment' - workspace_resource['spec']['template']['spec']['serviceAccountName'] = service_account_name + workspace_resource[:spec][:template][:spec][:serviceAccountName] = service_account_name end workspace_resources end diff --git a/ee/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder.rb b/ee/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder.rb index 85beca1669d56aa6689df4c5fff135f8dc0533b3..fc0237014d5b78c909b3462e161bb035ab863893 100644 --- a/ee/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder.rb +++ b/ee/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder.rb @@ -29,20 +29,32 @@ def self.build(context) # Create an array of workspace_rails_info hashes based on the workspaces. These indicate the desired updates # to the workspace, which will be returned in the payload to the agent to be applied to kubernetes workspace_rails_infos = workspaces_to_be_returned.map do |workspace| - config_to_apply, config_to_apply_resources_include_type = config_to_apply(workspace: workspace, + config_to_apply, config_to_apply_resources_include_type = generate_config_to_apply(workspace: workspace, update_type: update_type, logger: logger) observability_for_rails_infos[workspace.name] = { config_to_apply_resources_included: config_to_apply_resources_include_type } + config_to_apply_yaml_stream = + # config_to_apply_yaml will be returned as nil if generate_config_to_apply returned nil + if config_to_apply + # Dump the config_to_apply to yaml with stringified keys, so it can be sent to the agent. This is + # the last time we will have access to it before it is returned to the agent in the reconciliation + # response, so we have kept everything as a hash with symbolized keys until now. + config_to_apply.map { |resource| YAML.dump(resource.deep_stringify_keys) }.join + else + # Return an empty string, which is valid YAML to represent a YAML `stream` with "zero" documents: + # https://yaml.org/spec/1.2.2/#streams + "" + end + { name: workspace.name, namespace: workspace.namespace, desired_state: workspace.desired_state, actual_state: workspace.actual_state, deployment_resource_version: workspace.deployment_resource_version, - # NOTE: config_to_apply should be returned as null if config_to_apply returned nil - config_to_apply: config_to_apply, + config_to_apply: config_to_apply_yaml_stream, image_pull_secrets: workspace.workspaces_agent_config.image_pull_secrets.map(&:symbolize_keys) } end @@ -65,7 +77,7 @@ def self.build(context) # @param [String (frozen)] update_type # @param [RemoteDevelopment::Logger] logger # @return [Array] - def self.config_to_apply(workspace:, update_type:, logger:) + def self.generate_config_to_apply(workspace:, update_type:, logger:) return nil, NO_RESOURCES_INCLUDED unless should_include_config_to_apply?(update_type: update_type, workspace: workspace) @@ -92,13 +104,13 @@ def self.config_to_apply(workspace:, update_type:, logger:) ) end - desired_config_to_apply_array = workspace_resources.map do |resource| - YAML.dump(Gitlab::Utils.deep_sort_hash(resource).deep_stringify_keys) + stable_sorted_workspace_resources = workspace_resources.map do |resource| + Gitlab::Utils.deep_sort_hash(resource) end - return nil, NO_RESOURCES_INCLUDED unless desired_config_to_apply_array.present? + return nil, NO_RESOURCES_INCLUDED unless stable_sorted_workspace_resources.present? - [desired_config_to_apply_array.join, resources_include_type] + [stable_sorted_workspace_resources, resources_include_type] end # @param [String (frozen)] update_type @@ -109,7 +121,8 @@ def self.should_include_config_to_apply?(update_type:, workspace:) workspace.force_include_all_resources || workspace.desired_state_updated_more_recently_than_last_response_to_agent? end - private_class_method :should_include_config_to_apply?, :config_to_apply + + private_class_method :should_include_config_to_apply?, :generate_config_to_apply end end end diff --git a/ee/spec/features/remote_development/workspaces_spec.rb b/ee/spec/features/remote_development/workspaces_spec.rb index 2786861a8bcd54db32d158ee6483f345483dd71a..d93ea5c579c102ec557618d3f04a3e405da6b05e 100644 --- a/ee/spec/features/remote_development/workspaces_spec.rb +++ b/ee/spec/features/remote_development/workspaces_spec.rb @@ -153,7 +153,7 @@ def do_reconcile_post(params:, agent_token:) click_button 'Actions' additional_args_for_expected_config_to_apply = - build_additional_args_for_expected_config_to_apply( + build_additional_args_for_expected_config_to_apply_yaml_stream( network_policy_enabled: true, dns_zone: workspaces_agent_config.dns_zone, namespace_path: group.path, diff --git a/ee/spec/lib/remote_development/namespace_cluster_agent_mapping_operations/create/main_integration_spec.rb b/ee/spec/lib/remote_development/namespace_cluster_agent_mapping_operations/create/main_integration_spec.rb index a252acc9718ba81d2919da9839014b04b4a0662b..ad2c2deb755a91da918588665a83e6321ba824b2 100644 --- a/ee/spec/lib/remote_development/namespace_cluster_agent_mapping_operations/create/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/namespace_cluster_agent_mapping_operations/create/main_integration_spec.rb @@ -23,9 +23,13 @@ expect(response.fetch(:status)).to eq(:success) expect(response[:message]).to be_nil expect(response[:payload]).not_to be_nil - expect(response.dig(:payload, :namespace_cluster_agent_mapping)).not_to be_nil + response => { + payload: { + namespace_cluster_agent_mapping: mapping + } + } - mapping = response.dig(:payload, :namespace_cluster_agent_mapping) + expect(mapping).not_to be_nil # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 expect(mapping.cluster_agent_id).to eq(cluster_agent.id) expect(mapping.namespace_id).to eq(namespace.id) diff --git a/ee/spec/lib/remote_development/workspace_operations/create/workspace_variables_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/workspace_variables_spec.rb index 48a2ac544f3986288da63bb332fc0b22e87b8ac6..b6eae65fcc1124a2c31e46f602cab11ddc56d77c 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/workspace_variables_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/workspace_variables_spec.rb @@ -128,13 +128,13 @@ workspace_id: workspace_id }, { - key: 'GITLAB_WORKFLOW_INSTANCE_URL', + key: "GITLAB_WORKFLOW_INSTANCE_URL", value: Gitlab::Routing.url_helpers.root_url, variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id }, { - key: 'GITLAB_WORKFLOW_TOKEN_FILE', + key: "GITLAB_WORKFLOW_TOKEN_FILE", value: "/.workspace-data/variables/file/gl_token", variable_type: RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES[:environment], workspace_id: workspace_id @@ -188,7 +188,7 @@ allow(Gitlab::Routing).to receive_message_chain(:url_helpers, :root_url).and_return("https://gitlab.com") end - it 'defines correct variables' do + it "defines correct variables" do expect(variables).to eq(expected_variables) end end diff --git a/ee/spec/lib/remote_development/workspace_operations/reconcile/main_integration_spec.rb b/ee/spec/lib/remote_development/workspace_operations/reconcile/main_integration_spec.rb index 0678cf76c8d3774be99b18add864e70e081f88cc..c555b8d8db51e4cfc6358f74334c915d40002f4f 100644 --- a/ee/spec/lib/remote_development/workspace_operations/reconcile/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/reconcile/main_integration_spec.rb @@ -10,7 +10,20 @@ it "sets desired_state to #{expected_desired_state}" do response = subject expect(response[:message]).to be_nil - expect(response.dig(:payload, :workspace_rails_infos)).not_to be_nil + + response => { + payload: { + workspace_rails_infos: [ + *_, + { + desired_state: actual_desired_state + }, + *_ + ] + } + } + + expect(actual_desired_state).to eq(expected_desired_state) expect(workspace.reload.desired_state).to eq(expected_desired_state) end @@ -34,11 +47,31 @@ end it 'uses versioned config values' do - config_to_apply = - YAML.load_stream(response.dig(:payload, :workspace_rails_infos, 0, :config_to_apply)) + response => { + payload: { + workspace_rails_infos: [ + { + config_to_apply: config_to_apply_yaml_stream + }, + *_ + ] + } + } - service = config_to_apply.find { |config| config["kind"] == "Service" }.to_h - host_template_annotation = service.dig("metadata", "annotations", "workspaces.gitlab.com/host-template") + config_to_apply = yaml_safe_load_symbolized(config_to_apply_yaml_stream) + + config_to_apply => [ + *_, + { + kind: "Service", + metadata: { + annotations: { + "workspaces.gitlab.com/host-template": host_template_annotation + } + } + }, + *_ + ] expect(host_template_annotation).to include(dns_zone) expect(host_template_annotation).not_to include(new_dns_zone) @@ -47,7 +80,9 @@ let_it_be(:image_pull_secrets) { [{ name: "secret-name", namespace: "secret-namespace" }] } let_it_be(:dns_zone) { 'workspaces.localdev.me' } + # noinspection RubyArgCount --https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 let_it_be(:user) { create(:user) } + # noinspection RubyArgCount --https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 let_it_be(:agent, refind: true) { create(:cluster_agent) } let_it_be(:workspaces_agent_config, refind: true) do create(:workspaces_agent_config, dns_zone: dns_zone, agent: agent, image_pull_secrets: image_pull_secrets) @@ -131,7 +166,7 @@ let(:expected_desired_state) { desired_state } let(:expected_actual_state) { actual_state } let(:expected_deployment_resource_version) { deployment_resource_version_from_agent } - let(:expected_config_to_apply) { nil } + let(:expected_config_to_apply_yaml_stream) { "" } let(:error_from_agent) { nil } let(:workspace_agent_info) do @@ -154,7 +189,7 @@ desired_state: expected_desired_state, actual_state: expected_actual_state, deployment_resource_version: expected_deployment_resource_version, - config_to_apply: expected_config_to_apply, + config_to_apply: expected_config_to_apply_yaml_stream, image_pull_secrets: image_pull_secrets } end @@ -259,7 +294,7 @@ provisioned_workspace_rails_info = workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } # Since the workspace is now in Error state, the config should not be returned to the agent - expect(provisioned_workspace_rails_info.fetch(:config_to_apply)).to be_nil + expect(provisioned_workspace_rails_info.fetch(:config_to_apply)).to eq("") # then test everything in the infos expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) @@ -299,7 +334,7 @@ desired_state: expected_desired_state, actual_state: expected_actual_state, deployment_resource_version: expected_deployment_resource_version, - config_to_apply: nil, + config_to_apply: "", image_pull_secrets: image_pull_secrets } end @@ -322,7 +357,7 @@ workspace2_rails_info = workspace_rails_infos.detect { |info| info.fetch(:name) == workspace2.name } - expect(workspace2_rails_info.fetch(:config_to_apply)).to be_nil + expect(workspace2_rails_info.fetch(:config_to_apply)).to eq("") # then test everything in the infos expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) @@ -410,8 +445,8 @@ context 'when desired_state does not match actual_state' do let(:deployment_resource_version_from_agent) { workspace.deployment_resource_version } - let(:expected_config_to_apply) do - create_config_to_apply( + let(:expected_config_to_apply_yaml_stream) do + create_config_to_apply_yaml_stream( workspace: workspace, started: expected_value_for_started, egress_ip_rules: egress_ip_rules, @@ -449,10 +484,11 @@ .to eq(expected_deployment_resource_version) # test the config to apply first to get a more specific diff if it fails - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } - expect(provisioned_workspace_rails_info.fetch(:config_to_apply)) - .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) + actual_workspace_rails_info = workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } + actual_config_to_apply = yaml_safe_load_symbolized(actual_workspace_rails_info.fetch(:config_to_apply)) + expected_config_to_apply = + yaml_safe_load_symbolized(expected_workspace_rails_info.fetch(:config_to_apply)) + expect(actual_config_to_apply).to eq(expected_config_to_apply) # then test everything in the infos expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) @@ -520,8 +556,8 @@ let(:expected_actual_state) { RemoteDevelopment::WorkspaceOperations::States::UNKNOWN } let(:expected_value_for_started) { false } - let(:expected_config_to_apply) do - create_config_to_apply( + let(:expected_config_to_apply_yaml_stream) do + create_config_to_apply_yaml_stream( workspace: workspace, started: expected_value_for_started, egress_ip_rules: egress_ip_rules, @@ -542,10 +578,11 @@ config_to_apply_hash = YAML.safe_load( response[:payload].fetch(:workspace_rails_infos)[0][:config_to_apply] ) - expected_config_to_apply_hash = YAML.safe_load(expected_config_to_apply) + expected_config_to_apply_hash = YAML.safe_load(expected_config_to_apply_yaml_stream) expect(config_to_apply_hash).to eq(expected_config_to_apply_hash) - expect(response[:payload][:workspace_rails_infos][0][:config_to_apply]).to eq(expected_config_to_apply) + expect(response[:payload][:workspace_rails_infos][0][:config_to_apply]) + .to eq(expected_config_to_apply_yaml_stream) expect(response[:payload][:workspace_rails_infos]).to eq(expected_workspace_rails_infos) end @@ -600,13 +637,14 @@ let(:actual_state) { RemoteDevelopment::WorkspaceOperations::States::CREATION_REQUESTED } let_it_be(:unprovisioned_workspace) do + # noinspection RubyArgCount --https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 create(:workspace, :unprovisioned, agent: agent, user: user) end let(:workspace_agent_infos) { [] } - let(:expected_config_to_apply) do - create_config_to_apply( + let(:expected_config_to_apply_yaml_stream) do + create_config_to_apply_yaml_stream( workspace: unprovisioned_workspace, started: expected_value_for_started, include_all_resources: true, @@ -624,7 +662,7 @@ desired_state: desired_state, actual_state: actual_state, deployment_resource_version: nil, - config_to_apply: expected_config_to_apply, + config_to_apply: expected_config_to_apply_yaml_stream, image_pull_secrets: image_pull_secrets } end diff --git a/ee/spec/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator_spec.rb b/ee/spec/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator_spec.rb index f20f94b6589c837cb8772eef772b5c4e70958f1f..c697c8221338d1c5963159c21cc36c730a3ab6cc 100644 --- a/ee/spec/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/reconcile/output/desired_config_generator_spec.rb @@ -44,22 +44,20 @@ end let(:expected_config) do - YAML.load_stream( - create_config_to_apply( - workspace: workspace, - started: started, - include_network_policy: workspace.workspaces_agent_config.network_policy_enabled, - include_all_resources: include_all_resources, - egress_ip_rules: workspace.workspaces_agent_config.network_policy_egress, - max_resources_per_workspace: max_resources_per_workspace, - default_resources_per_workspace_container: default_resources_per_workspace_container, - allow_privilege_escalation: workspace.workspaces_agent_config.allow_privilege_escalation, - use_kubernetes_user_namespaces: workspace.workspaces_agent_config.use_kubernetes_user_namespaces, - default_runtime_class: workspace.workspaces_agent_config.default_runtime_class, - agent_labels: workspace.workspaces_agent_config.labels, - agent_annotations: workspace.workspaces_agent_config.annotations, - image_pull_secrets: image_pull_secrets - ) + create_config_to_apply( + workspace: workspace, + started: started, + include_network_policy: workspace.workspaces_agent_config.network_policy_enabled, + include_all_resources: include_all_resources, + egress_ip_rules: workspace.workspaces_agent_config.network_policy_egress, + max_resources_per_workspace: max_resources_per_workspace, + default_resources_per_workspace_container: default_resources_per_workspace_container, + allow_privilege_escalation: workspace.workspaces_agent_config.allow_privilege_escalation, + use_kubernetes_user_namespaces: workspace.workspaces_agent_config.use_kubernetes_user_namespaces, + default_runtime_class: workspace.workspaces_agent_config.default_runtime_class, + agent_labels: workspace.workspaces_agent_config.labels, + agent_annotations: workspace.workspaces_agent_config.annotations, + image_pull_secrets: image_pull_secrets ) end @@ -74,8 +72,18 @@ context 'when desired_state results in started=true' do it 'returns expected config with the replicas set to one' do expect(workspace_resources).to eq(expected_config) - deployment = workspace_resources.find { |resource| resource.fetch('kind') == 'Deployment' } - expect(deployment.dig('spec', 'replicas')).to eq(1) + workspace_resources => [ + *_, + { + kind: "Deployment", + spec: { + replicas: replicas + } + }, + *_ + ] + + expect(replicas).to eq(1) end end @@ -85,8 +93,17 @@ it 'returns expected config with the replicas set to zero' do expect(workspace_resources).to eq(expected_config) - deployment = workspace_resources.find { |resource| resource.fetch('kind') == 'Deployment' } - expect(deployment.dig('spec', 'replicas')).to eq(0) + workspace_resources => [ + *_, + { + kind: "Deployment", + spec: { + replicas: replicas + } + }, + *_ + ] + expect(replicas).to eq(0) end end @@ -95,7 +112,7 @@ it 'returns expected config without network policy' do expect(workspace_resources).to eq(expected_config) - network_policy_resource = workspace_resources.select { |resource| resource.fetch('kind') == 'NetworkPolicy' } + network_policy_resource = workspace_resources.select { |resource| resource.fetch(:kind) == 'NetworkPolicy' } expect(network_policy_resource).to be_empty end end @@ -107,24 +124,33 @@ it 'returns expected config with defaults for the container resources set' do expect(workspace_resources).to eq(expected_config) - deployment = workspace_resources.find { |resource| resource.fetch('kind') == 'Deployment' } - resources_per_workspace_container = deployment.dig('spec', 'template', 'spec', - 'containers').map do |container| - container.fetch('resources') - end - resources = default_resources_per_workspace_container.deep_stringify_keys - expect(resources_per_workspace_container).to all(eq resources) + workspace_resources => [ + *_, + { + kind: "Deployment", + spec: { + template: { + spec: { + containers: containers + } + } + } + }, + *_ + ] + resources_per_workspace_container = containers.map { |container| container.fetch(:resources) } + expect(resources_per_workspace_container).to all(eq(default_resources_per_workspace_container)) end end context 'when there are image-pull-secrets' do let(:image_pull_secrets) { [{ name: 'secret-name', namespace: 'secret-namespace' }] } - let(:expected_image_pull_secrets_names) { [{ 'name' => 'secret-name' }] } + let(:expected_image_pull_secrets_names) { [{ name: 'secret-name' }] } it 'returns expected config with a service account resource configured' do expect(workspace_resources).to eq(expected_config) - service_account_resource = workspace_resources.find { |resource| resource.fetch('kind') == 'ServiceAccount' } - expect(service_account_resource.fetch('imagePullSecrets')).to eq(expected_image_pull_secrets_names) + service_account_resource = workspace_resources.find { |resource| resource.fetch(:kind) == 'ServiceAccount' } + expect(service_account_resource.to_h.fetch(:imagePullSecrets)).to eq(expected_image_pull_secrets_names) end end @@ -138,7 +164,7 @@ it 'returns expected config with resource quota set' do expect(workspace_resources).to eq(expected_config) - resource_quota = workspace_resources.find { |resource| resource.fetch('kind') == 'ResourceQuota' } + resource_quota = workspace_resources.find { |resource| resource.fetch(:kind) == 'ResourceQuota' } expect(resource_quota).not_to be_nil end end diff --git a/ee/spec/lib/remote_development/workspace_operations/reconcile/output/devfile_parser_spec.rb b/ee/spec/lib/remote_development/workspace_operations/reconcile/output/devfile_parser_spec.rb index 0f4f3ded494041e46cd4117419917c533f144860..f051d397cf52d42db274957c9cd4492016912fec 100644 --- a/ee/spec/lib/remote_development/workspace_operations/reconcile/output/devfile_parser_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/reconcile/output/devfile_parser_spec.rb @@ -34,7 +34,30 @@ end # rubocop:enable RSpec/VerifiedDoubleReference - let(:processed_devfile_yaml) { example_processed_devfile_yaml } + let(:processed_devfile_yaml) do + yaml = example_processed_devfile_yaml + + puts yaml + + # Add additional entries to the processed devfile yaml to ensure full coverage of all DevfileParser logic + hash = yaml_safe_load_symbolized(yaml) + + # Add explicit resources to main component container + hash => { + components: [ + *_, + { + name: "tooling-container", + container: tooling_container + }, + *_ + ] + } + tooling_container[:memoryLimit] = "786Mi" + + YAML.dump(hash.deep_stringify_keys) + end + let(:domain_template) { "" } let(:egress_ip_rules) do [{ @@ -75,26 +98,24 @@ end let(:expected_workspace_resources) do - YAML.load_stream( - create_config_to_apply( - workspace: workspace, - workspace_variables_environment: {}, - workspace_variables_file: {}, - started: true, - include_inventory: false, - include_network_policy: false, - include_all_resources: false, - dns_zone: dns_zone, - egress_ip_rules: egress_ip_rules, - default_resources_per_workspace_container: default_resources_per_workspace_container, - allow_privilege_escalation: allow_privilege_escalation, - use_kubernetes_user_namespaces: use_kubernetes_user_namespaces, - default_runtime_class: default_runtime_class, - agent_labels: agent_labels, - agent_annotations: agent_annotations, - image_pull_secrets: image_pull_secrets, - core_resources_only: true - ) + create_config_to_apply( + workspace: workspace, + workspace_variables_environment: {}, + workspace_variables_file: {}, + started: true, + include_inventory: false, + include_network_policy: false, + include_all_resources: false, + dns_zone: dns_zone, + egress_ip_rules: egress_ip_rules, + default_resources_per_workspace_container: default_resources_per_workspace_container, + allow_privilege_escalation: allow_privilege_escalation, + use_kubernetes_user_namespaces: use_kubernetes_user_namespaces, + default_runtime_class: default_runtime_class, + agent_labels: agent_labels, + agent_annotations: agent_annotations, + image_pull_secrets: image_pull_secrets, + core_resources_only: true ) end @@ -108,13 +129,13 @@ context "when the DevFile parser is successful in parsing k8s core resources" do let(:domain_template) { "{{.port}}-#{workspace.name}.#{dns_zone}" } - let(:labels) { agent_labels.merge('agent.gitlab.com/id' => workspace.agent.id) } + let(:labels) { agent_labels.merge('agent.gitlab.com/id': workspace.agent.id) } let(:annotations) do agent_annotations.merge({ - 'config.k8s.io/owning-inventory' => "#{workspace.name}-workspace-inventory", - 'workspaces.gitlab.com/host-template' => domain_template, - 'workspaces.gitlab.com/id' => workspace.id, - 'workspaces.gitlab.com/max-resources-per-workspace-sha256' => + 'config.k8s.io/owning-inventory': "#{workspace.name}-workspace-inventory", + 'workspaces.gitlab.com/host-template': domain_template, + 'workspaces.gitlab.com/id': workspace.id, + 'workspaces.gitlab.com/max-resources-per-workspace-sha256': Digest::SHA256.hexdigest(max_resources_per_workspace.sort.to_h.to_s) }) end @@ -122,19 +143,32 @@ context "when allow_privilege_escalation is true" do let(:allow_privilege_escalation) { true } - it 'returns workspace_resources with allow_privilege_escalation set to true' do + it 'returns workspace_resources with allow_privilege_escalation set to true', + :unlimited_max_formatted_output_length do expect(k8s_resources_for_workspace_core).to eq(expected_workspace_resources) - deployment = k8s_resources_for_workspace_core.find do |resource| - resource.fetch('kind') == 'Deployment' - end.to_h - container_privilege_escalation = deployment.dig('spec', 'template', 'spec', 'containers').map do |container| - container.dig('securityContext', 'allowPrivilegeEscalation') + k8s_resources_for_workspace_core => [ + *_, + { + kind: "Deployment", + spec: { + template: { + spec: { + containers: containers, + initContainers: init_containers + } + } + } + }, + *_ + ] + + container_privilege_escalation = containers.map do |container| + container.fetch(:securityContext).fetch(:allowPrivilegeEscalation) end - init_container_privilege_escalation = deployment.dig('spec', 'template', 'spec', - 'initContainers').map do |container| - container.dig('securityContext', 'allowPrivilegeEscalation') + init_container_privilege_escalation = init_containers.map do |init_container| + init_container.fetch(:securityContext).fetch(:allowPrivilegeEscalation) end all_containers_privilege_escalation = init_container_privilege_escalation + container_privilege_escalation @@ -148,10 +182,23 @@ it 'returns workspace_resources with hostUsers set to true' do expect(k8s_resources_for_workspace_core).to eq(expected_workspace_resources) - deployment = k8s_resources_for_workspace_core.find do |resource| - resource.fetch('kind') == 'Deployment' - end.to_h - expect(deployment.dig('spec', 'template', 'spec', 'hostUsers')).to be(true) + + k8s_resources_for_workspace_core => [ + *_, + { + kind: "Deployment", + spec: { + template: { + spec: { + hostUsers: host_users + } + } + } + }, + *_ + ] + + expect(host_users).to be(true) end end end @@ -189,8 +236,8 @@ let(:error_type) { StandardError } let(:error_message) do <<~MSG.squish - An unrecoverable error occurred when invoking the devfile gem, this may hint that a gem with a wrong - architecture is being used. + An unrecoverable error occurred when invoking the devfile gem, this may hint that a gem with a wrong + architecture is being used. MSG end diff --git a/ee/spec/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder_spec.rb b/ee/spec/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder_spec.rb index 2b89d5de2b4c4f5b6c8236028f8f89f496cb66aa..9afc50910e9628f281597389055f57fab6b05136 100644 --- a/ee/spec/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/reconcile/output/response_payload_builder_spec.rb @@ -66,7 +66,11 @@ ] end - let(:returned_workspace_rails_infos) do + let(:expected_returned_workspace_rails_infos) do + config_to_apply_yaml_stream = generated_config_to_apply&.map do |resource| + YAML.dump(resource.deep_stringify_keys) + end&.join + [ { name: workspace.name, @@ -75,10 +79,7 @@ desired_state: desired_state, actual_state: actual_state, image_pull_secrets: image_pull_secrets, - config_to_apply: - generated_config_to_apply&.map do |resource| - YAML.dump(resource.deep_stringify_keys) - end&.join + config_to_apply: config_to_apply_yaml_stream || "" } ] end @@ -86,7 +87,7 @@ let(:expected_returned_value) do context.merge( response_payload: { - workspace_rails_infos: returned_workspace_rails_infos, + workspace_rails_infos: expected_returned_workspace_rails_infos, settings: settings }, observability_for_rails_infos: { @@ -128,6 +129,38 @@ it "includes config_to_apply with all resources included" do expect(returned_value).to eq(expected_returned_value) end + + context "when config_to_apply contains multiple resources" do + let(:generated_config_to_apply) do + [ + { + a: 1 + }, + { + b: 2 + }, + { + c: 3 + } + ] + end + + it "includes all resources" do + expect(returned_value).to eq(expected_returned_value) + returned_value[:response_payload][:workspace_rails_infos].first[:config_to_apply] + returned_value => { + response_payload: { + workspace_rails_infos: [ + { + config_to_apply: config_to_apply_yaml_stream + }, + ] + } + } + loaded_multiple_docs = YAML.load_stream(config_to_apply_yaml_stream) + expect(loaded_multiple_docs.size).to eq(3) + end + end end context "when update_type is PARTIAL" do @@ -176,7 +209,7 @@ described_class::NO_RESOURCES_INCLUDED end - it "does not includes config_to_apply and returns it as nil" do + it "does not includes config_to_apply and returns it as an empty string" do expect(returned_value).to eq(expected_returned_value) end end @@ -196,6 +229,7 @@ stub_const( "RemoteDevelopment::WorkspaceOperations::Reconcile::Output::DesiredConfigGeneratorV2", Class.new do + # @param [Object] _ # @return [Hash] def self.generate_desired_config(_) {} @@ -205,6 +239,7 @@ def self.generate_desired_config(_) end it "includes config_to_apply with all resources included" do + # noinspection RubyResolve -- This constant is stubbed allow(RemoteDevelopment::WorkspaceOperations::Reconcile::Output::DesiredConfigGeneratorV2) .to(receive(:generate_desired_config)) { generated_config_to_apply } diff --git a/ee/spec/models/remote_development/remote_development_agent_config_spec.rb b/ee/spec/models/remote_development/remote_development_agent_config_spec.rb index 0db7291e7fb0a0766ce41c768da6fde20ed5dda2..6cb740bc64bae1d65e27b6e2c0f0bb1931a5034b 100644 --- a/ee/spec/models/remote_development/remote_development_agent_config_spec.rb +++ b/ee/spec/models/remote_development/remote_development_agent_config_spec.rb @@ -13,13 +13,13 @@ let(:default_network_policy_egress) do [ { - allow: "0.0.0.0/0", - except: [ - - "10.0.0.0/8", - - "172.16.0.0/12", - - "192.168.0.0/16" + "allow" => "0.0.0.0/0", + "except" => [ + -"10.0.0.0/8", + -"172.16.0.0/12", + -"192.168.0.0/16" ] - }.deep_stringify_keys + } ] end diff --git a/ee/spec/models/remote_development/workspaces_agent_config_spec.rb b/ee/spec/models/remote_development/workspaces_agent_config_spec.rb index 590ca4d15a83921e213830cad6136288ec597753..c4561245b3ad3df34bf40e58e169968f4289cdd7 100644 --- a/ee/spec/models/remote_development/workspaces_agent_config_spec.rb +++ b/ee/spec/models/remote_development/workspaces_agent_config_spec.rb @@ -9,13 +9,13 @@ let(:default_network_policy_egress) do [ { - allow: "0.0.0.0/0", - except: [ + "allow" => "0.0.0.0/0", + "except" => [ -"10.0.0.0/8", -"172.16.0.0/12", -"192.168.0.0/16" ] - }.deep_stringify_keys + } ] end diff --git a/ee/spec/requests/remote_development/integration_spec.rb b/ee/spec/requests/remote_development/integration_spec.rb index cbd0b42a5f89781264371ed7f06c778d8c8b10a5..b0e40dd4a369ffe8f9c83804e7a6b132e05d4008 100644 --- a/ee/spec/requests/remote_development/integration_spec.rb +++ b/ee/spec/requests/remote_development/integration_spec.rb @@ -137,6 +137,7 @@ create(:ee_cluster_agent, project: agent_project, created_by_user: agent_admin_user, project_id: agent_project.id) end + # @return [void] def do_create_workspaces_agent_config # Perform an agent update post which will cause a workspaces_agent_config record to be created # based on the cluster agent's config file. @@ -181,8 +182,11 @@ def do_create_workspaces_agent_config expect( graphql_data_at(:namespace, :remoteDevelopmentClusterAgents, :nodes, 0, :workspacesAgentConfig) ).to eq(expected_agent_config) + + nil end + # @return [void] def do_create_mapping namespace_create_remote_development_cluster_agent_mapping_create_mutation_args = { @@ -194,9 +198,12 @@ def do_create_mapping input: namespace_create_remote_development_cluster_agent_mapping_create_mutation_args, user: agent_admin_user ) + + nil end # rubocop:disable Metrics/AbcSize -- We want this to stay a single method + # @return [RemoteDevelopment::Workspace] def do_create_workspace # FETCH THE AGENT CONFIG VIA THE GRAPHQL API, SO WE CAN USE ITS VALUES WHEN CREATING WORKSPACE cluster_agent_gid = "gid://gitlab/Clusters::Agent/#{agent.id}" @@ -240,10 +247,11 @@ def do_create_workspace types = RemoteDevelopment::Enums::Workspace::WORKSPACE_VARIABLE_TYPES all_actual_vars = RemoteDevelopment::WorkspaceVariable.where(workspace: workspace) + # noinspection RailsParamDefResolve -- RubyMine is incorrectly detecting this as ActiveRecord #select method actual_user_provided_vars = all_actual_vars.select(&:user_provided) all_actual_vars = all_actual_vars.map { |v| { key: v.key, type: types.invert[v.variable_type], value: v.value } } - .sort_by { |v| v[:key] } + .sort_by { |v| v[:key] } # Check that user provided variables had their flag set correctly. expect(actual_user_provided_vars.count).to eq(user_provided_variables.count) @@ -267,6 +275,7 @@ def do_create_workspace # rubocop:enable Metrics/AbcSize # @param [String] cluster_agent_gid + # @return [Hash] def workspace_create_mutation_args(cluster_agent_gid) { desired_state: states::RUNNING, @@ -282,17 +291,24 @@ def workspace_create_mutation_args(cluster_agent_gid) end # @param [RemoteDevelopment::Workspace] workspace + # @return [void] def do_start_workspace(workspace) do_change_workspace_state(workspace: workspace, desired_state: states::RUNNING) + + nil end # @param [RemoteDevelopment::Workspace] workspace + # @return [void] def do_stop_workspace(workspace) do_change_workspace_state(workspace: workspace, desired_state: states::STOPPED) + + nil end # @param [RemoteDevelopment::Workspace] workspace # @param [String] desired_state + # @return [void] def do_change_workspace_state(workspace:, desired_state:) workspace_update_mutation_args = { id: global_id_of(workspace), @@ -310,11 +326,14 @@ def do_change_workspace_state(workspace:, desired_state:) # noinspection RubyResolve expect(workspace.reload.desired_state_updated_at).to eq(Time.current) + + nil end # @param [Symbol] name # @param [Hash] input # @param [User] user + # @return [Hash] def do_graphql_mutation_post(name:, input:, user:) mutation = graphql_mutation(name, input) post_graphql_mutation(mutation, current_user: user) @@ -368,7 +387,7 @@ def do_reconcile_post(params:, agent_token:) workspace = do_create_workspace additional_args_for_expected_config_to_apply = - build_additional_args_for_expected_config_to_apply( + build_additional_args_for_expected_config_to_apply_yaml_stream( network_policy_enabled: true, dns_zone: dns_zone, namespace_path: workspace_project_namespace.full_path, diff --git a/ee/spec/support/helpers/remote_development/integration_spec_helpers.rb b/ee/spec/support/helpers/remote_development/integration_spec_helpers.rb index 67a18a0c0fe2c20adb8b9ec60ecb9b4fd8607bb5..eec7cfd85f4a50e26ae3be8247292535a344a686 100644 --- a/ee/spec/support/helpers/remote_development/integration_spec_helpers.rb +++ b/ee/spec/support/helpers/remote_development/integration_spec_helpers.rb @@ -8,7 +8,9 @@ module IntegrationSpecHelpers # @param [String] dns_zone # @param [String] namespace_path # @param [String] project_name - def build_additional_args_for_expected_config_to_apply( + # @param [Array] image_pull_secrets + # @return [Hash] + def build_additional_args_for_expected_config_to_apply_yaml_stream( network_policy_enabled:, dns_zone:, namespace_path:, @@ -27,7 +29,8 @@ def build_additional_args_for_expected_config_to_apply( # @param [Array] workspace_agent_infos # @param [String] update_type # @param [QA::Resource::Clusters::AgentToken] agent_token - # @param [settingd] Hash + # @param [Hash] settings + # @return [Hash] def simulate_agentk_reconcile_post(workspace_agent_infos:, update_type:, agent_token:, settings:) post_params = { update_type: update_type, @@ -50,9 +53,11 @@ def simulate_agentk_reconcile_post(workspace_agent_infos:, update_type:, agent_t # @param [String] expected_desired_state # @param [String] expected_actual_state # @param [String] expected_resource_version - # @param [Hash] expected_config_to_apply + # @param [String] expected_config_to_apply_yaml_stream # @param [Integer] expected_rails_infos_count + # @param [Integer] time_to_travel_after_poll # rubocop:disable Metrics/ParameterLists -- This is a test helper, not worth introducing a parameters object, at least for now. + # @return [void] def simulate_poll( workspace:, agent_token:, @@ -61,7 +66,7 @@ def simulate_poll( expected_desired_state:, expected_actual_state:, expected_resource_version:, - expected_config_to_apply:, + expected_config_to_apply_yaml_stream:, expected_rails_infos_count: 1, time_to_travel_after_poll: nil ) @@ -86,7 +91,7 @@ def simulate_poll( expected_desired_state: expected_desired_state, expected_actual_state: expected_actual_state, expected_resource_version: expected_resource_version, - expected_config_to_apply: expected_config_to_apply, + expected_config_to_apply_yaml_stream: expected_config_to_apply_yaml_stream, expected_rails_infos_count: expected_rails_infos_count ) @@ -101,6 +106,8 @@ def simulate_poll( # `.../workspace_operations/reconcile/persistence/workspaces_to_be_returned_finder.rb` travel(reconciliation_interval_seconds) end + + nil end # @param [Hash] response_json @@ -108,15 +115,16 @@ def simulate_poll( # @param [String] expected_desired_state # @param [String] expected_actual_state # @param [String] expected_resource_version - # @param [Hash] expected_config_to_apply + # @param [String] expected_config_to_apply_yaml_stream # @param [Integer] expected_rails_infos_count + # @return [void] def assert_response( response_json, workspace:, expected_desired_state:, expected_actual_state:, expected_resource_version:, - expected_config_to_apply:, + expected_config_to_apply_yaml_stream:, expected_rails_infos_count: ) infos = response_json.fetch(:workspace_rails_infos) @@ -136,21 +144,24 @@ def assert_response( expect(info.fetch(:actual_state)).to eq(expected_actual_state) expect(info.fetch(:deployment_resource_version)).to eq(expected_resource_version) - config_to_apply = info.fetch(:config_to_apply) - expect(config_to_apply).to eq(expected_config_to_apply) + config_to_apply_yaml_stream = info.fetch(:config_to_apply) + expect(config_to_apply_yaml_stream).to eq(expected_config_to_apply_yaml_stream) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token - # @param [Hash] additional_args_for_create_config_to_apply - def simulate_first_poll(workspace:, agent_token:, **additional_args_for_create_config_to_apply) + # @param [Hash] additional_args_for_create_config_to_apply_yaml_stream + # @return [void] + def simulate_first_poll(workspace:, agent_token:, **additional_args_for_create_config_to_apply_yaml_stream) # SIMULATE RECONCILE RESPONSE TO AGENTK SENDING NEW WORKSPACE - expected_config_to_apply = create_config_to_apply( + expected_config_to_apply_yaml_stream = create_config_to_apply_yaml_stream( workspace: workspace, started: true, include_all_resources: true, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) simulate_poll( @@ -161,12 +172,15 @@ def simulate_first_poll(workspace:, agent_token:, **additional_args_for_create_c expected_resource_version: nil, expected_desired_state: RUNNING, expected_actual_state: CREATION_REQUESTED, - expected_config_to_apply: expected_config_to_apply + expected_config_to_apply_yaml_stream: expected_config_to_apply_yaml_stream ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @return [void] def simulate_second_poll(workspace:, agent_token:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO RUNNING ACTUAL_STATE @@ -187,17 +201,20 @@ def simulate_second_poll(workspace:, agent_token:) expected_desired_state: RUNNING, expected_actual_state: RUNNING, expected_resource_version: resource_version, - expected_config_to_apply: nil + expected_config_to_apply_yaml_stream: "" ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token - # @param [Hash] additional_args_for_create_config_to_apply + # @param [Hash] additional_args_for_create_config_to_apply_yaml_stream + # @return [void] def simulate_third_poll( workspace:, agent_token:, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) # SIMULATE RECONCILE RESPONSE TO AGENTK UPDATING WORKSPACE TO STOPPED DESIRED_STATE @@ -210,10 +227,10 @@ def simulate_third_poll( resource_version: resource_version ) - expected_config_to_apply = create_config_to_apply( + expected_config_to_apply_yaml_stream = create_config_to_apply_yaml_stream( workspace: workspace, started: false, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) simulate_poll( @@ -224,12 +241,15 @@ def simulate_third_poll( expected_desired_state: STOPPED, expected_actual_state: RUNNING, expected_resource_version: resource_version, - expected_config_to_apply: expected_config_to_apply + expected_config_to_apply_yaml_stream: expected_config_to_apply_yaml_stream ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @return [void] def simulate_fourth_poll(workspace:, agent_token:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO STOPPING ACTUAL_STATE @@ -250,12 +270,15 @@ def simulate_fourth_poll(workspace:, agent_token:) expected_desired_state: STOPPED, expected_actual_state: STOPPING, expected_resource_version: resource_version, - expected_config_to_apply: nil + expected_config_to_apply_yaml_stream: "" ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @return [void] def simulate_fifth_poll(workspace:, agent_token:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO STOPPED ACTUAL_STATE @@ -276,11 +299,14 @@ def simulate_fifth_poll(workspace:, agent_token:) expected_desired_state: STOPPED, expected_actual_state: STOPPED, expected_resource_version: resource_version, - expected_config_to_apply: nil + expected_config_to_apply_yaml_stream: "" ) + + nil end # @param [QA::Resource::Clusters::AgentToken] agent_token + # @return [void] def simulate_sixth_poll(agent_token:) # SIMULATE RECONCILE RESPONSE TO AGENTK FOR PARTIAL RECONCILE TO SHOW NO RAILS_INFOS ARE SENT @@ -293,18 +319,21 @@ def simulate_sixth_poll(agent_token:) expected_desired_state: STOPPING, expected_actual_state: STOPPED, expected_resource_version: resource_version, - expected_config_to_apply: nil, + expected_config_to_apply_yaml_stream: "", expected_rails_infos_count: 0 ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token - # @param [Hash] additional_args_for_create_config_to_apply + # @param [Hash] additional_args_for_create_config_to_apply_yaml_stream + # @return [void] def simulate_seventh_poll( workspace:, agent_token:, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) # SIMULATE RECONCILE RESPONSE TO AGENTK FOR FULL RECONCILE TO SHOW ALL WORKSPACES ARE SENT IN RAILS_INFOS @@ -317,11 +346,11 @@ def simulate_seventh_poll( resource_version: resource_version ) - expected_config_to_apply = create_config_to_apply( + expected_config_to_apply_yaml_stream = create_config_to_apply_yaml_stream( workspace: workspace, started: false, include_all_resources: true, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) simulate_poll( @@ -332,17 +361,20 @@ def simulate_seventh_poll( expected_desired_state: STOPPED, expected_actual_state: STOPPED, expected_resource_version: resource_version, - expected_config_to_apply: expected_config_to_apply + expected_config_to_apply_yaml_stream: expected_config_to_apply_yaml_stream ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token - # @param [Hash] additional_args_for_create_config_to_apply + # @param [Hash] additional_args_for_create_config_to_apply_yaml_stream + # @return [void] def simulate_eighth_poll( workspace:, agent_token:, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) # SIMULATE RECONCILE RESPONSE TO AGENTK UPDATING WORKSPACE TO RUNNING DESIRED_STATE @@ -355,10 +387,10 @@ def simulate_eighth_poll( resource_version: resource_version ) - expected_config_to_apply = create_config_to_apply( + expected_config_to_apply_yaml_stream = create_config_to_apply_yaml_stream( workspace: workspace, started: true, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) simulate_poll( @@ -369,12 +401,16 @@ def simulate_eighth_poll( expected_desired_state: RUNNING, expected_actual_state: STOPPED, expected_resource_version: resource_version, - expected_config_to_apply: expected_config_to_apply + expected_config_to_apply_yaml_stream: expected_config_to_apply_yaml_stream ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @param [Integer] time_to_travel_after_poll + # @return [void] def simulate_ninth_poll(workspace:, agent_token:, time_to_travel_after_poll:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO RUNNING ACTUAL_STATE @@ -395,18 +431,21 @@ def simulate_ninth_poll(workspace:, agent_token:, time_to_travel_after_poll:) expected_desired_state: RUNNING, expected_actual_state: RUNNING, expected_resource_version: resource_version, - expected_config_to_apply: nil, + expected_config_to_apply_yaml_stream: "", time_to_travel_after_poll: time_to_travel_after_poll ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token - # @param [Hash] additional_args_for_create_config_to_apply + # @param [Hash] additional_args_for_create_config_to_apply_yaml_stream + # @return [void] def simulate_tenth_poll( workspace:, agent_token:, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) # SIMULATE RECONCILE RESPONSE TO AGENTK UPDATING WORKSPACE TO STOPPED DESIRED_STATE @@ -419,10 +458,10 @@ def simulate_tenth_poll( resource_version: resource_version ) - expected_config_to_apply = create_config_to_apply( + expected_config_to_apply_yaml_stream = create_config_to_apply_yaml_stream( workspace: workspace, started: false, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) simulate_poll( @@ -433,12 +472,15 @@ def simulate_tenth_poll( expected_desired_state: STOPPED, expected_actual_state: RUNNING, expected_resource_version: resource_version, - expected_config_to_apply: expected_config_to_apply + expected_config_to_apply_yaml_stream: expected_config_to_apply_yaml_stream ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @return [void] def simulate_eleventh_poll(workspace:, agent_token:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO STOPPING ACTUAL_STATE @@ -459,12 +501,16 @@ def simulate_eleventh_poll(workspace:, agent_token:) expected_desired_state: STOPPED, expected_actual_state: STOPPING, expected_resource_version: resource_version, - expected_config_to_apply: nil + expected_config_to_apply_yaml_stream: "" ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @param [Integer] time_to_travel_after_poll + # @return [void] def simulate_twelfth_poll(workspace:, agent_token:, time_to_travel_after_poll:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO STOPPED ACTUAL_STATE @@ -485,18 +531,21 @@ def simulate_twelfth_poll(workspace:, agent_token:, time_to_travel_after_poll:) expected_desired_state: STOPPED, expected_actual_state: STOPPED, expected_resource_version: resource_version, - expected_config_to_apply: nil, + expected_config_to_apply_yaml_stream: "", time_to_travel_after_poll: time_to_travel_after_poll ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token - # @param [Hash] additional_args_for_create_config_to_apply + # @param [Hash] additional_args_for_create_config_to_apply_yaml_stream + # @return [void] def simulate_thirteenth_poll( workspace:, agent_token:, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) # SIMULATE RECONCILE RESPONSE TO AGENTK UPDATING WORKSPACE TO TERMINATED DESIRED_STATE @@ -509,10 +558,10 @@ def simulate_thirteenth_poll( resource_version: resource_version ) - expected_config_to_apply = create_config_to_apply( + expected_config_to_apply_yaml_stream = create_config_to_apply_yaml_stream( workspace: workspace, started: false, - **additional_args_for_create_config_to_apply + **additional_args_for_create_config_to_apply_yaml_stream ) simulate_poll( @@ -523,12 +572,15 @@ def simulate_thirteenth_poll( expected_desired_state: TERMINATED, expected_actual_state: STOPPED, expected_resource_version: resource_version, - expected_config_to_apply: expected_config_to_apply + expected_config_to_apply_yaml_stream: expected_config_to_apply_yaml_stream ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @return [void] def simulate_fourteenth_poll(workspace:, agent_token:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO STOPPING ACTUAL_STATE @@ -548,12 +600,15 @@ def simulate_fourteenth_poll(workspace:, agent_token:) expected_desired_state: TERMINATED, expected_actual_state: TERMINATING, expected_resource_version: '6', - expected_config_to_apply: nil + expected_config_to_apply_yaml_stream: "" ) + + nil end # @param [Workspace] workspace # @param [QA::Resource::Clusters::AgentToken] agent_token + # @return [void] def simulate_fifteenth_poll(workspace:, agent_token:) # SIMULATE RECONCILE REQUEST FROM AGENTK UPDATING WORKSPACE TO TERMINATED ACTUAL_STATE @@ -573,8 +628,10 @@ def simulate_fifteenth_poll(workspace:, agent_token:) expected_desired_state: TERMINATED, expected_actual_state: TERMINATED, expected_resource_version: '6', - expected_config_to_apply: nil + expected_config_to_apply_yaml_stream: "" ) + + nil end 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 41ee3a67bae9ed905f7cee6968a306304929d7ac..ac8848fb7e3546cf7a1e5577f1917a5c13def7fc 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 @@ -270,7 +270,7 @@ def create_workspace_agent_info_hash( # rubocop:enable Lint/DuplicateBranch # rubocop:enable Layout/LineLength - config_to_apply_yaml = create_config_to_apply( + config_to_apply = create_config_to_apply( workspace: workspace, workspace_variables_environment: workspace_variables_environment, workspace_variables_file: workspace_variables_file, @@ -280,35 +280,20 @@ def create_workspace_agent_info_hash( include_all_resources: false, dns_zone: dns_zone ) - config_to_apply = YAML.load_stream(config_to_apply_yaml) - latest_k8s_deployment_info = config_to_apply.detect { |config| config.fetch('kind') == 'Deployment' } - latest_k8s_deployment_info['metadata']['resourceVersion'] = resource_version - latest_k8s_deployment_info['status'] = YAML.safe_load(status) + + latest_k8s_deployment_info = config_to_apply.detect { |config| config.fetch(:kind) == 'Deployment' } + latest_k8s_deployment_info[:metadata][:resourceVersion] = resource_version + latest_k8s_deployment_info[:status] = yaml_safe_load_symbolized(status) info[:latest_k8s_deployment_info] = latest_k8s_deployment_info info[:error_details] = error_details - info.deep_symbolize_keys.to_h + info end - # rubocop:enable Metrics/ParameterLists - # rubocop:enable Metrics/CyclomaticComplexity - # rubocop:enable Metrics/PerceivedComplexity - def create_workspace_rails_info( - name:, - namespace:, - desired_state:, - actual_state:, - deployment_resource_version: nil, - config_to_apply: nil - ) - { - name: name, - namespace: namespace, - desired_state: desired_state, - actual_state: actual_state, - deployment_resource_version: deployment_resource_version, - config_to_apply: config_to_apply - }.compact + # rubocop:enable Metrics/ParameterLists, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + + def create_config_to_apply_yaml_stream(workspace:, **args) + create_config_to_apply(workspace: workspace, **args).map { |resource| YAML.dump(resource.deep_stringify_keys) }.join end def create_config_to_apply(workspace:, **args) @@ -438,13 +423,15 @@ def create_config_to_apply_v3( ) ) - workspace_resource_quota = workspace_resource_quota( - workspace_name: workspace.name, - workspace_namespace: workspace.namespace, - labels: labels, - annotations: annotations, - max_resources_per_workspace: max_resources_per_workspace - ) + if max_resources_per_workspace.present? + workspace_resource_quota = workspace_resource_quota( + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: annotations, + max_resources_per_workspace: max_resources_per_workspace + ) + end workspace_service_account = workspace_service_account( name: workspace.name, @@ -472,15 +459,11 @@ def create_config_to_apply_v3( end end - resources.map do |resource| - yaml = YAML.dump(Gitlab::Utils.deep_sort_hash(resource).deep_stringify_keys) - yaml.gsub!('test-project', project_name) - yaml.gsub!('test-group', namespace_path) - yaml.gsub!('http://localhost/', root_url) - yaml - end.join + normalize_resources(namespace_path, project_name, resources) end + # rubocop:enable Metrics/ParameterLists, Metrics/AbcSize + def workspace_inventory(workspace_name:, workspace_namespace:, agent_id:) { kind: "ConfigMap", @@ -497,6 +480,7 @@ def workspace_inventory(workspace_name:, workspace_namespace:, agent_id:) } end + # rubocop:disable Metrics/ParameterLists -- Cleanup as part of https://gitlab.com/gitlab-org/gitlab/-/issues/421687 def workspace_deployment( workspace_name:, workspace_namespace:, @@ -823,7 +807,8 @@ def workspace_deployment( deployment end - # rubocop:enable Metrics/ParameterLists, Metrics/AbcSize + + # rubocop:enable Metrics/ParameterLists def workspace_service( workspace_name:, @@ -961,6 +946,17 @@ def workspace_resource_quota( annotations:, max_resources_per_workspace: ) + max_resources_per_workspace => { + limits: { + cpu: limits_cpu, + memory: limits_memory + }, + requests: { + cpu: requests_cpu, + memory: requests_memory + } + } + { apiVersion: "v1", kind: "ResourceQuota", @@ -972,10 +968,10 @@ def workspace_resource_quota( }, spec: { hard: { - "limits.cpu": max_resources_per_workspace.dig(:limits, :cpu), - "limits.memory": max_resources_per_workspace.dig(:limits, :memory), - "requests.cpu": max_resources_per_workspace.dig(:requests, :cpu), - "requests.memory": max_resources_per_workspace.dig(:requests, :memory) + "limits.cpu": limits_cpu, + "limits.memory": limits_memory, + "requests.cpu": requests_cpu, + "requests.memory": requests_memory } } } @@ -1064,15 +1060,37 @@ def workspace_secret_file( } end + def normalize_resources(namespace_path, project_name, resources) + # Convert to YAML to normalize project_name, namespace_path, and root_url + normalized_resources_yaml = resources.map do |resource| + yaml = YAML.dump(resource) + yaml.gsub!('test-project', project_name) + yaml.gsub!('test-group', namespace_path) + yaml.gsub!('http://localhost/', root_url) + yaml + end.join + + # Convert back to array of hashes, symbolizing keys, and deep sorting for test fixture comparison stability + YAML.load_stream(normalized_resources_yaml).map do |resource| + sorted_then_symbolized_resource = Gitlab::Utils.deep_sort_hash(resource).deep_symbolize_keys + symbolized_then_sorted_resource = Gitlab::Utils.deep_sort_hash(resource.deep_symbolize_keys) + + # Verify there's no unexpected sorting instability for symbols vs. strings + raise "Sorting stability order error!" unless sorted_then_symbolized_resource == symbolized_then_sorted_resource + + sorted_then_symbolized_resource + end + end + def get_workspace_variables_environment(workspace_variables:) workspace_variables.with_variable_type_environment.each_with_object({}) do |workspace_variable, hash| - hash[workspace_variable.key] = workspace_variable.value + hash[workspace_variable.key.to_sym] = workspace_variable.value end end def get_workspace_variables_file(workspace_variables:) workspace_variables.with_variable_type_file.each_with_object({}) do |workspace_variable, hash| - hash[workspace_variable.key] = workspace_variable.value + hash[workspace_variable.key.to_sym] = workspace_variable.value end end @@ -1085,7 +1103,12 @@ def get_workspace_host_template_environment(workspace_name, dns_zone) end def yaml_safe_load_symbolized(yaml) - YAML.safe_load(yaml).to_h.deep_symbolize_keys + is_multiple_docs = YAML.load_stream(yaml).size > 1 + if is_multiple_docs + YAML.load_stream(yaml).map { |doc| YAML.safe_load(YAML.dump(doc)).deep_symbolize_keys } + else + YAML.safe_load(yaml).deep_symbolize_keys + end end def example_default_devfile_yaml