diff --git a/ee/app/validators/json_schemas/workspaces_kubernetes.json b/ee/app/validators/json_schemas/workspaces_kubernetes.json index c7732393179360ce2dca83f4983bca59d943ed84..47683746b34934bfba6abd7592500dc454a0fed7 100644 --- a/ee/app/validators/json_schemas/workspaces_kubernetes.json +++ b/ee/app/validators/json_schemas/workspaces_kubernetes.json @@ -1,131 +1,44 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", "description": "This schema contains the objects created by devfile-gem in the order they are supposed to appear.", - "anyOf": [ - { - "type": "array", - "unevaluatedItems": false, - "prefixItems": [ - { - "$ref": "#/definitions/workspace_inventory_config_map" - }, - { - "$ref": "#/definitions/deployment" - }, - { - "$ref": "#/definitions/service" - }, - { - "$ref": "#/definitions/persistent_volume_claim" - }, - { - "$ref": "#/definitions/service_account" - }, - { - "$ref": "#/definitions/network_policy" - }, - { - "$ref": "#/definitions/scripts_config_map" - }, - { - "$ref": "#/definitions/secrets_inventory_config_map" - }, - { - "$ref": "#/definitions/resource_quota" - }, - { - "$ref": "#/definitions/env_var_secret" - }, - { - "$ref": "#/definitions/file_secret" - } - ] - }, - { - "type": "array", - "unevaluatedItems": false, - "prefixItems": [ - { - "$ref": "#/definitions/workspace_inventory_config_map" - }, - { - "$ref": "#/definitions/deployment" - }, - { - "$ref": "#/definitions/service" - }, - { - "$ref": "#/definitions/persistent_volume_claim" - }, - { - "$ref": "#/definitions/service_account" - }, - { - "$ref": "#/definitions/network_policy" - }, - { - "$ref": "#/definitions/scripts_config_map" - }, - { - "$ref": "#/definitions/secrets_inventory_config_map" - }, - { - "$ref": "#/definitions/env_var_secret" - }, - { - "$ref": "#/definitions/file_secret" - } - ] - }, - { - "type": "array", - "unevaluatedItems": false, - "prefixItems": [ - { - "$ref": "#/definitions/workspace_inventory_config_map" - }, - { - "$ref": "#/definitions/secrets_inventory_config_map" - } - ] - }, - { - "type": "array", - "unevaluatedItems": false, - "prefixItems": [ - { - "$ref": "#/definitions/workspace_inventory_config_map" - }, - { - "$ref": "#/definitions/deployment" - }, - { - "$ref": "#/definitions/service" - }, - { - "$ref": "#/definitions/persistent_volume_claim" - }, - { - "$ref": "#/definitions/service_account" - }, - { - "$ref": "#/definitions/network_policy" - }, - { - "$ref": "#/definitions/secrets_inventory_config_map" - }, - { - "$ref": "#/definitions/resource_quota" - }, - { - "$ref": "#/definitions/env_var_secret" - }, - { - "$ref": "#/definitions/file_secret" - } - ] - } - ], + "type": "array", + "items": { + "anyOf": [ + { + "$ref": "#/definitions/workspace_inventory_config_map" + }, + { + "$ref": "#/definitions/deployment" + }, + { + "$ref": "#/definitions/service" + }, + { + "$ref": "#/definitions/persistent_volume_claim" + }, + { + "$ref": "#/definitions/service_account" + }, + { + "$ref": "#/definitions/network_policy" + }, + { + "$ref": "#/definitions/scripts_config_map" + }, + { + "$ref": "#/definitions/secrets_inventory_config_map" + }, + { + "$ref": "#/definitions/resource_quota" + }, + { + "$ref": "#/definitions/env_var_secret" + }, + { + "$ref": "#/definitions/file_secret" + } + ] + }, "additionalProperties": false, "definitions": { "workspace_inventory_config_map": { diff --git a/ee/app/validators/remote_development/desired_config_array_validator.rb b/ee/app/validators/remote_development/desired_config_array_validator.rb new file mode 100644 index 0000000000000000000000000000000000000000..4a254251c2413352e3c812bec770cfd0388edb6a --- /dev/null +++ b/ee/app/validators/remote_development/desired_config_array_validator.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module RemoteDevelopment + class DesiredConfigArrayValidator < ActiveModel::EachValidator + EXPECTED_SEQUENCE = [ + { kind: "ConfigMap", name_pattern: /workspace-inventory$/ }, + { kind: "Deployment", name_pattern: /.*/ }, + { kind: "Service", name_pattern: /.*/ }, + { kind: "PersistentVolumeClaim", name_pattern: /.*/ }, + { kind: "ServiceAccount", name_pattern: /.*/ }, + { kind: "NetworkPolicy", name_pattern: /.*/ }, + { kind: "ConfigMap", name_pattern: /scripts-configmap$/ }, + { kind: "ConfigMap", name_pattern: /secrets-inventory$/ }, + { kind: "ResourceQuota", name_pattern: /.*/ }, + { kind: "Secret", name_pattern: /env-var$/ }, + { kind: "Secret", name_pattern: /file$/ } + ].freeze + + # @param [RemoteDevelopment::DesiredConfig] record + # @param [Symbol] attribute + # @param [Array] value + # @return [void] The result is stored in the errors param + def validate_each(record, attribute, value) + unless value.is_a?(Array) + record.errors.add(attribute, "must be an array") + return + end + + if value.empty? + record.errors.add(attribute, "must not be empty") + return + end + + value = value.map(&:deep_symbolize_keys) + normalized_expected_array = normalize_expected_order(value) + normalized_config_array = value.map do |item| + item => { + kind: String => item_kind, + metadata: { + name: String => item_name + }, + ** + } + + "#{item_kind}/#{item_name}" + end + + validate_order(normalized_config_array, normalized_expected_array, record.errors, attribute) + end + + private + + # @param [Array] normalized_config_array + # @param [Array] normalized_expected_array + # @param [ActiveModel::Errors] errors - stores the validation results + # @param [Symbol] attribute_symbol - symbol of the attribute associated with the error + # @return [Void] + def validate_order(normalized_config_array, normalized_expected_array, errors, attribute_symbol) + normalized_config_array.each_with_index do |item, index| + expected_index = normalized_expected_array.index(item) + + if expected_index.nil? + errors.add(attribute_symbol, "item #{item} at index #{index} is unexpected") + next + end + + next if expected_index == index + + errors.add(attribute_symbol, "item #{item} at index #{index} must be at #{expected_index}") + end + end + + # This method normalizes the expected order of items based on the config array + # It creates a mapping of expected positions for each kind/name combination + # + # @param [Array] config_array The array of configuration items to normalize + # @return [Array] An array with expected order of items + def normalize_expected_order(config_array) + expected_positions = [] + + EXPECTED_SEQUENCE.each_with_index do |expected, _| + config_array.each do |item| + item => { + kind: String => item_kind, + metadata: { + name: String => item_name + } + } + + expected_positions << "#{item_kind}/#{item_name}" if matches?(item_kind, item_name, expected) + end + end + + expected_positions + end + + # Validates if the given configuration matches the expected value + # + # @param [String] kind The type of configuration to validate + # @param [String] name The name of the configuration item + # @param [Object] expected The expected value to match against. See {#EXPECTED_SEQUENCE} above. + # @return [Boolean] Returns true if the configuration matches the expected value, false otherwise + def matches?(kind, name, expected) + kind == expected[:kind] && expected[:name_pattern].match?(name) + end + end +end diff --git a/ee/lib/remote_development/workspace_operations/desired_config.rb b/ee/lib/remote_development/workspace_operations/desired_config.rb index cc3b15076599d0a0552afea5763769712be82e46..9bb737bb1daa7ea82b28dffc38c168f99bb132f5 100644 --- a/ee/lib/remote_development/workspace_operations/desired_config.rb +++ b/ee/lib/remote_development/workspace_operations/desired_config.rb @@ -20,6 +20,7 @@ class DesiredConfig detail_errors: true, size_limit: 64.kilobytes } + validates :desired_config_array, 'remote_development/desired_config_array': true # @param [DesiredConfig] other # @return [Boolean] diff --git a/ee/spec/fixtures/remote_development/example.desired_config.json b/ee/spec/fixtures/remote_development/example.desired_config.json index f6f91165c69a016189aa36e14cdff66d3bef8f2e..70cbf6f9b6d08747c7fd79bd9201ff1cd57fe9d0 100644 --- a/ee/spec/fixtures/remote_development/example.desired_config.json +++ b/ee/spec/fixtures/remote_development/example.desired_config.json @@ -332,6 +332,39 @@ }, "status": {} }, + { + "apiVersion": "v1", + "kind": "PersistentVolumeClaim", + "metadata": { + "annotations": { + "environment": "production", + "team": "engineering", + "config.k8s.io/owning-inventory": "workspace-991-990-fedcba-workspace-inventory", + "workspaces.gitlab.com/host-template": "{{.port}}-workspace-991-990-fedcba.workspaces.dev.test", + "workspaces.gitlab.com/id": "993", + "workspaces.gitlab.com/max-resources-per-workspace-sha256": "24aefc317e11db538ede450d1773e273966b9801b988d49e1219f2a9bf8e7f66" + }, + "creationTimestamp": null, + "labels": { + "app": "workspace", + "tier": "development", + "agent.gitlab.com/id": "991" + }, + "name": "workspace-991-990-fedcba-gl-workspace-custom-pvc", + "namespace": "gl-rd-ns-991-990-fedcba" + }, + "spec": { + "accessModes": [ + "ReadWriteOnce" + ], + "resources": { + "requests": { + "storage": "50Gi" + } + } + }, + "status": {} + }, { "apiVersion": "v1", "automountServiceAccountToken": false, diff --git a/ee/spec/lib/remote_development/workspace_operations/desired_config_spec.rb b/ee/spec/lib/remote_development/workspace_operations/desired_config_spec.rb index 5e1f90997a7f1ad2848f4892f4cfa149c4d0a457..e434cab9b2da4cd5e8fdc9408da524732cfaed7e 100644 --- a/ee/spec/lib/remote_development/workspace_operations/desired_config_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/desired_config_spec.rb @@ -46,7 +46,7 @@ end end - it 'fails' do + it 'fails', :unlimited_max_formatted_output_length do expect(desired_config).to be_invalid # This validation does not fail for other kinds except for ConfigMap @@ -54,8 +54,8 @@ expect(desired_config.errors[:desired_config_array]) .to include( "object property at `/0/invalid-field` is a disallowed additional property", - "object property at `/6/invalid-field` is a disallowed additional property", - "object property at `/7/invalid-field` is a disallowed additional property" + "object property at `/7/invalid-field` is a disallowed additional property", + "object property at `/8/invalid-field` is a disallowed additional property" ) end end @@ -122,7 +122,7 @@ let(:expected_difference) do [ [ - "-", "[10]", + "-", "[11]", { apiVersion: "v1", data: {}, diff --git a/ee/spec/support/fast_spec/remote_development/fast_spec_helper_support.rb b/ee/spec/support/fast_spec/remote_development/fast_spec_helper_support.rb index fdfe3933a4aa964105496e8ee80d305bd3c1a395..c37b5949901bd7e850145e9898907e2938c73342 100644 --- a/ee/spec/support/fast_spec/remote_development/fast_spec_helper_support.rb +++ b/ee/spec/support/fast_spec/remote_development/fast_spec_helper_support.rb @@ -13,6 +13,7 @@ require_relative '../../../../app/models/remote_development/enums/workspace_variable' require_relative '../../../../app/models/remote_development/workspace_state_helpers' require_relative '../../../../app/validators/ee/json_schema_validator' +require_relative '../../../../app/validators/remote_development/desired_config_array_validator' require_relative '../../../../lib/remote_development/files' require_relative '../../../../lib/remote_development/remote_development_constants' require_relative '../../../../lib/remote_development/workspace_operations/create/create_constants' diff --git a/ee/spec/validators/remote_development/desired_config_array_validator_spec.rb b/ee/spec/validators/remote_development/desired_config_array_validator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a721c374420f6a23a11d3860fa1268b8ee9107e3 --- /dev/null +++ b/ee/spec/validators/remote_development/desired_config_array_validator_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::DesiredConfigArrayValidator, feature_category: :workspaces do + include_context "with remote development shared fixtures" + let(:model) do + Class.new do + # @return [String] + def self.name + "DesiredConfigArrayValidatorTest" + end + + include ActiveModel::Model + include ActiveModel::Validations + + attr_accessor :desired_config_array + alias_method :desired_config_before_type_cast, :desired_config_array + + validates :desired_config_array, 'remote_development/desired_config_array': true + end.new + end + + let(:desired_config_array_with_jumbled_items) do + items = create_desired_config_array + items[0], items[-1] = items[-1], items[0] + items[6], items[7] = items[7], items[6] + items + end + + let(:desired_config_array_with_unexpected_items) do + items = create_desired_config_array + items << { kind: "UnexpectedKind", metadata: { name: "unexpected-item" } } + items + end + + let(:desired_config_array_with_missing_items) { create_desired_config_array.tap { |items| items.delete_at(3) } } + + using RSpec::Parameterized::TableSyntax + + where(:desired_config_array, :validity, :errors) do + # rubocop:disable Layout/LineLength -- The RSpec table syntax often requires long lines for errors + # @formatter:off - Turn off RubyMine autoformatting + create_desired_config_array | true | {} + ref(:desired_config_array_with_missing_items) | true | {} + ref(:desired_config_array_with_unexpected_items) | false | { desired_config_array: ["item UnexpectedKind/unexpected-item at index 12 is unexpected"] } + ref(:desired_config_array_with_jumbled_items) | false | { desired_config_array: ["item Secret/workspace-991-990-fedcba-file at index 0 must be at 11", "item ConfigMap/workspace-991-990-fedcba-scripts-configmap at index 6 must be at 7", "item NetworkPolicy/workspace-991-990-fedcba at index 7 must be at 6", "item ConfigMap/workspace-991-990-fedcba-workspace-inventory at index 11 must be at 0"] } + nil | false | { desired_config_array: ['must be an array'] } + {} | false | { desired_config_array: ['must be an array'] } + [] | false | { desired_config_array: ['must not be empty'] } + # @formatter:on + # rubocop:enable Layout/LineLength + end + + with_them do + before do + model.desired_config_array = desired_config_array + model.validate + end + + it { expect(model.valid?).to eq(validity) } + it { expect(model.errors.messages).to eq(errors) } + end +end