dependabot-core
dependabot-core copied to clipboard
Add support for Cargo private registries
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.rb
to 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.
Currently reaching out to @cloudsmith-io. It appears that we are stymied with the Cargo download endpoint - crates.io
returns metadata when the full version path is not specified to download the crate file, but private registries do not all follow this pattern. I'm trying to work with them to provide some API expectation, such as /api/crates/{crate}
, that could be what Dependabot expects for private registries to fetch the latest version.
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.
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. 👍
@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!
@carogalvin I don't think we are on a tight timetable for it fully working, but I know that presently the introduction of the (non-functional) private registry breaks Cargo being able to update public dependencies as well. If there is a way that we could get that piece of it resolved sooner (I'm not sure if merging this would do it without fixing your internal code as well), we would really appreciate it, but I understand the need to get that slotted in the roadmap. Thanks!
@CodingAnarchy OK, are you able to submit a GitHub support request for the problem with private registries in the meantime? That's the best way to get it into our team's process and then hopefully we can solve that bug a little sooner then pick this up next quarter to fix properly.
@CodingAnarchy OK, are you able to submit a GitHub support request for the problem with private registries in the meantime? That's the best way to get it into our team's process and then hopefully we can solve that bug a little sooner then pick this up next quarter to fix properly.
I'm also waiting for this work to land and can submit a GItHub support request for this too
@carogalvin I've opened a support ticket with some of the details of the error that is blocking Dependabot for us. Please let me know if there is anything else you need from me to help get this bug sorted. Really appreciate the help!
but I know that presently the introduction of the (non-functional) private registry breaks Cargo being able to update public dependencies as well. If there is a way that we could get that piece of it resolved sooner (I'm not sure if merging this would do it without fixing your internal code as well),
Would it help if we split this PR into 2 parts, one for unblocking updates that only use the public registry, and another for actually adding private registry support (which will require us to fix our internal code)? If we do that, adding support for git registries should be relatively easy.
Otherwise we will have to wait until the team can fund support for cargo private registry URLs, which still looks like April.
Would it help if we split this PR into 2 parts, one for unblocking updates that only use the public registry, and another for actually adding private registry support (which will require us to fix our internal code)?
I'm not sure that it can be split as updated here. The reason this fixes the public registry case is that cargo
is now aware from the config that the private registry has a specified index URL. We may be able to just do the parts that move the .cargo/config.toml
file into the part doing the update and unblock updates (assuming I am not missing something about how Dependabot or Cargo is managing that part), though I am not sure that I will be able to do that quickly either myself. I'm about to go out on parental leave until April, so I have no idea how much time I will have to split the PRs.
@CodingAnarchy I've opened a PR to at least start to address the issue where defining a private registry breaks dependabot: https://github.com/dependabot/dependabot-core/pull/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.
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 dependabot-cli
and the Dependabot credential proxy. So @mctofu was correct when he said that this was working. There is still an issue with updating a package that exists only in a private registry, but it may be a bug in Azure Artifacts; I'm prosecuting that internally.
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!
@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 job.yml
file which you used for testing which passes with the cargo private registry based on the comment? I am trying to update the docs and in the process, I tried it locally and getting some parse error.
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?
updater | 2024/05/06 20:46:12 ERROR Error processing nuget (JSON::ParserError)
updater | 2024/05/06 20:46:12 ERROR unexpected token at '{"name":"nuget","vers":"0.2.1","deps":[{"name":"anyhow","req":"^1.0.40","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"bytes","req":"^1.4.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"home","req":"^0.5.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"indicatif","req":"^0.17","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"junction","req":"^1.0.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"reqwest","req":"^0.11","features":["json","blocking"],"optional":false,"default_features":false,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"reqwest-middleware","req":"^0.2.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"reqwest-retry","req":"^0.4.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"semver","req":"^1","features":["serde","serde"],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"serde","req":"^1","features":["derive","derive"],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"serde_json","req":"^1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"service_error","req":"^0.1.1","features":["reqwest"],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":null,"package":null},{"name":"thiserror","req":"^1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"tokio","req":"^1.26.0","features":["macros","sync","time","fs","rt-multi-thread"],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"tracing","req":"^0.1.37","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"zip","req":"^0.6.2","features":["deflate"],"optional":false,"default_features":false,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"mockito","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"dev","registry":"https://github.com/rust-lang/crates.io-index","package":null}],"cksum":"c5a9dfd7380a70e46a876ed67ae11d8e67bc92bcc5e626b0a05f14f3d58d0cac","features":{},"yanked":false,"links":null,"v":2}
updater | {"name":"nuget","vers":"0.1.0","deps":[{"name":"anyhow","req":"^1.0.40","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"bytes","req":"^1.4.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"home","req":"^0.5.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"indicatif","req":"^0.17","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"junction","req":"^1.0.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"reqwest","req":"^0.11","features":["json","blocking"],"optional":false,"default_features":false,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"reqwest-middleware","req":"^0.2.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"reqwest-retry","req":"^0.3.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"semver","req":"^1","features":["serde","serde"],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"serde","req":"^1","features":["derive","derive"],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"serde_json","req":"^1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"thiserror","req":"^1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"tokio","req":"^1.26.0","features":["macros","sync","time","fs"],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"tracing","req":"^0.1.37","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"zip","req":"^0.6.2","features":["deflate"],"optional":false,"default_features":false,"target":null,"kind":"normal","registry":"https://github.com/rust-lang/crates.io-index","package":null},{"name":"mockito","req":"^1.1.0","features":[],"optional":false,"default_features":true,"target":null,"kind":"dev","registry":"https://github.com/rust-lang/crates.io-index","package":null}],"cksum":"e678fc543007ade32539623426e912dd9544c82fde2627c9b3d4f31cd43f0994","features":{},"yanked":false,"links":null,"v":2}
updater | '
updater | 2024/05/06 20:46:12 ERROR /home/dependabot/dependabot-updater/vendor/ruby/3.1.0/gems/json-2.6.3/lib/json/common.rb:216:in `parse'
updater | 2024/05/06 20:46:12 ERROR /home/dependabot/dependabot-updater/vendor/ruby/3.1.0/gems/json-2.6.3/lib/json/common.rb:216:in `parse'
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:
- The public crates.io website's JSON format is not specified and subject to change, so trying to scrape it is not officially supported.
- Private registries don't support the crates.io website endpoint, and so dependabot-core can't actually handle their responses.
- From a Microsoft internal perspective, we filter all public crates through a private registry, and we may quarantine or yank crates that the public crates.io service has published. So we can't go to the public crates.io as the source of truth for public crates.
(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:
job:
package-manager: cargo
allowed-updates:
- dependency-type: direct
update-type: all
source:
provider: azure
repo: mscodehub/Rust/_git/rust.crate-knowledge
directory: "/."
branch: user/rjelling/1.75
credentials:
- type: cargo_registry
url: https://pkgs.dev.azure.com/mscodehub/Rust/_packaging/Rust_PublicPackages/Cargo/index
cargo_registry: Rust_PublicPackages
token: Basic [base64-encoded PAT]
- type: cargo_registry
url: https://pkgs.dev.azure.com/mscodehub/Rust/_packaging/Rust_CrateReview/Cargo/index
cargo_registry: Rust_CrateReview
token: Basic [base64-encoded PAT]
@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!'
20
Created the issue: https://github.com/dependabot/dependabot-core/issues/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.
@RobJellinghaus: Could you also share the cargo.toml and cargo.lock file if you have it? I want to have look to both them.
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 index_url = cargo_config_field("registries.#{registry_name}.index")
from the .cargo/config.toml
which I have not created in my test repo.
def cargo_config
@cargo_config ||= get_original_file(".cargo/config.toml")
end
Edited: After adding the .cargo/config.json
, now getting the same error which you are getting:
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