Skip to content

docs(memos): sync template standards updates#349

Open
mberlofa wants to merge 2 commits into
mainfrom
docs/memos-template-standards-sync
Open

docs(memos): sync template standards updates#349
mberlofa wants to merge 2 commits into
mainfrom
docs/memos-template-standards-sync

Conversation

@mberlofa

@mberlofa mberlofa commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • document optional ingressClassName rendering and networkPolicy.extraEgress behavior for Memos
  • expose Memos NetworkPolicy controls and empty ingress class option in the playground registry
  • register Memos in the playground site-sync map

Validation

  • make site-sync-check CHART=memos
  • npm run lint
  • npm run format:check
  • npm run build

Chart PR: helmforgedev/charts#671
Issue: helmforgedev/charts#633

Summary by CodeRabbit

  • New Features

    • Updated the olivetin chart playground with an expanded “Ingress Class” selector (including an empty choice).
    • Added a new NetworkPolicy collapsible section, gated by networkPolicy.enabled, including “Database Egress Port” (default 5432).
    • Included memos in the set of charts eligible for site-synced playground behavior.
  • Documentation

    • Expanded the configuration reference for Ingress and NetworkPolicy, clarifying how empty Ingress Class values work and how extraEgress interacts with egress isolation and DNS egress.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fe44a479-d602-4b65-9fe5-83133b3a86c8

📥 Commits

Reviewing files that changed from the base of the PR and between a37f814 and 74492b6.

📒 Files selected for processing (1)
  • src/pages/docs/charts/memos.mdx
✅ Files skipped from review due to trivial changes (1)
  • src/pages/docs/charts/memos.mdx

📝 Walkthrough

Walkthrough

Updates the olivetin playground config to add an empty Ingress Class option and a new NetworkPolicy section with a Database Egress Port field. Expands memos configuration docs for Ingress and NetworkPolicy, and registers memos in the playground site-sync mapping.

Changes

Playground Config and Documentation Updates

Layer / File(s) Summary
Olivetin playground config changes
src/data/playground-configs.ts
Adds an empty option to the Ingress Class select and introduces a collapsible NetworkPolicy section gated by networkPolicy.enabled, with a Database Egress Port field defaulting to 5432.
Memos documentation and playground registration
src/pages/docs/charts/memos.mdx, src/pages/playground.astro
Expands memos configuration reference documentation for Ingress and NetworkPolicy, and adds a memos entry to the siteSyncPlaygroundConfigs mapping.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • helmforgedev/site#320: Also updates src/pages/playground.astro to register a chart in the same site-sync playground mapping.
  • helmforgedev/site#322: Also extends src/pages/playground.astro’s chart registry with a new slug for playground filtering.
  • helmforgedev/site#330: Also modifies src/pages/playground.astro by adding another chart to the same playground config map.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the PR's Memos documentation and playground registry updates.
Description check ✅ Passed The description includes a summary, validation steps, and issue references, with only minor template deviations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/memos-template-standards-sync

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/data/playground-configs.ts (1)

8140-8146: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Memos ingress class options lack the empty option now documented in memos.mdx.

The updated memos.mdx documents setting ingress.ingressClassName: "" to omit spec.ingressClassName, mirroring the pattern already used for apache and now olivetin (options: ['', ...]). The memos entry here still only offers ['traefik', 'nginx'], so playground users can't exercise the documented behavior.

♻️ Proposed fix
         {
           label: 'Ingress Class',
           key: 'ingress.ingressClassName',
           type: 'select',
           default: 'traefik',
-          options: ['traefik', 'nginx'],
+          options: ['', 'traefik', 'nginx'],
           description: 'Ingress controller class',
         },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/data/playground-configs.ts` around lines 8140 - 8146, The Memos ingress
class selector is missing the empty option that is now documented, so update the
playground config entry for ingress.ingressClassName to include an empty choice
alongside the existing traefik and nginx values. Use the existing pattern from
the apache and olivetin config entries in playground-configs.ts, keeping the
same label, key, and description while extending the options list so users can
select "" and omit spec.ingressClassName.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/data/playground-configs.ts`:
- Around line 8140-8146: The Memos ingress class selector is missing the empty
option that is now documented, so update the playground config entry for
ingress.ingressClassName to include an empty choice alongside the existing
traefik and nginx values. Use the existing pattern from the apache and olivetin
config entries in playground-configs.ts, keeping the same label, key, and
description while extending the options list so users can select "" and omit
spec.ingressClassName.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7368d274-6320-4af0-abbf-bec0881de11f

📥 Commits

Reviewing files that changed from the base of the PR and between 365449a and a37f814.

📒 Files selected for processing (3)
  • src/data/playground-configs.ts
  • src/pages/docs/charts/memos.mdx
  • src/pages/playground.astro

@mberlofa

mberlofa commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Synced the Memos NetworkPolicy docs with helmforgedev/charts#671.

What changed:

  • Documented networkPolicy.egressIsolation for baseline DNS/HTTPS egress isolation without custom rules.
  • Documented networkPolicy.dnsEgress and its default kube-system / k8s-app: kube-dns selector.
  • Kept networkPolicy.extraEgress documented as also enabling isolation while appending custom rules.

Validation:

  • npm run format:check passed.
  • npm run lint passed.
  • npm run build passed.
  • make release-check REPO=site passed.
  • make attribution-check REPO=site passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant