Skip to content
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.

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

SSVM goes into alert state if secstorage.allowed.internal.sites is set to an IP (not CIDR) #10172

Open
Pearl1594 opened this issue Jan 9, 2025 · 2 comments · May be fixed by #10174
Open

SSVM goes into alert state if secstorage.allowed.internal.sites is set to an IP (not CIDR) #10172

Pearl1594 opened this issue Jan 9, 2025 · 2 comments · May be fixed by #10174

Comments

@Pearl1594
Copy link
Contributor

ISSUE TYPE
  • Bug Report
COMPONENT NAME
SSVM
CLOUDSTACK VERSION
4.19.2 and above
CONFIGURATION
OS / ENVIRONMENT
SUMMARY

Historically, it was possible to set IP addresses to secstorage.allowed.internal.sites global setting. In 4.19.2 with the inclusion of the following check: https://github.com/apache/cloudstack/pull/9567/files#diff-86103c46b8773747d21e718c0a245134a6c8bb7880d4b6ee959c47f8396cbad7R630-R632, we only accept CIDRs - which seems to be a regression

STEPS TO REPRODUCE
Set  `secstorage.allowed.internal.sites`  to an IP and recreate SSVM.
EXPECTED RESULTS
SSVM should come up
ACTUAL RESULTS

SSVM goes into alert state


SecondaryStorageListener says there is an error in the connect process for 69 due to Invalid CIDR: 192.168.50.6 com.cloud.utils.exception.CloudRuntimeException: Invalid CIDR: 192.168.50.6
    at com.cloud.utils.net.NetUtils.getCleanIp4Cidr(NetUtils.java:634)
    at org.apache.cloudstack.secondarystorage.SecondaryStorageManagerImpl.getAllowedInternalSiteCidrs(SecondaryStorageManagerImpl.java:404)
    at org.apache.cloudstack.secondarystorage.SecondaryStorageManagerImpl.generateFirewallConfiguration(SecondaryStorageManagerImpl.java:437)
    at com.cloud.storage.secondary.SecondaryStorageListener.processConnect(SecondaryStorageListener.java:87)
    at com.cloud.agent.manager.AgentManagerImpl.notifyMonitorsOfConnection(AgentManagerImpl.java:553)
    at com.cloud.agent.manager.AgentManagerImpl.sendReadyAndGetAttache(AgentManagerImpl.java:1116)
    at com.cloud.agent.manager.AgentManagerImpl.handleConnectedAgent(AgentManagerImpl.java:1135)
    at com.cloud.agent.manager.AgentManagerImpl$HandleAgentConnectTask.runInContext(AgentManagerImpl.java:1227)
    at org.apache.cloudstack.managed.context.ManagedContextRunnable$1.run(ManagedContextRunnable.java:49)
    at org.apache.cloudstack.managed.context.impl.DefaultManagedContext$1.call(DefaultManagedContext.java:56)
    at org.apache.cloudstack.managed.context.impl.DefaultManagedContext.callWithContext(DefaultManagedContext.java:103)
    at org.apache.cloudstack.managed.context.impl.DefaultManagedContext.runWithContext(DefaultManagedContext.java:53)
    at org.apache.cloudstack.managed.context.ManagedContextRunnable.run(ManagedContextRunnable.java:46)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at java.base/java.lang.Thread.run(Thread.java:840)
@DaanHoogland DaanHoogland added this to the 4.19.2 milestone Jan 9, 2025
@DaanHoogland
Copy link
Contributor

@Pearl1594 , I see the isValidIp4Cidr(cidr) check in other places as well, so why would it be wrong in this place? Should we amend the methos to allow a single address as a valid /32 cidr?

@Pearl1594
Copy link
Contributor Author

Yes, I think it should identify IPs as /32 to behave like it used to.

@DaanHoogland DaanHoogland linked a pull request Jan 9, 2025 that will close this issue
14 tasks
@DaanHoogland DaanHoogland linked a pull request Jan 9, 2025 that will close this issue
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants