Skip to content

Limit Istio Sidecar Scope to reduce memory and make cluster more scalable #3052

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/istio-1-24/istio-install/base/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2836,7 +2836,7 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
image: gcr.io/istio-release/proxyv2:1.24.2
image: gcr.io/istio-release/proxyv2:1.24.3
name: istio-proxy
ports:
- containerPort: 15021
Expand Down
1 change: 1 addition & 0 deletions common/istio-1-24/istio-install/base/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ resources:
- gateway_authorizationpolicy.yaml
- deny_all_authorizationpolicy.yaml
- gateway.yaml
- sidecar-prune-egress.yaml

patches:
- path: patches/service.yaml
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: networking.istio.io/v1alpha3
kind: Sidecar
metadata:
name: prune-egress
spec:
egress:
- hosts:
- "./*" # use mTLS within the namespace

Check warning on line 8 in common/istio-1-24/istio-install/base/sidecar-prune-egress.yaml

View workflow job for this annotation

GitHub Actions / format_YAML_files

8:13 [comments] too few spaces before comment
Copy link
Member

@juliusvonkohout juliusvonkohout Mar 16, 2025

Choose a reason for hiding this comment

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

Does that block stuff that is not on the servicemesh by default ? E.g. talking from your jupyterlab to a deployment in the same namespace with istio-injection disabled. Or egress to a non-istio native kubernetes service in another namespaces or just the internet in general. Because that must be still allowed. Maybe we could just block inter kubeflow-profile namespace communication by default.

Copy link
Author

Choose a reason for hiding this comment

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

Yes all traffic is still allowed, this rule just removes mTLS and egress rules from being enforced, which I don't think we have any anyways. The "./*" still enables mTLS in the same namespace. See the istio documentation for details. I would love to hear from a kubeflow istio person about this idea

Copy link
Member

@juliusvonkohout juliusvonkohout Mar 17, 2025

Choose a reason for hiding this comment

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

We have many mtls rules for the namespace "kubeflow". Please check the destination rules in the manifests first. We should not prune the existing explicit ones.

Copy link
Author

Choose a reason for hiding this comment

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

These destination rules all point to other services in the "kubeflow" namespace, which this policy allows. If you want, we can also have this policy only apply to user namespaces (kubeflow-*) by using a kyverno policy

Copy link
Member

Choose a reason for hiding this comment

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

There is "kubeflow", knative-serving, istio-system, oauth2-proxy etc whil the user namespaces are named arbitrarily. They could be thisismycoolusernamespace. Also kyverno is not yet part of the default platform and we need to work with the tools that are there by default.

"These destination rules all point to other services in the "kubeflow" namespace, which this policy allows" if that is true it might work. but please add a test in this PR here to verify the functionality (connection and mtls).

- "kubeflow/*" # use mTLS when communicating with kubeflow namespace

Check warning on line 9 in common/istio-1-24/istio-install/base/sidecar-prune-egress.yaml

View workflow job for this annotation

GitHub Actions / format_YAML_files

9:20 [comments] too few spaces before comment
2 changes: 1 addition & 1 deletion common/istio-cni-1-24/istio-install/base/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3148,7 +3148,7 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
image: gcr.io/istio-release/proxyv2:1.24.2
image: gcr.io/istio-release/proxyv2:1.24.3
name: istio-proxy
ports:
- containerPort: 15021
Expand Down