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

Fix diff for CRD .spec.preserveUnknownFields #2506

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

lblackstone
Copy link
Member

Proposed changes

The v4 diffing logic relies on resource normalization to accurately compare input values to live state. Most resources are normalize by converting the Unstructured resource to its typed equivalent and then back, but this process requires a defined conversion scheme. However, CRDs do not have a conversion scheme defined.

CRD resources have a deprecated parameter ".spec.preserveUnknownFields" that the apiserver removes from the created CRD if the value is false. This causes problems for diffing, because the inputs and the live state are continuously out of sync.

This change adds CRD-specific normalization logic to avoid causing a continuous diff if this field is set in the resource definition.

Related issues (optional)

Fix #2505

The v4 diffing logic relies on resource normalization to accurately compare input values to live state. Most resources are normalize by converting the Unstructured resource to its typed equivalent and then back, but this process requires a defined conversion scheme. However, CRDs do not have a conversion scheme defined.

CRD resources have a deprecated parameter ".spec.preserveUnknownFields" that the apiserver removes from the created CRD if the value is false. This causes problems for diffing, because the inputs and the live state are continuously out of sync.

This change adds CRD-specific normalization logic to avoid causing a continuous diff if this field is set in the resource definition.
@lblackstone lblackstone requested a review from a team July 19, 2023 23:58
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Copy link
Contributor

@rquitales rquitales left a comment

Choose a reason for hiding this comment

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

LGTM. Good catch!

@lblackstone lblackstone enabled auto-merge (squash) July 20, 2023 00:06
@lblackstone lblackstone merged commit e1e16c9 into master Jul 20, 2023
17 checks passed
@lblackstone lblackstone deleted the lblackstone/crd-normalize branch July 20, 2023 00:37
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.

CRD .spec.preserveUnknownFields has continuous diff
2 participants