Skip to content

server: check startip and startipv6 of shared network #10704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
weizhouapache merged 1 commit into from
Apr 16, 2025
Merged

server: check startip and startipv6 of shared network #10704

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions server/src/main/java/com/cloud/network/NetworkServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1635,10 +1635,19 @@
throwInvalidIdException("Network offering with specified id doesn't support adding multiple ip ranges", ntwkOff.getUuid(), NETWORK_OFFERING_ID);
}

if (GuestType.Shared == ntwkOff.getGuestType() && !ntwkOff.isSpecifyVlan() && Objects.isNull(associatedNetworkId)) {
throw new CloudRuntimeException("Associated network must be provided when creating Shared networks when specifyVlan is false");
}


if (GuestType.Shared == ntwkOff.getGuestType()) {
if (!ntwkOff.isSpecifyIpRanges()) {
throw new CloudRuntimeException("The 'specifyipranges' parameter should be true for Shared Networks");

Check warning on line 1642 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L1642 

Added line #L1642 was not covered by tests
}
Comment on lines +1641 to +1643
Copy link
Member

@weizhouapache weizhouapache Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is already a check when create the network offering

cloudstack/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

Lines 6720 to 6730 in b2b2218

if (specifyIpRanges) {
if (type == GuestType.Isolated) {
if (serviceProviderMap.containsKey(Service.SourceNat)) {
throw new InvalidParameterValueException("SpecifyIpRanges can only be true for Shared network offerings and Isolated with no SourceNat service");
}
}
} else {
if (type == GuestType.Shared) {
throw new InvalidParameterValueException("SpecifyIpRanges should always be true for Shared network offerings");
}
}

Copy link
Contributor Author

@DaanHoogland DaanHoogland Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, leaving it in in case someone hacks the DB.

if (ipv4 && Objects.isNull(startIP)) {
throw new CloudRuntimeException("IPv4 address range needs to be provided");

Check warning on line 1645 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L1645 

Added line #L1645 was not covered by tests
}
if (ipv6 && Objects.isNull(startIPv6)) {
throw new CloudRuntimeException("IPv6 address range needs to be provided");

Check warning on line 1648 in server/src/main/java/com/cloud/network/NetworkServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L1648 

Added line #L1648 was not covered by tests
}
Comment on lines +1647 to +1649
Copy link
Contributor Author

@DaanHoogland DaanHoogland Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wido , we merged this and then came to the conclusion this may interfere with SLAAC setups. Can you comment?

Copy link
Contributor

@wido wido Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In shared networks you will never use a start and endip as SLAAC does the work. So these variables will never be used.

The subnet needs to be provided, a /64 and that's it. People can fill in dummy information here, it will not do anything and not break anything

Copy link
Contributor Author

@DaanHoogland DaanHoogland Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, so in spite of this workaround (no high priority) we don’ t even need the UI elements for ipv6 , you are saying @wido ? (cc @weizhouapache )

Copy link
Member

@weizhouapache weizhouapache Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think we do not need the start ipv6 and end ipv6 for shared networks on UI
the UI change is low priority,
the check on startipv6 in java should be removed soon

Copy link
Contributor Author

@DaanHoogland DaanHoogland Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #10746

}
Pair<Integer, Integer> interfaceMTUs = validateMtuConfig(publicMtu, privateMtu, zone.getId());
mtuCheckForVpcNetwork(vpcId, interfaceMTUs, publicMtu);

Expand Down
Loading