Fix spurious resource replacements during API version upgrades with refresh#4429
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4429 +/- ##
==========================================
+ Coverage 59.44% 59.47% +0.03%
==========================================
Files 91 91
Lines 11480 11531 +51
==========================================
+ Hits 6824 6858 +34
- Misses 4020 4024 +4
- Partials 636 649 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
✅ Fix Verified - Spurious Replacement EliminatedSuccessfully tested the improved fix (commit 87fd4f5) using the blind variant reproduction scenario. The fix eliminates the spurious replacement bug reported in #4400. Test SetupStarting State (Critical Mixed Condition):
Provider Version: Built from Test ResultsOriginal Bug Behavior (v2.90.0)Problem: Provider read resource using NEW API version (2024-10-02-preview) and compared it with state containing OLD API version outputs, causing false positive property differences. Fixed Behavior (This PR)Solution: Provider now correctly looks up resource metadata using the OLD API version ( State EvolutionBefore Refresh: {
"type": "azure-native:containerservice/v20241002preview:ManagedCluster",
"azureApiVersion": "2024-01-02-preview"
}After {
"type": "azure-native:containerservice/v20241002preview:ManagedCluster",
"azureApiVersion": "2024-10-02-preview"
}The API version was seamlessly upgraded during refresh without triggering any spurious replacements. ✨ Key Improvements in This Revision
Test Logs
The fix is working as intended and ready for review! |
Adds replay test to prevent regression of spurious replacements when upgrading from v2.90.0 (versioned containerservice/v20240102preview) to v3 (unversioned containerservice with default API version). Test validates fix in lookupResourceWithAPIVersion() that ensures provider uses OLD API version from state during refresh, not NEW API version from provider metadata. Key aspects: - Two-program structure: v2 uses versioned type, v3 uses unversioned - No explicit alias needed: Pulumi automatically aliases compatible types - Recorded GRPC interactions enable fast CI replay without Azure credentials - Validates the "mixed state" condition where azureApiVersion differs from provider metadata default Includes: - TestUpgradeAksApiVersion_2_90_0 test function - test-programs/upgrade-aks-api-version/ (v2 program) - test-programs/upgrade-aks-api-version/v3/ (v3 program) - Recorded gRPC interactions with v2.90.0 for replay - Comprehensive README documenting test strategy Related to #4400, PR #4429
Fixes #4400 When upgrading a resource to a new API version (e.g., ManagedCluster from v20240102preview to v20241002preview), `pulumi up --refresh` incorrectly triggered resource replacement even though no actual changes were made. Root Cause: ----------- During `pulumi up --refresh` with an API version change: 1. The resource URN type updates to the new API version (via alias) 2. The Read method looks up metadata using the NEW URN type 3. When normalizing old state to inputs, it used the NEW schema for BOTH old state (from old API version) and new state (from Azure) 4. This schema mismatch caused properties unique to the new API version to appear as "additions", triggering spurious diffs and replacements The Fix: -------- The Read method now detects API version changes by comparing the azureApiVersion in state with the current resource metadata version. When a change is detected: 1. Look up the old API version's resource metadata 2. Use the OLD schema when normalizing old state to inputs 3. Use the NEW schema when normalizing new state to inputs 4. Calculate diff between properly schema-aligned projections 5. Fall back to preserving old inputs if old metadata unavailable This ensures the provider's "input-input diffing" architecture correctly handles cross-version state migration during refresh operations. Implementation Details: ---------------------- - Added lookupResourceWithAPIVersion() to fetch metadata for specific API versions - Added apiVersionToVersionPart() to convert API version formats - Modified Read() to perform schema-aware state normalization - Added unit tests for API version format conversion - All existing provider tests pass (no regressions) The fix is backward compatible and only affects behavior during API version upgrades via refresh operations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Improved the lookupResourceWithAPIVersion implementation by: - Using openapi.ApiToSdkVersion() instead of custom parsing logic - Adding ARM path validation to verify resource identity - Removing apiVersionToVersionPart() helper and its tests This reduces code complexity while improving robustness by leveraging well-tested conversion functions that handle all API version formats. Related to #4400
Adds replay test to prevent regression of spurious replacements when upgrading from v2.90.0 (versioned containerservice/v20240102preview) to v3 (unversioned containerservice with default API version). Test validates fix in lookupResourceWithAPIVersion() that ensures provider uses OLD API version from state during refresh, not NEW API version from provider metadata. Key aspects: - Two-program structure: v2 uses versioned type, v3 uses unversioned - No explicit alias needed: Pulumi automatically aliases compatible types - Recorded GRPC interactions enable fast CI replay without Azure credentials - Validates the "mixed state" condition where azureApiVersion differs from provider metadata default Includes: - TestUpgradeAksApiVersion_2_90_0 test function - test-programs/upgrade-aks-api-version/ (v2 program) - test-programs/upgrade-aks-api-version/v3/ (v3 program) - Recorded gRPC interactions with v2.90.0 for replay - Comprehensive README documenting test strategy Related to #4400, PR #4429
Replace manual type token parsing with well-tested utilities from the resources and openapi packages: - Use resources.ParseToken() instead of manual string splitting - Use openapi.ApiToSdkVersion() for canonical version conversion - Use resources.BuildToken() to construct candidate type tokens - Improve error messages to distinguish missing resources from unavailable API versions This makes the code more maintainable and ensures consistent handling of both versioned (azure-native:compute/v20210301:VM) and unversioned (azure-native:compute:VM) type tokens across v2 and v3 providers. Related to #4400
When the Diff method detects that the API version in state differs from the current provider metadata, emit a warning suggesting the user run `pulumi refresh` to update the resource state with the new API version schema. This helps users understand that running refresh will provide better schema alignment and avoid potential spurious diffs when API versions change (e.g., during provider upgrades). The warning: - Only appears when there's an actual API version mismatch - Uses the same detection pattern as the Read method fix for issue #4400 - Handles edge cases gracefully (missing azureApiVersion, invalid types) - Provides clear, actionable guidance Related to #4400 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Tests the fix for issue #4400 where API version changes during provider upgrade caused spurious replacements during refresh. The test validates that: - Read() succeeds when state has old API version but provider has new - Old schema is used for normalizing old state (via lookupResourceWithAPIVersion) - azureApiVersion output is updated to new version after Read - No errors occur during schema-aware diff calculation Uses mock resource map with both old (2024-01-02-preview) and new (2024-10-02-preview) API version metadata to simulate the upgrade scenario. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
15a455c to
53c9387
Compare
|
This PR has been shipped in release v3.11.0. |
Problem
When upgrading resources to a new API version using aliases and running
pulumi up --refresh, the provider incorrectly reported spurious property changes and marked resources for replacement, even though the actual Azure resource hadn't changed.Example scenario:
2024-01-02-preview2024-10-02-previewpulumi up --refreshReported in: #4400
Root Cause
The provider uses "input-input diffing" to detect changes:
The bug: When API version changed via alias, the provider used the NEW schema to normalize BOTH old and new state. This caused properties that only exist in the new schema to appear as "additions".
Concrete Example
Old state (deployed with v2.90.0 using API 2024-01-02-preview):
{ "dnsPrefix": "test-aks-4400", "location": "eastus", "agentPoolProfiles": [...], "azureApiVersion": "2024-01-02-preview" }New Azure response (read with API 2024-10-02-preview):
{ "dnsPrefix": "test-aks-4400", "location": "eastus", "agentPoolProfiles": [...], "kind": "Base", // New property in 2024-10-02-preview "networkProfile": { "podLinkLocalAccess": "IMDS" // New property in 2024-10-02-preview }, "azureApiVersion": "2024-10-02-preview" }Before fix: Both states normalized with 2024-10-02-preview schema
{dnsPrefix, location, agentPoolProfiles}(missing new properties){dnsPrefix, location, agentPoolProfiles, kind, networkProfile}kindandnetworkProfile.podLinkLocalAccessbeing added → triggers replacement ❌After fix: Schema-aware normalization
{dnsPrefix, location, agentPoolProfiles}{dnsPrefix, location, agentPoolProfiles}(new properties dropped as defaults)Solution
Modified the
Readmethod inprovider.goto detect API version changes and use schema-aware normalization:azureApiVersionfrom state vs current metadataKey Code Changes
API Version Detection (lines 1376-1392):
Schema-Aware Normalization (lines 1460-1477):
Testing
✅ Unit tests:
TestReadWithApiVersionMismatchfor Read() with API version mismatchlookupResourceWithAPIVersion()TestApiVersionToVersionPartfor API version conversion utilities✅ E2E regression test:
TestUpgradeAksApiVersion_2_90_0replay-based test✅ Manual testing:
pulumi up --refresh- no spurious replacements ✅azureApiVersionupdated correctlykindandnetworkProfile.podLinkLocalAccesshandled correctly as defaultsAdditional Enhancements
User-facing warning for API version changes:
pulumi refreshto align schemas and update resource stateCode simplifications:
openapi.ApiToSdkVersion()andopenapi.SdkToApiVersion()utilities for format conversionresources.ParseToken()andresources.BuildToken()helpers for type token manipulationFiles Changed
provider/pkg/provider/provider.go- Core fix, helper methods, and user warningsprovider/pkg/provider/provider_test.go- Unit test for Read() with API version mismatchprovider/pkg/provider/resource_lookup_test.go- Unit tests for API version conversionprovider/pkg/provider/provider_e2e_test.go- E2E regression test infrastructureprovider/pkg/provider/test-programs/upgrade-aks-api-version/- E2E test program and recordingsCLAUDE.md- Documentation on testing API version upgradesFixes #4400