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 1 commit
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
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

ptgott marked this conversation as resolved.
Show resolved Hide resolved
type Redirect struct {
ptgott marked this conversation as resolved.
Show resolved Hide resolved
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.
ptgott marked this conversation as resolved.
Show resolved Hide resolved
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 := path.Join(teleportClonePath, "docs", "config.json")
ptgott marked this conversation as resolved.
Show resolved Hide resolved
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))
ptgott marked this conversation as resolved.
Show resolved Hide resolved
}

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/"
ptgott marked this conversation as resolved.
Show resolved Hide resolved

// 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
ptgott marked this conversation as resolved.
Show resolved Hide resolved
}
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
Loading