Skip to content

Conversation

aanm
Copy link
Member

@aanm aanm commented Aug 5, 2020

Marked for backport on all branches so that users are clarify about this on all Cilium versions. Ping me if the backport didn't apply correctly.

aanm added 2 commits August 5, 2020 12:48
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Aug 5, 2020
@aanm aanm requested review from joestringer and pchaigno August 5, 2020 13:22
@aanm aanm requested a review from a team as a code owner August 5, 2020 13:22
@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 36.874% when pulling 243a036 on aanm:pr/add-k8s-docs into d80e66f on cilium:master.

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think ipBlock must match pod IPs per the KNP specs? Otherwise LGTM

+-------------------------------+----------------------------------------------+
| Feature | Tracking Issue |
+===============================+==============================================+
| ``ipBlock`` set with a pod IP | https://github.com/cilium/cilium/issues/9209 |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK this isn't actually specified in KNP standards.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, but it is also not specified anywhere that it can't be a pod IP. There are upstream k8s tests that were currently skipping because of this limitation.

# We currently skip the following tests:
# should not allow access by TCP when a policy specifies only SCTP
# - Cilium does not support SCTP yet
# should allow egress access to server in CIDR block and
# should ensure an IP overlapping both IPBlock.CIDR and IPBlock.Except is allowed
# - TL;DR Cilium does not allow to specify pod CIDRs as part of the policy
# because it conflicts with the pod's security identity.
# - More info at https://github.com/cilium/cilium/issues/9209

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's fair. Probably worth adding a comment to the related issue though to explain that the same traffic can (and should) be allowed via endpointSelector/podSelector policy instead of ipBlock - ie show the mitigation in case someone clicks through to that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants