From 9aaa814bf80668ada8bc98da2cfc80a7d1f18fb6 Mon Sep 17 00:00:00 2001 From: Eron Wright Date: Mon, 17 Nov 2025 17:08:20 -0800 Subject: [PATCH] Fix IsNotFound to validate Azure error codes before removing resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The IsNotFound() function was incorrectly treating any HTTP 404 response as a legitimate resource deletion, even when the 404 came from proxies, WAFs, or network intermediaries rather than from Azure itself. This caused resources to be unexpectedly removed from Pulumi state. Changes: - Updated IsNotFound() to validate error codes before confirming deletion - Valid Azure "not found" codes: NotFound, ResourceNotFound, ResourceGroupNotFound - Added warning logs when 404 lacks valid Azure error codes - Returns false (preserves resource) when error code validation fails - Applied to all error types: azure.RequestError, azcore.ResponseError, PulumiAzcoreResponseError Testing: - Added comprehensive test coverage for all scenarios - Tests verify 404 with valid codes returns true - Tests verify 404 without codes returns false - All azure package tests passing Fixes https://github.com/pulumi/pulumi-azure-native/issues/4415 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- provider/pkg/azure/azure.go | 53 +++++++++++-- provider/pkg/azure/azure_test.go | 96 ++++++++++++++++++++++-- provider/pkg/azure/client_azcore_test.go | 14 +++- 3 files changed, 149 insertions(+), 14 deletions(-) diff --git a/provider/pkg/azure/azure.go b/provider/pkg/azure/azure.go index c45b4ed4e24f..4c5e9ce0b665 100644 --- a/provider/pkg/azure/azure.go +++ b/provider/pkg/azure/azure.go @@ -48,21 +48,64 @@ func BuildUserAgent(partnerID string) (userAgent string) { return } -// IsNotFound returns true if the error is a HTTP 404. +// IsNotFound returns true if the error is a HTTP 404 with a valid Azure "not found" error code. +// This helps distinguish legitimate Azure "not found" responses from proxy/WAF 404 responses. func IsNotFound(err error) bool { + validNotFoundCodes := map[string]bool{ + "NotFound": true, + "ResourceNotFound": true, + "ResourceGroupNotFound": true, + } + if requestError, ok := err.(azure.RequestError); ok { - return requestError.StatusCode == http.StatusNotFound + if requestError.StatusCode == http.StatusNotFound { + // Check if ServiceError contains a valid Azure error code + if requestError.ServiceError != nil && validNotFoundCodes[requestError.ServiceError.Code] { + return true + } + logging.V(3).Infof("Received HTTP 404 without valid Azure error code (ServiceError=%v). "+ + "This may indicate a proxy/WAF response rather than a legitimate Azure resource not found error.", + requestError.ServiceError) + return false + } } if requestError, ok := err.(*azure.RequestError); ok { - return requestError.StatusCode == http.StatusNotFound + if requestError.StatusCode == http.StatusNotFound { + // Check if ServiceError contains a valid Azure error code + if requestError.ServiceError != nil && validNotFoundCodes[requestError.ServiceError.Code] { + return true + } + logging.V(3).Infof("Received HTTP 404 without valid Azure error code (ServiceError=%v). "+ + "This may indicate a proxy/WAF response rather than a legitimate Azure resource not found error.", + requestError.ServiceError) + return false + } } if responseError, ok := err.(*azcore.ResponseError); ok { - return responseError.StatusCode == http.StatusNotFound + if responseError.StatusCode == http.StatusNotFound { + // Check if ErrorCode contains a valid Azure error code + if validNotFoundCodes[responseError.ErrorCode] { + return true + } + logging.V(3).Infof("Received HTTP 404 without valid Azure error code (ErrorCode=%q). "+ + "This may indicate a proxy/WAF response rather than a legitimate Azure resource not found error.", + responseError.ErrorCode) + return false + } } if responseError, ok := err.(*PulumiAzcoreResponseError); ok { - return responseError.StatusCode == http.StatusNotFound + if responseError.StatusCode == http.StatusNotFound { + // Check if ErrorCode contains a valid Azure error code + if validNotFoundCodes[responseError.ErrorCode] { + return true + } + logging.V(3).Infof("Received HTTP 404 without valid Azure error code (ErrorCode=%q). "+ + "This may indicate a proxy/WAF response rather than a legitimate Azure resource not found error.", + responseError.ErrorCode) + return false + } } return false diff --git a/provider/pkg/azure/azure_test.go b/provider/pkg/azure/azure_test.go index 594230ccb082..8e9febe9ba67 100644 --- a/provider/pkg/azure/azure_test.go +++ b/provider/pkg/azure/azure_test.go @@ -76,34 +76,116 @@ func TestBuildUserAgent(t *testing.T) { } func TestIsNotFound(t *testing.T) { - t.Run("autorest", func(t *testing.T) { - assert.True(t, IsNotFound(&autorestAzure.RequestError{ + t.Run("autorest with valid error code", func(t *testing.T) { + // Test all valid "not found" error codes + validCodes := []string{"NotFound", "ResourceNotFound", "ResourceGroupNotFound"} + for _, code := range validCodes { + assert.True(t, IsNotFound(&autorestAzure.RequestError{ + DetailedError: autorest.DetailedError{ + StatusCode: http.StatusNotFound, + }, + ServiceError: &autorestAzure.ServiceError{ + Code: code, + }, + }), "Should return true for error code: "+code) + } + }) + + t.Run("autorest with 404 but no error code", func(t *testing.T) { + // 404 without ServiceError (e.g., proxy/WAF response) + assert.False(t, IsNotFound(&autorestAzure.RequestError{ DetailedError: autorest.DetailedError{ + StatusCode: http.StatusNotFound, + }, + ServiceError: nil, + })) + }) + t.Run("autorest with 404 but invalid error code", func(t *testing.T) { + // 404 with non-matching error code + assert.False(t, IsNotFound(&autorestAzure.RequestError{ + DetailedError: autorest.DetailedError{ StatusCode: http.StatusNotFound, }, + ServiceError: &autorestAzure.ServiceError{ + Code: "SomeOtherError", + }, })) + }) + + t.Run("autorest with non-404 status", func(t *testing.T) { assert.False(t, IsNotFound(&autorestAzure.RequestError{ DetailedError: autorest.DetailedError{ StatusCode: http.StatusForbidden, }, + ServiceError: &autorestAzure.ServiceError{ + Code: "ResourceNotFound", + }, })) }) - t.Run("azcore", func(t *testing.T) { - assert.True(t, IsNotFound(&azcore.ResponseError{ + t.Run("azcore with valid error code", func(t *testing.T) { + // Test all valid "not found" error codes + validCodes := []string{"NotFound", "ResourceNotFound", "ResourceGroupNotFound"} + for _, code := range validCodes { + assert.True(t, IsNotFound(&azcore.ResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: code, + }), "Should return true for error code: "+code) + } + }) + + t.Run("azcore with 404 but no error code", func(t *testing.T) { + // 404 without ErrorCode (e.g., proxy/WAF response) + assert.False(t, IsNotFound(&azcore.ResponseError{ StatusCode: http.StatusNotFound, + ErrorCode: "", })) + }) + + t.Run("azcore with 404 but invalid error code", func(t *testing.T) { + // 404 with non-matching error code + assert.False(t, IsNotFound(&azcore.ResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: "SomeOtherError", + })) + }) + + t.Run("azcore with non-404 status", func(t *testing.T) { assert.False(t, IsNotFound(&azcore.ResponseError{ StatusCode: http.StatusForbidden, + ErrorCode: "ResourceNotFound", + })) + }) + + t.Run("provider with valid error code", func(t *testing.T) { + // Test all valid "not found" error codes + validCodes := []string{"NotFound", "ResourceNotFound", "ResourceGroupNotFound"} + for _, code := range validCodes { + assert.True(t, IsNotFound(&PulumiAzcoreResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: code, + }), "Should return true for error code: "+code) + } + }) + + t.Run("provider with 404 but no error code", func(t *testing.T) { + // 404 without ErrorCode (e.g., proxy/WAF response) + assert.False(t, IsNotFound(&PulumiAzcoreResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: "", })) }) - t.Run("provider", func(t *testing.T) { - assert.True(t, IsNotFound(&PulumiAzcoreResponseError{ + t.Run("provider with 404 but invalid error code", func(t *testing.T) { + // 404 with non-matching error code + assert.False(t, IsNotFound(&PulumiAzcoreResponseError{ StatusCode: http.StatusNotFound, - ErrorCode: "ResourceNotFound", + ErrorCode: "SomeOtherError", })) + }) + + t.Run("provider with non-404 status", func(t *testing.T) { assert.False(t, IsNotFound(&PulumiAzcoreResponseError{ StatusCode: http.StatusForbidden, ErrorCode: "Unauthorized", diff --git a/provider/pkg/azure/client_azcore_test.go b/provider/pkg/azure/client_azcore_test.go index 441821de78b9..27a98f4b957f 100644 --- a/provider/pkg/azure/client_azcore_test.go +++ b/provider/pkg/azure/client_azcore_test.go @@ -854,11 +854,21 @@ func TestNewResponseError(t *testing.T) { assert.Equal(t, `Status=409 Message="{"foo": "bar"}"`, err.Error()) }) - t.Run("404 is recognized by IsNotFound", func(t *testing.T) { + t.Run("404 with valid error code is recognized by IsNotFound", func(t *testing.T) { resp := &http.Response{ StatusCode: 404, - Body: io.NopCloser(strings.NewReader(`not found`)), + Body: io.NopCloser(strings.NewReader(`{"error": {"code": "ResourceNotFound", "message": "not found"}}`)), + Header: http.Header{"X-Ms-Error-Code": []string{"ResourceNotFound"}}, } assert.True(t, IsNotFound(newResponseError(resp))) }) + + t.Run("404 without error code is NOT recognized by IsNotFound", func(t *testing.T) { + // This simulates a proxy/WAF 404 response without Azure error codes + resp := &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader(`not found`)), + } + assert.False(t, IsNotFound(newResponseError(resp))) + }) }