Network: default egress policy Allow for Isolated networks on fresh installations - #13684
Network: default egress policy Allow for Isolated networks on fresh installations#13684andrijapanicsb wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13684 +/- ##
============================================
+ Coverage 19.65% 19.69% +0.03%
- Complexity 19792 19801 +9
============================================
Files 6368 6368
Lines 574881 574886 +5
Branches 70351 70351
============================================
+ Hits 112970 113200 +230
+ Misses 449639 449414 -225
Partials 12272 12272
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package kvm |
|
@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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18655 |
…nstallations The built-in Isolated network offerings were seeded with egress_default_policy = false (Deny), while: - the createNetworkOffering API already defaults egressdefaultpolicy to true (Allow) when the parameter is omitted (NetworkOfferingBaseCmd), - the UI's Add Network Offering form contradicted that API default by preselecting Deny and sending an explicit egressdefaultpolicy=false, - the Kubernetes service rejects isolated offerings with egress Deny and creates its own default offering with egress Allow, - VPC tiers have no such baked-in Deny: allow-all (default_allow ACL) is a first-class choice. Seed both built-in Isolated offerings with egress Allow on fresh installations, and align the UI form default with the existing API default. For DefaultIsolatedNetworkOffering (no Firewall service) the flag is inert and set only so API responses do not advertise a misleading Deny policy. No global setting is introduced: per-offering configurability already exists via the egressdefaultpolicy parameter, and a setting consumed once at first-boot seeding but live for later createNetworkOffering calls would have inconsistent lifecycle semantics. Backward compatibility: createDefaultNetworkOfferings() only runs on first boot (guarded by the 'init' configuration flag) and persistDefaultNetworkOffering() is find-or-create by unique name - it never updates an existing row. No upgrade SQL is shipped, deliberately: egress enforcement reads the offering row live on every VR rule programming, so flipping existing rows would change the behavior of existing networks. Upgraded clouds keep Deny on the pre-existing built-in offering; an Allow-by-default offering for new networks on upgraded clouds is left as a follow-up (new/versioned offering). Unit tests cover the fresh-install seeding values and the API default for omitted/explicit egressdefaultpolicy.
b768d9a to
0bd4dec
Compare
|
Reworked per review: dropped the global setting — it had inconsistent lifecycle semantics (read once at first-boot seeding, but live for every later createNetworkOffering call) and an Isolated-scoped name with API-wide effect. What the PR does now: both built-in Isolated offerings are seeded with egress Allow on fresh installations only, and the UI form default is aligned with the existing API default (the UI was sending an explicit deny, overriding it). Added unit tests for the seeding values and the API default. Upgrades remain untouched by construction — the seeding only runs on first boot and never updates an existing row, and no upgrade SQL is shipped on purpose. An Allow-by-default offering for new networks on upgraded clouds is a possible follow-up. |
|
@blueorangutan package kvm |
Problem
On a fresh CloudStack installation, the built-in
DefaultIsolatedNetworkOfferingWithSourceNatServiceoffering — theAvailability.Requiredoffering that auto-creates a user's first isolated network at VM deployment — is persisted withegress_default_policy = false(Deny). The very first thing almost every new user hits is "my VMs have no internet", and the offering's egress policy is immutable after creation (updateNetworkOfferingdoes not exposeegressdefaultpolicy), so it cannot be fixed without DB surgery or recreating offerings and networks.This Deny default is a historical artifact, inconsistent with the rest of the codebase:
NetworkOfferingBaseCmd.getEgressDefaultPolicy()returnstruewhen theegressdefaultpolicyparameter is omitted, so offerings created via API/cloudmonkey default to Allow. Only the seeded built-in offering and the UI form said Deny.egressdefaultpolicy=false, overriding the API default for every offering created through the UI.KubernetesClusterManagerImplrejects non-VPC offerings with egress Deny and creates its own default offering withegressdefaultpolicy = true; the shipped default isolated offering cannot be used for CKS today.default_allowACL) is a first-class choice.What this PR changes
Scope is deliberately fresh installations and new offerings only:
ConfigurationServerImpl.createDefaultNetworkOfferings(): both built-in Isolated offerings are now seeded withegress_default_policy = true(Allow). ForDefaultIsolatedNetworkOffering(no SourceNat, no Firewall service) the flag is not enforced anywhere; it is set for consistency so API responses don't advertise a misleading Deny policy.AddNetworkOffering.vue: the Egress default policy radio now preselects Allow, aligning the UI with the documented API default. The parameter is still sent explicitly when Deny is selected; admins retain the choice per offering.NetworkOfferingVO: adds the missingsetEgressDefaultPolicy(boolean)setter.ConfigurationServerImplTestverifies the seeded isolated offerings carry Allow (and that the shared offering seeding is unchanged); a newCreateNetworkOfferingCmdTestlocks in the API behavior — Allow when the parameter is omitted, explicit Allow/Deny honored.No global setting is introduced. Per-offering configurability already exists via the
egressdefaultpolicyparameter, and a global setting would have had confusing dual semantics: consumed once at first-boot seeding (before an operator can realistically set it, since the configuration row does not exist yet at that point) versus consumed live by every latercreateNetworkOfferingcall — the two could silently disagree. It would also have applied to Shared/L2/VPC offering creation despite its Isolated-scoped name.Backward compatibility — existing installations provably untouched
createDefaultNetworkOfferings()runs only on first boot, guarded by theinitconfiguration flag inpersistDefaultValues(). It never executes on an upgraded installation.NetworkOfferingDaoImpl.persistDefaultNetworkOffering()is find-or-create by unique name; it never updates an existing row.offering.isEgressDefaultPolicy()inNetworkOrchestrator/CommandSetupHelper) on every VR rule programming, so flipping existing rows would silently change the egress behavior of every existing isolated network — including networks created after the upgrade from the pre-existing built-in offering. On upgraded clouds, the built-in offering (and all networks using it, old and new) therefore keeps Deny. Giving upgraded clouds an Allow default for new networks without touching existing ones requires a new/versioned built-in offering and is left as an explicit follow-up.FW_EGRESS_RULESdefault ACCEPT, user rules inverted to DROP) is already exercised today by any admin-created Allow offering.How to test
listNetworkOfferings name=DefaultIsolatedNetworkOfferingWithSourceNatService→egressdefaultpolicy: true.iptables -S FW_EGRESS_RULESshows default ACCEPT; guest has outbound connectivity with no egress rules configured.select name, egress_default_policy from network_offerings where name like 'DefaultIsolated%';unchanged (0 for pre-existing rows).ConfigurationServerImplTest#testCreateDefaultNetworkOfferingsSeedsIsolatedOfferingsWithEgressAllow,CreateNetworkOfferingCmdTest.Alternatives considered
network.isolated.default.egress.policy.allow, earlier revision of this PR): dropped for the lifecycle/naming reasons above.