fix(render): do not overwrite function docker network if set, start crossplane-container in same network

cmd/crossplane/render/annotation.go

@@ -0,0 +1,24 @@
+package render
+
+import "strings"
+
+// Annotations is a type used to store annotations.
+type Annotations map[string]string
+
+// NewAnnotationsFromStrings parses an array of strings in the format "key=value" into a map.
+// Silently skips strings in incorrect format.
+func NewAnnotationsFromStrings(annotations []string) Annotations {
+	result := make(Annotations, 0)
+	for _, annotation := range annotations {
+		parts := strings.SplitN(annotation, "=", 2)
+
+		if len(parts) != 2 {
+			continue
+		}
+
+		key, value := parts[0], parts[1]
+		result[key] = value
+	}
+
+	return result
+}

cmd/crossplane/render/engine.go

@@ -58,9 +58,10 @@ type Engine interface {
 // EngineFlags contains flags for configuring the render engine. It is embedded
 // by render command structs to provide shared engine configuration.
 type EngineFlags struct {
-	CrossplaneVersion string `help:"Version of the Crossplane image to use for rendering. Defaults to the latest stable version." placeholder:"VERSION" xor:"crossplane-selector"`
-	CrossplaneImage   string `help:"Override the full Crossplane Docker image reference for rendering."                           placeholder:"IMAGE"   xor:"crossplane-selector"`
-	CrossplaneBinary  string `help:"Path to a local crossplane binary to use instead of Docker."                                  placeholder:"PATH"    type:"existingfile"       xor:"crossplane-selector"`
+	CrossplaneVersion       string `help:"Version of the Crossplane image to use for rendering. Defaults to the latest stable version." placeholder:"VERSION"   xor:"crossplane-selector"`
+	CrossplaneImage         string `help:"Override the full Crossplane Docker image reference for rendering."                           placeholder:"IMAGE"     xor:"crossplane-selector"`
+	CrossplaneBinary        string `help:"Path to a local crossplane binary to use instead of Docker."                                  placeholder:"PATH"      type:"existingfile"       xor:"crossplane-selector,crossplane-docker"`
+	CrossplaneDockerNetwork string `help:"The docker network to start the crossplane container in"                                      xor:"crossplane-docker"`
 }
 
 // NewEngineFromFlags creates an Engine from the flag configuration. If a binary
@@ -71,7 +72,7 @@ func NewEngineFromFlags(f *EngineFlags, log logging.Logger) Engine {
 		return &localRenderEngine{BinaryPath: f.CrossplaneBinary}
 	}
 
-	return &dockerRenderEngine{image: crossplaneImageFromFlags(f), log: log}
+	return &dockerRenderEngine{image: crossplaneImageFromFlags(f), network: f.CrossplaneDockerNetwork, log: log}
 }
 
 func crossplaneImageFromFlags(f *EngineFlags) string {

cmd/crossplane/render/engine_docker.go

@@ -53,8 +53,7 @@ func (realContainerRunner) Run(ctx context.Context, img string, opts ...docker.R
 type dockerRenderEngine struct {
 	// image is the Crossplane Docker image reference.
 	image string
-	// network is the Docker network to connect the container to. When set,
-	// the container joins this network so it can reach function containers.
+	// network is the Docker network to connect the container to.
 	network string
 
 	log logging.Logger
@@ -83,12 +82,20 @@ func (e *dockerRenderEngine) CheckContextSupport() error {
 // containers also join it. The returned cleanup function removes the
 // network.
 func (e *dockerRenderEngine) Setup(ctx context.Context, fns []pkgv1.Function) (func(), error) {
-	networkID, networkName, err := createRenderNetwork(ctx)
+	var networkID, networkName string
+
+	if e.network != "" {
+		// e.network was pre-configured, we don't own the network, so there is nothing to clean up.
+		return func() {}, nil
+	}
+
+	var err error
+	networkID, networkName, err = createRenderNetwork(ctx)
 	if err != nil {
 		return func() {}, errors.Wrap(err, "cannot create Docker network for rendering")
 	}
-
 	e.network = networkName
+
 	injectNetworkAnnotation(fns, networkName)
 
 	cleanup := func() { //nolint:contextcheck // Detached context for cleanup.

cmd/crossplane/render/network.go

@@ -25,9 +25,27 @@ import (
 
 	"github.com/crossplane/crossplane-runtime/v2/pkg/errors"
 
+	pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1"
+
 	"github.com/crossplane/cli/v2/internal/docker"
 )
 
+// SetDefaultCrossplaneDockerNetwork defaults the Crossplane render engine's
+// Docker network to the first function runtime Docker network annotation.
+// If network is already set, return with no changes.
+func (f *EngineFlags) SetDefaultCrossplaneDockerNetwork(fns []pkgv1.Function) {
+	if f.CrossplaneDockerNetwork != "" {
+		return
+	}
+
+	for _, fn := range fns {
+		if value, ok := fn.Annotations[AnnotationKeyRuntimeDockerNetwork]; ok && value != "" {
+			f.CrossplaneDockerNetwork = value
+			return
+		}
+	}
+}
+
 // createRenderNetwork creates a temporary Docker bridge network for render.
 // Function containers and the Crossplane render container join this network so
 // they can reach each other. Returns the network ID and name.

cmd/crossplane/render/network_test.go

@@ -0,0 +1,76 @@
+package render
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+
+	pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1"
+)
+
+func TestSetDefaultCrossplaneDockerNetwork(t *testing.T) {
+	type args struct {
+		flags     EngineFlags
+		functions []pkgv1.Function
+	}
+	type want struct {
+		flags EngineFlags
+	}
+
+	cases := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"ExplicitNetworkIsPreserved": {
+			reason: "An explicit --crossplane-docker-network value should not be overwritten by function annotations.",
+			args: args{
+				flags: EngineFlags{CrossplaneDockerNetwork: "explicit-network"},
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: "function-network"}),
+				},
+			},
+			want: want{
+				flags: EngineFlags{CrossplaneDockerNetwork: "explicit-network"},
+			},
+		},
+		"FirstFunctionAnnotationIsUsed": {
+			reason: "When no explicit network is set, the render engine should join the first function runtime Docker network.",
+			args: args{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{"example.org/other": "ignored"}),
+					functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: "first-network"}),
+					functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: "second-network"}),
+				},
+			},
+			want: want{
+				flags: EngineFlags{CrossplaneDockerNetwork: "first-network"},
+			},
+		},
+		"NoNetwork": {
+			reason: "The flags should remain unchanged when no function has a runtime Docker network annotation.",
+			args: args{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(nil),
+					functionWithAnnotations(map[string]string{"example.org/other": "ignored"}),
+				},
+			},
+			want: want{},
+		},
+		"NoFunctionsPreservesDefaultBehavior": {
+			reason: "No functions should leave CrossplaneDockerNetwork unset so engine setup can use its default temporary network behavior.",
+			want:   want{},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Log(tc.reason)
+
+			tc.args.flags.SetDefaultCrossplaneDockerNetwork(tc.args.functions)
+			if diff := cmp.Diff(tc.want.flags, tc.args.flags); diff != "" {
+				t.Errorf("SetDefaultCrossplaneDockerNetwork(...), -want, +got:\n%s", diff)
+			}
+		})
+	}
+}

cmd/crossplane/render/op/cmd.go

@@ -167,6 +167,8 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte
 		}
 	}
 
+	c.SetDefaultCrossplaneDockerNetwork(fns)
+
 	engine := c.newEngine(&c.EngineFlags, log)
 
 	seedCtx := len(c.ContextValues) > 0 || len(c.ContextFiles) > 0

cmd/crossplane/render/op/cmd_test.go

@@ -259,6 +259,29 @@ func TestCmdRun(t *testing.T) {
 			},
 			want: want{err: cmpopts.AnyError},
 		},
+		"FunctionAnnotationNetworkDefaultsEngineNetwork": {
+			reason: "A runtime Docker network supplied by --function-annotations should default the Crossplane engine Docker network before the engine is created.",
+			args: args{
+				cmd: Cmd{
+					Operation:           "operation.yaml",
+					Functions:           "functions.yaml",
+					FunctionAnnotations: []string{render.AnnotationKeyRuntimeDockerNetwork + "=override-network"},
+					Timeout:             time.Minute,
+					fs:                  newTestFS(nil),
+					newEngine: func(flags *render.EngineFlags, _ logging.Logger) render.Engine {
+						if diff := cmp.Diff("override-network", flags.CrossplaneDockerNetwork); diff != "" {
+							t.Errorf("CrossplaneDockerNetwork: -want, +got:\n%s", diff)
+						}
+						return &render.MockEngine{
+							MockSetup: func(_ context.Context, _ []pkgv1.Function) (func(), error) {
+								return func() {}, errors.New("setup blew up")
+							},
+						}
+					},
+				},
+			},
+			want: want{err: cmpopts.AnyError},
+		},
 		"LoadFunctionCredentialsError": {
 			reason: "Missing function credentials file should return a wrapped load error.",
 			args: args{

cmd/crossplane/render/render.go

@@ -151,14 +151,19 @@ func RewriteAddressesForDocker(fns []*renderv1alpha1.FunctionInput) []*renderv1a
 	return fns
 }
 
-// injectNetworkAnnotation sets the Docker network annotation on all functions
+// injectNetworkAnnotation sets the Docker network annotation
+// on all functions without existing runtime-docker-network annotation
 // so their containers join the specified network.
 func injectNetworkAnnotation(fns []pkgv1.Function, networkName string) {
 	for i := range fns {
 		if fns[i].Annotations == nil {
 			fns[i].Annotations = make(map[string]string)
 		}
-		fns[i].Annotations[AnnotationKeyRuntimeDockerNetwork] = networkName
+
+		_, ok := fns[i].Annotations[AnnotationKeyRuntimeDockerNetwork]
+		if !ok {
+			fns[i].Annotations[AnnotationKeyRuntimeDockerNetwork] = networkName
+		}
 	}
 }
 

cmd/crossplane/render/render_test.go

@@ -0,0 +1,93 @@
+package render
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1"
+)
+
+func TestOverrideFunctionAnnotations(t *testing.T) {
+	type args struct {
+		functions   []pkgv1.Function
+		annotations []string
+	}
+	type want struct {
+		functions []pkgv1.Function
+		err       error
+	}
+
+	cases := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"AnnotationsAreAppliedToAllFunctions": {
+			reason: "Function annotation flags are global overrides applied to every function before rendering.",
+			args: args{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{"example.org/existing": "value"}),
+					functionWithAnnotations(nil),
+				},
+				annotations: []string{"example.org/override=override-value"},
+			},
+			want: want{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{"example.org/existing": "value", "example.org/override": "override-value"}),
+					functionWithAnnotations(map[string]string{"example.org/override": "override-value"}),
+				},
+			},
+		},
+		"ExistingAnnotationIsOverridden": {
+			reason: "A function annotation flag should replace an existing annotation with the same key.",
+			args: args{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: "function-network"}),
+				},
+				annotations: []string{AnnotationKeyRuntimeDockerNetwork + "=override-network"},
+			},
+			want: want{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: "override-network"}),
+				},
+			},
+		},
+		"InvalidAnnotationReturnsError": {
+			reason: "Invalid function annotation flags should fail instead of being silently ignored.",
+			args: args{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: "function-network"}),
+				},
+				annotations: []string{"malformed"},
+			},
+			want: want{
+				functions: []pkgv1.Function{
+					functionWithAnnotations(map[string]string{AnnotationKeyRuntimeDockerNetwork: "function-network"}),
+				},
+				err: cmpopts.AnyError,
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Log(tc.reason)
+
+			err := OverrideFunctionAnnotations(tc.args.functions, tc.args.annotations)
+			if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" {
+				t.Errorf("OverrideFunctionAnnotations(...), -want, +got:\n%s", diff)
+			}
+
+			if diff := cmp.Diff(tc.want.functions, tc.args.functions); diff != "" {
+				t.Errorf("OverrideFunctionAnnotations(...), -want, +got:\n%s", diff)
+			}
+		})
+	}
+}
+
+func functionWithAnnotations(annotations map[string]string) pkgv1.Function {
+	return pkgv1.Function{ObjectMeta: metav1.ObjectMeta{Annotations: annotations}}
+}

cmd/crossplane/render/xr/cmd.go

@@ -220,6 +220,8 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte
 		}
 	}
 
+	c.SetDefaultCrossplaneDockerNetwork(fns)
+
 	engine := c.newEngine(&c.EngineFlags, log)
 
 	seedCtx := len(c.ContextValues) > 0 || len(c.ContextFiles) > 0

cmd/crossplane/render/xr/cmd_test.go

@@ -299,6 +299,30 @@ func TestCmdRun(t *testing.T) {
 			},
 			want: want{err: cmpopts.AnyError},
 		},
+		"FunctionAnnotationNetworkDefaultsEngineNetwork": {
+			reason: "A runtime Docker network supplied by --function-annotations should default the Crossplane engine Docker network before the engine is created.",
+			args: args{
+				cmd: Cmd{
+					CompositeResource:   "xr.yaml",
+					Composition:         "composition.yaml",
+					Functions:           "functions.yaml",
+					FunctionAnnotations: []string{render.AnnotationKeyRuntimeDockerNetwork + "=override-network"},
+					Timeout:             time.Minute,
+					fs:                  newTestFS(nil),
+					newEngine: func(flags *render.EngineFlags, _ logging.Logger) render.Engine {
+						if diff := cmp.Diff("override-network", flags.CrossplaneDockerNetwork); diff != "" {
+							t.Errorf("CrossplaneDockerNetwork: -want, +got:\n%s", diff)
+						}
+						return &render.MockEngine{
+							MockSetup: func(_ context.Context, _ []pkgv1.Function) (func(), error) {
+								return func() {}, errors.New("setup blew up")
+							},
+						}
+					},
+				},
+			},
+			want: want{err: cmpopts.AnyError},
+		},
 		"LoadXRDError": {
 			reason: "Missing XRD file should return a wrapped load error.",
 			args: args{