From 8c6ed114e93a78a8107451921c7c46196ee69293 Mon Sep 17 00:00:00 2001 From: Steven Soroka Date: Fri, 8 Oct 2021 23:44:11 -0400 Subject: [PATCH] remove context timeouts for okta --- internal/cmd/login.go | 4 ++-- internal/engine/engine.go | 12 ++++++------ internal/kubernetes/kubernetes.go | 8 +++++--- internal/registry/data.go | 2 +- internal/registry/okta.go | 6 +++--- internal/registry/registry.go | 2 +- internal/timer/timer.go | 4 ++++ 7 files changed, 22 insertions(+), 16 deletions(-) diff --git a/internal/cmd/login.go b/internal/cmd/login.go index cf6a1f149e..4cecf734ae 100644 --- a/internal/cmd/login.go +++ b/internal/cmd/login.go @@ -44,7 +44,7 @@ func login(registry string, useCurrentConfig bool) error { defer func() { if err := lock.Unlock(); err != nil { - fmt.Fprintln(os.Stderr, "Failed to unlock login.") + fmt.Fprintf(os.Stderr, "Failed to unlock login. (%s)\n", lock.Path()) } }() @@ -300,7 +300,7 @@ func promptShouldSkipTLSVerify(host string, skipTLSVerify bool) (shouldSkipTLSVe proceed := false - fmt.Fprintf(os.Stderr, "Could not verify certificate for host %s\n", termenv.String(host).Bold()) + fmt.Fprintf(os.Stderr, "Could not verify certificate for host %q: %s\n", host, err) prompt := &survey.Confirm{ Message: "Are you sure you want to continue?", diff --git a/internal/engine/engine.go b/internal/engine/engine.go index d17024124a..4a27080a47 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -297,13 +297,13 @@ func Run(options Options) error { timer.Start(5*time.Second, func() { endpoint, err := k8s.Endpoint() if err != nil { - logging.L.Error(err.Error()) + logging.L.Error("endpoint: " + err.Error()) return } url, err := urlx.Parse(endpoint) if err != nil { - logging.L.Error(err.Error()) + logging.L.Error("url parse: " + err.Error()) return } @@ -319,7 +319,7 @@ func Run(options Options) error { } } - logging.L.Error(err.Error()) + logging.L.Error("cache get: " + err.Error()) return } @@ -331,17 +331,17 @@ func Run(options Options) error { }, }).Execute() if err != nil { - logging.L.Error(err.Error()) + logging.L.Error("Couldn't create destination: " + err.Error()) return } roles, _, err := client.RolesApi.ListRoles(ctx).DestinationId(destination.Id).Execute() if err != nil { - logging.L.Error(err.Error()) + logging.L.Error("couldn't list roles: " + err.Error()) } err = k8s.UpdateRoles(roles) if err != nil { - logging.L.Error(err.Error()) + logging.L.Error("couldn't update roles: " + err.Error()) return } }) diff --git a/internal/kubernetes/kubernetes.go b/internal/kubernetes/kubernetes.go index ef75db7fc3..ae41047b62 100644 --- a/internal/kubernetes/kubernetes.go +++ b/internal/kubernetes/kubernetes.go @@ -338,12 +338,14 @@ func (k *Kubernetes) UpdateRoles(roles []api.Role) error { err := k.updateRoleBindings(rbSubjects) if err != nil { - return err + return fmt.Errorf("update role bindings: %w", err) } - err = k.updateClusterRoleBindings(crbSubjects) + if err = k.updateClusterRoleBindings(crbSubjects); err != nil { + return fmt.Errorf("update cluster role bindings: %w", err) + } - return err + return nil } func (k *Kubernetes) ec2ClusterName() (string, error) { diff --git a/internal/registry/data.go b/internal/registry/data.go index f77556f155..00de676650 100644 --- a/internal/registry/data.go +++ b/internal/registry/data.go @@ -297,7 +297,7 @@ func (s *Source) SyncUsers(db *gorm.DB, k8s *kubernetes.Kubernetes, okta Okta) e emails, err = okta.Emails(s.Domain, s.ClientId, apiToken) if err != nil { - return fmt.Errorf("sync okta users: %w", err) + return fmt.Errorf("sync okta emails: %w", err) } default: return nil diff --git a/internal/registry/okta.go b/internal/registry/okta.go index 88adfe25c7..f1241bf595 100644 --- a/internal/registry/okta.go +++ b/internal/registry/okta.go @@ -26,14 +26,14 @@ func NewOkta() Okta { // ValidateOktaConnection requests the client from Okta to check for errors on the response func (o *oktaImplementation) ValidateOktaConnection(domain string, clientID string, apiToken string) error { - _, _, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithRequestTimeout(30), okta.WithRateLimitMaxRetries(3), okta.WithToken(apiToken)) + _, _, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithRequestTimeout(30), okta.WithRateLimitMaxRetries(1), okta.WithToken(apiToken)) return err } func (o *oktaImplementation) Emails(domain string, clientID string, apiToken string) ([]string, error) { defer timer.LogTimeElapsed(time.Now(), "okta user sync") - ctx, client, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithRequestTimeout(30), okta.WithRateLimitMaxRetries(3), okta.WithToken(apiToken)) + ctx, client, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithToken(apiToken)) if err != nil { return nil, err } @@ -77,7 +77,7 @@ func (o *oktaImplementation) Emails(domain string, clientID string, apiToken str func (o *oktaImplementation) Groups(domain string, clientID string, apiToken string) (map[string][]string, error) { defer timer.LogTimeElapsed(time.Now(), "okta group sync") - ctx, client, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithRequestTimeout(30), okta.WithRateLimitMaxRetries(3), okta.WithToken(apiToken)) + ctx, client, err := okta.NewClient(context.TODO(), okta.WithOrgUrl("https://"+domain), okta.WithToken(apiToken)) if err != nil { return nil, err } diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 28b905436b..ad380da9a1 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -130,7 +130,7 @@ func Run(options Options) error { } // schedule the user and group sync jobs - interval := 30 * time.Second + interval := 60 * time.Second if options.SyncInterval > 0 { interval = time.Duration(options.SyncInterval) * time.Second } else { diff --git a/internal/timer/timer.go b/internal/timer/timer.go index 33f130ad3e..5aae7f8278 100644 --- a/internal/timer/timer.go +++ b/internal/timer/timer.go @@ -16,6 +16,10 @@ func NewTimer() *Timer { } } +// Start calls sync() every interval. if sync() runs long, +// the next interval will not be started until it completes. +// if intervals are missed they will be skipped, so sync() is +// free to run as long as it needs to func (t *Timer) Start(interval time.Duration, sync func()) { ticker := time.NewTicker(interval)