Add support for Cargo private registries #8719
Conversation
|
Currently reaching out to @cloudsmith-io. It appears that we are stymied with the Cargo download endpoint - At present this would unbreak Dependabot failing to understand the private registry, but will not allow for updates for those registries (because it cannot resolve the latest version). I think this is still useful for a partial fix, as right now it is completely inoperable for those of us with a single dependency linked to a private registry, but obviously not a complete resolution. |
CodingAnarchy
force-pushed
the
cargo-private-registries
branch
from
8d4ae73
to
8f9a93c
Compare
CodingAnarchy
force-pushed
the
cargo-private-registries
branch
from
1f9abba
to
3041c90
Compare
|
Hey @CodingAnarchy 👋 Bart from Cloudsmith here, I have responded to your support query from our new portal for which you should have received an email. We apologise for the inconvenience and looking to help you out asap! |
|
@BartoszBlizniak Thanks a ton! I think that response gave me the information I needed to enable this to resolve the latest version of the private registry. 👍 |
…ependabot-core into cargo-private-registries
|
@jeffwidman I saw you had commented on the previous PR I branched this from offering to answer questions and provide support. Is there anything that I can do to accelerate the review process here? This is blocking some teams at Shopify and we would love to get it sorted. Please let me know if there is anything I need to update to improve this PR so it can be accepted. |
|
@CodingAnarchy hey, I'm the PM for Dependabot here at GitHub. We're excited about this contribution from you! There are some steps we need to take on our internal repos for it to work once it's merged which we can't commit to doing right now 😓 HOWEVER, we will be able to do this next quarter (April-ish) and would like to revisit merging this at that time. Would that work for you? I apologize for our internal processes slowing this down! |
CodingAnarchy
force-pushed
the
cargo-private-registries
branch
from
3727753
to
61f72ae
Compare
|
@CodingAnarchy I've opened a PR to at least start to address the issue where defining a private registry breaks dependabot: #9109 I'm not sure if that will fully unblock you, but I think it will at least get past the initial cargo error where it can't find the definition of the private registry. |
…_GLOBAL_CREDENTIAL_PROVIDER=cargo:token`.
|
Hello @CodingAnarchy and @pavera, I work for Microsoft on internal Rust support, and I have been testing this PR against our internal private registries. I forked it and created my own branch based on this one, and merged it up to date with dependabot-core main. After some bug fixes, I'm happy to say it's mostly working in my current testing, including using My question for @CodingAnarchy is: are you still driving this PR? I see from your GitHub activity that you've been back online for all of April, but I haven't seen any activity on this branch. I don't want to create a new PR unless you are stepping back from this one, since you did do most of the work and my fixes are relatively small. Please advise? If I don't hear back from you within a week (by May 7th), I will create a new PR. For reference, my fork and branch are here: https://github.com/RobJellinghaus/dependabot-core/tree/user/rjelling/cargo-private-registries |
|
(also tagging @jakecoffman, @abdulapopoola, and @brbayes-msft as FYI) |
|
Hey @RobJellinghaus! I just got reminded of this the other day, and was about to resolve this against the changes in main. Instead, I've just merged up to your forked branch, which made it quite easy to resolve. Thanks for taking care of those fixes! I was able to replicate the results of your testing locally as well, so my understanding now is that the @pavera and the rest of the Dependabot team should be good to merge this. @pavera @mctofu If there is anything else that I can do to enable this support, please let me know! |
…ependabot-core into cargo-private-registries
|
@CodingAnarchy I am going to deploy this PR. There is still some work left from our side on the API side, which I have started already. Thank you everyone who has worked on this PR. |
|
@RobJellinghaus Could you share the I am using the below job.yml file with the Dependabot-cli job:
allowed-updates:
- dependency-type: direct
update-type: all
commit-message-options:
prefix:
prefix-development:
include-scope:
credentials-metadata:
- type: cargo_registry
url: https://cargo.cloudsmith.io/honeyankit/test/
- type: git_source
host: github.com
debug:
dependencies:
dependency-groups: []
dependency-group-to-refresh:
existing-pull-requests: []
existing-group-pull-requests: []
experiments:
record-ecosystem-versions: true
record-update-job-unknown-error: true
proxy-cached: true
globs: true
ignore-conditions: []
lockfile-only: false
max-updater-run-time: 2700
package-manager: cargo
proxy-log-response-body-on-auth-failure: true
requirements-update-strategy:
reject-external-code: false
security-advisories: []
security-updates-only: false
source:
provider: github
repo: dsp-testing/cargo-private-registry-test
branch:
directory: "/."
api-endpoint: https://api.github.com/
hostname: github.com
commit: 722e7fc32b20504059494040
updating-a-pull-request: false
update-subdependencies: false
vendor-dependencies: false
repo-private: true
credentials:
- type: cargo_registry
url: "https://cargo.cloudsmith.io/honeyankit/test/"
token: "Bearer <token>"Note: I have also tried the below for still getting the same error: credentials:
- type: cargo_registry
- registry: honeyankit-test
url: "https://cargo.cloudsmith.io/honeyankit/test/"
token: "Token <token>" |
|
Hello @honeyankit, let me guess, the parse error you are getting is similar to this one? I am sorry that I did not alert you to this problem sooner, I was actually aware of this a couple of weeks ago. But because the root issue involves some more work in dependabot-core, I wanted to sync with the GitHub Dependabot team before bringing this up. However, they were in an offsite last week and the next sync opportunity is early next week, so I was letting this slide until then. That was a bad call and I apologize. The parse error is because dependabot-core is trying to fetch the latest version of a package from a private registry, and the response is actually not JSON. The Cargo registry protocol is specified to return a sequence of JSON objects (e.g. the Cargo index file for a crate is a text file with one JSON object per line): https://doc.rust-lang.org/cargo/reference/registry-index.html#index-files But dependabot-core does not know this, and thinks the whole response is invalid. The root problem here is that it turns out dependabot-core has always done the wrong thing when it comes to looking up the latest version of a package. Currently dependabot-core actually hits the public crates.io web site -- not the crates.io sparse registry! -- to determine the latest version of a public crate. This is wrong, for three reasons:
(All of this is almost certainly because the dependabot-core implementation predates the sparse registry work, so the current index file format didn't exist when the dependabot-core code was originally written.) This all means that we actually need a new issue, "Dependabot-core Cargo implementation must support Cargo sparse index file format" -- this was not on your radar when you implemented this, and as I said I did not do a good job of promptly communicating the issue two weeks ago. I will be syncing with the GitHub team early next week and will make the issue immediately thereafter, if you don't beat me to it. Unfortunately I am personally heads down in urgent internal security work for the time being (duration TBD), otherwise I would be digging in here already. For total clarity, this does indeed mean that I was wrong to have reported that the PR as a whole worked properly against private registries. It does work up to the point of authentication. But when it comes to actually retrieving private registry crate version information, it stops working, so end-to-end it is in fact still broken. I should not have given a different impression, and I apologize for that as well. The job.yml file is not the problem here, but for completeness, mine looked like this: |
|
@RobJellinghaus : No worries. Gald we catched it earlier. My error is coming pretty early in the paring the cargo file_parser. I will have to look into what is the issue. Based on your job.yml file I changed mine to include as cloudsmith provide Basic auth but same issue. credentials:
- type: cargo_registry
- registry: honeyankit-test
url: "https://cargo.cloudsmith.io/honeyankit/test/"
token: "Basic <token>"024/05/17 21:28:00 INFO Starting job processing
2024/05/17 21:28:00 ERROR undefined method `name' for nil
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:295:in `parsed_file'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:237:in `cargo_config_from_file'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:228:in `cargo_config_field'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:176:in `registry_source_details'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:169:in `source_from_declaration'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:271:in `git_req?'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:258:in `block in version_from_lockfile'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:257:in `select'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:257:in `version_from_lockfile'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:72:in `block (3 levels) in manifest_dependencies'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:70:in `each'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:70:in `block (2 levels) in manifest_dependencies'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:69:in `each'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:69:in `block in manifest_dependencies'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:68:in `each'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:68:in `manifest_dependencies'
2024/05/17 21:28:00 ERROR /home/dependabot/cargo/lib/dependabot/cargo/file_parser.rb:30:in `parse'
2024/05/17 21:28:00 ERROR /home/dependabot/dependabot-updater/lib/dependabot/dependency_snapshot.rb:197:in `parse_files!'
20Created the issue: #9764 |
|
@RobJellinghaus: Could you also share the cargo.toml and cargo.lock file if you have it? I want to have look to both them. |
|
Also tagging @CodingAnarchy as my apologies also should go to him, since he landed the PR partly based on my claim that it worked. Which it did, as far as it went... but this problem was exposed. So completing this was valid, but more work is still needed. |
I'm sorry, that I can't do -- this is not open source, it's a Microsoft repository not a public GitHub one. If you have a Microsoft / Azure DevOps account, you can fetch it directly, please email me (at my Microsoft account, rjelling) for the details there. |
|
@RobJellinghaus: My issue is that it is looking to get the def cargo_config
@cargo_config ||= get_original_file(".cargo/config.toml")
endEdited: After adding the Calling https://cargo.cloudsmith.io/honeyankit/test/he/ll/hello-world to fetch metadata for hello-world from sparse+https://cargo.cloudsmith.io/honeyankit/test/
2024/05/17 22:26:35 ERROR Error processing hello-world (JSON::ParserError)
2024/05/17 22:26:35 ERROR unexpected token at '{"name": "hello-world", "vers": "1.0.1", "deps": [], "cksum": "8a55b58def1ecc7aa8590c7078f379ec9a85328363ffb81d4354314b132b95c4", "features": {}, "yanked": false, "links": null}' |
|
I think I have fixed it with my initial changes: Fetched metadata for hello-world from sparse+https://cargo.cloudsmith.io/honeyankit/test/ successfully
{"name": "hello-world", "vers": "1.0.0", "deps": [], "cksum": "b2c263921f1114820f4acc6b542d72bbc859ce7023c5b235346b157074dcccc7", "features": {}, "yanked": false, "links": null}
{"name": "hello-world", "vers": "1.0.1", "deps": [], "cksum": "8a55b58def1ecc7aa8590c7078f379ec9a85328363ffb81d4354314b132b95c4", "features": {}, "yanked": false, "links": null}
2024/05/17 22:53:12 INFO Latest version is 1.0.1
2024/05/17 22:53:12 INFO No update needed for hello-world 1.0.1
2024/05/17 22:53:12 INFO Finished job processing |
|
Issue in this comment is now fixed: https://github.com/dependabot/dependabot-core/pull/9783/files#r1607372525 |
Working to resolve #3478 and enable the support for private Cargo registries. I built this off of #6868, which appeared to be abandoned a while ago in an incomplete state.
I've used the
bin/dry-run.rbto validate this works for Cargo's sparse registries in the dry run scenario (using a private repository with Cloudsmith), and added a bit of clean up and unit testing.