Skip to content

fix(gateway): add deny-all sentinel to ACL plugin when no consumers exist#277

Merged
ron96g merged 1 commit into
mainfrom
feature/acl-deny-all-sentinel
Mar 18, 2026
Merged

fix(gateway): add deny-all sentinel to ACL plugin when no consumers exist#277
ron96g merged 1 commit into
mainfrom
feature/acl-deny-all-sentinel

Conversation

@ron96g
Copy link
Copy Markdown
Member

@ron96g ron96g commented Mar 17, 2026

Kong rejects an ACL plugin with an empty allow list due to schema
validation. When no realm defaults, route defaults, or ConsumeRoute
resources exist, the allow list was sent as an empty array, causing a
400 error from Kong.
Add a 'deny_all' sentinel group to the allow list when it would
otherwise be empty. No real consumer will ever belong to this group,
so the plugin is accepted by Kong while effectively blocking all
traffic to the route.

…xist

Kong rejects an ACL plugin with an empty allow list due to schema
validation. When no realm defaults, route defaults, or ConsumeRoute
resources exist, the allow list was sent as an empty array, causing a
400 error from Kong.
Add a '__deny_all__' sentinel group to the allow list when it would
otherwise be empty. No real consumer will ever belong to this group,
so the plugin is accepted by Kong while effectively blocking all
traffic to the route.
Copilot AI review requested due to automatic review settings March 17, 2026 16:25
@ron96g ron96g self-assigned this Mar 17, 2026
@ron96g ron96g added bug Something isn't working go Pull requests that update go code gateway Anything related to gateway domain labels Mar 17, 2026
Copy link
Copy Markdown
Contributor

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.

Pull request overview

Fixes Kong schema validation failures when configuring the ACL plugin with an empty allow list by inserting a deny-all sentinel group when no consumers are allowed for a route.

Changes:

  • Introduce plugin.DenyAllGroup ("__deny_all__") as a sentinel ACL group constant.
  • Update AccessControlFeature.Apply to add the sentinel group when the computed allow-set is empty.
  • Extend feature builder tests to assert the sentinel is not present when consumers exist and is present when none exist.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
gateway/pkg/kong/client/plugin/acl.go Adds exported DenyAllGroup sentinel constant for the ACL plugin.
gateway/internal/features/feature/access_control.go Injects the deny-all sentinel into ACL allow list when no consumers were added.
gateway/internal/features/builder_test.go Adds assertions and a new test covering the empty-consumer (deny-all) behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gateway/pkg/kong/client/plugin/acl.go
Comment thread gateway/internal/features/feature/access_control.go
Copy link
Copy Markdown
Member

@ellevaoche ellevaoche left a comment

Choose a reason for hiding this comment

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

For me it looks good!

But there is one small question: Is it necessary, that the consumer group "deny_all" is created upfront (one time setup task) via kong admin api?
I think not, because it's not validated by kong if the group really exist. It's only a string in the plugin, right?

@ron96g ron96g merged commit 0dd6017 into main Mar 18, 2026
121 checks passed
@ron96g ron96g deleted the feature/acl-deny-all-sentinel branch March 18, 2026 09:25
dtit-semantic-release Bot pushed a commit that referenced this pull request Mar 18, 2026
# [0.18.0](v0.17.0...v0.18.0) (2026-03-18)

### Bug Fixes

* **common-server:** race-condition and security improvements ([#272](#272)) ([9d08e02](9d08e02))
* **gateway:** add deny-all sentinel to ACL plugin when no consumers exist ([#277](#277)) ([0dd6017](0dd6017))

### Features

* **common-libs:** improve error handling, add feature flags, and fix logger type ([#260](#260)) ([03165a0](03165a0))
* **common-server:** unify latency format in logs ([#267](#267)) ([5fbb19e](5fbb19e))
* **gateway:** add buffering config, dynamic upstream, access control fix, and secret-manager integration ([#259](#259)) ([719c58f](719c58f))
* introduce event-driven communication to the control plane ([#265](#265)) ([f1ed1f5](f1ed1f5))
* **rover-ctl:** add EventSpec handler, env placeholder substitution, and HTTP client improvements ([#264](#264)) ([f15e0f7](f15e0f7))
* **rover-ctl:** support for get-info-many cmd ([#246](#246)) ([1d9f194](1d9f194))
* **secret-manager:** add write strategy support and improve caching ([#263](#263)) ([3f311bf](3f311bf))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working gateway Anything related to gateway domain go Pull requests that update go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants