fix: Fix aws_route timeouts-only update fix from upstream PR #47413 - #6547
Merged
Conversation
Contributor
Does the PR have any schema changes?Looking good! No breaking changes found. Maintainer note: consult the runbook for dealing with any breaking changes. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6547 +/- ##
=======================================
Coverage 15.83% 15.83%
=======================================
Files 376 376
Lines 91842 91842
=======================================
Hits 14544 14544
Misses 77276 77276
Partials 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Thank you for this pull request @pierskarsenbarg. I have a few preliminary questions:
Thanks again! |
Member
Author
|
corymhall
requested changes
Jul 21, 2026
corymhall
left a comment
Member
There was a problem hiding this comment.
Thanks for putting this together. The patch itself looks pretty low-risk because it does not introduce any schema or public API changes, and it matches the proposed upstream fix.
Before merging we should add some more tests:
- Run the new upstream acceptance test in CI. The EC2 job in
.github/workflows/aws-upstream-tests.ymlcurrently runs onlyTestAccEC2KeyPair_publicKey, soTestAccVPCRoute_timeoutsOnlyChangewas not exercised despite the EC2 job passing. Add the new Route test to the EC2 test expression, ideally alongsideTestAccVPCRoute_basic. - Add a Pulumi-side regression test. We need an example that creates an
aws:ec2:Route, changes only itscustomTimeouts, performs an actualpulumi up, and verifies that the update succeeds without replacing the Route. The test should fail without the patch and pass with it.
Once those are covered, I think this is a reasonable patch to carry. The risk of permanent API divergence is low because it changes only internal update behavior, although upstream may still choose a somewhat different implementation since hashicorp/terraform-provider-aws#47413 has not yet been reviewed or merged.
…7413 Adds a new patch that backports hashicorp/terraform-provider-aws#47413, which fixes aws_route failing with "route target attribute not specified" when only the timeouts block changes, by adding an early return in resourceRouteUpdate when no route target attributes have changed.
…date Adds examples/test-programs/regress-6549, a minimal VPC/InternetGateway/RouteTable/Route program plus a step1 variant that only adds customTimeouts, and TestRegress6549 which deploys the base program, updates in place, and asserts the change summary shows an in-place update with no replacement (#6549).
pierskarsenbarg
force-pushed
the
patch/vpc-route-timeouts-only-update
branch
from
July 21, 2026 13:02
0d00733 to
caa35c3
Compare
…>v7 transition The regression test previously ran both pulumi up calls against the locally-built (already patched) provider, so it never exercised state created by an unpatched v6 provider and could pass without the fix. Switch to testProviderUpgrade, which deploys the base program against a downloaded, unpatched v6 provider and only links the locally-built provider for the post-UpdateSource update, matching the pattern used by TestJobQueueUpgrade and TestBucketToBucketUpgradeTs.
The default envRegion:true option sets aws:region to a deliberately invalid canary value, expecting the provider to fall back to aws:envRegion instead. Combined with NewSourcePath (multi-step UpdateSource tests), this canary leaked through and was used verbatim for the baseline (v6.78.0) deploy's STS call, causing AWS credential validation to fail via DNS lookup on "sts.INVALID_REGION.amazonaws.com". TestJobQueueUpgrade and TestBucketToBucketUpgradeTs, the only other NewSourcePath-based tests, already disable setEnvRegion for the same reason.
…ress6549 The v6->v7 provider upgrade introduces incidental diffs on unrelated resources (vpc, internet gateway, route table), not just the Route whose customTimeouts changed, so the previously hard-coded ResourceChanges map no longer matches. Use assertup.HasNoReplacements plus a simple check that an update occurred, since what this regression test actually needs to verify is that the update succeeds without replacing the Route, not the exact per-resource-type counts.
Member
Author
|
@corymhall I've added in the tests you suggested. |
corymhall
approved these changes
Jul 22, 2026
Contributor
|
This PR has been shipped in release v7.39.0. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
aws_routeupdate failures with "route target attribute not specified" when only thetimeoutsblock changes (or when a provider upgrade produces a timeouts-only diff).resourceRouteUpdatewhen no route target attributes have changed, avoiding an unnecessary/erroring API call.TestAccVPCRoute_timeoutsOnlyChangeand its supporting config helper.Test plan
./scripts/upstream.sh init -fand confirm the patch applies cleanlyTestAccVPCRoute_timeoutsOnlyChange/TestAccVPCRoute_basicininternal/service/ec2Fixed: #6549