Skip to content

Commit

Permalink
ignore polling error from nonstandard LRO APIs (#341)
Browse files Browse the repository at this point in the history
* ignore polling error from nonstandard LRO APIs

* add comment and linked issues
  • Loading branch information
ms-henglu authored Aug 17, 2023
1 parent 157088d commit 19e4772
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ ENHANCEMENTS:

BUG FIXES:
- Fix a bug that `azapi_resource` resource doesn't store the `id` in the state when error happens during the creation.
- Fix a bug that errors from the polling API which don't follow the ARM LRO guidelines are not handled properly.

## v1.8.0
FEATURES:
Expand Down
45 changes: 42 additions & 3 deletions internal/clients/resource_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ func (client *ResourceClient) CreateOrUpdate(ctx context.Context, resourceID str
resp, err := pt.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{
Frequency: 10 * time.Second,
})
return resp, err
if err == nil {
return resp, nil
}
if !client.shouldIgnorePollingError(err) {
return nil, err
}
}
if err := runtime.UnmarshalAsJSON(resp, &responseBody); err != nil {
return nil, err
Expand Down Expand Up @@ -134,7 +139,12 @@ func (client *ResourceClient) Delete(ctx context.Context, resourceID string, api
resp, err := pt.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{
Frequency: 10 * time.Second,
})
return resp, err
if err == nil {
return resp, nil
}
if !client.shouldIgnorePollingError(err) {
return nil, err
}
}
if err := runtime.UnmarshalAsJSON(resp, &responseBody); err != nil {
return nil, err
Expand Down Expand Up @@ -181,7 +191,12 @@ func (client *ResourceClient) Action(ctx context.Context, resourceID string, act
resp, err := pt.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{
Frequency: 10 * time.Second,
})
return resp, err
if err == nil {
return resp, nil
}
if !client.shouldIgnorePollingError(err) {
return nil, err
}
}

contentType := resp.Header.Get("Content-Type")
Expand Down Expand Up @@ -314,3 +329,27 @@ func (client *ResourceClient) List(ctx context.Context, url string, apiVersion s
"value": value,
}, nil
}

func (client *ResourceClient) shouldIgnorePollingError(err error) bool {
if err == nil {
return true
}
// there are some APIs that don't follow the ARM LRO guideline, return the response as is
if responseErr, ok := err.(*azcore.ResponseError); ok {
if responseErr.RawResponse != nil && responseErr.RawResponse.Request != nil {
// all control plane APIs must flow through ARM, ignore the polling error if it's not ARM
// issue: https://github.com/Azure/azure-rest-api-specs/issues/25356, in this case, the polling url is not exposed by ARM
pollRequest := responseErr.RawResponse.Request
if pollRequest.Host != strings.TrimPrefix(client.host, "https://") {
return true
}

// ignore the polling error if the polling url doesn't support GET method
// issue:https://github.com/Azure/azure-rest-api-specs/issues/25362, in this case, the polling url doesn't support GET method
if responseErr.StatusCode == http.StatusMethodNotAllowed {
return true
}
}
}
return false
}
6 changes: 3 additions & 3 deletions internal/services/azapi_resource_action_data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
type ActionDataSource struct{}

