improve register resiliency and idempotency

[patelspratik]

Makes device registration and deregistration resilient and idempotent, and wires up the previously-unused LinuxUser cache for SSH grants.

Register

• Pending-registration record is written before AddNode, so a crash or timeout mid-registration is recoverable
• Re-running brev register with an incomplete registration resumes it, reusing the same device ID (AddNode is idempotent on device_id)
• Already-registered machines now report node connectivity, check/start the Brev tunnel, and direct the user to deregister first — instead of hard-failing
• Org mismatch between the existing registration and the requested org produces an actionable error
• Interactive and non-interactive modes standardized; --ssh-port deprecated in favor of brev enable-ssh

Deregister

• Handles pending registrations: skips RemoveNode when the backend never confirmed the node, recovers the node by device ID where possible
• ListNodes failure during pending cleanup is non-fatal

LinuxUser cache

• Removed GetCachedLinuxUser/SaveCachedLinuxUser (present since BRE2-818, never wired) in favor of the picker we currently have

[drewmalin]

Nice

[drewmalin]

Paginate?

[drewmalin]

It looks like the old error message was more correct here (specifically the guidance to use --org-id <org-id>)?

[patelspratik]

ah ya, next PR should change this

[drewmalin]

It looks like this will happen if:

• the node truly wasn't there, or
• we had some failure in looking up the node

For the second case, I wonder if we actually shouldn't clean up local state so that we can retry the deregister or understand that a new registration shouldn't be allowed (or maybe we just abort the deregister altogether).

[patelspratik]

I was worried about the user being stuck in a loop where findNodeByDeviceID is repeatedly failing so they can't make progress

[drewmalin]

We can do this later, but in the spirit of idempotency I wonder if we could skip certain steps here (like re-installing netbird, re-collecting hardware profile, etc.)

[patelspratik]

ya, something like a progress manifest could be useful for that, but the management becomes a little annoying in case the values changed and we should be fetching fresh.

[drewmalin]

This kind of blew up the usage and leads to lots of Load(false) -- could we keep "Load" as always loading everything + performing the validation it does today, then add a helper for the relatively rare cases where we care only for non-pending registrations?

e.g.:

func (s *FileRegistrationStore) Load() (*DeviceRegistration, error) {
  if exists, err := s.Exists(); err != nil {
    return nil, breverrors.WrapAndTrace(err)
  } else if !exists {
    return nil, breverrors.New("device registration not found, run 'brev register' first")
  }

  var reg DeviceRegistration
    if err := files.ReadJSON(files.AppFs, s.path(), &reg); err != nil {
      return nil, breverrors.WrapAndTrace(err)
    }
    if err := reg.validate(); err != nil {
      return nil, err
  }

  return &reg, nil
}

func (r *DeviceRegistration) validate() error {
  switch r.Status {
  case "":
      if r.ExternalNodeID == "" || r.OrgID == "" {
          return breverrors.New("malformed registration")
      }
      r.Status = RegistrationStatusRegistered

  case RegistrationStatusPending:
      if r.DisplayName == "" || r.OrgID == "" || r.DeviceID == "" {
          return breverrors.New("malformed pending registration")
      }
      if r.ExternalNodeID != "" {
          return breverrors.New("pending registration unexpectedly has a node ID")
      }

  case RegistrationStatusRegistered:
      if r.ExternalNodeID == "" || r.OrgID == "" {
          return breverrors.New("malformed registered registration")
      }

    default:
      return fmt.Errorf("unknown registration status %q", r.Status)
  }

  return nil
}

And then if we need to report an error for pending:

reg, err := store.Load()
if err != nil {
  return nil, err
}
if reg.Status == RegistrationStatusPending {
  return nil, breverrors.New("device registration is incomplete; re-run 'brev register' to finish")
}

or even a helper function that is super explicit:

reg, err := LoadRegistrations(LoadRegistrations{
    AllowedStates: []string{RegistrationStatusPending, RegistrationStatusRegistersd},
})

[patelspratik]

I'm going to separate it out so the calls are more deliberate and maintain the previous behavior as best as possible so the existing callsites don't have to change