[go: up one dir, main page]

Skip to content

Backend: Update existing devfile_validator to return all errors

MR: Resolve "Backend: Update existing devfile_valid... (!193643 - merged)

Description

We want to allow users to validate their devfile and we want to show them all the errors that are present in the devfile instead of sending them only the first error.

To achieve this, we must ensure that before we do any check in the restrictions_enforcer, we check for the type of the field. If it does not satisfy the expected type, return the error for that specific field. Else, gather all the errors for that field, if any, and then return.

Acceptance criteria

  • Gather all the errors in the restrictions_enforcer before returning it.
  • Do no make any assumptions about the data type of the fields of the user provided devfile. Validate the data type as the first thing.

Implementation plan

In ee/lib/remote_development/devfile_operations/restrictions_enforcer.rb.

  1. Convert all and_then to map in self.enforce method. This is because we want to gather all the errors from each of the functions. If we were to use and_then, it would short-circuit the chain and we won't be able to gather all the errors.
  2. Add a new and_then at the end in self.enforce method and call a new get_result method.
    def self.get_result(context)
      errors = context[:devfile_validate_errors]
    
      return Gitlab::Fp::Result.ok(context) if errors.empty?
    
      Gitlab::Fp::Result.err(DevfileRestrictionsFailed.new({ details: errors, context: context }))
    end
  3. Rename the existing err method to append_err
    # @param [String] details
    # @param [Hash] context
    # @return [Hash]
    def self.append_err(details, context)
      context[:devfile_validate_errors].append(details)
    
      context
    end
  4. All the remaining methods should return a Hash instead of the current Gitlab::Fp::Result.
  5. In all of these methods, gather all the possible errors before returning. Ensure you check the data type of the field being validated in each of these methods. Here is the devfile schema - https://devfile.io/docs/2.2.0/devfile-schema e.g. validate_schema_version method would become
    # @param [Hash] context
    # @return [Hash]
    def self.validate_schema_version(context)
      devfile = devfile_to_validate(context)
    
      devfile_schema_version_string = devfile.fetch(:schemaVersion)
    
      unless devfile_schema_version_string.is_a?(String)
        return append_err(
          _("'schemaVersion' must be a string"),
          context
        )
      end
    
      begin
        devfile_schema_version = Gem::Version.new(devfile_schema_version_string)
      rescue ArgumentError
        return append_err(
          format(_("Invalid 'schemaVersion' '%{schema_version}'"), schema_version: devfile_schema_version_string),
          context
        )
      end
    
      minimum_schema_version = Gem::Version.new(REQUIRED_DEVFILE_SCHEMA_VERSION)
      unless devfile_schema_version == minimum_schema_version
        return append_err(
          format(
            _("'schemaVersion' '%{given_version}' is not supported, it must be '%{required_version}'"),
            given_version: devfile_schema_version_string,
            required_version: REQUIRED_DEVFILE_SCHEMA_VERSION
          ),
          context
        )
      end
    
      context
    end
Edited by Vishal Tak