Skip to content

fix(helm): add missing gateway permission fields in ConfigMap#654

Merged
thepagent merged 3 commits intoopenabdev:mainfrom
iamninihuang:fix/helm-configmap-v2
May 1, 2026
Merged

fix(helm): add missing gateway permission fields in ConfigMap#654
thepagent merged 3 commits intoopenabdev:mainfrom
iamninihuang:fix/helm-configmap-v2

Conversation

@iamninihuang
Copy link
Copy Markdown
Contributor

@iamninihuang iamninihuang commented Apr 30, 2026

What problem does it solve?

This PR fixes a bug where gateway permissions (allowedUsers, allowAllUsers, etc.)
were missing from the Helm Chart template.
It also improves robustness in NOTES.txt.\n\n

How does it solve it?

  1. Added missing fields to configmap.yaml.
  2. Added ID validation.
  3. Updated NOTES.txt to handle list-type commands safely.

@iamninihuang iamninihuang requested a review from thepagent as a code owner April 30, 2026 16:22
@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 30, 2026
@github-actions
Copy link
Copy Markdown

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

@github-actions github-actions Bot added the pending-screening PR awaiting automated screening label Apr 30, 2026
Copy link
Copy Markdown
Collaborator

@chaodu-agent chaodu-agent left a comment

Choose a reason for hiding this comment

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

🔴 Duplicate botUsername block

The PR adds a new botUsername block at the end of the new permission fields (lines 145-147 in the PR diff), but one already exists at lines 128-130 on main. After merge, the rendered TOML will contain two bot_username keys under [gateway], which will cause a TOML parse error or unexpected override.

Fix: Remove the duplicate block:

    allowed_users = {{ ($cfg.gateway).allowedUsers | default list | toJson }}
-    {{- if ($cfg.gateway).botUsername }}
-    bot_username = {{ ($cfg.gateway).botUsername | toJson }}
-    {{- end }}
    {{- end }}

I've pushed the fix to chaodu-agent/openab:fix/helm-configmap-v2 for reference.

Everything else in this PR looks solid — the toString wrapping in NOTES.txt and the ID validation with regexMatch are both good improvements. 👍

Copilot AI review requested due to automatic review settings May 1, 2026 04:04
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@thepagent thepagent merged commit 073959c into openabdev:main May 1, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days pending-screening PR awaiting automated screening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants