Merge pull request #289 from gravitational/paul.gottschling/2024-11-0…

…6-docs-redirs

Add a workflow to enforce new docs redirects

bot/internal/bot/docpaths.go

@@ -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"))
+	}
+
+	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
+}

bot/internal/bot/docpaths_test.go

@@ -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))
+		})
+	}
+}

bot/internal/github/github.go

@@ -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.
@@ -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(),
 			})
 		}