feature(dataplanes): add filtered XDS config to drawers

[johncowen]

Adds XDS configuration drawers to inbounds and outbounds so show only the listeners/clusters relevant to that inbound/outbound.

Closes #3182
Closes #3181

You can run this against a local cluster by setting a cookie like the following:

note: I removed an annoying ESLint rule here https://eslint.org/docs/latest/rules/multiline-ternary#when-not-to-use-it . We don't have any conventions in regards to ternaries, do whatever works for you. Generally you've decided to use a ternary because the format you've personally chosen reads nicer - furthermore its pointless having a eslint rule to force wrapping of something that is designed for single lines. Not only that but the eslint rule we had makes these super hard to read.

---

Note: This will not pass lint tests and is purposefully an early WIP PRed for preview site purposes.

---

Notes taken during work:

Adding some pseudo code to help explain the filtering/selecting we are doing:

Inbounds

I use '127.0.0.1:0000' below in place of the actual dataplane.networking.address

• dynamic_listeners[].name === '127.0.0.1:0000'
• {dynamic_active_clusters, static_clusters}.cluster.load_assignment.endpoints[].lb_endpoints[].endpoint.address.socket_address.{address:port_value} === '127.0.0.1:0000' (I'm not totally sure we should be using this one)

Note for an inbound we do not know the type of the service (i.e. is it a _msvc_ a _mzsvc_ etc) so we can't filter by cluster name.

---

Improvements: we should look for inbound:ignore.ip.address:0000 use prefix/suffix because we need to be careful with ipv6.

• dynamic_listeners[].name === 'inbound:ignore.ip.address:0000'
• dynamic_active_clusters[].cluster.name === 'ignore.host:0000'

Outbounds

I use outbound below in place of the actual Envoy stats name (e.g. default_demo-app_kuma-demo_default_msvc_5000)

• dynamic_active_clusters[].cluster.name === <outbound>
• dynamic_endpoint_configs[].endpoint_config.cluster_name === <outbound>

Note we don't have any information about a specific outbound apart from the clusterName/envoy stats prefix. We can take information from the dataplane associated with the outbound, but not the outbound itself.

Improvements:

• dynamic_listeners[].name === 'outbound:<outbound>' to be future proof with xds configs, outbound listeners should use the clustername instead of an IP/port combo kuma#12093.

If we need to add more instance of filtering/selecting it would be helpful to express what needs to be done using the above examples

[netlify]

✅ Deploy Preview for kuma-gui ready!

Name	Link
🔨 Latest commit	3afdfd2
🔍 Latest deploy log	https://app.netlify.com/sites/kuma-gui/deploys/674daf6b76aec10008ea717a
😎 Deploy Preview	https://deploy-preview-3186--kuma-gui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[lahabana]

Looks great! There's only 1 bug with clusters

https://konghq.zoom.us/clips/share/A2F3MRZnZDRPSjZfT1E5bVhNRTRERWl1MnlnAQ

For the inbound issue I observe: kumahq/kuma#12123

[johncowen]

PR to fix up the issue with clusters is here:

#3239

[schogges]

note: I removed an annoying ESLint rule here https://eslint.org/docs/latest/rules/multiline-ternary#when-not-to-use-it . We don't have any conventions in regards to ternaries, do whatever works for you. Generally you've decided to use a ternary because the format you've personally chosen reads nicer - furthermore its pointless having a eslint rule to force wrapping of something that is designed for single lines. Not only that but the eslint rule we had makes these super hard to read.

I agree with this 👍 since you speak about readability, I'd like to bring up no-nested-ternary 🙂 I think this might be a good addition to our lint rules as it enforces short and one-lined ternary expressions, which are great. But when it comes to more complex flows, I'd rather use if-else or a mix of ternary and if-else. Nesting ternaries hurts readability IMO. (But have not seen any nested ones yet tbh 😄 )

[johncowen]

I agree with this 👍 since you speak about readability, I'd like to bring up no-nested-ternary 🙂

More than happy to take look at it.

The only thing is ternaries are about more than readability, they allow you to have type safety at runtime in javascript, which while I don't think is as important as the type safety, I believe is also a micro-performance improvement in javascript engines (but lets not get into micro-optimizations, thats side benefit)

Its not so much using the ternaries, its more the avoidance of using let.

let a = '1'
if(true) {
  a = '2'
}
// a is a string

vs

const a = true ? '2' : '1'
// a is '1' | '2'

In the second form:

• The JS engine knows a can only ever be '1' or '2' , not any-string (in TS) or anything (in JS), and therefore optimize.
• The JS engine knows that a will never be set again, and therefore optimize.

So I use these inline forms a lot (and did previous to Typescript) via ternaries and inline functions for try/catch and switch among other constructs for these reasons, not necessarily for readability. If there's a nested ternary somewhere there's a chance its for the above reasons.

I have an example of a change a made recently that outlines this, lemme try and find it.

edit:

Not a ternary but its the same idea, if you take a look at this code from a recent PR (😆 oh its actually this PR):

kuma-gui/packages/kuma-gui/src/test-support/mocks/src/meshes/_/dataplanes/_/_overview.ts

Lines 21 to 26 in 765d9f5

	let type: 'gateway_builtin' | 'gateway_delegated' | 'proxy' = 'proxy'
	if (name.includes('-gateway_builtin') || env('KUMA_DATAPLANE_TYPE', 'standard') === 'builtin') {
	type = 'gateway_builtin'
	} else if (name.includes('-gateway_delegated') || env('KUMA_DATAPLANE_TYPE', 'standard') === 'delegated') {
	type = 'gateway_delegated'
	}

In #3186 I'm changing it to this:

kuma-gui/packages/kuma-gui/src/test-support/mocks/src/meshes/_/dataplanes/_/_overview.ts

Lines 24 to 35 in c42fe8d

	const type = (() => {
	switch (true) {
	case defaultType === 'builtin':
	case name.includes('-gateway_builtin'):
	return 'BUILTIN'
	case defaultType === 'delegated':
	case name.includes('-gateway_delegated'):
	return 'DELEGATED'
	default:
	return 'STANDARD'
	}
	})()

Using the inline function gives me automatic type safety both in TS at build time and JS at runtime, and honestly in this case, in avoiding the multiple if conditionals the inline-function/switch is far easier to read to my eyes at least.

[schogges]

I agree with this 👍 since you speak about readability, I'd like to bring up no-nested-ternary 🙂

More than happy to take look at it.

The only thing is ternaries are about more than readability, they allow you to have type safety at runtime in javascript, which while I don't think is as important as the type safety, I believe is also a micro-performance improvement in javascript engines (but lets not get into micro-optimizations, thats side benefit)

Its not so much using the ternaries, its more the avoidance of using let.

let a = '1'
if(true) {
  a = '2'
}
// a is a string

vs

const a = true ? '2' : '1'
// a is '1' | '2'

In the second form:

• The JS engine knows a can only ever be '1' or '2' , not any-string (in TS) or anything (in JS), and therefore optimize.
• The JS engine knows that a will never be set again, and therefore optimize.

So I use these inline forms a lot (and did previous to Typescript) via ternaries and inline functions for try/catch and switch among other constructs for these reasons, not necessarily for readability. If there's a nested ternary somewhere there's a chance its for the above reasons.

I have an example of a change a made recently that outlines this, lemme try and find it.

edit:

Not a ternary but its the same idea, if you take a look at this code from a recent PR (😆 oh its actually this PR):

kuma-gui/packages/kuma-gui/src/test-support/mocks/src/meshes/_/dataplanes/_/_overview.ts

Lines 21 to 26 in 765d9f5

	let type: 'gateway_builtin' | 'gateway_delegated' | 'proxy' = 'proxy'
	if (name.includes('-gateway_builtin') || env('KUMA_DATAPLANE_TYPE', 'standard') === 'builtin') {
	type = 'gateway_builtin'
	} else if (name.includes('-gateway_delegated') || env('KUMA_DATAPLANE_TYPE', 'standard') === 'delegated') {
	type = 'gateway_delegated'
	}

In #3186 I'm changing it to this:

kuma-gui/packages/kuma-gui/src/test-support/mocks/src/meshes/_/dataplanes/_/_overview.ts

Lines 24 to 35 in c42fe8d

	const type = (() => {
	switch (true) {
	case defaultType === 'builtin':
	case name.includes('-gateway_builtin'):
	return 'BUILTIN'
	case defaultType === 'delegated':
	case name.includes('-gateway_delegated'):
	return 'DELEGATED'
	default:
	return 'STANDARD'
	}
	})()

Using the inline function gives me automatic type safety both in TS at build time and JS at runtime, and honestly in this case, in avoiding the multiple if conditionals the inline-function/switch is far easier to read to my eyes at least.

Yeah, I like the switch-case 👍 😄 another great way of avoiding nested (or chained) ternaries, having the optimization benefits and it's a lot easier to read and understand.

[johncowen]

Oh look I found a nested-ternary:

kuma-gui/packages/kuma-gui/src/app/data-planes/data/DataplaneOverview.ts

Line 92 in 61c99f3

dataplaneType: networking.type === 'gateway' ? dpTypes.builtin : typeof networking.gateway !== 'undefined' ? dpTypes.delegated : dpTypes.standard,

🤔 when I think about what the above file used to be like, I kinda prefer the direction it's going.

I think that, the fact that we have them already, and the type/js benefits I wouldn't be super keen on adding that rule, plus the more i think about it its very similar to saying "don't use multiple conditionals inside an if statement" (i.e. &&'s and ||'s)

If you feel super strong about it happy to reconsider, but at the moment it doesn't feel like a super important error causing problem that we might want to add a lint rule for.

[schogges]

LGTM! 👍

[schogges]

Oh look I found a nested-ternary:

kuma-gui/packages/kuma-gui/src/app/data-planes/data/DataplaneOverview.ts

Line 92 in 61c99f3

dataplaneType: networking.type === 'gateway' ? dpTypes.builtin : typeof networking.gateway !== 'undefined' ? dpTypes.delegated : dpTypes.standard,

🤔 when I think about what the above file used to be like, I kinda prefer the direction it's going.

I think that, the fact that we have them already, and the type/js benefits I wouldn't be super keen on adding that rule, plus the more i think about it its very similar to saying "don't use multiple conditionals inside an if statement" (i.e. &&'s and ||'s)

If you feel super strong about it happy to reconsider, but at the moment it doesn't feel like a super important error causing problem that we might want to add a lint rule for.

I think ternaries are getting unreadable a lot quicker when nesting than a complex boolean expression. But yeah, at some point it probably makes sense to outsource and group them into variables.
Anyways, will think about it and will probably bring it up again 🙂

[johncowen]

One last thing I wanted to do here thats vaguely related to above convo, I added fake.kuma.certificate for various reasons here. Then typically I had to stabilise a test because of that. I did it in separate commits in case.

I'll leave this hanging for a little while today, but if I hear no more I'll go ahead and merge seeing as I already have approval on everything else. But lmk if there's anything else