Repository: jenkins-x/lighthouse
Pull request #905 (2020-08-13 20:10:13)

feat: Make config-updater file name in comment a link
Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>


Patch:
diff --git a/pkg/plugins/approve/approve.go b/pkg/plugins/approve/approve.go
index 3e632a34..bebce35e 100644
--- a/pkg/plugins/approve/approve.go
+++ b/pkg/plugins/approve/approve.go
@@ -146,7 +146,7 @@ func helpProvider(config *plugins.Configuration, enabledRepos []string) (*plugin
 }
 
 func handleGenericCommentEvent(pc plugins.Agent, ce scmprovider.GenericCommentEvent) error {
-	baseURL, err := url.Parse(ce.Link)
+	baseURL, err := url.Parse(ce.IssueLink)
 	if err != nil {
 		return errors.Wrapf(err, "failed to parse URL %s", ce.Link)
 	}
diff --git a/pkg/plugins/approve/approvers/owners.go b/pkg/plugins/approve/approvers/owners.go
index 873e350d..b9aa3b50 100644
--- a/pkg/plugins/approve/approvers/owners.go
+++ b/pkg/plugins/approve/approvers/owners.go
@@ -575,7 +575,7 @@ func (a ApprovedFile) String() string {
 	if strings.HasSuffix(a.filepath, ".md") {
 		fullOwnersPath = a.filepath
 	}
-	link := urlForProvider(a.providerType, a.baseURL, a.owner, a.repo, a.branch, fullOwnersPath)
+	link := util.BlobURLForProvider(a.providerType, a.baseURL, a.owner, a.repo, a.branch, fullOwnersPath)
 
 	return fmt.Sprintf("- ~~[%s](%s)~~ [%v]\n", fullOwnersPath, link, strings.Join(a.approvers.List(), ","))
 }
@@ -585,25 +585,10 @@ func (ua UnapprovedFile) String() string {
 	if strings.HasSuffix(ua.filepath, ".md") {
 		fullOwnersPath = ua.filepath
 	}
-	link := urlForProvider(ua.providerType, ua.baseURL, ua.owner, ua.repo, ua.branch, fullOwnersPath)
+	link := util.BlobURLForProvider(ua.providerType, ua.baseURL, ua.owner, ua.repo, ua.branch, fullOwnersPath)
 	return fmt.Sprintf("- **[%s](%s)**\n", fullOwnersPath, link)
 }
 
-func urlForProvider(providerType string, baseURL *url.URL, owner, repo, branch string, ownersPath string) string {
-	switch providerType {
-	case "stash":
-		u := fmt.Sprintf("%s/projects/%s/repos/%s/browse/%v", strings.TrimSuffix(baseURL.String(), "/"), strings.ToUpper(owner), repo, ownersPath)
-		if branch != "master" {
-			u = fmt.Sprintf("%s?at=%s", u, url.QueryEscape("refs/heads/"+branch))
-		}
-		return u
-	case "gitlab":
-		return fmt.Sprintf("%s/%s/%s/-/blob/%s/%v", strings.TrimSuffix(baseURL.String(), "/"), owner, repo, branch, ownersPath)
-	default:
-		return fmt.Sprintf("%s/%s/%s/blob/%s/%v", strings.TrimSuffix(baseURL.String(), "/"), owner, repo, branch, ownersPath)
-	}
-}
-
 // GenerateTemplate takes a template, name and data, and generates
 // the corresponding string.
 func GenerateTemplate(templ, name string, data interface{}) (string, error) {
diff --git a/pkg/plugins/updateconfig/updateconfig.go b/pkg/plugins/updateconfig/updateconfig.go
index 9903ffbd..c0e3fb91 100644
--- a/pkg/plugins/updateconfig/updateconfig.go
+++ b/pkg/plugins/updateconfig/updateconfig.go
@@ -21,6 +21,7 @@ import (
 	"bytes"
 	"compress/gzip"
 	"fmt"
+	"net/url"
 	"path"
 	"strings"
 	"unicode/utf8"
@@ -29,7 +30,9 @@ import (
 	config2 "github.com/jenkins-x/lighthouse/pkg/config"
 	"github.com/jenkins-x/lighthouse/pkg/pluginhelp"
 	"github.com/jenkins-x/lighthouse/pkg/plugins"
+	"github.com/jenkins-x/lighthouse/pkg/util"
 	zglob "github.com/mattn/go-zglob"
+	errors2 "github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	coreapi "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -75,6 +78,7 @@ type scmProviderClient interface {
 	GetPullRequestChanges(org, repo string, number int) ([]*scm.Change, error)
 	GetFile(org, repo, filepath, commit string) ([]byte, error)
 	GetPullRequest(org, repo string, number int) (*scm.PullRequest, error)
+	ProviderType() string
 }
 
 type commentPruner interface {
@@ -349,6 +353,13 @@ func handle(spc scmProviderClient, kc corev1.ConfigMapsGetter, cp commentPruner,
 
 	var validationErrors []string
 
+	baseURL, err := url.Parse(pr.Link)
+	if err != nil {
+		return errors2.Wrapf(err, "failed to parse URL %s", pr.Link)
+	}
+	baseURL.Path = ""
+	baseURL.RawQuery = ""
+
 	for cm, data := range toUpdate {
 		fg := &scmFileGetter{
 			org:    org,
@@ -378,7 +389,8 @@ func handle(spc scmProviderClient, kc corev1.ConfigMapsGetter, cp commentPruner,
 				yamlErr = yaml.Unmarshal(content, &m)
 			}
 			if yamlErr != nil {
-				validationErrors = append(validationErrors, fmt.Sprintf("In file %s for config map %s:\n%s", upd.Filename, cm.Name, indentErrMsg(yamlErr)))
+				link := util.BlobURLForProvider(spc.ProviderType(), baseURL, org, repo, pr.Head.Sha, upd.Filename)
+				validationErrors = append(validationErrors, fmt.Sprintf("In file [%s](%s) for config map **%s**:\n%s", upd.Filename, link, cm.Name, indentErrMsg(yamlErr)))
 			}
 		}
 	}
diff --git a/pkg/util/scmclient.go b/pkg/util/scmclient.go
index 5adc9e3b..9f10fdd6 100644
--- a/pkg/util/scmclient.go
+++ b/pkg/util/scmclient.go
@@ -4,7 +4,9 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"os"
+	"strings"
 
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/go-scm/scm/factory"
@@ -139,3 +141,19 @@ func GetSCMToken(gitKind string) (string, error) {
 func HMACToken() string {
 	return os.Getenv("HMAC_TOKEN")
 }
+
+// BlobURLForProvider gets the link to the blob for an individual file in a commit or branch
+func BlobURLForProvider(providerType string, baseURL *url.URL, owner, repo, branch string, fullPath string) string {
+	switch providerType {
+	case "stash":
+		u := fmt.Sprintf("%s/projects/%s/repos/%s/browse/%v", strings.TrimSuffix(baseURL.String(), "/"), strings.ToUpper(owner), repo, fullPath)
+		if branch != "master" {
+			u = fmt.Sprintf("%s?at=%s", u, url.QueryEscape("refs/heads/"+branch))
+		}
+		return u
+	case "gitlab":
+		return fmt.Sprintf("%s/%s/%s/-/blob/%s/%v", strings.TrimSuffix(baseURL.String(), "/"), owner, repo, branch, fullPath)
+	default:
+		return fmt.Sprintf("%s/%s/%s/blob/%s/%v", strings.TrimSuffix(baseURL.String(), "/"), owner, repo, branch, fullPath)
+	}
+}


Test patch:
diff --git a/pkg/plugins/updateconfig/updateconfig_test.go b/pkg/plugins/updateconfig/updateconfig_test.go
index 0a1b54f2..e052265a 100644
--- a/pkg/plugins/updateconfig/updateconfig_test.go
+++ b/pkg/plugins/updateconfig/updateconfig_test.go
@@ -62,6 +62,7 @@ func TestUpdateConfig(t *testing.T) {
 		Author: scm.User{
 			Login: "foo",
 		},
+		Link: "https://github.com/kubernetes/kubernetes/pull/1",
 	}
 
 	testcases := []struct {
@@ -1014,7 +1015,7 @@ func TestUpdateConfig(t *testing.T) {
 			},
 			errorCommentContains: []string{
 				"Validation error founds in config map file(s):",
-				"In file invalid/invalid-yaml-config.yaml for config map config:",
+				"In file [invalid/invalid-yaml-config.yaml](https://github.com/kubernetes/kubernetes/blob/abcdef/invalid/invalid-yaml-config.yaml) for config map **config**:",
 				"> error converting YAML to JSON: yaml: line 2: mapping values are not allowed in this context",
 			},
 			expectedStatus: []*scm.StatusInput{{
@@ -1055,7 +1056,7 @@ func TestUpdateConfig(t *testing.T) {
 			},
 			errorCommentContains: []string{
 				"Validation error founds in config map file(s):",
-				"In file invalid/invalid-config.yaml for config map config:",
+				"In file [invalid/invalid-config.yaml](https://github.com/kubernetes/kubernetes/blob/abcdef/invalid/invalid-config.yaml) for config map **config**:",
 				"> error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type config.Config",
 			},
 			expectedStatus: []*scm.StatusInput{{
@@ -1103,8 +1104,8 @@ func TestUpdateConfig(t *testing.T) {
 			},
 			errorCommentContains: []string{
 				"Validation error founds in config map file(s):",
-				"In file invalid/invalid-yaml-config.yaml for config map config:",
-				"In file invalid/invalid-yaml-plugins.yaml for config map plugins:",
+				"In file [invalid/invalid-yaml-config.yaml](https://github.com/kubernetes/kubernetes/blob/abcdef/invalid/invalid-yaml-config.yaml) for config map **config**:",
+				"In file [invalid/invalid-yaml-plugins.yaml](https://github.com/kubernetes/kubernetes/blob/abcdef/invalid/invalid-yaml-plugins.yaml) for config map **plugins**:",
 				"> error converting YAML to JSON: yaml: line 2: mapping values are not allowed in this context",
 			},
 			expectedStatus: []*scm.StatusInput{{