Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nkzk wants to merge 7 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f9aba35
fix(render): do not override function docker-network annotation
nkzk Jun 4, 2026
70f6cdd
fix: if AnnotationKeyRuntimeDockerNetwork is set, start crossplane co…
nkzk Jun 4, 2026
556ef50
test: update newEngineFunc signature
nkzk Jun 4, 2026
9ed0203
fix: remove network from engineflage, use dockerRenderEngine
nkzk Jun 4, 2026
396a2d1
fix: move cleanup to the empty network branch
nkzk Jun 4, 2026
f30628a
refactor: move common annotation logic to render/annotation.go
nkzk Jun 9, 2026
3eeb462
chore: specify that injectNetworkAnnotation does not overwrite annota…
nkzk Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions cmd/crossplane/render/annotation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package render

import "strings"

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
}
8 changes: 5 additions & 3 deletions cmd/crossplane/render/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ type EngineFlags struct {

// NewEngineFromFlags creates an Engine from the flag configuration. If a binary
// path is set, it returns a local engine. Otherwise it returns a Docker engine
// using the resolved image reference.
func NewEngineFromFlags(f *EngineFlags, log logging.Logger) Engine {
// using the resolved image reference. The network parameter sets the Docker
// network the render container should join; it is derived from function
// annotations (AnnotationKeyRuntimeDockerNetwork) by the caller.
func NewEngineFromFlags(f *EngineFlags, network string, log logging.Logger) Engine {

@adamwg adamwg Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to introduce a new string argument here. Two possible options:

  1. Add DockerNetwork to EngineFlags. This would let users specify --docker-network on the command-line, which seems like it could be useful, but Run can still infer it from function annotations by default.
  2. Introduce functional arguments (EngineWithNework(...)) here, so that we have a place to put all the possible optional things. WithLogger could become one of these, simplifying the signature futher.

I think I lean toward option 1 since it has the side-effect of letting users specify the docker network name as a flag.

if f.CrossplaneBinary != "" {
return &localRenderEngine{BinaryPath: f.CrossplaneBinary}
}

return &dockerRenderEngine{image: crossplaneImageFromFlags(f), log: log}
return &dockerRenderEngine{image: crossplaneImageFromFlags(f), network: network, log: log}
}

func crossplaneImageFromFlags(f *EngineFlags) string {
Expand Down
30 changes: 19 additions & 11 deletions cmd/crossplane/render/engine_docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -83,19 +82,28 @@ 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)
if err != nil {
return func() {}, errors.Wrap(err, "cannot create Docker network for rendering")
}
var networkID, networkName string

if e.network == "" {

@adamwg adamwg Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will also fix #96 :-).

Style nit: prefer to return early and keep the larger logic outdented, so:

if e.network != "" {
	return func() {}, nil
}
// Network creation logic goes here.

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)

e.network = networkName
injectNetworkAnnotation(fns, networkName)
cleanup := func() { //nolint:contextcheck // Detached context for cleanup.
_ = removeRenderNetwork(context.Background(), networkID)
}

cleanup := func() { //nolint:contextcheck // Detached context for cleanup.
_ = removeRenderNetwork(context.Background(), networkID)
return cleanup, nil
}

return cleanup, nil
// e.network was pre-configured by the caller (e.g. from a function
// annotation). We don't own the network, so there is nothing to clean up.
return func() {}, nil
}

// Render marshals the request, runs it through a Docker container, and returns
Expand Down
10 changes: 8 additions & 2 deletions cmd/crossplane/render/op/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ type Cmd struct {
fs afero.Fs

// newEngine constructs the render Engine.
newEngine func(*render.EngineFlags, logging.Logger) render.Engine
newEngine func(*render.EngineFlags, string, logging.Logger) render.Engine
}

// Help prints out the help for the alpha render op command.
Expand Down Expand Up @@ -167,7 +167,13 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte
}
}

engine := c.newEngine(&c.EngineFlags, log)
var network string
annotations := render.NewAnnotationsFromStrings(c.FunctionAnnotations)
if value, ok := annotations[render.AnnotationKeyRuntimeDockerNetwork]; ok {
network = value
}

engine := c.newEngine(&c.EngineFlags, network, log)

seedCtx := len(c.ContextValues) > 0 || len(c.ContextFiles) > 0
captureCtx := c.IncludeContext
Expand Down
4 changes: 2 additions & 2 deletions cmd/crossplane/render/op/cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ var includeFunctionResultsOutput string
//go:embed testdata/cmd/output/include-full-operation.yaml
var includeFullOperationOutput string

func newEngineFunc(engine render.Engine) func(*render.EngineFlags, logging.Logger) render.Engine {
return func(*render.EngineFlags, logging.Logger) render.Engine {
func newEngineFunc(engine render.Engine) func(*render.EngineFlags, string, logging.Logger) render.Engine {
return func(*render.EngineFlags, string, logging.Logger) render.Engine {
return engine
}
}
Expand Down
9 changes: 7 additions & 2 deletions cmd/crossplane/render/render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
10 changes: 8 additions & 2 deletions cmd/crossplane/render/xr/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ type Cmd struct {
fs afero.Fs

// newEngine constructs the render Engine.
newEngine func(*render.EngineFlags, logging.Logger) render.Engine
newEngine func(*render.EngineFlags, string, logging.Logger) render.Engine
}

// Help prints out the help for the render command.
Expand Down Expand Up @@ -224,7 +224,13 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte
}
}

engine := c.newEngine(&c.EngineFlags, log)
var network string
annotations := render.NewAnnotationsFromStrings(c.FunctionAnnotations)

@adamwg adamwg Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also consider network annotations on the functions in the functions file? c.FunctionAnnotations would override those, but it seems like it would be nice to have render work by default when all functions are already annotated with the same network name.

if value, ok := annotations[render.AnnotationKeyRuntimeDockerNetwork]; ok {
network = value
}

engine := c.newEngine(&c.EngineFlags, network, log)

seedCtx := len(c.ContextValues) > 0 || len(c.ContextFiles) > 0
captureCtx := c.IncludeContext
Expand Down
4 changes: 2 additions & 2 deletions cmd/crossplane/render/xr/cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ var includeFunctionResultsOutput string
//go:embed testdata/cmd/output/include-full-xr.yaml
var includeFullXROutput string

func newEngineFunc(engine render.Engine) func(*render.EngineFlags, logging.Logger) render.Engine {
return func(*render.EngineFlags, logging.Logger) render.Engine {
func newEngineFunc(engine render.Engine) func(*render.EngineFlags, string, logging.Logger) render.Engine {
return func(*render.EngineFlags, string, logging.Logger) render.Engine {
return engine
}
}
Expand Down
Loading