You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "dheeraj12347 (via GitHub)" <gi...@apache.org> on 2026/07/05 10:41:18 UTC
[PR] Add allowedValues support for API parameter discovery [cloudstack]
dheeraj12347 opened a new pull request, #13543:
URL: https://github.com/apache/cloudstack/pull/13543
## Summary
This PR adds support for exposing allowed parameter values through the API Discovery framework.
### Framework changes
- Add `allowedValues` attribute to the `@Parameter` annotation
- Extend `ApiParameterResponse` to expose allowed values
- Update `ApiDiscoveryServiceImpl` to populate `allowedValues`
### API annotations
Add `allowedValues` annotations to representative API parameters with fixed documented values.
The annotations cover:
- Snapshot APIs
- Backup Schedule APIs
- Firewall APIs
- IP Forwarding APIs
- Template APIs
- ISO APIs
- VPC Offering APIs
## Testing
Successfully built using:
```bash
mvn -pl api,plugins/api/discovery -am -DskipTests clean install
```
I also investigated runtime verification using the simulator. The simulator Docker image runs from an embedded source tree, so local source changes require rebuilding the simulator image before they are reflected in `listApis`.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on code in PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#discussion_r3556849029
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java:
##########
@@ -92,10 +92,15 @@ public class CreateVPCOfferingCmd extends BaseAsyncCreateCmd {
@Parameter(name = ApiConstants.SERVICE_CAPABILITY_LIST, type = CommandType.MAP, description = "Desired service capabilities as part of VPC offering", since = "4.4")
private Map<String, List<String>> serviceCapabilityList;
- @Parameter(name = ApiConstants.INTERNET_PROTOCOL,
- type = CommandType.STRING,
- description = "The internet protocol of the offering. Options are IPv4 and dualstack. Default is IPv4. dualstack will create an offering that supports both IPv4 and IPv6",
- since = "4.17.0")
+ @Parameter(
+ name = ApiConstants.INTERNET_PROTOCOL,
+ type = CommandType.STRING,
+ description = "The internet protocol of the offering. Options are IPv4 and dualstack. Default is IPv4. dualstack will create an offering that supports both IPv4 and IPv6",
+ since = "4.17.0",
+ allowedValues = {
+ "IPv4",
+ "dualstack"
+ })
Review Comment:
> I wonder if it's possible to point to existing enums - so that, in future if the list changes i.e, the enum, this will always stay in sync. With the hard-coded approach there's a risk of these values going state. It's a good first approach, but worth checking the feasibility of pointing to existing enums
Hi @Pearl1594 ,
I investigated this further. The parameter is validated using the existing NetUtils.InternetProtocol enum, and I confirmed it's already used throughout the codebase via NetUtils.InternetProtocol.fromValue(...). Since annotation attributes must be compile-time constants, they can't directly reference Enum.values(). I updated the documented allowed value to use the enum's canonical name (DualStack) for consistency. If there's another preferred approach for keeping these values synchronized, I'd be happy to follow it.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add allowedValues support for API parameter discovery [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#issuecomment-4894933323
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/13543?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `30.76923%` with `9 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 20.40%. Comparing base ([`a4a52c9`](https://app.codecov.io/gh/apache/cloudstack/commit/a4a52c96659d9f94fd080096aa673916ae221ef2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0270119`](https://app.codecov.io/gh/apache/cloudstack/commit/02701192d21ea2e57086bd7371dd2ec6f4dfb644?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 105 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/cloudstack/pull/13543?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../cloudstack/api/response/ApiParameterResponse.java](https://app.codecov.io/gh/apache/cloudstack/pull/13543?src=pr&el=tree&filepath=plugins%2Fapi%2Fdiscovery%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fapi%2Fresponse%2FApiParameterResponse.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9hcGkvZGlzY292ZXJ5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2FwaS9yZXNwb25zZS9BcGlQYXJhbWV0ZXJSZXNwb25zZS5qYXZh) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/13543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../cloudstack/discovery/ApiDiscoveryServiceImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/13543?src=pr&el=tree&filepath=plugins%2Fapi%2Fdiscovery%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fdiscovery%2FApiDiscoveryServiceImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9hcGkvZGlzY292ZXJ5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2Rpc2NvdmVyeS9BcGlEaXNjb3ZlcnlTZXJ2aWNlSW1wbC5qYXZh) | 57.14% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/13543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #13543 +/- ##
============================================
+ Coverage 18.08% 20.40% +2.31%
- Complexity 16721 18786 +2065
============================================
Files 6037 5757 -280
Lines 542580 520931 -21649
Branches 66428 60824 -5604
============================================
+ Hits 98149 106293 +8144
+ Misses 433409 403057 -30352
- Partials 11022 11581 +559
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/13543/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/13543/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/13543/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `20.40% <30.76%> (+1.15%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/cloudstack/pull/13543?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add allowedValues support for API parameter discovery [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#issuecomment-4886220843
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add allowedValues support for API parameter discovery [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#issuecomment-4886274697
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 18464
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#discussion_r3624896637
##########
plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java:
##########
@@ -236,10 +237,19 @@ protected ApiDiscoveryResponse getCmdRequestMap(Class<?> cmdClass, APICommand ap
paramResponse.setSince(parameterAnnotation.since());
}
paramResponse.setRelated(parameterAnnotation.entityType()[0].getName());
- if (parameterAnnotation.authorized() != null) {
- paramResponse.setAuthorizedRoleTypes(Arrays.asList(parameterAnnotation.authorized()));
- }
- response.addParam(paramResponse);
+
+ String[] allowedValues = parameterAnnotation.allowedValues();
Review Comment:
Nit: this new block is indented 4 spaces less than the surrounding code (`paramResponse.setRelated(...)` above and the closing brace below are both at the deeper indent), looks like a stray de-indent.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#issuecomment-5121156070
Hi @Pearl1594 and @soreana,
I've addressed the review comments:
Updated the internet protocol values to match the canonical enum names where applicable.
Fixed the indentation issue in ApiDiscoveryServiceImpl.
Verified the updated tests locally.
I've now marked this PR as Ready for Review. I'd appreciate any further feedback when you have time.
As outlined in my GSoC proposal, the next phase (PR4) builds on the allowedValues support introduced in this PR by annotating the remaining API parameters and adding the planned lint/check. Since that work depends on the functionality introduced here, I'd prefer to wait for this PR to be reviewed and merged first so that the follow-up changes are based on main and remain easier to review.
Thank you!
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add allowedValues support for API parameter discovery [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#issuecomment-4913041616
Hi @DaanHoogland , @Pearl1594 and @soreana
I updated the unit test to mock Parameter.allowedValues() by adding:
Mockito.when(parameterMock.allowedValues()).thenReturn(new String[]{});
This fixes the NullPointerException in ApiDiscoveryServiceImplTest, and I verified that the test passes locally with:
mvn -pl plugins/api/discovery \
-Dtest=ApiDiscoveryServiceImplTest \
-DfailIfNoTests=false test
Please let me know if there are any other changes you'd like me to make.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#discussion_r4158492257
##########
plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java:
##########
@@ -236,10 +237,19 @@ protected ApiDiscoveryResponse getCmdRequestMap(Class<?> cmdClass, APICommand ap
paramResponse.setSince(parameterAnnotation.since());
}
paramResponse.setRelated(parameterAnnotation.entityType()[0].getName());
- if (parameterAnnotation.authorized() != null) {
- paramResponse.setAuthorizedRoleTypes(Arrays.asList(parameterAnnotation.authorized()));
- }
- response.addParam(paramResponse);
+
+ String[] allowedValues = parameterAnnotation.allowedValues();
Review Comment:
can we line this up with the setRelated line above? looks like it went 4 spaces too deep now, along with the lines after it
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#discussion_r3680590572
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java:
##########
@@ -92,10 +92,15 @@ public class CreateVPCOfferingCmd extends BaseAsyncCreateCmd {
@Parameter(name = ApiConstants.SERVICE_CAPABILITY_LIST, type = CommandType.MAP, description = "Desired service capabilities as part of VPC offering", since = "4.4")
private Map<String, List<String>> serviceCapabilityList;
- @Parameter(name = ApiConstants.INTERNET_PROTOCOL,
- type = CommandType.STRING,
- description = "The internet protocol of the offering. Options are IPv4 and dualstack. Default is IPv4. dualstack will create an offering that supports both IPv4 and IPv6",
- since = "4.17.0")
+ @Parameter(
+ name = ApiConstants.INTERNET_PROTOCOL,
+ type = CommandType.STRING,
+ description = "The internet protocol of the offering. Options are IPv4 and dualstack. Default is IPv4. dualstack will create an offering that supports both IPv4 and IPv6",
+ since = "4.17.0",
+ allowedValues = {
+ "IPv4",
+ "dualstack"
+ })
Review Comment:
I agree with @Pearl1594
you could add annotation similar to `entityType` (e.g. entityType = ServiceOfferingResponse.class). for example
```
allowedValueType = NetworkOffering.NetworkMode.class
```
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add allowedValues support for API parameter discovery [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#issuecomment-4886218102
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on code in PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#discussion_r4172975962
##########
plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java:
##########
@@ -236,10 +237,19 @@ protected ApiDiscoveryResponse getCmdRequestMap(Class<?> cmdClass, APICommand ap
paramResponse.setSince(parameterAnnotation.since());
}
paramResponse.setRelated(parameterAnnotation.entityType()[0].getName());
- if (parameterAnnotation.authorized() != null) {
- paramResponse.setAuthorizedRoleTypes(Arrays.asList(parameterAnnotation.authorized()));
- }
- response.addParam(paramResponse);
+
+ String[] allowedValues = parameterAnnotation.allowedValues();
Review Comment:
Thanks @Damans227, Fixed the indentation of the allowedValues block to align it with the setRelated line and the surrounding code.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on code in PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#discussion_r3659572937
##########
plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java:
##########
@@ -236,10 +237,19 @@ protected ApiDiscoveryResponse getCmdRequestMap(Class<?> cmdClass, APICommand ap
paramResponse.setSince(parameterAnnotation.since());
}
paramResponse.setRelated(parameterAnnotation.entityType()[0].getName());
- if (parameterAnnotation.authorized() != null) {
- paramResponse.setAuthorizedRoleTypes(Arrays.asList(parameterAnnotation.authorized()));
- }
- response.addParam(paramResponse);
+
+ String[] allowedValues = parameterAnnotation.allowedValues();
Review Comment:
> Nit: this new block is indented 4 spaces less than the surrounding code (`paramResponse.setRelated(...)` above and the closing brace below are both at the deeper indent), looks like a stray de-indent.
Thanks for the review! I've fixed the indentation to match the surrounding code. Please let me know if there are any other changes you'd like me to make.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add allowedValues support for API parameter discovery [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on code in PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#discussion_r3553064963
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java:
##########
@@ -92,10 +92,15 @@ public class CreateVPCOfferingCmd extends BaseAsyncCreateCmd {
@Parameter(name = ApiConstants.SERVICE_CAPABILITY_LIST, type = CommandType.MAP, description = "Desired service capabilities as part of VPC offering", since = "4.4")
private Map<String, List<String>> serviceCapabilityList;
- @Parameter(name = ApiConstants.INTERNET_PROTOCOL,
- type = CommandType.STRING,
- description = "The internet protocol of the offering. Options are IPv4 and dualstack. Default is IPv4. dualstack will create an offering that supports both IPv4 and IPv6",
- since = "4.17.0")
+ @Parameter(
+ name = ApiConstants.INTERNET_PROTOCOL,
+ type = CommandType.STRING,
+ description = "The internet protocol of the offering. Options are IPv4 and dualstack. Default is IPv4. dualstack will create an offering that supports both IPv4 and IPv6",
+ since = "4.17.0",
+ allowedValues = {
+ "IPv4",
+ "dualstack"
+ })
Review Comment:
I wonder if it's possible to point to existing enums - so that, in future if the list changes i.e, the enum, this will always stay in sync. With the hard-coded approach there's a risk of these values going state. It's a good first approach, but worth checking the feasibility of pointing to existing enums
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GSoC: Add allowedValues support for API parameter discovery [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on PR #13543:
URL: https://github.com/apache/cloudstack/pull/13543#issuecomment-5268194473
@dheeraj12347 could you please merge https://github.com/apache/cloudstack/pull/13783 into this PR and address the comment provided on not hard-coding the enum values. Thanks.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org