[tombentley]

This proposes allowing the operator to generate Kubernetes NetworkPolicies based on the KafkaProxyIngresses, KafkaServices and KafkaProtocolFilters which make up a KafkaProxy.

[k-wall]

That's already the subject of #134, so we need to watch for a conflict between the two proposals.

[tombentley]

Noted

[k-wall]

Thanks @tombentley - on first reading this looks reasonable. I will take a deeper look.

[MarkNSweep]

This KafkaProxyOperatorConfig looks like it wants to be its own thing rather than part of this proposal as it's a general mechanism for configuring the operator. Once it exists other features will want to hang their operator-level options off it too, so it feels like it deserves a proposal and a design discussion of its own rather than arriving as a side effect of NetworkPolicy generation.

Could it be split out, with a speculative reference kept here showing just the shape this proposal needs from it (the resourceGeneration toggle and the policyAttachment block) and a note that the exact schema is subject to that separate proposal?

[tombentley]

Yes, it is a general mechanism for configuring the proxy. A dedicated proposal would be a good idea. However, apart from this proposal, there's no immediate need to put anything in the spec. It seems odd to open a proposal for a CR with an empty spec.

[MarkNSweep]

On your TODO here ('this would be, without policies, lots of access, is that right/defensible ?'), I'd lean towards no for the filter case. The stated goal of the feature is auditability, and a default that silently allows a filter both ingress and egress to anywhere is the situation an audit is trying to catch. Defaulting a filter with no attached KafkaProxyNetworkPolicy to deny, or to requiring an explicit attachment to a well known policy via requiredAttachments would show the explicit decision to allow all and seems more in keeping with that goal than the automatic default-allow. The downside is that this stricter requirement could break existing filters if they need network access.

[tombentley]

I've (consciously) been trying to have my cake and eat it too: On the one hand we're being asked to make it deny-by-default, but on the other hand doing that without a new major version, or migration period, will break existing setups when they upgrade.

The upgrade is a real problem because (without taking special measures) you can't write the policy (a CR) without the CRD, which arrives at the same time as the new operator.

I think we could do something a little nicer: Use deny-by-default for new installs and allow-by-default for existing installs. This would have to be implemented in the operator itself (not the installation marchinery/manifests/OLM whatever).

[MarkNSweep]

The requiredAttachments / permittedAttachments / prohibitedAttachments trio is quite a lot of machinery. Could we start with required and the default now, and let the other two follow if a user asks for them? or because this isn't a new feature as such, but is attempting to retro-fit over existing deployments that we'll need all the options the three give us to allow users the ability to correctly configure everything?

[tombentley]

If we're going to make the distinction between new installs vs existing installs when deciding whether to deny-/allow-by-default, then this becomes the place where the behaviour would be spelled out.

So for an existing install, we'd use:

policyAttachment:
  policyTarget:
    - group: kroxylicious.io
      kind: KafkaProxyIngress
      permittedAttachments: # the KafkaProxyIngressNetworkPolicy need not exist
        - group: kroxylicious.io
           kind: KafkaProxyIngressNetworkPolicy
           whenAbsent: allow # when its missing we allow by default

And for a new install we'd use:

policyAttachment:
  policyTarget:
    - group: kroxylicious.io
      kind: KafkaProxyIngress
      requiredAttachments: # a KafkaProxyIngressNetworkPolicy must exist
        - group: kroxylicious.io
           kind: KafkaProxyIngressNetworkPolicy                                                                                                                                                  

But perhaps we can drop prohibitedAttachments for now, to try to reduce the complexity budget.

[robobario]

Thanks for the proposal @tombentley, getting my head around it slowly.

[robobario]

https://github.com/kroxylicious/design/blob/main/proposals/142-kube-api-routing.md has been accepted now, should we add KafkaProtocolRouter to the list? A user's router could also want to make network calls.

[robobario]

Would user supplied ports completely replace our generated ones? Taking the user's specifications is less surprising for audit, but more likely to leave them without connectivity if they don't allow ingress to the listen port. Since KPI port-allocation is an operator responsibility I don't think it would really be practical for a user to declare these, so I'm wondering, is there any case we can imagine where a user would want to add additional ports to the ingress policy beyond our listen ports and should we support that? I can't think of any, so maybe we could use CEL to block that combination early (referencing a KPI + ingress ports declared) and prevent misconfiguration?

I can imagine usages for ingress ports for the other types, like for Filters/Routers it would be possible for users to start up their own listening servers and want to configure ingress.

[robobario]

I think we should state here that the new resource is optional and we don't try to explicitly install a default. It looks like there's rationale already in the rejected alternatives about it being painful to keep helm/OLM/manifests in sync if we tried to do this.

[robobario]

(I later came upon the requirement in the goals to not manipulate existing NetworkPolicies, maybe that should be answered first, do we need to support this idea of different behaviour for new vs existing)

[robobario]

How are we planning to implement this. In the rejected alternatives it mentions we can't discriminate between new/existing since the optional resources could be absent.

The resourceGeneration section of the operator config is out of sync with this saying generation is enabled by default with no distinction between new/existing.

Does this goal need to be walked back? Or something added to describe how we can discriminate between new/existing?

(claude spotted)

[robobario]

KafkaProtocolFilter and KafkaService can be shared by multiple KafkaProxy, should this sentence be updated to say we generate NetworkPolicies per targeted KafkaProxy plural?

(claude spotted)

[robobario]

in the other bootstrapServers cases we nail down which are allowed to use namespace and pod selectors, vs ipBlock. Does the strimziKafkaRef have a similar restriction to selectors only because it's cluster local? We should add an equivalent statement about which types are allowed.

(claude spotted)

[robobario]

this is fine for our proxy resources, but what about the pods/namespaces targeted by the ingress/egress rules?

should I, on the proxy team, be able to allow the proxy to egress to some other teams namespace without something like ReferenceGrant for the other team to allow access? Can I allow egress to kube-system namespace for example?

(claude discovered)

[robobario]

similar question about allowing egress to some other teams namespace containing Kafka. Is it okay to not have a ReferenceGrant type resource, which would give the Kafka team the ability to grant the policy

[robobario]

Might be worth a callout that we've increased the Proxy blast radius. A kroxy operator watching all namespaces would be able to touch other tenants NetworkPolicy. This new access to the firewall type rules should be called out as a new risk.

(claude found)

[robobario]

One risk is that there's no visibility into whether your restrictions are widened. If I create a deny all egress rule for my filter, it can be flipped to an allow all by some other team/role setting up some other Filter with a wider policy, how do I know my policy has been weakened?

Maybe for visibility you could include it in the KafkaProxyNetworkPolicy, status, like list off all the associated KafkaProxyNetworkPolicies and whether their policy is redundant/widened?