Merge branch 'main' into feat-32257-add-comments-unchanged-lines-and-…

…show

MAINTAINERS

@@ -63,3 +63,4 @@ Tim-Niclas Oelschläger <[email protected]> (@zokkis)
 Yu Liu <[email protected]> (@HEREYUA)
 Kemal Zebari <[email protected]> (@kemzeb)
 Rowan Bohde <[email protected]> (@bohde)
+hiifong <[email protected]> (@hiifong)

custom/conf/app.example.ini

@@ -1944,6 +1944,13 @@ LEVEL = Info
 ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio`
 ;MINIO_SECRET_ACCESS_KEY =
 ;;
+;; Preferred IAM Endpoint to override Minio's default IAM Endpoint resolution only available when STORAGE_TYPE is `minio`. 
+;; If not provided and STORAGE_TYPE is `minio`, will search for and derive endpoint from known environment variables 
+;; (AWS_CONTAINER_AUTHORIZATION_TOKEN, AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, AWS_CONTAINER_CREDENTIALS_RELATIVE_URI, 
+;; AWS_CONTAINER_CREDENTIALS_FULL_URI, AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME, AWS_REGION), 
+;; or the DefaultIAMRoleEndpoint if not provided otherwise.
+;MINIO_IAM_ENDPOINT =
+;; 
 ;; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio`
 ;MINIO_BUCKET = gitea
 ;;
@@ -2688,6 +2695,13 @@ LEVEL = Info
 ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio`
 ;MINIO_SECRET_ACCESS_KEY =
 ;;
+;; Preferred IAM Endpoint to override Minio's default IAM Endpoint resolution only available when STORAGE_TYPE is `minio`. 
+;; If not provided and STORAGE_TYPE is `minio`, will search for and derive endpoint from known environment variables 
+;; (AWS_CONTAINER_AUTHORIZATION_TOKEN, AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, AWS_CONTAINER_CREDENTIALS_RELATIVE_URI, 
+;; AWS_CONTAINER_CREDENTIALS_FULL_URI, AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME, AWS_REGION), 
+;; or the DefaultIAMRoleEndpoint if not provided otherwise.
+;MINIO_IAM_ENDPOINT =
+;;
 ;; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio`
 ;MINIO_BUCKET = gitea
 ;;

models/organization/team_repo.go

@@ -9,6 +9,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/perm"
 	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unit"
 
 	"xorm.io/builder"
 )
@@ -83,3 +84,16 @@ func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode per
 		OrderBy("name").
 		Find(&teams)
 }
+
+// GetTeamsWithAccessToRepoUnit returns all teams in an organization that have given access level to the repository special unit.
+func GetTeamsWithAccessToRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type) ([]*Team, error) {
+	teams := make([]*Team, 0, 5)
+	return teams, db.GetEngine(ctx).Where("team_unit.access_mode >= ?", mode).
+		Join("INNER", "team_repo", "team_repo.team_id = team.id").
+		Join("INNER", "team_unit", "team_unit.team_id = team.id").
+		And("team_repo.org_id = ?", orgID).
+		And("team_repo.repo_id = ?", repoID).
+		And("team_unit.type = ?", unitType).
+		OrderBy("name").
+		Find(&teams)
+}

models/organization/team_repo_test.go

@@ -0,0 +1,31 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package organization_test
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/organization"
+	"code.gitea.io/gitea/models/perm"
+	"code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unit"
+	"code.gitea.io/gitea/models/unittest"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetTeamsWithAccessToRepoUnit(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41})
+	repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61})
+
+	teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
+	assert.NoError(t, err)
+	if assert.Len(t, teams, 2) {
+		assert.EqualValues(t, 21, teams[0].ID)
+		assert.EqualValues(t, 22, teams[1].ID)
+	}
+}

models/repo/user_repo.go

@@ -11,7 +11,6 @@ import (
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/container"
-	api "code.gitea.io/gitea/modules/structs"
 
 	"xorm.io/builder"
 )
@@ -146,57 +145,6 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us
 	return users, nil
 }
 
