Merge pull request #122 from phisco/allow-only-env-patches

fix: allow using function only to patch to and from the environment

Author: negz

fn.go

@@ -115,6 +115,14 @@ func (f *Function) RunFunction(ctx context.Context, req *fnv1beta1.RunFunctionRe
 		log.Debug("Loaded Composition environment from Function context", "context-key", fncontext.KeyEnvironment)
 	}
 
+	// Patching code assumes that the environment has a GVK, as it uses
+	// runtime.DefaultUnstructuredConverter.FromUnstructured. This is a bit odd,
+	// but it's what we've done in the past. We'll set a default GVK here if one
+	// isn't set.
+	if env.GroupVersionKind().Empty() {
+		env.SetGroupVersionKind(internalEnvironmentGVK)
+	}
+
 	if input.Environment != nil {
 		// Run all patches that are from the (observed) XR to the environment or
 		// from the environment to the (desired) XR.

fn_test.go

@@ -12,7 +12,6 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/utils/ptr"
 
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
@@ -52,7 +51,7 @@ func TestRunFunction(t *testing.T) {
 					Results: []*fnv1beta1.Result{
 						{
 							Severity: fnv1beta1.Severity_SEVERITY_FATAL,
-							Message:  "invalid Function input: resources: Required value: resources is required",
+							Message:  "invalid Function input: resources: Required value: resources or environment patches are required",
 						},
 					},
 				},
@@ -95,7 +94,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -126,7 +125,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 			want: want{
@@ -145,7 +144,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -210,7 +209,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -282,7 +281,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -380,7 +379,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -436,7 +435,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -537,7 +536,7 @@ func TestRunFunction(t *testing.T) {
 							// Note "new-resource" doesn't appear here.
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 					Results: []*fnv1beta1.Result{
 						{
 							Severity: fnv1beta1.Severity_SEVERITY_WARNING,
@@ -652,7 +651,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -715,7 +714,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -785,7 +784,7 @@ func TestRunFunction(t *testing.T) {
 							},
 						},
 					},
-					Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}},
+					Context: contextWithEnvironment(nil),
 				},
 			},
 		},
@@ -1198,6 +1197,44 @@ func TestRunFunction(t *testing.T) {
 					Context: contextWithEnvironment(map[string]interface{}{
 						"widgets": "10",
 					})}}},
+		"OnlyEnvironmentPatchesIsAllowed": {
+			reason: "Having only environment patches should be allowed and work as expected.",
+			args: args{
+				req: &fnv1beta1.RunFunctionRequest{
+					Input: resource.MustStructObject(&v1beta1.Resources{
+						Environment: &v1beta1.Environment{
+							Patches: []v1beta1.EnvironmentPatch{
+								{
+									Type: v1beta1.PatchTypeFromCompositeFieldPath,
+									Patch: v1beta1.Patch{
+										FromFieldPath: ptr.To[string]("spec.widgets"),
+										ToFieldPath:   ptr.To[string]("envKey"),
+									},
+								},
+							},
+						},
+					}),
+					Observed: &fnv1beta1.State{
+						Composite: &fnv1beta1.Resource{
+							Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"XR","spec":{"widgets":"10"}}`),
+						},
+					},
+				},
+			},
+			want: want{
+				rsp: &fnv1beta1.RunFunctionResponse{
+					Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)},
+					Desired: &fnv1beta1.State{
+						Composite: &fnv1beta1.Resource{
+							Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"XR"}`),
+						},
+					},
+					Context: contextWithEnvironment(map[string]interface{}{
+						"envKey": "10",
+					}),
+				},
+			},
+		},
 	}
 
 	for name, tc := range cases {
@@ -1227,7 +1264,7 @@ func contextWithEnvironment(data map[string]interface{}) *structpb.Struct {
 		data = map[string]interface{}{}
 	}
 	u := unstructured.Unstructured{Object: data}
-	u.SetGroupVersionKind(schema.GroupVersionKind{Group: "internal.crossplane.io", Version: "v1alpha1", Kind: "Environment"})
+	u.SetGroupVersionKind(internalEnvironmentGVK)
 	d, err := structpb.NewStruct(u.UnstructuredContent())
 	if err != nil {
 		panic(err)

patches.go

@@ -7,6 +7,7 @@ import (
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/utils/ptr"
 
@@ -30,6 +31,10 @@ const (
 	errFmtInvalidPatchPolicy          = "invalid patch policy %s"
 )
 
+var (
+	internalEnvironmentGVK = schema.GroupVersionKind{Group: "internal.crossplane.io", Version: "v1alpha1", Kind: "Environment"}
+)
+
 // A PatchInterface is a patch that can be applied between resources.
 type PatchInterface interface {
 	GetType() v1beta1.PatchType

validate.go

@@ -39,8 +39,8 @@ func ValidateResources(r *v1beta1.Resources) *field.Error {
 			return err
 		}
 	}
-	if len(r.Resources) == 0 {
-		return field.Required(field.NewPath("resources"), "resources is required")
+	if len(r.Resources) == 0 && (r.Environment == nil || len(r.Environment.Patches) == 0) {
+		return field.Required(field.NewPath("resources"), "resources or environment patches are required")
 	}
 	for i, r := range r.Resources {
 		if err := ValidateComposedTemplate(r); err != nil {