Copilot commented on code in PR #2703:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2703#discussion_r2895161107
##########
test/e2e/scaffold/scaffold.go:
##########
@@ -313,6 +313,43 @@ func (s *Scaffold) NewAPISIXClientWithTLSProxy(host
string) *httpexpect.Expect {
})
}
+// NewAPISIXClientForPort creates an HTTP client for a specific APISIX port.
+// Uses existing tunnels if available, otherwise creates a new one.
+func (s *Scaffold) NewAPISIXClientForPort(port int) (*httpexpect.Expect,
error) {
+ // Check if we can reuse existing tunnels
+ switch port {
+ case 80:
+ return s.NewAPISIXClient(), nil
+ case 443:
+ return s.NewAPISIXHttpsClient(""), nil
+ case 9100:
+ return s.NewAPISIXClientOnTCPPort(), nil
+ }
+
+ // Create new tunnel for custom port
+ serviceName := s.dataplaneService.Name
+ tunnel := k8s.NewTunnel(s.kubectlOptions, k8s.ResourceTypeService,
serviceName, 0, port)
+ if err := tunnel.ForwardPortE(s.t); err != nil {
+ return nil, fmt.Errorf("failed to create tunnel for port %d:
%w", port, err)
+ }
+ s.addFinalizers(tunnel.Close)
+
+ u := url.URL{
+ Scheme: "http",
+ Host: tunnel.Endpoint(),
+ }
+ return httpexpect.WithConfig(httpexpect.Config{
+ BaseURL: u.String(),
+ Client: &http.Client{
+ Transport: &http.Transport{TLSClientConfig:
&tls.Config{InsecureSkipVerify: true}},
+ Timeout: 3 * time.Second,
Review Comment:
NewAPISIXClientForPort creates an httpexpect client without the redirect
policy used by the other APISIX clients (they set CheckRedirect to
http.ErrUseLastResponse). This means tests using this helper may silently
follow redirects while other helpers don’t, leading to inconsistent assertions.
Consider reusing the same http.Client settings (Transport + CheckRedirect) as
NewAPISIXClient/NewAPISIXClientOnTCPPort for parity.
```suggestion
Timeout: 3 * time.Second,
CheckRedirect: func(req *http.Request, via
[]*http.Request) error {
return http.ErrUseLastResponse
},
```
##########
internal/adc/translator/httproute.go:
##########
@@ -524,6 +525,133 @@ func calculateHTTPRoutePriority(match
*gatewayv1.HTTPRouteMatch, ruleIndex int,
return priority
}
+// translateBackendsToUpstreams processes the BackendRefs of an HTTPRouteRule,
+// builds upstreams, assigns them to the service (single upstream or
traffic-split
+// plugin for multiple), and injects fault-injection on backend errors.
+func (t *Translator) translateBackendsToUpstreams(
+ tctx *provider.TranslateContext,
+ rule gatewayv1.HTTPRouteRule,
+ httpRoute *gatewayv1.HTTPRoute,
+ service *adctypes.Service,
+) (enableWebsocket *bool, backendErr error) {
+ upstreams := make([]*adctypes.Upstream, 0)
+ weightedUpstreams :=
make([]adctypes.TrafficSplitConfigRuleWeightedUpstream, 0)
+
+ for _, backend := range rule.BackendRefs {
+ if backend.Namespace == nil {
+ namespace := gatewayv1.Namespace(httpRoute.Namespace)
+ backend.Namespace = &namespace
+ }
+ upstream := adctypes.NewDefaultUpstream()
+ upNodes, protocol, err := t.translateBackendRef(tctx,
backend.BackendRef, DefaultEndpointFilter)
+ if err != nil {
+ backendErr = err
+ continue
+ }
+ if len(upNodes) == 0 {
+ continue
+ }
+ if protocol == internaltypes.AppProtocolWS || protocol ==
internaltypes.AppProtocolWSS {
+ enableWebsocket = ptr.To(true)
+ }
+
+ t.AttachBackendTrafficPolicyToUpstream(backend.BackendRef,
tctx.BackendTrafficPolicies, upstream)
+ upstream.Nodes = upNodes
+ upstream.Scheme = appProtocolToUpstreamScheme(protocol)
Review Comment:
In translateBackendsToUpstreams, the upstream scheme is being set
unconditionally from the Service/AppProtocol (`upstream.Scheme =
appProtocolToUpstreamScheme(protocol)`). This overwrites any scheme that may
have been set by BackendTrafficPolicy (AttachBackendTrafficPolicyToUpstream
sets upstream.Scheme), changing behavior compared to the previous logic which
only filled scheme when it was empty. Preserve policy-configured scheme by only
setting scheme when it’s not already set (or use cmp.Or/empty-check).
```suggestion
if upstream.Scheme == "" {
upstream.Scheme = appProtocolToUpstreamScheme(protocol)
}
```
--
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]
