Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement API endpoint to link a package to a repo #23851

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+218 −11
Open

Implement API endpoint to link a package to a repo #23851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
339af37
Implement API endpoint to link a package to a repo
sclu1034 Mar 30, 2023
84a2bc3
Add integration test for package linking API
sclu1034 Mar 31, 2023
10df3c4
Rename function
sclu1034 Apr 2, 2023
6e2661b
Improve error reporting
sclu1034 May 2, 2023
f07cfa9
Fix package repository link permission check
sclu1034 Jan 17, 2024
9fb1f1c
Refactor router group
sclu1034 Jan 22, 2024
221dcdc
Allow {owner}/{name} repo format when linking package
sclu1034 Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions routers/api/v1/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1407,11 +1407,14 @@ func Routes() *web.Route {

// NOTE: these are Gitea package management API - see packages.CommonRoutes and packages.DockerContainerRoutes for endpoints that implement package manager APIs
m.Group("/packages/{username}", func() {
m.Group("/{type}/{name}/{version}", func() {
m.Get("", reqToken(), packages.GetPackage)
m.Delete("", reqToken(), reqPackageAccess(perm.AccessModeWrite), packages.DeletePackage)
m.Get("/files", reqToken(), packages.ListPackageFiles)
})
m.Group("/{type}/{name}", func() {
m.Group("/{version}", func() {
m.Get("", packages.GetPackage)
m.Delete("", reqPackageAccess(perm.AccessModeWrite), packages.DeletePackage)
m.Get("/files", packages.ListPackageFiles)
})
m.Post("/link", reqPackageAccess(perm.AccessModeWrite), packages.LinkPackage)
}, reqToken())
m.Get("/", reqToken(), packages.ListPackages)
}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryPackage), context_service.UserAssignmentAPI(), context.PackageAssignmentAPI(), reqPackageAccess(perm.AccessModeRead))

Expand Down
98 changes: 98 additions & 0 deletions routers/api/v1/packages/package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@
package packages

import (
"errors"
"fmt"
"net/http"
"strconv"
"strings"

"code.gitea.io/gitea/models/packages"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/modules/context"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
Expand Down Expand Up @@ -213,3 +218,96 @@ func ListPackageFiles(ctx *context.APIContext) {

ctx.JSON(http.StatusOK, apiPackageFiles)
}

// LinkPackage sets a repository link for a package
func LinkPackage(ctx *context.APIContext) {
// swagger:operation POST /packages/{owner}/{type}/{name}/link package linkPackage
// ---
// summary: Link a package to a repository
// parameters:
// - name: owner
// in: path
// description: owner of the package
// type: string
// required: true
// - name: type
// in: path
// description: type of the package
// type: string
// required: true
// - name: name
// in: path
// description: name of the package
// type: string
// required: true
// - name: repo_id
// in: query
// description: ID of the repository to link. Pass `0` to unlink
// type: integer
// required: false
// - name: repo_name
// in: query
// description: name of the repository to link, format `{owner}/{name}`
// type: string
// required: false
// responses:
// "201":
// "$ref": "#/responses/empty"
// "404":
// "$ref": "#/responses/notFound"

pkg, err := packages.GetPackageByName(ctx, ctx.ContextUser.ID, packages.Type(ctx.Params("type")), ctx.Params("name"))
if err != nil {
if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, "GetPackageByName", err)
} else {
ctx.Error(http.StatusInternalServerError, "GetPackageByName", err)
}
return
}

var repoID int64

formRepoID := ctx.FormString("repo_id")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to expose the repository's id? I don't think we should do that. Using OwnerName and RepoName should be better.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by "exposing" in this context. The existing GET /repos/{owner}/{repo} endpoint makes the id value available in its response, so that information is already "exposed" to every consumer of the public API.

What's the harm in giving the user a choice which identifier they can run this new endpoint against?

Copy link

@KoltesDigital KoltesDigital Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sclu1034 I think last comments refer to the fact that most, if not all the API, refer to orgs and repos using their name, and not their id. At least I already played with the API and I don't recall I ever sent requests using the repo ids. It just feels more natural to use the repo name instead.

Moreover, the web frontend only allow linking to a repository of the same owner. If that is by design, you would have to drop the owner part in the repo name.
EDIT: from @KN4CK3R's previous comment, it is indeed by design.

Just my last two cents on the endpoint: there could only be zero or one link to a repository, so I'd rather use PUT and DELETE.

Copy link
Member

@lunny lunny Oct 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moreover, the web frontend only allow linking to a repository of the same owner. If that is by design, you would have to drop the owner part in the repo name.

I think this is by design. It's weird to link a package to another owner's repository.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny well I'm still wondering. I develop some Unity (game engine) packages. Distribution relies on NPM protocol, although packages are obviously not meant for Nodejs (it's C#). And I also develop some Nodejs packages. But Gitea only provides a single NPM registry per owner. So it seems to me the only way to cleanly separate both usages is to make a dedicated "owner", so to have two registries. But this is only for distribution. Development still occurs in the same org.

repoName := ctx.FormString("repo_name")

if len(formRepoID) == 0 && len(repoName) == 0 {
ctx.Error(http.StatusBadRequest, "Missing parameter", fmt.Errorf("Either `repo_id` or `repo_name` must be given"))
return
}

if len(formRepoID) > 0 {
repoID, err = strconv.ParseInt(ctx.FormString("repo_id"), 10, 64)
if err != nil {
ctx.Error(http.StatusBadRequest, "Invalid parameter", fmt.Errorf("`repo_id` must be a valid integer, if given"))
return
}
} else {
nameParts := strings.Split(repoName, "/")
if len(nameParts) != 2 {
ctx.Error(http.StatusBadRequest, "Invalid parameter", fmt.Errorf("`repo_name` must be of format `{owner}/{name}`, if given"))
return
}

repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, nameParts[0], nameParts[1])
if err != nil {
if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, "GetRepositoryByOwnerAndName", err)
} else {
ctx.Error(http.StatusInternalServerError, "GetRepositoryByOwnerAndName", err)
}

return
}

repoID = repo.ID
}

err = packages_service.LinkPackageToRepository(ctx, ctx.Doer, pkg, repoID)
if err != nil {
ctx.Error(http.StatusInternalServerError, "LinkPackageToRepository", err)
return
}

ctx.Status(http.StatusNoContent)
}
28 changes: 28 additions & 0 deletions services/packages/packages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (

"code.gitea.io/gitea/models/db"
packages_model "code.gitea.io/gitea/models/packages"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log"
Expand Down Expand Up @@ -657,3 +659,29 @@ func RemoveAllPackages(ctx context.Context, userID int64) (int, error) {
}
return count, nil
}

func LinkPackageToRepository(ctx context.Context, doer *user_model.User, p *packages_model.Package, repoID int64) error {
if repoID != 0 {
repo, err := repo_model.GetRepositoryByID(ctx, repoID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This MUST check if the repo belongs to the package owner. And there should be a test for linking a repo of another user.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid further back and forth on permissions, and me doing best-effort guesses, can you give me a detailed run down on who should be allowed to do what?

if err != nil {
return fmt.Errorf("Error getting repository %d: %w", repoID, err)
}

perms, err := access_model.GetUserRepoPermission(ctx, repo, doer)
if err != nil {
return fmt.Errorf("Error getting permissions for user %d on repository %d: %w", doer.ID, repo.ID, err)
}

canWrite := perms.CanWrite(unit.TypePackages)

if !canWrite {
return fmt.Errorf("No permission to link this package and repository, or packages are disabled")
}
}

if err := packages_model.SetRepositoryLink(ctx, p.ID, repoID); err != nil {
return fmt.Errorf("Error updating package: %w", err)
}

return nil
}
52 changes: 52 additions & 0 deletions templates/swagger/v1_json.tmpl
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 32 additions & 6 deletions tests/integration/api_packages_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models/db"
packages_model "code.gitea.io/gitea/models/packages"
container_model "code.gitea.io/gitea/models/packages/container"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
Expand All @@ -31,9 +32,17 @@ import (
func TestPackageAPI(t *testing.T) {
defer tests.PrepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
session := loginUser(t, user.Name)
tokenReadPackage := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadPackage)
tokenWritePackage := getTokenForLoggedInUser(
t,
session,
auth_model.AccessTokenScopeWritePackage,
auth_model.AccessTokenScopeReadPackage,
auth_model.AccessTokenScopeWriteRepository,
)
tokenDeletePackage := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWritePackage)

packageName := "test-package"
Expand Down Expand Up @@ -99,7 +108,8 @@ func TestPackageAPI(t *testing.T) {
assert.Nil(t, ap1.Repository)

// link to public repository
assert.NoError(t, packages_model.SetRepositoryLink(db.DefaultContext, p.ID, 1))
req = NewRequestf(t, "POST", fmt.Sprintf("/api/v1/packages/%s/generic/%s/link?repo_name=%s/%s", user.Name, packageName, repo.OwnerName, repo.Name)).AddTokenAuth(tokenWritePackage)
MakeRequest(t, req, http.StatusNoContent)

req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s/generic/%s/%s", user.Name, packageName, packageVersion)).
AddTokenAuth(tokenReadPackage)
Expand All @@ -108,10 +118,15 @@ func TestPackageAPI(t *testing.T) {
var ap2 *api.Package
DecodeJSON(t, resp, &ap2)
assert.NotNil(t, ap2.Repository)
assert.EqualValues(t, 1, ap2.Repository.ID)
assert.EqualValues(t, repo.ID, ap2.Repository.ID)

// link to private repository
assert.NoError(t, packages_model.SetRepositoryLink(db.DefaultContext, p.ID, 2))
// link to repository without write access, should fail
req = NewRequestf(t, "POST", fmt.Sprintf("/api/v1/packages/%s/generic/%s/link?repo_id=%d", user.Name, packageName, 3)).AddTokenAuth(tokenWritePackage)
MakeRequest(t, req, http.StatusInternalServerError)

// remove link
req = NewRequestf(t, "POST", fmt.Sprintf("/api/v1/packages/%s/generic/%s/link?repo_id=%d", user.Name, packageName, 0)).AddTokenAuth(tokenWritePackage)
MakeRequest(t, req, http.StatusNoContent)

req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s/generic/%s/%s", user.Name, packageName, packageVersion)).
AddTokenAuth(tokenReadPackage)
Expand All @@ -121,7 +136,18 @@ func TestPackageAPI(t *testing.T) {
DecodeJSON(t, resp, &ap3)
assert.Nil(t, ap3.Repository)

assert.NoError(t, packages_model.UnlinkRepositoryFromAllPackages(db.DefaultContext, 2))
// force link to a repository the currently logged-in user doesn't have access to
privateRepoID := int64(6)
assert.NoError(t, packages_model.SetRepositoryLink(db.DefaultContext, p.ID, privateRepoID))

req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s/generic/%s/%s", user.Name, packageName, packageVersion)).AddTokenAuth(tokenReadPackage)
resp = MakeRequest(t, req, http.StatusOK)

var ap4 *api.Package
DecodeJSON(t, resp, &ap4)
assert.Nil(t, ap4.Repository)

assert.NoError(t, packages_model.UnlinkRepositoryFromAllPackages(db.DefaultContext, privateRepoID))
})
})

Expand Down