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

De/restructure objects via JSON #48

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Conversation

guicassolato
Copy link
Contributor

I've been having issues with using the default Kubernetes API Machinery converter to restruct/destruct objects based on types that contain embedded fields. In particular, converting objects of such kinds often result in data loss.

This is an attempt to workaround those issues by converting via JSON. Although, theoretically more expensive, I hope this can prevent at least the data loss, which is definitely the most concerning problem.

@guicassolato guicassolato self-assigned this Oct 25, 2024
@guicassolato guicassolato force-pushed the fix/unstructured-objects branch 2 times, most recently from 2cf0e95 to 441b361 Compare October 25, 2024 15:42
Signed-off-by: Guilherme Cassolato <[email protected]>
@guicassolato guicassolato force-pushed the fix/unstructured-objects branch from 441b361 to 3a07f46 Compare October 25, 2024 15:50
@guicassolato guicassolato marked this pull request as ready for review October 25, 2024 15:50
@guicassolato
Copy link
Contributor Author

I've tested this with Kuadrant/kuadrant-operator#952 and it works!

This will fix conversion of nested fields such as https://github.com/Kuadrant/authorino/blob/8e530e5593a123a70fce3e815a9228b17f707651/api/v1beta3/auth_config_types.go#L261.

Probably, we'll no longer have to implement custom unmarshallers like this one either:

// UnmarshalJSON unmarshals the AuthPolicySpec from JSON byte array.
// This should not be needed, but runtime.DefaultUnstructuredConverter.FromUnstructured does not work well with embedded structs.
func (s *AuthPolicySpec) UnmarshalJSON(j []byte) error {

Copy link
Contributor

@KevFan KevFan left a comment

Choose a reason for hiding this comment

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

Looks good to me! 💯

@guicassolato guicassolato merged commit ca213ee into main Oct 25, 2024
13 checks passed
@guicassolato guicassolato deleted the fix/unstructured-objects branch October 25, 2024 16:28
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.

2 participants