-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Sprig templating capability to transforms and CombineFromComposite #63
Conversation
Signed-off-by: Philip Clark <[email protected]>
Signed-off-by: Philip Clark <[email protected]>
Signed-off-by: Philip Clark <[email protected]>
Signed-off-by: Philip Clark <[email protected]>
@phclark I'm a little wary of mixing Go templating into the P&T function. Would it not be reasonable to expect folks to use https://github.com/crossplane-contrib/function-go-templating if they wanted this kind of expressiveness? |
@negz Sorry, I just noticed this reply. The use case for having Go templating as a Transform type vs defining the entire manifest as a Go template is that you often only need advanced templating capabilities (compared to Format) when defining a string field in a manifest. For example, converting an array parameter into a Json list of strings to be injected as IAM resources in a policy document. |
Whilst this does introduce more complexity, right now our teams are often spending hours trying to implement simple tasks using the existing small sets of transforms and trying to create massively complex regexps. The risks associated with these implementations are much greater, since the implementations are very brittle. A definite 👍 on this from here. go-templating is another option but it's a significant shift in how we write compositions when all we need is a very simple patch step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry folks, right now I am leaning toward not approving this.
I understand that this enables things you can't do today with P&T. However, the point of P&T was to be for simpler use cases. It intentionally does not do everything someone could need it to do.
My suggestion would be that if you want or need sprig templating, you would be better served by migrating to https://github.com/crossplane-contrib/function-go-templating/.
@@ -257,6 +258,11 @@ type StringTransform struct { | |||
// Extract a match from the input using a regular expression. | |||
// +optional | |||
Regexp *StringTransformRegexp `json:"regexp,omitempty"` | |||
|
|||
// Render a template using the input as data. | |||
// See https://pkg.go.dev/github.com/pschlump/sprig for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be Masterminds/sprig?
Coming back around... I regard the below as a "simple use case" that is trivial using this PR but not without it. - type: CombineFromComposite
toFieldPath: "spec.forProvider.projectId"
combine:
strategy: template
variables:
- fromFieldPath: metadata.name
variableName: name
- fromFieldPath: metadata.uid
variableName: uid
template:
template: '{{ .name }}-{{ .uid | sha1sum | trunc 8 }}' How would I approach this using the main branch version of This "need" is in a composition file that is 1200 lines long already, so just moving it into |
@phclark @withnale, I think the point is that with this many other features would become redundant, e.g. most other string transforms could be replaced with just a string transform of type |
You wouldn't. The goal of this function is not full expressiveness. The intent behind P&T is to cover the simple "I need to copy data X from resource Y to resource Z" case. It was forced to grow more expressive (and thus hard to reason about) than that because there was no alternative. There are now alternatives in the form of other functions, including one specifically dedicated to Go templating. We need to cap the complexity of this function somewhere. Introducing a nested templating language with its own set of functions (in addition to the "first-class" transforms we already support) seems like a reasonable place to draw the line. |
My position here hasn't changed. I appreciate the effort, but I'm going to close this PR. I think https://github.com/crossplane-contrib/function-go-templating/ is a great alternative to explore if you need this level of expressiveness. |
This add the ability to use sprig templates as a transform, or with CombineFromComposite instead of simple fmt. The benefit of this is similar to function-go-template, except that we can benefit from the power of templates within the patch-and-transform context.