-
Notifications
You must be signed in to change notification settings - Fork 66
feat(worker): prefer mounted projected ServiceAccount token for NVCF/NVCT auth #847
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
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 | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -64,13 +64,13 @@ func (c *Client) ConnectIndefinitely(ctx context.Context) (context.Context, erro | |||||||||||||||
| } | ||||||||||||||||
| utils.SleepWithContext(ctx, sleepDuration) | ||||||||||||||||
| } | ||||||||||||||||
| maxElapsed := time.Until(token.Expiry) | ||||||||||||||||
| if maxElapsed <= 0 { | ||||||||||||||||
| maxElapsed = 5 * time.Minute | ||||||||||||||||
| } | ||||||||||||||||
| err = backoff.Retry(func() error { | ||||||||||||||||
| return c.connect(ctx) | ||||||||||||||||
| }, backoff.WithContext(backoff.NewExponentialBackOff(backoff.WithMaxElapsedTime(maxElapsed)), ctx)) | ||||||||||||||||
| maxElapsed := time.Until(token.Expiry) | ||||||||||||||||
|
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Handle token-source errors before reading
Use the five-minute retry default when Proposed fix- maxElapsed := time.Until(token.Expiry)
- if maxElapsed <= 0 {
- maxElapsed = 5 * time.Minute
- }
+ maxElapsed := 5 * time.Minute
+ if err == nil && token != nil {
+ if expiresIn := time.Until(token.Expiry); expiresIn > 0 {
+ maxElapsed = expiresIn
+ }
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| if maxElapsed <= 0 { | ||||||||||||||||
| maxElapsed = 5 * time.Minute | ||||||||||||||||
| } | ||||||||||||||||
| err = backoff.Retry(func() error { | ||||||||||||||||
| return c.connect(ctx) | ||||||||||||||||
| }, backoff.WithContext(backoff.NewExponentialBackOff(backoff.WithMaxElapsedTime(maxElapsed)), ctx)) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| zap.L().Error("failed to reconnect to NVCF", zap.Error(err)) | ||||||||||||||||
| return | ||||||||||||||||
|
|
@@ -91,14 +91,20 @@ func (c *Client) connect(ctx context.Context) error { | |||||||||||||||
| InstanceId: c.instanceId, | ||||||||||||||||
| FunctionId: c.functionId, | ||||||||||||||||
| FunctionVersionId: c.functionVersionId, | ||||||||||||||||
| }, auth.GrpcTokenFromSource(c.NvcfTokenProvider)) | ||||||||||||||||
| }, auth.GrpcTokenFromSource(c.NvcfTokenProvider, c.delegatedToken)) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return fmt.Errorf("failed to send connect request to NVCF: %w", err) | ||||||||||||||||
| } | ||||||||||||||||
| if connected.ConnectedRegion == "" { | ||||||||||||||||
| return fmt.Errorf("nvcf did not respond with a connected region") | ||||||||||||||||
| } | ||||||||||||||||
| c.updateConnectedRegions(connected.ConnectedRegion, connected.OtherRegions) | ||||||||||||||||
| if c.delegatedToken { | ||||||||||||||||
| // The mounted JWT stays the credential for the life of the process; NVCF issues no | ||||||||||||||||
| // replacement token on this path and nothing is persisted. | ||||||||||||||||
| zap.L().Info("connected to NVCF", zap.String("region", connected.ConnectedRegion), zap.Strings("secondaryRegions", connected.OtherRegions)) | ||||||||||||||||
| return nil | ||||||||||||||||
| } | ||||||||||||||||
| oauthToken := &oauth2.Token{ | ||||||||||||||||
| AccessToken: connected.NvcfWorkerToken, | ||||||||||||||||
| Expiry: connected.Expiration.AsTime(), | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| /* | ||
| 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 nvcf | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/base64" | ||
| "os" | ||
| "path/filepath" | ||
| "strconv" | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "golang.org/x/oauth2" | ||
| "google.golang.org/grpc" | ||
| "google.golang.org/protobuf/types/known/timestamppb" | ||
|
|
||
| pb "github.com/NVIDIA/nvcf/src/libraries/go/worker/proto/nvcf" | ||
| "github.com/NVIDIA/nvcf/src/libraries/go/worker/token" | ||
| ) | ||
|
|
||
| func fakePSAT(exp int64) string { | ||
| header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`)) | ||
| claims := base64.RawURLEncoding.EncodeToString([]byte( | ||
| `{"sub":"system:serviceaccount:inst-1:nvcf-worker","aud":["nvcf-icms:cl-1"],"exp":` + | ||
| strconv.FormatInt(exp, 10) + `}`)) | ||
| return header + "." + claims + ".fakesig" | ||
| } | ||
|
|
||
| // mountPSAT writes a fake PSAT under a temporary allowed root and points the env var at it. | ||
| func mountPSAT(t *testing.T) string { | ||
| t.Helper() | ||
| root, err := filepath.EvalSymlinks(t.TempDir()) | ||
| require.NoError(t, err) | ||
| old := token.MountedTokenRoot | ||
| token.MountedTokenRoot = root + "/" | ||
| t.Cleanup(func() { token.MountedTokenRoot = old }) | ||
| path := filepath.Join(root, "token") | ||
| jwt := fakePSAT(time.Now().Add(15 * time.Minute).Unix()) | ||
| require.NoError(t, os.WriteFile(path, []byte(jwt), 0600)) | ||
| t.Setenv(token.MountedTokenPathEnvKey, path) | ||
| return jwt | ||
| } | ||
|
|
||
| func TestCreateClient_NoMountedJWT_UsesBootstrap(t *testing.T) { | ||
| t.Setenv(token.MountedTokenPathEnvKey, filepath.Join(t.TempDir(), "absent")) | ||
| fqdn := startMockServer(t, &mockWorkerServer{}) | ||
|
|
||
| client, err := CreateClient(fqdn, nil, "bootstrap-token", nil, "nca", "inst", "fn", "fnv", t.TempDir(), DefaultNvcfClientTimeout) | ||
| require.NoError(t, err) | ||
| assert.False(t, client.delegatedToken) | ||
| tok, err := client.NvcfTokenProvider.Token() | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "bootstrap-token", tok.AccessToken) | ||
| } | ||
|
|
||
| func TestCreateClient_MountedJWT_RequiresHTTPS(t *testing.T) { | ||
| mountPSAT(t) | ||
| fqdn := startMockServer(t, &mockWorkerServer{}) // http:// | ||
|
|
||
| _, err := CreateClient(fqdn, nil, "bootstrap-token", nil, "nca", "inst", "fn", "fnv", t.TempDir(), DefaultNvcfClientTimeout) | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), "requires TLS") | ||
| } | ||
|
|
||
| func TestCreateClient_MountedJWT_PreferredOverBootstrapAndCache(t *testing.T) { | ||
| jwt := mountPSAT(t) | ||
| sharedDir := t.TempDir() | ||
| require.NoError(t, token.CacheToken(filepath.Join(sharedDir, cachedNvcfTokenFilename), | ||
| &oauth2.Token{AccessToken: "cached-token", Expiry: time.Now().Add(time.Hour)})) | ||
|
|
||
| client, err := CreateClient("https://127.0.0.1:1", nil, "bootstrap-token", nil, "nca", "inst", "fn", "fnv", sharedDir, DefaultNvcfClientTimeout) | ||
| require.NoError(t, err) | ||
| assert.True(t, client.delegatedToken) | ||
| tok, err := client.NvcfTokenProvider.Token() | ||
| require.NoError(t, err) | ||
| assert.Equal(t, jwt, tok.AccessToken, "mounted JWT wins over cached and bootstrap tokens") | ||
| } | ||
|
|
||
| func TestCreateClient_MountedJWT_UnreadableIsAnError(t *testing.T) { | ||
| if os.Geteuid() == 0 { | ||
| t.Skip("root can read mode 0000 files") | ||
| } | ||
| mountPSAT(t) | ||
| require.NoError(t, os.Chmod(os.Getenv(token.MountedTokenPathEnvKey), 0000)) | ||
|
|
||
| _, err := CreateClient("https://127.0.0.1:1", nil, "bootstrap-token", nil, "nca", "inst", "fn", "fnv", t.TempDir(), DefaultNvcfClientTimeout) | ||
| require.Error(t, err) | ||
| assert.False(t, strings.Contains(err.Error(), "no mounted JWT"), "read failures must not be treated as no token mounted") | ||
| } | ||
|
|
||
| // delegatedConnectClient answers ConnectOnce in-process. A real plaintext gRPC connection | ||
| // would reject the per-RPC credentials because a mounted JWT requires TLS. | ||
| type delegatedConnectClient struct { | ||
| pb.WorkerClient | ||
| } | ||
|
|
||
| func (delegatedConnectClient) ConnectOnce(context.Context, *pb.WorkerConnect, ...grpc.CallOption) (*pb.WorkerConnectOnceResponse, error) { | ||
| return &pb.WorkerConnectOnceResponse{ | ||
| ConnectedRegion: "us-east-1", | ||
| NvcfWorkerToken: "", // NVCF issues no replacement token on the delegated path | ||
| Expiration: timestamppb.New(time.Now().Add(15 * time.Minute)), | ||
| }, nil | ||
| } | ||
|
|
||
| func TestConnect_DelegatedToken_KeepsPSATAndPersistsNothing(t *testing.T) { | ||
| fqdn := startMockServer(t, &mockWorkerServer{}) | ||
| c := newTestClient(t, fqdn) | ||
| c.Client = delegatedConnectClient{} | ||
| c.delegatedToken = true | ||
| before, err := c.NvcfTokenProvider.Token() | ||
| require.NoError(t, err) | ||
|
|
||
| require.NoError(t, c.connect(context.Background())) | ||
|
|
||
| after, err := c.NvcfTokenProvider.Token() | ||
| require.NoError(t, err) | ||
| assert.Equal(t, before.AccessToken, after.AccessToken, "PSAT source must remain installed") | ||
| _, statErr := os.Stat(filepath.Join(c.sharedConfigDir, cachedNvcfTokenFilename)) | ||
| assert.True(t, os.IsNotExist(statErr), "no token may be persisted on the delegated path") | ||
| regions := c.ConnectedRegions.Load() | ||
| require.NotNil(t, regions) | ||
| assert.Equal(t, "us-east-1", regions.Primary) | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.