address comments

porter-dev · May 6, 2024 · d9e3286 · d9e3286
1 parent caea466
commit d9e3286
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 19 deletions.
diff --git a/api/server/handlers/user/migrate.go b/api/server/handlers/user/migrate.go
@@ -1,6 +1,7 @@
 package user
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -43,8 +44,8 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 
 	r = r.Clone(ctx)
 
-	user, _ := r.Context().Value(types.UserScope).(*models.User)
-	if !strings.HasSuffix(user.Email, "@porter.run") {
+	thisUser, _ := r.Context().Value(types.UserScope).(*models.User)
+	if !strings.HasSuffix(thisUser.Email, "@porter.run") {
 		err := telemetry.Error(ctx, span, nil, "user is not a porter user")
 		u.HandleAPIError(w, r, apierrors.NewErrForbidden(err))
 		return
@@ -58,7 +59,7 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	}
 
 	var usersMissingAuthMechanism []uint
-	migrationErrors := map[uint]string{}
+	migrationErrors := map[string][]uint{}
 
 	for _, user := range users {
 		// skip users that are already migrated
@@ -82,7 +83,8 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 			}
 		}
 
-		if user.Password != "" {
+		switch {
+		case user.Password != "":
 			password := user.Password
 			createIdentityBody.Credentials = &ory.IdentityWithCredentials{
 				Oidc: nil,
@@ -93,7 +95,7 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 				},
 				AdditionalProperties: nil,
 			}
-		} else if user.GithubUserID != 0 {
+		case user.GithubUserID != 0:
 			createIdentityBody.Credentials = &ory.IdentityWithCredentials{
 				Oidc: &ory.IdentityWithCredentialsOidc{
 					Config: &ory.IdentityWithCredentialsOidcConfig{
@@ -107,7 +109,7 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 					},
 				},
 			}
-		} else if user.GoogleUserID != "" {
+		case user.GoogleUserID != "":
 			createIdentityBody.Credentials = &ory.IdentityWithCredentials{
 				Oidc: &ory.IdentityWithCredentialsOidc{
 					Config: &ory.IdentityWithCredentialsOidcConfig{
@@ -121,14 +123,19 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 					},
 				},
 			}
-		} else {
+		default:
 			usersMissingAuthMechanism = append(usersMissingAuthMechanism, user.ID)
 			continue
 		}
 
 		createdIdentity, _, err := u.Config().Ory.IdentityAPI.CreateIdentity(u.Config().OryApiKeyContextWrapper(ctx)).CreateIdentityBody(createIdentityBody).Execute()
 		if err != nil {
-			migrationErrors[user.ID] = fmt.Sprintf("error creating identity: %s", err.Error())
+			errString := fmt.Sprintf("error creating identity: %s", err.Error())
+			if len(migrationErrors[err.Error()]) == 0 {
+				migrationErrors[errString] = []uint{}
+			}
+			migrationErrors[errString] = append(migrationErrors[errString], user.ID)
+
 			continue
 		}
 
@@ -137,18 +144,25 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 
 		_, err = u.Repo().User().UpdateUser(user)
 		if err != nil {
-			migrationErrors[user.ID] = fmt.Sprintf("error updating user: %s", err.Error())
+			errString := fmt.Sprintf("error updating user: %s", err.Error())
+			if len(migrationErrors[err.Error()]) == 0 {
+				migrationErrors[errString] = []uint{}
+			}
+			migrationErrors[errString] = append(migrationErrors[errString], user.ID)
 			continue
 		}
 	}
 
-	telemetry.WithAttributes(span,
-		telemetry.AttributeKV{Key: "users-missing-auth-mechanism", Value: usersMissingAuthMechanism},
-		telemetry.AttributeKV{Key: "migration-errors", Value: migrationErrors},
-	)
+	var errs []error
+	if len(usersMissingAuthMechanism) > 0 {
+		errs = append(errs, fmt.Errorf("users missing auth mechanism: %v", usersMissingAuthMechanism))
+	}
+	for errString, userIds := range migrationErrors {
+		errs = append(errs, fmt.Errorf("%s: %v", errString, userIds))
+	}
 
-	if len(usersMissingAuthMechanism) > 0 || len(migrationErrors) > 0 {
-		err := telemetry.Error(ctx, span, nil, "error migrating users")
+	if len(errs) > 0 {
+		err := telemetry.Error(ctx, span, errors.Join(errs...), "error migrating users")
 		u.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusInternalServerError))
 		return
 	}

diff --git a/api/server/shared/config/config.go b/api/server/shared/config/config.go
@@ -124,7 +124,7 @@ type Config struct {
 
 	TelemetryConfig telemetry.TracerConfig
 
-	Ory                     *ory.APIClient
+	Ory                     ory.APIClient
 	OryApiKeyContextWrapper func(ctx context.Context) context.Context
 }
 

diff --git a/api/server/shared/config/loader/loader.go b/api/server/shared/config/loader/loader.go
@@ -396,7 +396,7 @@ func (e *EnvConfigLoader) LoadConfig() (res *config.Config, err error) {
 		URL: InstanceEnvConf.ServerConf.OryUrl,
 	}}
 
-	res.Ory = ory.NewAPIClient(c)
+	res.Ory = *ory.NewAPIClient(c)
 	res.OryApiKeyContextWrapper = func(ctx context.Context) context.Context {
 		return context.WithValue(ctx, ory.ContextAccessToken, InstanceEnvConf.ServerConf.OryApiKey)
 	}

diff --git a/internal/telemetry/span.go b/internal/telemetry/span.go
@@ -92,8 +92,6 @@ func WithAttributes(span trace.Span, attrs ...AttributeKV) {
 				zone, offset := val.Zone()
 				span.SetAttributes(attribute.String(prefixSpanKey(fmt.Sprintf("%s-timezone", string(attr.Key))), zone))
 				span.SetAttributes(attribute.Int(prefixSpanKey(fmt.Sprintf("%s-offset", string(attr.Key))), offset))
-			default:
-				span.SetAttributes(attribute.String(prefixSpanKey(string(attr.Key)), fmt.Sprintf("%v", val)))
 			}
 		}
 	}

diff --git a/zarf/helm/.serverenv b/zarf/helm/.serverenv
@@ -94,6 +94,7 @@ NEON_CLIENT_ID=
 # NEON_CLIENT_SECRET is used to integrate with Neon
 NEON_CLIENT_SECRET=
 
+// Note: Ory values can be found in 1Password
 // ORY_URL is the URL for Ory
 ORY_URL=
 // ORY_API_KEY authenticates with Ory