Copilot commented on code in PR #11976:
URL: https://github.com/apache/cloudstack/pull/11976#discussion_r3568652228
##########
server/src/main/java/com/cloud/api/ApiServer.java:
##########
@@ -348,6 +348,19 @@ public class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
, ConfigKey.Scope.Global, null, null, null, null, null,
ConfigKey.Kind.Select,
EnumSet.allOf(ApiSessionKeyCheckOption.class).stream().map(Enum::toString).collect(Collectors.joining(",
")));
+ public static final ConfigKey<Boolean> ApiDisallowInternalIds = new
ConfigKey<>(
+ ConfigKey.CATEGORY_ADVANCED,
+ Boolean.class,
+ "api.disallow.internal.ids",
+ "true",
+ "When enabled, APIs will not honour requests containing internal
database IDs. "
+ + "Only UUIDs will be accepted as entity identifiers. "
+ + "By default, internal IDs are still accepted for
backward compatibility with pre-3.x APIs.",
+ true,
+ ConfigKey.Scope.Global
+ );
Review Comment:
The config key’s description says internal IDs are accepted by default for
backward compatibility, but the declared default value is "true" (which
disallows internal IDs). This is confusing for operators and makes it unclear
what behavior to expect out of the box. Update the description (or the default)
so they match.
##########
server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java:
##########
@@ -510,7 +511,7 @@ private Long translateUuidToInternalId(final String uuid,
final Parameter annota
}
Long internalId = null;
// If annotation's empty, the cmd existed before 3.x try conversion to
long
- final boolean isPre3x = annotation.since().isEmpty();
+ final boolean isPre3x = annotation.since().isEmpty() &&
!ApiServer.ApiDisallowInternalIds.value();
Review Comment:
`isPre3x` is now also gated by the global config
`api.disallow.internal.ids`, so the name is misleading (it no longer strictly
means “API existed before 3.x”). Renaming it to reflect its meaning (whether
internal-ID fallback is allowed) will make the control flow and comments easier
to understand and maintain.
##########
server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java:
##########
@@ -510,7 +511,7 @@ private Long translateUuidToInternalId(final String uuid,
final Parameter annota
}
Long internalId = null;
// If annotation's empty, the cmd existed before 3.x try conversion to
long
- final boolean isPre3x = annotation.since().isEmpty();
+ final boolean isPre3x = annotation.since().isEmpty() &&
!ApiServer.ApiDisallowInternalIds.value();
Review Comment:
This change introduces a new config-controlled behavior (rejecting
numeric/internal IDs even for pre-3.x APIs), but there are no unit tests
covering it. Please add tests that assert: (1) numeric IDs are rejected when
`api.disallow.internal.ids=true`, and (2) numeric IDs are still accepted for
pre-3.x APIs when the config is set to false (if backward compatibility is
intended).
--
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]
