metal-stack/firewall-controller

implementation is not matching the cwnp spec

mwennrich opened this issue · 5 comments

CRD describes spec.egress.port as numerical or named port ( https://github.com/metal-stack/firewall-controller/blob/master/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml#L65-L72 ) while in validator only int ports are supported (

func validatePorts(ports []networking.NetworkPolicyPort) *multierror.Error {
var errors *multierror.Error
for _, p := range ports {
if p.Port != nil && p.Port.Type != intstr.Int {
errors = multierror.Append(errors, fmt.Errorf("only int ports are supported, but %v given", p.Port))
}
)

@mwennrich If i'm correct, egress and ingress fields specify IPs(or subnets) outside of cluster, from which traffic is allowed. Does it even makes sense to allow to specify named port? Probably a better solution would be to replace NetworkPolicyPort fields with custom port field that would be always int.

egress is for communication with outside endpoints, ingress for communication with inside endpoints.

@majst01 Yep, i got that) What i meant is that, both for egress and ingress rules we specify addresses outside of cluster, so named ports don't even make sense for that case. So, for cleanliness, it's probably better to introduce our custom structure for ports. That would be backwards compatible with NetworkPolicyPort(except that it would only allow int type for ports).

Maybe i misunderstood this. Comment to ports field says:

List of ports which should be made accessible on the cluster for this rule

So, does it mean that ports field lists ports of services in cluster?

So at least description must be adopted