Add support for Client ID in Azure VM auto-discovery by atburke · Pull Request #32360 · gravitational/teleport

atburke · 2023-09-21T23:37:47Z

This PR adds the client_id option to the Discovery Service for Azure VMs, which sets the client ID of the managed identity for discovered nodes to use when joining the cluster. This allows the discovered nodes to be discovered while having multiple managed identities assigned.

Resolves #28839.

github-actions · 2023-09-22T00:19:53Z

🤖 Vercel preview here: https://docs-943ksq91b-goteleport.vercel.app/docs/ver/14.x

github-actions · 2023-09-27T23:33:29Z

🤖 Vercel preview here: https://docs-a3qgfzw01-goteleport.vercel.app/docs/ver/14.x

r0mant · 2023-09-28T00:04:03Z

Can we not introduce a basically free-form arbitrary "join params" field that's easy to abuse IMO, and instead add a specific field for the Azure client ID?

r0mant · 2023-09-28T00:05:15Z

As mentioned above, I would just create a specific flag for the Azure client ID and avoid generic "join params".

r0mant · 2023-09-28T00:06:03Z

Can we this passed to the installer web API as a parameter that would return the script with proper client ID baked in, instead of environment variable?

r0mant · 2023-09-28T00:06:47Z

Let's not introduce a custom join params "CLI protocol" with custom parsing format, it will quickly get very messy.

github-actions · 2023-09-28T20:16:27Z

🤖 Vercel preview here: https://docs-2eg5uwx3t-goteleport.vercel.app/docs/ver/14.x

r0mant · 2023-09-28T22:55:45Z

Suggested change

Azure *AzureInstallParams `yaml:"azure"`

Azure *AzureInstallParams `yaml:"azure,omitempty"`

r0mant · 2023-09-28T23:10:58Z

I'm probably just missing something but one thing I don't understand is how is this ClientID actually being used to pick the correct Azure identity client? In this PR I see that it's being passed to the install script and then to the join config, but where is it actually being used?

teleport/lib/auth/register.go

Lines 697 to 708 in 492e3b3

func registerUsingAzureMethod(client joinServiceClient, token string, params RegisterParams) (*proto.Certs, error) {

ctx := context.Background()

certs, err := client.RegisterUsingAzureMethod(ctx, func(challenge string) (*proto.RegisterUsingAzureMethodRequest, error) {

imds := azure.NewInstanceMetadataClient()

if !imds.IsAvailable(ctx) {

return nil, trace.AccessDenied("could not reach instance metadata. Is Teleport running on an Azure VM?")

}

ad, err := imds.GetAttestedData(ctx, challenge)

if err != nil {

return nil, trace.Wrap(err)

}

accessToken, err := imds.GetAccessToken(ctx, params.AzureParams.ClientID)

It's been supported by the Azure join method for a while, this PR just makes it work for discovered nodes too.

github-actions · 2023-09-29T00:07:44Z

🤖 Vercel preview here: https://docs-fkg28baez-goteleport.vercel.app/docs/ver/14.x

This change adds the `client_id` optio nto the Discovery Service for Azure VMs, which sets the client ID of the managed identity for discovered nodes to use when joining the cluster. This allows the discovered nodes to be discovered while having multiple managed identities assigned.

github-actions · 2023-09-29T16:21:27Z

🤖 Vercel preview here: https://docs-c6q2otdkb-goteleport.vercel.app/docs/ver/14.x

github-actions · 2023-09-29T16:22:01Z

🤖 Vercel preview here: https://docs-6a0v4xmmd-goteleport.vercel.app/docs/ver/14.x

public-teleport-github-review-bot · 2023-09-29T16:40:20Z

@atburke See the table below for backport results.

Branch	Result
branch/v14	Create PR

atburke added the backport/branch/v14 label Sep 21, 2023

atburke had a problem deploying to vercel September 21, 2023 23:37 — with GitHub Actions Failure

github-actions Bot added discovery documentation size/sm labels Sep 21, 2023

github-actions Bot requested review from capnspacehook, kimlisa, lsgunn-teleport, ptgott, r0mant, xinding33 and zmb3 September 21, 2023 23:38

atburke temporarily deployed to vercel September 21, 2023 23:57 — with GitHub Actions Inactive

ptgott approved these changes Sep 22, 2023

View reviewed changes

kimlisa approved these changes Sep 22, 2023

View reviewed changes

Comment thread lib/client/api.go Outdated

Comment thread lib/config/fileconf.go Outdated

atburke force-pushed the atburke/azure-discovery-client-id branch from 0464480 to fda6e69 Compare September 27, 2023 23:17

atburke temporarily deployed to vercel September 27, 2023 23:17 — with GitHub Actions Inactive

r0mant requested changes Sep 28, 2023

View reviewed changes

atburke force-pushed the atburke/azure-discovery-client-id branch from fda6e69 to df83661 Compare September 28, 2023 20:00

atburke temporarily deployed to vercel September 28, 2023 20:00 — with GitHub Actions Inactive

r0mant reviewed Sep 28, 2023

View reviewed changes

atburke temporarily deployed to vercel September 28, 2023 23:51 — with GitHub Actions Inactive

r0mant approved these changes Sep 29, 2023

View reviewed changes

public-teleport-github-review-bot Bot removed request for capnspacehook, xinding33 and zmb3 September 29, 2023 00:10

public-teleport-github-review-bot Bot removed the request for review from lsgunn-teleport September 29, 2023 00:10

atburke added the changelog label Sep 29, 2023

atburke force-pushed the atburke/azure-discovery-client-id branch from da732ac to 40a30ab Compare September 29, 2023 16:05

atburke changed the title ~~Specify Client ID in Azure VM auto-discovery~~ Add support for Client ID in Azure VM auto-discovery Sep 29, 2023

atburke temporarily deployed to vercel September 29, 2023 16:05 — with GitHub Actions Inactive

atburke enabled auto-merge September 29, 2023 16:05

atburke added this pull request to the merge queue Sep 29, 2023

Merged via the queue into master with commit 15d190b Sep 29, 2023

atburke deleted the atburke/azure-discovery-client-id branch September 29, 2023 16:39

atburke mentioned this pull request Sep 29, 2023

[v14] Add support for Client ID in Azure VM auto-discovery #32800

Merged

	Azure *AzureInstallParams `yaml:"azure"`
	Azure *AzureInstallParams `yaml:"azure,omitempty"`

	func registerUsingAzureMethod(client joinServiceClient, token string, params RegisterParams) (*proto.Certs, error) {
	ctx := context.Background()
	certs, err := client.RegisterUsingAzureMethod(ctx, func(challenge string) (*proto.RegisterUsingAzureMethodRequest, error) {
	imds := azure.NewInstanceMetadataClient()
	if !imds.IsAvailable(ctx) {
	return nil, trace.AccessDenied("could not reach instance metadata. Is Teleport running on an Azure VM?")
	}
	ad, err := imds.GetAttestedData(ctx, challenge)
	if err != nil {
	return nil, trace.Wrap(err)
	}
	accessToken, err := imds.GetAccessToken(ctx, params.AzureParams.ClientID)

Conversation

atburke commented Sep 21, 2023

Uh oh!

github-actions Bot commented Sep 22, 2023

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 27, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 28, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 29, 2023

Uh oh!

github-actions Bot commented Sep 29, 2023

Uh oh!

github-actions Bot commented Sep 29, 2023

Uh oh!

public-teleport-github-review-bot Bot commented Sep 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants