diff --git a/cmd/crossplane/render/annotation.go b/cmd/crossplane/render/annotation.go new file mode 100644 index 00000000..f88a55a7 --- /dev/null +++ b/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 +} diff --git a/cmd/crossplane/render/engine.go b/cmd/crossplane/render/engine.go index 1e08ce45..d8ebed0c 100644 --- a/cmd/crossplane/render/engine.go +++ b/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 { diff --git a/cmd/crossplane/render/engine_docker.go b/cmd/crossplane/render/engine_docker.go index b67cf54b..7815f138 100644 --- a/cmd/crossplane/render/engine_docker.go +++ b/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. diff --git a/cmd/crossplane/render/network.go b/cmd/crossplane/render/network.go index 1772f6e3..a0413018 100644 --- a/cmd/crossplane/render/network.go +++ b/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. diff --git a/cmd/crossplane/render/network_test.go b/cmd/crossplane/render/network_test.go new file mode 100644 index 00000000..fde13f0f --- /dev/null +++ b/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) + } + }) + } +} diff --git a/cmd/crossplane/render/op/cmd.go b/cmd/crossplane/render/op/cmd.go index 04e6bc93..032a1dcf 100644 --- a/cmd/crossplane/render/op/cmd.go +++ b/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 diff --git a/cmd/crossplane/render/op/cmd_test.go b/cmd/crossplane/render/op/cmd_test.go index f17ec0c7..c82b94a4 100644 --- a/cmd/crossplane/render/op/cmd_test.go +++ b/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{ diff --git a/cmd/crossplane/render/render.go b/cmd/crossplane/render/render.go index 5cc373f9..90d406ea 100644 --- a/cmd/crossplane/render/render.go +++ b/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 + } } } diff --git a/cmd/crossplane/render/render_test.go b/cmd/crossplane/render/render_test.go new file mode 100644 index 00000000..6d87ae2d --- /dev/null +++ b/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}} +} diff --git a/cmd/crossplane/render/xr/cmd.go b/cmd/crossplane/render/xr/cmd.go index b7543a6c..71cb51d6 100644 --- a/cmd/crossplane/render/xr/cmd.go +++ b/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 diff --git a/cmd/crossplane/render/xr/cmd_test.go b/cmd/crossplane/render/xr/cmd_test.go index d57cc14a..f7e8cadb 100644 --- a/cmd/crossplane/render/xr/cmd_test.go +++ b/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{