func TestAccActionDataSource_basic(t *testing.T) {
data := acceptance.BuildTestData(t, "azapi_resource_action", "test")
data := acceptance.BuildTestData(t, "data.azapi_resource_action", "test")
r := ActionDataSource{}

data.DataSourceTest(t, []resource.TestStep{
Expand All @@ -23,7 +23,7 @@ func TestAccActionDataSource_basic(t *testing.T) {
}

func TestAccActionDataSource_providerPermissions(t *testing.T) {
data := acceptance.BuildTestData(t, "azapi_resource_action", "test")
data := acceptance.BuildTestData(t, "data.azapi_resource_action", "test")
r := ActionDataSource{}

data.DataSourceTest(t, []resource.TestStep{
Expand All @@ -35,7 +35,7 @@ func TestAccActionDataSource_providerPermissions(t *testing.T) {
}

func TestAccActionDataSource_providerAction(t *testing.T) {
data := acceptance.BuildTestData(t, "azapi_resource_action", "test")
data := acceptance.BuildTestData(t, "data.azapi_resource_action", "test")
r := ActionDataSource{}

data.DataSourceTest(t, []resource.TestStep{
Expand Down
80 changes: 80 additions & 0 deletions internal/services/azapi_resource_action_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ func TestAccActionResource_providerAction(t *testing.T) {
})
}

func TestAccActionResource_nonstandardLRO(t *testing.T) {
data := acceptance.BuildTestData(t, "azapi_resource_action", "test")
r := ActionResource{}

data.DataSourceTest(t, []resource.TestStep{
{
Config: r.nonstandardLRO(data),
Check: resource.ComposeTestCheckFunc(),
},
})
}

func (r ActionResource) basic(data acceptance.TestData) string {
return fmt.Sprintf(`
%[1]s
Expand Down Expand Up @@ -107,3 +119,71 @@ resource "azapi_resource_action" "test" {
}
`
}

func (r ActionResource) nonstandardLRO(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}
resource "azurerm_resource_group" "test" {
name = "acctestrg-%[2]s"
location = "%[1]s"
}
resource "azurerm_storage_account" "test" {
name = "acctestsa%[2]s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "LRS"
}
resource "azurerm_service_plan" "test" {
name = "acctestsp%[2]s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
os_type = "Windows"
sku_name = "Y1"
}
resource "azurerm_windows_function_app" "test" {
name = "acctestfa%[2]s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
storage_account_name = azurerm_storage_account.test.name
storage_account_access_key = azurerm_storage_account.test.primary_access_key
service_plan_id = azurerm_service_plan.test.id
site_config {}
}
data "azapi_resource_id" "host" {
type = "Microsoft.Web/sites/host@2022-03-01"
parent_id = azurerm_windows_function_app.test.id
name = "default"
}
data "azapi_resource_id" "functionKey" {
type = "Microsoft.Web/sites/host/functionKeys@2022-03-01"
parent_id = data.azapi_resource_id.host.id
name = "tf_key"
}
resource "azapi_resource_action" "test" {
type = "Microsoft.Web/sites/host/functionKeys@2022-03-01"
resource_id = data.azapi_resource_id.functionKey.id
method = "PUT"
response_export_values = ["*"]
body = jsonencode({
properties = {
name = "test_key"
value = "test_value"
}
})
}
`, data.LocationPrimary, data.RandomStringOfLength(10))
}
63 changes: 63 additions & 0 deletions internal/services/azapi_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,19 @@ func TestAccGenericResource_ignoreChangesArray(t *testing.T) {
})
}

func TestAccGenericResource_nonstandardLRO(t *testing.T) {
data := acceptance.BuildTestData(t, "azapi_resource", "test")
r := GenericResource{}
data.ResourceTest(t, r, []resource.TestStep{
{
Config: r.nonstandardLRO(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
})
}

func (GenericResource) Exists(ctx context.Context, client *clients.Client, state *terraform.InstanceState) (*bool, error) {
resourceType := state.Attributes["type"]
id, err := parse.ResourceIDWithResourceType(state.ID, resourceType)
Expand Down Expand Up @@ -1299,6 +1312,56 @@ resource "azapi_update_resource" "test" {
`, r.template(data), data.RandomInt())
}

func (r GenericResource) nonstandardLRO(data acceptance.TestData) string {
return fmt.Sprintf(`
%[1]s
resource "azurerm_storage_account" "test" {
name = "acctestsa%[2]s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "LRS"
}
resource "azurerm_storage_container" "test" {
name = "acctestsc%[2]s"
storage_account_name = azurerm_storage_account.test.name
}
resource "azapi_resource" "test" {
type = "Microsoft.CostManagement/exports@2022-10-01"
name = "acctest%[2]s"
parent_id = azurerm_resource_group.test.id
body = jsonencode({
properties = {
schedule = {
recurrence = "Monthly"
recurrencePeriod = {
from = "2030-12-29T00:00:00Z"
to = "2030-12-30T00:00:00Z"
}
status = "Inactive"
}
definition = {
timeframe = "TheLastMonth"
type = "Usage"
}
format = "Csv"
deliveryInfo = {
destination = {
container = azurerm_storage_container.test.name
resourceId = azurerm_storage_account.test.id
}
}
}
})
}
`, r.template(data), data.RandomStringOfLength(10))
}

func (GenericResource) template(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
Expand Down

0 comments on commit 19e4772

Please sign in to comment.