Skip to content

Allow ChildReconciler to opt-in to dangerous chilren#506

Merged
scothis merged 1 commit intoreconcilerio:mainfrom
scothis:danger-duck
May 3, 2025
Merged

Allow ChildReconciler to opt-in to dangerous chilren#506
scothis merged 1 commit intoreconcilerio:mainfrom
scothis:danger-duck

Conversation

@scothis
Copy link
Member

@scothis scothis commented Mar 30, 2024

Duck types represent a partial view of a resource. It is dangerous to perform operation that assume the client has the full resource when it only has a portion of the resource. Calling update in this case can result in data loss as fields that are not part of the duck type will be dropped on the server. Likewise, it is difficult to create a resource with a partial object as required/essential fields may not be available.

This change allows users to explicitly override these concerns and use the ChildReconciler and ChildSetReconciler to manage a child via a duck type. This should only be done when the user is certain the duck typed object contains all fields represented on the resource.

@scothis
Copy link
Member Author

scothis commented Mar 30, 2024

Originally vmware-labs/reconciler-runtime#455. Rebased and package imports updated.

@codecov
Copy link

codecov bot commented Mar 30, 2024

Codecov Report

Attention: Patch coverage is 34.78261% with 60 lines in your changes missing coverage. Please review.

Project coverage is 57.13%. Comparing base (56b0037) to head (db7f09f).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
duck/client.go 0.00% 28 Missing ⚠️
reconcilers/child.go 7.69% 11 Missing and 1 partial ⚠️
reconcilers/objectmanager.go 56.25% 7 Missing ⚠️
testing/client.go 0.00% 5 Missing ⚠️
reconcilers/childset.go 20.00% 3 Missing and 1 partial ⚠️
testing/config.go 75.00% 1 Missing and 1 partial ⚠️
testing/tracker.go 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
- Coverage   57.53%   57.13%   -0.40%     
==========================================
  Files          37       37              
  Lines        4340     4414      +74     
==========================================
+ Hits         2497     2522      +25     
- Misses       1737     1784      +47     
- Partials      106      108       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mamachanko
Copy link
Contributor

Do you plan to hold off of this change until #509?

@scothis
Copy link
Member Author

scothis commented Apr 9, 2024

Was originally holding off until @squeedee was able to verify the behavior. Now it's worth holding until we can verify server side apply.

Duck types represent a partial view of a resource. It is dangerous to
perform operation that assume the client has the full resource when it
only has a portion of the resource. Calling update in this case can
result in data loss as fields that are not part of the duck type will be
dropped on the server. Likewise, it is difficult to create a resource
with a partial object as required/essential fields may not be available.

This change allows users to explicitly override these concerns and use
the UpdatingObjectManager to manage a child via a duck type. This should
only be done when the user is certain the duck typed object contains all
fields represented on the resource.

Signed-off-by: Scott Andrews <scott@andrews.me>
@scothis scothis merged commit e07a96c into reconcilerio:main May 3, 2025
2 of 4 checks passed
@scothis scothis deleted the danger-duck branch May 3, 2025 18:39
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