Automate removing Computed on tags_all for PF resources #4151
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. Maintainer note: consult the runbook for dealing with any breaking changes. |
t0yv0
force-pushed
the
t0yv0/auto-tags-all
branch
from
51155cd to
bbe3750
Compare
| + "github.com/hashicorp/terraform-plugin-framework/resource/schema" | ||
| +) | ||
| + | ||
| +type transformedFrameworkProvider struct { |
There was a problem hiding this comment.
There is an interesting Go problem here I'd love eyes on cc @blampe - the intent is to override the response of provider.Provider interface at some depth that is all I'd like to do is change this response :
provider.Resources(ctx)[17 /*arbitrary index*/]().Schema(ctx, request, response)
Applying the technique of embedding interfaces into structs and overriding.
However..
There is an open set of extra interfaces like resource.ResourceWithModifyPlan though that the framework might be probing for.
The danger with struct wrappers it would seem is that the wrapped provider or resource will no longer pass a type test on these interfaces and possibly lead to invalid behavior? I'm implementing all these interfaces here that I could find, but there still is the risk that new interfaces will be added upstream and this code will need updates and fail quietly. Is there anything better we can do?
There was a problem hiding this comment.
I'm a bit worried about adding this without running the full set of upstream tests.
I don't think we have enough coverage in our existing tests to catch potential problems here.
| + "github.com/hashicorp/terraform-plugin-framework/resource/schema" | ||
| +) | ||
| + | ||
| +type transformedFrameworkProvider struct { |
There was a problem hiding this comment.
I'm a bit worried about adding this without running the full set of upstream tests.
I don't think we have enough coverage in our existing tests to catch potential problems here.
|
That's reasonable, we could build #3022 first and then rearrange things so that the provider modifications happen so that upstream tests exercise our patches, it could be very valuable here. |
Upgrade upstream to [5.63.0](https://github.com/hashicorp/terraform-provider-aws/releases/tag/v5.63.0) Fixes #4373 Fixes #4372 Fixes #3713 Fixes #4369 There are some changes to patches: - #4369 is removed as it is part of upstream now - #3713 is removed as there was no clean conflict resolution; this patch is under test so if test suite passes the removal is safe - #2746 patch is edited to remove rds/security_group resource code that got removed upstream but was preserved in patches. This code appears to be dead / unused code but continues to generate conflicts - added a patch for gamelift.Matchmaking* resources; these are [deprecated](#4375) but remain supported until the next major release. Upstream moved gamelift to Go SDK v2 so the patch is needed to preserve the Go SDK v1 code for these resources without re-coding them. - added a patch to fix tags for new PF-based resources; this can go away once we land #4151 --------- Co-authored-by: Florian Stadler <[email protected]>
|
Notes from team discussion: we can try running some upstream tests over PF, but probably targeting all modules is too hard; we'll target a few modules, exclude upstream tags tests if needed, and unblock this. |
Fixes #2962
The change is related to #3709 and most of it is folded into that patch; with this PR the tags_all attribute is changing to be non-Computed as before, but the change is no longer a source-level change but is a quick runtime pass instead. This removes the burden of patching the provider repeatedly for this concern.
Obsolete patches are removed.
Fixes #3711
Fixes #3714
Fixes #3715
Fixes #3717
Fixes #3721
Fixes #3725
Fixes #3727
All removed patches:
patches/0029-Fix-markTagsAllNotComputedForResources-to-recognize-.patch
patches/0032-DisableTagSchemaCheck-for-PF-provider.patch
patches/0033-Run-scripts-patch_computed_only.sh-to-patch-eks-pod_.patch
patches/0035-Fix-tags_all-Computed-for-PF-resources.patch
patches/0039-Patch-osis_pipeline-tags-flags.patch
patches/0041-Do-not-Compute-tags_all-of-aws_bedrock_provisioned_m.patch
patches/0043-securitylake_subscriber-tags_all-patch.patch
patches/0044-Patch-tags-ComputedOnly-for-m2-resources.patch
patches/0053-Patch-tags-ComputedOnly-on-bedrockagent-and-other-mo.patch
patches/0055-Fix-tags_all-Computed-for-aws_datazone_domain.patch
patches/0058-Fix-tags_all-Computed-for-PF-resources.patch
patches/0059-Fix-tags_all-Computed-for-PF-resources.patch
patches/0060-Fix-tags_all-Computed-for-PF-resources.patch
patches/0062-Patch-tags-ComputedOnly-for-appfabric-networkmonitor.patch