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

Can't add app_role value as null #130

Closed
guri-s opened this issue Jul 26, 2019 · 10 comments · Fixed by #157
Closed

Can't add app_role value as null #130

guri-s opened this issue Jul 26, 2019 · 10 comments · Fixed by #157

Comments

@guri-s
Copy link

guri-s commented Jul 26, 2019

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform 0.11.14 and AzureAD Provider 0.5.0

Affected Resource(s)

  • azuread_application

Terraform Configuration Files

resource "azuread_application" "xxxx" {
  name                       = "xxxxx"
  homepage                   = "https://sso.online.xxxxxxx.com"
  available_to_other_tenants = false
  oauth2_allow_implicit_flow = false

  reply_urls = [
    "https://sso.online.xxxxxxx.com,
  ]

  app_role = [
    {
      "allowed_member_types" = ["User"]
      "description"          = "msiam_access"
      "display_name"         = "msiam_access"
      "is_enabled"           = true
      "value"                = ""
    },
    {
      allowed_member_types = ["User"]
      "description"        = "User"
      "display_name"       = "User"
      "is_enabled"         = true
      "value"              = ""
    },
  ]
}

Debug Output

We should be able to set a null value for the app_role.
If I look at the app manifest in Azure the value is 'null'. But it is not allowed to be set in tf.
Below is from azure manifest of the app

"appRoles": [
		{
			"allowedMemberTypes": [
				"User"
			],
			"description": "User",
			"displayName": "User",
			"id": "xxxx-xx-xx-x--xxxx-xxx",
			"isEnabled": true,
			"lang": null,
			"origin": "Application",
			"value": null
		},
		{
			"allowedMemberTypes": [
				"User"
			],
			"description": "msiam_access",
			"displayName": "msiam_access",
			"id": "xxxxxx-xxxx-xxxx",
			"isEnabled": true,
			"lang": null,
			"origin": "Application",
			"value": null
		}
	],

Expected Behavior

We should be able to set the app_role as null

Actual Behavior

Error: azuread_application.xxxxx: "app_role.0.value" must not be empty

Steps to Reproduce

  1. terraform plan
@enorlando

This comment has been minimized.

@matsest
Copy link

matsest commented Aug 6, 2019

Looks like the error comes from the validation function NoEmptyStrings

https://github.com/terraform-providers/terraform-provider-azuread/blob/83d4ab741630a3bd1236b5c4863703ea982aad30/azuread/helpers/validate/strings.go#L9-L21

which is called in the definition of value

https://github.com/terraform-providers/terraform-provider-azuread/blob/7e185411a66e3c0d6556df86def092b16431ab66/azuread/resource_application.go#L139-L143

I guess we either need to have a different validation which allows for null (or "") in app_role.value.

@matsest
Copy link

matsest commented Aug 6, 2019

In addition, since you have
https://github.com/terraform-providers/terraform-provider-azuread/blob/4fdb3ae9dcf6a730a4fddc88b1f5cb0415ca1dc0/azuread/resource_application.go#L103-L105

you can't set id manually - it will automatically delete the approle and create a new with a computed id. We will need to be able to set this as a static id to not remove and create new app roles for each apply. This goes for the msiam_access app roles but also other new ones.

@guri-s

This comment has been minimized.

@enorlando

This comment has been minimized.

@enorlando

This comment has been minimized.

@guri-s

This comment has been minimized.

@katbyte katbyte added this to the v0.7.0 milestone Sep 19, 2019
@katbyte
Copy link
Collaborator

katbyte commented Sep 19, 2019

@guri-s & @enorlando this seems like an easy enough fix to get in for v0.7.0

@ghost
Copy link

ghost commented Nov 10, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 10, 2019
@ghost
Copy link

ghost commented Nov 15, 2019

This has been released in version 0.7.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azuread" {
    version = "~> 0.7.0"
}
# ... other configuration ...

@ghost ghost unlocked this conversation Nov 15, 2019
@ghost ghost locked and limited conversation to collaborators Nov 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants