Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AzureAD creating an ID with applications/GUID instead just GUID with latest version v2.44.1 #1227

Open
arun-bejjanki-kr opened this issue Oct 25, 2023 · 26 comments

Comments

@arun-bejjanki-kr
Copy link

With v2.44.1 version of azuread provider id is creating with applications/GUID instead GUID.

Versions

terraform ">= 1.4.0, < 1.5.5"
azuread >= 2.20.0, < 2.44.0"
azurerm >= 3.11.0, < 4.0.0"

Expected Behavior
resource "azuread_application" "application" {
id = ""
. . .
. . .

Actual Behavior
resource "azuread_application" "application" {
id = "/applications/"
. . .
. . .

@manicminer
Copy link
Contributor

Hi @arun-bejjanki-kr, thanks for raising this issue. This is due to an ID migration that happened in the latest release (see #1214 and #1221), to accommodate some new application resources. If you're using the id attribute in your configuration to reference the object ID, the correct attribute to reference is object_id, which hasn't changed.

Please let me know if this resolves your issue, if not please post your config and I'll be happy to take a look and see whether there's a bug lurking in the latest version. Thanks!

@nuryupin-kr
Copy link

Is id attribute now considered 'legacy' and will be deprecated?

@manicminer
Copy link
Contributor

manicminer commented Oct 26, 2023

No, the id attribute is an internal implementation and its value is not guaranteed to be in any particular format

@jgschwendswica
Copy link

jgschwendswica commented Oct 27, 2023

@manicminer one thing I've noticed with the new id:
According to the provider documentation the import statements expect this format: /applications/00000000-0000-0000-0000-000000000000/subResource/....

However, the Id returned by azuread_application_registration is in the format application/00000000-0000-0000-0000-000000000000 which leads to constant changes in the plan.

As a workaround to get a consistant plan I just prefixed my reference to the id with '/':

resource "azuread_application_redirect_uris" "next_new" {
application_id = "/${azuread_application_registration.next_new.id}"
type = "Web"

redirect_uris = [
"https://yoururl.com/oauth2/callback"
]
}

This could be a quick win to fix this

@manicminer
Copy link
Contributor

@jgschwendswica Thanks for the insight. I wasn't able to reproduce that issue where the ID is missing the first character, would you be able to provide more details on the steps you took that led to that? Thanks!

@jgschwendswica
Copy link

Looks like I missed the leading forward slash during the import of my azuread_application_registration.
Since I was able to import it without any error, I didn't even noticed it until now :/
So no Issue at all :)

@manicminer
Copy link
Contributor

Thanks for the feedback! We should be catching that at import time 👍

@kansberry-kr
Copy link

I have a question on this too. Are you saying that the id of the resource in the Terraform state file is different then the id of the resource in Azure? It seems like the state file should contain exactly what Azure API returns for this value.

@manicminer
Copy link
Contributor

manicminer commented Oct 30, 2023

Whilst we often use the same resource ID that's provided by Azure, we sometimes elect to augment them or otherwise change them. In the AzureRM provider we make a best effort to match a resource ID with its Resource Manager URI, with other providers like AzureAD it's not as straightforward as there often isn't a unique addressable ID provided by data plane APIs like MS Graph, and so we're moving to commoditize resource IDs in this provider such that they can be managed more easily using the same libraries using similar paradigms. This aids development of the provider going forward. It's also why we have for a long time exposed explicit attributes like object_id which are documented as the appropriate attribute to use when an object ID is needed. Note that several resources managed by this provider, and many future resources, do not have a unique ID, or the unique ID is scoped or otherwise complex and has no useful meaning outside the provider.

@stevehipwell
Copy link

@manicminer it looks like the v2.44.1 release should have had a SemVer major bump as these changes are breaking; we're having to refactor our azuread_service_principal resources which used to have application_id set from the azuread_application application_id following the previous documented pattern.

Note that the latest docs for azuread_service_principal have completely removed reference to the application_id input which should be deprecated and have the client_id input incorrectly set as required. The VS Code extension is currently throwing errors about the missing application_id input.

@manicminer
Copy link
Contributor

manicminer commented Nov 13, 2023

@stevehipwell Whilst the deprecated application_id property was removed from documentation for both the azuread_application and azuread_service_principal resources, in the provider schema they do still exist as deprecated properties.

In the azuread_service_principal resource, the application_id and client_id properties are both accepted as mutually exclusive properties.

As such, whilst the documentation now reflects only the new properties, this isn't a breaking change as existing configurations will continue to work.

@stevehipwell
Copy link

As such, whilst the documentation now reflects only the new properties, this isn't a breaking change as existing configurations will continue to work.

@manicminer the issue here is that they won't continue to work, existing configurations break due to the addition of the applications/ prefix to the application_id.

@manicminer
Copy link
Contributor

manicminer commented Nov 13, 2023

@stevehipwell Sorry, I misread the context of your message, to which I was replying.

I understand that some configurations may have broken as a result of incorrect usage of the id attribute for an azuread_application resource, however the correct attribute to use in this context is, and always has been, object_id. I do also appreciate that a previous documentation bug for the azuread_application_password resource did mistakenly refer to the id attribute in a single example, though I note that this was a single occurrence and all other documentation specifically points to the object_id attribute, which has not changed.

There is a state migration in place to gracefully handle the changing of the id attribute when upgrading to the latest provider versions, which avoids causing persistent diffs or other breaking side effects. If you have an existing configuration that is broken specifically due to format change of the id attribute, please post your configuration and associated Terraform output and I'll be happy to investigate further.

@stevehipwell
Copy link

@manicminer the headline documentation (first example for the resource) I linked in my first comment was using application_id which immediately broke due to this change.

@manicminer
Copy link
Contributor

@stevehipwell Could you elaborate further on the breakage you have experienced? The application_id attribute for the azuread_application resource has not changed. Yes it is now deprecated, but it is still being set to the same value as it always was. Additionally the application_id property for the azuread_service_principal resource still exists in deprecated form, so will continue to work.

The following configuration continues to work as expected?

terraform {
  required_providers {
    azuread = {
      source  = "hashicorp/azuread"
      version = "~>2.45.0"
    }
  }
}

provider "azuread" {}

resource "azuread_application" "test" {
  display_name = "aaatest-appid"
}

resource "azuread_service_principal" "test" {
  application_id = azuread_application.test.application_id
}
Screenshot 2023-11-13 at 20 06 48

@nfrappart
Copy link

Hello,
I'm having a similar issue with the application_id property requiring /applications/ prefix.
here is my case.
I come from:

resource "azuread_application_password" "owner" {
  application_object_id = azuread_application.owner.object_id
  end_date_relative = "24h"
  display_name = "bootstrap secret vaild 24h, do not delete"

which gets me the following warning:

│ Warning: Argument is deprecated
│
│   with azuread_application_password.owner,
│   on iam.tf line 39, in resource "azuread_application_password" "owner":
│   39:   application_object_id = azuread_application.owner.object_id
│
│ The `application_object_id` property has been replaced with the `application_id` property and will be removed in version 3.0 of the AzureAD provider
│
│ (and 3 more similar warnings elsewhere)

Fine, let's update this to:

resource "azuread_application_password" "owner" {
  application_id = azuread_application.owner.object_id
  end_date_relative = "24h"
  display_name = "bootstrap secret vaild 24h, do not delete"

but then, I get:

│ Error: parsing "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx": parsing the Application ID: the number of segments didn't match

│ Expected a Application ID that matched (containing 2 segments):

│ > /applications/00000000-0000-0000-0000-000000000000

│ However this value was provided (which was parsed into 0 segments):

│ > xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

│ The following Segments are expected:

│ * Segment 0 - this should be the literal value "applications"
│ * Segment 1 - this should be the user specified value for this applicationId [for example "00000000-0000-0000-0000-000000000000"]

│ The following Segments were parsed:

│ * Segment 0 - not found
│ * Segment 1 - not found


│   with azuread_application_password.owner,
│   on iam.tf line 39, in resource "azuread_application_password" "owner":
│   39:   application_id = azuread_application.owner.object_id

So it appears application_id property is expecting /applications/00000000-0000-0000-0000-000000000000whilst application_object_id was happy with 00000000-0000-0000-0000-000000000000.

Now, having looked at my state file, it appears that the id property is actually prefixing the /applications expected by the resource azuread_application_password. But after reading your explanation about the id attribute, I'm not sure we can safely use it. However, looking at the recently added azuread_application_registration, a seemingly identitcal id appears to have been added for this very usage:

id - The resource ID for the application, for use when referencing this resource in your Terraform configuration.

But there is no reference of the id attribute in the azuread_application documentation.

Could you please clarify?

Thanks for your help!

@manicminer
Copy link
Contributor

manicminer commented Nov 14, 2023

@nfrappart As per the docs, you'll need to consume the id attribute of your azuread_application resource, e.g.

resource "azuread_application_password" "owner" {
  application_id = azuread_application.owner.id
}

To clarify my earlier point about the id property: it's intended use is for dependency-based referencing by other resources within your Terraform configuration. It's actual value, however, is opaque and should not be relied upon for other purposes - for those we have additional attributes such as object_id and client_id (formerly application_id), the values of which are contractual and can be safely used when you need them.

I do realise that all these IDs and complex values can get pretty confusing, that is actually one of the main reasons behind this refactoring. Another reason is to establish a convention within the provider for identifying resources, because at the moment we have a mix of object IDs (which is a real AAD primitive), UUIDs which are not object IDs, and complex concatenations unique to each resource. AAD does not offer a single commodity resource identifier across all APIs.

Our documentation is the primary reference here, if you check the Examples for each of these resources you will find the latest recommended usage for each resource respectively. Hope this helps!

@nfrappart
Copy link

thanks for the lightning fast feedback @manicminer
This confirms my thoughts. I agree with the complexity go all the ids when working with Entra. I got a lot of colleagues getting confuse between, application_ic, object_id, client_id, uuid etc. Your refactoring is very welcome 👍🏻

Can I suggest however that the documentation for azuread_application gets updated to include the id attribute reference? It's present in the azuread_application_registration page (where I found it) but not for azuread_application. I think it would clarify for a lot of people to have it there too, as the description is pretty straightforward.

thanks again.

@manicminer
Copy link
Contributor

Can I suggest however that the documentation for azuread_application gets updated to include the id attribute reference? It's present in the azuread_application_registration page (where I found it) but not for azuread_application. I think it would clarify for a lot of people to have it there too, as the description is pretty straightforward.

Thanks for the suggestion, that's a good shout and we should be consistent here

@stockmaj
Copy link

I have tried converting my federated identity credential to use application_id instead of application_object_id with the right hand side of the assignment using data.azuread_application.application.id, .object_id, .client_id, and .application_id and in each case I get the error referenced in @nfrappart's post 2 days ago. At this time, I cannot find any alternative than continuing to use the application_object_id property.

This is creating new resources, not updating existing resources.

data "azuread_application" "application" {
  display_name = "test"
}

resource "azuread_application_federated_identity_credential" "application_federated_identity_credential" {

  application_id = data.azuread_application.application.id
  display_name          = "test"
  issuer                = azurerm_kubernetes_cluster.cluster.oidc_issuer_url
  subject               = "system:serviceaccount:test:test"
  audiences             = ["api://AzureADTokenExchange"]
}

@manicminer
Copy link
Contributor

@stockmaj Thanks for reporting. The azuread_application data source doesn't yet have an updated ID format, we'll fix this in the next release.

@stockmaj
Copy link

@stockmaj Thanks for reporting. The azuread_application data source doesn't yet have an updated ID format, we'll fix this in the next release.

Thanks!

@manicminer
Copy link
Contributor

@arun-bejjanki-kr Can you confirm if my earlier comment helps to resolve your issue? Thanks!

@jayctran
Copy link

Running into the same issue when using the client_id from azurerm_user_assigned_identity.

resource "azurerm_user_assigned_identity" "example" {
  name                = "uami-example"
  resource_group_name = "rg-example"
  location            = "australiaeast"
}

resource "azuread_application_federated_identity_credential" "aks_stepcertificates_federatedidentity" {
  application_id = "/applications/${azurerm_user_assigned_identity.example.client_id}"
  display_name   = "example"
  description    = "example"
  audiences      = ["api://AzureADTokenExchange"]
  issuer         = var.oidc_issuer_url
  subject        = var.subject
}

With future fixes what is the format we should be expecting in azuread_application_federated_identity_credential.application_id?

ie. will it be just the GUID or will the two providers somehow sync on what to use for application_id

@manicminer
Copy link
Contributor

@jayctran Due to historic ambiguous naming in Azure, the term "application ID" means different things depending on where you look. We are standardising on the term "client ID" for an application's GUID but to avoid issuing any breaking changes this means in the interim we have multiple attributes with borrowed naming. Please refer to the provider documentation for the best examples of which attributes to use, cross-referencing with the attribute descriptions if you need to brush up on what they mean. The azuread_application_federated_identity_credential resource doc shows the correct usage.

@jayctran
Copy link

jayctran commented Feb 15, 2024

Thanks @manicminer, yes I understand there'll be some differences. In my example I managed to get it working by prefixing "/applications/".

What I meant to say was using the Managed Identity module from azurerm, both client_id or principal_id attributes are just GUIDS, and there's no attributes with a prefix of "/applications/". I did end up getting it working by hardcoding the prefix but thought I'd flag the difference between the two providers as part of this issue.

EDIT: after looking into this more, please disregard everything I mentioned - there's actually azurerm_federated_identity_credential which is what I was after - has nothing to do with this particular issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants