Change wording on toServices limitations (see #20067)

[atykhyy]

#21052 updated Cilium documentation to say that, in network policy rules, toServices statements cannot be combined with toPorts statements. I believe it would be more informative for users of Cilium to say (following RFC 2119) that toServices must not be combined with toPorts, as technically Cilium accepts such a configuration as valid but handles it in the unexpected and unuseful manner described in #20067.

[maintainer-s-little-helper]

Commit e832a28b6303cb0fc3de2603d0e12f6df2d4bc96 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

[qmonnet]

Looks good, thank you!

[christarazi]

Thanks for the PR!

Checkpatch (checks your commits) is failing with

   Warning: WARNING:TYPO_SPELLING: 'unuseful' may be misspelled - perhaps 'useless'?
  #11: 
  as valid but handles it in the unexpected and unuseful manner described
                                                ^^^^^^^^

[joestringer]

I agree about the "must not" RFC 2119 wording. Personally I'd avoid making any statements about how the limitation behaves, since it's a limitation - we don't want users to rely on this, this is not API, it is subject to change without warning. Ideally someone will pick this up and fix the limitation, and at that point I'd hope that users aren't assuming that the limitation always has these semantics.

But 🤷 I don't feel super strongly about this. Just want to say, we reserve the right to break this statement at any time.

[atykhyy]

I agree that documenting incorrect behavior is not a very good idea, even if it's explicitly stated that the behavior is incorrect. On the other hand a simple statement of limitation feels insufficiently scary: the rule apparently works (makes the service accessible), but has non-obvious and potentially dangerous additional effects. Perhaps it's best to simply make rules with both toServices and toPorts fail validation? A couple lines in pkg\policy\api\rule_validation.go:sanitize(*EgressRule) would suffice.

[ti-mo]

/test

[ti-mo]

Some changes were made to CI in past weeks so this might need a rebase. I've approved the workflows and marked this a release blocker. This would be nice to ship in 1.14.

[ti-mo]

@atykhyy Please rebase to re-run Travis and to pick up some fixes for flakes etc.

[ti-mo]

@atykhyy one test failing:

--- FAIL: Test/PolicyAPITestSuite#01/TestToServicesSanitize (0.00s)

    rule_validation_test.go:630: 

        ... value *errors.errorString = &errors.errorString{s:"Combining ToServices and ToPorts is not supported yet"} ("Combining ToServices and ToPorts is not supported yet")

[atykhyy]

@ti-mo Right. It's the test which checks for toServices+toPorts policy which fails validation per this PR. I have reversed the test result.

[ti-mo]

Sorry for the trouble, looks like image builds are failing, something probably changed on main. Could you give this another rebase? 🙏

[ti-mo]

/test