Find a better way to specify default value for CI config entry node
The issue was raised from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9666#note_24549599
The background is that we want some default values for .gitlab-ci.yml, for example the following config:
job0:
script: pwd
Should be interpreted as if: (just a simplified example, not how it's working right now)
job0:
script: pwd
stage: test
That is if stage is not specified, it would use a default value. To retrieve the value, we do something like this:
job = Gitlab::Ci::Config::Entry::Factory.new(Gitlab::Ci::Config::Entry::Job)
.value({script: 'pwd'}).metadata(name: :job0).with(key: :job0).create!
job.compose!
job.value # => {:script=>["pwd"], :name=>:job0, :commands=>"pwd", :stage=>"test"}
How this was currently implemented was actually in Job#value:
def value
@config.merge(to_hash.compact)
end
# Simplified, not actual code
def to_hash
{ name: name,
script: script_value,
commands: commands,
stage: stage_value }
end
If we remove that stage in to_hash, then the default stage test won't be set. The reason resides in Node#value:
def value
if leaf?
@config
else
meaningful = @entries.select do |_key, value|
value.specified? && value.relevant?
end
Hash[meaningful.map { |key, entry| [key, entry.value] }]
end
end
Here the value would only pick specified? and relevant? nodes. However in the original example, stage is not specified, which is a Unspecified node wrapping around Stage node. It's not specified therefore it's not populated in the value. It's only populated at the moment because Node#value is overridden by Job#value.
For now, I am doing the same for Cache:
def value
super.merge(key: key_value)
end
Ideally, we should probably not override any value method, and have a way to populate default values.
Note that Node.default is only used to populate the node whenever it's an Unspecified node, but the particular node is not populated in Node#value. This is a bit confusing. If it's never picked in Node#value, why are we setting the default value? It's because Job#value is using it. Now also Cache#value would be using it.
We should also be careful that we should not break gitlab-ce#18775
It would be nice if we have an API like this:
entry :stage, Entry::Stage, unspecified: :default,
description: 'Pipeline stage this job will be executed into.'
Or maybe just make this default, so we don't tell the difference if it's unspecified or having a default. Might need to explore a bit.