AlinsRan opened a new pull request, #2860:
URL: https://github.com/apache/apisix-ingress-controller/pull/2860
### Type of change:
- [x] Bugfix
### What this PR does / why we need it:
`spec.http[].backends[].servicePort: ""` is accepted by the API server,
reported as `Accepted=True` with an empty message, and answers 503. Three
separate problems stack up to produce that.
**1. Nothing rejects the value.** `servicePort` is an int-or-string with no
constraint beyond `required`. The string form is compared against Service port
names, so `""` either matches nothing, or matches a single-port Service that
omits its port name, which Kubernetes allows. The same misconfiguration
therefore works on one Service and 503s on another, and the working case is
worse: the user never learns the field is wrong.
**2. The failure never reaches the status.** `validateHTTPBackend` logs and
returns nil when the port does not match:
```go
if !slices.ContainsFunc(service.Spec.Ports, func(port corev1.ServicePort)
bool { ... }) {
r.Log.Error(errors.New("service port not found"), "failed to match
service port", ...)
return nil // no error, so processApisixRoute
succeeds
}
tctx.Services[serviceNN] = &service // skipped
```
`processApisixRoute` returns nil, so `updateStatus` writes `Accepted=True`.
**3. The message blames the wrong object.** Because the Service was never
added to `tctx.Services`, the translator reports `service not found` for a
Service that exists with healthy endpoints. `getPortFromService` has the
accurate error, but nothing reaches it. The translation error is then swallowed
by `buildUpstream`, which `continue`s, leaving the default upstream with no
nodes, which is the 503.
This PR rejects the value in three places, each covering a case the others
cannot:
- a CEL rule on the CRD, which is the only point that can name the offending
field at apply time
- `validateHTTPBackend`, before the reference is resolved, so objects that
predate the CRD rule are reported rather than published, and the accidental
match against an unnamed Service port cannot happen
- `getPortFromService`, so the translator cannot make that match through
another path
A Service that resolves but has no such port now reports `InvalidSpec` and
says the port was not found rather than the Service. A missing Service is left
alone: that path stays lenient so applying a route alongside its Service still
works.
The CEL test harness needed a fix to exercise any int-or-string rule: it
decoded JSON numbers with `encoding/json`, producing `float64`, and CEL rejects
that where the schema declares `x-kubernetes-int-or-string`. It now decodes
with the apimachinery helper that yields `int64`, as the API server does.
### Pre-submission checklist:
- [x] Did you explain what problem does this PR solve? Or what new features
have been added?
- [x] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [ ] Is this PR backward compatible?
Behavior changes worth calling out:
- An ApisixRoute whose Service exists but has no matching port is now
rejected instead of accepted-and-503. Since `processApisixRoute` is shared with
the ADC admission webhook, that also means such a route is refused at apply
time. A route whose Service does not exist yet is unaffected.
- An empty `servicePort` that happened to work against an unnamed
single-port Service now fails. That match was accidental and would have broken
as soon as a second port was added to the Service.
Tests: CEL coverage in `api/v2/apisixroute_types_test.go` for the rejected
and accepted forms, and reconciler coverage in
`internal/controller/apisixroute_serviceport_test.go` for the empty port
against both a named and an unnamed Service port, the unknown-port message, and
the healthy path. The two reconciler tests fail on `master`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
