Uh oh!
There was an error while loading. Please reload this page.
Add support for 4.22.1.0 release - #143
Draft
Pearl1594 wants to merge 5 commits into
Draft
Conversation
…144) * Add support for managing guest OS category - create, delete, update --------- Co-authored-by: Pearl Dsilva <pearl1954@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CloudStack Go SDK/codegen artifacts to align with Apache CloudStack 4.22.1.0, primarily by incorporating updated API metadata (listApis.json) and regenerating affected services (including new params/response fields and new GuestOS OS-category APIs).
Changes:
- Override
queryAsyncJobResult.jobidhandling viarequiredParams.goand adjust codegen/client async polling logic. - Regenerate multiple service bindings from updated
generate/listApis.json(new request params likeresourceid/resourcetype, new response fields likealloweddetails, etc.). - Add GuestOS OS-category API surface (
addOsCategory/deleteOsCategory/updateOsCategory) and corresponding tests/mocks.
Reviewed changes
Copilot reviewed 53 out of 55 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/GuestOSService_test.go | Adds tests for new GuestOS OS-category APIs. |
| generate/requiredParams.go | Adds required-param override for queryAsyncJobResult.jobid. |
| generate/layout.go | Registers OS-category commands under GuestOS service for generation/layout. |
| generate/generate.go | Updates generated GetAsyncJobResult template logic for QueryAsyncJobResult params creation. |
| cloudstack/cloudstack.go | Updates client helper GetAsyncJobResult implementation. |
| cloudstack/AsyncjobService.go | Adds resourceid/resourcetype support to async-job query/list params. |
| cloudstack/GuestOSService.go | Adds OS-category request/response types + ids filter for ListOsTypes. |
| cloudstack/GuestOSService_mock.go | Updates gomock interface to include OS-category methods/constructors. |
| cloudstack/ZoneService.go | Extends VMware DC VM response structs with hypervisor details. |
| cloudstack/VPNService.go | Doc comment capitalization/wording updates. |
| cloudstack/VPCService.go | Doc comment capitalization/wording updates. |
| cloudstack/VolumeService.go | Adds storageid param support to CreateVolume + doc comment updates. |
| cloudstack/VMGroupService.go | Doc comment wording updates (“Instance” terminology). |
| cloudstack/VirtualNetworkFunctionsService.go | Adds alloweddetails response field(s). |
| cloudstack/VirtualMachineService.go | Adds params (osid, usevddk) and response field(s) (alloweddetails), plus doc comment wording updates. |
| cloudstack/UserService.go | Doc comment wording update (“Instance” terminology). |
| cloudstack/UCSService.go | Doc comment capitalization update. |
| cloudstack/TemplateService.go | Doc comment capitalization/wording updates. |
| cloudstack/SystemVMService.go | Doc comment wording updates (“System VM” terminology). |
| cloudstack/SSHService.go | Adds alloweddetails response field + doc comment wording update. |
| cloudstack/SnapshotService.go | Adds snapshot chain fields (parent, parentname) + doc comment wording updates. |
| cloudstack/ResourcemetadataService.go | Doc comment wording update (“Instance” terminology). |
| cloudstack/ResourceIconService.go | Doc comment capitalization update. |
| cloudstack/QuotaService.go | Doc comment capitalization/wording updates. |
| cloudstack/ProjectService.go | Doc comment capitalization updates. |
| cloudstack/PortableIPService.go | Doc comment capitalization updates. |
| cloudstack/NicService.go | Adds alloweddetails response field + doc comment wording update. |
| cloudstack/NetworkService.go | Doc comment capitalization/wording updates. |
| cloudstack/NetworkACLService.go | Doc comment capitalization + typo fix (“bettwen” → “between”). |
| cloudstack/NetscalerService.go | Doc comment capitalization updates. |
| cloudstack/ManagementService.go | Adds version filter param to management server list/metrics. |
| cloudstack/LoadBalancerService.go | Doc comment wording/capitalization updates. |
| cloudstack/LDAPService.go | Doc comment capitalization update. |
| cloudstack/KubernetesService.go | Adds isourl field to supported version responses. |
| cloudstack/ISOService.go | Adds alloweddetails response field + doc comment wording updates. |
| cloudstack/InternalLBService.go | Doc comment wording updates (“Instance” terminology). |
| cloudstack/InfrastructureUsageService.go | Doc comment capitalization update. |
| cloudstack/ImageStoreService.go | Doc comment capitalization update. |
| cloudstack/HostService.go | Adds details to AddSecondaryStorage params; adds version filter param to host list/metrics; doc comment wording updates. |
| cloudstack/FirewallService.go | Doc comment wording/capitalization updates. |
| cloudstack/ExtensionService.go | Adds reservedresourcedetails param/field to extension create/update and responses. |
| cloudstack/DiagnosticsService.go | Doc comment wording update (“System VMs” capitalization). |
| cloudstack/ConsoleEndpointService.go | Doc comment wording update (“Instance” terminology). |
| cloudstack/ConfigurationService.go | Adds capability field snapshotshowchainsize. |
| cloudstack/CertificateService.go | Doc comment wording update (“Templates” capitalization). |
| cloudstack/BrocadeVCSService.go | Doc comment capitalization update. |
| cloudstack/BaremetalService.go | Doc comment capitalization/wording updates. |
| cloudstack/BackupService.go | Adds mountopts fields; adjusts backup schedule response struct; adds alloweddetails field; doc comment wording updates. |
| cloudstack/AutoScaleService.go | Doc comment wording updates (“Instance” terminology). |
| cloudstack/APIDiscoveryService.go | Doc comment capitalization update. |
| cloudstack/AnnotationService.go | Doc comment capitalization updates. |
| cloudstack/AffinityGroupService.go | Adds alloweddetails response field + doc comment wording update. |
| cloudstack/AddressService.go | Doc comment wording/capitalization updates (“IP”, “Account”). |
| cloudstack/AccountService.go | Doc comment wording/capitalization updates. |
Files not reviewed (1)
- cloudstack/GuestOSService_mock.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+542
to
+544
| pn(" p := &QueryAsyncJobResultParams{}") | ||
| pn(" p.p = make(map[string]interface{})") | ||
| pn(" p.SetJobID(jobid)") |
Comment on lines
+492
to
+494
| p := &QueryAsyncJobResultParams{} | ||
| p.p = make(map[string]interface{}) | ||
| p.SetJobID(jobid) |
Comment on lines
+2441
to
+2447
| if ostypeid, ok := m["ostypeid"].(float64); ok { | ||
| m["ostypeid"] = strconv.Itoa(int(ostypeid)) | ||
| b, err = json.Marshal(m) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } |
| } | ||
| // Create a console endpoint to connect to a VM console | ||
| // Create a console endpoint to connect to a Instance console |
| } | ||
| // Attaches an ISO to a virtual machine. | ||
| // Attaches an ISO to an Instance. |
| } | ||
| // Attaches a disk volume to a virtual machine. | ||
| // Attaches a disk volume to an Instance. |
Comment on lines
+73
to
+75
| "queryAsyncJobResult": { | ||
| "jobid", | ||
| }, |
* Adds support to upload k8s ISO from local * update nested response --------- Co-authored-by: Pearl Dsilva <pearl1954@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 57 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- cloudstack/GuestOSService_mock.go: Generated file
- cloudstack/KubernetesService_mock.go: Generated file
Suppressed comments (8)
generate/generate.go:1
- The generator now emits code that instantiates
QueryAsyncJobResultParamsdirectly and mutates its internal map, bypassing the service constructor. Prefer emittingcs.Asyncjob.NewQueryAsyncJobResultParams(jobid)(or an exported constructor/helper) to avoid coupling generated code to internal struct details and to preserve any future constructor invariants.
cloudstack/cloudstack.go:1 GetAsyncJobResultis now manually creatingQueryAsyncJobResultParamsand writing to its internalpmap. Prefer using the public constructorcs.Asyncjob.NewQueryAsyncJobResultParams(jobid)to keep this code resilient to future changes in param initialization and reduce reliance on internal fields.
test/KubernetesService_test.go:1t.Errorfis printf-style; passingerr.Error()directly can triggergo vetwarnings (non-constant format string) and can mis-format messages containing%. Prefert.Errorf(\"GetUploadParamsForKubernetesSupportedVersion: %v\", err)(ort.Fatalfif the test cannot continue).
test/GuestOSService_test.go:1t.Errorfis printf-style; passingerr.Error()directly can triggergo vetwarnings and can mis-format messages containing%. Prefert.Errorf(\"AddOsCategory: %v\", err)(ort.Fatalfif the test cannot continue).
cloudstack/KubernetesService.go:3742- The
checksumparameter is serialized twice. This is redundant and makes it harder to audit generated params; remove the duplicate block so each param is set exactly once.
if v, found := p.p["checksum"]; found {
u.Set("checksum", v.(string))
}
if v, found := p.p["checksum"]; found {
u.Set("checksum", v.(string))
}
cloudstack/KubernetesService.go:3998
- This method fetches upload parameters (it does not perform the upload). Update the doc comment to reflect the actual behavior (e.g., 'Gets upload parameters for a supported Kubernetes version') to avoid misleading API consumers.
// Upload a supported Kubernetes version
func (s *KubernetesService) GetUploadParamsForKubernetesSupportedVersion(p *GetUploadParamsForKubernetesSupportedVersionParams) (*GetUploadParamsForKubernetesSupportedVersionResponse, error) {
resp, err := s.cs.newRequest("getUploadParamsForKubernetesSupportedVersion", p.toURLValues())
cloudstack/GuestOSService.go:2447
DeleteOsCategoryResponsedoes not define anostypeidfield, butUnmarshalJSONtries to coerce it. This looks like accidental leftover code and adds confusing, unnecessary work—remove theostypeidconversion block.
if ostypeid, ok := m["ostypeid"].(float64); ok {
m["ostypeid"] = strconv.Itoa(int(ostypeid))
b, err = json.Marshal(m)
if err != nil {
return err
}
}
cloudstack/LoadBalancerService.go:5734
- There is an extra space in the comment ('list of Instances'). Consider fixing to improve generated docs readability.
// Removes an Instance or a list of Instances from a load balancer rule.
Comment on lines
+672
to
+681
| Id string `json:"id"` | ||
| Intervaltype string `json:"intervaltype"` | ||
| JobID string `json:"jobid"` | ||
| Jobstatus int `json:"jobstatus"` | ||
| Maxbackups int `json:"maxbackups"` | ||
| Quiescevm bool `json:"quiescevm"` | ||
| Schedule string `json:"schedule"` | ||
| Timezone string `json:"timezone"` | ||
| Virtualmachineid string `json:"virtualmachineid"` | ||
| Virtualmachinename string `json:"virtualmachinename"` |
Co-authored-by: Pearl Dsilva <pearl1954@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 61 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- cloudstack/GuestOSService_mock.go: Generated file
- cloudstack/KubernetesService_mock.go: Generated file
Suppressed comments (5)
generate/generate.go:1
- The generator emits code that directly instantiates
QueryAsyncJobResultParamsand mutates its internal map (p.p). This bypasses the public constructor (NewQueryAsyncJobResultParams(jobid)) and couples generated output to internal struct layout. Prefer generating a call to the constructor whenjobidis required; it keeps encapsulation and avoids breakage if the params struct changes.
cloudstack/cloudstack.go:1 GetAsyncJobResultnow constructsQueryAsyncJobResultParamsby manually allocating the struct and setting the internalpmap. SinceAsyncjobServicealready providesNewQueryAsyncJobResultParams(jobid), using the constructor would avoid relying on unexported/internal fields and keep the code resilient to future param-struct changes.
cloudstack/KubernetesService.go:3742- In
GetUploadParamsForKubernetesSupportedVersionParams.toURLValues,checksumis handled twice (the samep.p[\"checksum\"]block appears twice). This is redundant and likely indicates a generator issue or a missing second field (e.g., a different checksum-related param). Remove the duplicate block (or replace it with the intended param) to avoid confusion and ensure the generated parameters match the API spec.
if v, found := p.p["checksum"]; found {
u.Set("checksum", v.(string))
}
cloudstack/GuestOSService.go:2447
DeleteOsCategoryResponse.UnmarshalJSONcontains logic to coerceostypeid, butDeleteOsCategoryResponsedoesn't define anostypeidfield. This adds unnecessary complexity and extra marshal/unmarshal work. Recommend removing theostypeidconversion block and keeping only the conversions that the response type actually needs (e.g.,success).
if ostypeid, ok := m["ostypeid"].(float64); ok {
m["ostypeid"] = strconv.Itoa(int(ostypeid))
b, err = json.Marshal(m)
if err != nil {
return err
}
}
cloudstack/ConsoleEndpointService.go:102
- The article is incorrect in the doc comment; it should be 'an Instance' (not 'a Instance').
// Create a console endpoint to connect to a Instance console
Comment on lines
+672
to
+681
| Id string `json:"id"` | ||
| Intervaltype string `json:"intervaltype"` | ||
| JobID string `json:"jobid"` | ||
| Jobstatus int `json:"jobstatus"` | ||
| Maxbackups int `json:"maxbackups"` | ||
| Quiescevm bool `json:"quiescevm"` | ||
| Schedule string `json:"schedule"` | ||
| Timezone string `json:"timezone"` | ||
| Virtualmachineid string `json:"virtualmachineid"` | ||
| Virtualmachinename string `json:"virtualmachinename"` |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 61 changed files in this pull request and generated no new comments.
Files not reviewed (2)
- cloudstack/GuestOSService_mock.go: Generated file
- cloudstack/KubernetesService_mock.go: Generated file
Suppressed comments (6)
cloudstack/cloudstack.go:1
- This manually constructs
QueryAsyncJobResultParamsand writes to the unexportedpmap directly. This is brittle and bypasses the existing constructor (cs.Asyncjob.NewQueryAsyncJobResultParams(jobid)), which already enforces required params and encapsulates initialization. Prefer using the constructor (and/or ensure setters can initialize internal state) to avoid coupling callers to internal struct fields.
cloudstack/PoolService.go:1 Successis typed asbool, but several CloudStack APIs historically returnsuccessas a string ("true"/"false"). IfconfigureStorageAccessreturns the string form,json.Unmarshalwill error and the call will fail. Consider adding anUnmarshalJSONforStorageAccessResponsesimilar to other response types in this repo that normalizesuccessinto a boolean.
test/PoolService_test.go:1t.Errorf(err.Error())treats the error text as a format string, which can misbehave if it contains%and is generally discouraged bygo vet. Prefert.Error(err)ort.Errorf(\"...: %v\", err).
cloudstack/KubernetesService.go:3742- In
GetUploadParamsForKubernetesSupportedVersionParams.toURLValues(), thechecksumparameter is set twice (duplicate block). Remove the duplicate block to avoid redundant work and reduce the chance of future inconsistencies.
if v, found := p.p["checksum"]; found {
u.Set("checksum", v.(string))
}
cloudstack/GuestOSService.go:2447
DeleteOsCategoryResponse.UnmarshalJSONcontains anostypeidnormalization block, butDeleteOsCategoryResponsedoes not define anostypeidfield. This looks like copy/paste or generator leakage and makes the custom unmarshal harder to reason about. Removing the unused conversion block would reduce confusion and maintenance cost.
if ostypeid, ok := m["ostypeid"].(float64); ok {
m["ostypeid"] = strconv.Itoa(int(ostypeid))
b, err = json.Marshal(m)
if err != nil {
return err
}
}
cloudstack/ConsoleEndpointService.go:102
- Fix article usage: 'a Instance' should be 'an Instance'.
// Create a console endpoint to connect to a Instance console
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The main change in this PR is: