Skip to content

Commit 06869e5

Browse files
committed
fix(environment): Validate against correct patch types
Using the correct patch types requires changing the order in which xr and env are passed to ApplyToObjects to the "correct" order (xr, env) instead of (env, xr). Here env takes over the role of the composed resource just like in `Apply`. Signed-off-by: Maximilian Blatt <maximilian.blatt-extern@deutschebahn.com>
1 parent 11be501 commit 06869e5

File tree

3 files changed

+21
-22
lines changed

3 files changed

+21
-22
lines changed

fn_test.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -658,9 +658,9 @@ func TestRunFunction(t *testing.T) {
658658
Environment: &v1beta1.Environment{
659659
Patches: []v1beta1.EnvironmentPatch{
660660
{
661-
Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
661+
Type: v1beta1.PatchTypeToCompositeFieldPath,
662662
Patch: v1beta1.Patch{
663-
FromFieldPath: ptr.To[string]("data.widgets"),
663+
FromFieldPath: ptr.To[string]("widgets"),
664664
ToFieldPath: ptr.To[string]("spec.watchers"),
665665
Transforms: []v1beta1.Transform{
666666
{
@@ -713,10 +713,10 @@ func TestRunFunction(t *testing.T) {
713713
Environment: &v1beta1.Environment{
714714
Patches: []v1beta1.EnvironmentPatch{
715715
{
716-
Type: v1beta1.PatchTypeToEnvironmentFieldPath,
716+
Type: v1beta1.PatchTypeFromCompositeFieldPath,
717717
Patch: v1beta1.Patch{
718718
FromFieldPath: ptr.To[string]("spec.watchers"),
719-
ToFieldPath: ptr.To[string]("data.widgets"),
719+
ToFieldPath: ptr.To[string]("widgets"),
720720
Transforms: []v1beta1.Transform{
721721
{
722722
Type: v1beta1.TransformTypeMath,
@@ -764,7 +764,7 @@ func TestRunFunction(t *testing.T) {
764764
Patches: []v1beta1.ComposedPatch{{
765765
Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
766766
Patch: v1beta1.Patch{
767-
FromFieldPath: ptr.To[string]("data.widgets"),
767+
FromFieldPath: ptr.To[string]("widgets"),
768768
ToFieldPath: ptr.To[string]("spec.watchers"),
769769
Transforms: []v1beta1.Transform{{
770770
Type: v1beta1.TransformTypeConvert,
@@ -816,7 +816,7 @@ func TestRunFunction(t *testing.T) {
816816
Patches: []v1beta1.ComposedPatch{{
817817
Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
818818
Patch: v1beta1.Patch{
819-
FromFieldPath: ptr.To[string]("data.widgets"),
819+
FromFieldPath: ptr.To[string]("widgets"),
820820
ToFieldPath: ptr.To[string]("spec.watchers"),
821821
Transforms: []v1beta1.Transform{{
822822
Type: v1beta1.TransformTypeConvert,
@@ -872,7 +872,7 @@ func TestRunFunction(t *testing.T) {
872872
Patches: []v1beta1.ComposedPatch{{
873873
Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
874874
Patch: v1beta1.Patch{
875-
FromFieldPath: ptr.To[string]("data.widgets"),
875+
FromFieldPath: ptr.To[string]("widgets"),
876876
ToFieldPath: ptr.To[string]("spec.watchers"),
877877
Transforms: []v1beta1.Transform{{
878878
Type: v1beta1.TransformTypeConvert,
@@ -937,18 +937,16 @@ func TestRunFunction(t *testing.T) {
937937
}
938938

939939
// Crossplane sends as context a fake resource:
940-
// { "apiVersion": "internal.crossplane.io/v1alpha1", "kind": "Environment", "data": {... the actual environment content ...} }
940+
// { "apiVersion": "internal.crossplane.io/v1alpha1", "kind": "Environment", ... the actual environment content ... }
941941
// See: https://github.com/crossplane/crossplane/blob/806f0d20d146f6f4f1735c5ec6a7dc78923814b3/internal/controller/apiextensions/composite/environment_fetcher.go#L85C1-L85C1
942942
// That's because the patching code expects a resource to be able to use
943943
// runtime.DefaultUnstructuredConverter.FromUnstructured to convert it back to
944-
// an object. This is also why all patches need to specify the full path from data.
944+
// an object.
945945
func contextWithEnvironment(data map[string]interface{}) *structpb.Struct {
946946
if data == nil {
947947
data = map[string]interface{}{}
948948
}
949-
u := unstructured.Unstructured{Object: map[string]interface{}{
950-
"data": data,
951-
}}
949+
u := unstructured.Unstructured{Object: data}
952950
u.SetGroupVersionKind(schema.GroupVersionKind{Group: "internal.crossplane.io", Version: "v1alpha1", Kind: "Environment"})
953951
d, err := structpb.NewStruct(u.UnstructuredContent())
954952
if err != nil {

render.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,15 @@ func RenderEnvironmentPatches(env *unstructured.Unstructured, oxr, dxr *composit
6666
for i, p := range ps {
6767
p := p
6868
switch p.GetType() {
69-
case v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment:
70-
if err := ApplyToObjects(&p, env, oxr); err != nil {
69+
case v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite:
70+
if err := ApplyToObjects(&p, oxr, env); err != nil {
7171
return errors.Wrapf(err, errFmtPatch, p.GetType(), i)
7272
}
73-
case v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment:
74-
if err := ApplyToObjects(&p, env, dxr); err != nil {
73+
case v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeCombineToComposite:
74+
if err := ApplyToObjects(&p, dxr, env); err != nil {
7575
return errors.Wrapf(err, errFmtPatch, p.GetType(), i)
7676
}
77-
case v1beta1.PatchTypePatchSet, v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite, v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeCombineToComposite:
77+
case v1beta1.PatchTypePatchSet, v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment, v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment:
7878
// nothing to do
7979
}
8080
}

validate.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,12 @@ func ValidateEnvironment(e *v1beta1.Environment) *field.Error {
9898
}
9999
for i, p := range e.Patches {
100100
p := p
101-
switch p.GetType() { //nolint:exhaustive // Intentionally targeting only environment patches.
102-
case v1beta1.PatchTypeCombineToEnvironment,
103-
v1beta1.PatchTypeCombineFromEnvironment,
104-
v1beta1.PatchTypeFromEnvironmentFieldPath,
105-
v1beta1.PatchTypeToEnvironmentFieldPath:
101+
switch p.GetType() { //nolint:exhaustive // Only target valid patches according the API spec
102+
case
103+
v1beta1.PatchTypeFromCompositeFieldPath,
104+
v1beta1.PatchTypeToCompositeFieldPath,
105+
v1beta1.PatchTypeCombineFromComposite,
106+
v1beta1.PatchTypeCombineToComposite:
106107
default:
107108
return field.Invalid(field.NewPath("patches").Index(i).Key("type"), p.GetType(), "invalid environment patch type")
108109
}

0 commit comments

Comments
 (0)