search

Kuadrant / policy-machinery

Machinery for implementing Gateway API policies
Apache License 2.0
9 stars 2 forks source link

Add validate function #21

Closed Boomatang closed 2 months ago

Boomatang commented 2 months ago

Closes: #12

validate function is based of Kahn's algorithm https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm

eguzki commented 2 months ago

General question: According to https://github.com/Kuadrant/policy-machinery/issues/12

With the current Gateway, Route and Attached Policy I don't see how we can create a graph with a loop.

I do quite agree with that statement. So, why is the validation important for the kuadrant context? Is this a requirement for something?

guicassolato commented 2 months ago

General question: According to #12

With the current Gateway, Route and Attached Policy I don't see how we can create a graph with a loop.

I do quite agree with that statement. So, why is the validation important for the kuadrant context? Is this a requirement for something?

Even if the relationships between Gateway API resources cannot generate a loop, custom targetRefs and custom link functions defined by the user still can.

In general, please do not review the changes here with the Kuadrant use-case in mind only. This library is supposed to be general-purpose for any implementation that wants a custom controller backed by a topology of Gateway API resources. That means any Gateway API resources supported by the library (the ones Kuadrant depends on or not), plus policies (which are almost all of them implementation-specific), and any Kubernetes resources.

eguzki commented 2 months ago

In general, please do not review the changes here with the Kuadrant use-case in mind only. This library is supposed to be general-purpose for any implementation that wants a custom controller backed by a topology of Gateway API resources. That means any Gateway API resources supported by the library (the ones Kuadrant depends on or not), plus policies (which are almost all of them implementation-specific), and any Kubernetes resources.

Fair, I should have written gateway api context instead of kuadrant context. I can see that at the level of the topology, but I cannot see that for the GatewayAPI topology. So I propose to add the allowLoops at the level of the topology and the GatewayAPITopology sets that attribute to false.

guicassolato commented 2 months ago

In general, please do not review the changes here with the Kuadrant use-case in mind only. This library is supposed to be general-purpose for any implementation that wants a custom controller backed by a topology of Gateway API resources. That means any Gateway API resources supported by the library (the ones Kuadrant depends on or not), plus policies (which are almost all of them implementation-specific), and any Kubernetes resources.

Fair, I should have written gateway api context instead of kuadrant context. I can see that at the level of the topology, but I cannot see that for the GatewayAPI topology. So I propose to add the allowLoops at the level of the topology and the GatewayAPITopology sets that attribute to false.

@eguzki, it is implemented at the level of the generic topology. A Gateway API topology is still a topology where loops can happen however. On a base Gateway API topology, users still add policies with custom targetRefs and other kinds of custom linking functions that can produce loops.