Have ComposedTemplates take patchsets and templates as args

This allows it to be used with both Compositions (in the webhook) and
CompositionRevisions (in the controller code).

Signed-off-by: Nic Cope <nicc@rk0n.org>
This commit is contained in:
Nic Cope 2023-04-13 14:40:48 -07:00
parent 234deec1a2
commit 5b49147836
5 changed files with 86 additions and 93 deletions

View File

@ -304,13 +304,11 @@ func CombineString(format string, vars []any) (any, error) {
return fmt.Sprintf(format, vars...), nil
}
// TODO(negz): Perhaps have this take PatchSets and ComposedTemplates as args?
// ComposedTemplates returns a revision's composed resource templates with any
// patchsets dereferenced.
func ComposedTemplates(cs v1.CompositionRevisionSpec) ([]v1.ComposedTemplate, error) {
// ComposedTemplates returns the supplied composed resource templates with any
// supplied patchsets dereferenced.
func ComposedTemplates(pss []v1.PatchSet, cts []v1.ComposedTemplate) ([]v1.ComposedTemplate, error) {
pn := make(map[string][]v1.Patch)
for _, s := range cs.PatchSets {
for _, s := range pss {
for _, p := range s.Patches {
if p.Type == v1.PatchTypePatchSet {
return nil, errors.New(errPatchSetType)
@ -319,8 +317,8 @@ func ComposedTemplates(cs v1.CompositionRevisionSpec) ([]v1.ComposedTemplate, er
pn[s.Name] = s.Patches
}
ct := make([]v1.ComposedTemplate, len(cs.Resources))
for i, r := range cs.Resources {
ct := make([]v1.ComposedTemplate, len(cts))
for i, r := range cts {
var po []v1.Patch
for _, p := range r.Patches {
if p.Type != v1.PatchTypePatchSet {

View File

@ -1182,7 +1182,8 @@ func TestComposedTemplates(t *testing.T) {
}
type args struct {
cs v1.CompositionRevisionSpec
pss []v1.PatchSet
cts []v1.ComposedTemplate
}
type want struct {
@ -1198,18 +1199,16 @@ func TestComposedTemplates(t *testing.T) {
"NoCompositionPatchSets": {
reason: "Patches defined on a composite resource should be applied correctly if no PatchSets are defined on the composition",
args: args{
cs: v1.CompositionRevisionSpec{
Resources: []v1.ComposedTemplate{
{
Patches: []v1.Patch{
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.name"),
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.namespace"),
},
cts: []v1.ComposedTemplate{
{
Patches: []v1.Patch{
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.name"),
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.namespace"),
},
},
},
@ -1235,16 +1234,14 @@ func TestComposedTemplates(t *testing.T) {
"UndefinedPatchSet": {
reason: "Should return error and not modify the patches field when referring to an undefined PatchSet",
args: args{
cs: v1.CompositionRevisionSpec{
Resources: []v1.ComposedTemplate{{
Patches: []v1.Patch{
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-1"),
},
cts: []v1.ComposedTemplate{{
Patches: []v1.Patch{
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-1"),
},
}},
},
},
}},
},
want: want{
err: errors.Errorf(errFmtUndefinedPatchSet, "patch-set-1"),
@ -1253,69 +1250,67 @@ func TestComposedTemplates(t *testing.T) {
"DefinedPatchSets": {
reason: "Should de-reference PatchSets defined on the Composition when referenced in a composed resource",
args: args{
cs: v1.CompositionRevisionSpec{
// PatchSets, existing patches and references
// should output in the correct order.
PatchSets: []v1.PatchSet{
{
Name: "patch-set-1",
Patches: []v1.Patch{
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.namespace"),
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("spec.parameters.test"),
},
// PatchSets, existing patches and references
// should output in the correct order.
pss: []v1.PatchSet{
{
Name: "patch-set-1",
Patches: []v1.Patch{
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.namespace"),
},
},
{
Name: "patch-set-2",
Patches: []v1.Patch{
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.annotations.patch-test-1"),
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.annotations.patch-test-2"),
Transforms: []v1.Transform{{
Type: v1.TransformTypeMap,
Map: &v1.MapTransform{
Pairs: map[string]extv1.JSON{
"k-1": asJSON("v-1"),
"k-2": asJSON("v-2"),
},
},
}},
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("spec.parameters.test"),
},
},
},
Resources: []v1.ComposedTemplate{
{
Patches: []v1.Patch{
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-2"),
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.name"),
},
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-1"),
},
{
Name: "patch-set-2",
Patches: []v1.Patch{
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.annotations.patch-test-1"),
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.annotations.patch-test-2"),
Transforms: []v1.Transform{{
Type: v1.TransformTypeMap,
Map: &v1.MapTransform{
Pairs: map[string]extv1.JSON{
"k-1": asJSON("v-1"),
"k-2": asJSON("v-2"),
},
},
}},
},
},
{
Patches: []v1.Patch{
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-1"),
},
},
},
cts: []v1.ComposedTemplate{
{
Patches: []v1.Patch{
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-2"),
},
{
Type: v1.PatchTypeFromCompositeFieldPath,
FromFieldPath: pointer.String("metadata.name"),
},
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-1"),
},
},
},
{
Patches: []v1.Patch{
{
Type: v1.PatchTypePatchSet,
PatchSetName: pointer.String("patch-set-1"),
},
},
},
@ -1376,7 +1371,7 @@ func TestComposedTemplates(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got, err := ComposedTemplates(tc.args.cs)
got, err := ComposedTemplates(tc.args.pss, tc.args.cts)
if diff := cmp.Diff(tc.want.ct, got); diff != "" {
t.Errorf("\n%s\nrs.ComposedTemplates(...): -want, +got:\n%s", tc.reason, diff)

View File

@ -170,8 +170,8 @@ func NewPTComposer(kube client.Client, o ...PTComposerOption) *PTComposer {
// Compose resources using the bases, patches, and transforms specified by the
// supplied Composition.
func (c *PTComposer) Compose(ctx context.Context, xr resource.Composite, req CompositionRequest) (CompositionResult, error) { //nolint:gocyclo // Breaking this up doesn't seem worth yet more layers of abstraction.
// Inline PatchSets from Composition Spec before composing resources.
ct, err := ComposedTemplates(req.Revision.Spec)
// Inline PatchSets before composing resources.
ct, err := ComposedTemplates(req.Revision.Spec.PatchSets, req.Revision.Spec.Resources)
if err != nil {
return CompositionResult{}, errors.Wrap(err, errInline)
}

View File

@ -519,8 +519,8 @@ func NewXRCDPatchAndTransformer(composite, composed Renderer) *XRCDPatchAndTrans
// PatchAndTransform updates the supplied composition state by running all
// patches and transforms within the CompositionRequest.
func (pt *XRCDPatchAndTransformer) PatchAndTransform(ctx context.Context, req CompositionRequest, s *PTFCompositionState) error {
// Inline PatchSets from Composition Spec before composing resources.
ct, err := ComposedTemplates(req.Revision.Spec)
// Inline PatchSets before composing resources.
ct, err := ComposedTemplates(req.Revision.Spec.PatchSets, req.Revision.Spec.Resources)
if err != nil {
return errors.Wrap(err, errInline)
}

View File

@ -48,7 +48,7 @@ const (
// validatePatchesWithSchemas validates the patches of a composition against the resources schemas.
func (v *Validator) validatePatchesWithSchemas(ctx context.Context, comp *v1.Composition) (errs field.ErrorList) {
// Let's first dereference patchSets
resources, err := composite.ComposedTemplates(comp.Spec)
resources, err := composite.ComposedTemplates(comp.Spec.PatchSets, comp.Spec.Resources)
if err != nil {
errs = append(errs, field.Invalid(field.NewPath("spec", "resources"), comp.Spec.Resources, err.Error()))
return errs