Fix continuous diff for Secret stringData field #2511
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the `stringData` field is set on a Secret resource, then Kubernetes will automatically replace it with an equivalent `data` field. This is causing a continuous diff with the updated logic from 6e329f3. This change updates the input diff logic to include this rewrite.
|
The test fails without this change. It looks like it was erroneously expecting refresh changes before, which covered up this regression. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
This comment was marked as outdated.
This comment was marked as outdated.
rquitales
reviewed
Jul 21, 2023
| @@ -65,22 +65,35 @@ func ToUnstructured(object metav1.Object) (*unstructured.Unstructured, error) { | |||
| // This process normalizes semantically-equivalent resources into an identical output, which is important for diffing. | |||
| // If the scheme is not defined, then return the original resource. | |||
| func Normalize(uns *unstructured.Unstructured) (*unstructured.Unstructured, error) { | |||
| var result *unstructured.Unstructured | |||
|
|
|||
| if IsCRD(uns) { | |||
There was a problem hiding this comment.
nit: preference for switch case here, but will follow up in a later PR/discussion to get this fix out ASAP.
rquitales
approved these changes
Jul 21, 2023
EronWright
reviewed
Sep 19, 2023
| result, err = ToUnstructured(obj) | ||
| // Return the input resource rather than an error if this operation fails. | ||
| if err != nil { | ||
| return uns, err |
There was a problem hiding this comment.
Was it intentional to return the err here? The comment seems to say otherwise.
| obj, err := FromUnstructured(uns) | ||
| // Return the input resource rather than an error if this operation fails. | ||
| if err != nil { | ||
| return uns, nil |
There was a problem hiding this comment.
Was it intentional to ignore all errors, e.g. a value parsing error? For example, given creationTimestamp: invalid_value, a time parser error is caught and ignored.
EronWright
added a commit
that referenced
this pull request
Sep 21, 2023
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
for Pulumi's contribution guidelines.
Help us merge your changes more quickly by adding more details such
as labels, milestones, and reviewers.-->
### Proposed changes
<!--Give us a brief description of what you've done and what it solves.
-->
This PR improves the handling of the read-only metadata fields of
Kubernetes objects, such as `creationTimestamp` and `resourceVersion`.
The provider now ignores these fields when they appear as resource
inputs, to address a few quirks:
1. Unparseable values do not cause an API Server error during preview.
2. A subsequent refresh does not show a difference to the `__inputs`
property.
3. The behavior is now uniform for standard resource types and for
custom resource types.
An integration test is provided to show that such fields are ignored as
inputs, and are still available as outputs. Manual tests confirmed
identical behavior for custom resource objects.
### Related issues (optional)
Closes #2351
Related:
- #2511
- #2445
<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
If the
stringDatafield is set on a Secret resource, then Kubernetes will automatically replace it with an equivalentdatafield. This is causing a continuous diff with the updated logic from 6e329f3. This change updates the input diff logic to include this rewrite.Related issues (optional)
Fix #2507