Fix FindBuild functionality for annotated git tags.
Recently, COS started using annotated tags for associating a git tag
with a manifest-snapshot commit. Due to this, the FindBuild
functionality broke as annotated tags creates a new git object and
gitiles package didn't provide any API for mapping the git object with
the commitSHA associated with manifest-snapshot.
Now, gerrit package is being used as it provides all the information
such as Object and Revision from which the commitSHA associated with
a commit in manifest-snapshot can be mapped to the git tag i.e.
buildnumber.
Also, handle the cases where the changelist/commitSHA is on main branch.
BUG=b/180129577
TEST=https://paste.googleplex.com/5381390490140672
Change-Id: I30c63a890f5e3092d2c7c6b2a16ebc1ed3d141a9
Reviewed-on: https://cos-review.googlesource.com/c/cos/tools/+/12431
Reviewed-by: Robert Kolchmeyer <rkolchmeyer@google.com>
Tested-by: Vaibhav Rustagi <vaibhavrustagi@google.com>
diff --git a/go.mod b/go.mod
index 091a1b8..2640d7f 100644
--- a/go.mod
+++ b/go.mod
@@ -5,6 +5,7 @@
require (
cloud.google.com/go v0.57.0
cloud.google.com/go/storage v1.10.0
+ github.com/andygrunwald/go-gerrit v0.0.0-20201231163137-46815e48bfe0
github.com/beevik/etree v1.1.0
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/google/go-cmp v0.5.1
diff --git a/go.sum b/go.sum
index c09867d..029c628 100644
--- a/go.sum
+++ b/go.sum
@@ -41,6 +41,8 @@
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
+github.com/andygrunwald/go-gerrit v0.0.0-20201231163137-46815e48bfe0 h1:1IlIh8TmY+eAX17cPIUzT4e5R5bQoEngAO5QFcGHbrA=
+github.com/andygrunwald/go-gerrit v0.0.0-20201231163137-46815e48bfe0/go.mod h1:soxaYLbAFToS0OelBriItCts/mtUZOuLBkCk1Xv4ZSo=
github.com/beevik/etree v1.1.0 h1:T0xke/WvNtMoCqgzPhkX2r4rjY3GDZFi+FjpRZY2Jbs=
github.com/beevik/etree v1.1.0/go.mod h1:r8Aw8JqVegEf0w2fDnATrX9VpkMcyFeM0FhwO62wh+A=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
@@ -98,6 +100,8 @@
github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=
github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
+github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no=
github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=
diff --git a/src/cmd/changelogctl/main.go b/src/cmd/changelogctl/main.go
index 211b61d..0838acb 100755
--- a/src/cmd/changelogctl/main.go
+++ b/src/cmd/changelogctl/main.go
@@ -38,9 +38,10 @@
"golang.org/x/oauth2"
"golang.org/x/oauth2/google"
- log "github.com/sirupsen/logrus"
"github.com/urfave/cli/v2"
"go.chromium.org/luci/common/api/gerrit"
+
+ log "github.com/sirupsen/logrus"
)
const (
diff --git a/src/pkg/changelog/changelog.go b/src/pkg/changelog/changelog.go
index f82bee0..3d6fea3 100644
--- a/src/pkg/changelog/changelog.go
+++ b/src/pkg/changelog/changelog.go
@@ -37,6 +37,7 @@
"cos.googlesource.com/cos/tools.git/src/pkg/utils"
"github.com/beevik/etree"
+
log "github.com/sirupsen/logrus"
gitilesApi "go.chromium.org/luci/common/api/gitiles"
gitilesProto "go.chromium.org/luci/common/proto/gitiles"
diff --git a/src/pkg/findbuild/findbuild.go b/src/pkg/findbuild/findbuild.go
index f1117eb..944e98d 100644
--- a/src/pkg/findbuild/findbuild.go
+++ b/src/pkg/findbuild/findbuild.go
@@ -34,8 +34,6 @@
package findbuild
import (
- "context"
-
"fmt"
"net/http"
"regexp"
@@ -46,11 +44,12 @@
"cos.googlesource.com/cos/tools.git/src/pkg/utils"
"github.com/beevik/etree"
- log "github.com/sirupsen/logrus"
- "go.chromium.org/luci/common/api/gerrit"
- gitilesApi "go.chromium.org/luci/common/api/gitiles"
"go.chromium.org/luci/common/proto/git"
"go.chromium.org/luci/common/proto/gitiles"
+
+ gerrit "github.com/andygrunwald/go-gerrit"
+ log "github.com/sirupsen/logrus"
+ gitilesApi "go.chromium.org/luci/common/api/gitiles"
gitilesProto "go.chromium.org/luci/common/proto/gitiles"
)
@@ -163,51 +162,50 @@
}
// queryCL retrieves the list of CLs matching a query from Gerrit
-func queryCL(client *gerrit.Client, clID, instanceURL string) (*gerrit.Change, utils.ChangelogError) {
- log.Debug("Retrieving CL List from Gerrit")
+func queryCL(client *gerrit.Client, clID, instanceURL string) (gerrit.ChangeInfo, utils.ChangelogError) {
+ log.Debugf("Retrieving CL List from Gerrit for clID: %q", clID)
query := queryString(clID)
- queryOptions := gerrit.ChangeQueryParams{
- Query: query,
- N: 1,
- Options: []string{"CURRENT_REVISION"},
- }
- ctx, cancel := context.WithTimeout(context.Background(), requestMaxAge)
- defer cancel()
- clList, _, err := client.ChangeQuery(ctx, queryOptions)
+ queryOptions := &gerrit.QueryChangeOptions{}
+ queryOptions.Query = []string{query}
+ queryOptions.AdditionalFields = []string{"CURRENT_REVISION"}
+ queryOptions.Limit = 1
+
+ clList, _, err := client.Changes.QueryChanges(queryOptions)
if err != nil {
log.Errorf("queryCL: Error retrieving change for input %s:\n%v", clID, err)
httpCode := utils.GerritErrCode(err)
if httpCode == "403" {
- return nil, utils.ForbiddenError
+ return gerrit.ChangeInfo{}, utils.ForbiddenError
} else if httpCode == "400" || httpCode == "404" {
- return nil, utils.CLNotFound(clID)
+ return gerrit.ChangeInfo{}, utils.CLNotFound(clID)
}
- return nil, utils.InternalServerError
+ return gerrit.ChangeInfo{}, utils.InternalServerError
}
- if len(clList) == 0 {
+ if len(*clList) == 0 {
log.Errorf("queryCL: CL with identifier %s not found", clID)
- return nil, utils.CLNotFound(clID)
+ return gerrit.ChangeInfo{}, utils.CLNotFound(clID)
}
- change := clList[0]
- if change.Submitted == "" {
+ change := (*clList)[0]
+ log.Debugf("Found CL: %+v", change)
+ if change.Submitted == nil {
log.Debugf("Provided CL identifier %s maps to an unsubmitted CL", clID)
- return nil, utils.CLNotSubmitted(strconv.Itoa(change.ChangeNumber), instanceURL)
+ return gerrit.ChangeInfo{}, utils.CLNotSubmitted(strconv.Itoa(change.Number), instanceURL)
}
return change, nil
}
-func getCLData(client *gerrit.Client, clID, instanceURL string) (*clData, utils.ChangelogError) {
+func getCLData(clID, instanceURL string, httpClient *http.Client) (*clData, utils.ChangelogError) {
log.Debugf("Retrieving CL data from Gerrit for changeID: %s", clID)
- change, err := queryCL(client, clID, instanceURL)
+ gerritClient, clientErr := gerrit.NewClient(instanceURL, httpClient)
+ if clientErr != nil {
+ log.Errorf("failed to establish Gerrit client for host %s:\n%v", instanceURL, clientErr)
+ return nil, utils.InternalServerError
+ }
+ change, err := queryCL(gerritClient, clID, instanceURL)
if err != nil {
return nil, err
}
log.Debugf("Target CL found with SHA %s on repo %s, branch %s", change.CurrentRevision, change.Project, change.Branch)
- parsedTime, parseErr := time.Parse("2006-01-02 15:04:05.000000000", change.Submitted)
- if parseErr != nil {
- log.Errorf("getTargetData: Error parsing submission time %s for CL %d:\n%v", change.Submitted, change.ChangeNumber, err)
- return nil, utils.InternalServerError
- }
// If a repository has non-conventional branch names, need to convert the
// repository branch name to a release branch name
release := change.Branch
@@ -218,20 +216,26 @@
release = rule.defaultRelease
}
}
+ // In case the branch associated with the change is "main", branch
+ // on the manifest-snapshot will be master.
+ if change.Branch == "main" {
+ release = "master"
+ }
// Strip chromium prefixes
project := change.Project
if matches := crosRepoRe.FindStringSubmatch(project); matches != nil {
project = matches[1]
}
+ submittedTime := *change.Submitted
return &clData{
- CLNum: strconv.Itoa(change.ChangeNumber),
+ CLNum: strconv.Itoa(change.Number),
InstanceURL: instanceURL,
Project: project,
Release: release,
Branch: change.Branch,
Revision: change.CurrentRevision,
- SearchStartRange: parsedTime,
- SearchEndRange: parsedTime.AddDate(0, 0, defaultSearchRange),
+ SearchStartRange: submittedTime.Time,
+ SearchEndRange: submittedTime.Time.AddDate(0, 0, defaultSearchRange),
}, nil
}
@@ -284,20 +288,23 @@
}
// repoTags retrieves all tags belonging to a repository
-func repoTags(client gitilesProto.GitilesClient, repo string) (*gitilesProto.RefsResponse, error) {
+func repoTags(client *gerrit.Client, repo string) (map[string]string, error) {
log.Debugf("Retrieving tags for repository %s", repo)
- request := &gitilesProto.RefsRequest{
- Project: repo,
- RefsPath: "refs/tags",
- }
- ctx, cancel := context.WithTimeout(context.Background(), requestMaxAge)
- defer cancel()
- res, err := client.Refs(ctx, request)
+ tagInfos, _, err := client.Projects.ListTags(repo, &gerrit.ProjectBaseOptions{})
if err != nil {
log.Errorf("error retrieving tags:\n%v", err)
return nil, err
}
- return res, nil
+ tags := make(map[string]string)
+ for _, tagInfo := range *tagInfos {
+ log.Debugf("Tag found: %+v", tagInfo)
+ commitSHA := tagInfo.Revision
+ if tagInfo.Object != "" {
+ commitSHA = tagInfo.Object
+ }
+ tags[tagInfo.Ref] = commitSHA
+ }
+ return tags, nil
}
// candidateBuildNums returns a list of build numbers from a list of possible
@@ -534,14 +541,27 @@
clData.SearchEndRange = clData.SearchStartRange.AddDate(0, 0, defaultSearchRange)
log.Debugf("CL submitted earlier than first build, set search range to starting time from %v to %v", clData.SearchStartRange, clData.SearchEndRange)
}
- tagResp, err := repoTags(gitilesClient, request.ManifestRepo)
+ // Creating a Gerrit client based on manifest-snapshot repository.
+ // The client will be used for finding information associated with
+ // an annotated git tag.
+ instanceURL, err := utils.CreateGerritURL(request.GitilesHost)
+ if err != nil {
+ log.Errorf("failed to create Gerrit URL from Gitiles Host %q: %v", request.GitilesHost, err)
+ return "", utils.InternalServerError
+ }
+ gerritClient, err := gerrit.NewClient(instanceURL, request.HTTPClient)
+ if err != nil {
+ log.Errorf("failed to establish Gerrit client for host %s:\n%v", instanceURL, err)
+ return "", utils.InternalServerError
+ }
+ tagResp, err := repoTags(gerritClient, request.ManifestRepo)
if err != nil {
log.Errorf("failed to retrieve tags for project %s:\n%v", request.ManifestRepo, err)
return "", utils.InternalServerError
}
cache := &iterCache{
GitilesClient: gitilesClient,
- Tags: tagResp.Revisions,
+ Tags: tagResp,
ManifestCommits: manifestCommits,
}
@@ -564,17 +584,12 @@
log.Error("expected non-nil request")
return nil, utils.InternalServerError
}
- gerritClient, err := gerrit.NewClient(request.HTTPClient, request.GerritHost)
- if err != nil {
- log.Errorf("failed to establish Gerrit client for host %s:\n%v", request.GerritHost, err)
- return nil, utils.InternalServerError
- }
gitilesClient, err := gitilesApi.NewRESTClient(request.HTTPClient, request.GitilesHost, true)
if err != nil {
log.Errorf("failed to establish Gitiles client for host %s:\n%v", request.GitilesHost, err)
return nil, utils.InternalServerError
}
- clData, clErr := getCLData(gerritClient, request.CL, request.GerritHost)
+ clData, clErr := getCLData(request.CL, request.GerritHost, request.HTTPClient)
if clErr != nil {
return nil, clErr
}
diff --git a/src/pkg/findbuild/findbuild_test.go b/src/pkg/findbuild/findbuild_test.go
index 96269e3..a7c6a25 100644
--- a/src/pkg/findbuild/findbuild_test.go
+++ b/src/pkg/findbuild/findbuild_test.go
@@ -21,7 +21,6 @@
"testing"
"time"
- "go.chromium.org/luci/common/api/gerrit"
"golang.org/x/oauth2"
"golang.org/x/oauth2/google"
)
@@ -34,7 +33,7 @@
)
func getHTTPClient() (*http.Client, error) {
- creds, err := google.FindDefaultCredentials(context.Background(), gerrit.OAuthScope)
+ creds, err := google.FindDefaultCredentials(context.Background(), "https://www.googleapis.com/auth/gerritcodereview")
if err != nil || len(creds.JSON) == 0 {
return nil, fmt.Errorf("no application default credentials found - run `gcloud auth application-default login` and try again")
}
@@ -102,7 +101,7 @@
ShouldFallback: false,
},
"non-existant CL": {
- Change: "9999999999",
+ Change: "9",
GerritHost: externalGerritURL,
GitilesHost: externalGitilesURL,
FallbackGerritHost: externalFallbackGerritURL,
@@ -178,6 +177,14 @@
OutputBuildNum: "12371.1001.0",
ShouldFallback: true,
},
+ "main branch": {
+ Change: "808c2270202fbd79367c4c46b6223e6dfc2d1d01",
+ GerritHost: externalGerritURL,
+ GitilesHost: externalGitilesURL,
+ FallbackGerritHost: externalFallbackGerritURL,
+ ManifestRepo: externalManifestRepo,
+ OutputBuildNum: "16101.0.0",
+ },
}
httpClient, _ := getHTTPClient()
diff --git a/src/pkg/utils/gobutils.go b/src/pkg/utils/gobutils.go
index a154441..f3b28bf 100644
--- a/src/pkg/utils/gobutils.go
+++ b/src/pkg/utils/gobutils.go
@@ -22,10 +22,12 @@
import (
"context"
"fmt"
+ "regexp"
"time"
- log "github.com/sirupsen/logrus"
"go.chromium.org/luci/common/proto/git"
+
+ log "github.com/sirupsen/logrus"
gitilesProto "go.chromium.org/luci/common/proto/gitiles"
)
@@ -124,3 +126,17 @@
log.Debugf("Retrieved %d commits from %s in %s\n", len(allCommits), repo, time.Since(start))
return allCommits, response.NextPageToken != "", nil
}
+
+// CreateGerritURL creates a Gerrit URL from a given
+// Gitiles Host URL. For example: If the given Gitiles
+// Host URL is: https://cos.googlesource.com, then it will
+// return https://cos-review.googlesource.com. In case the
+// Gitiles Host URL is in incorrect format, error is returned.
+func CreateGerritURL(gitilesHostURL string) (string, error) {
+ re := regexp.MustCompile("(.+://)?(.+?).googlesource.com")
+ ss := re.FindStringSubmatch(gitilesHostURL)
+ if len(ss) > 0 {
+ return fmt.Sprintf("https://%s-review.googlesource.com", ss[len(ss)-1]), nil
+ }
+ return "", fmt.Errorf("failed to created Gerrit Host URL from invalid Gitiles Host URL: %q", gitilesHostURL)
+}
diff --git a/src/pkg/utils/gobutils_test.go b/src/pkg/utils/gobutils_test.go
index 8b8013d..5a12948 100644
--- a/src/pkg/utils/gobutils_test.go
+++ b/src/pkg/utils/gobutils_test.go
@@ -211,3 +211,35 @@
})
}
}
+
+func TestCreateGerritURL(t *testing.T) {
+ tests := map[string]struct {
+ input string
+ output string
+ wantErr bool
+ }{
+ "SuccessHTTP": {
+ input: "https://cos.googlesource.com",
+ output: "https://cos-review.googlesource.com",
+ },
+ "SuccessWithoutHTTP": {
+ input: "cos.googlesource.com",
+ output: "https://cos-review.googlesource.com",
+ },
+ "Failure": {
+ input: "https://cos.googleSource.com",
+ wantErr: true,
+ },
+ }
+ for name, test := range tests {
+ t.Run(name, func(t *testing.T) {
+ gotOutput, err := CreateGerritURL(test.input)
+ if gotErr := err != nil; gotErr != test.wantErr {
+ t.Fatalf("CreateGerritURL(%s) = %v", test.input, err)
+ }
+ if !test.wantErr && gotOutput != test.output {
+ t.Fatalf("CreateGerritURL(%s), got %s, want %s", test.input, gotOutput, test.output)
+ }
+ })
+ }
+}
diff --git a/src/pkg/utils/utils.go b/src/pkg/utils/utils.go
index 7022039..d93efa0 100644
--- a/src/pkg/utils/utils.go
+++ b/src/pkg/utils/utils.go
@@ -16,8 +16,9 @@
"syscall"
"time"
- log "github.com/golang/glog"
"github.com/pkg/errors"
+
+ log "github.com/golang/glog"
)
var (
