remove context timeouts for okta (#429)

infrahq · Oct 12, 2021 · 84a916c · 84a916c
1 parent 13e1407
commit 84a916c
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)