dependabot / dependabot-core

šŸ¤– Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.72k stars 1.02k forks source link

Composer dependencies update not possible #10031

Closed thesebas closed 4 months ago

thesebas commented 4 months ago

Is there an existing issue for this?

Package ecosystem

composer

Package manager version

2.x

Language version

php 8.2

Manifest location and content before the Dependabot update

No response

dependabot.yml content

No response

Updated dependency

No response

What you expected to see, versus what you actually saw

expected (from before update)

updater | 2024/06/17 03:09:34 INFO <job_842492520> Starting job processing
updater | 2024/06/17 03:09:35 INFO <job_842492520> Starting update job for example-company/some-project
updater | 2024/06/17 03:09:35 INFO <job_842492520> Checking all dependencies for version updates...
updater | 2024/06/17 03:09:35 INFO <job_842492520> Checking if example-company/some-lib 1.6.0 needs updating
  proxy | 2024/06/17 03:09:35 [016] GET https://composer.example-company.com:443/packages.json
  proxy | 2024/06/17 03:09:35 [016] * authenticating composer registry request (host: composer.example-company.com)
  proxy | 2024/06/17 03:09:38 [016] 200 https://composer.example-company.com:443/packages.json
  proxy | 2024/06/17 03:09:38 [018] GET https://packagist.org:443/packages.json
  proxy | 2024/06/17 03:09:38 [018] 200 https://packagist.org:443/packages.json
  proxy | 2024/06/17 03:09:38 [021] GET https://repo.packagist.org:443/p2/example-company/some-lib.json
  proxy | 2024/06/17 03:09:38 [021] 404 https://repo.packagist.org:443/p2/example-company/some-lib.json
  proxy | 2024/06/17 03:09:39 [023] GET https://composer.example-company.com:443/packages.json
  proxy | 2024/06/17 03:09:39 [023] * authenticating composer registry request (host: composer.example-company.com)
  proxy | 2024/06/17 03:09:41 [023] 200 https://composer.example-company.com:443/packages.json
  proxy | 2024/06/17 03:09:41 [025] GET https://composer.example-company.com:443/p2/example-company/some-lib.json
  proxy | 2024/06/17 03:09:41 [025] * authenticating composer registry request (host: composer.example-company.com)
  proxy | 2024/06/17 03:09:43 [025] 200 https://composer.example-company.com:443/p2/example-company/some-lib.json

today I see

updater | 2024/06/18 04:00:29 INFO <job_843630420> Starting job processing
updater | 2024/06/18 04:00:30 INFO <job_843630420> Starting update job for example-company/some-project
updater | 2024/06/18 04:00:30 INFO <job_843630420> Checking all dependencies for version updates...
updater | 2024/06/18 04:00:30 INFO <job_843630420> Checking if example-company/some-lib 1.6.0 needs updating
  proxy | 2024/06/18 04:00:30 [016] GET https://composer.example-company.com:443/packages.json
  proxy | 2024/06/18 04:00:30 [016] * authenticating composer registry request (host: composer.example-company.com)
  proxy | 2024/06/18 04:00:32 [016] 200 https://composer.example-company.com:443/packages.json
  proxy | 2024/06/18 04:00:32 [018] GET https://packagist.org:443/packages.json
  proxy | 2024/06/18 04:00:33 [018] 200 https://packagist.org:443/packages.json
  proxy | 2024/06/18 04:00:33 [021] GET https://repo.packagist.org:443/p2/example-company/some-lib.json
  proxy | 2024/06/18 04:00:33 [021] 404 https://repo.packagist.org:443/p2/example-company/some-lib.json
updater | 2024/06/18 04:00:34 INFO <job_843630420> Handled error whilst updating example-company/some-lib: dependency_file_not_resolvable {:message=>"Could not parse version constraint : Invalid version string \"\""}

Native package manager behavior

No response

Images of the diff or a link to the PR, issue, or logs

Probably related to #10018

Smallest manifest that reproduces the issue

No response

Tawmu commented 4 months ago

We're seeing the same issue across a lot of our repos, with exactly the same symptoms above. Started a day or two back.

TwanVermeulen commented 4 months ago

+1. Here this issue started 3 days ago

joshuafredrickson commented 4 months ago

This started on all of our repos that use wpackagist as of Jun 17, 2024.

d-claassen commented 4 months ago

This started on all of our repos that use wpackagist as of Jun 17, 2024.

