Skip to content
Open
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
24 changes: 18 additions & 6 deletions internal/docs/alert_rule_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,25 @@ Alert rule templates require Elastic Stack version 9.2.0 or later.

`)

builder.WriteString("The following alert rule templates are available:\n\n")

for _, template := range templates {
builder.WriteString(fmt.Sprintf("**%s**\n\n", template.Attributes.Name))
builder.WriteString(fmt.Sprintf("%s\n\n", template.Attributes.Description))
}
builder.WriteString("**The following alert rule templates are available:**\n\n")
renderAlertRuleCollapsibleTable(&builder, templates)
builder.WriteString("\n")
}
Comment on lines -84 to 90
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering in another approach to avoid being breaking change this PR.

I think it could be applied the new rendering depending on the spec version that the package manifest has.

So for instance, if spec version is >= 3.6.0, then the rendering process would use the new collapsible tables, and if not it would use the previous rendering.

As this method receives the packagetRoot it could be extracted the spec version from there.

I did a test locally with these changes:

  • This snippet changes the rendering depending on the spec
  • as example, 3.6.0 is chosen version to use the new rendering
 func renderAlertRuleTemplates(packageRoot string, linksMap linkMap) (string, error) {
+       manifest, err := packages.ReadPackageManifestFromPackageRoot(packageRoot)
+       if err != nil {
+               return "", fmt.Errorf("failed to read package manifest: %w", err)
+       }
+
+       specVersion, err := semver.NewVersion(manifest.SpecVersion)
+       if err != nil {
+               return "", fmt.Errorf("failed to parse package format version %q: %w", manifest.SpecVersion, err)
+       }
+
+       useCollapseTable := specVersion.GreaterThanEqual(semver.MustParse("3.6.0"))
+
        templatesDir := filepath.Join(packageRoot, "kibana", "alerting_rule_template")
 
        if _, err := os.Stat(templatesDir); os.IsNotExist(err) {
@@ -30,7 +45,7 @@ func renderAlertRuleTemplates(packageRoot string, linksMap linkMap) (string, err
 
        var templates []alertRuleTemplate
 
-       err := filepath.WalkDir(templatesDir, func(path string, d fs.DirEntry, err error) error {
+       err = filepath.WalkDir(templatesDir, func(path string, d fs.DirEntry, err error) error {
                if err != nil {
                        return err
                }
@@ -81,15 +96,26 @@ Alert rule templates require Elastic Stack version 9.2.0 or later.
 
 `)
 
-               builder.WriteString("**The following alert rule templates are available:**\n\n")
-               renderAlertRuleCollapsibleTable(&builder, templates)
-               builder.WriteString("\n")
+               if useCollapseTable {
+                       renderAlertRuleCollapsibleTable(&builder, templates)
+               } else {
+                       renderAlertRuleTable(&builder, templates)
+               }
        }
 
        return builder.String(), nil
 }
 
+func renderAlertRuleTable(builder *strings.Builder, templates []alertRuleTemplate) {
+       builder.WriteString("The following alert rule templates are available:\n\n")
+       for _, template := range templates {
+               builder.WriteString(fmt.Sprintf("**%s**\n\n", template.Attributes.Name))
+               builder.WriteString(fmt.Sprintf("%s\n\n", template.Attributes.Description))
+       }
+}
+
 func renderAlertRuleCollapsibleTable(builder *strings.Builder, templates []alertRuleTemplate) {
+       builder.WriteString("**The following alert rule templates are available:**\n\n")
        builder.WriteString("<details>\n")
        builder.WriteString("<summary>View the alert rule templates</summary>\n\n")
        builder.WriteString("| Name | Description |\n")
@@ -102,4 +128,5 @@ func renderAlertRuleCollapsibleTable(builder *strings.Builder, templates []alert
                        escaper.Replace(description)))
        }
        builder.WriteString("\n</details>\n")
+       builder.WriteString("\n")
 }


return builder.String(), nil
}

