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

Add NetworkPolicies to isolate ArgoCD from rest of cluster #166

Merged
merged 7 commits into from
Jun 21, 2024

Conversation

HappyTetrahedron
Copy link
Contributor

Implements #149

Checklist

  • The PR has a meaningful title. It will be used to auto-generate the
    changelog.
    The PR has a meaningful description that sums up the change. It will be
    linked in the changelog.
  • PR contains a single logical change (to build a better changelog).
  • Update the documentation.
  • Categorize the PR by adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog.
  • Link this PR to related issues or PRs.

@HappyTetrahedron HappyTetrahedron added the enhancement New feature or request label Jun 19, 2024
@HappyTetrahedron HappyTetrahedron marked this pull request as draft June 19, 2024 13:42
@HappyTetrahedron HappyTetrahedron force-pushed the feat/isolation-netpol branch 2 times, most recently from fb8981d to 3480ea3 Compare June 20, 2024 08:49
@HappyTetrahedron HappyTetrahedron marked this pull request as ready for review June 20, 2024 08:50
@HappyTetrahedron
Copy link
Contributor Author

I figured that for non-openshift, we can specify the monitoring NS via allow_from_namespaces. I didn't feel like cross-pulling that info from all over the hierarchy.

RKE is as yet untested though; gonna do that on Friday.

@HappyTetrahedron HappyTetrahedron requested a review from a team June 20, 2024 08:52
Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

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

For setups which use component-prometheus, we should be able to generate the correct NetworkPolicy for the monitoring stack via component-prometheus's component library with prometheus.NetworkPolicy(), see also the example in https://hub.syn.tools/prometheus/how-tos/cluster-monitoring.html#_advertise_metrics_from_a_component

component/argocd.jsonnet Outdated Show resolved Hide resolved
@HappyTetrahedron HappyTetrahedron requested a review from simu June 20, 2024 12:59
Copy link
Member

@simu simu left a comment

Choose a reason for hiding this comment

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

I figured that for non-openshift, we can specify the monitoring NS via allow_from_namespaces. I didn't feel like cross-pulling that info from all over the hierarchy.

As mentioned in the previous review comment already, we should use prometheus.NetworkPolicy() which exists exactly so that components can provide seamless integration with component-prometheus instead of requiring users of component-prometheus to explicitly configure their Prometheus namespace when network policies are enabled, especially since we already handle the other half of it by wrapping the namespace in prometheus.RegisterNamespace() when component-prometheus is installed on the cluster.

component/namespace.jsonnet Outdated Show resolved Hide resolved
component/networkpolicy.libsonnet Outdated Show resolved Hide resolved
HappyTetrahedron and others added 5 commits June 20, 2024 16:24
Template version: main (26ee71e)
Template version: main (26ee71e)
Template version: main (26ee71e)
@HappyTetrahedron
Copy link
Contributor Author

As mentioned in the previous review comment already, we should use prometheus.NetworkPolicy()

I should learn how to read 🙈

@HappyTetrahedron HappyTetrahedron requested a review from simu June 21, 2024 09:24
@HappyTetrahedron HappyTetrahedron merged commit b24d3fa into master Jun 21, 2024
13 checks passed
@HappyTetrahedron HappyTetrahedron deleted the feat/isolation-netpol branch June 21, 2024 12:50
Copy link

github-actions bot commented Jun 21, 2024

No bump labels present

🛠️ Auto tagging disabled

@HappyTetrahedron
Copy link
Contributor Author

naw I did it wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump:minor enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants