Skip to content

Conversation

@synackd
Copy link
Contributor

@synackd synackd commented Mar 19, 2025

Closes #65

Turns out, UnmarshalJSON() needed to be revised and a MarshalJSON() needed to be added for CloudConfigFile. Base64 encoding/decoding caused by json.Marshal()/json.Unmarshal() was causing mismatches between CloudConfigFile.Encoding and the actual encoding of CloudConfigFile.Content.

For UnmarshalJSON(), continue unmarshalling Content into a string, but don't try to base64 decode into the struct. Encoding will let the reader know if they need to decode it or not.

For MarshalJSON(), use the auxiliary struct like UnmarshalJSON() does to convert the bytes to a string so that json.Marshal() doesn't try to base64-encode the bytes.

@synackd
Copy link
Contributor Author

synackd commented Mar 19, 2025

Just for posterity if anyone wants to mess around with variations of the code, I used this for my own testing before testing with cloud-init itself: https://go.dev/play/p/eWegMtwpleC

@davidallendj
Copy link
Contributor

davidallendj commented Mar 19, 2025

Is this related to the automatic encoding/decoding base64 strings that you mentioned earlier today?

Edit: Iterations 3, 4, and 5 look the same from the go.dev link. What's the different between them? Are these just multiple runs?

@synackd
Copy link
Contributor Author

synackd commented Mar 19, 2025

Is this related to the automatic encoding/decoding base64 strings that you mentioned earlier today?

Yes. Both the unmarshaller/marshaller functions use an auxiliary struct that has Content as a string to mitigate this. Before, only unmarshalling into CloudConfigFile prevented unnecessary base64 decoding using the aux struct, but since there was no custom marshalling function to do the same, the bytes were getting base64-encoded in the JSON no matter whatEncoding was.

Edit: Iterations 3, 4, and 5 look the same from the go.dev link. What's the different between them? Are these just multiple runs?

Yeah, I was making sure that multiple iterations weren't seeing an endlessly-recurring base64 encode chain.

@alexlovelltroy
Copy link
Member

Added tests that confirm the behavior I expect.

LGTM

Copy link
Member

@alexlovelltroy alexlovelltroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@synackd
Copy link
Contributor Author

synackd commented Mar 24, 2025

Thanks for adding the tests! Just one small change I added, that being using base64-encoded data instead of plain for the TestCloudConfigFile_MarshalJSON_Base64 test.

@alexlovelltroy alexlovelltroy merged commit d532ac7 into main Mar 24, 2025
2 checks passed
@synackd synackd deleted the synackd/fix-encoding branch March 24, 2025 16:52
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.

[BUG] Encoding mismatch when getting group cloud-init data

4 participants