Support app rule actions which shouldn't require 'expression' field

[at-k]

• a behavior of app rule api is different whether 'expression' field exists or not.
• e.g. AWS application

resource "onelogin_app_rules" "rule" {
  name    = "Test Developer"

  app_id = <aws app id>
  match  = "all"
  conditions {
    operator = "ri"
    source   = "has_role"
    value    = onelogin_roles.role.id
  }
  actions {
    action     = "set_role"
    // expression = ""
    value      = [
      "arn:aws:iam::xxxx:role/xxxx-role",
    ]
  }
}

• 'Map from Onelogin' is selected, because 'expression' field is post as "" string
  
• after this pr merged, 'expression' is set to nil when it is empty - 'From Existing' is selected (expected)
  
• action spec
• https://developers.onelogin.com/api-docs/2/app-rules/create-rule

[dcaponi]

Hey @at-k this has been a notorious issue for a while #16

My concern with this approach is that setting expression = "" is a valid use case for some and this change would prevent that entirely.

If your change fixes the issue, Im inclined to merge it to satisfy the majority of use cases where expression is either null or something that is not an empty string.

Do you have any thoughts around this?

[at-k]

Hi @dcaponi , I added action type translation on provider to keep both action w / and w/o expression available.

When action is 'set_role_from_existing', the provider set expression nil on Create/Update. In addition, when onelogin api return app rules with no expression field, the provider judges that the rule is 'set_role_from_existing' and translate it.

I'm not familiar with onelogin api specification, so I'm not sure this provider behavior satisfy the specification of api.

How do you think this approach?

[dcaponi]

Hi @at-k unfortunately that would not satisfy the specification. You have if act != "set_role_from_existing" { on line 35 and the action set_role_from_existing is not a valid App Rule Action.

I'm still trying to deep dive in here, but something is happening at the Terraform level I believe, where I'm getting "" for Expression when I don't include the field in my main.tf (as you show in your example). I suspect it has something to do with the way data is retrieved on a nested resource.

I'm looking for a workaround for this, but it may need to be escalated to Hashicorp. Thank you for your continued patience with the matter.

[at-k]

the action set_role_from_existing is not a valid App Rule Action.

@dcaponi Thank you for your review.

Yes I know the action is not exist actually. So I intend that the action 'set_role_from_existing' is translated into "set_role" before submitting to onelogin api.
But I forgot to add commit including action translation. I fixed and added commit to PR.

I checked provider behavior by following example code.

resource "onelogin_app_rules" "rule" {
  name    = "Test Developer 10"
  enabled = true

  app_id = "698140"
  match  = "all"
  conditions {
    operator = "ri"
    source   = "has_role"
    value    = onelogin_roles.role.id
  }
  actions {
    action     = "set_role_from_existing"
    //action     = "set_role"
    //expression = ".*"
    value      = [
      "arn:aws:iam::057575985710:role/benefit-developer",
      "arn:aws:iam::057575985710:role/benefit-admin",
    ]
  }
}

  # onelogin_app_rules.rule will be updated in-place
  ~ resource "onelogin_app_rules" "rule" {
        id       = "287740"
        name     = "Test Developer 10"
        # (4 unchanged attributes hidden)

      ~ actions {
          ~ action = "set_role" -> "set_role_from_existing"
          ~ value  = [
                "arn:aws:iam::057575985710:role/benefit-developer",
              + "arn:aws:iam::057575985710:role/benefit-admin",
            ]
        }

        # (1 unchanged block hidden)
    }

the action is registered as 'set_role_from_existing' on terraform state, but actual action sent to onelogin api is 'set_role' without 'expression' field.

The result is

Please review again.

[dcaponi]

@at-k I think I see where you're going with this. You want to add this "custom" action set_role_from_existing that doesn't use expression and so by using set_role_from_existing you're also asking to always set expression to nil.

If my understanding is correct, would you please replace the action field in the onelogin_app_rule.md description so others know about this. Here is what it should say, please feel free to change it if I am wrong about any of this.

* `action` - The action to apply. See [List Actions](https://developers.onelogin.com/api-docs/2/app-rules/list-conditions) for possible values. *Note*: The action `set_role_from_existing` may also be used, however doing so will always clear the `expression` field as it is not accepted when mapping a rule from existing roles.

Please confirm and I'll merge.

P.S. While this should solve the immediate problem, I think upon further analysis of the problem there will have to be some changes to the way the provider handles unpacking nested Terraform resources. In this case, it seems that when unpacking the app rule with a nested action resource, all the sub-fields are getting set to the "zero" value in Go which is incorrect. I did notice that unpacking each sub-resource individually seems to exhibit normal behavior. There will be a change coming that corrects this. Since the list of acceptable actions is dynamic, it's possible for set_role_from_existing to mean something different in the future so I don't want to rest on this for too long.

[at-k]

@dcaponi thank you for your review. As you say, this PR adds a custom action which is acceptable only on terraform code.

I also think this solution is just a temporary work around, and should be replaced by other approach based on onelogin api specification. So the other approach to make this feature available is welcome to me.

[dcaponi]

I'm going to merge this for now to unblock you. I have a suspicion that Hashicorp may need to change the way they handle "" values so we can make this behavior better.

Here's the issue I opened with them for reference hashicorp/terraform-plugin-sdk#741

Here's the PR I'm working on if you're interested #47

[at-k]

@dcaponi I found that the current available release version is still v0.1.12 published 24 days ago.

$ tf init 
...
onelogin/onelogin: no available releases match the given constraints 0.1.14

https://registry.terraform.io/providers/onelogin/onelogin/latest

I don't know how to register the release of provider to terraform official registry, but can you update them to make it available from terraform?

[dcaponi]

@at-k sorry about that 😅 something was wrong with the publish action I set up. v0.1.14 should be up now.