applicationset
applicationset copied to clipboard
Add a new untyped template field, to support passing JSON/YAML objects into ApplicationSet template fields
This issue is opened to discuss the passing of JSON/YAML objects into the template
, rather than just individual string parameters.
This would be the ability, with the Git file generator, to pass a config.yaml like this (or equiv JSON):
cluster:
helmParameters:
- name: image.version
value: 1.1.1
and then have an ApplicationSet with a template like this:
spec:
# (...)
template:
spec:
source:
# (...)
helm:
parameters: {{ cluster.helmParameters}}
Such that when the Application is generated for this ApplicationSet, the YAML object (described by the config.json) will be inserted into the appropriate spot in the Helm parameters:
spec:
# (...)
template:
spec:
source:
repoURL: https://github.com/argoproj/argocd-example-apps.git
targetRevision: HEAD
path: guestbook
helm:
parameters:
- name: image.version
value: 1.1.1
However, (this is getting into implementation details) but I suspect it will require us to introduce a new field, as an alternative to template
, like this:
// ApplicationSetSpec represents a class of application set state.
type ApplicationSetSpec struct {
Generators []ApplicationSetGenerator `json:"generators"` // old
Template ApplicationSetTemplate `json:"template"` // old
UntypedTemplate apiextensionsv1.JSON `json:"templateUntyped"` // new hotness
SyncPolicy *ApplicationSetSyncPolicy `json:"syncPolicy,omitempty"` // old
}
which would look like this:
spec:
# (...)
templateUntyped:
spec:
source:
helm: # (...)
There's most definitely a better name for it than 'untyped template', but that's the idea :smile:. Ideally there would be a way to use the existing template
field for this, but I don't see a way: I don't want to lose the typed nature of the existing template
field, and yet we will need to break that structure with this feature.
#323 just to relate them
Idea is very good, can we also consider the situation where we would like to deploy same application in multiple namespaces in the given cluster.
can we also add some functions that will allow us to have defaults for the template var?
for example {{ myvar.myproperty | default "test" }}
It will be very helpful, especially in the application set where we want to set the default chart version if not specified by user.
we have a use case where we are deploying the same app in multiple namespaces, clusters in different regions. For each instance, we had to create a new confi.yaml. Can we add some feature that can avoid having breadcrumbs and just refer to a common config for diff instances of the same app?
@chetan-rns:
Overview of work required
The high level overview of the work for this issue is this:
- Add the
templateUntyped
field to ApplicationSet spec- This is like
template
, but represented in the CRD as aapiextensionsv1.JSON
(and thus can theoretically contain any JSON value, but SHOULD still contain the same fields astemplate
, BUT with support for JSON parameters)
- This is like
- Allow generators to return parameters that are JSON objects (rather than only supporting string parameters)
- A generator can EITHER support only string parameters (this is the current behaviour for all generators), OR it can support BOTH string parameters and JSON parameters.
- For this issue, we will only add JSON support to one generator.
- Update the generator interface to allow generators to indicate that they support returning JSON objects.
- A generator should return true for
GeneratorSupportsJsonObjectParamValues()
if it supports JSON objects, false otherwise.
- A generator should return true for
- Update the Git file generator to add support for generating JSON parameters
- As part of this issue will we only be added support to the Git file generator.
- We can use this generator as our test tube for this new functionality, making it our first generator to support both JSON/string values. Hopefully we can catch any initial issues here.
- Update the logic that calls generators, to use this new feature
- Add new tests
Update the generator interface, to allow generators to declare their support for returning json object parameters
Update the Generator
interface:
- Currently it is:
-
GenerateParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, applicationSetInfo *argoprojiov1alpha1.ApplicationSet) ([]map[string]string, error)
-
- Add new functions:
- New function which returns a
[]map[string]interface{}
, rather than a[]map[string]string
:-
GenerateJSONParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, applicationSetInfo *argoprojiov1alpha1.ApplicationSet) ([]map[string]interface{}, error)
-
- New function which allows individiual generators to 'opt-in' to support for JSON parameters.
-
func GeneratorSupportsJsonObjectParamValues() bool
- If a generator supports Json Object parameters, it should return true, otherwise false.
-
- New function which returns a
For a generator, the GenerateJSONParams
function will only be called if the templateUntyped
field is non-empty, and the template
field is empty.
With the exception of the Git file generator, all existing generators should return false
for GeneratorSupportsJsonObjectParamValues
and GenerateJSONParams
should return an error if called.
Add support for these new JSON param functions to the Git file generator
In the Git file generator:
- The
GenerateParams
function will work as normal, and should require no changes. - In the
GenerateJSONParams
function, we will now parse JSON and/or YAML of files from Git, and return the full structure (rather than flattening that structure as we do forGenerateParams
:
For example, the following JSON file:
{
"aws_account": "123456",
"asset_id": "11223344",
"cluster": {
"owner": "[email protected]",
"name": "engineering-dev",
"address": "https://1.2.3.4"
},
}
would be converted into the following ApplicationSet parameters:
aws_account: "123456"
asset_id: "11223344"
cluster: "cluster": { "owner": "[email protected]", "name": "engineering-dev", "address": "https://1.2.3.4" }
(cluster
is a single parameter containing the full JSON object)
Note this is different from the existing GenerateParams
logic, which would create these (flattened) params:
aws_account: "123456"
asset_id: "11223344"
cluster.owner: "[email protected]"
cluster.name: "engineering-dev"
cluster.address: "https://1.2.3.4"
Update Transform
in generator_spec_processor.go
to support both generator modes
The line
params, err := g.GenerateParams(&requestedGenerator, appSet)
will now be
// If the ApplicationSet has the templateUntyped field defind, and the generator supports it, call the generator with json object support
if appSet.Spec.TemplateUntyped != nil && g.GeneratorSupportsJsonObjectParamValues() {
params, err = g.GenerateJSONParams(/* */)
} else {
params, err = g.GenerateParams(/* */)
}
Such that GenerateJSONParams
will now be called if the generator supports it, AND if the user requests it (by specifing a value in templateUntyped
).
Update the Application CR, and Reconcile
Add a new field to type ApplicationSetSpec struct { }
, templateUntyped
of type apiextensionsv1.JSON
In Reconcile (or one of the validation methods), detect if both template
and templateUntyped
are non-empty; if so, this is an error: either one or the other should be specified, but not both.
If template
field is specified in the ApplicationSet, then generateApplications
function in applicationset_controller.go
should work as normal. (this scenario doesn't change)
OTOH If templateUntyped
field is specified, do the following:
In generateApplications
, convert the templateUntyped to a string (JSON string).
Next, for each of the parameters generated (by the generator earlier in generateApplications
function), convert them to JSON as follows:
- parse as YAML; if succeeds, convert to JSON string
- parse as JSON; if succeeds, no-op
- otherwise, leave the parameter as a string
This should produce a []map[string]string, where the map's key is the parameter name, and the map's value is a valid JSON value (or just a normal string).
Create a new version of RenderTemplateParams
Create a new function, which is a new version of RenderTemplateParams
(see util.go) with these parameters (notice the first parameter is different):
func (r *Render) RenderTemplateParams(tmpl string, syncPolicy *argoprojiov1alpha1.ApplicationSetSyncPolicy, params map[string]string) (*argov1alpha1.Application, error) { /* ... */ }
Where tmpl
is the templateUntyped
field converted to a JSON string, and params are the JSON string parameters from above.
This new function should otherwise work like the old function, but this new function does not need to tmplBytes, err := json.Marshal(tmpl)
, it can instead just use the tmpl
string field.
The goal for this function is to render the JSON parameters (from params
) in the JSON template (from tmpl
), convert it back into an Argo CD Application object (to make sure the template is still a valid Application), then return it.
Call the new version of RenderTemplateParams
in generateApplications
Finally, call the function in generateApplications
:
for _, a := range t {
for _, p := range a.Params {
app, err := r.Renderer.RenderTemplateParams(/* templateUntyped variable converted to a string */, applicationSetInfo.Spec.SyncPolicy, p)
if err != nil {
log.WithError(err).WithField("params", a.Params).WithField("generator", requestedGenerator).
Error("error generating application from params")
if firstError == nil {
firstError = err
}
continue
}
res = append(res, *app)
}
}
Finally...
After all this, you should now:
- Be able to specific an Application CR containing a
templateUntyped
field - The Git file generator should be able to process JSON/YAML files from Git repositories, and convert them into JSON objects (rather than strings)
- You should be able to insert those JSON values into ApplicationSet, via the
templateUntyped
field. - The existing behaviour of ApplicationSet controller is unchanged, such that we don't break backwards compatibility. The new behavior defined here will only be 'activated' if
templateUntyped
field is defined.
re: implementation, if and when you get this far, post a PR and let me play with it, as I want to make sure it has the desired characteristics we are looking for. (I don't want y'all to spend some time writing tests, only for us to discover that a major change is needed which invalidates those tests!). So when the above is implemented, before writing any tests, let me know and I'll check out your branch.
@jgwest Thanks for the detailed overview. I can work on this issue. Can you please assign it to me?
@chetan-rns have you made any progress on this? I'm looking at a related PR dealing with functions in templates. Depending on the design choices, that work may overlap a bit with this.
@crenshaw-dev I had started working on it a couple of weeks ago, but haven't reached far. So I'm okay if there will be any changes in the design choices. I will take a look at the PR.
That sounds like a nice feature! With the suggested implementation, would it also be possible to have something like an all-parameters
flag?
It would allow to expand all parameters into the template, without needing to know a "root parameter". In the initial example the "root parameter" was cluster.helmParameters
. I'd love, if it would be possible to remove that and directly injecting all generated parameters. When hjaving this possiblity, I could use the same config file in the generator for a helm Chart as well directly as a values-file for a manually deployed Chart.
An example config file could look like this:
name: image.version
value: 1.1.1
other:
parameters:
- foo: bar
and then have an ApplicationSet using a template like this:
spec:
# (...)
template:
spec:
source:
# (...)
helm:
parameters: {{ ** }}
which should result in an Application like this:
spec:
# (...)
template:
spec:
source:
repoURL: https://github.com/argoproj/argocd-example-apps.git
targetRevision: HEAD
path: guestbook
helm:
parameters:
- name: image.version
value: 1.1.1
other:
parameters:
- foo: bar
@ffendt that should be possible with goTemplate: true
(a 2.5 feature) and toJson
from the sprig library.