[client] Stop netbird login from retrying a refusal for 30 seconds

`netbird up` and `netbird login` both run Login through the backoff cycle, and
each carried its own copy of the list of codes that end it. Only up.go learned
about codes.FailedPrecondition, so a refused `netbird login` kept retrying and
then reported "login backoff cycle failed" instead of what the daemon said.

terminalLoginError is now that list, once, next to WithBackOff — the duplicated
copies are what let the two commands disagree in the first place.

Reported by cubic-dev-ai on PR #7398.
This commit is contained in:
riccardom
2026-09-02 16:25:55 +02:00
parent 6790c34b08
commit 34cae56ee3
4 changed files with 60 additions and 11 deletions
+1 -6
View File
@@ -9,8 +9,6 @@ import (
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"golang.org/x/term"
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"
"github.com/netbirdio/netbird/client/internal"
"github.com/netbirdio/netbird/client/internal/auth"
@@ -144,10 +142,7 @@ func doDaemonLogin(ctx context.Context, cmd *cobra.Command, providedSetupKey str
err = WithBackOff(func() error {
var backOffErr error
loginResp, backOffErr = client.Login(ctx, &loginRequest)
if s, ok := gstatus.FromError(backOffErr); ok && (s.Code() == codes.InvalidArgument ||
s.Code() == codes.PermissionDenied ||
s.Code() == codes.NotFound ||
s.Code() == codes.Unimplemented) {
if terminalLoginError(backOffErr) {
loginErr = backOffErr
return nil
}
+31
View File
@@ -20,6 +20,8 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"
"github.com/netbirdio/netbird/client/anonymize"
daddr "github.com/netbirdio/netbird/client/internal/daemonaddr"
@@ -285,6 +287,35 @@ func DialClientGRPCServer(ctx context.Context, addr string) (*grpc.ClientConn, e
return grpc.DialContext(ctx, target, opts...)
}
// terminalLoginError reports whether a Login failure is final, so the backoff
// cycle stops and the caller is told what the daemon said instead of "login
// backoff cycle failed" thirty seconds later. Retrying cannot change any of
// these answers: the request is malformed, the caller is not allowed, the
// target does not exist, a precondition on the daemon refuses it (the
// update-settings kill switch, an MDM-managed field), or the method is not
// implemented.
//
// Both `netbird up` and `netbird login` run Login through the backoff, and
// they each carried their own copy of this list — which is how one of them
// ended up retrying a refusal the other treated as final.
func terminalLoginError(err error) bool {
s, ok := gstatus.FromError(err)
if !ok {
return false
}
switch s.Code() {
case codes.InvalidArgument,
codes.PermissionDenied,
codes.NotFound,
codes.FailedPrecondition,
codes.Unimplemented:
return true
default:
return false
}
}
// WithBackOff execute function in backoff cycle.
func WithBackOff(bf func() error) error {
return backoff.RetryNotify(bf, CLIBackOffSettings, func(err error, duration time.Duration) {
+1 -5
View File
@@ -403,11 +403,7 @@ func doDaemonUp(ctx context.Context, cmd *cobra.Command, client proto.DaemonServ
err = WithBackOff(func() error {
var backOffErr error
loginResp, backOffErr = client.Login(ctx, loginRequest)
if s, ok := gstatus.FromError(backOffErr); ok && (s.Code() == codes.InvalidArgument ||
s.Code() == codes.PermissionDenied ||
s.Code() == codes.NotFound ||
s.Code() == codes.FailedPrecondition ||
s.Code() == codes.Unimplemented) {
if terminalLoginError(backOffErr) {
loginErr = backOffErr
return nil
}
+27
View File
@@ -55,3 +55,30 @@ func TestRefusedSettingsUpdate(t *testing.T) {
})
}
}
// Both `netbird up` and `netbird login` drive Login through the backoff cycle,
// and a final answer has to stop it: retrying a refusal only replaces the
// daemon's reason with "login backoff cycle failed" thirty seconds later.
func TestTerminalLoginError(t *testing.T) {
tests := []struct {
name string
err error
wantTerminal bool
}{
{name: "settings refused by the kill switch", err: gstatus.Errorf(codes.FailedPrecondition, "update settings are disabled"), wantTerminal: true},
{name: "field managed by MDM", err: gstatus.Errorf(codes.FailedPrecondition, "fields managed by MDM policy: managementURL"), wantTerminal: true},
{name: "caller not allowed", err: gstatus.Errorf(codes.PermissionDenied, "nope"), wantTerminal: true},
{name: "malformed request", err: gstatus.Errorf(codes.InvalidArgument, "nope"), wantTerminal: true},
{name: "profile not found", err: gstatus.Errorf(codes.NotFound, "nope"), wantTerminal: true},
{name: "method missing on an older daemon", err: gstatus.Errorf(codes.Unimplemented, "nope"), wantTerminal: true},
{name: "daemon unreachable, worth retrying", err: gstatus.Errorf(codes.Unavailable, "connection refused"), wantTerminal: false},
{name: "transient internal failure", err: gstatus.Errorf(codes.Internal, "boom"), wantTerminal: false},
{name: "not a status error", err: errors.New("boom"), wantTerminal: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.wantTerminal, terminalLoginError(tt.err))
})
}
}