Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure that policies with no valid peers are enforced #65

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

zeeke
Copy link
Member

@zeeke zeeke commented Jul 18, 2024

If a policy rule has a from (or to) selector that matches no pods, the subject pod has to not be reached by (or has to not reach) any pods.

The following example helps clarify the reasons behind these: Given a scenario with 3 pods (A, B, C) and a rule like:

podSelector:
    matchLabels:
      name: A
  ingress:
  - from:
    - podSelector:
        matchLabels:
          name: B
  policyTypes:
  - Ingress

Pod A can be reached only by pod B. Pod C can't reach A, and this has to be ensured even if pod B is deleted.

Add an end-to-end test case to validate this scenario and adjust unit tests accordingly.

cc @pliurh @s1061123

zeeke added 2 commits July 18, 2024 16:40
If a policy rule has a `from` (or `to`) selector that matches no pods,
the subject pod has to not be reached by  (or has to not reach) any pods.

The following example helps clarify the reasons behind these:
Given a scenario  with 3 pods (A, B, C) and a rule like:
```
podSelector:
    matchLabels:
      name: A
  ingress:
  - from:
    - podSelector:
        matchLabels:
          name: B
  policyTypes:
  - Ingress
```
Pod A can be reached only by pod B. Pod C can't reach A, and this has to be
ensured even if pod B is deleted.

Add an end-to-end test case to validate this scenario and adjust unit
tests accordingly.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
@coveralls
Copy link

coveralls commented Jul 18, 2024

Pull Request Test Coverage Report for Build 9993738204

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 58.742%

Totals Coverage Status
Change from base Build 8616153887: -0.2%
Covered Lines: 1102
Relevant Lines: 1876

💛 - Coveralls

@@ -280,7 +280,6 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
writeLine(ipt.policyIndex, "-A", fmt.Sprintf("MULTI-%d-INGRESS", pIndex), "-j", chainName)

s.podMap.Update(s.podChanges)
validPeers := 0

Choose a reason for hiding this comment

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

it seems to me that you don't need to count. This could be a boolean and switches to true when there is at least one valid peer

Copy link
Member Author

Choose a reason for hiding this comment

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

I completely removed that variable, as in my idea there is no need to check if there is at least one valid peer. Does it sound good?

Choose a reason for hiding this comment

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

right, I did not see it correctly

@bpickard22
Copy link

/assign bpickard22

@pliurh
Copy link

pliurh commented Sep 2, 2024

LGTM

@zeeke
Copy link
Member Author

zeeke commented Sep 5, 2024

@pliurh @bpickard22 can I somehow help to move this forward?

@dougbtv dougbtv merged commit 76cd193 into k8snetworkplumbingwg:master Sep 10, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants