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

New data source 'azuread_domains' #27

Merged
merged 7 commits into from
Jan 28, 2019

Conversation

tiwood
Copy link
Contributor

@tiwood tiwood commented Jan 22, 2019

This PR adds a new data source for Domains registered in Azure AD.

  • New data source 'azuread_domains'
  • Add markdown documentation
  • Add tests

Example Usage

// This gets all **verified** domains currently registered in Azure AD
data "azuread_domains" "my_domains" {}

// This gets all domains currently registered in Azure AD
data "azuread_domains" "my_domains" {
 include_unverified = true
}

// This gets only the default domain registered in Azure AD
data "azuread_domains" "default_domain" {
 only_default = true
}

// This will return only the tenant root domain (foo.onmicrosoft.com)
data "azuread_domains" "my_root_domain" {
 only_initial = true
}

Example Output

domains = [
    {
        authentication_type = "Managed",
        domain_name = hashicorp.onmicrosoft.com,
        is_default = 1,
        is_initial = 1,
        is_verified = 1
    }
]

Use cases

  • This will be helpful to create objects where the user principal name is required (say [email protected]) and the domain suffix is not known during runtime (testing for example)

@tombuildsstuff
Copy link
Contributor

hey @tiwood

Thanks for this PR :)

I've spent a little bit of time playing with this and on the whole this looks pretty good - however I'm wondering if we want to expose a couple of extra filters here? I've put together this gist showing what I mean - what do you think?

Thanks

@tiwood
Copy link
Contributor Author

tiwood commented Jan 24, 2019

hey @tiwood

Thanks for this PR :)

I've spent a little bit of time playing with this and on the whole this looks pretty good - however I'm wondering if we want to expose a couple of extra filters here? I've put together this gist showing what I mean - what do you think?

Thanks

Hey @tombuildsstuff,

I'm still thinking about what the default output and the parameters should be, as I'm not 100% sold on only filters, as each domain can have multiple of the "only properties" at once.

What do you think about the following:

  • azuread_domains without params will only return verified domains
  • There will be an option to include_unverified (Conflicts with only_tenant_domain)
  • There will be an option to only_tenant_domain (Conflicts with include_unverified)
  • The following properties will be included in the output for each domain:
    is_verified is_default authentication_type

I think this is easier to understand, WDYT?

@tombuildsstuff
Copy link
Contributor

@tiwood include sounds good 👍 I was otherwise going to suggest filter (but it's the same thought)

The only question I've got is around the tenant_domain field, I feel like it'd be better to consistently make this is_initial - WDYT?

@ghost ghost removed the waiting-response label Jan 24, 2019
@tiwood
Copy link
Contributor Author

tiwood commented Jan 25, 2019

@tombuildsstuff, I've changed the resource like I proposed and updated the original description accordingly.

For consistency I've renamed the attribute only_tenant_domain to only_initial.

Again a short summary:

  • without attributes the data source will return all VERIFIED domains registered in AAD.
  • if only_initial is set to true only the initial tenant domain is returned (the foo.onmicrosoft.com domain)
  • if you set include_unverified to true unverified domains will be included in the output.

@katbyte
Copy link
Collaborator

katbyte commented Jan 25, 2019

Thanks @tiwood,

I think you may want to mark the only_initial as conflicting with include_unverified.

Also only_default may be another useful filter to add?

@tiwood
Copy link
Contributor Author

tiwood commented Jan 25, 2019

@katbyte, they actually may be use cases where you want to create objects in the default domain, therefore I've added the option to set only_default.

Additionally I've added the ConflictsWith validation:

  • include_unverified conflicts with only_default and only_initial as default and initial domains have to be verified by design.
  • only_defaultand only_initialare conflicting with each other.

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @tiwood LGTM now 👍

katbyte added a commit that referenced this pull request Jan 28, 2019
@katbyte katbyte merged commit 2d21a89 into hashicorp:master Jan 28, 2019
@ghost
Copy link

ghost commented Mar 5, 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 Mar 5, 2019
@tiwood tiwood deleted the ds_azuread_domains branch March 14, 2019 14:32
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.

3 participants