Skip to content
This repository was archived by the owner on Apr 13, 2020. It is now read-only.

Commit 71ebd85

Browse files
evanlouiemtarng
andauthored
Fix Incorrect Backend SVC for Default IngressRoutes (#441)
- Fixed bug where the Traefik IngressRoute generated via `reconcile` for an `isDefault` ringed service was pointing to a non-existent backend k8s service when `k8sBackend` is not provided. - Cause: Incorrect computed value for the IngressRoute `spec.routes[].services[].name`; was falling back to `metadata.name` when `k8sBackend` was no provided -- which did does not contain the `ringName`. - Fix: Expand logic for computing the IngressRoute `metadata.name`. 1. Always compute the _ringed_ version of the service name. 2. Use the _ringed_ service name for `metadata.name` when not `isDefault` -- use non-ringed otherwise. 3. Use the _ringed_ service name as fallback for the backing k8s service if `k8sBackend` not provided -- otherwise compute the _ringed_ `k8sBackend`. Co-authored-by: Michael Tarng <[email protected]>
1 parent 7611617 commit 71ebd85

File tree

2 files changed

+42
-10
lines changed

2 files changed

+42
-10
lines changed

src/lib/traefik/ingress-route.test.ts

+31-5
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ describe("TraefikIngressRoute - Unit Tests", () => {
160160
},
161161

162162
{
163-
name: "configured correctly when isDefault === false",
163+
name:
164+
"!isDefault & k8sBackend => metadata.name === ringed(service-name) && spec.routes[].services[].name == ringed(k8sBackend)",
164165
actual: (): unknown =>
165166
create("my-service", "master", 80, "/version/and/path", {
166167
k8sBackend: "my-k8s-svc",
@@ -174,7 +175,7 @@ describe("TraefikIngressRoute - Unit Tests", () => {
174175
match: expect.stringMatching(/.*ring.*master/i),
175176
services: expect.arrayContaining([
176177
expect.objectContaining({
177-
name: expect.stringContaining("master"),
178+
name: "my-k8s-svc-master",
178179
}),
179180
]),
180181
}),
@@ -184,7 +185,8 @@ describe("TraefikIngressRoute - Unit Tests", () => {
184185
},
185186

186187
{
187-
name: "configured correctly when isDefault === true",
188+
name:
189+
"isDefault & k8sBackend => metadata.name == serviceName && spec.routes[].services[].name == ringed(k8sBackend)",
188190
actual: (): unknown =>
189191
create("my-service", "master", 80, "/version/and/path", {
190192
k8sBackend: "my-k8s-svc",
@@ -198,7 +200,31 @@ describe("TraefikIngressRoute - Unit Tests", () => {
198200
match: expect.not.stringMatching(/.*ring.*master/i),
199201
services: expect.arrayContaining([
200202
expect.objectContaining({
201-
name: expect.stringContaining("master"),
203+
name: "my-k8s-svc-master",
204+
}),
205+
]),
206+
}),
207+
]),
208+
},
209+
}),
210+
},
211+
212+
{
213+
name:
214+
"isDefault & !k8sBackend => metadata.name == serviceName && spec.routes[].services[].name == ringed(serviceName)",
215+
actual: (): unknown =>
216+
create("my-service", "master", 80, "/version/and/path", {
217+
isDefault: true,
218+
}),
219+
expected: expect.objectContaining<PartialIngressRoute>({
220+
metadata: { name: "my-service" },
221+
spec: {
222+
routes: expect.arrayContaining([
223+
expect.objectContaining({
224+
match: expect.not.stringMatching(/.*ring.*master/i),
225+
services: expect.arrayContaining([
226+
expect.objectContaining({
227+
name: "my-service-master",
202228
}),
203229
]),
204230
}),
@@ -227,7 +253,7 @@ describe("TraefikIngressRoute - Throwable", () => {
227253
{
228254
name: "throws when meta.name is invalid: dash (-) prefix",
229255
actual: (): unknown =>
230-
create("-invalid-serivce&name", "valid-ring", 80, "v1"),
256+
create("-invalid-service&name", "valid-ring", 80, "v1"),
231257
throws: true,
232258
},
233259

src/lib/traefik/ingress-route.ts

+11-5
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,13 @@ export const create = (
7171
): TraefikIngressRoute => {
7272
const { entryPoints, k8sBackend, middlewares = [], namespace, isDefault } =
7373
opts ?? {};
74-
const name =
75-
ringName && !isDefault ? `${serviceName}-${ringName}` : serviceName;
74+
75+
const ringedServiceName = ringName
76+
? `${serviceName}-${ringName}`
77+
: serviceName;
78+
79+
// IngressRoute name is _ringed_ depending on isDefault
80+
const ingressName = isDefault ? serviceName : ringedServiceName;
7681

7782
const routeMatchPathPrefix = `PathPrefix(\`${versionAndPath}\`)`;
7883
const routeMatchHeaders = isDefault
@@ -82,18 +87,19 @@ export const create = (
8287
.filter((rule): rule is NonNullable<typeof rule> => !!rule)
8388
.join(" && ");
8489

90+
// Compute the _ringed_ k8sBackend if provided - else fallback to the _ringed_ service name
8591
const backendService =
86-
k8sBackend && ringName ? `${k8sBackend}-${ringName}` : name;
92+
k8sBackend && ringName ? `${k8sBackend}-${ringName}` : ringedServiceName;
8793

8894
// validate fields
89-
dns.assertIsValid("metadata.name", name);
95+
dns.assertIsValid("metadata.name", ingressName);
9096
dns.assertIsValid("spec.routes[].services[].name", backendService);
9197

9298
return {
9399
apiVersion: "traefik.containo.us/v1alpha1",
94100
kind: "IngressRoute",
95101
metadata: {
96-
name,
102+
name: ingressName,
97103
...(namespace ? { namespace } : {}),
98104
},
99105
spec: {

0 commit comments

Comments
 (0)