Skip to content

Commit 0bd4dec

Browse files
Network: default egress policy Allow for Isolated networks on fresh installations
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.
1 parent 4f11707 commit 0bd4dec

5 files changed

Lines changed: 103 additions & 1 deletion

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.cloudstack.api.command.admin.network;
19+
20+
import org.junit.Test;
21+
import org.junit.runner.RunWith;
22+
import org.mockito.junit.MockitoJUnitRunner;
23+
import org.springframework.test.util.ReflectionTestUtils;
24+
25+
import static org.junit.Assert.assertEquals;
26+
27+
@RunWith(MockitoJUnitRunner.class)
28+
public class CreateNetworkOfferingCmdTest {
29+
30+
@Test
31+
public void testEgressDefaultPolicyIsAllowWhenParameterOmitted() {
32+
CreateNetworkOfferingCmd cmd = new CreateNetworkOfferingCmd();
33+
assertEquals("createNetworkOffering must default to egress Allow when egressdefaultpolicy is not specified",
34+
Boolean.TRUE, cmd.getEgressDefaultPolicy());
35+
}
36+
37+
@Test
38+
public void testEgressDefaultPolicyExplicitDenyIsHonored() {
39+
CreateNetworkOfferingCmd cmd = new CreateNetworkOfferingCmd();
40+
ReflectionTestUtils.setField(cmd, "egressDefaultPolicy", Boolean.FALSE);
41+
assertEquals(Boolean.FALSE, cmd.getEgressDefaultPolicy());
42+
}
43+
44+
@Test
45+
public void testEgressDefaultPolicyExplicitAllowIsHonored() {
46+
CreateNetworkOfferingCmd cmd = new CreateNetworkOfferingCmd();
47+
ReflectionTestUtils.setField(cmd, "egressDefaultPolicy", Boolean.TRUE);
48+
assertEquals(Boolean.TRUE, cmd.getEgressDefaultPolicy());
49+
}
50+
}

engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ public boolean isEgressDefaultPolicy() {
346346
return egressdefaultpolicy;
347347
}
348348

349+
public void setEgressDefaultPolicy(boolean egressDefaultPolicy) {
350+
this.egressdefaultpolicy = egressDefaultPolicy;
351+
}
352+
349353
public NetworkOfferingVO(String name, String displayText, TrafficType trafficType, boolean systemOnly, boolean specifyVlan, Integer rateMbps,
350354
Integer multicastRateMbps, boolean isDefault, Availability availability, String tags, Network.GuestType guestType, boolean conserveMode,
351355
boolean specifyIpRanges, boolean isPersistent, boolean internalLb, boolean publicLb, boolean isForVpc) {

server/src/main/java/com/cloud/server/ConfigurationServerImpl.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,12 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
10671067
Network.GuestType.Isolated, true, false, false, false, true, false);
10681068

10691069
defaultIsolatedSourceNatEnabledNetworkOffering.setState(NetworkOffering.State.Enabled);
1070+
// Default egress policy is Allow on fresh installations, consistent with the
1071+
// createNetworkOffering API default (egressdefaultpolicy=true when not specified).
1072+
// Existing installations are not affected: this method only runs on first boot
1073+
// (guarded by the "init" configuration flag) and persistDefaultNetworkOffering()
1074+
// never updates an already existing offering.
1075+
defaultIsolatedSourceNatEnabledNetworkOffering.setEgressDefaultPolicy(true);
10701076
defaultIsolatedSourceNatEnabledNetworkOffering.setSupportsVmAutoScaling(true);
10711077
defaultIsolatedSourceNatEnabledNetworkOffering = _networkOfferingDao.persistDefaultNetworkOffering(defaultIsolatedSourceNatEnabledNetworkOffering);
10721078

@@ -1084,6 +1090,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
10841090
false, true, null, null, true, Availability.Optional, null, Network.GuestType.Isolated, true, true, false, false, false, false);
10851091

10861092
defaultIsolatedEnabledNetworkOffering.setState(NetworkOffering.State.Enabled);
1093+
// This offering carries no Firewall service, so the flag is not enforced anywhere;
1094+
// it is set for consistency so API responses do not advertise a misleading Deny policy.
1095+
defaultIsolatedEnabledNetworkOffering.setEgressDefaultPolicy(true);
10871096
defaultIsolatedEnabledNetworkOffering = _networkOfferingDao.persistDefaultNetworkOffering(defaultIsolatedEnabledNetworkOffering);
10881097

