Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update HCS Schema Files #1914

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

helsaawy
Copy link
Contributor

HCS schema v2 files (internal/hcs/schema2) haven't been updated in a while, are missing several types, do not define enum values (eg, ModifyResourceType values are manually defined in internal\protocol\guestresource), and default to int32 for all integer-valued fields (eg, hcsschema.ConsoleSize should have uint16 fields)).

This PR is split into 2 PRs:

  1. Run swagger code generator (swagger-codegen-cli) on HCS schema version 2.5 (based on Windows Server 2022 (OS build 20348)) files to regenerate the hcsschema package (internal/hcs/schema2). Additionally, add helper functions and manual updates to the package.
  2. Update the rest of the repo to use the new structs, where appropriate. This mostly entailed updating variable and struct (field) names.

Additionally, this PR adds a manifest of the files changed (and added) to the schema package (in doc.go), so manual updates can be re-applied if the schema files are re-created in the future.

Note: The files show the API version as 2.4, that is controlled by the OpenAPI schema generator, which we have no control over.
Additionally, even though schema version 2.5 fields are added, since the API is versioned, no new behavior is introduced when making requests.

The major changes within internal/hcs/schema2 are:

  1. Types are updated from their v2.1 names (eg, Processor2 is now VirtualMachineProcessor).
  2. Integer types are now their appropriate format (eg, hcsschema.ConsoleSize is now a uint16 instead of int32). This doesn't apply to integer arrays because of limitations in swagger codegen).
  3. Fix acronym capitalization.
  4. Add internal/hcs/schema2/doc.go to describe package design considerations, add helper functions, and list files that have been manually updated.
  5. Added *_lookup.go files to augment certain enum types with conversion functions. This is specifically for integer-valued enums, which OpenAPI strips information from in the spec, as well as common enums (ie,OSType and SystemType) which we convert from strings.
  6. Add helper functions (in *_extra.go) files for creating certain request messages.
  7. Deleted duplicate files, as well as swagger-specific ancillary files (ie, internal/hcs/schema2/configuration.go, which is never used).
  8. Switch from interface{} to *json.RawMessage globally within package to simplify (un)marshalling, and reduce the number of manual updates needed, since certain instances of interface{} will need it.

The generated schema files (re)define types and values manually created in internal\protocol\guestrequest and internal\protocol\guestresource, but the latter are left for a future PR to consolidate/remove.

@helsaawy helsaawy requested a review from a team as a code owner September 21, 2023 21:47
Run swagger code generator (`swagger-codegen-cli`) on HCS schema
(version 2.5) based on Windows Server 2022 (OS build 20348) files to
regenerate the hcsschema package (`internal/hcs/schema2`).

See for schema version information.
https://learn.microsoft.com/en-us/virtualization/api/hcs/schemareference#schema-version-map

(The files show the API version as 2.4, that is controlled by the
OpenAPI schema generator, which we have no control over.)

Change adds several new schema objects and fields, and updates the types
of other fields (eg, `hcsschema.ConsoleSize` is now a `uint16` instead
of `int32`).

Fix acronym capitalization.

Add `internal/hcs/schema2/doc.go` to describe package design
considerations, add helper functions, and list files that are manually
updated.

Added `*_lookup.go` files to augment certain enum types with conversion
functions. This is specifically for integer-valued enums, which OpenAPI
strips information from in the spec, as well as common enums (ie,
`OSType` and `SystemType`) which we convert from strings.

Deleted duplicate files, as well as swagger-specific ancillary files
(ie, `internal/hcs/schema2/configuration.go`, created by:
https://github.com/swagger-api/swagger-codegen/blob/v2.4.32/modules/swagger-codegen/src/main/resources/go/configuration.mustache)

Switch from `interface{}` to `json.RawMessage` globally within package
to simplify (un)marshalling and pre-compute JSON message components.

Add comment to (mostly string) fields with the expected pattern.

Signed-off-by: Hamza El-Saawy <[email protected]>
Update package to use new `hcsshema` (`internal/hcs/schema2`) package.
Changes are minimal:
 - Use new constants (eg, `hcsschema.ModifyRequestType_ADD` instead of
   `guestrequest.RequestTypeAdd`)
 - Rename certain structs (eg, `hcsschema.VirtualMachineMemory` instead
   of `hcsschema.Memory2`)
 - Update certain field types (eg, `uint16` for `hcsschema.SystemTime`
   fields)

Updates to `internal\protocol\guestrequest` and `internal\protocol\guestresource`
are deferred for future work, when guest protocol definitions and logic
(in `internal\guest\prot` and `internal\guest\bridge`) can be be fully
merged with the host code in `internal\hcs\schema2` and `internal\gcs`.

Signed-off-by: Hamza El-Saawy <[email protected]>
@anmaxvl
Copy link
Contributor

anmaxvl commented Sep 27, 2023

a lot of schema definitions are not currently being used. Is the idea to define them anyway for potential 3rd party integrations or?

@helsaawy helsaawy marked this pull request as draft September 27, 2023 18:27
@helsaawy
Copy link
Contributor Author

a lot of schema definitions are not currently being used. Is the idea to define them anyway for potential 3rd party integrations or?

we dont really have control over what is generated, since we get everything defined in the schema file
we could theoretically manually prune files after the fact (or from the source schema file), but its probably best to leave it alone since theyre not hurting anyone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants