updatecli
updatecli copied to clipboard
Ability to globally define the `scms` block (v0.17+)
Is your feature request related to a problem?
Looking at the new syntax of updatecli manifest for version 0.17.0 and above like in this PR, we can see the block scms
is repeated word for word in every manifest.
Describe the solution you'd like I'd like the ability to define it globally so we don't have to repeat it on every manifest needing a pull request.
Describe alternatives you've considered N/A
Additional context N/A
I have been thinking about this issue quite a lot recently and it will require some refactoring anyway. At the moment everything is defined per pipeline where a pipeline matches an updatecli configuration file.
I am thinking to have something like:
.draft.yaml
scms: # Moving the SCM configuration at this level
scm;
kind: github
pipelines:
specDirectory: "./updatecli.d" # So we can read all configuration from a directory like we do now
spec:
- name: Bump updatecli file
specFile: "updatecli.yaml" ## So we can reference existing updatecli configuration without having to copy them here
- name: Bump go version
spec: ## So we can insert a pipeline configuration here
title: Bump golang version
sources:
...
conditions:
...
targets:
...
Something that I couldn't really decide is if If we need uniq source/condition/targets or if we need uniq "pipeline"
We might consider using viper for these, to allow specifying in a standard format either throug env vars, CLI flags or config files.
I vote for only reworking the SCM first as there is a need for it expressed by @lemeurherve in this issue (while having a top level wrapper config file could prove interesting but it not asked by a user for now)
Even if it's an interesting idea that could solve the problem, I have concerns with the viper approach as it introduces a new mechanism for the configuration.
What about having a new flag --base-config
that we would use like updatecli diff --base-config base.yaml --config updatecli.d --values values.yaml
and so we would do a deep merge between base.yaml and each of values under updatecli.d
base.yaml could contain any valid updatecli config.
We already have the ability to merge values files so we could have the same approach for config files as well. https://github.com/updatecli/updatecli/blob/b527434e4f72a3a6a8a6c6489bb375723c86d3e9/pkg/core/config/template.go#L148
Please note that the current function Merge
is not a deep merge so some improvement needs to be done on that side
I don't see any arguments against your proposal of a base config: it seems legit if it fixes the issue "As a end user, I don't want to maintain 1 copy of my SCM block for each pipeline, without changing my user experience".
I'm concerned by us, trying to re-invent a configuration mechanism with merge (that are note deep merge: which defeats the whole purpose of the configuration that could be overloaded): if we do not do deep merge, then we should allow any map/dictionnary in the manifests that could be defined on different locations, such as scm.
Switching to viper might not feel easy but it will immediatly underline the flaws of our configuration model and point at these flaws so we can fix it one time for all.
But it could be interesting to specify the new "base config": you should not have to break the exisint configuration model: viper provides you with the attribute config
, you don't have to care if it comes from an ENV var UPDATECLI_CONFIG
, or from a config flag --config
or from the attribute config:
in the base config file (or all of them with merging).
I am considering working on the current issue as It could simplify the autodiscovery manifest by not having to specify scm or action configuration.
Right now I see two ways to solve
1. New top level key "global" in existing manifest
We could have a new top level key named "global" such as
scms:
default1:
kind: git
spec:
...
global:
scms:
default2:
kind: git
spec:
...
Resources defines in a global section would be available to any pipeline as long as a pipeline do not specify a key such as "default2" in this example
2. New cli flag
Similarly to my idea explained in https://github.com/updatecli/updatecli/issues/430#issuecomment-1000703155 We could have a flag name "--gobal-config" or "--base-config", or "--default-global" that would work similarly to "--config" and would allow us to specify any Updatecli manifest as the default config.
Regarding the deep-merge topic, my guts tell me to avoid that rabbit hole but I don't have strong opinion
I'm not sure about the 1. New top level key "global" in existing manifest
. As a user, it seems confusing: which YAML manifest file holds the global? What if I got 2 different files? etc.
The 2nd one looks really promising!
Thanks for your feedback, I am also more in favor of the second approach. I'll try to find some time to dig into it