Skip to content

Fix error in Mapper::Normalizer when CI include value type is invalid

Leaminn Ma requested to merge fix-ci-include-value-type-error into master

What does this MR do and why?

In #425139 (closed), we logged a backtrace that indicated an error:

undefined method `deep_symbolize_keys' for 123:Integer

This error occurs when expanding CI includes at Config::External::Mapper::Normalizer::29.

It can be reproduced by specifying an invalid value type for the include: keyword like so:

include: 123 # Also fails when this is a Boolean

It also errors when you include an array of locations like so:

include:
  - 123 # Also fails when this is a Boolean or Array

This MR fixes this problem by adding type validation on the location value. It should only be either a String or Hash type.

Notes about the approach:

  • You might be wondering why this validation isn't already covered by validates :config, hash_or_string: true in Ci::Config::Entry::Include. This is because included files are expanded and mostly validated before @root.compose! runs in Ci::Config.
  • Some of the validations in Ci::Config::Entry::Include do get executed, but that's only in certain scenarios where the include keyword "falls through" to the @root.compose! step. This can happen if it's not removed by Ci::Config::External::Processor#remove_include_keyword!. I think this is why we haven't just removed these Entry classes yet.
  • For the reasons above, it seems the most appropriate to add the validation logic to Ci::Config::External::Mapper::Normalizer. Also, it:
    • Aligns with the notion of "normalizing"
    • Is one of the first steps in the file expansion process
    • Is directly dependent on the value type being a string or hash

Resolves #425139 (closed).

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots

Before fix After fix
Screenshot_2024-01-23_at_4.49.01_PM Screenshot_2024-01-23_at_5.27.38_PM

How to set up and validate locally

  1. Update your project's .gitlab-ci.yml file with:
include: 123
  1. Observe that it outputs a validation error message as seen under Screenshots.

Related to #425139 (closed)

Edited by Leaminn Ma

Merge request reports