Open jgwest opened 3 years ago
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:
The high level overview of the work for this issue is this:
templateUntyped
field to ApplicationSet spec
template
, but represented in the CRD as a apiextensionsv1.JSON
(and thus can theoretically contain any JSON value, but SHOULD still contain the same fields as template
, BUT with support for JSON parameters)GeneratorSupportsJsonObjectParamValues()
if it supports JSON objects, false otherwise.Update the Generator
interface:
GenerateParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, applicationSetInfo *argoprojiov1alpha1.ApplicationSet) ([]map[string]string, error)
[]map[string]interface{}
, rather than a []map[string]string
:
GenerateJSONParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, applicationSetInfo *argoprojiov1alpha1.ApplicationSet) ([]map[string]interface{}, error)
func GeneratorSupportsJsonObjectParamValues() bool
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.
In the Git file generator:
GenerateParams
function will work as normal, and should require no changes.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 for GenerateParams
:For example, the following JSON file:
{
"aws_account": "123456",
"asset_id": "11223344",
"cluster": {
"owner": "cluster-admin@company.com",
"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": "cluster-admin@company.com", "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: "cluster-admin@company.com"
cluster.name: "engineering-dev"
cluster.address: "https://1.2.3.4"
Transform
in generator_spec_processor.go
to support both generator modesThe 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
).
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:
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:
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).
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.
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)
}
}
After all this, you should now:
templateUntyped
fieldtemplateUntyped
field.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.
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):
and then have an ApplicationSet with a template like this:
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:
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:which would look like this:
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 existingtemplate
field, and yet we will need to break that structure with this feature.