Same goes for us. We did not get this error at Jun 17, 2024, 3:34 AM GMT+2, we did get it a day later at Jun 18, 2024, 3:56 AM GMT+2. That matches the timeframe in which the mentioned PR #10018 got merged.

Sending out an explicit ping to @thavaahariharangit and @robaiken, who were involved in #10018. Maybe this rings a bell as the changes from that PR might still be fresh in memory? šŸ™

mrtus commented 4 months ago

Would it be an idea to revert the changes again?

The impact seems to be for all packages that are not hosted on packagist, which means probably all using private packages are impacted by this?

Looks like the change is causing more impact than the reason the change was implemented for?

jeffwidman commented 4 months ago

Thanks for the heads up... I've raised this internally, so we'll try to get it reverted or fixed in the next few days.

mrtus commented 4 months ago

Thanks for the heads up... I've raised this internally, so we'll try to get it reverted or fixed in the next few days.

@jeffwidman Would you be able to provide a status update or updated estimated time regarding this topic?

abdulapopoola commented 4 months ago

Sorry everyone about the continued churn on this; we're open to doing a revert since this is revealing a couple of unforeseen issues. Should either get a fix or a revert before end of week.

mrtus commented 4 months ago

Sorry everyone about the continued churn on this; we're open to doing a revert since this is revealing a couple of unforeseen issues. Should either get a fix or a revert before end of week.

Thank you for the update @abdulapopoola

Reverting would be nice, it means unblocking everyone and creating time to come up with a well tested fix.

thavaahariharangit commented 4 months ago

Changes caused this issue is reverted in PR, Sorry for the inconvenience caused

kasperg commented 4 months ago

@thavaahariharangit Is it possible that this is still partially in effect?

I see it for packages from third party repositories or declared directly in composer.json e.g.

updater | 2024/07/09 12:12:06 INFO <job_853467555> Handled error whilst updating npm-asset/select2: dependency_file_not_resolvable {:message=>"Could not parse version constraint == >= 4.0.13: Invalid version string \">= 4.0.13\""}
updater | 2024/07/09 12:11:58 INFO <job_853467555> Handled error whilst updating bower-asset/jsnlog: dependency_file_not_resolvable {:message=>"Could not parse version constraint == >= 2.30.0: Invalid version string \">= 2.30.0\""}
updater | 2024/07/09 12:12:02 INFO <job_853467555> Handled error whilst updating custom-namespace/custom-package: dependency_file_not_resolvable {:message=>"Could not parse version constraint == >= 2024.27.0: Invalid version string \">= 2024.27.0\""}
mrtus commented 4 months ago

Seeing the same issues appear again on our side too after https://github.com/dependabot/dependabot-core/pull/10150

Packagist returns a 404 for packages not hosted on packagist and after that things start failing.

A dependency through packagist āœ…

updater | 2024/07/09 12:37:41 INFO <job_853476870> Checking if phpunit/phpunit 11.2.6 needs updating
  proxy | 2024/07/09 12:37:41 [028] GET https://repo.packagist.org:443/p2/phpunit/phpunit.json
  proxy | 2024/07/09 12:37:41 [028] 200 https://repo.packagist.org:443/p2/phpunit/phpunit.json
updater | 2024/07/09 12:37:41 INFO <job_853476870> Latest version is 11.2.6
updater | 2024/07/09 12:37:41 INFO <job_853476870> No update needed for phpunit/phpunit 11.2.6

A Github hosted package āŒ

updater | 2024/07/09 12:37:41 INFO <job_853476870> Checking if teamleader/tracing 1.0.1 needs updating
  proxy | 2024/07/09 12:37:41 [030] GET https://repo.packagist.org:443/p2/teamleader/tracing.json
  proxy | 2024/07/09 12:37:41 [030] 404 https://repo.packagist.org:443/p2/teamleader/tracing.json
updater | 2024/07/09 12:37:42 INFO <job_853476870> Handled error whilst updating teamleader/tracing: dependency_file_not_resolvable {:message=>"Could not parse version constraint == >= 1.0.1: Invalid version string \">= 1.0.1\""}
thavaahariharangit commented 4 months ago

@mrtus new solution has been deployed as part of PR: https://github.com/dependabot/dependabot-core/pull/10188

Please let me know your thoughts.

mrtus commented 4 months ago

@mrtus new solution has been deployed as part of PR: #10188

Please let me know your thoughts.

Thank you @thavaahariharangit!

I'll press some buttons and see what happens!

mrtus commented 4 months ago

@thavaahariharangit Seems to work again šŸ‘

