diff --git a/server/events/markdown_renderer.go b/server/events/markdown_renderer.go index 124bd74dc8..ccbbd6887c 100644 --- a/server/events/markdown_renderer.go +++ b/server/events/markdown_renderer.go @@ -39,7 +39,12 @@ var ( ) // MarkdownRenderer renders responses as markdown. -type MarkdownRenderer struct{} +type MarkdownRenderer struct { + // GitlabSupportsCommonMark is true if the version of GitLab we're + // using supports the CommonMark markdown format. + // If we're not configured with a GitLab client, this will be false. + GitlabSupportsCommonMark bool +} // CommonData is data that all responses have. type CommonData struct { @@ -156,13 +161,18 @@ func (m *MarkdownRenderer) renderProjectResults(results []ProjectResult, common // shouldUseWrappedTmpl returns true if we should use the wrapped markdown // templates that collapse the output to make the comment smaller on initial -// load. +// load. Some VCS providers or versions of VCS providers don't support this +// syntax. func (m *MarkdownRenderer) shouldUseWrappedTmpl(vcsHost models.VCSHostType, output string) bool { // Bitbucket Cloud and Server don't support the folding markdown syntax. if vcsHost == models.BitbucketServer || vcsHost == models.BitbucketCloud { return false } + if vcsHost == models.Gitlab && !m.GitlabSupportsCommonMark { + return false + } + return strings.Count(output, "\n") > maxUnwrappedLines } diff --git a/server/events/markdown_renderer_test.go b/server/events/markdown_renderer_test.go index ddebe6edef..3d8d791e19 100644 --- a/server/events/markdown_renderer_test.go +++ b/server/events/markdown_renderer_test.go @@ -582,56 +582,74 @@ $$$ // VCS hosts during an error. func TestRenderProjectResults_WrappedErr(t *testing.T) { cases := []struct { - VCSHost models.VCSHostType - Output string - ShouldWrap bool + VCSHost models.VCSHostType + GitlabCommonMarkSupport bool + Output string + ShouldWrap bool }{ { - models.Github, - strings.Repeat("line\n", 1), - false, + VCSHost: models.Github, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.Github, - strings.Repeat("line\n", 13), - true, + VCSHost: models.Github, + Output: strings.Repeat("line\n", 13), + ShouldWrap: true, }, { - models.Gitlab, - strings.Repeat("line\n", 1), - false, + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: false, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.Gitlab, - strings.Repeat("line\n", 13), - true, + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: false, + Output: strings.Repeat("line\n", 13), + ShouldWrap: false, }, { - models.BitbucketCloud, - strings.Repeat("line\n", 1), - false, + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: true, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.BitbucketCloud, - strings.Repeat("line\n", 13), - false, + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: true, + Output: strings.Repeat("line\n", 13), + ShouldWrap: true, }, { - models.BitbucketServer, - strings.Repeat("line\n", 1), - false, + VCSHost: models.BitbucketCloud, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.BitbucketServer, - strings.Repeat("line\n", 13), - false, + VCSHost: models.BitbucketCloud, + Output: strings.Repeat("line\n", 13), + ShouldWrap: false, + }, + { + VCSHost: models.BitbucketServer, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, + }, + { + VCSHost: models.BitbucketServer, + Output: strings.Repeat("line\n", 13), + ShouldWrap: false, }, } - mr := events.MarkdownRenderer{} for _, c := range cases { t.Run(fmt.Sprintf("%s_%v", c.VCSHost.String(), c.ShouldWrap), func(t *testing.T) { + mr := events.MarkdownRenderer{ + GitlabSupportsCommonMark: c.GitlabCommonMarkSupport, + } + rendered := mr.Render(events.CommandResult{ ProjectResults: []events.ProjectResult{ { @@ -675,57 +693,74 @@ $$$ // VCS hosts for a single project. func TestRenderProjectResults_WrapSingleProject(t *testing.T) { cases := []struct { - VCSHost models.VCSHostType - Output string - ShouldWrap bool + VCSHost models.VCSHostType + GitlabCommonMarkSupport bool + Output string + ShouldWrap bool }{ { - models.Github, - strings.Repeat("line\n", 1), - false, + VCSHost: models.Github, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.Github, - strings.Repeat("line\n", 13), - true, + VCSHost: models.Github, + Output: strings.Repeat("line\n", 13), + ShouldWrap: true, + }, + { + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: false, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.Gitlab, - strings.Repeat("line\n", 1), - false, + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: false, + Output: strings.Repeat("line\n", 13), + ShouldWrap: false, }, { - models.Gitlab, - strings.Repeat("line\n", 13), - true, + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: true, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.BitbucketCloud, - strings.Repeat("line\n", 1), - false, + VCSHost: models.Gitlab, + GitlabCommonMarkSupport: true, + Output: strings.Repeat("line\n", 13), + ShouldWrap: true, }, { - models.BitbucketCloud, - strings.Repeat("line\n", 13), - false, + VCSHost: models.BitbucketCloud, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, }, { - models.BitbucketServer, - strings.Repeat("line\n", 1), - false, + VCSHost: models.BitbucketCloud, + Output: strings.Repeat("line\n", 13), + ShouldWrap: false, }, { - models.BitbucketServer, - strings.Repeat("line\n", 13), - false, + VCSHost: models.BitbucketServer, + Output: strings.Repeat("line\n", 1), + ShouldWrap: false, + }, + { + VCSHost: models.BitbucketServer, + Output: strings.Repeat("line\n", 13), + ShouldWrap: false, }, } - mr := events.MarkdownRenderer{} for _, c := range cases { for _, cmd := range []events.CommandName{events.PlanCommand, events.ApplyCommand} { t.Run(fmt.Sprintf("%s_%s_%v", c.VCSHost.String(), cmd.String(), c.ShouldWrap), func(t *testing.T) { + mr := events.MarkdownRenderer{ + GitlabSupportsCommonMark: c.GitlabCommonMarkSupport, + } var pr events.ProjectResult switch cmd { case events.PlanCommand: diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 314fa720e9..508ae7d2de 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -15,20 +15,32 @@ package vcs import ( "fmt" - "github.com/pkg/errors" - "github.com/runatlantis/atlantis/server/logging" "net" "net/url" "strings" + "github.com/hashicorp/go-version" + "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/logging" + "github.com/lkysow/go-gitlab" "github.com/runatlantis/atlantis/server/events/models" ) type GitlabClient struct { Client *gitlab.Client + // Version is set to the server version. + Version *version.Version } +// commonMarkSupported is a version constraint that is true when this version of +// GitLab supports CommonMark, a markdown specification. +// See https://about.gitlab.com/2018/07/22/gitlab-11-1-released/ +var commonMarkSupported = MustConstraint(">=11.1") + +// gitlabClientUnderTest is true if we're running under go test. +var gitlabClientUnderTest = false + // NewGitlabClient returns a valid GitLab client. func NewGitlabClient(hostname string, token string, logger *logging.SimpleLogger) (*GitlabClient, error) { client := &GitlabClient{ @@ -37,31 +49,40 @@ func NewGitlabClient(hostname string, token string, logger *logging.SimpleLogger // If not using gitlab.com we need to set the URL to the API. if hostname != "gitlab.com" { - // Check if they've also provided a scheme so we don't prepend it - // again. - scheme := "https" - hostname := hostname - schemeSplit := strings.Split(hostname, "://") - if len(schemeSplit) > 1 { - scheme = schemeSplit[0] - hostname = schemeSplit[1] + u, err := url.Parse(hostname) + if err != nil { + return nil, errors.Wrapf(err, "parsing hostname %q", hostname) } // Warn if this hostname isn't resolvable. The GitLab client // doesn't give good error messages in this case. - ips, err := net.LookupIP(hostname) + ips, err := net.LookupIP(u.Hostname()) if err != nil { - logger.Warn("unable to resolve %q: %s", hostname, err) + logger.Warn("unable to resolve %q: %s", u.Hostname(), err) } else if len(ips) == 0 { - logger.Warn("found no IPs while resolving %q", hostname) + logger.Warn("found no IPs while resolving %q", u.Hostname()) } - apiURL := fmt.Sprintf("%s://%s/api/v4/", scheme, hostname) + scheme := "https" + if u.Scheme != "" { + scheme = u.Scheme + } + apiURL := fmt.Sprintf("%s://%s/api/v4/", scheme, u.Host) if err := client.Client.SetBaseURL(apiURL); err != nil { return nil, errors.Wrapf(err, "setting GitLab API URL: %s", apiURL) } } + // Determine which version of GitLab is running. + if !gitlabClientUnderTest { + var err error + client.Version, err = client.GetVersion() + if err != nil { + return nil, err + } + logger.Info("determined GitLab is running version %s", client.Version.String()) + } + return client, nil } @@ -143,3 +164,46 @@ func (g *GitlabClient) GetMergeRequest(repoFullName string, pullNum int) (*gitla mr, _, err := g.Client.MergeRequests.GetMergeRequest(repoFullName, pullNum) return mr, err } + +// GetVersion returns the version of the Gitlab server this client is using. +func (g *GitlabClient) GetVersion() (*version.Version, error) { + req, err := g.Client.NewRequest("GET", "/version", nil, nil) + if err != nil { + return nil, err + } + versionResp := new(gitlab.Version) + _, err = g.Client.Do(req, versionResp) + if err != nil { + return nil, err + } + // We need to strip any "-ee" or similar from the resulting version because go-version + // uses that in its constraints and it breaks the comparison we're trying + // to do for Common Mark. + split := strings.Split(versionResp.Version, "-") + parsedVersion, err := version.NewVersion(split[0]) + if err != nil { + return nil, errors.Wrapf(err, "parsing response to /version: %q", versionResp.Version) + } + return parsedVersion, nil +} + +// SupportsCommonMark returns true if the version of Gitlab this client is +// using supports the CommonMark markdown format. +func (g *GitlabClient) SupportsCommonMark() bool { + // This function is called even if we didn't construct a gitlab client + // so we need to handle that case. + if g == nil { + return false + } + + return commonMarkSupported.Check(g.Version) +} + +// MustConstraint returns a constraint. It panics on error. +func MustConstraint(constraint string) version.Constraints { + c, err := version.NewConstraint(constraint) + if err != nil { + panic(err) + } + return c +} diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 8b42820bb9..16215a53cc 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -1,12 +1,16 @@ package vcs import ( - . "github.com/runatlantis/atlantis/testing" "testing" + + "github.com/hashicorp/go-version" + . "github.com/runatlantis/atlantis/testing" ) // Test that the base url gets set properly. func TestNewGitlabClient_BaseURL(t *testing.T) { + gitlabClientUnderTest = true + defer func() { gitlabClientUnderTest = false }() cases := []struct { Hostname string ExpBaseURL string @@ -33,3 +37,45 @@ func TestNewGitlabClient_BaseURL(t *testing.T) { }) } } + +// This function gets called even if GitlabClient is nil +// so we need to test that. +func TestGitlabClient_SupportsCommonMarkNil(t *testing.T) { + var gl *GitlabClient + Equals(t, false, gl.SupportsCommonMark()) +} + +func TestGitlabClient_SupportsCommonMark(t *testing.T) { + cases := []struct { + version string + exp bool + }{ + { + "11.0", + false, + }, + { + "11.1", + true, + }, + { + "11.2", + true, + }, + { + "12.0", + true, + }, + } + + for _, c := range cases { + t.Run(c.version, func(t *testing.T) { + vers, err := version.NewVersion(c.version) + Ok(t, err) + gl := GitlabClient{ + Version: vers, + } + Equals(t, c.exp, gl.SupportsCommonMark()) + }) + } +} diff --git a/server/server.go b/server/server.go index 2ae23a1fd1..5128e9ce65 100644 --- a/server/server.go +++ b/server/server.go @@ -205,7 +205,9 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { if err != nil && flag.Lookup("test.v") == nil { return nil, errors.Wrap(err, "initializing terraform") } - markdownRenderer := &events.MarkdownRenderer{} + markdownRenderer := &events.MarkdownRenderer{ + GitlabSupportsCommonMark: gitlabClient.SupportsCommonMark(), + } boltdb, err := boltdb.New(userConfig.DataDir) if err != nil { return nil, err