Merge pull request #450 from sapcc/memory-usage-improvements

internal/api/auth/api.go

@@ -92,14 +92,14 @@ func (a *API) handleGetAuth(w http.ResponseWriter, r *http.Request) {
 			scope := req.Scopes[0]
 			if scope.ResourceType == "repository" {
 				repoScope := scope.ParseRepositoryScope(req.IntendedAudience)
-				account, err := keppel.FindAccount(a.db, repoScope.AccountName)
+				accountExists, err := keppel.DoesAccountExist(a.db, repoScope.AccountName)
 				if respondWithError(w, http.StatusInternalServerError, err) {
 					return
 				}
 
 				// if we don't have this account locally, but the request is an anycast
 				// request and one of our peers has the account, ask them to issue the token
-				if account == nil {
+				if !accountExists {
 					err := a.reverseProxyTokenReqToUpstream(w, r, req.IntendedAudience, repoScope.AccountName)
 					if !errors.Is(err, keppel.ErrNoSuchPrimaryAccount) {
 						respondWithError(w, http.StatusInternalServerError, err)

internal/api/registry/blobs.go

@@ -170,7 +170,9 @@ func (a *API) handleGetOrHeadBlob(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Docker-Content-Digest", blob.Digest.String())
 	w.WriteHeader(http.StatusOK)
 	if r.Method != http.MethodHead {
-		_, err = io.Copy(w, reader)
+		// The use of io.LimitReader() here is a hint to io.Copy() to not allocate
+		// a buffer bigger than the expected size of the blob if the blob is small.
+		_, err = io.Copy(w, io.LimitReader(reader, int64(lengthBytes))) //nolint:gosec // lengthBytes will probably not be above 2^63 :)
 		if err != nil {
 			logg.Error("unexpected error from io.Copy() while sending blob to client: %s", err.Error())
 		}

internal/auth/filter.go

@@ -20,6 +20,8 @@
 package auth
 
 import (
+	"database/sql"
+	"errors"
 	"fmt"
 
 	"github.com/sapcc/go-bits/httpext"
@@ -166,21 +168,33 @@ func filterRepoActions(ip string, scope Scope, uid keppel.UserIdentity, audience
 		return nil, nil
 	}
 
-	account, err := keppel.FindAccount(db, repoScope.AccountName)
-	if err != nil {
-		return nil, err
-	}
-	if account == nil {
+	// NOTE: As an optimization, this only loads the few required fields for the account
+	// instead of the entire `accounts` row. Before this optimization, the loads
+	// via keppel.FindAccount() at this callsite made up 8% of all allocations
+	// performed by keppel-api.
+	var (
+		authTenantID     string
+		rbacPoliciesJSON string
+	)
+	err := db.QueryRow(
+		`SELECT auth_tenant_id, rbac_policies_json FROM accounts WHERE name = $1`,
+		repoScope.AccountName,
+	).Scan(&authTenantID, &rbacPoliciesJSON)
+	if errors.Is(err, sql.ErrNoRows) {
+		// if the account does not exist, we cannot give access to it
+		// (this is not an error, because an error would leak information on which accounts exist)
 		return nil, nil
+	} else if err != nil {
+		return nil, err
 	}
 
 	isAllowedAction := map[string]bool{
-		"pull":   uid.HasPermission(keppel.CanPullFromAccount, account.AuthTenantID),
-		"push":   uid.HasPermission(keppel.CanPushToAccount, account.AuthTenantID),
-		"delete": uid.HasPermission(keppel.CanDeleteFromAccount, account.AuthTenantID),
+		"pull":   uid.HasPermission(keppel.CanPullFromAccount, authTenantID),
+		"push":   uid.HasPermission(keppel.CanPushToAccount, authTenantID),
+		"delete": uid.HasPermission(keppel.CanDeleteFromAccount, authTenantID),
 	}
 
-	policies, err := keppel.ParseRBACPolicies(*account)
+	policies, err := keppel.ParseRBACPoliciesField(rbacPoliciesJSON)
 	if err != nil {
 		return nil, fmt.Errorf("while parsing account RBAC policies: %w", err)
 	}

internal/keppel/database_helpers.go

@@ -64,6 +64,13 @@ func FindReducedAccount(db gorp.SqlExecutor, name models.AccountName) (*models.R
 	return &a, err
 }
 
+// DoesAccountExist checks if an account with the given name exists in the DB.
+func DoesAccountExist(db gorp.SqlExecutor, name models.AccountName) (bool, error) {
+	var count uint64
+	err := db.QueryRow(`SELECT COUNT(*) FROM accounts WHERE name = $1`, name).Scan(&count)
+	return count > 0, err
+}
+
 var blobGetQueryByRepoName = sqlext.SimplifyWhitespace(`
 	SELECT b.*
 	  FROM blobs b

internal/keppel/rbac_policy.go

@@ -128,10 +128,18 @@ func (r *RBACPolicy) ValidateAndNormalize(strategy ReplicationStrategy) error {
 
 // ParseRBACPolicies parses the RBAC policies for the given account.
 func ParseRBACPolicies(account models.Account) ([]RBACPolicy, error) {
-	if account.RBACPoliciesJSON == "" || account.RBACPoliciesJSON == "[]" {
+	return ParseRBACPoliciesField(account.RBACPoliciesJSON)
+}
+
+// ParseRBACPoliciesField is like ParseRBACPolicies, but only takes the
+// RBACPoliciesJSON field of type Account instead of the whole Account.
+//
+// This is useful when the full Account has not been loaded from the DB.
+func ParseRBACPoliciesField(buf string) ([]RBACPolicy, error) {
+	if buf == "" || buf == "[]" {
 		return nil, nil
 	}
 	var policies []RBACPolicy
-	err := json.Unmarshal([]byte(account.RBACPoliciesJSON), &policies)
+	err := json.Unmarshal([]byte(buf), &policies)
 	return policies, err
 }