Skip to content

Conversation

@pvannierop
Copy link
Contributor

This PR will add networkPolicy to the new radar-redis service. It also updates the netpol of existing services that connect to radar-redis.

@pvannierop pvannierop requested a review from ewelinagr October 27, 2025 15:47
@pvannierop pvannierop self-assigned this Oct 27, 2025
@github-actions
Copy link

github-actions bot commented Oct 27, 2025

Great PR! Please pay attention to the following items before merging:

Files matching charts/*/values.yaml:

  • Is the PR adding a new container? Please reviewer, add it to the models (internal process)
  • Is the PR adding a new parameter? Please, ensure it’s documented in the README.md

This is an automatically generated QA checklist based on modified files.

podSelector:
matchLabels:
{{ include "common.labels.matchLabels" . | indent 6 }}
{{- tpl (toYaml .Values.networkpolicy) . | nindent 2 }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a problem here, as tpl would also include this custom line from .Values.networkpolicy:

  enabled: true

I don't think it's a valid field in the Kubernetes NetworkPolicy.spec. Also, in other component's network policies we don't use enabled property, if we inject it to network policy with "tpl".

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe remove the enabled and just use:
{{- if .Values.networkpolicy }}
check like in other network policies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ewelinagr Fixed!

@pvannierop pvannierop force-pushed the feat/add-redis-netpol branch from f25d9a4 to 021e8d7 Compare October 29, 2025 08:07
@pvannierop pvannierop requested a review from ewelinagr October 29, 2025 12:20
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.

3 participants