starred/go-proxmox

Fork 0

mirror of https://github.com/luthermonson/go-proxmox.git synced 2026-04-26 17:35:48 +03:00

[GH-ISSUE #241] Inconsitent data when marshalling and unmarshalling Storage to json #62

New issue

Closed

opened 2026-03-03 15:29:57 +03:00 by kerem · 3 comments

kerem commented

2026-03-03 15:29:57 +03:00

Owner

Originally created by @aeber on GitHub (Feb 12, 2026).
Original GitHub issue: https://github.com/luthermonson/go-proxmox/issues/241

I think i stumbled uppon some strange behaviour when using the latest version of go-proxmox.

I was trying to run the test suite from https://github.com/ionos-cloud/cluster-api-provider-proxmox against the main branch of go-proxmox and encountered some tests where Storage.Name somehow was set to "".

Digging a bit deeper i found that the Storage struct has two fields that map to the same JSON key (case-insensitively):

Name    string `json:"storage"`  // marshals as "storage"
Storage string                   // marshals as "Storage"

When this struct is marshalled and unmarshalled the aux structs Name field in L1081 matches both storage and Storage.
And if the initial Storage struct looks like

{Name: "iso", "Content": "iso", Enabled: 1}

the json marshaller will make this:

{"Node":"","storage":"test-storage","Enabled":1,"used_fraction":0,"Active":0,"Content":"iso","Shared":0,"Avail":0,"Type":"","Used":0,"Total":0,"Storage":""}

and the custom unmarshaller will then transform it back to a Storage struct. But then the Name field is missing as apparently the Storage field in the json overrides whatever is in the storage field (lower-case) in the json.

I'm not quite sure how to continue from here. Of course all the tests in cluster-api-provider-proxmox could be rewritten to also contain a Storage field with the same value as the Name field. But this duplication feels wrong.

Thinking on this some more the Storage.Storage field actually looks quite redundant. Except for some tests all the code seems to use Storage.Name.
So maybe this field can be removed? As this field is an exported field this would be a breaking change.
Another option could be to omit this field when marshalling by changing it to

Storage      string `json:"-"`

or adding a Storage field to the custom unmarshallers aux struct.

The issue was introduced by https://github.com/luthermonson/go-proxmox/pull/237 where the custom unmarshaller was added.

Originally created by @aeber on GitHub (Feb 12, 2026). Original GitHub issue: https://github.com/luthermonson/go-proxmox/issues/241 I think i stumbled uppon some strange behaviour when using the latest version of go-proxmox. I was trying to run the test suite from https://github.com/ionos-cloud/cluster-api-provider-proxmox against the `main` branch of go-proxmox and encountered some tests where `Storage.Name` somehow was set to `""`. Digging a bit deeper i found that the [`Storage` struct](https://github.com/luthermonson/go-proxmox/blob/c8de04376b1bdeec9782e90ff35ec8240a5375a9/types.go#L1059) has two fields that map to the same JSON key (case-insensitively): ```go Name string `json:"storage"` // marshals as "storage" Storage string // marshals as "Storage" ``` When this struct is marshalled and unmarshalled the aux structs `Name` field in L1081 matches both `storage` and `Storage`. And if the initial `Storage` struct looks like ``` {Name: "iso", "Content": "iso", Enabled: 1} ``` the json marshaller will make this: ``` {"Node":"","storage":"test-storage","Enabled":1,"used_fraction":0,"Active":0,"Content":"iso","Shared":0,"Avail":0,"Type":"","Used":0,"Total":0,"Storage":""} ``` and the custom unmarshaller will then transform it back to a `Storage` struct. But then the `Name` field is missing as apparently the `Storage` field in the json overrides whatever is in the `storage` field (lower-case) in the json. I'm not quite sure how to continue from here. Of course all the tests in [cluster-api-provider-proxmox](https://github.com/ionos-cloud/cluster-api-provider-proxmox) could be rewritten to also contain a `Storage` field with the same value as the `Name` field. But this duplication feels wrong. Thinking on this some more the `Storage.Storage` field actually looks quite redundant. Except for some tests all the code seems to use `Storage.Name`. So maybe this field can be removed? As this field is an exported field this would be a breaking change. Another option could be to omit this field when marshalling by changing it to ``` Storage string `json:"-"` ``` or adding a `Storage` field to the custom unmarshallers `aux` struct. The issue was introduced by https://github.com/luthermonson/go-proxmox/pull/237 where the custom unmarshaller was added.

kerem closed this issue

2026-03-03 15:29:57 +03:00

kerem commented

2026-03-03 15:29:58 +03:00

Author

Owner

@luthermonson commented on GitHub (Feb 13, 2026):

Ugh, ya that's not good. I'll try and look at this tomorrow but I think you're spot on with the root cause

@luthermonson commented on GitHub (Feb 13, 2026): Ugh, ya that's not good. I'll try and look at this tomorrow but I think you're spot on with the root cause

kerem commented

2026-03-03 15:29:58 +03:00

Author

Owner

@aeber commented on GitHub (Feb 17, 2026):

Thanks!
If you have time to spare, a new release would be much appreciated.

@aeber commented on GitHub (Feb 17, 2026): Thanks! If you have time to spare, a new release would be much appreciated.

kerem commented

2026-03-03 15:29:58 +03:00

Author

Owner

@luthermonson commented on GitHub (Feb 18, 2026):

yup! lemme do that now

@luthermonson commented on GitHub (Feb 18, 2026): yup! lemme do that now

kerem referenced this issue

2026-03-03 15:30:16 +03:00

[PR #62] [MERGED] Cleanup #115

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/go-proxmox#62

No description provided.

Rows
Columns