lunny/helm-chart
1
0
Fork 1
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

WIP: Implementing Network Policy From PR 207 #306

Closed
safaG wants to merge 3 commits from netpol into main
pull from: netpol
merge into: lunny:main
Conversation 7 Commits 3 Files Changed 3 +927 -6845
safaG commented 2022-03-12 04:46:43 +00:00 (Migrated from gitea.com)
Copy Link

From PR #207

Hi All!

I have created network policy yaml file and adjusted helpers.tpl file in order to stop gitea pods from communicating outside of gitea pods. What I have is really basic as I am not a pro with helm charts. Maybe there is another way of doing it better but this is what I have. What I did was to add below to _helpers.tpl file:

{{/*
Network Policy labels
*/}}
{{- define "gitea.netpolLabels" -}}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end -}}

I have added this file to pull the unique label that Gitea creates on all pods. Then I created the networkpolicy.yaml file and used the above label under matchLabels: in the networkpolicy.yaml file

I have tested this with a new deployment and everything seemed working fine. However not sure if it will be a breaking change with existing deployments, I have not tested that.

From PR [#207](https://gitea.com/gitea/helm-chart/pulls/207) Hi All! I have created network policy yaml file and adjusted helpers.tpl file in order to stop gitea pods from communicating outside of gitea pods. What I have is really basic as I am not a pro with helm charts. Maybe there is another way of doing it better but this is what I have. What I did was to add below to _helpers.tpl file: ``` {{/* Network Policy labels */}} {{- define "gitea.netpolLabels" -}} app.kubernetes.io/instance: {{ .Release.Name }} {{- end -}} ``` I have added this file to pull the unique label that Gitea creates on all pods. Then I created the networkpolicy.yaml file and used the above label under matchLabels: in the networkpolicy.yaml file I have tested this with a new deployment and everything seemed working fine. However not sure if it will be a breaking change with existing deployments, I have not tested that.
safaG commented 2022-03-12 04:50:11 +00:00 (Migrated from gitea.com)
Copy Link

@justusbunsi this is ready to be test.

@justusbunsi this is ready to be test.
justusbunsi (Migrated from gitea.com) reviewed 2022-04-21 09:47:44 +00:00
justusbunsi (Migrated from gitea.com) left a comment
Copy Link

Please add a new section about the possible networkpolicy settings inside README.md.

Please add a new section about the possible networkpolicy settings inside README.md.
templates/_helpers.tpl
@ -121,1 +65,4 @@
Network Policy labels
*/}}
{{- define "gitea.netpolLabels" -}}
app.kubernetes.io/instance: {{ .Release.Name }}
justusbunsi (Migrated from gitea.com) commented 2022-04-21 09:47:44 +00:00
Copy Link

Only selecting the instance will also find pods of memcached deployment which could create unexpected errors if the policies are too strict. I'd suggest using the existing gitea.selectorLabels helper function instead of defining a new one. The existing one combines app.kubernetes.io/instance label with app.kubernetes.io/name label and therefore only will find the pod(s) created by the Gitea Statefulset.

Only selecting the instance will also find pods of `memcached` deployment which could create unexpected errors if the policies are too strict. I'd suggest using the existing `gitea.selectorLabels` helper function instead of defining a new one. The existing one combines `app.kubernetes.io/instance` label with `app.kubernetes.io/name` label and therefore only will find the pod(s) created by the Gitea Statefulset.
templates/gitea/network-policy.yaml
@ -0,0 +6,4 @@
spec:
podSelector:
matchLabels:
{{- include "gitea.netpolLabels" . | nindent 6 }}
justusbunsi (Migrated from gitea.com) commented 2022-04-21 09:48:41 +00:00
Copy Link

As suggested above:

- {{- include "gitea.netpolLabels" . | nindent 6 }}
+ {{- include "gitea.selectorLabels" . | nindent 6 }}
As suggested above: ```diff - {{- include "gitea.netpolLabels" . | nindent 6 }} + {{- include "gitea.selectorLabels" . | nindent 6 }} ```
templates/gitea/network-policy.yaml
@ -0,0 +11,4 @@
- from:
- podSelector:
matchLabels:
{{- include "gitea.netpolLabels" . | nindent 10 }}
justusbunsi (Migrated from gitea.com) commented 2022-04-21 09:49:07 +00:00
Copy Link

