Skip to content

Commit

Permalink
Add a workflow to enforce new docs redirects
Browse files Browse the repository at this point in the history
Add a `docpaths` workflow to the bot to ensure that any renamed or
deleted docs pages accompany a new redirect in the docs config file
(`config.json`). This way, we can prevent docs changes from introducing
404s, whether because of delays in search engine indexing or broken
links from Teleport-owned sites.

The workflow takes a path to a `config.json` file, loads the redirect
configuration, and checks whether all renamed and deleted pages
correspond to a redirect. This change adds a `docpaths` value of the
`workflow` flag and an optional `teleport-path` flag for the path to
a `gravitational/teleport` clone so the workflow can locate a docs
configuration file.
  • Loading branch information
ptgott committed Dec 24, 2024
1 parent b37f702 commit 5b46fbd
Show file tree
Hide file tree
Showing 4 changed files with 351 additions and 26 deletions.
122 changes: 122 additions & 0 deletions bot/internal/bot/docpaths.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package bot

import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"path"
"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 RedirectConfig
}

type RedirectConfig []Redirect

type Redirect struct {
Source string
Destination string
Permanent bool
}

// CheckDocsPathsForMissingRedirects takes the relative path to a
// gravitational/teleport clone. It assumes that there is a file called
// "docs/config.json" at the root of the directory that lists redirects in the
// "redirects" field.
//
// 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 configuration file.
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"))
}

docsConfigPath := path.Join(teleportClonePath, "docs", "config.json")
f, err := os.Open(docsConfigPath)
if err != nil {
return trace.Wrap(fmt.Errorf("unable to open docs config file at %v: %v", docsConfigPath, err))
}
defer f.Close()

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

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(fmt.Errorf("unable to fetch files for PR %v: %v", b.c.Environment.Number, err))
}

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

return nil
}

const docsPrefix = "docs/pages/"

// toURLPath converts a local docs page path to a URL path in the format found
// in a docs redirect configuration.
func toURLPath(p string) string {
d, f := filepath.Split(p)

// The page is a category index page, since the name of the file is the
// same as the name of the containing directory. 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
if strings.HasSuffix(d, strings.Replace(f, ".mdx", "/", 1)) {
p = d
}
return strings.Replace(
strings.Replace(p, docsPrefix, "/", 1),
".mdx", "/", 1)
}

// 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 RedirectConfig, 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 RedirectConfig
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: RedirectConfig{
{
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: RedirectConfig{},
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: RedirectConfig{
{
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: RedirectConfig{},
expected: []string{"/databases/mysql/"},
},
{
description: "modified docs page",
files: github.PullRequestFiles{
{
Name: "docs/pages/databases/mysql.mdx",
Additions: 50,
Deletions: 15,
Status: "modified",
},
},
redirects: RedirectConfig{},
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: RedirectConfig{
{
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: RedirectConfig{
{
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: RedirectConfig{
{
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

0 comments on commit 5b46fbd

Please sign in to comment.