Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: respect RATTLER_AUTH_FILE when using AuthenticationStorage::default() #636

Merged
merged 8 commits into from
May 6, 2024
Merged

feat: respect RATTLER_AUTH_FILE when using AuthenticationStorage::default() #636

merged 8 commits into from
May 6, 2024

Conversation

0xbe7a
Copy link
Contributor

@0xbe7a 0xbe7a commented Apr 29, 2024

The environment variable RATTLER_AUTH_FILE is currently used by both pixi and rattler-build to specify an alternative storage location for the credentials.json file, which is crucial for authenticating to remote repositories. This feature is particularly valuable in CI or managed environments, where authentication is managed externally and credentials are stored at ephemeral paths.

Currently, both tools handle the RATTLER_AUTH_FILE independently, each tool implementing its own method to default to AuthenticationStorage when the environment variable is not set. This is evidenced in their respective codebases:

Despite their similar approaches, this independent handling has led to at least one consistency issue as seen in pixi issue #1240.

To address this and enhance consistency across tools that utilize RATTLER_AUTH_FILE, I propose integrating the handling of this environment variable directly into the default implementation (impl Default) of AuthenticationStorage. This change will ensure that any tool relying on AuthenticationStorage will inherently respect the RATTLER_AUTH_FILE path if set, without requiring each tool to implement its own handling. The proposed change aligns with the intended use of the environment variable, suggesting a default behavior that could be expected across all tools built on the rattler framework.

This modification will not remove flexibility, as developers can still explicitly construct AuthenticationStorage instances without this default behavior if needed.

@0xbe7a 0xbe7a changed the title respect RATTLER_AUTH_FILE when using AuthenticationStorage::default() respect RATTLER_AUTH_FILE when using AuthenticationStorage::default() Apr 29, 2024
@0xbe7a 0xbe7a changed the title respect RATTLER_AUTH_FILE when using AuthenticationStorage::default() feat: respect RATTLER_AUTH_FILE when using AuthenticationStorage::default() Apr 29, 2024
baszalmstra
baszalmstra requested changes May 6, 2024
View reviewed changes
Copy link
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for leaving this PR for so long! It completely missed my radar!

I am strongly opposed to reading environment variables in normal happy flow code. So I would prefer to move this implementation to a function that is explicit about reading this from the environment. I added a suggestion as a review comment.

crates/rattler_networking/src/authentication_storage/storage.rs Outdated Show resolved Hide resolved
crates/rattler_networking/src/authentication_middleware.rs Outdated Show resolved Hide resolved
@0xbe7a 0xbe7a requested a review from baszalmstra May 6, 2024 09:09
@baszalmstra
Copy link
Collaborator

Thanks!

@baszalmstra baszalmstra merged commit 84e6a63 into mamba-org:main May 6, 2024
14 checks passed
@baszalmstra baszalmstra mentioned this pull request May 6, 2024
Merged
@0xbe7a 0xbe7a deleted the rattler_auth_file branch May 6, 2024 11:58
baszalmstra added a commit that referenced this pull request May 6, 2024
@baszalmstra
chore: release (#638)
f2c4c19 
## 🤖 New release
* `rattler`: 0.23.2 -> 0.24.0 (⚠️ API breaking changes)
* `rattler_conda_types`: 0.22.0 -> 0.22.1 (✓ API compatible changes)
* `rattler_networking`: 0.20.4 -> 0.20.5 (✓ API compatible changes)
* `rattler_package_streaming`: 0.20.7 -> 0.20.8
* `rattler_shell`: 0.20.1 -> 0.20.2
* `rattler_lock`: 0.22.4 -> 0.22.5
* `rattler_repodata_gateway`: 0.19.10 -> 0.19.11
* `rattler_solve`: 0.21.0 -> 0.21.1
* `rattler_virtual_packages`: 0.19.8 -> 0.19.9
* `rattler_index`: 0.19.9 -> 0.19.10

### ⚠️ `rattler` breaking changes

```
--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.31.0/src/lints/enum_variant_added.ron

Failed in:
  variant LinkFileError:FailedToComputeSha in /tmp/.tmpB34zKy/rattler/crates/rattler/src/install/link.rs:108
  variant LinkFileError:FailedToComputeSha in /tmp/.tmpB34zKy/rattler/crates/rattler/src/install/link.rs:108
```

<details><summary><i><b>Changelog</b></i></summary><p>

## `rattler`
<blockquote>

##
[0.24.0](rattler-v0.23.2...rattler-v0.24.0)
- 2024-05-06

### Fixed
- use the output of `readlink` as hash for softlinks
([#643](#643))
- sha computation of symlinks was failing sometimes
([#641](#641))
</blockquote>

## `rattler_conda_types`
<blockquote>

##
[0.22.1](rattler_conda_types-v0.22.0...rattler_conda_types-v0.22.1)
- 2024-05-06

### Added
- expose `*Record.noarch` in Python bindings
([#635](#635))
</blockquote>

## `rattler_networking`
<blockquote>

##
[0.20.5](rattler_networking-v0.20.4...rattler_networking-v0.20.5)
- 2024-05-06

### Added
- respect `RATTLER_AUTH_FILE` when using
AuthenticationStorage::default()
([#636](#636))
</blockquote>

## `rattler_package_streaming`
<blockquote>

##
[0.20.8](rattler_package_streaming-v0.20.7...rattler_package_streaming-v0.20.8)
- 2024-05-06

### Other
- updated the following local packages: rattler_conda_types,
rattler_networking
</blockquote>

## `rattler_shell`
<blockquote>

##
[0.20.2](rattler_shell-v0.20.1...rattler_shell-v0.20.2)
- 2024-05-06

### Other
- updated the following local packages: rattler_conda_types
</blockquote>

## `rattler_lock`
<blockquote>

##
[0.22.5](rattler_lock-v0.22.4...rattler_lock-v0.22.5)
- 2024-05-06

### Other
- updated the following local packages: rattler_conda_types
</blockquote>

## `rattler_repodata_gateway`
<blockquote>

##
[0.19.11](rattler_repodata_gateway-v0.19.10...rattler_repodata_gateway-v0.19.11)
- 2024-05-06

### Other
- updated the following local packages: rattler_conda_types,
rattler_networking
</blockquote>

## `rattler_solve`
<blockquote>

##
[0.21.1](rattler_solve-v0.21.0...rattler_solve-v0.21.1)
- 2024-05-06

### Other
- updated the following local packages: rattler_conda_types
</blockquote>

## `rattler_virtual_packages`
<blockquote>

##
[0.19.9](rattler_virtual_packages-v0.19.8...rattler_virtual_packages-v0.19.9)
- 2024-05-06

### Other
- updated the following local packages: rattler_conda_types
</blockquote>

## `rattler_index`
<blockquote>

##
[0.19.10](rattler_index-v0.19.9...rattler_index-v0.19.10)
- 2024-05-06

### Other
- updated the following local packages: rattler_conda_types
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Signed-off-by: Bas <4995967+baszalmstra@users.noreply.github.com>
Co-authored-by: Bas <4995967+baszalmstra@users.noreply.github.com>
@0xbe7a 0xbe7a mentioned this pull request May 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baszalmstra baszalmstra baszalmstra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@0xbe7a @baszalmstra