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

Adds service_principal_object_id attribute to data_source_arm_client_config #175

Merged
merged 2 commits into from
Jul 20, 2017

Conversation

whiskeyjay
Copy link
Contributor

The service principal ID is needed for granting a SP access to certain resources. For example, adding secrets to KeyVault and give access to the SP in the configuration (although currently the TF KeyVault resource doesn't support adding secrets yet). With this new attribute uses won't have to manually copy the object ID of the SP from portal and hard code it in the configuration file.

Copy link
Member

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

Hey @whiskeyjay

Thanks for this - one small question but this otherwise LGTM :)

@@ -43,6 +46,22 @@ func testAzureRMClientConfigAttr(name, key, value string) resource.TestCheckFunc
}
}

func testAzureRMClientConfigGUIDAttr(name, key string) resource.TestCheckFunc {
return func(s *terraform.State) error {
r, err := regexp.Compile("^[A-Fa-f0-9]{8}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{12}$")
Copy link
Member

Choose a reason for hiding this comment

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

would it make more sense to use this method in the go.uuid library (which is already vendored) for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it is easier this way because I don't have to extract the attribute value from state myself?

Copy link
Member

Choose a reason for hiding this comment

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

fair point - it's not a blocker by any means - just seeing if we can reuse the UUID parser if it made sense :)

@tombuildsstuff
Copy link
Member

Tests pass:

$ TF_ACC=1 envchain azurerm go test ./azurerm -v -timeout 120m -run TestAccAzureRMClientConfig
=== RUN   TestAccAzureRMClientConfig_basic
--- PASS: TestAccAzureRMClientConfig_basic (11.59s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	11.620s

@tombuildsstuff tombuildsstuff merged commit 905fee3 into hashicorp:master Jul 20, 2017
tombuildsstuff added a commit that referenced this pull request Jul 20, 2017
@ghost
Copy link

ghost commented Apr 1, 2020

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 👉 hashibot-feedback@hashicorp.com. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants