-
Notifications
You must be signed in to change notification settings - Fork 31
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.
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
2ecf65c
b333928
7c8ab38
a8b460b
2f50157
c5a9989
063d073
29274e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I tried to apply this I got
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
i tried, some issue in injecting variable values, if i harcode them like this they work and are installed, this is how i am installing:
Finally: 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 |
| 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| varReference: | ||
| - path: spec/http/route/destination/host | ||
| kind: VirtualService |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I 'm not too familiar with |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| 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) | ||
kimwnasptd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: deployment | ||
| spec: | ||
| template: | ||
| spec: | ||
| containers: | ||
| - name: manager | ||
| env: | ||
| - name: SERVICE_MESH_MODE | ||
| value: ambient | ||
| - name: GATEWAY_NAME | ||
| value: kubeflow-gateway | ||
| - name: GATEWAY_NAMESPACE | ||
|
||
| value: istio-system | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| $patch: delete | ||
| apiVersion: v1 | ||
| kind: Namespace | ||
| metadata: | ||
| name: system |
| 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,9 @@ type ProfileReconciler struct { | |
| UserIdPrefix string | ||
| WorkloadIdentity string | ||
| DefaultNamespaceLabelsPath string | ||
| ServiceMeshMode string | ||
| GatewayName string | ||
| GatewayNamespace string | ||
| } | ||
|
|
||
| // +kubebuilder:rbac:groups=core,resources=namespaces,verbs="*" | ||
|
|
@@ -127,13 +130,20 @@ func (r *ProfileReconciler) Reconcile(ctx context.Context, request ctrl.Request) | |
| ns := &corev1.Namespace{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Annotations: map[string]string{"owner": instance.Spec.Owner.Name}, | ||
| // inject istio sidecar to all pods in target namespace by default. | ||
| Labels: map[string]string{ | ||
| istioInjectionLabel: "enabled", | ||
| }, | ||
| Name: instance.Name, | ||
| Labels: map[string]string{}, | ||
| Name: instance.Name, | ||
| }, | ||
| } | ||
|
|
||
| // Set istio-injection label based on service mesh mode | ||
| if r.ServiceMeshMode == "ambient" { | ||
| // In ambient mode, disable sidecar injection but enable ambient mesh | ||
|
||
| ns.Labels[istioInjectionLabel] = "disabled" | ||
| ns.Labels["istio.io/dataplane-mode"] = "ambient" | ||
| } else { | ||
| // In sidecar mode (default), inject istio sidecar to all pods in target namespace | ||
| ns.Labels[istioInjectionLabel] = "enabled" | ||
| } | ||
kimwnasptd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| setNamespaceLabels(ns, defaultKubeflowNamespaceLabels) | ||
| logger.Info("List of labels to be added to namespace", "labels", ns.Labels) | ||
| if err := controllerutil.SetControllerReference(instance, ns, r.Scheme); err != nil { | ||
|
|
@@ -178,8 +188,21 @@ func (r *ProfileReconciler) Reconcile(ctx context.Context, request ctrl.Request) | |
| for k, v := range foundNs.Labels { | ||
| oldLabels[k] = v | ||
| } | ||
|
|
||
| // Apply service mesh mode labels to existing namespace | ||
| if r.ServiceMeshMode == "ambient" { | ||
| // In ambient mode, disable sidecar injection but enable ambient mesh | ||
| foundNs.Labels[istioInjectionLabel] = "disabled" | ||
| foundNs.Labels["istio.io/dataplane-mode"] = "ambient" | ||
| } else { | ||
| // In sidecar mode (default), inject istio sidecar to all pods in target namespace | ||
| foundNs.Labels[istioInjectionLabel] = "enabled" | ||
| // Remove ambient mode label if it exists | ||
| delete(foundNs.Labels, "istio.io/dataplane-mode") | ||
| } | ||
|
|
||
| setNamespaceLabels(foundNs, defaultKubeflowNamespaceLabels) | ||
| logger.Info("List of labels to be added to found namespace", "labels", ns.Labels) | ||
| logger.Info("List of labels to be added to found namespace", "labels", foundNs.Labels) | ||
| if !reflect.DeepEqual(oldLabels, foundNs.Labels) { | ||
| err = r.Update(ctx, foundNs) | ||
| if err != nil { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the code, the
gatewayandgateway-namespaceare not used at all. The HTTPRoute is applied during deployment (instead of a VirtualService) and not by the profile-controller binary AFAIK.The AuthorizationPolicy that the profile-controller already creates is what needs to be changed to secure profile namespaces in Istio ambient mode. To achieve this, it needs to be attached to a waypoint, since this is an L7 AuthorizationPolicy (see this example).
This is done using a targetRef instead of a selector. The targetRef in that case is indeed of
kind: Gatewaybut the gateway has a gateway-class: waypoint. Thus, using thewaypointnaming in the code is more clear, since it actually targets a different resource than the HTTPRoute.