route restartModule through viamClient dial infra and implement TestRestartModule

[JosephBorodach]

Overview

• Refactors restartModule to use vc.prepareDialInner + vc.connectToRobot instead of manually fetching a machine API key and calling client.New directly. This makes it consistent with how connectToShellService establishes robot connections and removes the dependency on GetRobotAPIKeys for dialing
• Implements TestRestartModule

I haven't spent much time in this area so would definitely appreciate double checking that my changes to restartModule make sense

[cheukt]

changes lgtm but @abe-winter definitely has touched this code more

[abe-winter]

old version called GetRobotAPIKeys explicitly and passed to client.New as rpc.WithEntityCredentials

new version uses rpc.WithExternalAuth (inside prepareDialInner)

I don't have enough goutils depth to understand the difference between ExternalAuth and EntityCredentials -- is that a meaningful or risky change?

[JosephBorodach]

@abe-winter

does it matter that the old one has an explicit error case and the new one doesn't?

No - the check existed because the machine API key was the credential used to connect - no key = no connection. The new code uses the CLI user's own credentials instead so whether the machine has API keys registered can never cause a failure

What's the practical difference between WithEntityCredentials and WithExternalAuth?

• WithEntityCredentials: the client sends the credential directly to the machine and the machine validates it locally. One hop - no cloud needed
• WithExternalAuth: client first contacts authAddr (i.e. app.viam.com) to get a signed token for entity, then presents that token to the machine. Two hops - requires cloud connectivity

In our change, token users go through WithExternalAuth and API key users go through WithEntityCredentials - the same split that already exists in connectToShellService

I don't have enough goutils depth to understand the difference between ExternalAuth and EntityCredentials -- is that a meaningful or risky change?

I don't think so - the new behavior is exactly what connectToShellService already does and that works for both token and API key users

This doesn't introduce something new - it aligns restartModule with the established working pattern. I'm pretty sure the original code was the odd one out

The change doesn't boil down to ExternalAuth vs EntityCredentials

Practical difference
I think there is a bit of confusion about the change

• Old code: both token and API key users connected to the machine using a machine API key - WithEntityCredentials
• New code: token users use WithExternalAuth, API key users effectively use WithEntityCredentials with their own key

But it's not a risky change bc prepareDial (which wraps prepareDialInner) is the established pattern for machine connections across the cli and which is used in connectToShellService which is also highly used

[abe-winter]

In our change, token users go through WithExternalAuth and API key users go through WithEntityCredentials - the same split that already exists in connectToShellService

^ where is this happening? from reading I thought the new version is always using WithExternalAuth

[JosephBorodach]

In our change, token users go through WithExternalAuth and API key users go through WithEntityCredentials - the same split that already exists in connectToShellService

^ where is this happening? from reading I thought the new version is always using WithExternalAuth

It's in prepareDialInner - WithExternalAuth is only added inside this block

if t, ok := c.conf.Auth.(*token); ok {
	rpcOpts = append(rpcOpts, rpc.WithExternalAuth(c.baseURL.Host, partFqdn))

For API key users, c.conf.Auth is *apiKey not *token, so that branch doesn't run. They get WithEntityCredentials from c.conf.DialOptions() --> conf.Auth.dialOpts() instead

func (k *apiKey) dialOpts() rpc.DialOption {
	return rpc.WithEntityCredentials(
		k.KeyID,
		rpc.Credentials{
			Type:    "api-key",
			Payload: k.KeyCrypto,
		},
	)
}

[abe-winter]

ah I didn't read carefully. thanks for clarification, lgtm

[abe-winter]

wait actually -- there are two dialOpts in cli/auth.go

how do you know c.conf.DialOptions() goes to apiKey.dialOpts rather than token.dialOpts?

[JosephBorodach]

wait actually -- there are two dialOpts in cli/auth.go

how do you know c.conf.DialOptions() goes to apiKey.dialOpts rather than token.dialOpts?

@abe-winter

conf.Auth.dialOpts() is an interface method so it dispatches to whichever concrete type conf.Auth holds

• If the user is logged in with a token, it calls token.dialOpts() which returns WithStaticAuthenticationMaterial
• If the user is logged in with a api key, it calls apiKey.dialOpts() which returns WithEntityCredentials