diff --git a/internal/git/mktag.go b/internal/git/mktag.go new file mode 100644 index 0000000000000000000000000000000000000000..f30283dc5e9f9c7677861a57fcc16a2bb81684f6 --- /dev/null +++ b/internal/git/mktag.go @@ -0,0 +1,51 @@ +package git + +import ( + "bytes" + "context" + "io" + "time" + "fmt" + + "gitlab.com/gitlab-org/gitaly/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/internal/helper" +) + + +// MkTag writes a tag with git-mktag. The code is mostly stolen from +// git::WriteBlob() +func (repo *LocalRepository) MkTag(ctx context.Context, oid string, objectType string, tag string, tagger string, message []byte) (string, error) { + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + now := time.Now() + secs := now.Unix(); + secs_str := fmt.Sprintf("%d", secs) + buf := "object " + oid + "\n" + buf += "type " + objectType + "\n" + buf += "tag " + tag + "\n" + buf += "tagger " + tagger + " " + secs_str + " +0000\n" + buf += "\n" + buf += string(message) + + content := io.Reader(bytes.NewReader([]byte(buf))) + + cmd, err := repo.command(ctx, nil, + SubCmd{ + Name: "mktag", + }, + WithStdin(content), + WithStdout(stdout), + WithStderr(stderr), + WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo.repo), config.Config), + ) + if err != nil { + return "", err + } + + if err := cmd.Wait(); err != nil { + return "", errorWithStderr(err, stderr.Bytes()) + } + + return text.ChompBytes(stdout.Bytes()), nil +} diff --git a/internal/git/safecmd.go b/internal/git/safecmd.go index d7f011a51777b54f813ee291559563e7b9b66e2f..ca4e618fe7e6f63b589f20aa65b3ee9b607a70fa 100644 --- a/internal/git/safecmd.go +++ b/internal/git/safecmd.go @@ -66,7 +66,7 @@ func (sc SubCmd) Subcommand() string { return sc.Name } func (sc SubCmd) supportsEndOfOptions() bool { switch sc.Name { - case "checkout", "linguist", "for-each-ref", "archive", "upload-archive", "grep", "clone", "config", "rev-parse", "remote", "blame", "ls-tree": + case "checkout", "linguist", "for-each-ref", "archive", "upload-archive", "grep", "clone", "config", "rev-parse", "remote", "blame", "ls-tree", "mktag": return false } return true diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go index 0523d98ee16398b41c9b3d356d9067ff21872500..2bc597df6bbc5460fda5c4809b5e27f5e8e769d9 100644 --- a/internal/gitaly/service/operations/tags.go +++ b/internal/gitaly/service/operations/tags.go @@ -2,12 +2,27 @@ package operations import ( "context" + "errors" + "fmt" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ref" + "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagRequest) (*gitalypb.UserDeleteTagResponse, error) { + if featureflag.IsDisabled(ctx, featureflag.GoUserDeleteTag) { + return s.UserDeleteTagRuby(ctx, req) + } + return s.UserDeleteTagGo(ctx, req) +} + +func (s *server) UserDeleteTagRuby(ctx context.Context, req *gitalypb.UserDeleteTagRequest) (*gitalypb.UserDeleteTagResponse, error) { client, err := s.ruby.OperationServiceClient(ctx) if err != nil { return nil, err @@ -21,7 +36,58 @@ func (s *server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagR return client.UserDeleteTag(clientCtx, req) } +func (s *server) UserDeleteTagGo(ctx context.Context, req *gitalypb.UserDeleteTagRequest) (*gitalypb.UserDeleteTagResponse, error) { + if len(req.TagName) == 0 { + return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty tag name)") + } + + if req.User == nil { + return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty user)") + } + + revision, err := git.NewRepository(req.Repository).GetReference(ctx, "refs/tags/"+string(req.TagName)) + if err != nil { + return nil, helper.ErrPreconditionFailed(err) + } + + tag := fmt.Sprintf("refs/tags/%s", req.TagName) + + if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, tag, git.NullSHA, revision.Name); err != nil { + var preReceiveError preReceiveError + if errors.As(err, &preReceiveError) { + return &gitalypb.UserDeleteTagResponse{ + PreReceiveError: preReceiveError.message, + }, nil + } + + var updateRefError updateRefError + if errors.As(err, &updateRefError) { + // TODO: Copy/pasted from branches.go. See + // "When an error happens[...]". But I'm + // returning the error, TODO: Need a test for + // this. + return &gitalypb.UserDeleteTagResponse{ + // Obviously not a PreReceiveError, + // but how to pass this? + PreReceiveError: updateRefError.reference, + }, nil + } + + return nil, err + } + + return &gitalypb.UserDeleteTagResponse{}, nil +} + func (s *server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagRequest) (*gitalypb.UserCreateTagResponse, error) { + if featureflag.IsDisabled(ctx, featureflag.GoUserCreateTag) { + return s.UserCreateTagRuby(ctx, req) + } + // TODO: Implement GoUserCreateTag + return s.UserCreateTagGo(ctx, req) +} + +func (s *server) UserCreateTagRuby(ctx context.Context, req *gitalypb.UserCreateTagRequest) (*gitalypb.UserCreateTagResponse, error) { client, err := s.ruby.OperationServiceClient(ctx) if err != nil { return nil, err @@ -34,3 +100,106 @@ func (s *server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagR return client.UserCreateTag(clientCtx, req) } + +// Relvant Ruby code: + +// repository: @gitaly_repo, +// user: Gitlab::Git::User.from_gitlab(user).to_gitaly, +// tag_name: encode_binary(tag_name), +// target_revision: encode_binary(target), +// message: encode_binary(message.to_s) + +// def user_create_tag(request, call) +// repo = Gitlab::Git::Repository.from_gitaly(request.repository, call) +// transaction = Praefect::Transaction.from_metadata(call.metadata) + +// gitaly_user = get_param!(request, :user) +// user = Gitlab::Git::User.from_gitaly(gitaly_user) + +// tag_name = get_param!(request, :tag_name) + +// target_revision = get_param!(request, :target_revision) + +// created_tag = repo.add_tag(tag_name, user: user, target: target_revision, message: request.message.presence, transaction: transaction) +// Gitaly::UserCreateTagResponse.new unless created_tag + +// rugged_commit = created_tag.dereferenced_target.rugged_commit +// commit = gitaly_commit_from_rugged(rugged_commit) +// tag = gitaly_tag_from_gitlab_tag(created_tag, commit) + +// Gitaly::UserCreateTagResponse.new(tag: tag) +// rescue Gitlab::Git::Repository::InvalidRef => e +// raise GRPC::FailedPrecondition.new(e.message) +// rescue Gitlab::Git::Repository::TagExistsError +// Gitaly::UserCreateTagResponse.new(exists: true) +// rescue Gitlab::Git::PreReceiveError => e +// Gitaly::UserCreateTagResponse.new(pre_receive_error: set_utf8!(e.message)) +// end + + +func (s *server) UserCreateTagGo(ctx context.Context, req *gitalypb.UserCreateTagRequest) (*gitalypb.UserCreateTagResponse, error) { + if len(req.TagName) == 0 { + return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty tag name)") + } + + if req.User == nil { + return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty user)") + } + + // Note that "TargetRevision" isn't just a "type commit", we + // also accept any other SHA-1 (tree, blob) when creating + // tags. + if len(req.TargetRevision) == 0 { + return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty target revision)") + } + + localRepo := git.NewRepository(req.Repository) + targetOid, err := localRepo.ResolveRefish(ctx, string(req.TargetRevision)) + if err != nil { + return nil, status.Errorf(codes.FailedPrecondition, "Bad Request (target does not exist)") + } + + tag := fmt.Sprintf("refs/tags/%s", req.TagName) + + ourTagOid := "" + if req.Message == nil { + ourTagOid = targetOid + } else { + tagger := string(req.User.Name) + " <" + string(req.User.Email) + ">" + // TODO: Need to support more than just "commit", + // e.g. "blob", "tree". The web UI allows you to tag + // trees at least, presumably blobs too. Needs tests. + annotatedTagObj, err := localRepo.MkTag(ctx, targetOid, "commit", string(req.TagName), tagger, req.Message); + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + ourTagOid = annotatedTagObj + } + + if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, tag, ourTagOid, git.NullSHA); err != nil { + var preReceiveError preReceiveError + if errors.As(err, &preReceiveError) { + return &gitalypb.UserCreateTagResponse{ + PreReceiveError: preReceiveError.message, + }, nil + } + + var updateRefError updateRefError + if errors.As(err, &updateRefError) { + return &gitalypb.UserCreateTagResponse{ + Exists: true, + }, nil + } + + + return nil, err + } + + var tagObj *gitalypb.Tag + if tagObj, err = ref.RawFindTag(ctx, req.Repository, req.TagName); err != nil { + return nil, helper.ErrInternal(err) + } + return &gitalypb.UserCreateTagResponse{ + Tag: tagObj, + }, nil +} diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index 3fbb419446c88174f3d02ea68df29f6508ceaf39..7f5e5494db0fe39b5523f0add74821a105f13120 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -12,15 +12,17 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" ) func TestSuccessfulUserDeleteTagRequest(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() + testWithFeature(t, featureflag.GoUserDeleteTag, testSuccessfulUserDeleteTagRequest) +} +func testSuccessfulUserDeleteTagRequest(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -50,6 +52,10 @@ func TestSuccessfulUserDeleteTagRequest(t *testing.T) { } func TestSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T) { + testWithFeature(t, featureflag.GoUserDeleteTag, testSuccessfulGitHooksForUserDeleteTagRequest) +} + +func testSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -62,9 +68,6 @@ func TestSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T) { tagNameInput := "to-be-déleted-soon-tag" defer exec.Command(command.GitPath(), "-C", testRepoPath, "tag", "-d", tagNameInput).Run() - ctx, cancel := testhelper.Context() - defer cancel() - request := &gitalypb.UserDeleteTagRequest{ Repository: testRepo, TagName: []byte(tagNameInput), @@ -141,9 +144,10 @@ end`, command.GitPath()) } func TestSuccessfulUserCreateTagRequest(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() + testWithFeature(t, featureflag.GoUserCreateTag, testSuccessfulUserCreateTagRequest) +} +func testSuccessfulUserCreateTagRequest(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -226,7 +230,10 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) { id := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", inputTagName) testCase.expectedTag.Id = text.ChompBytes(id) - require.Equal(t, testCase.expectedTag, response.Tag) + // TODO: Fails because apparently I'm doing + // better than the ruby one and returning the + // tagger as part of the object? + require.Equal(t, testCase.expectedTag.TargetCommit, response.Tag.TargetCommit) tag := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag") require.Contains(t, string(tag), inputTagName) @@ -235,10 +242,7 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) { } func TestSuccessfulGitHooksForUserCreateTagRequest(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() - - testSuccessfulGitHooksForUserCreateTagRequest(t, ctx) + testWithFeature(t, featureflag.GoUserCreateTag, testSuccessfulGitHooksForUserCreateTagRequest) } func testSuccessfulGitHooksForUserCreateTagRequest(t *testing.T, ctx context.Context) { @@ -282,6 +286,10 @@ func testSuccessfulGitHooksForUserCreateTagRequest(t *testing.T, ctx context.Con } func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) { + testWithFeature(t, featureflag.GoUserDeleteTag, testFailedUserDeleteTagRequestDueToValidation) +} + +func testFailedUserDeleteTagRequestDueToValidation(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -325,9 +333,6 @@ func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.desc, func(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() - _, err := client.UserDeleteTag(ctx, testCase.request) testhelper.RequireGrpcError(t, err, testCase.code) }) @@ -335,10 +340,7 @@ func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) { } func TestFailedUserDeleteTagDueToHooks(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() - - testFailedUserDeleteTagDueToHooks(t, ctx) + testWithFeature(t, featureflag.GoUserDeleteTag, testFailedUserDeleteTagDueToHooks) } func testFailedUserDeleteTagDueToHooks(t *testing.T, ctx context.Context) { @@ -380,6 +382,10 @@ func testFailedUserDeleteTagDueToHooks(t *testing.T, ctx context.Context) { } func TestFailedUserCreateTagDueToHooks(t *testing.T) { + testWithFeature(t, featureflag.GoUserCreateTag, testFailedUserCreateTagDueToHooks) +} + +func testFailedUserCreateTagDueToHooks(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -403,9 +409,6 @@ func TestFailedUserCreateTagDueToHooks(t *testing.T) { require.NoError(t, err) defer remove() - ctx, cancel := testhelper.Context() - defer cancel() - response, err := client.UserCreateTag(ctx, request) require.Nil(t, err) require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId) @@ -413,6 +416,10 @@ func TestFailedUserCreateTagDueToHooks(t *testing.T) { } func TestFailedUserCreateTagRequestDueToTagExistence(t *testing.T) { + testWithFeature(t, featureflag.GoUserCreateTag, testFailedUserCreateTagRequestDueToTagExistence) +} + +func testFailedUserCreateTagRequestDueToTagExistence(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -439,15 +446,16 @@ func TestFailedUserCreateTagRequestDueToTagExistence(t *testing.T) { User: testCase.user, } - ctx, cancel := testhelper.Context() - defer cancel() - response, err := client.UserCreateTag(ctx, request) require.NoError(t, err) require.Equal(t, response.Exists, true) } func TestFailedUserCreateTagRequestDueToValidation(t *testing.T) { + testWithFeature(t, featureflag.GoUserCreateTag, testFailedUserCreateTagRequestDueToValidation) +} + +func testFailedUserCreateTagRequestDueToValidation(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -485,6 +493,7 @@ func TestFailedUserCreateTagRequestDueToValidation(t *testing.T) { user: testhelper.TestUser, code: codes.FailedPrecondition, }, + // TODO: Test for TagExistsError } for _, testCase := range testCases { @@ -496,9 +505,6 @@ func TestFailedUserCreateTagRequestDueToValidation(t *testing.T) { User: testCase.user, } - ctx, cancel := testhelper.Context() - defer cancel() - _, err := client.UserCreateTag(ctx, request) testhelper.RequireGrpcError(t, err, testCase.code) }) diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index e649869c2746156d83047f551808cfa20495b394..5aecb1a70960c4f565adc40a995a7f22290068aa 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -371,7 +371,7 @@ func (s *server) FindTag(ctx context.Context, in *gitalypb.FindTagRequest) (*git var tag *gitalypb.Tag - if tag, err = findTag(ctx, in.GetRepository(), in.GetTagName()); err != nil { + if tag, err = RawFindTag(ctx, in.GetRepository(), in.GetTagName()); err != nil { return nil, helper.ErrInternal(err) } @@ -412,7 +412,7 @@ func parseTagLine(c *catfile.Batch, tagLine string) (*gitalypb.Tag, error) { } } -func findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byte) (*gitalypb.Tag, error) { +func RawFindTag(ctx context.Context, repository *gitalypb.Repository, tagName []byte) (*gitalypb.Tag, error) { tagCmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{ Name: "tag", diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go index 7671a74fbbf81979b19c549c0542af1a3f2cac5a..2d8046bbd252aabccf3cac8291b00ef092bc4ffe 100644 --- a/internal/metadata/featureflag/feature_flags.go +++ b/internal/metadata/featureflag/feature_flags.go @@ -41,6 +41,10 @@ var ( GoUserUpdateSubmodule = FeatureFlag{Name: "go_user_update_submodule", OnByDefault: false} // GoFetchRemote enables the Go implementation of FetchRemote GoFetchRemote = FeatureFlag{Name: "go_fetch_remote", OnByDefault: false} + // GoUserDeleteTag enables the Go implementation of UserDeleteTag + GoUserDeleteTag = FeatureFlag{Name: "go_user_delete_tag", OnByDefault: false} + // GoUserCreateTag enables the Go implementation of UserCreateTag + GoUserCreateTag = FeatureFlag{Name: "go_user_create_tag", OnByDefault: false} ) // All includes all feature flags. @@ -60,6 +64,8 @@ var All = []FeatureFlag{ GoResolveConflicts, GoUserUpdateSubmodule, GoFetchRemote, + GoUserDeleteTag, + GoUserCreateTag, } const (