As suggested above:

- {{- include "gitea.netpolLabels" . | nindent 6 }}
+ {{- include "gitea.selectorLabels" . | nindent 6 }}
As suggested above: ```diff - {{- include "gitea.netpolLabels" . | nindent 6 }} + {{- include "gitea.selectorLabels" . | nindent 6 }} ```
templates/gitea/network-policy.yaml
@ -0,0 +13,4 @@
matchLabels:
{{- include "gitea.netpolLabels" . | nindent 10 }}
- ipBlock:
cidr: {{ .Values.networkpolicy.cidr }}
justusbunsi (Migrated from gitea.com) commented 2022-04-21 10:10:39 +00:00
Copy Link
- cidr: {{ .Values.networkpolicy.cidr }}
+ cidr: {{ required ".Values.networkpolicy.cidr must be configured" .Values.networkpolicy.cidr }}

See next comment

```diff - cidr: {{ .Values.networkpolicy.cidr }} + cidr: {{ required ".Values.networkpolicy.cidr must be configured" .Values.networkpolicy.cidr }} ``` See [next comment](https://gitea.com/gitea/helm-chart/pulls/306/files#issuecomment-695448)
values.yaml
@ -391,3 +159,1 @@
nodeSelector: {}
tolerations: []
affinity: {}
cidr: 10.0.0.0/8
justusbunsi (Migrated from gitea.com) commented 2022-04-21 10:07:54 +00:00
Copy Link

This cidr value will most likely differ for every Kubernetes cluster which makes a default value not useful here, IMO.
To indicate that this setting has to be defined when .Values.networkpolicy.enabled is true, you could add a required condition when using the value, which breaks the helm install if the value is not specified.

My suggestion is to make that cidr line a comment. It would align with other conditional settings of other configuration sections.

This `cidr` value will most likely differ for every Kubernetes cluster which makes a default value not useful here, IMO. To indicate that this setting has to be defined when `.Values.networkpolicy.enabled` is `true`, you could add a _required_ condition when using the value, which breaks the _helm install_ if the value is not specified. My suggestion is to make that cidr line a comment. It would align with other conditional settings of other configuration sections.
justusbunsi (Migrated from gitea.com) commented 2022-07-13 16:40:44 +00:00
Copy Link

ipBlock/cidr is supposed to be used only from matching external ip

[from https://kubernetes.io/docs/concepts/services-networking/network-policies/]
ipBlock: This selects particular IP CIDR ranges to allow as ingress sources or egress destinations. These should be cluster-external IPs, since Pod IPs are ephemeral and unpredictable.

ipBlock/cidr is supposed to be used only from matching external ip [from https://kubernetes.io/docs/concepts/services-networking/network-policies/] ipBlock: This selects particular IP CIDR ranges to allow as ingress sources or egress destinations. These should be cluster-external IPs, since Pod IPs are ephemeral and unpredictable.
jouve commented 2022-07-13 16:55:40 +00:00 (Migrated from gitea.com)
Copy Link

this PR will break ingress/load balancer usage (as it disallow traffic the ingress controller to gitea pods)

this PR will break ingress/load balancer usage (as it disallow traffic the ingress controller to gitea pods)
jouve commented 2022-07-13 16:56:02 +00:00 (Migrated from gitea.com)
Copy Link

should gitea pods even be talking between themselves ?

should gitea pods even be talking between themselves ?
justusbunsi commented 2022-07-13 17:11:27 +00:00 (Migrated from gitea.com)
Copy Link

To me this PR is still considered WIP. There are open things.

  • egress configuration
  • maybe just map the full network policy as-is to the template
To me this PR is still considered WIP. There are open things. - egress configuration - maybe just map the full network policy as-is to the template
pat-s commented 2023-03-28 21:46:12 +00:00 (Migrated from gitea.com)
Copy Link

This looks stale meanwhile. I'd vote to close if nobody wants to actively tackle this (again) in the near future?

This looks stale meanwhile. I'd vote to close if nobody wants to actively tackle this (again) in the near future?
👍 1
pat-s commented 2023-04-01 07:30:52 +00:00 (Migrated from gitea.com)
Copy Link

Feel free to re-open if desired!

Feel free to re-open if desired!

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
has/backport
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/other
No Label
kind/feature
status/wip
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
lunny
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#306
No description provided.
Delete Branch "netpol"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?