Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a workflow to enforce new docs redirects #289

Merged
merged 2 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 125 additions & 0 deletions bot/internal/bot/docpaths.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package bot

import (
"context"
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"

"github.com/gravitational/shared-workflows/bot/internal/github"
"github.com/gravitational/trace"
)

// DocsConfig represents the structure of the config.json file found in the docs
// directory of a Teleport repo. We only use Redirects, so other fields have the
// "any" type.
type DocsConfig struct {
Navigation any
Variables any
Redirects []DocsRedirect
}

type DocsRedirect struct {
Source string
Destination string
Permanent bool
}

// CheckDocsPathsForMissingRedirects checks whether a PR has renamed or deleted
// any docs files and, if so, returns an error if any of these docs files does
// not correspond to a new redirect in the docs configuration file.
//
// teleportClonePath is a relative path to a gravitational/teleport clone. It is
// assumed that there is a file called docs/config.json at the root of the
// directory that lists redirects in the redirects field.
func (b *Bot) CheckDocsPathsForMissingRedirects(ctx context.Context, teleportClonePath string) error {
if teleportClonePath == "" {
return trace.Wrap(errors.New("unable to load Teleport documentation config with an empty path"))
ptgott marked this conversation as resolved.
Show resolved Hide resolved
}

docsConfigPath := filepath.Join(teleportClonePath, "docs", "config.json")
f, err := os.Open(docsConfigPath)
if err != nil {
return trace.BadParameter("unable to load Teleport documentation config with an empty path")
}
defer f.Close()

var c DocsConfig
if err := json.NewDecoder(f).Decode(&c); err != nil {
return trace.Wrap(err, "unable to load redirect configuration from %v: %v", docsConfigPath)
}

files, err := b.c.GitHub.ListFiles(ctx, b.c.Environment.Organization, b.c.Environment.Repository, b.c.Environment.Number)
if err != nil {
return trace.Wrap(err, "unable to fetch files for PR %v: %v", b.c.Environment.Number)
}

m := missingRedirectSources(c.Redirects, files)
if len(m) > 0 {
return trace.Wrap(err, "docs config at %v is missing redirects for the following renamed or deleted pages: %v", docsConfigPath, strings.Join(m, ","))
}

return nil
}

var docsPrefix = filepath.Join("docs", "pages")

// toURLPath converts a local docs page path to a URL path in the format found
// in a docs redirect configuration.
//
// If the name of the file is the same as the name of the containing directory,
// the page is a category index page. This means that the route Docusaurus
// generates for the page is the path to the containing directory, omitting the
// final path segment.
//
// See:
// https://docusaurus.io/docs/sidebar/autogenerated#category-index-convention
func toURLPath(p string) string {
// Redirect sources do not include the docs content directory path.
redir := strings.TrimPrefix(p, docsPrefix)
ext := filepath.Ext(redir)
// Redirect sources do not contain file extensions.
redir = strings.TrimSuffix(redir, ext)

// The file is a category index page, so return the path to its
// containing directory.
if strings.HasSuffix(filepath.Dir(redir), filepath.Base(redir)) {
redir = filepath.Dir(redir)
}

// Add a trailing slash to the final redirect path
return redir + "/"
}

// missingRedirectSources checks renamed or deleted docs pages in files to
// ensure that there is a corresponding redirect source in conf. For any missing
// redirects, it lists redirect sources that should be in conf.
func missingRedirectSources(conf []DocsRedirect, files github.PullRequestFiles) []string {
sources := make(map[string]struct{})
for _, s := range conf {
sources[s.Source] = struct{}{}
}

res := []string{}
for _, f := range files {
if !strings.HasPrefix(f.Name, docsPrefix) {
continue
}

switch f.Status {
case "renamed":
p := toURLPath(f.PreviousName)
if _, ok := sources[p]; !ok {
res = append(res, p)
}
case "removed":
p := toURLPath(f.Name)
if _, ok := sources[p]; !ok {
res = append(res, p)
}
}
}
return res
}
191 changes: 191 additions & 0 deletions bot/internal/bot/docpaths_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
package bot

import (
"testing"

"github.com/gravitational/shared-workflows/bot/internal/github"
"github.com/stretchr/testify/assert"
)

func TestMissingRedirectSources(t *testing.T) {
cases := []struct {
description string
files github.PullRequestFiles
redirects []DocsRedirect
expected []string
}{
{
description: "renamed docs path with adequate redirects",
files: github.PullRequestFiles{
{
Name: "docs/pages/databases/protect-mysql.mdx",
Additions: 0,
Deletions: 0,
Status: "renamed",
PreviousName: "docs/pages/databases/mysql.mdx",
},
},
redirects: []DocsRedirect{
{
Source: "/databases/mysql/",
Destination: "/databases/protect-mysql/",
Permanent: true,
},
},
expected: []string{},
},
{
description: "renamed docs path with missing redirects",
files: github.PullRequestFiles{
{
Name: "docs/pages/databases/protect-mysql.mdx",
Additions: 0,
Deletions: 0,
Status: "renamed",
PreviousName: "docs/pages/databases/mysql.mdx",
},
},
redirects: []DocsRedirect{},
expected: []string{"/databases/mysql/"},
},
{
description: "deleted docs path with adequate redirects",
files: github.PullRequestFiles{
{
Name: "docs/pages/databases/mysql.mdx",
Additions: 0,
Deletions: 0,
Status: "removed",
},
},
redirects: []DocsRedirect{
{
Source: "/databases/mysql/",
Destination: "/databases/protect-mysql/",
Permanent: true,
},
}, expected: []string{},
},
{
description: "deleted docs path with missing redirects",
files: github.PullRequestFiles{
{
Name: "docs/pages/databases/mysql.mdx",
Additions: 0,
Deletions: 200,
Status: "removed",
},
},
redirects: []DocsRedirect{},
expected: []string{"/databases/mysql/"},
},
{
description: "modified docs page",
files: github.PullRequestFiles{
{
Name: "docs/pages/databases/mysql.mdx",
Additions: 50,
Deletions: 15,
Status: "modified",
},
},
redirects: []DocsRedirect{},
expected: []string{},
},
{
description: "renamed docs path with nil redirects",
files: github.PullRequestFiles{
{
Name: "docs/pages/databases/protect-mysql.mdx",
Additions: 0,
Deletions: 0,
Status: "renamed",
PreviousName: "docs/pages/databases/mysql.mdx",
},
},
redirects: nil,
expected: []string{"/databases/mysql/"},
},
{
description: "redirects with nil files",
files: nil,
redirects: []DocsRedirect{
{
Source: "/databases/mysql/",
Destination: "/databases/protect-mysql/",
Permanent: true,
},
}, expected: []string{},
},
// Accommodate the Docusaurus logic of generating a route for a
// section index page, e.g., docs/pages/slug/slug.mdx, at a URL
// path that consists only of the containing directory, e.g.,
// /docs/slug/.
{
description: "renamed section index page with adequate redirects",
files: github.PullRequestFiles{
{
Name: "docs/pages/enroll-resources/databases/databases.mdx",
Additions: 0,
Deletions: 0,
Status: "renamed",
PreviousName: "docs/pages/databases/databases.mdx",
},
},
redirects: []DocsRedirect{
{
Source: "/databases/",
Destination: "/enroll-resources/databases/",
Permanent: true,
},
},
expected: []string{},
},
{
description: "renamed section index page with inadequate redirects",
files: github.PullRequestFiles{
{
Name: "docs/pages/enroll-resources/applications/applications.mdx",
Additions: 0,
Deletions: 0,
Status: "renamed",
PreviousName: "docs/pages/applications/applications.mdx",
},
},
redirects: []DocsRedirect{
{
Source: "/applications/",
Destination: "/enroll-resources/applications/applications/",
Permanent: true,
},
},
expected: []string{},
},
}

for _, c := range cases {
t.Run(c.description, func(t *testing.T) {
assert.Equal(t, c.expected, missingRedirectSources(c.redirects, c.files))
})
}
}

func Test_toURLPath(t *testing.T) {
cases := []struct {
description string
input string
expected string
}{
{
description: "section index page",
input: "docs/pages/databases/databases.mdx",
expected: "/databases/",
},
}

for _, c := range cases {
t.Run(c.description, func(t *testing.T) {
assert.Equal(t, c.expected, toURLPath(c.input))
})
}
}
13 changes: 9 additions & 4 deletions bot/internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ type PullRequestFile struct {
Deletions int
// Status is either added, removed, modified, renamed, copied, changed, unchanged
Status string
// PreviousName is the name of the file prior to renaming. The GitHub
// API only assigns this if Status is "renamed". For deleted files, the
// GitHub API uses Name.
PreviousName string
}

// PullRequestFiles is a list of pull request files.
Expand Down Expand Up @@ -410,10 +414,11 @@ func (c *Client) ListFiles(ctx context.Context, organization string, repository

for _, file := range page {
files = append(files, PullRequestFile{
Name: file.GetFilename(),
Additions: file.GetAdditions(),
Deletions: file.GetDeletions(),
Status: file.GetStatus(),
Name: file.GetFilename(),
Additions: file.GetAdditions(),
Deletions: file.GetDeletions(),
Status: file.GetStatus(),
PreviousName: file.GetPreviousFilename(),
})
}

Expand Down
Loading
Loading