-// GetReviewers get all users can be requested to review:
-// * for private repositories this returns all users that have read access or higher to the repository.
-// * for public repositories this returns all users that have read access or higher to the repository,
-// all repo watchers and all organization members.
-// TODO: may be we should have a busy choice for users to block review request to them.
-func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64) ([]*user_model.User, error) {
-	// Get the owner of the repository - this often already pre-cached and if so saves complexity for the following queries
-	if err := repo.LoadOwner(ctx); err != nil {
-		return nil, err
-	}
-
-	cond := builder.And(builder.Neq{"`user`.id": posterID}).
-		And(builder.Eq{"`user`.is_active": true})
-
-	if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate {
-		// This a private repository:
-		// Anyone who can read the repository is a requestable reviewer
-
-		cond = cond.And(builder.In("`user`.id",
-			builder.Select("user_id").From("access").Where(
-				builder.Eq{"repo_id": repo.ID}.
-					And(builder.Gte{"mode": perm.AccessModeRead}),
-			),
-		))
-
-		if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID {
-			// as private *user* repos don't generate an entry in the `access` table,
-			// the owner of a private repo needs to be explicitly added.
-			cond = cond.Or(builder.Eq{"`user`.id": repo.Owner.ID})
-		}
-	} else {
-		// This is a "public" repository:
-		// Any user that has read access, is a watcher or organization member can be requested to review
-		cond = cond.And(builder.And(builder.In("`user`.id",
-			builder.Select("user_id").From("access").
-				Where(builder.Eq{"repo_id": repo.ID}.
-					And(builder.Gte{"mode": perm.AccessModeRead})),
-		).Or(builder.In("`user`.id",
-			builder.Select("user_id").From("watch").
-				Where(builder.Eq{"repo_id": repo.ID}.
-					And(builder.In("mode", WatchModeNormal, WatchModeAuto))),
-		).Or(builder.In("`user`.id",
-			builder.Select("uid").From("org_user").
-				Where(builder.Eq{"org_id": repo.OwnerID}),
-		)))))
-	}
-
-	users := make([]*user_model.User, 0, 8)
-	return users, db.GetEngine(ctx).Where(cond).OrderBy(user_model.GetOrderByName()).Find(&users)
-}
-
 // GetIssuePostersWithSearch returns users with limit of 30 whose username started with prefix that have authored an issue/pull request for the given repository
 // If isShowFullName is set to true, also include full name prefix search
 func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string, isShowFullName bool) ([]*user_model.User, error) {

models/repo/user_repo_test.go

@@ -38,46 +38,3 @@ func TestRepoAssignees(t *testing.T) {
 		assert.NotContains(t, []int64{users[0].ID, users[1].ID, users[2].ID}, 15)
 	}
 }
-
-func TestRepoGetReviewers(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	// test public repo
-	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
-
-	ctx := db.DefaultContext
-	reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2)
-	assert.NoError(t, err)
-	if assert.Len(t, reviewers, 3) {
-		assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID})
-	}
-
-	// should include doer if doer is not PR poster.
-	reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2)
-	assert.NoError(t, err)
-	assert.Len(t, reviewers, 3)
-
-	// should not include PR poster, if PR poster would be otherwise eligible
-	reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4)
-	assert.NoError(t, err)
-	assert.Len(t, reviewers, 2)
-
-	// test private user repo
-	repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
-
-	reviewers, err = repo_model.GetReviewers(ctx, repo2, 2, 4)
-	assert.NoError(t, err)
-	assert.Len(t, reviewers, 1)
-	assert.EqualValues(t, reviewers[0].ID, 2)
-
-	// test private org repo
-	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
-
-	reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 1)
-	assert.NoError(t, err)
-	assert.Len(t, reviewers, 2)
-
-	reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 2)
-	assert.NoError(t, err)
-	assert.Len(t, reviewers, 1)
-}

modules/setting/storage.go

@@ -43,6 +43,7 @@ type MinioStorageConfig struct {
 	Endpoint           string `ini:"MINIO_ENDPOINT" json:",omitempty"`
 	AccessKeyID        string `ini:"MINIO_ACCESS_KEY_ID" json:",omitempty"`
 	SecretAccessKey    string `ini:"MINIO_SECRET_ACCESS_KEY" json:",omitempty"`
+	IamEndpoint        string `ini:"MINIO_IAM_ENDPOINT" json:",omitempty"`
 	Bucket             string `ini:"MINIO_BUCKET" json:",omitempty"`
 	Location           string `ini:"MINIO_LOCATION" json:",omitempty"`
 	BasePath           string `ini:"MINIO_BASE_PATH" json:",omitempty"`

modules/setting/storage_test.go

@@ -470,6 +470,19 @@ MINIO_BASE_PATH = /prefix
 	cfg, err = NewConfigProviderFromData(`
 [storage]
 STORAGE_TYPE = minio
+MINIO_IAM_ENDPOINT = 127.0.0.1
+MINIO_USE_SSL = true
+MINIO_BASE_PATH = /prefix
+`)
+	assert.NoError(t, err)
+	assert.NoError(t, loadRepoArchiveFrom(cfg))
+	assert.EqualValues(t, "127.0.0.1", RepoArchive.Storage.MinioConfig.IamEndpoint)
+	assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL)
+	assert.EqualValues(t, "/prefix/repo-archive/", RepoArchive.Storage.MinioConfig.BasePath)
+
+	cfg, err = NewConfigProviderFromData(`
+[storage]
+STORAGE_TYPE = minio
 MINIO_ACCESS_KEY_ID = my_access_key
 MINIO_SECRET_ACCESS_KEY = my_secret_key
 MINIO_USE_SSL = true

modules/storage/minio.go

@@ -97,7 +97,7 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage,
 	}
 
 	minioClient, err := minio.New(config.Endpoint, &minio.Options{
-		Creds:        buildMinioCredentials(config, credentials.DefaultIAMRoleEndpoint),
+		Creds:        buildMinioCredentials(config),
 		Secure:       config.UseSSL,
 		Transport:    &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: config.InsecureSkipVerify}},
 		Region:       config.Location,
@@ -164,7 +164,7 @@ func (m *MinioStorage) buildMinioDirPrefix(p string) string {
 	return p
 }
 
-func buildMinioCredentials(config setting.MinioStorageConfig, iamEndpoint string) *credentials.Credentials {
+func buildMinioCredentials(config setting.MinioStorageConfig) *credentials.Credentials {
 	// If static credentials are provided, use those
 	if config.AccessKeyID != "" {
 		return credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, "")
@@ -184,7 +184,9 @@ func buildMinioCredentials(config setting.MinioStorageConfig, iamEndpoint string
 		&credentials.FileAWSCredentials{},
 		// read IAM role from EC2 metadata endpoint if available
 		&credentials.IAM{
-			Endpoint: iamEndpoint,
+			// passing in an empty Endpoint lets the IAM Provider
+			// decide which endpoint to resolve internally
+			Endpoint: config.IamEndpoint,
 			Client: &http.Client{
 				Transport: http.DefaultTransport,
 			},

modules/storage/minio_test.go

@@ -107,8 +107,9 @@ func TestMinioCredentials(t *testing.T) {
 		cfg := setting.MinioStorageConfig{
 			AccessKeyID:     ExpectedAccessKey,
 			SecretAccessKey: ExpectedSecretAccessKey,
+			IamEndpoint:     FakeEndpoint,
 		}
-		creds := buildMinioCredentials(cfg, FakeEndpoint)
+		creds := buildMinioCredentials(cfg)
 		v, err := creds.Get()
 
 		assert.NoError(t, err)
@@ -117,13 +118,15 @@ func TestMinioCredentials(t *testing.T) {
 	})
 
 	t.Run("Chain", func(t *testing.T) {
-		cfg := setting.MinioStorageConfig{}
+		cfg := setting.MinioStorageConfig{
+			IamEndpoint: FakeEndpoint,
+		}
 
 		t.Run("EnvMinio", func(t *testing.T) {
 			t.Setenv("MINIO_ACCESS_KEY", ExpectedAccessKey+"Minio")
 			t.Setenv("MINIO_SECRET_KEY", ExpectedSecretAccessKey+"Minio")
 
-			creds := buildMinioCredentials(cfg, FakeEndpoint)
+			creds := buildMinioCredentials(cfg)
 			v, err := creds.Get()
 
 			assert.NoError(t, err)
@@ -135,7 +138,7 @@ func TestMinioCredentials(t *testing.T) {
 			t.Setenv("AWS_ACCESS_KEY", ExpectedAccessKey+"AWS")
 			t.Setenv("AWS_SECRET_KEY", ExpectedSecretAccessKey+"AWS")
 
-			creds := buildMinioCredentials(cfg, FakeEndpoint)
+			creds := buildMinioCredentials(cfg)
 			v, err := creds.Get()
 
 			assert.NoError(t, err)
@@ -144,11 +147,11 @@ func TestMinioCredentials(t *testing.T) {
 		})
 
 		t.Run("FileMinio", func(t *testing.T) {
-			t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/minio.json")
 			// prevent loading any actual credentials files from the user
+			t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/minio.json")
 			t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/fake")
 
-			creds := buildMinioCredentials(cfg, FakeEndpoint)
+			creds := buildMinioCredentials(cfg)
 			v, err := creds.Get()
 
 			assert.NoError(t, err)
@@ -161,7 +164,7 @@ func TestMinioCredentials(t *testing.T) {
 			t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/fake.json")
 			t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/aws_credentials")
 
-			creds := buildMinioCredentials(cfg, FakeEndpoint)
+			creds := buildMinioCredentials(cfg)
 			v, err := creds.Get()
 
 			assert.NoError(t, err)
@@ -187,7 +190,9 @@ func TestMinioCredentials(t *testing.T) {
 			defer server.Close()
 
 			// Use the provided EC2 Instance Metadata server
-			creds := buildMinioCredentials(cfg, server.URL)
+			creds := buildMinioCredentials(setting.MinioStorageConfig{
+				IamEndpoint: server.URL,
+			})
 			v, err := creds.Get()
 
 			assert.NoError(t, err)

routers/api/v1/repo/collaborators.go

@@ -17,6 +17,8 @@ import (
 	"code.gitea.io/gitea/routers/api/v1/utils"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
+	issue_service "code.gitea.io/gitea/services/issue"
+	pull_service "code.gitea.io/gitea/services/pull"
 	repo_service "code.gitea.io/gitea/services/repository"
 )
 
@@ -320,7 +322,13 @@ func GetReviewers(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	reviewers, err := repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
+	canChooseReviewer := issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, ctx.Repo.Repository, 0)
+	if !canChooseReviewer {
+		ctx.Error(http.StatusForbidden, "GetReviewers", errors.New("doer has no permission to get reviewers"))
+		return
+	}
+
+	reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "ListCollaborators", err)
 		return

routers/web/repo/issue_page_meta.go

@@ -19,7 +19,7 @@ import (
 	shared_user "code.gitea.io/gitea/routers/web/shared/user"
 	"code.gitea.io/gitea/services/context"
 	issue_service "code.gitea.io/gitea/services/issue"
-	repo_service "code.gitea.io/gitea/services/repository"
+	pull_service "code.gitea.io/gitea/services/pull"
 )
 
 type issueSidebarMilestoneData struct {
@@ -186,7 +186,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
 		if d.Issue == nil {
 			data.CanChooseReviewer = true
 		} else {
-			data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue)
+			data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue.PosterID)
 		}
 	}
 
@@ -231,13 +231,13 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
 
 	if data.CanChooseReviewer {
 		var err error
-		reviewers, err = repo_model.GetReviewers(ctx, repo, ctx.Doer.ID, posterID)
+		reviewers, err = pull_service.GetReviewers(ctx, repo, ctx.Doer.ID, posterID)
 		if err != nil {
 			ctx.ServerError("GetReviewers", err)
 			return
 		}
 
-		teamReviewers, err = repo_service.GetReviewerTeams(ctx, repo)
+		teamReviewers, err = pull_service.GetReviewerTeams(ctx, repo)
 		if err != nil {
 			ctx.ServerError("GetReviewerTeams", err)
 			return

routers/web/repo/repo.go

@@ -352,6 +352,9 @@ func Action(ctx *context.Context) {
 		ctx.Data["IsStaringRepo"] = repo_model.IsStaring(ctx, ctx.Doer.ID, ctx.Repo.Repository.ID)
 	}
 
+	// see the `hx-trigger="refreshUserCards ..."` comments in tmpl
+	ctx.RespHeader().Add("hx-trigger", "refreshUserCards")
+
 	switch ctx.PathParam(":action") {
 	case "watch", "unwatch", "star", "unstar":
 		// we have to reload the repository because NumStars or NumWatching (used in the templates) has just changed