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 30 minutes and 35 seconds. Learn how PR review limits work.

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

⌛ 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: d167185e-d7c9-4830-ad2c-f5085e7c7111

📥 Commits

Reviewing files that changed from the base of the PR and between a2deb33 and ce75dc1.

📒 Files selected for processing (3)

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

📝 Walkthrough

Walkthrough

This PR extends the render engine to accept an optional preconfigured Docker network, avoiding automatic temporary network creation and annotation overwrites. Commands now parse function and CLI annotations to populate the network parameter, enabling callers to reuse networks across renders.

Changes

Docker network preconfiguration support

Layer / File(s)	Summary
Annotation parsing utility cmd/crossplane/render/annotation.go	New Annotations map type and NewAnnotationsFromStrings function parse key=value strings from CLI or function metadata, skipping malformed entries.
Engine network configuration and conditional setup cmd/crossplane/render/engine.go, cmd/crossplane/render/engine_docker.go	EngineFlags.CrossplaneDockerNetwork parameter threads through NewEngineFromFlags to dockerRenderEngine. dockerRenderEngine.Setup conditionally creates a temporary Docker network only when e.network is empty; when preconfigured, it returns a no-op cleanup.
Network annotation preservation during render cmd/crossplane/render/render.go	injectNetworkAnnotation now checks for existing AnnotationKeyRuntimeDockerNetwork annotations before setting them, preserving caller-provided or preexisting network values.
Op and xr command annotation parsing and wiring cmd/crossplane/render/op/cmd.go, cmd/crossplane/render/xr/cmd.go	Both commands parse function annotations during Run to extract preconfigured networks and apply them to EngineFlags.CrossplaneDockerNetwork, with optional CLI overrides via --function-annotations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Issue #75 (linked): This PR directly addresses the bug where function Docker network annotations are overwritten and temporary networks are always created. The changes implement Option A: allowing callers to preconfigure the network so the engine skips temporary network creation and preserves existing annotations.

Suggested reviewers

• tampakrap
• jcogilvie

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title exceeds the 72-character limit at 104 characters; however, it is descriptive and clearly relates to the main changes: preventing docker network annotation overwrites and running crossplane-container in the specified network.	Consider shortening the title to fit within 72 characters while preserving the key message, such as: 'fix(render): preserve docker network and run container in same network'

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description clearly relates to the changeset by explaining what problems are being fixed and referencing the linked issue; it includes proper checklists and contribution process acknowledgments.
Linked Issues check	✅ Passed	All coding objectives from issue #75 are met: the PR preserves existing docker-network annotations [render.go], allows the crossplane container to run in a specified network [engine.go, engine_docker.go], and supports network configuration via both function annotations and CLI flags [cmd.go, xr/cmd.go, annotation.go].
Out of Scope Changes check	✅ Passed	All changes are directly related to addressing the linked issue: adding network configuration support, preventing annotation overwrites, and enabling the crossplane container to run in a specified network.
Breaking Changes	✅ Passed	All changes in cmd/** are backward compatible: optional field added to EngineFlags with zero value, and new exported types/functions added. No public APIs removed, renamed, or made required.
Feature Gate Requirement	✅ Passed	PR modifies only CLI code (cmd/crossplane/render/), not apis/**. Changes are bug fixes for annotation overwriting and network configuration, not experimental features requiring feature gates.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

cmd/crossplane/render/engine.go (1)

67-71: ⚡ Quick win

Update stale constructor docs after signature change.

Could you update this comment? On Line 69 it still mentions a network parameter, but NewEngineFromFlags now derives this from EngineFlags.CrossplaneDockerNetwork.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/crossplane/render/engine.go` around lines 67 - 71, Update the doc comment
for NewEngineFromFlags to remove the outdated reference to a `network parameter`
and instead state that the Docker network is derived from
EngineFlags.CrossplaneDockerNetwork; specifically edit the comment block above
the NewEngineFromFlags function to reflect that when no binary path is set it
returns a Docker engine using the resolved image reference and that the Docker
network is taken from EngineFlags.CrossplaneDockerNetwork (not supplied by the
caller).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/crossplane/render/op/cmd.go`:
- Around line 170-185: The override parsing for
render.AnnotationKeyRuntimeDockerNetwork is nested inside the if
c.EngineFlags.CrossplaneDockerNetwork == "" block so the --function-annotations
override never applies when a network is already set; move the block that parses
c.FunctionAnnotations (using render.NewAnnotationsFromStrings and checking
render.AnnotationKeyRuntimeDockerNetwork) out of that conditional and always run
it so that when an annotation value exists you set
c.EngineFlags.CrossplaneDockerNetwork (and/or c.CrossplaneDockerNetwork if used
elsewhere) to that value, ensuring the function-annotations override takes
precedence.

---

Nitpick comments:
In `@cmd/crossplane/render/engine.go`:
- Around line 67-71: Update the doc comment for NewEngineFromFlags to remove the
outdated reference to a `network parameter` and instead state that the Docker
network is derived from EngineFlags.CrossplaneDockerNetwork; specifically edit
the comment block above the NewEngineFromFlags function to reflect that when no
binary path is set it returns a Docker engine using the resolved image reference
and that the Docker network is taken from EngineFlags.CrossplaneDockerNetwork
(not supplied by the caller).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 14a09b90-8615-4a74-831b-924cd8db6271

📥 Commits

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

📒 Files selected for processing (6)

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

🚧 Files skipped from review as they are similar to previous changes (2)

• cmd/crossplane/render/engine_docker.go
• cmd/crossplane/render/render.go