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.

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

[mamba-content-trust] Add integration test #3234

Merged
merged 3 commits into from
Mar 20, 2024

Conversation

Hind-M
Copy link
Member

@Hind-M Hind-M commented Mar 14, 2024

  • Add integration test for mamba-content-trust (mamba client side) using conda-content-trust (server side)
  • Fix trusted_channels in context not working with more than one item
  • Remove repo_signed (generated when running the script)

@Hind-M Hind-M added the release::enhancements For enhancements PRs or implementing features label Mar 14, 2024
-Add test for mamba-content-trust (mamba client side) using conda-content-trust (server side)
-Add missing install with pip (to fix)
@Hind-M Hind-M marked this pull request as ready for review March 18, 2024 12:05
@Hind-M Hind-M requested a review from AntoinePrv March 18, 2024 14:20
Copy link
Member

@JohanMabille JohanMabille left a comment

Choose a reason for hiding this comment

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

sorry for the late review!

@@ -32,11 +32,11 @@ namespace mamba

explicit RepoCheckerStore(repo_checker_list checkers);

[[nodiscard]] auto find_checker(const Channel& chan) const -> const RepoChecker*;
Copy link
Member

Choose a reason for hiding this comment

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

Why removing the const here? It looks legit. Same question for methods below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it was legit until I realized that the trusted_channels in Context don't handle more that one item (giving errors if the first one is not the correct/used one). And that's because generate_index_checker() is fetching the root and delegation roles files and was called here (when populating the RepoCheckerStore and before knowing if the corresponding channel will be used) but is now moved to here. And since it's not const the RepoChecker and these methods can't be anymore.

Copy link
Member

Choose a reason for hiding this comment

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

🥲

@JohanMabille JohanMabille merged commit cad6793 into mamba-org:main Mar 20, 2024
32 checks passed
@Hind-M Hind-M deleted the cct_sign branch March 20, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release::enhancements For enhancements PRs or implementing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants