-
Notifications
You must be signed in to change notification settings - Fork 53
feat(nvca): provision worker identity for container function pods #846
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 all commits
756bbad
ba70057
3e84fed
3341a40
e04caab
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,128 @@ | ||
| /* | ||
| SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package mscontroller | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| rbacv1 "k8s.io/api/rbac/v1" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| const ( | ||
| // miniserviceWorkerSAName is the ServiceAccount name for MiniService worker identity. | ||
| // Each MiniService gets its own namespace, so a fixed name is sufficient. | ||
| miniserviceWorkerSAName = "nvcf-worker" | ||
| // miniserviceWorkerTokenVolumeName is the projected SAT volume name injected into the utils pod. | ||
| miniserviceWorkerTokenVolumeName = "nvcf-worker-token" | ||
| // miniserviceWorkerTokenMountPath is where the projected SAT is mounted inside the container. | ||
| miniserviceWorkerTokenMountPath = "/var/run/secrets/tokens" | ||
| // miniserviceWorkerTokenFilePath is the full path to the projected token file. | ||
| miniserviceWorkerTokenFilePath = miniserviceWorkerTokenMountPath + "/token" | ||
| // miniserviceWorkerTokenFilePathEnvKey is the env var pointing at the token file. | ||
| miniserviceWorkerTokenFilePathEnvKey = "NVCF_TOKEN_FILE_PATH" | ||
| // miniserviceWorkerIdentitySourceEnvKey indicates the active identity mechanism. | ||
| miniserviceWorkerIdentitySourceEnvKey = "NVCF_IDENTITY_SOURCE" | ||
| // miniserviceWorkerIdentitySourcePSAT is the value written when PSAT is the mechanism. | ||
| miniserviceWorkerIdentitySourcePSAT = "psat" | ||
| ) | ||
|
|
||
| // miniserviceWorkerTokenExpirationSeconds is the requested SAT lifetime. | ||
| var miniserviceWorkerTokenExpirationSeconds int64 = 900 | ||
|
|
||
| // ensureWorkerIdentity creates the worker ServiceAccount, Role, and RoleBinding in namespace. | ||
| // It is idempotent: existing objects are not modified. | ||
| func ensureWorkerIdentity(ctx context.Context, c client.Client, namespace string) error { | ||
| sa := &corev1.ServiceAccount{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: miniserviceWorkerSAName, Namespace: namespace}, | ||
| } | ||
| if err := c.Create(ctx, sa); err != nil && !apierrors.IsAlreadyExists(err) { | ||
| return fmt.Errorf("create worker ServiceAccount %s/%s: %w", namespace, miniserviceWorkerSAName, err) | ||
| } | ||
|
|
||
| role := &rbacv1.Role{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: miniserviceWorkerSAName, Namespace: namespace}, | ||
| Rules: nil, | ||
| } | ||
| if err := c.Create(ctx, role); err != nil && !apierrors.IsAlreadyExists(err) { | ||
| return fmt.Errorf("create worker Role %s/%s: %w", namespace, miniserviceWorkerSAName, err) | ||
| } | ||
|
|
||
| rb := &rbacv1.RoleBinding{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: miniserviceWorkerSAName, Namespace: namespace}, | ||
| RoleRef: rbacv1.RoleRef{ | ||
| APIGroup: "rbac.authorization.k8s.io", | ||
| Kind: "Role", | ||
| Name: miniserviceWorkerSAName, | ||
| }, | ||
| Subjects: []rbacv1.Subject{ | ||
| {Kind: "ServiceAccount", Name: miniserviceWorkerSAName, Namespace: namespace}, | ||
| }, | ||
| } | ||
| if err := c.Create(ctx, rb); err != nil && !apierrors.IsAlreadyExists(err) { | ||
| return fmt.Errorf("create worker RoleBinding %s/%s: %w", namespace, miniserviceWorkerSAName, err) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // injectWorkerTokenVolume assigns the worker ServiceAccount to pod, adds the projected SAT | ||
| // volume, and injects worker identity env vars into all non-init containers. | ||
| // The token audience is "nvcf-icms:<clusterID>" with a 900-second expiry. | ||
| // The volume is mounted read-only at /var/run/secrets/tokens in all non-init containers. | ||
| func injectWorkerTokenVolume(pod *corev1.Pod, clusterID string) { | ||
| pod.Spec.ServiceAccountName = miniserviceWorkerSAName | ||
| audience := "nvcf-icms:" + clusterID | ||
|
|
||
| volume := corev1.Volume{ | ||
| Name: miniserviceWorkerTokenVolumeName, | ||
| VolumeSource: corev1.VolumeSource{ | ||
| Projected: &corev1.ProjectedVolumeSource{ | ||
| Sources: []corev1.VolumeProjection{ | ||
| { | ||
| ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ | ||
| Audience: audience, | ||
| ExpirationSeconds: &miniserviceWorkerTokenExpirationSeconds, | ||
| Path: "token", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| pod.Spec.Volumes = append(pod.Spec.Volumes, volume) | ||
|
|
||
| mount := corev1.VolumeMount{ | ||
| Name: miniserviceWorkerTokenVolumeName, | ||
| MountPath: miniserviceWorkerTokenMountPath, | ||
| ReadOnly: true, | ||
| } | ||
| envVars := []corev1.EnvVar{ | ||
| {Name: miniserviceWorkerTokenFilePathEnvKey, Value: miniserviceWorkerTokenFilePath}, | ||
| {Name: miniserviceWorkerIdentitySourceEnvKey, Value: miniserviceWorkerIdentitySourcePSAT}, | ||
| } | ||
|
|
||
| for i := range pod.Spec.Containers { | ||
| pod.Spec.Containers[i].VolumeMounts = append(pod.Spec.Containers[i].VolumeMounts, mount) | ||
| pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, envVars...) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,6 +143,10 @@ func (v *helmMiniServiceValWebhookHandler) validateUpdate(ctx context.Context, _ | |
| func (v *helmMiniServiceValWebhookHandler) validate(ctx context.Context, obj client.Object) (warnings admission.Warnings, err error) { | ||
| var errs []error | ||
|
|
||
| // REQ-220: workload pods must never run as a worker ServiceAccount regardless of | ||
| // AllowWorkloadKubernetesAPIAccess, to prevent Helm charts from forging worker tokens. | ||
| errs = append(errs, validateWorkerSARestriction(obj)...) | ||
|
|
||
| if shouldEnforceResourceLimits(v.fff, obj) { | ||
| warns, verrs := v.validateResourceLimits(ctx, obj) | ||
| warnings = append(warnings, warns...) | ||
|
|
@@ -152,6 +156,32 @@ func (v *helmMiniServiceValWebhookHandler) validate(ctx context.Context, obj cli | |
| return warnings, errors.Join(errs...) | ||
| } | ||
|
|
||
| // validateWorkerSARestriction rejects any pod-bearing resource that specifies a worker | ||
| // ServiceAccount (name prefix "nvcf-worker-") as its service account. | ||
| func validateWorkerSARestriction(obj client.Object) []error { | ||
| var ps *corev1.PodSpec | ||
| switch t := obj.(type) { | ||
| case *corev1.Pod: | ||
| ps = &t.Spec | ||
| case *appsv1.Deployment: | ||
| ps = &t.Spec.Template.Spec | ||
| case *appsv1.ReplicaSet: | ||
| ps = &t.Spec.Template.Spec | ||
| case *appsv1.StatefulSet: | ||
| ps = &t.Spec.Template.Spec | ||
| case *batchv1.Job: | ||
| ps = &t.Spec.Template.Spec | ||
| case *batchv1.CronJob: | ||
| ps = &t.Spec.JobTemplate.Spec.Template.Spec | ||
| default: | ||
| return nil | ||
|
Comment on lines
+163
to
+177
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. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Reject reserved ServiceAccounts for ReplicaSets.
As per coding guidelines, webhook code must “Validate all webhook inputs.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
| if strings.HasPrefix(ps.ServiceAccountName, "nvcf-worker-") { | ||
| return []error{fmt.Errorf("workload pods may not use worker ServiceAccounts (prefix \"nvcf-worker-\")")} | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (v *helmMiniServiceValWebhookHandler) validateResourceLimits(ctx context.Context, obj client.Object) ( | ||
| warnings admission.Warnings, | ||
| errs []error, | ||
|
|
||
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.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add regression tests for worker identity provisioning.
This new identity path has no accompanying tests. Cover resource creation and repeat reconciliation. Cover the injected ServiceAccount, audience, token expiry, read-only mount, and environment values.
As per coding guidelines, "Code changes must include tests."
🤖 Prompt for AI Agents
Source: Coding guidelines