Fixing several issues related to k8s gateway api flow in Operator#1798
Merged
CrowleyRajapakse merged 1 commit intowso2:mainfrom Apr 28, 2026
Merged
Fixing several issues related to k8s gateway api flow in Operator#1798CrowleyRajapakse merged 1 commit intowso2:mainfrom
CrowleyRajapakse merged 1 commit intowso2:mainfrom
Conversation
Contributor
📝 WalkthroughSummaryThis pull request fixes several issues in the Kubernetes Gateway API flow within the Gateway Operator: Key ChangesGatewayClass Status Management
API Context Defaults
HTTPRoute Method Handling
Service Exposure Configuration
RBAC and Permissions
Helm and Documentation Updates
Files Modified
WalkthroughThe pull request introduces a new Sequence DiagramsequenceDiagram
participant Manager as K8s Manager
participant Controller as GatewayClass<br/>Reconciler
participant APIClient as K8s API Client
participant Config as OperatorConfig
Manager->>Controller: Reconcile(ctx, req)
Controller->>APIClient: Get GatewayClass
APIClient-->>Controller: GatewayClass or NotFound
alt GatewayClass not found
Controller-->>Manager: Return (no requeue)
else GatewayClass exists
Controller->>Controller: Check spec.controllerName<br/>vs PlatformGatewayControllerName
alt Controller name mismatch
Controller-->>Manager: Return (no requeue)
else Controller name matches
Controller->>Config: Check if name in<br/>ManagedGatewayClass
Config-->>Controller: Allowed or Not Allowed
Controller->>Controller: Compute desired<br/>Accepted condition<br/>(True/False)
Controller->>Controller: Compare with existing<br/>status.conditions[Accepted]
alt Condition unchanged
Controller-->>Manager: Return (no requeue)
else Condition changed
Controller->>APIClient: Get latest GatewayClass
APIClient-->>Controller: Current GatewayClass
Controller->>Controller: Set new Accepted<br/>condition
Controller->>APIClient: Merge patch status
APIClient-->>Controller: Patch success
Controller->>Controller: Log info message
Controller-->>Manager: Return (no requeue)
end
end
end
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kubernetes/helm/resources/gateway-api-operator-demo/README.md (1)
188-189:⚠️ Potential issue | 🟡 MinorAdd
04-02-httproute.yamlto the Files section.The apply steps include
04-02-httproute.yaml, but the file inventory does not list it. Keeping both sections aligned avoids confusion during demo setup.📄 Suggested doc fix
- `04-httproute.yaml` — `PathPrefix /hello`, GET → backend Service; annotations set `api-version`, **`context`** (omit or leave blank to use API context **`/`**), `display-name`, and optional `project-id` for the generated API payload +- `04-02-httproute.yaml` — `PathPrefix /hello`, method omitted (expands to all verbs), context annotation omitted (defaults to **`/`**), display-name set for generated API payload🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kubernetes/helm/resources/gateway-api-operator-demo/README.md` around lines 188 - 189, The README's Files section is missing `04-02-httproute.yaml` even though the apply steps reference it; update the Files list in gateway-api-operator-demo/README.md to include `04-02-httproute.yaml` with a brief description (similar style to the existing `04-httproute.yaml` entry: path/verb → backend Service, annotations including `api-version`, `context` (note omit/blank uses `/`), `display-name`, and optional `project-id`) so the file inventory matches the apply steps.
🧹 Nitpick comments (1)
kubernetes/gateway-operator/internal/controller/httproute_mapper_test.go (1)
111-141: Add an explicit missing-annotation case for the new context default.This subtest covers whitespace input, but the mapper contract now includes both blank and absent annotations defaulting to
/. A small case withAnnotations: nilor the key omitted would lock in that second branch too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kubernetes/gateway-operator/internal/controller/httproute_mapper_test.go` around lines 111 - 141, Add a subtest to httproute_mapper_test.go that covers the case where the HTTPRoute has no context annotation (Annotations is nil or the key omitted) and assert it defaults to "/", similar to the existing whitespace-only test: create a route without AnnHTTPRouteContext set, call BuildAPIConfigFromHTTPRoute(ctx, cl, route, "cluster.local", nil), require.NoError, and require.Equal(t, "/", spec.Context) so the mapper contract for absent annotations is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@kubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller.go`:
- Around line 56-57: The kubebuilder RBAC marker in
k8s_gatewayclass_controller.go currently grants cluster-scoped update/patch on
gatewayclasses; change the first marker line
"//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gatewayclasses,verbs=get;list;watch;update;patch"
to remove update;patch so it reads verbs=get;list;watch, leaving the status
marker ("gatewayclasses/status") with get;update;patch intact; update the marker
in the file (around the gatewayclass reconciler declarations) and then
regenerate RBAC manifests so config/rbac/role.yaml reflects the reduced
permissions.
In `@kubernetes/helm/operator-helm-chart/README.md`:
- Around line 63-65: Update the README entry for gateway.helm.chartPath to
clearly state that when gateway.helm.chartPath is non-empty it disables all
remote chart resolution: the Helm client will ignore gateway.helm.chartName,
gateway.helm.chartVersion and any registry authentication/config, so no OCI
pulls, repo lookups, or registry auth are used; replace the terse "no OCI pull"
note with this explicit behavior description to avoid misleading repo-backed
chart users.
In `@kubernetes/helm/operator-helm-chart/templates/_helpers.tpl`:
- Around line 233-242: Narrow the RBAC: remove the broad "update" and "patch"
verbs from the parent resource rule that lists resource "gatewayclasses"
(currently using verbs get,list,watch,update,patch) so that the parent rule is
read-only (get,list,watch); then add a separate RBAC rule for the subresource
"gatewayclasses/status" granting only the minimal verb(s) required for
reconciliation (e.g., "patch" and/or "update" on "gatewayclasses/status") so
status updates remain permitted but parent resource mutating verbs are
eliminated.
In `@kubernetes/helm/resources/gateway-api-operator-demo/04-02-httproute.yaml`:
- Line 2: Update the inline comment that documents the default REST handle so it
matches this route's name: change the reference from
"gateway-api-demo-hello-api" to "gateway-api-demo-hello-api-2" (and note the
override key gateway.api-platform.wso2.com/api-handle remains the same) so the
default-handle comment accurately reflects hello-api-2.
---
Outside diff comments:
In `@kubernetes/helm/resources/gateway-api-operator-demo/README.md`:
- Around line 188-189: The README's Files section is missing
`04-02-httproute.yaml` even though the apply steps reference it; update the
Files list in gateway-api-operator-demo/README.md to include
`04-02-httproute.yaml` with a brief description (similar style to the existing
`04-httproute.yaml` entry: path/verb → backend Service, annotations including
`api-version`, `context` (note omit/blank uses `/`), `display-name`, and
optional `project-id`) so the file inventory matches the apply steps.
---
Nitpick comments:
In `@kubernetes/gateway-operator/internal/controller/httproute_mapper_test.go`:
- Around line 111-141: Add a subtest to httproute_mapper_test.go that covers the
case where the HTTPRoute has no context annotation (Annotations is nil or the
key omitted) and assert it defaults to "/", similar to the existing
whitespace-only test: create a route without AnnHTTPRouteContext set, call
BuildAPIConfigFromHTTPRoute(ctx, cl, route, "cluster.local", nil),
require.NoError, and require.Equal(t, "/", spec.Context) so the mapper contract
for absent annotations is validated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: baebf824-a02f-43a2-a915-b0b97d458e66
📒 Files selected for processing (23)
kubernetes/gateway-operator/cmd/main.gokubernetes/gateway-operator/config/gateway_values.yamlkubernetes/gateway-operator/config/rbac/role.yamlkubernetes/gateway-operator/docs/CONFIGURATION.mdkubernetes/gateway-operator/docs/GATEWAY_API_IMPLEMENTATION_NOTES.mdkubernetes/gateway-operator/docs/GITHUB_DISCUSSION_GATEWAY_API.mdkubernetes/gateway-operator/internal/controller/gateway_api_controller_name.gokubernetes/gateway-operator/internal/controller/httproute_controller.gokubernetes/gateway-operator/internal/controller/httproute_mapper.gokubernetes/gateway-operator/internal/controller/httproute_mapper_test.gokubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller.gokubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller_test.gokubernetes/helm/gateway-helm-chart/Chart.yamlkubernetes/helm/gateway-helm-chart/values.yamlkubernetes/helm/operator-helm-chart/README.mdkubernetes/helm/operator-helm-chart/templates/_helpers.tplkubernetes/helm/operator-helm-chart/templates/configmap.yamlkubernetes/helm/operator-helm-chart/values.yamlkubernetes/helm/resources/gateway-api-operator-demo/01-gatewayclass.yamlkubernetes/helm/resources/gateway-api-operator-demo/02-gateway.yamlkubernetes/helm/resources/gateway-api-operator-demo/04-02-httproute.yamlkubernetes/helm/resources/gateway-api-operator-demo/04-httproute.yamlkubernetes/helm/resources/gateway-api-operator-demo/README.md
CrowleyRajapakse
force-pushed
the
apim-dpcp-sync
branch
from
bca843a to
0533fef
Compare
CrowleyRajapakse
force-pushed
the
apim-dpcp-sync
branch
from
0533fef to
582ad46
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
kubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller_test.go (1)
108-112: Consider adding ObservedGeneration assertion for consistency.The first test (
AcceptedWhenManaged) verifiesObservedGenerationmatches the GatewayClass generation. For consistency, consider adding a similar assertion here to verify the condition'sObservedGenerationequals2.Suggested addition
require.Equal(t, metav1.ConditionFalse, cond.Status) require.Equal(t, string(gatewayv1.GatewayClassReasonUnsupported), cond.Reason) + require.Equal(t, int64(2), cond.ObservedGeneration) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller_test.go` around lines 108 - 112, Add an assertion that the condition's ObservedGeneration equals 2 to mirror the check in AcceptedWhenManaged: after locating cond via meta.FindStatusCondition(updated.Status.Conditions, string(gatewayv1.GatewayClassConditionStatusAccepted)) add a require.Equal comparing int64(2) to cond.ObservedGeneration so the test verifies the condition observed the GatewayClass generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/gateway/kubernetes/gateway-operator.md`:
- Around line 459-460: Update the architecture summary lines that list watched
resources to include GatewayClass alongside the existing resources (APIGateway,
RestApi, Gateway, HTTPRoute, Service, APIPolicy, Secret, ConfigMap) so the watch
scope matches other sections; locate the resource-watch description strings (the
lines containing "Watches: APIGateway, RestApi; Gateway, HTTPRoute (+ Service,
APIPolicy, Secret, ConfigMap for Gateway API path)") and add "GatewayClass" into
the appropriate list entries in both occurrences (around the block referencing
Lines 459-460 and the similar block at 474-476) to ensure consistency with
documented GatewayClass handling/status.
- Line 456: The fenced code block used for the architecture diagram is missing a
language identifier (triggering markdownlint MD040); update the fenced block
that contains the Gateway Operator ASCII diagram by adding a language hint such
as text after the opening backticks (i.e., change ``` to ```text) so the diagram
block is fenced with a language identifier.
---
Nitpick comments:
In
`@kubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller_test.go`:
- Around line 108-112: Add an assertion that the condition's ObservedGeneration
equals 2 to mirror the check in AcceptedWhenManaged: after locating cond via
meta.FindStatusCondition(updated.Status.Conditions,
string(gatewayv1.GatewayClassConditionStatusAccepted)) add a require.Equal
comparing int64(2) to cond.ObservedGeneration so the test verifies the condition
observed the GatewayClass generation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e1b59ac-2b99-49f2-b02a-20304aa6f1f1
📒 Files selected for processing (26)
.github/workflows/operator-integration-test.ymldocs/gateway/kubernetes/gateway-operator.mdkubernetes/gateway-operator/cmd/main.gokubernetes/gateway-operator/config/gateway_values.yamlkubernetes/gateway-operator/config/rbac/role.yamlkubernetes/gateway-operator/docs/CONFIGURATION.mdkubernetes/gateway-operator/docs/GATEWAY_API_IMPLEMENTATION_NOTES.mdkubernetes/gateway-operator/docs/GITHUB_DISCUSSION_GATEWAY_API.mdkubernetes/gateway-operator/internal/controller/gateway_api_controller_name.gokubernetes/gateway-operator/internal/controller/httproute_controller.gokubernetes/gateway-operator/internal/controller/httproute_mapper.gokubernetes/gateway-operator/internal/controller/httproute_mapper_test.gokubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller.gokubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller_test.gokubernetes/helm/gateway-helm-chart/values.yamlkubernetes/helm/operator-helm-chart/README.mdkubernetes/helm/operator-helm-chart/templates/_helpers.tplkubernetes/helm/operator-helm-chart/templates/clusterrole-gatewayclass.yamlkubernetes/helm/operator-helm-chart/templates/clusterrolebinding-gatewayclass.yamlkubernetes/helm/operator-helm-chart/templates/configmap.yamlkubernetes/helm/operator-helm-chart/values.yamlkubernetes/helm/resources/gateway-api-operator-demo/01-gatewayclass.yamlkubernetes/helm/resources/gateway-api-operator-demo/02-gateway.yamlkubernetes/helm/resources/gateway-api-operator-demo/04-02-httproute.yamlkubernetes/helm/resources/gateway-api-operator-demo/04-httproute.yamlkubernetes/helm/resources/gateway-api-operator-demo/README.md
✅ Files skipped from review due to trivial changes (6)
- kubernetes/helm/resources/gateway-api-operator-demo/04-httproute.yaml
- kubernetes/gateway-operator/docs/CONFIGURATION.md
- kubernetes/gateway-operator/docs/GITHUB_DISCUSSION_GATEWAY_API.md
- kubernetes/helm/resources/gateway-api-operator-demo/01-gatewayclass.yaml
- kubernetes/gateway-operator/internal/controller/gateway_api_controller_name.go
- kubernetes/helm/resources/gateway-api-operator-demo/04-02-httproute.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
- kubernetes/helm/gateway-helm-chart/values.yaml
- kubernetes/helm/resources/gateway-api-operator-demo/02-gateway.yaml
- kubernetes/helm/operator-helm-chart/templates/_helpers.tpl
- kubernetes/helm/operator-helm-chart/README.md
- kubernetes/gateway-operator/internal/controller/k8s_gatewayclass_controller.go
- kubernetes/gateway-operator/internal/controller/httproute_mapper_test.go
- kubernetes/gateway-operator/docs/GATEWAY_API_IMPLEMENTATION_NOTES.md
|
|
||
| ## Architecture | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language identifier to the fenced block.
Line 456 uses a fenced code block without a language, which triggers markdownlint MD040. Use a language hint such as text for the architecture diagram.
Suggested doc fix
-```
+```text
┌─────────────────────────────────────────────────────────────────┐
│ Gateway Operator │
│ Watches: APIGateway, RestApi; Gateway, HTTPRoute (+ Service, │
│ APIPolicy, Secret, ConfigMap for Gateway API path) │
└─────────────────────────────────────────────────────────────────┘
...
-```
+```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 456-456: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/gateway/kubernetes/gateway-operator.md` at line 456, The fenced code
block used for the architecture diagram is missing a language identifier
(triggering markdownlint MD040); update the fenced block that contains the
Gateway Operator ASCII diagram by adding a language hint such as text after the
opening backticks (i.e., change ``` to ```text) so the diagram block is fenced
with a language identifier.
Comment on lines
+459
to
+460
| │ Watches: APIGateway, RestApi; Gateway, HTTPRoute (+ Service, │ | ||
| │ APIPolicy, Secret, ConfigMap for Gateway API path) │ |
Contributor
There was a problem hiding this comment.
Include GatewayClass in architecture watch scope for consistency.
Lines 459-460 and Lines 474-476 describe Gateway API flow but omit GatewayClass from the watched resources, while earlier sections document GatewayClass handling/status. Please add GatewayClass here to keep the architecture summary accurate and complete.
Also applies to: 474-476
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/gateway/kubernetes/gateway-operator.md` around lines 459 - 460, Update
the architecture summary lines that list watched resources to include
GatewayClass alongside the existing resources (APIGateway, RestApi, Gateway,
HTTPRoute, Service, APIPolicy, Secret, ConfigMap) so the watch scope matches
other sections; locate the resource-watch description strings (the lines
containing "Watches: APIGateway, RestApi; Gateway, HTTPRoute (+ Service,
APIPolicy, Secret, ConfigMap for Gateway API path)") and add "GatewayClass" into
the appropriate list entries in both occurrences (around the block referencing
Lines 459-460 and the similar block at 474-476) to ensure consistency with
documented GatewayClass handling/status.
renuka-fernando
approved these changes
Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Fixing several issues related to k8s gateway api flow in Operator
Fixes GatewayClass status.
Fixes Default api context to "/".
Fixes Method requirement in HTTPRoute.
Fixes Gateway chart default to LoadBalancer.