Skip to content

network: default egress policy 'allow' for Isolated networks (configurable)#13684

Open
andrijapanicsb wants to merge 1 commit into
apache:mainfrom
andrijapanicsb:isolated-network-egress-default-allow
Open

network: default egress policy 'allow' for Isolated networks (configurable)#13684
andrijapanicsb wants to merge 1 commit into
apache:mainfrom
andrijapanicsb:isolated-network-egress-default-allow

Conversation

@andrijapanicsb

Copy link
Copy Markdown
Contributor

Description

Isolated guest networks created from the built-in default network offerings
(DefaultIsolatedNetworkOfferingWithSourceNatService and
DefaultIsolatedNetworkOffering) currently deny all egress by default. This
comes from how they are seeded in ConfigurationServerImpl: they use a
NetworkOfferingVO constructor that does not set egressdefaultpolicy, so it
falls back to the Java primitive default (false = deny). The DB column default
is also false.

Meanwhile:

  • createNetworkOffering, when called without an explicit egressdefaultpolicy,
    already defaults to allow (NetworkOfferingBaseCmd#getEgressDefaultPolicy
    returned true when null).
  • VPC tiers, governed by NetworkACL, use the default "allow" ACL and permit
    egress out of the box.

So we ship two inconsistent defaults (built-in Isolated offering = deny,
API-created offering = allow), and the default "simple Isolated network"
experience is the more surprising of the two. Deny-by-default egress on a NAT'd
Isolated network provides little practical security value — the network has no
inbound reachability without explicit static-NAT/port-forwarding/LB + firewall
rules — but it regularly surprises operators whose freshly deployed VMs cannot
reach package mirrors, NTP, metadata, etc. until an allow-all egress rule is
added.

This PR makes allow the default for the built-in Isolated offerings and makes
the behaviour configurable.

Changes

  • New global setting network.isolated.default.egress.policy.allow (Boolean,
    default true).
  • The built-in default Isolated network offerings are seeded with the egress
    default policy taken from this setting.
  • createNetworkOffering without an explicit egressdefaultpolicy now follows
    the same setting instead of a hard-coded true, so the built-in and API paths
    agree.

Operators who prefer deny-by-default set the setting to false before first
initialization, or continue to create/use a custom offering with
egressdefaultpolicy = deny (that path is unchanged).

Backward compatibility

Deliberately scoped to be safe:

  • Existing deployments are untouched. The default offerings are seeded once,
    at first initialization; upgrades do not re-seed them. No data migration runs
    against existing network_offerings rows or existing networks, so every
    already-created offering and network keeps its current policy.
  • Only new deployments (and offerings created without the parameter) get
    allow-by-default.
  • The egressdefaultpolicy parameter and per-offering behaviour are unchanged;
    this only changes a default.
  • The egress_default_policy DB column default is intentionally not changed
    and no UPDATE is run against existing rows — the application always sets the
    value explicitly on insert, so the setting is the single control point.

The one real consideration is security posture on new clouds: an operator who
relied on the historical deny-by-default of the built-in offering now gets
allow-by-default. That is why it is an operator-overridable, documented setting
rather than a silent flip, and should be called out in the release notes.

A [DISCUSS] thread will be raised on dev@ so the community can weigh in on
making allow the shipped default vs. keeping deny and only adding the setting.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

How Has This Been Tested?

  • Modules api, engine/schema, and server compile cleanly with the change.
  • Manual verification of the seeded egress default policy on a fresh
    initialization and via listNetworkOfferings / listConfigurations is
    outstanding and will be added; happy to extend Marvin coverage for the new
    default if reviewers prefer.

Isolated guest networks created from the built-in default offerings
(DefaultIsolatedNetworkOfferingWithSourceNatService and
DefaultIsolatedNetworkOffering) previously denied all egress by default,
while createNetworkOffering without an explicit egressdefaultpolicy and
VPC tiers already allow egress. This aligns the built-in Isolated
offerings with allow-by-default and makes the behaviour configurable.

- Add global setting network.isolated.default.egress.policy.allow
  (default true).
- Seed the built-in default Isolated network offerings with the egress
  default policy taken from this setting.
- createNetworkOffering without an explicit egressdefaultpolicy now
  follows the same setting instead of a hard-coded allow.

Only affects fresh installations and offerings created without the
parameter; existing network offerings and networks are unchanged.
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 9.09091% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.64%. Comparing base (e1cf0f3) to head (b768d9a).

Files with missing lines Patch % Lines
...java/com/cloud/server/ConfigurationServerImpl.java 0.00% 5 Missing ⚠️
...in/java/com/cloud/offerings/NetworkOfferingVO.java 0.00% 3 Missing ⚠️
.../command/admin/network/NetworkOfferingBaseCmd.java 0.00% 1 Missing ⚠️
...ain/java/com/cloud/network/NetworkServiceImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #13684   +/-   ##
=========================================
  Coverage     19.64%   19.64%           
- Complexity    19796    19798    +2     
=========================================
  Files          6368     6368           
  Lines        575119   575128    +9     
  Branches      70382    70382           
=========================================
+ Hits         112994   113000    +6     
+ Misses       449839   449838    -1     
- Partials      12286    12290    +4     
Flag Coverage Δ
uitests 3.41% <ø> (ø)
unittests 20.92% <9.09%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@andrijapanicsb

Copy link
Copy Markdown
Contributor Author

@blueorangutan package kvm

@blueorangutan

Copy link
Copy Markdown

@andrijapanicsb a [SL] Jenkins job has been kicked to build packages. It will be bundled with kvm SystemVM template(s). I'll keep you posted as I make progress.

@andrijapanicsb
andrijapanicsb requested a review from Pearl1594 July 24, 2026 04:27
@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18655

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants