-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(libstore): add AWS SSO support for S3 authentication #14645
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
Open
lovesegfault
wants to merge
7
commits into
NixOS:master
Choose a base branch
from
lovesegfault:s3-sts
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+242
−19
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b5a4dd0
libstore: add AWS SSO support for S3 authentication
Mic92 ba005d9
refactor(libstore/aws-creds): improve error handling and logging
lovesegfault 7ff1cd5
fix(libstore/aws-creds): add STS support for default profile
lovesegfault b487580
chore(libstore/aws-creds): remove unused includes
lovesegfault 6e08faf
test(s3-binary-cache-store): add profile support for setup_for_s3
lovesegfault 1e627bc
test(s3-binary-cache-store): clear credential cache between tests
lovesegfault 294d846
test(s3-binary-cache-store): test profiles and provider chain
lovesegfault File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,7 +147,7 @@ in | |
| else: | ||
| machine.fail(f"nix path-info {pkg}") | ||
|
|
||
| def setup_s3(populate_bucket=[], public=False, versioned=False): | ||
| def setup_s3(populate_bucket=[], public=False, versioned=False, profiles=None): | ||
|
Member
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. BTW. It would make sense to start adding types here at some point. mypy is enabled in NixOS tests. |
||
| """ | ||
| Decorator that creates/destroys a unique bucket for each test. | ||
| Optionally pre-populates bucket with specified packages. | ||
|
|
@@ -157,23 +157,48 @@ in | |
| populate_bucket: List of packages to upload before test runs | ||
| public: If True, make the bucket publicly accessible | ||
| versioned: If True, enable versioning on the bucket before populating | ||
| profiles: Dict of AWS profiles to create, e.g.: | ||
| {"valid": {"access_key": "...", "secret_key": "..."}, | ||
| "invalid": {"access_key": "WRONG", "secret_key": "WRONG"}} | ||
| Profiles are created on the client machine at /root/.aws/credentials | ||
| """ | ||
| def decorator(test_func): | ||
| def wrapper(): | ||
| # Restart nix-daemon on both machines to clear the credential provider cache. | ||
| # The AwsCredentialProviderImpl singleton persists in the daemon process, | ||
| # and its cache can cause credentials from previous tests to be reused. | ||
| # We reset-failed first to avoid systemd's start rate limiting. | ||
| server.succeed("systemctl reset-failed nix-daemon.service nix-daemon.socket") | ||
| server.succeed("systemctl restart nix-daemon") | ||
| client.succeed("systemctl reset-failed nix-daemon.service nix-daemon.socket") | ||
| client.succeed("systemctl restart nix-daemon") | ||
|
|
||
| bucket = str(uuid.uuid4()) | ||
| server.succeed(f"mc mb minio/{bucket}") | ||
| try: | ||
| if public: | ||
| server.succeed(f"mc anonymous set download minio/{bucket}") | ||
| if versioned: | ||
| server.succeed(f"mc version enable minio/{bucket}") | ||
| if profiles: | ||
| # Build credentials file content | ||
| creds_content = "" | ||
| for name, creds in profiles.items(): | ||
| creds_content += f"[{name}]\n" | ||
| creds_content += f"aws_access_key_id = {creds['access_key']}\n" | ||
| creds_content += f"aws_secret_access_key = {creds['secret_key']}\n\n" | ||
| client.succeed("mkdir -p /root/.aws") | ||
| client.succeed(f"cat > /root/.aws/credentials << 'AWSCREDS'\n{creds_content}AWSCREDS") | ||
| if populate_bucket: | ||
| store_url = make_s3_url(bucket) | ||
| for pkg in populate_bucket: | ||
| server.succeed(f"{ENV_WITH_CREDS} nix copy --to '{store_url}' {pkg}") | ||
| test_func(bucket) | ||
| finally: | ||
| server.succeed(f"mc rb --force minio/{bucket}") | ||
| # Clean up AWS profiles if created | ||
| if profiles: | ||
| client.succeed("rm -rf /root/.aws") | ||
| # Clean up client store - only delete if path exists | ||
| for pkg in PKGS.values(): | ||
| client.succeed(f"[ ! -e {pkg} ] || nix store delete --ignore-liveness {pkg}") | ||
|
|
@@ -764,6 +789,108 @@ in | |
|
|
||
| print(" ✓ Compressed log uploaded with multipart") | ||
|
|
||
| @setup_s3( | ||
| populate_bucket=[PKGS['A']], | ||
| profiles={ | ||
| "valid": {"access_key": ACCESS_KEY, "secret_key": SECRET_KEY}, | ||
| "invalid": {"access_key": "INVALIDKEY", "secret_key": "INVALIDSECRET"}, | ||
| } | ||
| ) | ||
| def test_profile_credentials(bucket): | ||
| """Test that profile-based credentials work without environment variables""" | ||
| print("\n=== Testing Profile-Based Credentials ===") | ||
|
|
||
| store_url = make_s3_url(bucket, profile="valid") | ||
|
|
||
| # Verify store info works with profile credentials (no env vars) | ||
| client.succeed(f"HOME=/root nix store info --store '{store_url}' >&2") | ||
| print(" ✓ nix store info works with profile credentials") | ||
|
|
||
| # Verify we can copy from the store using profile | ||
| verify_packages_in_store(client, PKGS['A'], should_exist=False) | ||
| client.succeed(f"HOME=/root nix copy --no-check-sigs --from '{store_url}' {PKGS['A']}") | ||
| verify_packages_in_store(client, PKGS['A']) | ||
| print(" ✓ nix copy works with profile credentials") | ||
|
|
||
| # Clean up the package we just copied so we can test invalid profile | ||
| client.succeed(f"nix store delete --ignore-liveness {PKGS['A']}") | ||
| verify_packages_in_store(client, PKGS['A'], should_exist=False) | ||
|
|
||
| # Verify invalid profile fails when trying to copy | ||
| invalid_url = make_s3_url(bucket, profile="invalid") | ||
| client.fail(f"HOME=/root nix copy --no-check-sigs --from '{invalid_url}' {PKGS['A']} 2>&1") | ||
| print(" ✓ Invalid profile credentials correctly rejected") | ||
|
|
||
| @setup_s3( | ||
| populate_bucket=[PKGS['A']], | ||
| profiles={ | ||
| "wrong": {"access_key": "WRONGKEY", "secret_key": "WRONGSECRET"}, | ||
| } | ||
| ) | ||
| def test_env_vars_precedence(bucket): | ||
| """Test that environment variables take precedence over profile credentials""" | ||
| print("\n=== Testing Environment Variables Precedence ===") | ||
|
|
||
| # Use profile with wrong credentials, but provide correct creds via env vars | ||
| store_url = make_s3_url(bucket, profile="wrong") | ||
|
|
||
| # Ensure package is not in client store | ||
| verify_packages_in_store(client, PKGS['A'], should_exist=False) | ||
|
|
||
| # This should succeed because env vars (correct) override profile (wrong) | ||
| output = client.succeed( | ||
| f"HOME=/root {ENV_WITH_CREDS} nix copy --no-check-sigs --debug --from '{store_url}' {PKGS['A']} 2>&1" | ||
| ) | ||
| print(" ✓ nix copy succeeded with env vars overriding wrong profile") | ||
|
|
||
| # Verify the credential chain shows Environment provider was added | ||
| if "Added AWS Environment Credential Provider" not in output: | ||
| print("Debug output:") | ||
| print(output) | ||
| raise Exception("Expected Environment provider to be added to chain") | ||
| print(" ✓ Environment provider added to credential chain") | ||
|
|
||
| # Clean up the package so we can test again without env vars | ||
| client.succeed(f"nix store delete --ignore-liveness {PKGS['A']}") | ||
| verify_packages_in_store(client, PKGS['A'], should_exist=False) | ||
|
|
||
| # Without env vars, same URL should fail (proving profile creds are actually wrong) | ||
| client.fail(f"HOME=/root nix copy --no-check-sigs --from '{store_url}' {PKGS['A']} 2>&1") | ||
| print(" ✓ Without env vars, wrong profile credentials correctly fail") | ||
|
|
||
| @setup_s3( | ||
| populate_bucket=[PKGS['A']], | ||
| profiles={ | ||
| "testprofile": {"access_key": ACCESS_KEY, "secret_key": SECRET_KEY}, | ||
| } | ||
| ) | ||
| def test_credential_provider_chain(bucket): | ||
| """Test that debug logging shows which providers are added to the chain""" | ||
| print("\n=== Testing Credential Provider Chain Logging ===") | ||
|
|
||
| store_url = make_s3_url(bucket, profile="testprofile") | ||
|
|
||
| output = client.succeed( | ||
| f"HOME=/root nix store info --debug --store '{store_url}' 2>&1" | ||
| ) | ||
|
|
||
| # For a named profile, we expect to see these providers in the chain | ||
| expected_providers = ["Environment", "Profile", "IMDS"] | ||
| for provider in expected_providers: | ||
| msg = f"Added AWS {provider} Credential Provider to chain for profile 'testprofile'" | ||
| if msg not in output: | ||
| print("Debug output:") | ||
| print(output) | ||
| raise Exception(f"Expected to find: {msg}") | ||
| print(f" ✓ {provider} provider added to chain") | ||
|
|
||
| # SSO should be skipped (no SSO config for this profile) | ||
| if "Skipped AWS SSO Credential Provider for profile 'testprofile'" not in output: | ||
| print("Debug output:") | ||
| print(output) | ||
| raise Exception("Expected SSO provider to be skipped") | ||
| print(" ✓ SSO provider correctly skipped (not configured)") | ||
|
|
||
| # ============================================================================ | ||
| # Main Test Execution | ||
| # ============================================================================ | ||
|
|
@@ -797,6 +924,9 @@ in | |
| test_multipart_upload_basic() | ||
| test_multipart_threshold() | ||
| test_multipart_with_log_compression() | ||
| test_profile_credentials() | ||
| test_env_vars_precedence() | ||
| test_credential_provider_chain() | ||
|
|
||
| print("\n" + "="*80) | ||
| print("✓ All S3 Binary Cache Store Tests Passed!") | ||
|
|
||
Oops, something went wrong.
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.
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.
Shouldn't this be a member of the class? I'm not sure about the lifetime requirements of the sdk for the allocator.