Add a new untyped template field, to support passing JSON/YAML objects into ApplicationSet template fields

[jgwest]

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 😄. 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.

[rumstead]

#323 just to relate them

[parasappafluke]

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.

[mksha]

can we also add some functions that will allow us to have defaults for the template var?

[mksha]

for example
{{ myvar.myproperty | default "test" }}

[mksha]

It will be very helpful, especially in the application set where we want to set the default chart version if not specified by user.

[mksha]

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?

[jgwest]

@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 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)
• 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.
• 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.

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 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"

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.

[chetan-rns]

@jgwest Thanks for the detailed overview. I can work on this issue. Can you please assign it to me?