Copilot commented on code in PR #14467:
URL: https://github.com/apache/cloudstack/pull/14467#discussion_r4349403743
##########
engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java:
##########
@@ -138,6 +138,23 @@ public interface NetworkOrchestrationService {
ConfigKey<Integer> VmNetworkThrottlingRate = new
ConfigKey<Integer>("Network", Integer.class, "vm.network.throttling.rate",
"200",
"Default data transfer rate in megabits per second allowed in User
vm's default network.", true, ConfigKey.Scope.Zone);
+ ConfigKey<String> NetworkLBHaproxyStatsVisbility = new
ConfigKey<>("Network", String.class,
+ "network.loadbalancer.haproxy.stats.visibility", "global",
+ "Load Balancer(haproxy) stats visibility, the value can be one of
the following six parameters :
global,guest-network,link-local,disabled,all,default",
+ true, ConfigKey.Kind.Select,
"global,guest-network,link-local,disabled,all,default");
+
+ ConfigKey<String> NetworkLBHaproxyStatsUri = new ConfigKey<>("Network",
String.class,
+ "network.loadbalancer.haproxy.stats.uri", "/admin?stats",
+ "Load Balancer(haproxy) uri.", true);
+
+ ConfigKey<String> NetworkLBHaproxyStatsAuth = new ConfigKey<>("Secure",
String.class,
+ "network.loadbalancer.haproxy.stats.auth", "admin1:AdMiN123",
+ "Load Balancer(haproxy) authentication string in the format
username:password", true);
Review Comment:
This introduces a hard-coded default credential (`admin1:AdMiN123`) for a
security-sensitive setting. Even though the key is in the `Secure` category, a
known default credential is unsafe and can lead to exposed HAProxy stats
endpoints on fresh installs. Recommended: default this to `null`/empty
(disabled unless explicitly configured), or generate/store a unique secret
during installation/startup, and ensure the stats feature is off unless
explicitly enabled.
##########
engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java:
##########
@@ -138,6 +138,23 @@ public interface NetworkOrchestrationService {
ConfigKey<Integer> VmNetworkThrottlingRate = new
ConfigKey<Integer>("Network", Integer.class, "vm.network.throttling.rate",
"200",
"Default data transfer rate in megabits per second allowed in User
vm's default network.", true, ConfigKey.Scope.Zone);
+ ConfigKey<String> NetworkLBHaproxyStatsVisbility = new
ConfigKey<>("Network", String.class,
Review Comment:
The constant name `NetworkLBHaproxyStatsVisbility` is misspelled (should be
`Visibility`). Since this is a newly introduced public constant, it’s best to
correct it now to avoid propagating the typo across the codebase.
##########
engine/components-api/src/main/java/com/cloud/vm/snapshot/VMSnapshotManager.java:
##########
@@ -33,6 +33,9 @@ public interface VMSnapshotManager extends VMSnapshotService,
Manager {
ConfigKey<Integer> VMSnapshotMax = new ConfigKey<Integer>("Advanced",
Integer.class, "vmsnapshot.max", "10", "Maximum VM snapshots for a single VM",
true, ConfigKey.Scope.Account);
+ ConfigKey<Integer> VMSnapshotCreateWait = new ConfigKey<>("Advanced",
Integer.class, "vmsnapshot.create.wait", "1800",
+ "In second, timeout for create vm snapshot", true);
Review Comment:
The description is grammatically incorrect and ambiguous. Suggest updating
it to something like: “Timeout (in seconds) for VM snapshot creation.” (Same
phrasing issue appears on several newly introduced timeout-related `ConfigKey`
descriptions in this PR.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
