Skip to content

Commit

Permalink
Disable markdown folding on gitlab < 11.1.
Browse files Browse the repository at this point in the history 
On startup, make a call to the /version endpoint of Gitlab to get which
version we're running. Then use that version to determine whether we
should use the markdown folding syntax where the code is expandable.
Versions of Gitlab < 11.1 don't support the "Common Mark" markdown
format.

Fixes #315
  • Loading branch information
@lkysow
lkysow committed Nov 29, 2018
1 parent 1263ccb commit ba04564
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 74 deletions.
14 changes: 12 additions & 2 deletions server/events/markdown_renderer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
147 changes: 91 additions & 56 deletions server/events/markdown_renderer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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:
Expand Down
92 changes: 78 additions & 14 deletions server/events/vcs/gitlab_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Loading

0 comments on commit ba04564

Please sign in to comment.