hashicorp / terraform-provider-azurerm

Terraform provider for Azure Resource Manager
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs
Mozilla Public License 2.0
4.62k stars 4.66k forks source link

Removing workflow step from Logic App results in actions executed in wrong order #15518

Open wiktorn opened 2 years ago

wiktorn commented 2 years ago

Community Note

Terraform (and AzureRM Provider) Version

$ terraform -v
Terraform v1.1.6
on linux_amd64

Azurerm provider built from main branch.

Affected Resource(s)

Terraform Configuration Files

Initial configuration

Logic app with three steps executed one after another. To ensure that steps will be created in proper order, runAfter refers to the name of the preceding resource.


resource "azurerm_logic_app_action_custom" "init" {
  name         = "init"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "variables": [
            {
                "name": "var1",
                "type": "Integer",
                "value": 1
            }
        ]
    },
    "runAfter": {},
    "type": "InitializeVariable"
}
BODY
}

resource "azurerm_logic_app_action_custom" "add_one" {
  name         = "add_one"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "name": "var1",
        "value": 1
    },
    "runAfter": {
        "${azurerm_logic_app_action_custom.init.name}": ["Succeeded"]
    },
    "type": "IncrementVariable"
}
BODY
}

resource "azurerm_logic_app_action_custom" "add_two" {
  name         = "add_two"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "name": "var1",
        "value": 2
    },
    "runAfter": {
        "${azurerm_logic_app_action_custom.add_one.name}": ["Succeeded"]
    },
    "type": "IncrementVariable"
}
BODY
}

This applies without error

Modification of Logic App

Let's assume I want to remove the middle step. I need to also update add_two step so it depends on init step.

resource "azurerm_logic_app_action_custom" "init" {
  name         = "init"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "variables": [
            {
                "name": "var1",
                "type": "Integer",
                "value": 1
            }
        ]
    },
    "runAfter": {},
    "type": "InitializeVariable"
}
BODY
}

resource "azurerm_logic_app_action_custom" "add_two" {
  name         = "add_two"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "name": "var1",
        "value": 2
    },
    "runAfter": {
        "${azurerm_logic_app_action_custom.init.name}": ["Succeeded"]
    },
    "type": "IncrementVariable"
}
BODY
}

Expected Behaviour

Change applies without error. So first the last step is updated to reference the init step and only then add_two step is removed.

Actual Behaviour

Error: removing Action Action: (Name "add_one" / Workflow Name "acctestlaw-220219214631053460" / Resource Group "acctestRG-220219214631053460"): removing Action "add_one" from Logic App Workspace "acctestlaw-220219214631053460" (Resource Group "acctestRG-220219214631053460"): logic.WorkflowsClient#CreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="InvalidTemplate" Message="The template validation failed: 'The 'runAfter' property of template action 'add_two' at line '1' and column '182' contains non-existent action: 'add_one'.'."

Steps to Reproduce

I've created a test case to simplify debugging that

References

The issue here is somewhat connected with those two: #5197 and #7542. Although I really enjoy to composability given by separate workflow step resources it's painful when you cannot easily change the structure of the logic app. I don't understand why here referenced object is removed before reference is updated to something else. But if this is an issue that doesn't have easy solution, then being able to provide full workflow definition would be a great fallback.

We have just migrated away from azurerm_management_group_template_deployment and enjoy easier workflow composition as wel as better validation.

neil-yechenwei commented 2 years ago

@wiktorn , thanks for raising this issue. Seems Service API complains it has to remove the association between logic app action customs before destroy. So suggest use "create_before_destroy" to resolve this issue. Below is example.

provider "azurerm" {
  features {}
}

resource "azurerm_resource_group" "test" {
  name     = "acctestRG-logicappac-test01"
  location = "West Europe"
}

resource "azurerm_logic_app_workflow" "test" {
  name                = "acctestlaw-test01"
  location            = azurerm_resource_group.test.location
  resource_group_name = azurerm_resource_group.test.name
}

resource "azurerm_logic_app_action_custom" "init" {
  name         = "init"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "variables": [
            {
                "name": "var1",
                "type": "Integer",
                "value": 1
            }
        ]
    },
    "runAfter": {},
    "type": "InitializeVariable"
}
BODY

  lifecycle {
    create_before_destroy = true
  }
}