Unless it has already been done, may I suggest to cover the use-case that broke for us in a test scenario too?

I don't think this is an uncommon scenario, so creating coverage and preventing this from happening again would benefit everyone working on and using dependabot.

An example composer.json which was affected:

{
    "require": {
        "teamleadercrm/uuidifier": "v1.9.1" // a public or private composer package hosted on github
    },
    "repositories": [
        { // It's repository reference
            "type": "vcs",
            "no-api": true,
            "url":  "git@github.com:teamleadercrm/uuidifier.git"
        },
    ]
}

Maybe a dummy composer package can be introduced to create a similar setup to cover this scenario?

Let me know your thoughts on this!

thavaahariharangit commented 4 months ago

@mrtus

I did analysis on this, consequence of above configuration is that there will not be a latest_allowable version. I have covered this usecase with existing unit test below.

image

composer/spec/dependabot/composer/update_checker/version_resolver_spec.rb:91 https://github.com/dependabot/dependabot-core/pull/10188/files#diff-967b96de8a5d7b8170a34b0185a46d702245a96778814ab9cad40a0534e6dd4c-91 Please let me know your thoughts.

FYI @abdulapopoola

mrtus commented 4 months ago

I did analysis on this, consequence above configuration is there will not be a latest_allowable version. I have covered this usecase with existing unit test below.

Correct @thavaahariharangit

However I do not see that as a great integration test, I do think the test-scenario must live as close as possible to the reality.

I see there is already a fixture inplace https://github.com/dependabot/dependabot-core/pull/10188/files#diff-5aad7d8c049b761b19c656f3846602bd5bc6fd57a407ab0f32161d8a69198d9a, so why not expand on it and make sure the scenario is covered as a real example?

I believe mocked unit-tests are simply more probable for change than a fixture would be. (unless I am not understanding that fixture correctly).

Of course no hard requirement, I'm merely writing down my thoughts and thinking out loud on how the tests can be more robust.

thavaahariharangit commented 4 months ago

12072024

writing a test in core that uses an actual manifest file - in progress.

thavaahariharangit commented 4 months ago

@mrtus

I did a code analysis and found out that they have already covered this as part of below test.

https://github.com/dependabot/dependabot-core/blob/7f7108ceb8edde9ddc8f5e0f9f67d0dc2e7eb9cb/composer/spec/dependabot/composer/update_checker_spec.rb#L342

And it is using the below fixture,

https://github.com/dependabot/dependabot-core/blob/main/composer/spec/fixtures/projects/private_registry/composer.json

I feel this covers the above expectation. Please let me know your thoughts.

thavaahariharangit commented 4 months ago

@jeffwidman Regarding above How do I obtain a new GEMFURY_DEPLOY_TOKEN for testing. (to debug and ensure that working as expected in local/dev env )

FYI @abdulapopoola

mrtus commented 4 months ago

@mrtus

I did a code analysis and found out that they have already covered this as part of below test.

https://github.com/dependabot/dependabot-core/blob/7f7108ceb8edde9ddc8f5e0f9f67d0dc2e7eb9cb/composer/spec/dependabot/composer/update_checker_spec.rb#L342

And it is using the below fixture,

https://github.com/dependabot/dependabot-core/blob/main/composer/spec/fixtures/projects/private_registry/composer.json

I feel this covers the above expectation. Please let me know your thoughts.

šŸ‘Œ Good find!

I do have to ask (sorry), why did it not fail in CI pipeline while it did in production for everyone? While yes, it seems covered, it did not fail and prevent a deploy to production and service disruption, is that assumtion correct?

I think the best course of action here is to go back to a broken buil, try to reproduce and write a test that fails against the broken builds and succeeds on the latest builds. Feel free to temporarily use the example repository (it is public) I mentioned here https://github.com/dependabot/dependabot-core/issues/10031#issuecomment-2222220314 where we know that dependabot failed on.

I hope my concern is clear.

That said, I will remove myself from the conversation as I do not want to involve myself too much, I have no experience in the dependabot project and I feel like I am overstepping questioning the test-coverage. I hope you understand.

Thank you for your efforts so far @thavaahariharangit, I see you are involving other people as well so I believe you will bring this to a good ending!

thavaahariharangit commented 4 months ago

No not at all @mrtus your inputs are really helpful for me to understand and grasp more knowledge about the code, Thank you. As I am relatively new to this product development I am checking from @abdulapopoola and @jeffwidman as they are having more experience on this domain. I need to wait for their response as they are in a different time zone. :)