feat(inspect-api): support for inbound rules

[lobkovilya]

Motivation

#12560 added a new field InboundRules to FromRules structure:

type FromRules struct {
	// Rules is a map of InboundListener to a list of rules built by using 'spec.from' field.
	// Deprecated: use InboundRules instead
	Rules map[InboundListener]Rules
	// InboundRules is a map of InboundListener to a list of inbound rules built by using 'spec.rules' field.
	InboundRules map[InboundListener][]*inbound.Rule
}

this structure needs to be visible in the Inspect API /_rules endpoint.

Implementation information

It's a draft PR, at this moment only OpenAPI is changed.

Supporting documentation

Fix #12381

[johncowen]

Question: Just wondering why we have a specific interpretFromEntriesAsRules property for the client to check, when (according to whats said in this description) they could also check for existence of fromRules or inboundRules?

(might have totally missed reasons for this in other issues etc so lmk!)

[lobkovilya]

For some policies like MeshTimeout, MeshAccessLog and others we implicitly treat legacy spec.from as spec.rules because the conversion is straightforward. That's why if this flag interpretFromEntriesAsRules is set for the policy you don't need to display both fromRules and inboundRules, the latter is sufficient.

For MeshTrafficPermission and MeshFaultInjection the conversion of spec.from to spec.rules is not straightforward. Legacy spec.from and new spec.rules policies will co-exist and that's why GUI needs to show both fromRules and inboundRules.

[lahabana]

This is not right. fromAliasForRules maybe?

Have you tried to use a LLM to find a name? It's usually helpful for these things.

[lahabana]

Can we just keep it? And just mention that inboundRules are to be preferred.

Also can we rename toResourceRules to outboundRules?

So we'd have:

proxyRule: {},
outboundRules: {},
inboundRules: {}

?

Also getting side tracked here. But I feel like maybe our APIs are just not working out here and we should rethink them.

Should we:

GET /meshes/<mesh>/dataplanes/<dataplane>/_layout

labels:
   key: value
name: foo
inbounds:
  - port: "..."
    name: "..."
    labels: "..."
    envoy_entity_names: [...]
outbounds:
  - name: "..." # This would be the envoy name potentially
     envoy_entity_names: [...]
     entity: "<meta>" # This would be the meta the find the MeshService/MeshMZService...
  
proxyRules:
   - type: "<policyType>"
     warnings: []
     conf: "interface{}"
     origin: # respects the order in which they are applied
        - meta1
        - meta2

Once we have this we can have:

GET /meshes/<mesh>/dataplanes/<dataplane>/_rules/_outbound/<identifier>

rules:
- type: <the conf type>

and something similar for inbounds.

I think this would:

a) make it easier for frontend to extract meaningful data for the DP view
b) make APIs way less verbose for users.

Obviously this would ONLY work with MeshService.

[lahabana]

Btw this gets rid of your weird "interpretFlag"

[johncowen]

Also getting side tracked here. But I feel like maybe our APIs are just not working out here and we should rethink them.

I agree with this sentiment. It feels like we need something that begins with the inbounds/outbounds (the "connections") and once I have those I can discover the rules for those (the GUI begins with the inbounds/outbounds also). Whereas the _rules endpoint feels more like the reverse, I begin with the rules and discover the inbounds/outbounds for those.

[lobkovilya]

Once we have this we can have:

GET /meshes/<mesh>/dataplanes/<dataplane>/_rules/_outbound/<identifier>

If we use ResourceIdentifier as <identifier> then we probably don't need /_outbound part. Kinds MeshService, MeshMultiZoneService and MeshExternalService are always outbounds. Kind Dataplane is always inbound.

We'd have to come up with <identifier> format for public API. Internally we're using meshservice:mesh/mesh-1:namespace/ns-k8s:name/backend:section/http-port and it's not ideal for the URL.

[lahabana]

Don't we have a universal resource identifier no?

[lahabana]

Where will the actual sub rules be?

[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)