[vm] Remove egress restrictions on non-isolated vm by lllamnyp · Pull Request #1602 · cozystack/cozystack

Timofei Larkin (lllamnyp) · 2025-11-05T12:12:23Z

What this PR does

Virtual machines with a public IP have a network policy that explicitly allows them to communicate with the external internet. When created in a non-isolated tenant that has no network policies by default, creating such a policy, instead, restricts egress traffic originating from the VM from going anywhere, except the outside internet. Instead of allowing VMs to communicate with the outside internet via a network policy, this patch now adds a label to the VMs by which they will be targetted by Cozystack's default network policies, if such policies exist. When no policies are present, the VM's egress traffic will no longer be policed. Resolves #1601.

Release note

[virtual-machine,vm-instance] Remove the egress network policy from
virtual machines and govern egress traffic by default network policies
instead, resolving the bug in #1601.

Summary by CodeRabbit

Chores
- Removed outbound-to-world egress rules from service network policies.
- Updated VM manifests to move external-communication metadata into labels and mark allow-world-egress.
New Features
- Added tenant-level network policies to selectively permit world egress and to allow specific control-plane egress paths between tenant and control namespaces.

coderabbitai · 2025-11-05T12:12:35Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Removed per-VM egress-to-world rules from service CiliumNetworkPolicy, moved/added policy labels on VM resources, and introduced tenant-scoped and clusterwide Cilium policies that conditionally allow world egress for labeled VMs and route tenant egress targets.

Changes

Cohort / File(s)	Summary
NetworkPolicy egress removal `packages/apps/virtual-machine/templates/service.yaml`, `packages/apps/vm-instance/templates/service.yaml`	Deleted the egress block that used `toEntities: world` from CiliumNetworkPolicy resources, removing the explicit egress-to-world-only rule.
VM metadata updates `packages/apps/virtual-machine/templates/vm.yaml`, `packages/apps/vm-instance/templates/vm.yaml`	Removed annotation `policy.cozystack.io/allow-external-communication: "false"` from metadata.annotations and added labels `policy.cozystack.io/allow-external-communication: "false"` and `policy.cozystack.io/allow-world-egress: "true"` under metadata.labels; existing kubevirt annotations remain.
Tenant network policies added `packages/apps/tenant/templates/networkpolicy.yaml`	Added a namespace-scoped CiliumNetworkPolicy `allow-world-egress` (selecting pods by `policy.cozystack.io/allow-world-egress`) to permit egress to `world`, and added a clusterwide CiliumClusterwideNetworkPolicy (`{{ include "tenant.name" . }}-egress`) with templated egress rules targeting tenant/release/etcd-related endpoints.

Sequence Diagram(s)

sequenceDiagram
  participant User as User/Controller
  participant K8s as Kubernetes API
  participant Cilium as Cilium (CNP/CCNP)
  participant VM as VirtualMachine Pod
  participant Cluster as Cluster DNS/Services

  rect `#f0f8ff`
    note left of K8s: Apply VM manifests & tenant policies
  end

  User->>K8s: create VM (labels: allow-world-egress=true)
  K8s->>Cilium: reconcile CNP/CCNP (tenant + clusterwide egress rules)
  K8s->>VM: create VM Pod with labels
  VM->>Cluster: DNS / in-cluster request
  alt VM selected by tenant/cluster policies
    Cilium-->>Cluster: allow intra-cluster endpoints per clusterwide/tenant rules
    Cluster-->>VM: response (allowed)
  else No matching allow rules
    Cilium-->>VM: egress to world only (if namespace-scoped allow-world applies)
    Cluster-->>VM: blocked (if not permitted)
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Review items needing extra attention:
- Ensure removal of per-service egress blocks does not leave unintended policy gaps for isolated tenants.
- Validate the tenant clusterwide policy templating (namespace selection, labels, and conditional branches like tenant-root/etcd paths).
- Confirm label/annotation changes align with admission controllers and any selectors used elsewhere.

Possibly related PRs

[tenant] Fix networkpolicy for accessing externalIPs from the cluster #854 — Modifies packages/apps/tenant/templates/networkpolicy.yaml; likely related to tenant egress rules and entity allowances.

Suggested reviewers

klinch0

Poem

🐰 I hopped through YAML, nibbling a rule,
shifted a label, then left things cool.
Tenant winds whisper, cluster bells chime,
DNS can chat and VMs can climb.
Hop, hop — policies aligned in time.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: removing egress restrictions on non-isolated VMs, which directly matches the primary objective in the PR.
Linked Issues check	✅ Passed	The PR fully addresses issue #1601 by removing the problematic explicit egress policy from VMs and adding labels to allow selection by default policies, ensuring non-isolated VMs access cluster resources.
Out of Scope Changes check	✅ Passed	All changes are within scope: removing egress blocks from VM service policies, relocating annotations to labels in vm.yaml files, and adding new network policies to support the new labeling approach.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/unrestrict-egress-on-vms

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60c1f15 and e997495.

