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

feat(testkube): add networkpolicy support #784

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

windowsrefund
Copy link

@windowsrefund windowsrefund commented Mar 21, 2024

Pull request description

Adds Network Policies support

Checklist (choose whats happened)

  • breaking change! (describe)
  • tested locally
  • tested on cluster
  • added new dependencies
  • updated the docs
  • added a test

Breaking changes

  • none

Changes

  • none

Fixes

  • none

Additional Info

This will be useful to those folks who use network policies with a default-deny configuration. Only Testkube-specific traffic has been accommodated for and by default, these resources will not be created (must enable in values.yaml). I came up with these rules by watching for PacketDrop events (provided by kube-iptables-tailor) in the testkube namespace while running my Test Suites, etc. Hopefully, I didn't miss anything! Also, I am still on 1.16.64 so I added support for the dashboard if enabled.

I didn't update the README as I came to believe it may be auto-generated. LMK if I'm wrong and I'll be happy to update accordingly.

To see the template rendered:

cat << EOF > /tmp/values.yaml
networkPolicy:
  enabled: true
EOF

Now from inside the testkube chart directory

helm template testkube . -f values.yaml -f /tmp/values.yaml -s templates/networkpolicy.yaml

egress:
- to:
- podSelector:
matchLabels:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be done similar to how services use a helper method? Example https://github.com/kubeshop/helm-charts/blob/develop/charts/testkube-api/templates/service.yaml#L27

Copy link
Author

Choose a reason for hiding this comment

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

feel free to refactor to your hearts content

Copy link
Contributor

Choose a reason for hiding this comment

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

could you assist? :)

namespace: testkube
spec:
podSelector:
matchLabels:
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above comment for selector

ingress:
- ports:
- protocol: TCP
port: 9443
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this also be referenced through a param?

egress:
- to:
- podSelector:
matchLabels:
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment for selectors

port: 4222
- to:
- podSelector:
matchLabels:
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment for selectors, they should be used through a helper funcion (from the mongodb chart?)

ports:
- protocol: TCP
port: 4222
- to:
Copy link
Contributor

Choose a reason for hiding this comment

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

this might need to be dynamic if mongodb is disabled (customer is bringing own mongo)

Copy link
Contributor

@vsukhin vsukhin left a comment

Choose a reason for hiding this comment

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

I agree, idea is good. we need to remove hard code parts, - namespace, ports. etc

@emil2k emil2k removed their request for review March 26, 2024 09:01
@windowsrefund
Copy link
Author

Just wanted to mention these rules are still valid against the 2.0.17 Helm chart.

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.

None yet

4 participants