Allow maintaining nested sub-resources at arbitrary depth #2950
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
Does the PR have any schema changes?Looking good! No breaking changes found. |
thomas11
force-pushed
the
tkappler/MaintainSubResource-path
branch
from
e3461ed
to
99cc16c
Compare
thomas11
force-pushed
the
tkappler/MaintainSubResource-path
branch
from
62982a7
to
1b4f1ef
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2950 +/- ##
==========================================
+ Coverage 59.93% 60.18% +0.25%
==========================================
Files 64 64
Lines 10296 10361 +65
==========================================
+ Hits 6171 6236 +65
+ Misses 3602 3601 -1
- Partials 523 524 +1 ☔ View full report in Codecov by Sentry. |
thomas11
force-pushed
the
tkappler/MaintainSubResource-path
branch
from
1b4f1ef
to
340fa94
Compare
danielrbradley
approved these changes
Dec 20, 2023
thomas11
force-pushed
the
tkappler/MaintainSubResource-path
branch
from
340fa94
to
b2dc7c9
Compare
…nd a recursive traversal to populate it
This commit works for the Key Vault access policy case. Open points: - Tests. The code needs the provider to look up types, which are in a PartialMap which is hard to instantiate. Will probably extract a type lookup interface. - Handling of flattened containers - how does it mesh with the property path?
… and make it mockable
thomas11
force-pushed
the
tkappler/MaintainSubResource-path
branch
from
b2dc7c9
to
45da4ee
Compare
danielrbradley
approved these changes
Dec 21, 2023
thomas11
added a commit
that referenced
this pull request
Feb 3, 2024
This PR fixes a defect introduced by #2950. When handling sub-resource properties that may be maintained as stand-alone resources, we need to overwrite them in several code paths. Before, we set them to their default value `[]` on Create, but removed them entirely on Read. This causes unnecessary diffs on Refresh. This PR standardizes the handling on resetting to the default value `[]`. Resolves #3049
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.
This PR is a generalization of #2755, and groundwork for #594. I recommend reading their descriptions first.
The properties pointing to sub-resources that can be maintained as stand-alone resources can now be at any level, with
#/typereferences being followed. Traversal methods have been added toresources.AzureAPIPropertythat can gather the paths leading to sub-resource properties (and can also be used for other traversals).In the provider, a new function
findUnsetSubResourcePropertiesuses the collected sub-resource paths to look up these properties in API-shaped objects and collects the ones that are not set. InRead(), we remove them from outputs so they are not projected into inputs. InUpdate(), we use them to determine where we need to insert cloud state to preserve values not set in our inputs.In order to be able to unit test code that looks up types, this PR also introduces an abstraction
resources.TypeLookupFuncover the previous direct access toazureNativeProvider.resourceMap.Typeswhich is hard to mock.