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 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac
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");
}
Comment on lines +1641 to +1643

@weizhouapache weizhouapache Apr 15, 2025

Copy link
Copy Markdown
Member

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");
}
}

@DaanHoogland DaanHoogland Apr 15, 2025

Copy link
Copy Markdown
Contributor Author

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");
}
if (ipv6 && Objects.isNull(startIPv6)) {
throw new CloudRuntimeException("IPv6 address range needs to be provided");
}
Comment on lines +1647 to +1649

@DaanHoogland DaanHoogland Apr 17, 2025

Copy link
Copy Markdown
Contributor Author

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?

@wido wido Apr 17, 2025

Copy link
Copy Markdown
Contributor

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

@DaanHoogland DaanHoogland Apr 17, 2025

Copy link
Copy Markdown
Contributor Author

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 )

@weizhouapache weizhouapache Apr 17, 2025

Copy link
Copy Markdown
Member

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

@DaanHoogland DaanHoogland Apr 18, 2025

Copy link
Copy Markdown
Contributor Author

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