Skip to content

Commit

Permalink
fix(parser) support same-name services across NSes
Browse files Browse the repository at this point in the history 
Correctly build upstreams for multiple backends where multiple services
in the backends share the same name in different namespaces. Previously
services were indexed by name alone in the rule builder, resulting in
one clobbering the other(s).
  • Loading branch information
@rainest
rainest committed Jul 20, 2023
1 parent e5969be commit a3c2cfd
Show file tree
Hide file tree
Showing 3 changed files with 205 additions and 2 deletions.
2 changes: 1 addition & 1 deletion internal/dataplane/parser/ingressrules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer,
for _, k8sService := range k8sServices {
// at this point we know the Kubernetes service itself is valid and can be
// used for traffic, so cache it amongst the kong Services k8s services.
service.K8sServices[k8sService.Name] = k8sService
service.K8sServices[fmt.Sprintf("%s/%s", k8sService.Namespace, k8sService.Name)] = k8sService // TODO this will overwrite when services have the same name in different namespaces

// extract client certificates intended for use by the service
secretName := annotations.ExtractClientCertificate(k8sService.Annotations)
Expand Down
8 changes: 7 additions & 1 deletion internal/dataplane/parser/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,13 @@ func (p *Parser) getUpstreams(serviceMap map[string]kongstate.Service) []kongsta
var targets []kongstate.Target
for _, backend := range service.Backends {
// gather the Kubernetes service for the backend
k8sService, ok := service.K8sServices[backend.Name]
backendNamespace := backend.Namespace
if backendNamespace == "" {
// if the backend namespace isn't specified, it's in the same namespace as the referee route (which is,
// somewhat confusingly, the _service_ namespace in serviceMap services.
backendNamespace = service.Namespace
}
k8sService, ok := service.K8sServices[fmt.Sprintf("%s/%s", backendNamespace, backend.Name)]
if !ok {
p.registerTranslationFailure(
fmt.Sprintf("can't add target for backend %s: no kubernetes service found", backend.Name),
Expand Down
197 changes: 197 additions & 0 deletions test/integration/httproute_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import (

"github.com/google/uuid"
"github.com/kong/go-kong/kong"
"github.com/kong/kubernetes-testing-framework/pkg/clusters"
ktfkong "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong"
"github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators"
"github.com/samber/lo"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -513,6 +515,201 @@ func TestHTTPRouteMultipleServices(t *testing.T) {
helpers.EventuallyGETPath(t, proxyURL, "test-http-route-multiple-services-broken", http.StatusNotFound, "", emptyHeaderSet, ingressWait, waitTick)
}

func TestHTTPRouteMultipleServicesBroken(t *testing.T) {
ctx := context.Background()

ns, cleaner := helpers.Setup(ctx, t, env)

otherNs, err := clusters.GenerateNamespace(ctx, env.Cluster(), t.Name())
require.NoError(t, err)
cleaner.AddNamespace(otherNs)

t.Log("getting a gateway client")
gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config())
require.NoError(t, err)

t.Log("deploying a new gatewayClass")
gatewayClassName := uuid.NewString()
gwc, err := DeployGatewayClass(ctx, gatewayClient, gatewayClassName)
require.NoError(t, err)
cleaner.Add(gwc)

t.Log("deploying a new gateway")
gatewayName := uuid.NewString()
gateway, err := DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayv1beta1.Gateway) {
gw.Name = gatewayName
})
require.NoError(t, err)
cleaner.Add(gateway)

t.Log("creating a go-echo pod")
container1 := generators.NewContainer("echo-1", test.EchoImage, 1027)
testUUID1 := uuid.NewString()
container1.Env = []corev1.EnvVar{
{
Name: "POD_NAME",
Value: testUUID1,
},
}
deployment1 := generators.NewDeploymentForContainer(container1)
deployment1, err = env.Cluster().Client().AppsV1().Deployments(ns.Name).Create(ctx, deployment1, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(deployment1)

t.Log("creating an additional go-echo pod")
container2 := generators.NewContainer("echo-2", test.EchoImage, 1027)
testUUID2 := uuid.NewString()
container2.Env = []corev1.EnvVar{
{
Name: "POD_NAME",
Value: testUUID2,
},
}
deployment2 := generators.NewDeploymentForContainer(container2)
deployment2, err = env.Cluster().Client().AppsV1().Deployments(otherNs.Name).Create(ctx, deployment2, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(deployment2)

t.Logf("exposing deployment %s/%s via service", deployment1.Namespace, deployment1.Name)
service1 := generators.NewServiceForDeployment(deployment1, corev1.ServiceTypeLoadBalancer)
service1, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service1, metav1.CreateOptions{})
assert.NoError(t, err)
cleaner.Add(service1)

t.Logf("exposing deployment %s/%s via service", deployment2.Namespace, deployment2.Name)
service2 := generators.NewServiceForDeployment(deployment2, corev1.ServiceTypeLoadBalancer)
// we want identical names to test https://github.com/Kong/kubernetes-ingress-controller/issues/3860
service2.Name = service1.Name
service2, err = env.Cluster().Client().CoreV1().Services(otherNs.Name).Create(ctx, service2, metav1.CreateOptions{})
assert.NoError(t, err)
cleaner.Add(service2)

t.Log("adding an HTTPRoute with multi-backend rules")
var weight1 int32 = 75
var weight2 int32 = 25
httpPort := gatewayv1beta1.PortNumber(1027)
pathMatchPrefix := gatewayv1beta1.PathMatchPathPrefix
httpRoute := &gatewayv1beta1.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: uuid.NewString(),
Annotations: map[string]string{
annotations.AnnotationPrefix + annotations.StripPathKey: "true",
},
},
Spec: gatewayv1beta1.HTTPRouteSpec{
CommonRouteSpec: gatewayv1beta1.CommonRouteSpec{
ParentRefs: []gatewayv1beta1.ParentReference{{
Name: gatewayv1beta1.ObjectName(gateway.Name),
}},
},
Rules: []gatewayv1beta1.HTTPRouteRule{
{
Matches: []gatewayv1beta1.HTTPRouteMatch{
{
Path: &gatewayv1beta1.HTTPPathMatch{
Type: &pathMatchPrefix,
// TODO make this a better name
Value: kong.String("/test-1"),
},
},
},
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
{
BackendRef: gatewayv1beta1.BackendRef{
BackendObjectReference: gatewayv1beta1.BackendObjectReference{
Name: gatewayv1beta1.ObjectName(service1.Name),
Port: &httpPort,
Kind: util.StringToGatewayAPIKindPtr("Service"),
},
Weight: &weight1,
},
},
{
BackendRef: gatewayv1beta1.BackendRef{
BackendObjectReference: gatewayv1beta1.BackendObjectReference{
Name: gatewayv1beta1.ObjectName(service2.Name),
Port: &httpPort,
Namespace: lo.ToPtr(gatewayv1beta1.Namespace(otherNs.Name)),
Kind: util.StringToGatewayAPIKindPtr("Service"),
},
Weight: &weight2,
},
},
},
},
},
},
}

t.Logf("creating a ReferenceGrant that permits access from %s to services in %s", ns.Name, otherNs.Name)
grant := &gatewayv1beta1.ReferenceGrant{
ObjectMeta: metav1.ObjectMeta{
Name: uuid.NewString(),
Annotations: map[string]string{},
Namespace: otherNs.Name,
},
Spec: gatewayv1beta1.ReferenceGrantSpec{
From: []gatewayv1beta1.ReferenceGrantFrom{
{
Group: gatewayv1beta1.Group("gateway.networking.k8s.io"),
Kind: gatewayv1beta1.Kind("HTTPRoute"),
Namespace: gatewayv1beta1.Namespace(ns.Name),
},
},
To: []gatewayv1beta1.ReferenceGrantTo{
{
Group: gatewayv1beta1.Group(""),
Kind: gatewayv1beta1.Kind("Service"),
},
},
},
}

grant, err = gatewayClient.GatewayV1beta1().ReferenceGrants(otherNs.Name).Create(ctx, grant, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(grant)

httpRoute, err = gatewayClient.GatewayV1beta1().HTTPRoutes(ns.Name).Create(ctx, httpRoute, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(httpRoute)

t.Log("verifying that both backends are ready to receive traffic")
// TODO improve path names
helpers.EventuallyGETPath(t, proxyURL, "test-1", http.StatusOK, testUUID1, emptyHeaderSet, ingressWait, waitTick)
helpers.EventuallyGETPath(t, proxyURL, "test-1", http.StatusOK, testUUID2, emptyHeaderSet, ingressWait, waitTick)

t.Log("verifying that both backends receive requests according to weighted distribution")
firstRespName := "1"
secondRespName := "2"
toleranceDelta := 0.2
expectedRespRatio := map[string]int{
firstRespName: int(weight1),
secondRespName: int(weight2),
}
weightedLoadBalancingTestConfig := helpers.CountHTTPResponsesConfig{
Method: http.MethodGet,
// TODO names names names
Path: "test-1",
Headers: emptyHeaderSet,
Duration: 5 * time.Second,
RequestTick: 50 * time.Millisecond,
}
respCounter := helpers.CountHTTPGetResponses(t,
proxyURL,
weightedLoadBalancingTestConfig,
helpers.MatchRespByStatusAndContent(firstRespName, http.StatusOK, testUUID1),
helpers.MatchRespByStatusAndContent(secondRespName, http.StatusOK, testUUID2),
)
assert.InDeltaMapValues(t,
helpers.DistributionOfMapValues(respCounter),
helpers.DistributionOfMapValues(expectedRespRatio),
toleranceDelta,
"Response distribution does not match expected distribution within %f%% delta,"+
" request-count=%v, expected-ratio=%v",
toleranceDelta*100, respCounter, expectedRespRatio,
)
}

func TestHTTPRouteFilterHosts(t *testing.T) {
ctx := context.Background()

Expand Down

0 comments on commit a3c2cfd

Please sign in to comment.