Clarify compatibility constraints

[danwinship]

We need to be clear about our compatibility / API evolution constraints.

Specifically: what happens when the user creates a CNP that uses a feature which the CNP implementation doesn't know about?

Broadly speaking, there are two possible (useful) answers to that question:

1. It gets rejected (somehow), which interrupts the user's deployment workflow with a clear error and makes it clear that the CNP won't be supported.
2. It gets accepted, but the implementation is able to determine (somehow) that the CNP uses unsupported features, and then it has to do something sane in response.

While the first answer is generally better, it doesn't solve the problem of rollback: if a user upgrades their cluster, adds a CNP using a new feature, and then rolls the cluster back to the old version, then the old implementation will have to deal with the new CNP when it starts up again.

(Core APIs solve this problem by having a mandatory Alpha phase; as long as you wait until Beta before you use the feature, then it's guaranteed that if you roll back, you'll be rolling back to a release that still had the Alpha implementation. I don't think we can easily copy this.)

Possibility 1a: "Don't do that then" / API server validation rejects invalid CNPs.

If the CNP implementation itself always installs the CNP CRD itself, then only CNPs containing features supported by that CRD will validate, and CNPs containing unknown features would be rejected by the apiserver.

Having the CNP implementation install the CRD matches the way ANP is implemented/distributed currently. (The corresponding idea doesn't work for Gateway API because you might have multiple Gateway implementations in a single cluster, but at least for right now, it doesn't make sense to have multiple ANP/CNP implementations in a single cluster.)

The question is: can we guarantee that that will always work? I'm not sure we can. If we adopted something like Lior's proposed "IdentityNetworkPolicy", then in some clusters there would be two different components implementing different parts of the NetworkPolicy stack. Alternatively, if we later extend NP/CNP to support multi-network scenarios, then it might make sense to have one NP/CNP implementation for the cluster-default pod network, and a separate one for secondary pod networks.

So while this restriction makes sense now, it limits our future options.

Possibility 1b: A CNP-implementation-specific admission webhook rejects invalid CNPs

CNP implementations could be required to install a webhook that prevents the creation of CNPs that use features the CNP implementation doesn't understand. Basically, the webhook would deserialize the provided JSON object into the CNP implementation's generated golang version of the CRD, and then serialize it back to JSON, and see if any fields got lost.

This requires us to obey certain rules when adding new APIs, to ensure that implementations would be able to properly notice unsupported new features. For example, we could not add a new Action value, since that wouldn't show up as a difference when deserializing and reserializing the object. (Perhaps the webhook could run CRD validation against the object as well, to ensure that it validated against the version of the CRD it knows about? I'm not sure if the code for doing this is available outside the apiserver.)

Possibility 2a: The CNP implementation detects the existence of unsupported fields (and then does something)

The CNP implementation itself could recognize unsupported CNPs in roughly the same way the webhook would do in the previous example. For instance, it could use the generic client and deserialize to Unstructured to get the "raw" CNP from the apiserver, and then convert to the supported CNP struct and back as above.

Possibility 2b: We design the API in such a way that CNPs using unsupported features will have obvious "holes" (and when the implementation sees them, it does something)

This is the "exactly one field must be set, so if you see an object with no fields set then you know they must have used a feature you don't know about" approach. While this is theoretically easier to detect on the implementation end, it places more constraints on API evolution, and also (as compared to the previous approach), it means that the implementation must separately check for missing fields in every spot where they might occur, rather than being able to do a single top-level check for "some new field is missing".

If 2a is possible, it seems strictly superior to 2b.

---

With 2a and 2b, we also need to define what the implementation is supposed to do when it realizes that there is a bad CNP in the system. The docs currently suggest that the right approach is when in doubt, deny, but this runs the risk of completely breaking the cluster. Bowei was suggesting that a better approach is as soon as you see a bad CNP, stop processing updates until it is fixed, but this runs the risk of allowing the cluster to run for some length of time in an only-half-locked-down state. Neither approach is really very good. It seems clearly better to ensure that bad CNPs get rejected at creation time. The question is whether we need to deal with rollback also.

[bowei]

Slight nit: Is this an example of backward compat or forwards compat? It seems like the problem is what to do about an older implementation being compatible with a newer version of the CRD? Backwards compat talks about the a newer impl compatible with older config, which seem to be easier to handle (run tests)

[bowei]

It does feel like we should make it very hard to let users get into the skewed versions situation as it is very scary (basically 1b). We still need to define behavior for 2a | 2b just in case 1b fails, but it should come with lots of alerts etc.

Regarding forcing API structure to result in 2b -- my concern is that this will unnaturally paint us into a corner that we can't anticipate and at that point we get pretty stuck. With 2a there are some concerns about efficiency, but the scale of the Cluster-level network policy should be less of a problem.

(Note: My statement about stopping processing probably reflects my bias as a fully managed providers)

I was thinking if there was a reasonable "degraded mode" where you don't just go to a full deny all on unknown fields but allow some subset of the traffic. Example: v1 has "match TCP 80" and v2 has match "TCP 80 SYN"). In this case when you roll back to v1, then you could keep the DENY TCP 80, omitting the SYN, instead of DENY ALL.

It's not clear if that would really help: while it avoids some of the full outage, it still results in traffic blockage that the user did not intend.

[bowei]

I'm thinking that it's going to be hard to avoid something functionally similar to the alpha -> beta dance. We could state that for a secure deployment, people need to follow certain best practices otherwise they will get potentially insecure results? (Make implementation always ahead of CRD version or upgrade in two steps: implementation first, then validating webhook as a point of no return/rollback).

The basic problem with rollback is that network policy predicates have no real optionality and the blast radius of the API. This is in contrast with Gateway, where, in a lot of cases, the basic contract can be fulfilled and the blast radius is smaller.

Any degraded mode behavior would be quite unstable: there is definitely traffic that is being blocked that shouldn't be, and traffic that is not being accepted that should. The user needs to immediately intervene to fix the situation. (This is what led to my "fail static" suggestion).

[bowei]

One other possibility for what to do with config you don't understand: treat DENY and PASS similar to ACCEPT and ignore the rule. This makes it so that if the admin wants to use a new brand feature and they know that after rollback, it's predictable that those rules will disappear. I think this might be easier for the admins to reason about and for us to write best practices guides around.

[danwinship]

I'm thinking that it's going to be hard to avoid something functionally similar to the alpha -> beta dance.

The k8s project can impose mandatory-alpha-for-one-release on itself, but it's harder for us to impose that on third-party CNP implementations. And if they have a longer release cycle than k8s does then it's going to be even harder to get them to go along. ("You can add disabled alpha support for protocols in your 2026 release, and then actually support it in your 2027 release.")

We could state that for a secure deployment, people need to follow certain best practices

Yeah, I think we're going to have to accept that part of the answer here will be documentation. We can't make it impossible for someone to create a CNP with a new feature and then roll back their CNP implementation, but I'm pretty sure we also can't do anything generically correct if they do. The "best practice" here is "roll back your new CNPs before you roll back your CNP implementation".

(This is what led to my "fail static" suggestion).

But unfortunately "fail static" doesn't help in the rollback situation. Actually, in the cases where you allow invalid CNPs to be created, then "fail static" doesn't work even without rollback, if you restart your network plugin after the bad CNP is created, or add new nodes with new instances of the CNP implementation starting from scratch.

I was thinking if there was a reasonable "degraded mode" where you don't just go to a full deny all on unknown fields but allow some subset of the traffic. Example: v1 has "match TCP 80" and v2 has match "TCP 80 SYN"). In this case when you roll back to v1, then you could keep the DENY TCP 80, omitting the SYN, instead of DENY ALL.

That's the approach we did with NetworkPolicy (eg, "allow port range 8080-8088" degrades to "allow port 8080" if you don't recognize endPort) but I don't think we can make that work in CNP when both allows and denies are possible. Even if we can, it requires us to be very careful with our API design...

One other possibility for what to do with config you don't understand: treat DENY and PASS similar to ACCEPT and ignore the rule. This makes it so that if the admin wants to use a new brand feature and they know that after rollback, it's predictable that those rules will disappear. I think this might be easier for the admins to reason about and for us to write best practices guides around.

I like the idea of making it possible to write rules in such a way that they explicitly give the fallback behavior for older releases. I think we would need to add explicit syntax for this though.

Take the TCP 80 SYN case above. You don't want to write

1. deny TCP 80 SYN # preferred rule
2. deny TCP 80 # fallback rule

because in that case you end up always denying all TCP 80 packets anyway. You could try something like

1. deny TCP 80 SYN # preferred rule
2. pass TCP 80 ~SYN # skip next rule if TCP flags are recognized
3. deny TCP 80 # fallback rule

but that's tricky and may not be exactly what you meant (you're assuming that there are no other lower-precedence rules that would have matched the packet).

So what you really want is something like

1. if FeatureGateEnabled(TCPFlags) deny TCP 80 SYN
2. if !FeatureGateEnabled(TCPFlags) deny TCP 80

[bowei]

Notes from discussion of the above (with @danwinship and @npinaeva):

Implementations MUST introduce some way to ensure that in regular operation it is hard/impossible to introduce fields that the implementation does not recognize. This could be done by adding an admission webhook or in the case of a managed provider, having a tight control over the installed CRD (e.g. 1a and 1b).

We will document best practices that SHOULD be followed to avoid bad skew. Example: Always update implementation before CRD, Do API rollout in two phases: add implementation support first, stabilize and then update the API, etc.

It's going to be hard for the project to programmatically enforce that the best-practices are followed, so is a responsibility for the cluster operator.

This leaves rollback as the primary way user will get into trouble. To help deal with this:

• We will have a small client-side library implementing 2a, detection of version-skewed configuration. It is not required to use this library but it will be a production-quality example of how it could be done.

• Add a standardized condition to the CNP resource status that allows implementations to flag problematic configurations. We want to be able to support the follow flow for an admin:

  • Operator downgrades implementation. Cluster config now may need fixing.
  • Admin lists all ClusterNetworkPolicy objects -- and see that there are some (hopefully few) resources that have been flagged via this status.
  • Do the fixes/delete etc on the flagged policy resources.
  • Admin should see that all of the bad resources are fixed and the implementation should be able to work.

It's not required that an implementation flag ALL violating resources but it is recommended (SHOULD vs MUST).

Finally:

• Implementations are free to define how they handle unknown fields. This is a change from the current behavior where it is prescribed what happens (DENY, PASS => DENY ALL, ACCEPT => skip). There are some reasonable paths that implementations could take such as :
  • Ignore unknown fields on a rule level.
  • Ignore unknown fields on the CR level.
  • Fail static with previous configuration.
  • Current behavior (turn DENY to DENY ALL etc)
  • (Some other very clever way)