📒 Files selected for processing (5)

packages/apps/tenant/templates/networkpolicy.yaml (1 hunks)
packages/apps/virtual-machine/templates/service.yaml (0 hunks)
packages/apps/virtual-machine/templates/vm.yaml (1 hunks)
packages/apps/vm-instance/templates/service.yaml (0 hunks)
packages/apps/vm-instance/templates/vm.yaml (1 hunks)

💤 Files with no reviewable changes (2)

packages/apps/virtual-machine/templates/service.yaml
packages/apps/vm-instance/templates/service.yaml

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

packages/apps/virtual-machine/templates/vm.yaml

🔇 Additional comments (2)

packages/apps/vm-instance/templates/vm.yaml (1)

28-33: Labels correctly placed in pod metadata for network policy selection.

The labels are now positioned in spec.template.metadata.labels (lines 31–32), which ensures they are propagated to the pod created by KubeVirt and can be properly selected by network policies. The annotation kubevirt.io/allow-pod-bridge-network-live-migration: "true" is correctly retained. The combination of allow-external-communication: "false" (to exclude VMs from the broad external-communication policy) and allow-world-egress: "true" (to include them in the new world-egress policy) is semantically sound and addresses the prior review feedback.

packages/apps/tenant/templates/networkpolicy.yaml (1)

37-50: New world-egress policy correctly scoped to isolated tenants.

The new allow-world-egress policy (lines 37–50) is appropriately nested within the if .Values.isolated block. It selects pods labeled with policy.cozystack.io/allow-world-egress: "true" and permits egress to world entities. This complements the VM labels from the vm-instance template and ensures:

Isolated tenants: VMs with the label can reach external endpoints via the new policy

Non-isolated tenants: No policies are created, so VMs have unrestricted egress (resolving the original issue where VMs were inadvertently restricted)

The policy design prevents ingress from world (only allows egress), maintaining appropriate security boundaries.

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

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

gemini-code-assist · 2025-11-05T12:12:46Z

Summary of Changes

Hello Timofei Larkin (@lllamnyp), I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the egress traffic management for virtual machines, particularly those with public IPs in non-isolated tenants. It moves away from a potentially restrictive explicit network policy to a more flexible label-based approach. This ensures that VMs can communicate as intended, either governed by Cozystack's default network policies or, in their absence, without unintended egress restrictions.

Highlights

Egress Policy Removal: The explicit egress network policy that allowed VMs with public IPs to communicate with the external internet has been removed from both virtual-machine and vm-instance service templates. This policy was inadvertently restricting other egress traffic in non-isolated tenants.
Label-Based Egress Management: Instead of an explicit egress policy, a new label policy.cozystack.io/allow-external-communication: "true" has been added to the metadata of VirtualMachine resources in both virtual-machine and vm-instance templates. This label allows Cozystack's default network policies to target these VMs for egress control.
Bug Resolution: This change resolves issue VM network policy bug for non-isolated tenants #1601, addressing the bug where VMs in non-isolated tenants were restricted to only external internet communication due to the previous network policy.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	Gemini (@gemini-code-assist)	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request correctly identifies the cause of unintended egress traffic restrictions on virtual machines and proposes a sound solution: removing the specific egress network policy and using a label for targeting by default policies. While the removal of the egress policy is implemented correctly, the new label policy.cozystack.io/allow-external-communication: "true" is applied to the VirtualMachine resource itself, instead of its pod template (spec.template). As network policies target pods, the pod running the VM will not receive this label, and the fix will not be effective. I have left critical comments on both vm.yaml files explaining how to correctly apply the label to the pod template to make the solution work as intended.

gemini-code-assist · 2025-11-05T12:14:18Z

 metadata:
  name: {{ include "virtual-machine.fullname" . }}
  labels:
+    policy.cozystack.io/allow-external-communication: "true"


This label is intended for network policies to target the VM's pod, but it's placed on the VirtualMachine resource's metadata. Labels from the VirtualMachine are not propagated to the pod created by KubeVirt. For the network policy to apply, this label must be on the pod, which means it should be in spec.template.metadata.labels.

Additionally, there's a conflicting annotation policy.cozystack.io/allow-external-communication: "false" on line 66 within spec.template.metadata.annotations that should be removed.

To fix this, please move the label and remove the old annotation. The spec.template.metadata section should be updated as follows:

