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

Add azurerm_federated_identity_credential resource #1666

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mdanylyuk
Copy link
Contributor

1664

PR Checklist


Description

Does this introduce a breaking change

  • YES
  • NO

Changes in the code support working with azurerm_federated_identity_credential resource.
This is required for replacing deprecated AAD Pod Identity with Azure AD workload identity with Azure Kubernetes Service

Testing

@LaurentLesle LaurentLesle added this to the 5.7.1 milestone Jun 23, 2023
@arnaudlh arnaudlh linked an issue Jun 23, 2023 that may be closed by this pull request
1 task
@arnaudlh
Copy link
Member

Thanks for the PR @mdanylyuk, can you please add an example which can be fully tested in standalone tests (add an AKS cluster, or a scenario that can be self-sufficient).

When testing your code, getting the following:

│ Error: Invalid index
│ 
│   on ../managed_identities.tf line 36, in resource "azurerm_federated_identity_credential" "fidc_aks":
│   36:   issuer              = each.value.issuer != null ? each.value.issuer : local.combined_objects_aks_clusters[try(each.value.aks_cluster.lz_key, local.client_config.landingzone_key)][each.value.aks_cluster.key].oidc_issuer_url
│     ├────────────────
│     │ each.value.aks_cluster.lz_key is "aks"
│     │ local.client_config.landingzone_key is "examples"
│     │ local.combined_objects_aks_clusters is object with 1 attribute "examples"
│ 
│ The given key does not identify an element in this collection value.
╵
╷
│ Error: Invalid index
│ 
│   on ../managed_identities.tf line 36, in resource "azurerm_federated_identity_credential" "fidc_aks":
│   36:   issuer              = each.value.issuer != null ? each.value.issuer : local.combined_objects_aks_clusters[try(each.value.aks_cluster.lz_key, local.client_config.landingzone_key)][each.value.aks_cluster.key].oidc_issuer_url
│     ├────────────────
│     │ each.value.aks_cluster.lz_key is "platform"
│     │ local.client_config.landingzone_key is "examples"
│     │ local.combined_objects_aks_clusters is object with 1 attribute "examples"
│ 
│ The given key does not identify an element in this collection value.
╵
Terraform plan return code: 1
Error 1 on or near line 386: Error running terraform plan; exiting with status 1

@arnaudlh arnaudlh removed this from the 5.7.1 milestone Jun 27, 2023
@mdanylyuk
Copy link
Contributor Author

Thanks for the PR @mdanylyuk, can you please add an example which can be fully tested in standalone tests (add an AKS cluster, or a scenario that can be self-sufficient).

When testing your code, getting the following:

│ Error: Invalid index
│ 
│   on ../managed_identities.tf line 36, in resource "azurerm_federated_identity_credential" "fidc_aks":
│   36:   issuer              = each.value.issuer != null ? each.value.issuer : local.combined_objects_aks_clusters[try(each.value.aks_cluster.lz_key, local.client_config.landingzone_key)][each.value.aks_cluster.key].oidc_issuer_url
│     ├────────────────
│     │ each.value.aks_cluster.lz_key is "aks"
│     │ local.client_config.landingzone_key is "examples"
│     │ local.combined_objects_aks_clusters is object with 1 attribute "examples"
│ 
│ The given key does not identify an element in this collection value.
╵
╷
│ Error: Invalid index
│ 
│   on ../managed_identities.tf line 36, in resource "azurerm_federated_identity_credential" "fidc_aks":
│   36:   issuer              = each.value.issuer != null ? each.value.issuer : local.combined_objects_aks_clusters[try(each.value.aks_cluster.lz_key, local.client_config.landingzone_key)][each.value.aks_cluster.key].oidc_issuer_url
│     ├────────────────
│     │ each.value.aks_cluster.lz_key is "platform"
│     │ local.client_config.landingzone_key is "examples"
│     │ local.combined_objects_aks_clusters is object with 1 attribute "examples"
│ 
│ The given key does not identify an element in this collection value.
╵
Terraform plan return code: 1
Error 1 on or near line 386: Error running terraform plan; exiting with status 1

Thanks for the comment,
I added new resources, VNet and AKS, in the example - please check one more time.

@LaurentLesle LaurentLesle requested a review from arnaudlh July 3, 2023 06:11
@arnaudlh
Copy link
Member

arnaudlh commented Jul 3, 2023

hi @mdanylyuk, thanks for the update, im now getting the following error:

╷
│ Error: creating Federated Identity Credential (Subscription: ""
│ Resource Group Name: "bfqy-rg-security-rg1"
│ User Assigned Identity Name: "bfqy-msi-poc-mi"
│ Federated Identity Credential Name: "bfqy-msi-dev"): managedidentities.ManagedIdentitiesClient#FederatedIdentityCredentialsCreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="BadRequest" Message="The request format was unexpected. Federated Identity Credential must specify 'issuer', 'subject' and 'audience'."
│ 
│   with module.example.azurerm_federated_identity_credential.fidc_aks["dev_poc_msi_region1"],
│   on ../managed_identities.tf line 29, in resource "azurerm_federated_identity_credential" "fidc_aks":
│   29: resource "azurerm_federated_identity_credential" "fidc_aks" {
│ 
│ creating Federated Identity Credential (Subscription: ""
│ Resource Group Name: "bfqy-rg-security-rg1"
│ User Assigned Identity Name: "bfqy-msi-poc-mi"
│ Federated Identity Credential Name: "bfqy-msi-dev"): managedidentities.ManagedIdentitiesClient#FederatedIdentityCredentialsCreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned
│ an error. Status=400 Code="BadRequest" Message="The request format was unexpected. Federated Identity Credential must specify 'issuer', 'subject' and 'audience'."
╵
╷
│ Error: creating Federated Identity Credential (Subscription: ""
│ Resource Group Name: "bfqy-rg-security-rg1"
│ User Assigned Identity Name: "bfqy-msi-poc-mi"
│ Federated Identity Credential Name: "bfqy-msi-qa"): managedidentities.ManagedIdentitiesClient#FederatedIdentityCredentialsCreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="BadRequest" Message="The request format was unexpected. Federated Identity Credential must specify 'issuer', 'subject' and 'audience'."
│ 
│   with module.example.azurerm_federated_identity_credential.fidc_aks["qa_poc_msi_region1"],
│   on ../managed_identities.tf line 29, in resource "azurerm_federated_identity_credential" "fidc_aks":
│   29: resource "azurerm_federated_identity_credential" "fidc_aks" {
│ 
│ creating Federated Identity Credential (Subscription: ""
│ Resource Group Name: "bfqy-rg-security-rg1"
│ User Assigned Identity Name: "bfqy-msi-poc-mi"
│ Federated Identity Credential Name: "bfqy-msi-qa"): managedidentities.ManagedIdentitiesClient#FederatedIdentityCredentialsCreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned
│ an error. Status=400 Code="BadRequest" Message="The request format was unexpected. Federated Identity Credential must specify 'issuer', 'subject' and 'audience'."
╵
Terraform apply return code: 1
Error 1 on or near line 110: Error running terraform apply; exiting with status 1

@mdanylyuk
Copy link
Contributor Author

mdanylyuk commented Jul 3, 2023

bfqy-msi-poc-mi

Hello @arnaudlh,
Could you please attach tfplan to see the whole changes?

@arnaudlh
Copy link
Member

mdanylyuk-md/federated-identity-credential

@mdanylyuk, the plan works fine, its an apply time error. It is easy to repro, I'm just running the example here :)
Also, you are missing the following outputs in aks

output "oidc_issuer_url" {
  value = azurerm_kubernetes_cluster.aks.oidc_issuer_url
}

@mdanylyuk
Copy link
Contributor Author

output "oidc_issuer_url"

@arnaudlh, many thanks! It's really my mistake (sorry but I worked in the multiple repos with it and missed adding)
Output is added now

managed_identities.tf Outdated Show resolved Hide resolved
managed_identities.tf Outdated Show resolved Hide resolved
audience = each.value.audience
issuer = each.value.issuer != null ? each.value.issuer : local.combined_objects_aks_clusters[try(each.value.aks_cluster.lz_key, local.client_config.landingzone_key)][each.value.aks_cluster.key].oidc_issuer_url
parent_id = module.managed_identities[each.value.managed_identity_key].id
subject = each.value.subject != null ? each.value.subject : try("system:serviceaccount:${each.value.kubernetes.namespace}:${each.value.kubernetes.service_account}", null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still getting errors for the examples for the managed_identities = {
poc = { }

with the following
│ Federated Identity Credential Name: "xxgi-msi-dev"): managedidentities.ManagedIdentitiesClient#FederatedIdentityCredentialsCreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error.
│ Status=400 Code="BadRequest" Message="The request format was unexpected. Federated Identity Credential must specify 'issuer', 'subject' and 'audience'."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subject cant be null as per: https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/federated_identity_credential#subject

@arnaudlh
I know that it's a required parameter. But I set "null" for failing run in a case when a user does not provide "subject" or "kubernetes" parameters. If you are not ok with this logic I will remove "null"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets fail if we dont have a value, its better to fail at plan-time than at apply-time (the error message above).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets fail if we dont have a value, its better to fail at plan-time than at apply-time (the error message above).

But yes, with this condition plan will fail if you missed one of the parameters "subject" or "kubernetes"

##[error]╷
│ Error: Missing required argument
│ 
│   with module.solution.azurerm_federated_identity_credential.fidc_aks["dev_poc_dev"],
│   on .terraform/modules/solution/managed_identities.tf line 38, in resource "azurerm_federated_identity_credential" "fidc_aks":
│   38:   subject             = each.value.subject != null ? each.value.subject : try("system:serviceaccount:${each.value.kubernetes.namespace}:${each.value.kubernetes.service_account}", null)
│ 
│ The argument "subject" is required, but no definition was found.
╵

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still getting errors for the examples for the managed_identities = { poc = { }

with the following │ Federated Identity Credential Name: "xxgi-msi-dev"): managedidentities.ManagedIdentitiesClient#FederatedIdentityCredentialsCreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. │ Status=400 Code="BadRequest" Message="The request format was unexpected. Federated Identity Credential must specify 'issuer', 'subject' and 'audience'."

I've just tested on my environment and it works fine.
Please attach plan - want to see how it tries to create.

@najvv najvv mentioned this pull request Aug 28, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Add supporting azurerm_federated_identity_credential resources
3 participants