func renderAlertRuleCollapsibleTable(builder *strings.Builder, templates []alertRuleTemplate) {
builder.WriteString("<details>\n")
builder.WriteString("<summary>View the alert rule templates</summary>\n\n")
builder.WriteString("| Name | Description |\n")
builder.WriteString("|---|---|\n")
for _, t := range templates {
name := strings.TrimSpace(t.Attributes.Name)
description := strings.TrimSpace(strings.ReplaceAll(t.Attributes.Description, "\n", " "))
builder.WriteString(fmt.Sprintf("| %s | %s |\n",
escaper.Replace(name),
escaper.Replace(description)))
}
builder.WriteString("\n</details>\n")
}
87 changes: 78 additions & 9 deletions internal/docs/alert_rule_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ func TestRenderAlertRuleTemplates(t *testing.T) {
expectEmpty: false,
validateFunc: func(t *testing.T, result string) {
assert.Contains(t, result, "Alert rule templates provide pre-defined configurations")
assert.Contains(t, result, "**Test Alert Rule**")
assert.Contains(t, result, "This is a test alert rule description")
assert.Contains(t, result, "<details>")
assert.Contains(t, result, "<summary>View the alert rule templates</summary>")
assert.Contains(t, result, "</details>")
assert.Contains(t, result, "| Name | Description |")
assert.Contains(t, result, "| Test Alert Rule | This is a test alert rule description |")
},
},
{
Expand Down Expand Up @@ -93,10 +96,11 @@ func TestRenderAlertRuleTemplates(t *testing.T) {
expectError: false,
expectEmpty: false,
validateFunc: func(t *testing.T, result string) {
assert.Contains(t, result, "**First Rule**")
assert.Contains(t, result, "First description")
assert.Contains(t, result, "**Second Rule**")
assert.Contains(t, result, "Second description")
assert.Contains(t, result, "<details>")
assert.Contains(t, result, "</details>")
assert.Contains(t, result, "| Name | Description |")
assert.Contains(t, result, "| First Rule | First description |")
assert.Contains(t, result, "| Second Rule | Second description |")
},
},
{
Expand All @@ -120,7 +124,7 @@ func TestRenderAlertRuleTemplates(t *testing.T) {
expectError: false,
expectEmpty: false,
validateFunc: func(t *testing.T, result string) {
assert.Contains(t, result, "**Valid Rule**")
assert.Contains(t, result, "| Valid Rule | Valid description |")
assert.NotContains(t, result, "ignored")
assert.NotContains(t, result, "readme")
},
Expand Down Expand Up @@ -149,8 +153,73 @@ func TestRenderAlertRuleTemplates(t *testing.T) {
expectError: false,
expectEmpty: false,
validateFunc: func(t *testing.T, result string) {
// Should only have one rule mentioned
assert.Equal(t, 1, strings.Count(result, "**Root Rule**"))
assert.Equal(t, 1, strings.Count(result, "Root Rule"))
assert.Contains(t, result, "| Root Rule | Root description |")
},
},
{
name: "table structure is correct",
setupFunc: func(t *testing.T) string {
tmpDir := t.TempDir()
templatesDir := filepath.Join(tmpDir, "kibana", "alerting_rule_template")
require.NoError(t, os.MkdirAll(templatesDir, 0o755))

template1 := `{
"attributes": {
"name": "First Rule Name",
"description": "First Rule Description"
}
}`
template2 := `{
"attributes": {
"name": "Second Rule Name",
"description": "Second Rule Description"
}
}`
require.NoError(t, os.WriteFile(filepath.Join(templatesDir, "rule1.json"), []byte(template1), 0o644))
require.NoError(t, os.WriteFile(filepath.Join(templatesDir, "rule2.json"), []byte(template2), 0o644))
return tmpDir
},
expectError: false,
expectEmpty: false,
validateFunc: func(t *testing.T, result string) {
assert.Contains(t, result, "<details>")
assert.Contains(t, result, "<summary>View the alert rule templates</summary>")
assert.Contains(t, result, "</details>")
assert.Contains(t, result, "| Name | Description |")
assert.Contains(t, result, "|---|---|")
assert.Contains(t, result, "| First Rule Name | First Rule Description |")
assert.Contains(t, result, "| Second Rule Name | Second Rule Description |")
},
},
{
name: "special characters are escaped",
setupFunc: func(t *testing.T) string {
tmpDir := t.TempDir()
templatesDir := filepath.Join(tmpDir, "kibana", "alerting_rule_template")
require.NoError(t, os.MkdirAll(templatesDir, 0o755))

template := `{
"attributes": {
"name": "Rule with *bold* and {braces}",
"description": "Description with <angle> and {curly} brackets"
}
}`
require.NoError(t, os.WriteFile(filepath.Join(templatesDir, "special.json"), []byte(template), 0o644))
return tmpDir
},
expectError: false,
expectEmpty: false,
validateFunc: func(t *testing.T, result string) {
assert.Contains(t, result, "<details>")
assert.Contains(t, result, "</details>")
assert.Contains(t, result, `\*bold\*`)
assert.Contains(t, result, `\{braces\}`)
assert.Contains(t, result, `\<angle\>`)
assert.Contains(t, result, `\{curly\}`)
assert.NotContains(t, result, "*bold*")
assert.NotContains(t, result, "{braces}")
assert.NotContains(t, result, "<angle>")
},
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the approach mentioned in the other comment is accepted, it should be added a new test case here:

+               {
+                       name: "list structure is correct",
+                       setupFunc: func(t *testing.T) string {
+                               tmpDir := t.TempDir()
+                               createPackageManifestFile(t, tmpDir, "3.5.0")
+                               templatesDir := filepath.Join(tmpDir, "kibana", "alerting_rule_template")
+                               require.NoError(t, os.MkdirAll(templatesDir, 0o755))
+
+                               template1 := `{
+                                       "attributes": {
+                                               "name": "First Rule Name",
+                                               "description": "First Rule Description"
+                                       }
+                               }`
+                               template2 := `{
+                                       "attributes": {
+                                               "name": "Second Rule Name",
+                                               "description": "Second Rule Description"
+                                       }
+                               }`
+                               require.NoError(t, os.WriteFile(filepath.Join(templatesDir, "rule1.json"), []byte(template1), 0o644))
+                               require.NoError(t, os.WriteFile(filepath.Join(templatesDir, "rule2.json"), []byte(template2), 0o644))
+                               return tmpDir
+                       },
+                       expectError: false,
+                       expectEmpty: false,
+                       validateFunc: func(t *testing.T, result string) {
+                               assert.NotContains(t, result, "<details>")
+                               assert.NotContains(t, result, "<summary>View the alert rule templates</summary>")
+                               assert.NotContains(t, result, "</details>")
+                               assert.Contains(t, result, "**First Rule Name**")
+                               assert.Contains(t, result, "First Rule Description")
+                               assert.Contains(t, result, "**Second Rule Name**")
+                               assert.Contains(t, result, "Second Rule Description")
+                       },
+               },

That requires a new helper:

func createPackageManifestFile(t *testing.T, packageRoot, specVersion string) {
       t.Helper()
       manifest := fmt.Sprintf(`format_version: %s
type: integration
version: 1.0.0
`, specVersion)
       manifestFile := filepath.Join(packageRoot, packages.PackageManifestFile)
       require.NoError(t, os.WriteFile(manifestFile, []byte(manifest), 0o644))
}

all the other test cases would need to add a call to this helper to create a package manifest file with spec 3.6.0. For instance:

@@ -226,6 +272,7 @@ func TestRenderAlertRuleTemplates(t *testing.T) {
                        name: "unreadable file",
                        setupFunc: func(t *testing.T) string {
                                tmpDir := t.TempDir()
+                               createPackageManifestFile(t, tmpDir, "3.6.0")
                                templatesDir := filepath.Join(tmpDir, "kibana", "alerting_rule_template")
                                require.NoError(t, os.MkdirAll(templatesDir, 0o755))
 
@@ -240,6 +287,7 @@ func TestRenderAlertRuleTemplates(t *testing.T) {
                        name: "invalid json file",
                        setupFunc: func(t *testing.T) string {
                                tmpDir := t.TempDir()
+                               createPackageManifestFile(t, tmpDir, "3.6.0")
                                templatesDir := filepath.Join(tmpDir, "kibana", "alerting_rule_template")
                                require.NoError(t, os.MkdirAll(templatesDir, 0o755))

{
Expand Down
9 changes: 7 additions & 2 deletions test/packages/other/alert_rule_templates/docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ For more information, refer to the [Elastic documentation](https://www.elastic.c

Alert rule templates require Elastic Stack version 9.2.0 or later.

The following alert rule templates are available:
**The following alert rule templates are available:**

**[MongoDB Replication] Replica member down**
<details>
<summary>View the alert rule templates</summary>

| Name | Description |
|---|---|
| [MongoDB Replication] Replica member down | |

</details>