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

[grafana] feat(network policy): extend Ingress #3287

Closed
wants to merge 7 commits into from

Conversation

C4tWithShell
Copy link

@C4tWithShell C4tWithShell commented Aug 24, 2024

Add extraIngress parameter to network policy since it does not allow to specify additional sources from the grafana helm chart . For example, I can not set up load balancer in the cluster.

@CLAassistant
Copy link

CLAassistant commented Aug 24, 2024

CLA assistant check
All committers have signed the CLA.

@C4tWithShell C4tWithShell changed the title feat(networkpolicy): extendIngress [grafana] feat(network policy): extend Ingress Aug 24, 2024
@C4tWithShell
Copy link
Author

@Sheikh-Abubaker @jkatsos @maorfr @torstenwalter @Xtigyro @zanhsieh
Appreciate if able to review the PR when available, thanks!

Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker left a comment

Choose a reason for hiding this comment

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

@C4tWithShell Thanks for the Contribution! Have you tested the proposed solution ?

@C4tWithShell
Copy link
Author

@C4tWithShell Thanks for the Contribution! Have you tested the proposed solution ?

Yes, I did

Signed-off-by: Vladimir Shelkovnikov <[email protected]>
@Sheikh-Abubaker
Copy link
Collaborator

Sheikh-Abubaker commented Sep 5, 2024

@C4tWithShell If I didn't get you wrong in the extraIngress comments, you've illustrated an example of accepting traffic from NGINX right by setting appropriate matchLabels ? in order to use different ingress sources right ?

@Sheikh-Abubaker
Copy link
Collaborator

Sheikh-Abubaker commented Sep 5, 2024

I believe we could achieve the same result, by adding the appropriate labels in the existing networkpolicy.yaml right here:

I'd recommend you to first try and test using the existing networkpolicy.yaml utilizing the above feature and I believe it would help you to achieve the same result as extraIngress.

Incase you want to accept traffic from some different namespace, you could utilize the explicitNamespaceSelector feature for that:

explicitNamespacesSelector: {}

@Sheikh-Abubaker
Copy link
Collaborator

@C4tWithShell any updates on this one ?

@C4tWithShell
Copy link
Author

@C4tWithShell any updates on this one ?

Hi! Yes, I was able to configure ingress network policy with adding the addition label to ingress-nginx pod.
Even though I think it is a little bit confusing to set up network policy indirectly

@Sheikh-Abubaker
Copy link
Collaborator

Sheikh-Abubaker commented Sep 15, 2024

@C4tWithShell any updates on this one ?

Hi! Yes, I was able to configure ingress network policy with adding the addition label to ingress-nginx pod.

You mean by using the existing Network Policy right ?

@C4tWithShell
Copy link
Author

@C4tWithShell any updates on this one ?

Hi! Yes, I was able to configure ingress network policy with adding the addition label to ingress-nginx pod.

You mean by the existing Network Policy right ?

Yes. By adding the grafana-client label

@Sheikh-Abubaker
Copy link
Collaborator

Sheikh-Abubaker commented Sep 15, 2024

Yes. By adding the grafana-client label

All right! shall I close this one then @C4tWithShell ?

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.

4 participants