metadata: annotations: kubevirt.io/allow-pod-bridge-network-live-migration: "true" labels: policy.cozystack.io/allow-external-communication: "true" {{- include "virtual-machine.labels" . | nindent 8 }}

gemini-code-assist · 2025-11-05T12:14:18Z

 metadata:
  name: {{ include "virtual-machine.fullname" . }}
  labels:
+    policy.cozystack.io/allow-external-communication: "true"


This label is intended for network policies to target the VM's pod, but it's placed on the VirtualMachine resource's metadata. Labels from the VirtualMachine are not propagated to the pod created by KubeVirt. For the network policy to apply, this label must be on the pod, which means it should be in spec.template.metadata.labels.

Additionally, there's a conflicting annotation policy.cozystack.io/allow-external-communication: "false" on line 30 within spec.template.metadata.annotations that should be removed.

To fix this, please move the label and remove the old annotation. The spec.template.metadata section should be updated as follows:

metadata: annotations: kubevirt.io/allow-pod-bridge-network-live-migration: "true" labels: policy.cozystack.io/allow-external-communication: "true" {{- include "virtual-machine.labels" . | nindent 8 }}

## What this PR does Virtual machines with a public IP have a network policy that explicitly allows them to communicate with the external internet. When created in a non-isolated tenant that has no network policies by default, creating such a policy, instead, **restricts** egress traffic originating from the VM from going anywhere, __except__ the outside internet. Instead of allowing VMs to communicate with the outside internet via a network policy, this patch now adds a label to the VMs by which they will be targetted by Cozystack's default network policies, if such policies exist. When no policies are present, the VM's egress traffic will no longer be policed. Resolves #1601. ### Release note ```release-note [virtual-machine,vm-instance] Remove the egress network policy from virtual machines and govern egress traffic by default network policies instead, resolving the bug in #1601. ``` Signed-off-by: Timofei Larkin <[email protected]>

Signed-off-by: Andrei Kvapil <[email protected]>  ## What this PR does Revert per-vm network policies functionality introduced by #1611 As it is not working as expected any way. This is temporary solution before implementing full-fledged security groups in Cozystack fixes #1601 alternative solution: #1602 ### Release note  ```release-note [virtual-machine] Revert per-vm network policies ```

Timofei Larkin (lllamnyp) requested review from Andrei Kvapil (kvaps) and Nikita (nbykov0) as code owners November 5, 2025 12:12

dosubot Bot added the size/XS This PR changes 0-9 lines, ignoring generated files label Nov 5, 2025

dosubot Bot added the kind/bug Categorizes issue or PR as related to a bug label Nov 5, 2025

gemini-code-assist Bot reviewed Nov 5, 2025

View reviewed changes

Timofei Larkin (lllamnyp) force-pushed the fix/unrestrict-egress-on-vms branch from e6df544 to 590337a Compare November 5, 2025 12:18

dosubot Bot added size/S This PR changes 10-29 lines, ignoring generated files and removed size/XS This PR changes 0-9 lines, ignoring generated files labels Nov 5, 2025

Timofei Larkin (lllamnyp) enabled auto-merge November 5, 2025 12:41

Timofei Larkin (lllamnyp) force-pushed the fix/unrestrict-egress-on-vms branch 2 times, most recently from 60c1f15 to 6622496 Compare November 7, 2025 10:17

Timofei Larkin (lllamnyp) force-pushed the fix/unrestrict-egress-on-vms branch from 6622496 to e997495 Compare November 7, 2025 10:18

Andrei Kvapil (kvaps) mentioned this pull request Nov 7, 2025

[virtual-machine] Revert per-vm network policies #1611

Merged

Timofei Larkin (lllamnyp) closed this Nov 7, 2025

auto-merge was automatically disabled November 7, 2025 10:46
Pull request was closed

Navigation Menu

Repository navigation

[vm] Remove egress restrictions on non-isolated vm - #1602

[vm] Remove egress restrictions on non-isolated vm#1602
Timofei Larkin (lllamnyp) wants to merge 1 commit into
mainfrom
fix/unrestrict-egress-on-vms

Timofei Larkin (lllamnyp) commented Nov 5, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 5, 2025 •

edited

Loading

Other AI code review bot(s) detected

Uh oh!

gemini-code-assist Bot commented Nov 5, 2025

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Nov 5, 2025

Uh oh!

gemini-code-assist Bot Nov 5, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Timofei Larkin (lllamnyp) commented Nov 5, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

gemini-code-assist Bot commented Nov 5, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Timofei Larkin (lllamnyp) commented Nov 5, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 5, 2025 •

edited

Loading