diff --git a/pkg/corerp/handlers/arm_handler.go b/pkg/corerp/handlers/arm_handler.go index abc554ced93..c222849754d 100644 --- a/pkg/corerp/handlers/arm_handler.go +++ b/pkg/corerp/handlers/arm_handler.go @@ -83,18 +83,24 @@ func (handler *armHandler) lookupARMAPIVersion(ctx context.Context, id resources return "", err } - // We need to match on the resource type name without the provider namespace. - shortType := strings.TrimPrefix(id.TypeSegments()[0].Type, id.ProviderNamespace()+"/") + // id.Type() is extension-aware and joins nested segments (see radius-project/radius#12694). + fullType := id.Type() + shortType := strings.TrimPrefix(fullType, id.ProviderNamespace()+"/") for _, rt := range resp.ResourceTypes { - if !strings.EqualFold(shortType, *rt.ResourceType) { + if rt.ResourceType == nil { continue } - if rt.DefaultAPIVersion != nil { + if !strings.EqualFold(shortType, *rt.ResourceType) && !strings.EqualFold(fullType, *rt.ResourceType) { + continue + } + if rt.DefaultAPIVersion != nil && *rt.DefaultAPIVersion != "" { return *rt.DefaultAPIVersion, nil } - if len(rt.APIVersions) > 0 { - return *rt.APIVersions[0], nil + for _, version := range rt.APIVersions { + if version != nil && *version != "" { + return *version, nil + } } return "", fmt.Errorf("could not find API version for type %q, no supported API versions", id.Type()) diff --git a/pkg/portableresources/processors/resourceclient.go b/pkg/portableresources/processors/resourceclient.go index fbd31752129..59213138337 100644 --- a/pkg/portableresources/processors/resourceclient.go +++ b/pkg/portableresources/processors/resourceclient.go @@ -18,6 +18,7 @@ package processors import ( context "context" + "errors" "fmt" "strings" @@ -105,6 +106,14 @@ func (c *resourceClient) deleteAzureResource(ctx context.Context, id resources.I apiVersion, err := c.lookupARMAPIVersion(ctx, id) if err != nil { + // A failed API-version lookup happens before any DELETE is issued, so the 404 + // tolerance below never runs. Skipping an unresolvable output resource keeps + // recipe deletion from becoming permanently stuck (see radius-project/radius#12694). + if errors.Is(err, errARMAPIVersionNotFound) { + logger := ucplog.FromContextOrDiscard(ctx) + logger.Info(fmt.Sprintf("skipping deletion of %q: %v", id.String(), err)) + return nil + } return err } @@ -147,31 +156,39 @@ func (c *resourceClient) lookupARMAPIVersion(ctx context.Context, id resources.I return "", err } - // We need to match on the resource type name without the provider namespace. For an extension - // resource (eg: a Microsoft.Authorization/locks resource attached to another resource), the - // provider namespace and type come from the extension segments, not the primary type segments. - segments := id.TypeSegments() - if len(id.ExtensionSegments()) > 0 { - segments = id.ExtensionSegments() - } - shortType := strings.TrimPrefix(segments[0].Type, id.ProviderNamespace()+"/") for _, rt := range resp.ResourceTypes { - if !strings.EqualFold(shortType, *rt.ResourceType) { + if rt.ResourceType == nil || !armResourceTypeMatches(id, *rt.ResourceType) { continue } - if rt.DefaultAPIVersion != nil { + if rt.DefaultAPIVersion != nil && *rt.DefaultAPIVersion != "" { return *rt.DefaultAPIVersion, nil } - if len(rt.APIVersions) > 0 { - return *rt.APIVersions[0], nil + for _, version := range rt.APIVersions { + if version != nil && *version != "" { + return *version, nil + } } - return "", fmt.Errorf("could not find API version for type %q, no supported API versions", id.Type()) - + return "", fmt.Errorf("could not find API version for type %q, no supported API versions: %w", id.Type(), errARMAPIVersionNotFound) } - return "", fmt.Errorf("could not find API version for type %q, type was not found", id.Type()) + return "", fmt.Errorf("could not find API version for type %q, type was not found: %w", id.Type(), errARMAPIVersionNotFound) +} + +// errARMAPIVersionNotFound is returned when the ARM provider listing has no usable API +// version for the resource type. Delete treats this as skippable so one unresolvable +// output resource cannot brick the parent. +var errARMAPIVersionNotFound = errors.New("ARM API version not found") + +// armResourceTypeMatches compares an ARM resource ID to a resourceType string from +// Providers.Get. id.Type() is extension-aware and joins nested segments, so this +// matches both Microsoft.Authorization/locks and databaseAccounts/sqlDatabases. +// Providers list types as either the short name ("locks") or the fully-qualified name. +func armResourceTypeMatches(id resources.ID, listed string) bool { + fullType := id.Type() + shortType := strings.TrimPrefix(fullType, id.ProviderNamespace()+"/") + return strings.EqualFold(shortType, listed) || strings.EqualFold(fullType, listed) } func (c *resourceClient) deleteUCPResource(ctx context.Context, id resources.ID) error { diff --git a/pkg/portableresources/processors/resourceclient_test.go b/pkg/portableresources/processors/resourceclient_test.go index 3020961b9c3..3db693fdded 100644 --- a/pkg/portableresources/processors/resourceclient_test.go +++ b/pkg/portableresources/processors/resourceclient_test.go @@ -52,6 +52,10 @@ const ( // https://github.com/radius-project/radius/issues/12694. ARMExtensionResourceID = "/subscriptions/0000/resourceGroups/test-rg/providers/Microsoft.DocumentDB/databaseAccounts/test-account/providers/Microsoft.Authorization/locks/test-lock" ARMExtensionProviderPath = "/subscriptions/0000/providers/Microsoft.Authorization" + + // Nested ARM type (parent/child) as listed by Providers.Get, e.g. databaseAccounts/sqlDatabases. + ARMNestedResourceID = "/subscriptions/0000/resourceGroups/test-rg/providers/Microsoft.DocumentDB/databaseAccounts/test-account/sqlDatabases/test-db" + ARMNestedProviderPath = "/subscriptions/0000/providers/Microsoft.DocumentDB" ) func Test_Delete_InvalidResourceID(t *testing.T) { @@ -172,8 +176,12 @@ func Test_Delete_ARM(t *testing.T) { require.IsType(t, &ResourceError{}, err) }) - t.Run("failure - lookup API Version - resource type not found", func(t *testing.T) { + t.Run("success - skip delete when resource type is not found", func(t *testing.T) { mux := http.NewServeMux() + mux.HandleFunc(ARMResourceID, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("delete should not be called when API version lookup misses") + w.WriteHeader(http.StatusInternalServerError) + }) mux.HandleFunc(ARMProviderPath, handleJSONResponse(t, armresources.Provider{ Namespace: new("Microsoft.Compute"), ResourceTypes: []*armresources.ProviderResourceType{}, @@ -186,13 +194,15 @@ func Test_Delete_ARM(t *testing.T) { c.armClientOptions = newClientOptions(server.Client(), server.URL) err := c.Delete(t.Context(), ARMResourceID) - require.Error(t, err) - require.IsType(t, &ResourceError{}, err) - require.Contains(t, err.Error(), "could not find API version for type \"Microsoft.Compute/virtualMachines\", type was not found") + require.NoError(t, err) }) - t.Run("failure - lookup API Version - no api versions", func(t *testing.T) { + t.Run("success - skip delete when no api versions", func(t *testing.T) { mux := http.NewServeMux() + mux.HandleFunc(ARMResourceID, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("delete should not be called when API version lookup misses") + w.WriteHeader(http.StatusInternalServerError) + }) mux.HandleFunc(ARMProviderPath, handleJSONResponse(t, armresources.Provider{ Namespace: new("Microsoft.Compute"), ResourceTypes: []*armresources.ProviderResourceType{ @@ -210,9 +220,7 @@ func Test_Delete_ARM(t *testing.T) { c.armClientOptions = newClientOptions(server.Client(), server.URL) err := c.Delete(t.Context(), ARMResourceID) - require.Error(t, err) - require.IsType(t, &ResourceError{}, err) - require.Contains(t, err.Error(), "could not find API version for type \"Microsoft.Compute/virtualMachines\", no supported API versions") + require.NoError(t, err) }) t.Run("success - lookup API Version - extension resource", func(t *testing.T) { @@ -237,6 +245,56 @@ func Test_Delete_ARM(t *testing.T) { err := c.Delete(t.Context(), ARMExtensionResourceID) require.NoError(t, err) }) + + t.Run("success - lookup API Version - extension resource fully-qualified type", func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc(ARMExtensionResourceID, handleDeleteSuccess()) + mux.HandleFunc(ARMExtensionProviderPath, handleJSONResponse(t, armresources.Provider{ + Namespace: new("Microsoft.Authorization"), + ResourceTypes: []*armresources.ProviderResourceType{ + { + ResourceType: new("Microsoft.Authorization/locks"), + DefaultAPIVersion: new(ARMAPIVersion), + }, + }, + }, 200)) + + server := httptest.NewServer(mux) + defer server.Close() + + c := NewResourceClient(newArmOptions(server.URL), nil, nil) + c.armClientOptions = newClientOptions(server.Client(), server.URL) + + err := c.Delete(t.Context(), ARMExtensionResourceID) + require.NoError(t, err) + }) + + t.Run("success - lookup API Version - nested resource", func(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc(ARMNestedResourceID, handleDeleteSuccess()) + mux.HandleFunc(ARMNestedProviderPath, handleJSONResponse(t, armresources.Provider{ + Namespace: new("Microsoft.DocumentDB"), + ResourceTypes: []*armresources.ProviderResourceType{ + { + ResourceType: new("databaseAccounts"), + DefaultAPIVersion: new("9999-01-01"), + }, + { + ResourceType: new("databaseAccounts/sqlDatabases"), + DefaultAPIVersion: new(ARMAPIVersion), + }, + }, + }, 200)) + + server := httptest.NewServer(mux) + defer server.Close() + + c := NewResourceClient(newArmOptions(server.URL), nil, nil) + c.armClientOptions = newClientOptions(server.Client(), server.URL) + + err := c.Delete(t.Context(), ARMNestedResourceID) + require.NoError(t, err) + }) } func Test_Delete_Kubernetes(t *testing.T) {