feat(kuma-cp): add support for inspect api for new kind Dataplane and section name for selecting single inbound

[Automaat]

Fix: #12361

Motivation

We have introduced new kind and we need inspect api support for this, as it turns out it works out of the box so I've just added test for it.

This needs #12573 to be merged first

[github-actions]

Reviewer Checklist

🔍 Each of these sections need to be checked by the reviewer of the PR 🔍:
If something doesn't apply please check the box and add a justification if the reason is non obvious.

• Is the PR title satisfactory? Is this part of a larger feature and should be grouped using > Changelog?
• PR description is clear and complete. It Links to relevant issue as well as docs and UI issues
• This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as an image registry)
• IPv6 is taken into account (.e.g: no string concatenation of host port)
• Tests (Unit test, E2E tests, manual test on universal and k8s)
  • Don't forget ci/ labels to run additional/fewer tests
• Does this contain a change that needs to be notified to users? In this case, UPGRADE.md should be updated.
• Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label)

[lobkovilya]

why is the name required?

[Automaat]

I've made it required, just so it is not pointer to. string but I can change this if you think it should not be required

[lobkovilya]

Technically it doesn't matter as an empty value name: "" means name is not set. Could you please check with @johncowen or @schogges to understand what's the preferred way for such kind of situation?

[schogges]

I think it's better to omit name if it's not set. Theoretically it doesn't really matter because we'd catch that in our data-layer, but if name is an optional property, I'd treat it as such 🤔 from a TypeScript perspective I think it's better because we are forced to catch that case. An empty string is falsy, but we are not forced to handle that case. If we leave it as an empty string, we'd have to know that it could be an empty string and handle that.

For consistency reasons, are there similar cases and how do we handle that in these cases?

[johncowen]

TLDR; I'd go with name: "" if not set.

---

I'd say a non-union type is always best. So one single type, i.e. empty string "" in the case of strings, 0 (or sometimes even -1) in the case of numbers, arrays/slices should be empty [], objects are slightly trickier but generally {}. No three way booleans, always true or false, never omitted/undefined

No omitted's, undefineds or nulls

This makes the properties of an API response stable and clients are then less prone to error when they expect a property to be there and it suddenly isn't.

Theoretically it doesn't really matter because we'd catch that in our data-layer

We can fix these up in our data layer if necessary (and this is what we've been doing with older APIs), but moving forwards it would be great if the HTTP APIs follow this thinking also.

Theoretically the GUI is not the only HTTP API client, and not everyone will have a thin layer in between to smooth out any rough edges.

[johncowen]

Can't we just expect that?

I guess someone might be doing a bash thing with curl, so IMO I'd say we can't fully expect that.

---

I think the other thing, even if people are using the specs, they still have to add some sort of guard to check that name is there before doing something with it. This could be multiplied across a codebase (unless you have a centralised place like we do), and then you could multiply that by the amount of people using the HTTP API. That's a lot of unnecessary code

Alternatively, we could put whatever logic is needed to set name: "" in the backend (i.e. at source) instead then folks using the HTTP API don't have to check for existence before using name

[schogges]

Can't we just expect that?

I guess someone might be doing a bash thing with curl, so IMO I'd say we can't fully expect that.

Regardless of how the api might be consumed, there are plenty ways to check what is in the specs. Even when ignoring the specs and inspecting an API response and the name is not always returned one could infer that it's an optional property.

I think the other thing, even if people are using the specs, they still have to add some sort of guard to check that name is there before doing something with it. This could be multiplied across a codebase (unless you have a centralised place like we do), and then you could multiply that by the amount of people using the HTTP API. That's a lot of unnecessary code

I doubt that this is our problem 😅 also when I build something on top of the api and want to exclude the name when it's not set, it doesn't matter if it's returned or an empty string. In any case I have to check for that.

Alternatively, we could put whatever logic is needed to set name: "" in the backend (i.e. at source) instead then folks using the HTTP API don't have to check for existence before using name

But they would have to check why it's an empty string and what it means. They could even think it's a bug. This is just not obvious from my point of view and I think this could cause some confusion.

[johncowen]

Well it looks like we have pretty strong opposite opinions, so kinda feel like we aren't helping here 😅

@Automaat @lobkovilya any thoughts?

[johncowen]

❓ Are Inbound names unique?

[Automaat]

I'm leaning towards John opinion, to avoid nullable values in api. I think they should be unique, on Kubernetes they will be unique, on universal I think we should add validation for this. What do you think about it @lobkovilya?