resource "azurerm_logic_app_action_custom" "add_one" {
  name         = "add_one"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "name": "var1",
        "value": 1
    },
    "runAfter": {
        "${azurerm_logic_app_action_custom.init.name}": ["Succeeded"]
    },
    "type": "IncrementVariable"
}
BODY

  lifecycle {
    create_before_destroy = true
  }
}

resource "azurerm_logic_app_action_custom" "add_two" {
  name         = "add_two"
  logic_app_id = azurerm_logic_app_workflow.test.id

  body = <<BODY
{
    "inputs": {
        "name": "var1",
        "value": 2
    },
    "runAfter": {
        "${azurerm_logic_app_action_custom.add_one.name}": ["Succeeded"]
    },
    "type": "IncrementVariable"
}
BODY

  lifecycle {
    create_before_destroy = true
  }
}
wiktorn commented 2 years ago

Thanks @neil-yechenwei, looks like lifecycle indeed solves this issue. I was fearing that this might result in undefined behavior, as terraform will try to create first step and the name already exists. But this actually results in error (step already exists), and only in the rare situation, if you are reusing step names.

Maybe this lifecycle is worth documenting in azurerm_logic_app_action_* resources? Shall I create a PR for that?

neil-yechenwei commented 2 years ago

Sure. Thank you for raising PR.

wiktorn commented 2 years ago

Created PR (#15541). As I was testing this approach I noticed that there are still some edge cases which will not be solved by lifecycle (example: changing the step name with variable initialization).

The workaround that came to my mind regarding this is to just add manually "serial number" to workflow name and update it on each structural change to avoid such issues. I'm not sure if this approach is worth documenting though.

szymonbr commented 2 years ago

Http action uses run_after block argument in the resource definition so terraform could be aware dependencies of actions. I think the same solution should be applied for custom action .

poweryaki commented 4 months ago

As mentioned above, the lifecycle and reliance on other references or depends_on aren't able to handle all situations. The other major issue is that this doesn't work at scale. I.e. you can't use a for_each to create multiple custom actions say via a module as each action_custom resource won't know about 'runAfter' in the json. As @szymonbr commented, it would be nice to add run_after to the azurerm_logic_app_action_custom resource.

At the moment I am able to get around this whole problem by using separate create and destroy null resources which run az cli commands to monitor the status of dependencies (i.e. pause the creation/destruction of the custom action until the referenced runAfter exists or has been destroyed):

resource "null_resource" "action_custom_create_check" {

  triggers = {
      action_custom_name   = var.action_custom_name
      logic_app_name           = var.logic_app_name
      resource_group_name = var.resource_group_name
      run_after                      = var.run_after
      subscription_id            = var.subscription_id
  }

  provisioner "local-exec" {
    when = create
    interpreter = ["pwsh", "-Command"]
    command     = <THIS RUNS A LOOP TO CHECK IF THE 'RUN AFTER' ACTION HAS BEEN CREATED>

}

# This will create after the above, preventing the resource being created before the 'runAfter' dependency
resource "azurerm_logic_app_action_custom" "action" {
  name         = var.action_custom_name
  logic_app_id = var.logic_app_id
  body         = var.action_custom_body

  depends_on = [ null_resource.action_custom_create_check ]
}

resource "null_resource" "action_custom_destroy_check {

  triggers = {
    action_custom_name   = var.action_custom_name
    logic_app_name          = var.logic_app_name
    resource_group_name = var.resource_group_name
    run_before                   = var.run_before
    subscription_id            = var.subscription_id
  }

  provisioner "local-exec" {
    when = destroy
    interpreter = ["pwsh", "-Command"]
    command     = <THIS RUNS A LOOP TO CHECK IF THE 'RUN BEFORE' ACTION HAS BEEN DESTROYED>

   depends_on = [ azurerm_logic_app_action_custom.action ]
}

The above can be be used as a child module for creating a azurerm_logic_app_workflow. All custom actions will then sit in a loop during a terraform run until those with no dependencies are created or destroyed (depending on run_after or run_before)