hashicorp / terraform-provider-azuread

Terraform provider for Azure Active Directory
https://registry.terraform.io/providers/hashicorp/azuread/latest/docs
Mozilla Public License 2.0
419 stars 289 forks source link

Error: Retrieving Conditional Access Policy with object ID "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" #589

Open enorlando opened 2 years ago

enorlando commented 2 years ago

Community Note

Terraform (and AzureAD Provider) Version

Terraform v1.0.7

Affected Resource(s)

Terraform Configuration Files

resource "azuread_conditional_access_policy" "mfa-for-o365" {
  display_name = "MFA for O365"
  state        = "enabled"

  conditions {

    client_app_types = [
      "browser",
      "mobileAppsAndDesktopClients",
    ]

    applications {
      included_applications = ["Office365", ]
    }

   users {
      included_users = ["All"]
    }

    locations {
      included_locations = ["All", ]
    }

    platforms {
      included_platforms = ["all"]
    }
  }

  grant_controls {
    operator          = "OR"
    built_in_controls = ["mfa", ]
  }
}

Debug Output

Panic Output

Expected Behavior

Running a terraform plan should result in no changes to infrastructure

Actual Behavior

Error is produced

Error: Retrieving Conditional Access Policy with object ID "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"

with azuread_conditional_access_policy.mfa-for-o365,
on conditional-access-policy.tf line 336, in resource "azuread_conditional_access_policy" "mfa-for-o365":
336: resource "azuread_conditional_access_policy" "mfa-for-o365" {

ConditionalAccessPolicyClient.BaseClient.Get(): unexpected status 400 with OData error: BadRequest: 1037: The policy you requested contains preview features. Use the Beta endpoint to retrieve this policy.

Steps to Reproduce

  1. terraform plan

Important Factoids

Upon review of the portal, it seems that the Device state is causing the error. If I do not configure this setting, terraform plan will not produce an error

image

image

References

manicminer commented 2 years ago

Hi @enorlando, thanks for reporting this! Are you importing an existing policy, or have updated the policy outside of Terraform to enable the above preview feature?

enorlando commented 2 years ago

Hi @manicminer, thanks for the quick response. I imported the existing conditional access policy into terraform when the new resource was available in v2.2.0. Once updating the provider to v2.3.0, it errors out.

manicminer commented 2 years ago

That makes sense. In v2.2 we were using the beta API for conditional access policies, this was then changed in v2.3.

We're aiming to support the v1.0 API by default for all resources, and use the beta API for additional preview features where necessary, however it looks like this approach won't work for CA policies as the v1.0 API is refusing to serve the policy if preview features are used 🤦

We'll have a think and see if there a way to work around this. In the meantime you'll unfortunately be unable to reference or manage policies with preview features with Terraform.

kaovd commented 2 years ago

@manicminer Slightly confused here - Upstream still clearly states it is using the Beta endpoint? https://github.com/manicminer/hamilton/blob/main/msgraph/conditionalaccesspolicy.go#L22

Only other work around is developing seperate _preview resources / toggles if applicable although seems like a lot of work

manicminer commented 2 years ago

@kaovd We override the endpoint here: https://github.com/hashicorp/terraform-provider-azuread/blob/main/internal/services/conditionalaccess/client/client.go#L20

The problem of using a feature toggle is that it can only be implemented using an environment variable and not a configuration property, as the schema has to be declared by the provider before it's able to read the config.

kaovd commented 2 years ago

@kaovd We override the endpoint here: https://github.com/hashicorp/terraform-provider-azuread/blob/main/internal/services/conditionalaccess/client/client.go#L20

The problem of using a feature toggle is that it can only be implemented using an environment variable and not a configuration property, as the schema has to be declared by the provider before it's able to read the config.

Ah OK if Theres a local override easier to do env var implemenation

manicminer commented 2 years ago

If we were to feature toggle this resource, the other problem is that, because of the API incompatibility, it becomes a one-way switch. So opting into beta resources say provider-wide would preclude users from switching back without manually destroying the breaking resources. It would translate to a poor user experience, and hinging this behavior solely on an environment variable is non-intuitive.

Edit: To avoid any confusion; as a general problem this is something we're looking to solve! Some users wish to use the 1.0 API and others wish to use beta features, and in both cases some subset may want exclusions to their general policy. So we are looking to make this clearer and work out a way to satisfy these constraints.

kaovd commented 2 years ago

If we were to feature toggle this resource, the other problem is that, because of the API incompatibility, it becomes a one-way switch. So opting into beta resources say provider-wide would preclude users from switching back without manually destroying the breaking resources. It would translate to a poor user experience, and hinging this behavior solely on an environment variable is non-intuitive.

Edit: To avoid any confusion; as a general problem this is something we're looking to solve! Some users wish to use the 1.0 API and others wish to use beta features, and in both cases some subset may want exclusions to their general policy. So we are looking to make this clearer and work out a way to satisfy these constraints.

Well yep completely agree would be on users - as would rather provide the option with proper warning - although would end up requiring a new schema definition for blocks which are in preview, or could handle preview error api responses and explain to users they need to toggle for xyz feature. Considering whether having seperate preview resources where in demand would just be easier to do at this rate