api/types/network: generate network-inspect struct definitions from Swagger spec #50855
Conversation
5416d6c to
32fde53
Compare
32fde53 to
c326d03
Compare
|
This one needs a rebase now, @corhere |
Taken verbatim from https://github.com/go-swagger/go-swagger/blob/eee6eaf67f2f342970d277c85ba73f05aed4d114/generator/templates/structfield.gotmpl so the alterations from the upstream template can be easily diffed. Signed-off-by: Cory Snider <csnider@mirantis.com>
c326d03 to
f4acbfc
Compare
This comment was marked as outdated.
This comment was marked as outdated.
502b108 to
b895ee6
Compare
Replace the hand-rolled Network, Summary and Inspect struct types in api/types/network with types generated from the Swagger definition. Disable the generation of all unwanted marshalers and unmarshalers. Signed-off-by: Cory Snider <csnider@mirantis.com>
Replace hand-rolled struct definitions for api/types/network with types generated from the Swagger definitions: - ConfigReference - EndpointResource - NetworkingConfig - PeerInfo - ServiceInfo - Task Add Swagger definitions for ServiceInfo and Task. Signed-off-by: Cory Snider <csnider@mirantis.com>
b895ee6 to
e656f39
Compare
| // This file was generated by the swagger tool. | ||
| // Editing this file might prove futile when you re-run the swagger generate command | ||
|
|
There was a problem hiding this comment.
This one was not in the template we forked, right? (Was looking if we could get rid of it, because it's really just noise on a file with the standard format Code generated by go-swagger; DO NOT EDIT. header 😂
There was a problem hiding this comment.
| // Address represents an IP address | ||
| type Address struct { | ||
| Addr string | ||
| PrefixLen int | ||
| } |
There was a problem hiding this comment.
Got curious why we had this type, but looks like it's only used in fields that are deprecated and those are being removed by @akerouanton in #50846
Looks like that PR doesn't remove the Address type though; probably should 🤔
| description: | ||
| ID of the peer-node in the Swarm cluster. | ||
| type: "string" | ||
| x-omitempty: false |
There was a problem hiding this comment.
x-omitempty: false is not the default?
There was a problem hiding this comment.
There was a problem hiding this comment.
oh! that's surprising; or does it expect all fields that are not "nullable" to be added to required?
There was a problem hiding this comment.
Unless the JSON schema says an object property is required, its optional per the JSON Schema Validation spec.
Go-swagger is taking that to its logical conclusion: properties that aren't required are only serialized if they are set to a meaningful value. We have the choice to use whichever of required or x-omitempty is more appropriate for the circumstances.
The extension does not force a required field to get the “omitempty” modifier.
Given our tendency to deprecate and remove API fields, I think x-omitempty is more appropriate than required when describing the Engine API for the same reason required fields are harmful in protobuf land: what's required today may be deprecated tomorrow.
There was a problem hiding this comment.
Yup, that makes sense! And definitely, x-omitempty is more suitable to what our intent is. I was wondering if other tooling would treat it different.
|
@akerouanton PTAL - this one will conflict with your other PR; not sure what's the best order to merge; this one first or yours first. We also need to sync the v1.52 copy of the swagger file in the docs directory; ideally as part of this PR, but otherwise a follow-up is probably fine. |
|
Chatted with @akerouanton and he's ok with doing the rebase on his PR, so let me bring this one in |
- What I did
network.Inspect,network.Summaryand some of the referenced struct definitions with functionally-equivalent ones generated from the Swagger spec.- How I did it
I had to fork another go-swagger template to work around a curious behaviour where go-swagger would always force the "v" in
IPv4orIPv6to uppercase. It's not supposed to do that.Convincing go-swagger to generate struct definitions with non-pointer struct-valued fields is not too difficult once you get the hang of it. The trick is annotating the definition of the referenced type with
x-omitempty: falseandx-nullable: false. Passing--keep-spec-orderallows us to maintain the exact field order as the hand-rolled definitions to make them 100% drop-in replacements.- How to verify it
By inspection. The new structs should be identical to the hand-written structs aside from doc comment formatting and the presence of
jsonstruct-tags that explicitly set the field names to theencoding/jsondefaults. The struct field names and types should all match up 1:1 to the old struct definitions.- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)