Skip to content

Commit 27b41e8

Browse files
committed
Treat all errors encountered while patching as fatal
This will stop the reconcile. No further patches, resources, or functions will be processed. This commit breaks required field paths, in that we'll return a fatal result if a required field path is missing. This means a set of patches that could have been eventually consistent now cannot. I intend to address that in a following commit. Signed-off-by: Nic Cope <nicc@rk0n.org>
1 parent e863479 commit 27b41e8

File tree

3 files changed

+15
-37
lines changed

3 files changed

+15
-37
lines changed

fn.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -192,17 +192,13 @@ func (f *Function) RunFunction(ctx context.Context, req *fnv1beta1.RunFunctionRe
192192
"name", ocd.Resource.GetName())
193193
}
194194

195-
errs, store := RenderComposedPatches(ocd.Resource, dcd.Resource, oxr.Resource, dxr.Resource, env, t.Patches)
196-
for _, err := range errs {
197-
response.Warning(rsp, errors.Wrapf(err, "cannot render patches for composed resource %q", t.Name))
198-
log.Info("Cannot render patches for composed resource", "warning", err)
199-
warnings++
195+
// TODO(negz): No need for multiple errors?
196+
if err := RenderComposedPatches(ocd.Resource, dcd.Resource, oxr.Resource, dxr.Resource, env, t.Patches); err != nil {
197+
response.Fatal(rsp, errors.Wrapf(err, "cannot render patches for composed resource %q", t.Name))
198+
return rsp, nil
200199
}
201200

202-
if store {
203-
// Add or replace our desired resource.
204-
desired[resource.Name(t.Name)] = dcd
205-
}
201+
desired[resource.Name(t.Name)] = dcd
206202
}
207203

208204
if err := response.SetDesiredCompositeResource(rsp, dxr); err != nil {

fn_test.go

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -406,18 +406,10 @@ func TestRunFunction(t *testing.T) {
406406
},
407407
{
408408
// This patch should return an error,
409-
// because the required path does not
410-
// exist.
409+
// because the path is not an array.
411410
Type: v1beta1.PatchTypeFromCompositeFieldPath,
412411
Patch: v1beta1.Patch{
413-
FromFieldPath: ptr.To[string]("spec.doesNotExist"),
414-
ToFieldPath: ptr.To[string]("spec.explode"),
415-
Policy: &v1beta1.PatchPolicy{
416-
FromFieldPath: func() *v1beta1.FromFieldPathPolicy {
417-
r := v1beta1.FromFieldPathPolicyRequired
418-
return &r
419-
}(),
420-
},
412+
FromFieldPath: ptr.To[string]("spec.widgets[0]"),
421413
},
422414
},
423415
},
@@ -458,11 +450,10 @@ func TestRunFunction(t *testing.T) {
458450
},
459451
Results: []*fnv1beta1.Result{
460452
{
461-
Severity: fnv1beta1.Severity_SEVERITY_WARNING,
462-
Message: fmt.Sprintf("cannot render patches for composed resource %q: cannot apply the %q patch at index 1: spec.doesNotExist: no such field", "cool-resource", "FromCompositeFieldPath"),
453+
Severity: fnv1beta1.Severity_SEVERITY_FATAL,
454+
Message: fmt.Sprintf("cannot render patches for composed resource %q: cannot apply the %q patch at index 1: spec.widgets: not an array", "cool-resource", "FromCompositeFieldPath"),
463455
},
464456
},
465-
Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
466457
},
467458
},
468459
},

render.go

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,7 @@ func RenderEnvironmentPatches(env *unstructured.Unstructured, oxr, dxr *composit
8585
// patches that are to or from the supplied composite resource and environment
8686
// in the order they were defined. Properly selecting the right source or
8787
// destination between observed and desired resources.
88-
func RenderComposedPatches( //nolint:gocyclo // just a switch
89-
ocd *composed.Unstructured,
90-
dcd *composed.Unstructured,
91-
oxr *composite.Unstructured,
92-
dxr *composite.Unstructured,
93-
env *unstructured.Unstructured,
94-
ps []v1beta1.ComposedPatch,
95-
) (errs []error, store bool) {
88+
func RenderComposedPatches(ocd, dcd *composed.Unstructured, oxr, dxr *composite.Unstructured, env *unstructured.Unstructured, ps []v1beta1.ComposedPatch) error { //nolint:gocyclo // just a switch
9689
for i, p := range ps {
9790
p := p
9891
switch t := p.GetType(); t {
@@ -112,7 +105,7 @@ func RenderComposedPatches( //nolint:gocyclo // just a switch
112105
continue
113106
}
114107
if err := ApplyToObjects(&p, dxr, ocd); err != nil {
115-
errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i))
108+
return errors.Wrapf(err, errFmtPatch, t, i)
116109
}
117110
case v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment:
118111
// TODO(negz): Same as above, but for the Environment. What does it
@@ -125,7 +118,7 @@ func RenderComposedPatches( //nolint:gocyclo // just a switch
125118
continue
126119
}
127120
if err := ApplyToObjects(&p, env, ocd); err != nil {
128-
errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i))
121+
return errors.Wrapf(err, errFmtPatch, t, i)
129122
}
130123
// If either of the below renderings return an error, most likely a
131124
// required FromComposite or FromEnvironment patch failed. A required
@@ -135,17 +128,15 @@ func RenderComposedPatches( //nolint:gocyclo // just a switch
135128
// resource to our accumulated desired state.
136129
case v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite:
137130
if err := ApplyToObjects(&p, oxr, dcd); err != nil {
138-
errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i))
139-
return errs, false
131+
return errors.Wrapf(err, errFmtPatch, t, i)
140132
}
141133
case v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment:
142134
if err := ApplyToObjects(&p, env, dcd); err != nil {
143-
errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i))
144-
return errs, false
135+
return errors.Wrapf(err, errFmtPatch, t, i)
145136
}
146137
case v1beta1.PatchTypePatchSet:
147138
// Already resolved - nothing to do.
148139
}
149140
}
150-
return errs, true
141+
return nil
151142
}

0 commit comments

Comments
 (0)