10891098
for (Service service : defaultIsolatedNetworkOfferingProviders.keySet()) {

server/src/test/java/com/cloud/server/ConfigurationServerImplTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import com.cloud.dc.dao.VlanDao;
2424
import com.cloud.domain.dao.DomainDao;
2525
import com.cloud.network.dao.NetworkDao;
26+
import com.cloud.offering.NetworkOffering;
27+
import com.cloud.offerings.NetworkOfferingVO;
2628
import com.cloud.offerings.dao.NetworkOfferingDao;
2729
import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
2830
import com.cloud.service.dao.ServiceOfferingDao;
@@ -35,12 +37,16 @@
3537
import org.junit.Assert;
3638
import org.junit.Test;
3739
import org.junit.runner.RunWith;
40+
import org.mockito.ArgumentCaptor;
3841
import org.mockito.InjectMocks;
3942
import org.mockito.Mock;
4043
import org.mockito.Mockito;
4144
import org.mockito.Spy;
4245
import org.mockito.junit.MockitoJUnitRunner;
4346

47+
import java.util.HashMap;
48+
import java.util.Map;
49+
4450
@RunWith(MockitoJUnitRunner.class)
4551
public class ConfigurationServerImplTest {
4652

@@ -122,4 +128,37 @@ public void testUpdateSystemvmPassword() {
122128
//teardown
123129
System.setProperty("user.name", realusername);
124130
}
131+
132+
@Test
133+
public void testCreateDefaultNetworkOfferingsSeedsIsolatedOfferingsWithEgressAllow() {
134+
Mockito.when(_networkOfferingDao.persistDefaultNetworkOffering(Mockito.any(NetworkOfferingVO.class)))
135+
.thenAnswer(invocation -> invocation.getArgument(0));
136+
137+
try (TransactionLegacy txn = TransactionLegacy.open("testCreateDefaultNetworkOfferings")) {
138+
configurationServer.createDefaultNetworkOfferings();
139+
}
140+
141+
ArgumentCaptor<NetworkOfferingVO> captor = ArgumentCaptor.forClass(NetworkOfferingVO.class);
142+
Mockito.verify(_networkOfferingDao, Mockito.atLeastOnce()).persistDefaultNetworkOffering(captor.capture());
143+
144+
Map<String, NetworkOfferingVO> offeringsByName = new HashMap<>();
145+
for (NetworkOfferingVO offering : captor.getAllValues()) {
146+
offeringsByName.putIfAbsent(offering.getUniqueName(), offering);
147+
}
148+
149+
NetworkOfferingVO isolatedSourceNatOffering = offeringsByName.get(NetworkOffering.DefaultIsolatedNetworkOfferingWithSourceNatService);
150+
Assert.assertNotNull(isolatedSourceNatOffering);
151+
Assert.assertTrue("Built-in Isolated source-NAT offering must be seeded with egress default policy Allow on fresh installations",
152+
isolatedSourceNatOffering.isEgressDefaultPolicy());
153+
154+
NetworkOfferingVO isolatedOffering = offeringsByName.get(NetworkOffering.DefaultIsolatedNetworkOffering);
155+
Assert.assertNotNull(isolatedOffering);
156+
Assert.assertTrue("Built-in Isolated (no source-NAT) offering must be seeded with egress default policy Allow on fresh installations",
157+
isolatedOffering.isEgressDefaultPolicy());
158+
159+
NetworkOfferingVO sharedOffering = offeringsByName.get(NetworkOffering.DefaultSharedNetworkOffering);
160+
Assert.assertNotNull(sharedOffering);
161+
Assert.assertFalse("Built-in Shared offering seeding must remain unchanged",
162+
sharedOffering.isEgressDefaultPolicy());
163+
}
125164
}

ui/src/views/offering/AddNetworkOffering.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ export default {
704704
isolation: 'dedicated',
705705
conservemode: true,
706706
availability: 'optional',
707-
egressdefaultpolicy: 'deny',
707+
egressdefaultpolicy: 'allow',
708708
ispublic: this.isPublic,
709709
nsxsupportlb: true,
710710
routingmode: 'static'

0 commit comments

Comments
 (0)