Skip to content

feat: Add ambient mode support to profile-controller #127

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.

Sign up for GitHub

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
madmecodes wants to merge 8 commits into
base: main
Choose a base branch
from
+432 −37
Open

feat: Add ambient mode support to profile-controller #127

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2ecf65c
feat: Add ambient mode support to profile-controller
madmecodes Jul 15, 2025
b333928
update: review changes
madmecodes Aug 19, 2025
7c8ab38
update: variable load test fix
madmecodes Aug 26, 2025
a8b460b
update: Keep Variable Substitution Where It Works
madmecodes Aug 26, 2025
2f50157
update: review comments changes
madmecodes Aug 26, 2025
c5a9989
update: function used in pipeline test
madmecodes Aug 26, 2025
063d073
updated profile controller tests to use setNamespaceLabelsAndServiceMesh
madmecodes Aug 26, 2025
29274e3
feat: complete ambient mode support with HTTPRoute fixes
madmecodes Sep 10, 2025
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
4 changes: 4 additions & 0 deletions components/profile-controller/config/manager/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ configMapGenerator:
- ISTIO_INGRESS_GATEWAY_PRINCIPAL="cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account"
- NOTEBOOK_CONTROLLER_PRINCIPAL="cluster.local/ns/kubeflow/sa/notebook-controller-service-account"
- KFP_UI_PRINCIPAL="cluster.local/ns/kubeflow/sa/ml-pipeline-ui"
- SERVICE_MESH_MODE="istio-sidecar"
- WAYPOINT_NAME="waypoint"
- WAYPOINT_NAMESPACE=""
- CREATE_WAYPOINT="false"
name: config
8 changes: 8 additions & 0 deletions components/profile-controller/config/manager/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ spec:
- $(USERID_PREFIX)
- "-workload-identity"
- $(WORKLOAD_IDENTITY)
- "-service-mesh-mode"
- $(SERVICE_MESH_MODE)
- "-waypoint-name"
- $(WAYPOINT_NAME)
- "-waypoint-namespace"
- $(WAYPOINT_NAMESPACE)
- "-create-waypoint"
- $(CREATE_WAYPOINT)
envFrom:
- configMapRef:
name: config
Expand Down
14 changes: 14 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/authorizationpolicy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: security.istio.io/v1beta1
kind: AuthorizationPolicy
metadata:
name: profiles-kfam
spec:
action: ALLOW
rules:
- from:
- source:
principals:
- cluster.local/ns/kubeflow/sa/centraldashboard
selector:
matchLabels:
kustomize.component: profiles
30 changes: 30 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/httproute.yaml
View file
Open in desktop
Copy link
Contributor

@orfeas-k orfeas-k Aug 22, 2025

Choose a reason for hiding this comment

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

When I tried to apply this I got

The HTTPRoute "profiles-kfam" is invalid: 
* spec.hostnames[0]: Invalid value: "*": spec.hostnames[0] in body should match '^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'
* spec.parentRefs[0].namespace: Invalid value: "$(GATEWAY_NAMESPACE)": spec.parentRefs[0].namespace in body should match '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'
* spec.rules[0].backendRefs[0].namespace: Invalid value: "$(PROFILES_NAMESPACE)": spec.rules[0].backendRefs[0].namespace in body should match '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'

Copy link
Contributor Author

@madmecodes madmecodes Aug 26, 2025

Choose a reason for hiding this comment

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

When I tried to apply this I got

The HTTPRoute "profiles-kfam" is invalid: 
* spec.hostnames[0]: Invalid value: "*": spec.hostnames[0] in body should match '^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$'
* spec.parentRefs[0].namespace: Invalid value: "$(GATEWAY_NAMESPACE)": spec.parentRefs[0].namespace in body should match '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'
* spec.rules[0].backendRefs[0].namespace: Invalid value: "$(PROFILES_NAMESPACE)": spec.rules[0].backendRefs[0].namespace in body should match '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'

i tried, some issue in injecting variable values, if i harcode them like this they work and are installed,

- $(GATEWAY_NAMESPACE) to istio-system
- $(PROFILES_NAMESPACE) to kubeflow

this is how i am installing:

  1. kind create cluster --name profile-controller-test
  2. kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.2.0/standard-install.yaml
  3. kustomize build /Users/themadme/gsoc/manifests/common/istio/istio-crds/base | kubectl apply -f -

Finally:
kustomize build config/overlays/kubeflow-ambient | kubectl apply --dry-run=client -f -
Result: All Resources Validated Successfully

Am i missing anything?

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
name: profiles-kfam
spec:
parentRefs:
- name: $(GATEWAY_NAME)
namespace: $(GATEWAY_NAMESPACE)
hostnames:
- '*'
rules:
- matches:
- path:
type: PathPrefix
value: /kfam/
filters:
- type: RequestHeaderModifier
requestHeaderModifier:
add:
- name: x-forwarded-prefix
value: /kfam
- type: URLRewrite
urlRewrite:
path:
type: ReplacePrefixMatch
replacePrefixMatch: /kfam/
backendRefs:
- name: profiles-kfam
namespace: $(PROFILES_NAMESPACE)
port: 8081
47 changes: 47 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: kubeflow
resources:
- ../../base
- service.yaml
- httproute.yaml
- authorizationpolicy.yaml

commonLabels:
kustomize.component: profiles

patchesStrategicMerge:
- patches/kfam.yaml
- patches/manager.yaml
- patches/remove-namespace.yaml

configurations:
- params.yaml

vars:
- name: PROFILES_NAMESPACE
fieldref:
fieldpath: metadata.namespace
objref:
name: profiles-kfam
kind: Service
apiVersion: v1
- name: GATEWAY_NAME
fieldref:
fieldpath: spec.parentRefs[0].name
objref:
name: profiles-kfam
kind: HTTPRoute
apiVersion: gateway.networking.k8s.io/v1beta1
- name: GATEWAY_NAMESPACE
fieldref:
fieldpath: spec.parentRefs[0].namespace
objref:
name: profiles-kfam
kind: HTTPRoute
apiVersion: gateway.networking.k8s.io/v1beta1

images:
- name: ghcr.io/kubeflow/kubeflow/kfam
newName: ghcr.io/kubeflow/kubeflow/kfam
newTag: latest
3 changes: 3 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/params.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
varReference:
- path: spec/http/route/destination/host
kind: VirtualService
47 changes: 47 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/patches/kfam.yaml
View file
Open in desktop
Copy link
Contributor

@orfeas-k orfeas-k Jul 25, 2025

Choose a reason for hiding this comment

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

I 'm not too familiar with overlays but I think we should be able split the kubeflow to two flavours in order to avoid manifests duplication and only change what's needed in sidecar and ambient.

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment
spec:
template:
metadata:
labels:
sidecar.istio.io/inject: "true"
spec:
securityContext:
seccompProfile:
type: RuntimeDefault
containers:
- command:
- /access-management
- "-cluster-admin"
- $(ADMIN)
- "-userid-header"
- $(USERID_HEADER)
- "-userid-prefix"
- $(USERID_PREFIX)
- "-service-mesh-mode"
- $(SERVICE_MESH_MODE)
envFrom:
- configMapRef:
name: config
image: ghcr.io/kubeflow/kubeflow/kfam
imagePullPolicy: IfNotPresent
name: kfam
livenessProbe:
httpGet:
path: /metrics
port: 8081
initialDelaySeconds: 30
periodSeconds: 30
ports:
- containerPort: 8081
name: kfam-http
protocol: TCP
securityContext:
allowPrivilegeEscalation: false
runAsNonRoot: true
capabilities:
drop:
- ALL
serviceAccountName: controller-service-account
18 changes: 18 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/patches/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment
spec:
template:
spec:
containers:
- name: manager
env:
- name: SERVICE_MESH_MODE
value: istio-ambient
- name: WAYPOINT_NAME
value: waypoint
- name: WAYPOINT_NAMESPACE
value: ""
- name: CREATE_WAYPOINT
value: "true"
Copy link
Contributor

@orfeas-k orfeas-k Aug 22, 2025

Choose a reason for hiding this comment

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

Here it defaults to true while here to false. LEt's keep the default to false everywhere.

Copy link
Contributor Author

@madmecodes madmecodes Aug 26, 2025

Choose a reason for hiding this comment

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

Changed ambient patch to consistently use "false"

Copy link
Contributor

@orfeas-k orfeas-k Aug 22, 2025

Choose a reason for hiding this comment

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

I don't think we need those here since we have the envFrom in the deployment and the same envvars in the configmap, right?

Copy link
Contributor Author

@madmecodes madmecodes Aug 26, 2025

Choose a reason for hiding this comment

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

Kept only SERVICE_MESH_MODE=istio-ambient override, removed redundant WAYPOINT_NAME, WAYPOINT_NAMESPACE, and CREATE_WAYPOINT

5 changes: 5 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/patches/remove-namespace.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$patch: delete
apiVersion: v1
kind: Namespace
metadata:
name: system
7 changes: 7 additions & 0 deletions components/profile-controller/config/overlays/kubeflow-ambient/service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v1
kind: Service
metadata:
name: profiles-kfam
spec:
ports:
- port: 8081
Loading
Loading