Add terraform module for azure auto discovery#62145
Conversation
3f101f4 to
082b786
Compare
| - Azure user-assigned managed identity for Teleport Discovery Service to use. | ||
| - Azure federated identity credential that trusts the Teleport proxy as an issuer. | ||
| - Azure custom role definition and assignment that grant the minimum VM discovery and install permissions. | ||
| - Teleport `discovery_config` cluster resource that configures Teleport for Azure VM discovery. | ||
| - Teleport `integration` cluster resource for Azure OIDC. | ||
| - Teleport `token` cluster resource that allows Teleport nodes to join the cluster using Azure credentials. |
There was a problem hiding this comment.
I'd add short explanation how each of those resources will be used. For example, we say that we need user-assigned managed identity for Discovery Service, but we are not explaining how will that account be used exactly.
Having the purpose explained helps to ensure proper understanding.
082b786 to
c91ce19
Compare
hugoShaka
left a comment
There was a problem hiding this comment.
I'm no Terraform expert but this looks ok.
Can we make sure that:
- this code is linted with
tf lintand the CI enforces it - there is a clear owner of the module and it is covered by the test plan
We had other TF modules in the past that went unmainted and I had to remove. My main concern is that this does not follow the same path. I'd like us to ensure the same quality/support level as the go code we write.
examples/terraform-modules/teleport-azure-discovery/examples/single-subscription/main.tf
Outdated
Show resolved
Hide resolved
examples/terraform-modules/teleport-azure-discovery/examples/single-subscription/main.tf
Outdated
Show resolved
Hide resolved
|
@hugoShaka agreed.
i believe this is enforced with
i like this callout. im happy to be the owner, but im unsure where to specify it. i will update the testplan as well. |
This will add a terraform module that will support the automatic discovery of VMs in azure, using an OIDC integration. Closes #60818 Changelog: Added a terraform module to support automatic discovery of azure resources.
b7cc782 to
fe94eda
Compare
- align variable names - conditional creation - remove all inputs from the example
fe94eda to
94961ae
Compare
greedy52
left a comment
There was a problem hiding this comment.
any plan to support non-integration discovery?
| # optional | ||
| match_azure_regions = ["westus", "eastus"] // discover VMs in these US west and east regions. | ||
| match_azure_resource_groups = ["*"] // discover VMs in all resource groups | ||
| match_azure_tags = { "env" = ["example"] } |
There was a problem hiding this comment.
nit: add a comment on match_azure_tags.
| azure_managed_identity_location = azurerm_resource_group.example.location | ||
| azure_resource_group_name = azurerm_resource_group.example.name |
There was a problem hiding this comment.
nit: not very obvious what these two variables will be used for.
| # Custom role for discovery permissions | ||
| resource "azurerm_role_definition" "teleport_discovery" { |
There was a problem hiding this comment.
is this the resource preventing support for multi subscriptions? this is fine for now but i think we will eventually revisit this when working on "org" level discovery for azure.
| spec = { | ||
| azure = { | ||
| allow = [{ | ||
| subscription = local.azure_subscription_id |
There was a problem hiding this comment.
should we set resource group?
There was a problem hiding this comment.
I don't think so, because the VMs we discover won't necessarily be in the same resource group as the identity/role for discovery service
There was a problem hiding this comment.
how about match_azure_resource_groups?
There was a problem hiding this comment.
hmm. I guess we can use that, but it will require a code change because discovery supports wildcard resource group matcher and join tokens do not
|
could we provide a manual test plan for this PR? @GavinFrazar do you prefer updating this one or maybe opening a new PR as you? |
I'll provide a manual test plan and keep updating this PR |
Remove unsupported argument "resource_group_name".
It's not useful to output this.
This permission will be used by: 1. discovery service to perform wildcard subscription discovery 2. auth service to perform wildcard subscription VM join validation
It's so cheap to read the data source that we don't need to make it conditional and it improves the terraform plan.
In order to discover VMs in all subscriptions, the module must allow the user to configure the role and role assignment scopes. Additionally, the module must allow the user to specify the subscription(s) which should be discovered.
This will add a terraform module that will support the automatic discovery of VMs in azure, using an OIDC integration.
Closes #60818
Changelog: Added a terraform module to support automatic discovery of azure resources.