CSI: replace structs->api with serialization extension #12583
Conversation
The CSI HTTP API has to transform the CSI volume to redact secrets, remove the claims fields, and to consolidate the allocation stubs into a single slice of alloc stubs. This was done manually in #8590 but this is a large amount of code and has proven both very bug prone (see #8659, #8666, #8699, #8735, and #12150) and requires updating lots of code every time we add a field to volumes or plugins. In #10202 we introduce encoding improvements for the `Node` struct that allow a more minimal transformation. Apply this same approach to serializing `structs.CSIVolume` to API responses. Also, the original reasoning behind #8590 for plugins no longer holds because the counts are now denormalized within the state store, so we can simply remove this transformation entirely.
| @@ -8,8 +8,10 @@ var ( | |||
| // extendedTypes is a mapping of extended types to their extension function | |||
| // TODO: the duplicates could be simplified by looking up the base type in the case of a pointer type in ConvertExt | |||
There was a problem hiding this comment.
totally untested (and unnecessary), but you might also be able to re-frame this map as a type switch with the 2 types per case statement
There was a problem hiding this comment.
It looks like we could that in ConvertExt but we'd still need the list for calling SetInterfaceExt on the codec in NomadJsonEncodingExtensions
|
I'm going to plan on picking up #10471 in the next little bit as well. It'll be nice to ditch a bunch of this old grotty code. |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #10470
The CSI HTTP API has to transform the CSI volume to redact secrets,
remove the claims fields, and to consolidate the allocation stubs into
a single slice of alloc stubs. This was done manually in #8590 but
this is a large amount of code and has proven both very bug prone
(see #8659, #8666, #8699, #8735, and #12150) and requires updating
lots of code every time we add a field to volumes or plugins.
In #10202 we introduce encoding improvements for the
Nodestructthat allow a more minimal transformation. Apply this same approach to
serializing
structs.CSIVolumeto API responses.Also, the original reasoning behind #8590 for plugins no longer holds
because the counts are now denormalized within the state store, so we
can simply remove this transformation entirely.
I don't think this needs a changelog? It's not a user-facing behavior change and #10202 didn't include one.
I've tested this change by running capturing the output of
nomad volume status :volume_id,nomad plugin status :plugin_id,nomad operator api /v1/volume/csi/:volume_id | jq .andnomad operator api '/v1/plugin/csi/:plugin_id' | jq .for the following interesting states:I then captured the outputs before and after the update and compared them with
diffto verify that nothing other than timestamps have changed.