fix: support default credentials for workflow storage#4
Closed
yoosful wants to merge 1 commit into
Closed
Conversation
3 tasks
Owner
Author
|
Closing as duplicate. NVIDIA#751 was ported and merged upstream as NVIDIA#865, so NVIDIA#749 is already materially handled on upstream main. This PR was opened after checking only open PRs, which missed the closed/ported PR history. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Upstream issue: NVIDIA#749
This change completes the workflow storage
DefaultDataCredentialpath so workflow log/data storage can rely on ambient cloud credentials, including AWS EKS IRSA or Pod Identity, instead of requiring long-lived static access keys.The Python workflow config models already accept the
DataCredentialunion on currentmain; this PR aligns the remaining typed call sites and fixes the Go runtime mount path:DataCredentialinstead of static-only credentialsmount-s3subprocess instead of mutating the osmo-ctrl process environmentDefaultDataCredential/ no explicit credential pathsAWS_SESSION_TOKENin that static-credential caseWhy
Before this fix, a workflow storage config like:
could deserialize as a default credential, but the runtime mount path would unset/overwrite AWS credential environment variables before invoking
mount-s3. That breaks default credential chain flows where the pod should use ambient identity from the Kubernetes service account.Validation
bazel test //src/utils/connectors/tests:test_cli_configbazel test //src/utils/job/tests:test_taskbazel test //src/service/core/config/tests:test_configmap_loader_unitbazel test //src/runtime/pkg/data:data_testbazel test --runs_per_test=3 --cache_test_results=no //src/runtime/pkg/data:data_testChecklist