Revert "Network: default egress policy Allow for Isolated networks on fresh installations" - #14116
Conversation
… fresh i…" This reverts commit e0f3006.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14116 +/- ##
============================================
- Coverage 19.83% 19.79% -0.04%
+ Complexity 20026 20021 -5
============================================
Files 6371 6371
Lines 575986 575981 -5
Branches 70526 70526
============================================
- Hits 114235 114035 -200
- Misses 449326 449510 +184
- Partials 12425 12436 +11
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:
|
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Reverts the prior change that defaulted isolated network offerings to egress “Allow”, restoring “Deny” defaults and removing the associated server-side seeding behavior and tests.
Changes:
- Updates UI defaults for
egressdefaultpolicyfromallowtodenywhen creating a network offering. - Removes server-side seeding of
egressDefaultPolicy=truefor built-in isolated offerings on first boot. - Deletes/adjusts related Java tests and removes
NetworkOfferingVO#setEgressDefaultPolicy.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/offering/AddNetworkOffering.vue | Restores default egress policy to deny in the UI’s offering template. |
| server/src/main/java/com/cloud/server/ConfigurationServerImpl.java | Stops explicitly seeding isolated default offerings with egress Allow. |
| server/src/test/java/com/cloud/server/ConfigurationServerImplTest.java | Removes the test that asserted seeded isolated offerings default to egress Allow. |
| engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java | Removes the setter for egressdefaultpolicy. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmdTest.java | Deletes tests asserting API default behavior for egressdefaultpolicy. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -346,10 +346,6 @@ public boolean isEgressDefaultPolicy() { | |||
| return egressdefaultpolicy; | |||
| } | |||
|
|
|||
| defaultIsolatedSourceNatEnabledNetworkOffering.setState(NetworkOffering.State.Enabled); | ||
| // Default egress policy is Allow on fresh installations, consistent with the | ||
| // createNetworkOffering API default (egressdefaultpolicy=true when not specified). | ||
| // Existing installations are not affected: this method only runs on first boot | ||
| // (guarded by the "init" configuration flag) and persistDefaultNetworkOffering() | ||
| // never updates an already existing offering. | ||
| defaultIsolatedSourceNatEnabledNetworkOffering.setEgressDefaultPolicy(true); | ||
| defaultIsolatedSourceNatEnabledNetworkOffering.setSupportsVmAutoScaling(true); | ||
| defaultIsolatedSourceNatEnabledNetworkOffering = _networkOfferingDao.persistDefaultNetworkOffering(defaultIsolatedSourceNatEnabledNetworkOffering); |
| conservemode: true, | ||
| availability: 'optional', | ||
| egressdefaultpolicy: 'allow', | ||
| egressdefaultpolicy: 'deny', |
|
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.69% |
| Branch coverage | 18.90% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
Reverts #13684