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

[nkzk]

Description of your changes

Closes #75

Fixes:

• Do not overwrite the docker-network annotation in functions if it has already been set
• If the docker-network annotation is passed to the FunctionAnnotations flag, run crossplane-container in it.

I have:

• Read and followed Crossplane's contribution process.
• Run ./nix.sh flake check to ensure this PR is ready for review.
• Added or updated unit tests.
• Linked a PR or a docs tracking issue to document this change.
• Added backport release-x.y labels to auto-backport this PR.

Need help with this checklist? See the cheat sheet.

[adamwg]

Thanks for the PR, @nkzk! Would you mind creating an issue for this as well, for discoverability and tracking? I haven't reviewed in detail yet, but the described fixes sound reasonable.

[nkzk]

I see that there are already tests for passing function annotations to the engine. I had copilot help me create unit tests for injectNetworkAnnotations. Also ran flake check.

[nkzk]

Hmm, i got it to work in a devcontainer with this fix, but the current implemention has some issues in CI. But i think this can be solved on the user-side.

One of our earliest approaches was to start up functions as service-containers before running multiple/different renders, and it worked because gitlab/github connects the job-container to the bridge-network used by service-containers.

But since crossplane render will start up crossplane in another temporary bridge network, it doesnt seem that this will continue to work. However, my theory is that the user can specify the docker-network in their CI-provider (gitlab/github), and then specify the the docker-network flag in the crossplane render command with the fix in this branch to solve this.

We have another workflow which uses rootless DinD/PinP, but kind of the same issue there.

I'll do some more testing soon.

But let me know if something i say sounds off :D

[nkzk]

Think this PR is ready for review, i did some more testing in CI and did not completely figure it out yet, but i think its just an issue of configuring the docker-network in CI and setting that value as the function-docker-network flag in the render-command.

A quality of life improvement for us would be if we can spin up the crossplane container ourselves and make render use it. If we could configure the crossplane-containerthe same way as functions, with the development annotation to manage the container lifecycle ourselves, it would just simplify this alot for us.

But maybe its out of scope for this PR, i'm not sure whats the best way to implement this would be. But open to work on it if someone has some ideas.

[coderabbitai]

Warning

Review limit reached

@nkzk, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 34 minutes and 44 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d1c4990b-90a4-417e-86f3-6ac7ac044379

📥 Commits

Reviewing files that changed from the base of the PR and between 396a2d1 and 3eeb462.

📒 Files selected for processing (4)

• cmd/crossplane/render/annotation.go
• cmd/crossplane/render/op/cmd.go
• cmd/crossplane/render/render.go
• cmd/crossplane/render/xr/cmd.go

📝 Walkthrough

Walkthrough

The PR enables the render engine to accept a preconfigured Docker network, allowing callers to reuse existing networks instead of creating temporary ones on each render invocation. Both render subcommands now parse function annotations to detect and apply preconfigured network settings.

Changes

Docker network preconfiguration support

Layer / File(s)	Summary
Engine core and Docker network setup cmd/crossplane/render/engine.go, cmd/crossplane/render/engine_docker.go	NewEngineFromFlags now accepts a network parameter and forwards it to dockerRenderEngine. The Setup method conditionally creates a temporary Docker network only when e.network is empty; when preconfigured, it skips network creation and returns a no-op cleanup function.
Annotation preservation during render cmd/crossplane/render/render.go	injectNetworkAnnotation now checks if AnnotationKeyRuntimeDockerNetwork is already present before setting it, preserving any caller-supplied network annotation instead of unconditionally overwriting it.
Render op command annotation parsing cmd/crossplane/render/op/cmd.go, cmd/crossplane/render/op/cmd_test.go	The Cmd.newEngine field signature is updated to accept a network parameter. Run now parses FunctionAnnotations entries in key=value format, extracts the render.AnnotationKeyRuntimeDockerNetwork value, validates the format, and passes it to the engine constructor. Test helper updated to match.
Render xr command annotation parsing cmd/crossplane/render/xr/cmd.go, cmd/crossplane/render/xr/cmd_test.go	The Cmd.newEngine field signature is updated to accept a network parameter. Run now parses FunctionAnnotations entries in key=value format, extracts the render.AnnotationKeyRuntimeDockerNetwork value, validates the format, and passes it to the engine constructor. Test helper updated to match.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• #75: Addresses the core concern about network overwriting and the inability to reuse networks across render invocations by allowing callers to preconfigure a network that will not be overwritten or destroyed. The annotation preservation logic ensures that caller-supplied network configurations are respected during render.

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Breaking Changes	❌ Error	NewEngineFromFlags exported function signature changed by adding a required network parameter, breaking existing callers. PR lacks a 'breaking-change' label.	Add the 'breaking-change' label to the PR to acknowledge the breaking API change in cmd/crossplane/render/engine.go.
Title check	⚠️ Warning	The title exceeds the 72-character limit at 104 characters, though it accurately describes the core changes: preventing docker-network annotation override and running crossplane container in the specified network.	Reduce the title to 72 characters or fewer while retaining the key information, for example: 'fix(render): respect docker-network annotation for functions and engine'

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is well-related to the changeset, clearly referencing issue #75 and explaining the two main fixes being implemented with appropriate context.
Linked Issues check	✅ Passed	The changes comprehensively address both objectives from issue #75: preventing override of function docker-network annotations and ensuring the crossplane container starts in the specified network via FunctionAnnotations flag.
Out of Scope Changes check	✅ Passed	All code changes are directly scoped to the linked issue requirements: signature updates to accept network parameter, conditional network creation logic, annotation preservation, and extraction from FunctionAnnotations flag.
Feature Gate Requirement	✅ Passed	All changes are in cmd/crossplane/render/ (internal CLI), not apis/** (public API). Changes are bug fixes restoring previous behavior, not new experimental features requiring feature flags.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[adamwg]

A quality of life improvement for us would be if we can spin up the crossplane container ourselves and make render use it. If we could configure the crossplane-containerthe same way as functions, with the development annotation to manage the container lifecycle ourselves, it would just simplify this alot for us.

But maybe its out of scope for this PR, i'm not sure whats the best way to implement this would be. But open to work on it if someone has some ideas.

@nkzk Good thought - I can see how this would be useful. It's a little tricky, since the crossplane container in render doesn't actually run a server, it's just a one-off command (crossplane internal render ...).

For your use-case, would it be easier to download a crossplane binary and use the --crossplane-binary render flag? In that mode, the functions need to be accessible to the host (like with the old crossplane render), but there's no assumptions about inter-container networking.

[adamwg]

Thanks for filing an issue for this, and for the fix. A few comments inline, but the overall approach looks good to me.

[adamwg]

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.

[adamwg]

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.

[adamwg]

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.