changelog: Added crosland link
Added notice to changelog errors that indicate only Cusky builds are supported. HTML errors now display the Crosland link to retrieve the changelog for ChromiumOS builds.
BUG=b/160901711
TEST=unittests, run local
Change-Id: Ibecb75bd5ccbbdb848024b99355e33770a3959e2
diff --git a/src/cmd/changelog-cli/main.go b/src/cmd/changelog-cli/main.go
index f2e9f7c..7b6a622 100755
--- a/src/cmd/changelog-cli/main.go
+++ b/src/cmd/changelog-cli/main.go
@@ -78,7 +78,7 @@
if err != nil {
return fmt.Errorf("generateChangelog: failed to create http client: \n%v", err)
}
- sourceToTargetChanges, targetToSourceChanges, err := changelog.Changelog(httpClient, source, target, instance, manifestRepo, -1)
+ sourceToTargetChanges, targetToSourceChanges, err := changelog.Changelog(httpClient, source, target, instance, manifestRepo, "", -1)
if err != nil {
return fmt.Errorf("generateChangelog: error retrieving changelog between builds %s and %s on GoB instance: %s with manifest repository: %s\n%v",
source, target, instance, manifestRepo, err)
diff --git a/src/cmd/changelog-webapp/app.yaml b/src/cmd/changelog-webapp/app.yaml
index 38ba74c..c365fc5 100644
--- a/src/cmd/changelog-webapp/app.yaml
+++ b/src/cmd/changelog-webapp/app.yaml
@@ -23,3 +23,4 @@
COS_INTERNAL_FALLBACK_GERRIT_INSTANCE_NAME: "cos-internal-fallback-gerrit-instance"
COS_INTERNAL_GOB_INSTANCE_NAME: "cos-internal-gob-instance"
COS_INTERNAL_MANIFEST_REPO_NAME: "cos-internal-manifest-repo"
+ CROSLAND_NAME: "crosland-url"
diff --git a/src/cmd/changelog-webapp/controllers/pageHandlers.go b/src/cmd/changelog-webapp/controllers/pageHandlers.go
index 20bf4e5..31a558e 100644
--- a/src/cmd/changelog-webapp/controllers/pageHandlers.go
+++ b/src/cmd/changelog-webapp/controllers/pageHandlers.go
@@ -40,6 +40,7 @@
internalFallbackGerritInstance string
internalGoBInstance string
internalManifestRepo string
+ croslandURL string
externalGerritInstance string
externalFallbackGerritInstance string
externalGoBInstance string
@@ -78,6 +79,10 @@
if err != nil {
log.Fatalf("Failed to retrieve secret for COS_INTERNAL_MANIFEST_REPO_NAME with key name %s\n%v", os.Getenv("COS_INTERNAL_MANIFEST_REPO_NAME"), err)
}
+ croslandURL, err = getSecret(client, os.Getenv("CROSLAND_NAME"))
+ if err != nil {
+ log.Fatalf("Failed to retrieve secret for CROSLAND_NAME with key name %s\n%v", os.Getenv("CROSLAND_NAME"), err)
+ }
externalGerritInstance = os.Getenv("COS_EXTERNAL_GERRIT_INSTANCE")
externalFallbackGerritInstance = os.Getenv("COS_EXTERNAL_FALLBACK_GERRIT_INSTANCE")
externalGoBInstance = os.Getenv("COS_EXTERNAL_GOB_INSTANCE")
@@ -337,7 +342,7 @@
http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect)
return
}
- added, removed, utilErr := changelog.Changelog(httpClient, source, target, instance, manifestRepo, querySize)
+ added, removed, utilErr := changelog.Changelog(httpClient, source, target, instance, manifestRepo, croslandURL, querySize)
if utilErr != nil {
log.Errorf("error retrieving changelog between builds %s and %s on GoB instance: %s with manifest repository: %s\n%v\n",
source, target, externalGoBInstance, externalManifestRepo, utilErr)
diff --git a/src/pkg/changelog/changelog.go b/src/pkg/changelog/changelog.go
index 866b929..265e399 100644
--- a/src/pkg/changelog/changelog.go
+++ b/src/pkg/changelog/changelog.go
@@ -105,7 +105,7 @@
}
buildNum := strings.Replace(build[2], "-", ".", 3)
log.Debugf("resolveImageName: image name %s was resolved to build number %s", imageName, buildNum)
- return strings.Replace(build[2], "-", ".", 3)
+ return buildNum
}
// Creates a unique identifier for a repo + branch pairing.
@@ -320,7 +320,7 @@
//
// The second changelog contains all commits that are present in the source build
// but not present in the target build
-func Changelog(httpClient *http.Client, source string, target string, host string, repo string, querySize int) (map[string]*RepoLog, map[string]*RepoLog, utils.ChangelogError) {
+func Changelog(httpClient *http.Client, source, target, host, repo, croslandURL string, querySize int) (map[string]*RepoLog, map[string]*RepoLog, utils.ChangelogError) {
if httpClient == nil {
log.Error("httpClient is nil")
return nil, nil, utils.InternalServerError
@@ -335,13 +335,14 @@
if err != nil {
return nil, nil, err
}
- sourceRepos, err := mappedManifest(manifestClient, repo, sourceBuildNum)
- if err != nil {
- return nil, nil, err
- }
- targetRepos, err := mappedManifest(manifestClient, repo, targetBuildNum)
- if err != nil {
- return nil, nil, err
+ sourceRepos, sourceErr := mappedManifest(manifestClient, repo, sourceBuildNum)
+ targetRepos, targetErr := mappedManifest(manifestClient, repo, targetBuildNum)
+ if sourceErr != nil && sourceErr.HTTPCode() == "404" && targetErr != nil && targetErr.HTTPCode() == "404" {
+ return nil, nil, utils.BothBuildsNotFound(croslandURL, source, target)
+ } else if sourceErr != nil {
+ return nil, nil, sourceErr
+ } else if targetErr != nil {
+ return nil, nil, targetErr
}
clients[host] = manifestClient
diff --git a/src/pkg/changelog/changelog_test.go b/src/pkg/changelog/changelog_test.go
index e309b8e..8a17a8a 100644
--- a/src/pkg/changelog/changelog_test.go
+++ b/src/pkg/changelog/changelog_test.go
@@ -64,7 +64,7 @@
httpClient, _ := getHTTPClient()
// Test invalid source
- additions, removals, err := Changelog(httpClient, "15", "15043.0.0", cosInstance, defaultManifestRepo, -1)
+ additions, removals, err := Changelog(httpClient, "15", "15043.0.0", cosInstance, defaultManifestRepo, "", -1)
if additions != nil {
t.Errorf("changelog failed, expected nil additions, got %v", additions)
} else if removals != nil {
@@ -76,7 +76,7 @@
}
// Test invalid target
- additions, removals, err = Changelog(httpClient, "15043.0.0", "abx", cosInstance, defaultManifestRepo, -1)
+ additions, removals, err = Changelog(httpClient, "15043.0.0", "abx", cosInstance, defaultManifestRepo, "", -1)
if additions != nil {
t.Errorf("changelog failed, expected nil additions, got %v", additions)
} else if removals != nil {
@@ -88,7 +88,7 @@
}
// Test invalid instance
- additions, removals, err = Changelog(httpClient, "15036.0.0", "15041.0.0", "com", defaultManifestRepo, -1)
+ additions, removals, err = Changelog(httpClient, "15036.0.0", "15041.0.0", "com", defaultManifestRepo, "", -1)
if additions != nil {
t.Errorf("changelog failed, expected nil additions, got %v", additions)
} else if removals != nil {
@@ -100,7 +100,7 @@
}
// Test invalid manifest repo
- additions, removals, err = Changelog(httpClient, "15036.0.0", "15041.0.0", cosInstance, "cos/not-a-repo", -1)
+ additions, removals, err = Changelog(httpClient, "15036.0.0", "15041.0.0", cosInstance, "cos/not-a-repo", "", -1)
if additions != nil {
t.Errorf("changelog failed, expected nil additions, got %v", additions)
} else if removals != nil {
@@ -112,7 +112,7 @@
}
// Test build number higher than latest release
- additions, removals, err = Changelog(httpClient, "15036.0.0", "99999.0.0", cosInstance, defaultManifestRepo, -1)
+ additions, removals, err = Changelog(httpClient, "15036.0.0", "99999.0.0", cosInstance, defaultManifestRepo, "", -1)
if additions != nil {
t.Errorf("changelog failed, expected nil additions, got %v", additions)
} else if removals != nil {
@@ -124,7 +124,7 @@
}
// Test manifest with remote urls specified and no default URL
- additions, removals, err = Changelog(httpClient, "1.0.0", "2.0.0", cosInstance, defaultManifestRepo, -1)
+ additions, removals, err = Changelog(httpClient, "1.0.0", "2.0.0", cosInstance, defaultManifestRepo, "", -1)
if additions == nil {
t.Errorf("changelog failed, expected additions, got nil")
} else if removals == nil {
@@ -248,7 +248,7 @@
"9bc12bb411f357188d008864f80dfba43210b9d8",
"bf0dd3757826b9bc9d7082f5f749ff7615d4bcb3",
}
- additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, -1)
+ additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", -1)
if err != nil {
t.Errorf("changelog failed, expected no error, got %v", err)
} else if len(removals) != 0 {
@@ -292,7 +292,7 @@
"src/platform2",
"src/third_party/chromiumos-overlay",
}
- additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, -1)
+ additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", -1)
if err != nil {
t.Errorf("changelog failed, expected no error, got %v", err)
}
@@ -306,7 +306,7 @@
source = "15030.0.0"
target = "15050.0.0"
querySize := 50
- additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, querySize)
+ additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", querySize)
if err != nil {
t.Errorf("changelog failed, expected no error, got %v", err)
} else if additions == nil {
@@ -332,7 +332,7 @@
// Test changelog handles manifest with non-matching repositories
source = "12871.1177.0"
target = "12871.1179.0"
- additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, querySize)
+ additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", querySize)
if err != nil {
t.Errorf("changelog failed, expected no error, got %v", err)
} else if len(removals) != 0 {
@@ -357,7 +357,7 @@
// Test with different release branches
source = "13310.1035.0"
target = "15000.0.0"
- additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, querySize)
+ additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", querySize)
if err != nil {
t.Errorf("Changelog failed, expected no error, got %v", err)
} else if len(additions) == 0 {
@@ -369,7 +369,7 @@
// Test empty repository
source = "0.0.0"
target = "2.0.0"
- additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, querySize)
+ additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", querySize)
if additions != nil {
t.Errorf("changelog failed, expected nil additions, got %v", additions)
} else if removals != nil {
@@ -383,7 +383,7 @@
// Test image name
source = "cos-rc-85-13310-1034-0"
target = "cos-rc-85-13310-1030-0"
- additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, querySize)
+ additions, removals, err = Changelog(httpClient, source, target, cosInstance, defaultManifestRepo, "", querySize)
if err != nil {
t.Errorf("Changelog failed, expected no error, got %v", err)
} else if len(additions) != 0 {
diff --git a/src/pkg/utils/changelogerror.go b/src/pkg/utils/changelogerror.go
index f5b415f..f59a206 100644
--- a/src/pkg/utils/changelogerror.go
+++ b/src/pkg/utils/changelogerror.go
@@ -23,6 +23,7 @@
"errors"
"fmt"
"regexp"
+ "strings"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
@@ -119,13 +120,63 @@
return innerErr
}
+func croslandLink(croslandURL, source, target string) string {
+ return fmt.Sprintf("%s/log/%s..%s", croslandURL, source, target)
+}
+
+// BothBuildsNotFound indicates that neither build was not found
+func BothBuildsNotFound(croslandURL, source, target string) *UtilChangelogError {
+ return &UtilChangelogError{
+ httpCode: "404",
+ header: "Build Not Found",
+ err: strings.Join([]string{
+ "The builds associated with input",
+ source,
+ "and",
+ target,
+ "cannot be found. It may be possible that the inputs are either invalid or both belong",
+ "to pre-Cusky builds. If both of the inputs belong to pre-Cusky builds, note that this tool only supports changelogs",
+ "between Cusky builds. Otherwise, please input valid build numbers (example: 13310.1035.0) or valid image names",
+ "(example: cos-rc-85-13310-1034-0).",
+ }, " "),
+ htmlErr: fmt.Sprintf("%s %s and %s %s<br><br>%s %s <a href=%s target=\"_blank\">%s</a>. %s %s",
+ "The builds associated with input",
+ source,
+ target,
+ "could not be found.",
+ "It may be possible that the inputs are either invalid or both belong to pre-Cusky builds.",
+ "If both of the inputs belong to pre-Cusky builds, please check",
+ croslandLink(croslandURL, source, target),
+ croslandLink(croslandURL, source, target),
+ "Otherwise, please input valid build numbers",
+ "(example: 13310.1035.0) or valid image names (example: cos-rc-85-13310-1034-0).",
+ ),
+ }
+}
+
// BuildNotFound returns a ChangelogError object for changelog indicating
// the desired build could not be found
func BuildNotFound(buildNumber string) *UtilChangelogError {
return &UtilChangelogError{
httpCode: "404",
header: "Build Not Found",
- err: fmt.Sprintf("The build associated with input %s cannot be found. Please input a valid build number (example: 13310.1035.0) or a valid image name (example: cos-rc-85-13310-1034-0).", buildNumber),
+ err: strings.Join([]string{
+ "The build associated with input",
+ buildNumber,
+ "cannot be found. It may be possible that the input is either invalid or belongs to a",
+ "pre-Cusky build. If you entered a pre-Cusky build number or image name, note that changelog between",
+ "pre-Cusky and Cusky builds are not supported. Otherwise, please input a valid build number",
+ "(example: 13310.1035.0) or a valid image name (example: cos-rc-85-13310-1034-0).",
+ }, " "),
+ htmlErr: fmt.Sprintf("%s %s %s<br><br>%s %s %s %s",
+ "The build associated with input",
+ buildNumber,
+ "cannot be found.",
+ "It may be possible that either the input is either invalid or belongs to a",
+ "pre-Cusky build. If you entered a pre-Cusky build number or image name, note that changelog between",
+ "pre-Cusky and Cusky builds are not supported. Otherwise, please input a valid build number",
+ "(example: 13310.1035.0) or a valid image name (example: cos-rc-85-13310-1034-0).",
+ ),
}
}
diff --git a/src/pkg/utils/changelogerror_test.go b/src/pkg/utils/changelogerror_test.go
index e636449..f24ea1a 100644
--- a/src/pkg/utils/changelogerror_test.go
+++ b/src/pkg/utils/changelogerror_test.go
@@ -17,6 +17,7 @@
import (
"errors"
"fmt"
+ "strings"
"testing"
"google.golang.org/grpc/codes"
@@ -28,7 +29,7 @@
)
func testCLLink(clID, instanceURL string) string {
- return fmt.Sprintf("<a href=\"%s/q/%s\" target=\"_blank\">CL %s</a>", instanceURL, clID, clID)
+ return fmt.Sprintf("<a href=\"%s/c/%s\" target=\"_blank\">CL %s</a>", instanceURL, clID, clID)
}
func TestGerritErrCode(t *testing.T) {
@@ -87,11 +88,69 @@
}
}
+func TestBothBuildsNotFound(t *testing.T) {
+ source := "1a"
+ target := "ddddddd"
+ croslandURL := "https://www.google.com"
+ expectedCode := "404"
+ expectedErrHeader := "Build Not Found"
+ expectedErrStr := strings.Join([]string{
+ "The builds associated with input",
+ source,
+ "and",
+ target,
+ "cannot be found. It may be possible that the inputs are either invalid or both belong",
+ "to pre-Cusky builds. If both of the inputs belong to pre-Cusky builds, note that this tool only supports changelogs",
+ "between Cusky builds. Otherwise, please input valid build numbers (example: 13310.1035.0) or valid image names",
+ "(example: cos-rc-85-13310-1034-0).",
+ }, " ")
+ expectedHTMLErrStr := fmt.Sprintf("%s %s and %s %s<br><br>%s %s <a href=%s target=\"_blank\">%s</a>. %s %s",
+ "The builds associated with input",
+ source,
+ target,
+ "could not be found.",
+ "It may be possible that the inputs are either invalid or both belong to pre-Cusky builds.",
+ "If both of the inputs belong to pre-Cusky builds, please check",
+ croslandLink(croslandURL, source, target),
+ croslandLink(croslandURL, source, target),
+ "Otherwise, please input valid build numbers",
+ "(example: 13310.1035.0) or valid image names (example: cos-rc-85-13310-1034-0).",
+ )
+ err := BothBuildsNotFound(croslandURL, source, target)
+ if err.HTTPCode() != expectedCode {
+ t.Errorf("expected HTTP code %s, got %s", expectedCode, err.HTTPCode())
+ } else if err.Header() != expectedErrHeader {
+ t.Errorf("expected error header \"%s\", got %s", expectedErrHeader, err.Header())
+ } else if err.Error() != expectedErrStr {
+ t.Errorf("expected error string %s, got %s", expectedErrStr, err.Error())
+ } else if err.HTMLError() != expectedHTMLErrStr {
+ t.Errorf("expected html error string %s, got %s", expectedErrStr, err.HTMLError())
+ } else if err.Retryable() {
+ t.Errorf("expected retryable = false, got true")
+ }
+}
+
func TestBuildNotFound(t *testing.T) {
buildNumber := "15000.0.0"
expectedCode := "404"
expectedErrHeader := "Build Not Found"
- expectedErrStr := fmt.Sprintf("The build associated with input %s cannot be found. Please input a valid build number (example: 13310.1035.0) or a valid image name (example: cos-rc-85-13310-1034-0).", buildNumber)
+ expectedErrStr := strings.Join([]string{
+ "The build associated with input",
+ buildNumber,
+ "cannot be found. It may be possible that the input is either invalid or belongs to a",
+ "pre-Cusky build. If you entered a pre-Cusky build number or image name, note that changelog between",
+ "pre-Cusky and Cusky builds are not supported. Otherwise, please input a valid build number",
+ "(example: 13310.1035.0) or a valid image name (example: cos-rc-85-13310-1034-0).",
+ }, " ")
+ expectedHTMLErrStr := fmt.Sprintf("%s %s %s<br><br>%s %s %s %s",
+ "The build associated with input",
+ buildNumber,
+ "cannot be found.",
+ "It may be possible that either the input is either invalid or belongs to a",
+ "pre-Cusky build. If you entered a pre-Cusky build number or image name, note that changelog between",
+ "pre-Cusky and Cusky builds are not supported. Otherwise, please input a valid build number",
+ "(example: 13310.1035.0) or a valid image name (example: cos-rc-85-13310-1034-0).",
+ )
err := BuildNotFound(buildNumber)
if err.HTTPCode() != expectedCode {
t.Errorf("expected HTTP code %s, got %s", expectedCode, err.HTTPCode())
@@ -99,7 +158,7 @@
t.Errorf("expected error header \"%s\", got %s", expectedErrHeader, err.Header())
} else if err.Error() != expectedErrStr {
t.Errorf("expected error string %s, got %s", expectedErrStr, err.Error())
- } else if err.HTMLError() != expectedErrStr {
+ } else if err.HTMLError() != expectedHTMLErrStr {
t.Errorf("expected html error string %s, got %s", expectedErrStr, err.HTMLError())
} else if err.Retryable() {
t.Errorf("expected retryable = false, got true")
@@ -127,7 +186,7 @@
func TestCLLandingNotFound(t *testing.T) {
clID := "1540"
- expectedCode := "404"
+ expectedCode := "406"
expectedErrHeader := "No Build Found"
expectedErrStr := fmt.Sprintf("No build was found containing CL %s.", clID)
link := testCLLink(clID, testInstanceURL)
