Copilot commented on code in PR #13818:
URL: https://github.com/apache/cloudstack/pull/13818#discussion_r3758197077
##########
server/src/main/java/org/apache/cloudstack/network/ssl/CertServiceImpl.java:
##########
@@ -199,16 +200,44 @@ public List<SslCertResponse> listSslCerts(final
ListSslCertsCmd listSslCertCmd)
final Account caller = ctx.getCallingAccount();
final Long certId = listSslCertCmd.getCertId();
- final Long accountId = listSslCertCmd.getAccountId();
final Long lbRuleId = listSslCertCmd.getLbId();
final Long projectId = listSslCertCmd.getProjectId();
+ final Long accountId = listSslCertCmd.getAccountId();
+ final String accountName = listSslCertCmd.getAccountName();
+ final Long domainId = listSslCertCmd.getDomainId();
Review Comment:
`domainId` can be provided without `accountName` (and without `projectId`),
but it is effectively ignored because it is not part of the exclusivity check
and won’t route to finalizeOwner unless `accountName` is set. This makes the
new `domainid` parameter misleading. Recommend validating that `domainId`
requires `accountName` (or treating `domainId` as part of the account filter
selection logic) and rejecting `domainId`-only requests with an
InvalidParameterValueException.
##########
server/src/main/java/org/apache/cloudstack/network/ssl/CertServiceImpl.java:
##########
@@ -199,16 +200,44 @@ public List<SslCertResponse> listSslCerts(final
ListSslCertsCmd listSslCertCmd)
final Account caller = ctx.getCallingAccount();
final Long certId = listSslCertCmd.getCertId();
- final Long accountId = listSslCertCmd.getAccountId();
final Long lbRuleId = listSslCertCmd.getLbId();
final Long projectId = listSslCertCmd.getProjectId();
+ final Long accountId = listSslCertCmd.getAccountId();
+ final String accountName = listSslCertCmd.getAccountName();
+ final Long domainId = listSslCertCmd.getDomainId();
- final List<SslCertResponse> certResponseList = new
ArrayList<SslCertResponse>();
+ if (accountId != null && (StringUtils.isNotBlank(accountName) ||
domainId != null)) {
+ throw new InvalidParameterValueException("The accountid and
account/domainid are mutually exclusive");
+ }
- if (certId == null && accountId == null && lbRuleId == null &&
projectId == null) {
- throw new InvalidParameterValueException("Invalid parameters
either certificate ID or Account ID or Loadbalancer ID or Project ID required");
+ // Validate that only one of certid, lbid, projectid, or
accountid/account can be specified
+ ArrayList<Object> params = new ArrayList<>();
+ params.add(certId);
+ params.add(accountId != null ? accountId : accountName);
+ params.add(lbRuleId);
+ params.add(projectId);
Review Comment:
`domainId` can be provided without `accountName` (and without `projectId`),
but it is effectively ignored because it is not part of the exclusivity check
and won’t route to finalizeOwner unless `accountName` is set. This makes the
new `domainid` parameter misleading. Recommend validating that `domainId`
requires `accountName` (or treating `domainId` as part of the account filter
selection logic) and rejecting `domainId`-only requests with an
InvalidParameterValueException.
##########
server/src/main/java/org/apache/cloudstack/network/ssl/CertServiceImpl.java:
##########
@@ -199,16 +200,44 @@ public List<SslCertResponse> listSslCerts(final
ListSslCertsCmd listSslCertCmd)
final Account caller = ctx.getCallingAccount();
final Long certId = listSslCertCmd.getCertId();
- final Long accountId = listSslCertCmd.getAccountId();
final Long lbRuleId = listSslCertCmd.getLbId();
final Long projectId = listSslCertCmd.getProjectId();
+ final Long accountId = listSslCertCmd.getAccountId();
+ final String accountName = listSslCertCmd.getAccountName();
+ final Long domainId = listSslCertCmd.getDomainId();
- final List<SslCertResponse> certResponseList = new
ArrayList<SslCertResponse>();
+ if (accountId != null && (StringUtils.isNotBlank(accountName) ||
domainId != null)) {
+ throw new InvalidParameterValueException("The accountid and
account/domainid are mutually exclusive");
+ }
- if (certId == null && accountId == null && lbRuleId == null &&
projectId == null) {
- throw new InvalidParameterValueException("Invalid parameters
either certificate ID or Account ID or Loadbalancer ID or Project ID required");
+ // Validate that only one of certid, lbid, projectid, or
accountid/account can be specified
+ ArrayList<Object> params = new ArrayList<>();
+ params.add(certId);
+ params.add(accountId != null ? accountId : accountName);
+ params.add(lbRuleId);
+ params.add(projectId);
+
+ int nonNullIds = 0;
+ for (Object param : params) {
+ if (param != null) {
+ nonNullIds++;
+ }
+ }
+ if (nonNullIds > 1) {
+ throw new InvalidParameterValueException("Only one of certid,
lbid, projectid, or accountid/account can be specified");
}
+ Account owner;
+ if (StringUtils.isNotBlank(accountName) || projectId != null) {
+ owner = _accountMgr.finalizeOwner(caller, accountName, domainId,
projectId);
+ } else {
+ owner = caller;
+ }
Review Comment:
`domainId` can be provided without `accountName` (and without `projectId`),
but it is effectively ignored because it is not part of the exclusivity check
and won’t route to finalizeOwner unless `accountName` is set. This makes the
new `domainid` parameter misleading. Recommend validating that `domainId`
requires `accountName` (or treating `domainId` as part of the account filter
selection logic) and rejecting `domainId`-only requests with an
InvalidParameterValueException.
--
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]
