diff --git a/README.md b/README.md index 789f795..40110a1 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,21 @@ render: - name: SecretObjectReference package: sigs.k8s.io/gateway-api/apis/v1beta1 link: https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.SecretObjectReference + # Rewrite plain URLs in field, type and group/version doc comments to Markdown links. + # Markdown renderer only. Mappings target bare URLs: do not map a URL that already + # appears inside a Markdown link, as it would produce nested (invalid) Markdown. + linkMappings: + - url: https://example.com/old-page + link: docs-content://new/page.md + text: New page ``` +> [!NOTE] +> `linkMappings` are declared inline in the config file. This is convenient for a +> small, fixed set of mappings per repository. If you find yourself maintaining a +> large mapping set, consider whether the URLs should instead be linked directly in +> the source doc comments; external mapping-file support may be added later if needed. + ### Advanced Features #### Custom Markers diff --git a/config/config.go b/config/config.go index b1c792e..cdbe559 100644 --- a/config/config.go +++ b/config/config.go @@ -18,6 +18,7 @@ package config import ( + "fmt" "os" "github.com/goccy/go-yaml" @@ -52,8 +53,9 @@ const ( ) type RenderConfig struct { - KnownTypes []*KnownType `json:"knownTypes"` - KubernetesVersion string `json:"kubernetesVersion"` + KnownTypes []*KnownType `json:"knownTypes"` + KubernetesVersion string `json:"kubernetesVersion"` + LinkMappings []*LinkMapping `json:"linkMappings"` } type KnownType struct { @@ -62,6 +64,12 @@ type KnownType struct { Link string `json:"link"` } +type LinkMapping struct { + URL string `json:"url"` + Link string `json:"link"` + Text string `json:"text"` +} + const ( OutputModeSingle = "single" OutputModeGroup = "group" @@ -93,5 +101,12 @@ func Load(flags Flags) (*Config, error) { } conf.Flags = flags + + for i, lm := range conf.Render.LinkMappings { + if lm.URL == "" || lm.Link == "" || lm.Text == "" { + return nil, fmt.Errorf("render.linkMappings[%d]: url, link, and text are all required", i) + } + } + return &conf, nil } diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 0000000..b68f3f9 --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,91 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoad_LinkMappingsValidation(t *testing.T) { + tests := []struct { + name string + yaml string + wantErr bool + }{ + { + name: "valid mapping", + yaml: `render: + linkMappings: + - url: https://example.com/old + link: docs-content://new/page.md + text: New page +`, + }, + { + name: "empty url is rejected", + yaml: `render: + linkMappings: + - url: "" + link: docs-content://new/page.md + text: New page +`, + wantErr: true, + }, + { + name: "missing link is rejected", + yaml: `render: + linkMappings: + - url: https://example.com/old + text: New page +`, + wantErr: true, + }, + { + name: "missing text is rejected", + yaml: `render: + linkMappings: + - url: https://example.com/old + link: docs-content://new/page.md +`, + wantErr: true, + }, + { + name: "no mappings is valid", + yaml: `render: {} +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(path, []byte(tt.yaml), 0o600)) + + _, err := Load(Flags{Config: path}) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/renderer/markdown.go b/renderer/markdown.go index a270c10..aa24f11 100644 --- a/renderer/markdown.go +++ b/renderer/markdown.go @@ -20,6 +20,7 @@ import ( "fmt" "io/fs" "os" + "slices" "strings" "text/template" @@ -72,6 +73,7 @@ func (m *MarkdownRenderer) ToFuncMap() template.FuncMap { "RenderLocalLink": m.RenderLocalLink, "RenderType": m.RenderType, "RenderTypeLink": m.RenderTypeLink, + "RewriteLinks": m.RewriteLinks, "SafeID": m.SafeID, "ShouldRenderType": m.ShouldRenderType, "TypeID": m.TypeID, @@ -148,10 +150,29 @@ func (m *MarkdownRenderer) RenderGVLink(gv types.GroupVersionDetails) string { return m.RenderLocalLink(gv.GroupVersionString()) } +func (m *MarkdownRenderer) RewriteLinks(text string) string { + if m == nil || m.conf == nil { + return text + } + // Apply longer URLs first so that when one mapped URL is a prefix of another + // (e.g. ".../old" vs ".../old-page"), the more specific match wins regardless + // of the order mappings are declared in the config. + mappings := slices.Clone(m.conf.Render.LinkMappings) + slices.SortStableFunc(mappings, func(a, b *config.LinkMapping) int { + return len(b.URL) - len(a.URL) + }) + for _, lm := range mappings { + text = strings.ReplaceAll(text, lm.URL, m.RenderExternalLink(lm.Link, lm.Text)) + } + return text +} + func (m *MarkdownRenderer) RenderFieldDoc(text string) string { + out := m.RewriteLinks(text) + // Escape the pipe character, which has special meaning for Markdown as a way to format tables // so that including | in a comment does not result in wonky tables. - out := strings.ReplaceAll(text, "|", "\\|") + out = strings.ReplaceAll(out, "|", "\\|") // Escape the curly bracket character. out = strings.ReplaceAll(out, "{", "\\{") diff --git a/renderer/markdown_test.go b/renderer/markdown_test.go index ddd1063..8a6a83f 100644 --- a/renderer/markdown_test.go +++ b/renderer/markdown_test.go @@ -35,6 +35,147 @@ func newTestConfig(t *testing.T) *config.Config { return conf } +func TestMarkdownRenderer_RewriteLinks(t *testing.T) { + conf := &config.Config{ + Render: config.RenderConfig{ + LinkMappings: []*config.LinkMapping{ + { + URL: "https://example.com/old", + Link: "docs-content://new/page.md", + Text: "New page", + }, + { + URL: "https://example.com/other", + Link: "kibana://reference/other.md", + Text: "Other", + }, + }, + }, + } + r := &MarkdownRenderer{conf: conf} + + tests := []struct { + name string + renderer *MarkdownRenderer + text string + want string + }{ + { + name: "single substitution", + renderer: r, + text: "See https://example.com/old for details.", + want: "See [New page](docs-content://new/page.md) for details.", + }, + { + name: "multiple substitutions", + renderer: r, + text: "See https://example.com/old and https://example.com/other.", + want: "See [New page](docs-content://new/page.md) and [Other](kibana://reference/other.md).", + }, + { + name: "no match leaves text unchanged", + renderer: r, + text: "See https://example.com/unmapped for details.", + want: "See https://example.com/unmapped for details.", + }, + { + name: "no mappings configured", + renderer: &MarkdownRenderer{conf: &config.Config{}}, + text: "See https://example.com/old for details.", + want: "See https://example.com/old for details.", + }, + { + name: "nil config", + renderer: &MarkdownRenderer{conf: nil}, + text: "See https://example.com/old for details.", + want: "See https://example.com/old for details.", + }, + { + name: "nil renderer", + renderer: nil, + text: "See https://example.com/old for details.", + want: "See https://example.com/old for details.", + }, + { + name: "multiple occurrences of the same URL are all rewritten", + renderer: r, + text: "See https://example.com/old and again https://example.com/old.", + want: "See [New page](docs-content://new/page.md) and again [New page](docs-content://new/page.md).", + }, + { + // The shorter, prefix URL is declared first; the longer/more specific + // URL must still win. RewriteLinks sorts by descending URL length, so + // the result is independent of config order. + name: "longer URL wins over shorter prefix regardless of order", + renderer: &MarkdownRenderer{conf: &config.Config{ + Render: config.RenderConfig{ + LinkMappings: []*config.LinkMapping{ + {URL: "https://example.com/old", Link: "docs-content://other.md", Text: "Other"}, + {URL: "https://example.com/old-page", Link: "docs-content://new/page.md", Text: "New page"}, + }, + }, + }}, + text: "See https://example.com/old-page for details.", + want: "See [New page](docs-content://new/page.md) for details.", + }, + { + // KNOWN LIMITATION: mappings target bare URLs. A URL that already + // appears inside a Markdown link gets rewritten too, producing nested + // (invalid) Markdown. This is acceptable because the feature is meant + // for plain URLs in godoc; do not map a URL that is already linked. + name: "KNOWN LIMITATION: URL inside an existing Markdown link produces nested output", + renderer: r, + text: "See [the page](https://example.com/old) for details.", + want: "See [the page]([New page](docs-content://new/page.md)) for details.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.renderer.RewriteLinks(tt.text)) + }) + } +} + +func TestMarkdownRenderer_RenderFieldDoc_appliesLinkMappings(t *testing.T) { + conf := &config.Config{ + Render: config.RenderConfig{ + LinkMappings: []*config.LinkMapping{ + { + URL: "https://example.com/old", + Link: "docs-content://new/page.md", + Text: "New page", + }, + }, + }, + } + r := &MarkdownRenderer{conf: conf} + + got := r.RenderFieldDoc("See https://example.com/old for details.") + assert.Equal(t, "See [New page](docs-content://new/page.md) for details.", got) +} + +func TestMarkdownRenderer_RenderFieldDoc_rewriteThenEscape(t *testing.T) { + conf := &config.Config{ + Render: config.RenderConfig{ + LinkMappings: []*config.LinkMapping{ + { + URL: "https://example.com/old", + Link: "docs-content://new/page.md", + Text: "New page", + }, + }, + }, + } + r := &MarkdownRenderer{conf: conf} + + // Link rewriting happens before pipe-escaping: the URL becomes a Markdown + // link and the literal pipe in the surrounding text is still escaped so it + // does not break the Markdown table. + got := r.RenderFieldDoc("See https://example.com/old (a | b) for details.") + assert.Equal(t, "See [New page](docs-content://new/page.md) (a \\| b) for details.", got) +} + func TestMarkdownRenderer_TemplateValue(t *testing.T) { tests := []struct { name string diff --git a/templates/markdown/gv_details.tpl b/templates/markdown/gv_details.tpl index 30ad0d7..caddcbb 100644 --- a/templates/markdown/gv_details.tpl +++ b/templates/markdown/gv_details.tpl @@ -3,7 +3,7 @@ ## {{ $gv.GroupVersionString }} -{{ $gv.Doc }} +{{ markdownRewriteLinks $gv.Doc }} {{- if $gv.Kinds }} ### Resource Types diff --git a/templates/markdown/type.tpl b/templates/markdown/type.tpl index 7d89d04..f8b91a6 100644 --- a/templates/markdown/type.tpl +++ b/templates/markdown/type.tpl @@ -6,7 +6,7 @@ {{ if $type.IsAlias }}_Underlying type:_ _{{ markdownRenderTypeLink $type.UnderlyingType }}_{{ end }} -{{ $type.Doc }} +{{ markdownRewriteLinks $type.Doc }} {{ if $type.Validation -}} _Validation:_ diff --git a/test/api/v1/groupversion_info.go b/test/api/v1/groupversion_info.go index 26c7436..919d835 100644 --- a/test/api/v1/groupversion_info.go +++ b/test/api/v1/groupversion_info.go @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -// Package v1 contains API Schema definitions for the webapp v1 API group +// Package v1 contains API Schema definitions for the webapp v1 API group. See https://example.com/old-page for more. // +kubebuilder:object:generate=true // +groupName=webapp.test.k8s.elastic.co package v1 diff --git a/test/api/v1/guestbook_types.go b/test/api/v1/guestbook_types.go index 8eb71bc..d89557c 100644 --- a/test/api/v1/guestbook_types.go +++ b/test/api/v1/guestbook_types.go @@ -121,9 +121,9 @@ const ( // +kubebuilder:validation:Minimum=1 type PositiveInt int -// GuestbookEntry defines an entry in a guest book. +// GuestbookEntry defines an entry in a guest book. See https://example.com/old-page for more. type GuestbookEntry struct { - // Name of the guest (pipe | should be escaped) + // Name of the guest (pipe | should be escaped). See https://example.com/old-page for naming guidance. // +kubebuilder:validation:Required // +kubebuilder:validation:MaxLength=80 // +kubebuilder:validation:Pattern=`0*[a-z0-9]*[a-z]*[0-9]` diff --git a/test/config.yaml b/test/config.yaml index 2ebede7..4f655e2 100644 --- a/test/config.yaml +++ b/test/config.yaml @@ -18,3 +18,7 @@ render: - name: SecretObjectReference package: sigs.k8s.io/gateway-api/apis/v1beta1 link: https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.SecretObjectReference + linkMappings: + - url: https://example.com/old-page + link: docs-content://new/page.md + text: New page diff --git a/test/expected.asciidoc b/test/expected.asciidoc index dff065f..c409249 100644 --- a/test/expected.asciidoc +++ b/test/expected.asciidoc @@ -37,7 +37,7 @@ _Underlying type:_ _string_ [id="{anchor_prefix}-webapp-test-k8s-elastic-co-v1"] === webapp.test.k8s.elastic.co/v1 -Package v1 contains API Schema definitions for the webapp v1 API group +Package v1 contains API Schema definitions for the webapp v1 API group. See https://example.com/old-page for more. .Resource Types - xref:{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-embedded[$$Embedded$$] @@ -148,7 +148,7 @@ Guestbook is the Schema for the guestbooks API. -GuestbookEntry defines an entry in a guest book. +GuestbookEntry defines an entry in a guest book. See https://example.com/old-page for more. @@ -160,7 +160,7 @@ GuestbookEntry defines an entry in a guest book. [cols="20a,50a,15a,15a", options="header"] |=== | Field | Description | Default | Validation -| *`name`* __string__ | Name of the guest (pipe \| should be escaped) + | | MaxLength: 80 + +| *`name`* __string__ | Name of the guest (pipe \| should be escaped). See https://example.com/old-page for naming guidance. + | | MaxLength: 80 + Pattern: `0\*[a-z0-9]*[a-z]*[0-9]` + Required: \{} + diff --git a/test/expected.md b/test/expected.md index 538adf6..a1a1042 100644 --- a/test/expected.md +++ b/test/expected.md @@ -28,7 +28,7 @@ _Appears in:_ ## webapp.test.k8s.elastic.co/v1 -Package v1 contains API Schema definitions for the webapp v1 API group +Package v1 contains API Schema definitions for the webapp v1 API group. See [New page](docs-content://new/page.md) for more. ### Resource Types - [Embedded](#embedded) @@ -118,7 +118,7 @@ _Appears in:_ -GuestbookEntry defines an entry in a guest book. +GuestbookEntry defines an entry in a guest book. See [New page](docs-content://new/page.md) for more. @@ -127,7 +127,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `name` _string_ | Name of the guest (pipe \| should be escaped) | | MaxLength: 80
Pattern: `0*[a-z0-9]*[a-z]*[0-9]`
Required: \{\}
| +| `name` _string_ | Name of the guest (pipe \| should be escaped). See [New page](docs-content://new/page.md) for naming guidance. | | MaxLength: 80
Pattern: `0*[a-z0-9]*[a-z]*[0-9]`
Required: \{\}
| | `tags` _string array_ | Tags of the entry. | | items:Pattern: `[a-z]*`
| | `time` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta)_ | Time of entry | | | | `comment` _string_ | Comment by guest. This can be a multi-line comment.
Like this one.
Now let's test a list:
* a
* b
Another isolated comment.
Looks good? | | Pattern: `0*[a-z0-9]*[a-z]*[0-9]*\|\s`
| diff --git a/test/hide.md b/test/hide.md index d274f96..0fe4130 100644 --- a/test/hide.md +++ b/test/hide.md @@ -31,7 +31,7 @@ _Appears in:_ ## webapp.test.k8s.elastic.co/v1 -Package v1 contains API Schema definitions for the webapp v1 API group +Package v1 contains API Schema definitions for the webapp v1 API group. See [New page](docs-content://new/page.md) for more. ### Resource Types - [Embedded](#embedded) @@ -119,7 +119,7 @@ _Appears in:_ -GuestbookEntry defines an entry in a guest book. +GuestbookEntry defines an entry in a guest book. See [New page](docs-content://new/page.md) for more. @@ -128,7 +128,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `name` _string_ | Name of the guest (pipe \| should be escaped) | | MaxLength: 80
Pattern: `0*[a-z0-9]*[a-z]*[0-9]`
Required: \{\}
| +| `name` _string_ | Name of the guest (pipe \| should be escaped). See [New page](docs-content://new/page.md) for naming guidance. | | MaxLength: 80
Pattern: `0*[a-z0-9]*[a-z]*[0-9]`
Required: \{\}
| | `tags` _string array_ | Tags of the entry. | | items:Pattern: `[a-z]*`
| | `time` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#time-v1-meta)_ | Time of entry | | | | `comment` _string_ | Comment by guest. This can be a multi-line comment.
Like this one.
Now let's test a list:
* a
* b
Another isolated comment.
Looks good? | | Pattern: `0*[a-z0-9]*[a-z]*[0-9]*\|\s`
| diff --git a/test/templates/markdown/gv_details.tpl b/test/templates/markdown/gv_details.tpl index 103ae17..c39794c 100644 --- a/test/templates/markdown/gv_details.tpl +++ b/test/templates/markdown/gv_details.tpl @@ -3,7 +3,7 @@ ## {{ $gv.GroupVersionString }} -{{ $gv.Doc }} +{{ markdownRewriteLinks $gv.Doc }} {{- if index $gv.Markers "special" }} *Important: This package is special and should be treated differently.* diff --git a/test/templates/markdown/type.tpl b/test/templates/markdown/type.tpl index 1ef5d62..61f39e0 100644 --- a/test/templates/markdown/type.tpl +++ b/test/templates/markdown/type.tpl @@ -6,7 +6,7 @@ {{ if $type.IsAlias }}_Underlying type:_ _{{ markdownRenderTypeLink $type.UnderlyingType }}_{{ end }} -{{ $type.Doc }} +{{ markdownRewriteLinks $type.Doc }} {{ if $type.Validation -}} _Validation:_