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

Add package_manager for Composer v1 deprecation warning and unsupported error #10716

Merged

Conversation

kbukum1
Copy link
Contributor

@kbukum1 kbukum1 commented Oct 2, 2024

What are you trying to accomplish?

This PR introduces a package_manager subclass for Composer and sets Composer v1 as deprecated and unsupported. The deprecation warning and unsupported error are controlled by the following feature flags:

  • composer_v1_deprecation_warning
  • composer_v1_unsupported_error

Feature Flags PR: https://github.com/github/dependabot-api/pull/5763

This mirrors the approach used for Bundler v1, allowing us to guide users towards migrating to Composer v2 while providing a controlled rollout.

Anything you want to highlight for special attention from reviewers?

The deprecation and unsupported checks for Composer v1 are controlled by feature flags, similar to how it was implemented for Bundler v1. Please focus on verifying that the logic tied to the feature flags works as expected.

How will you know you've accomplished your goal?

We will monitor logs and metrics to ensure:

  • Composer v1 users see the deprecation warning when the feature flag is enabled.
  • Actions using Composer v1 are blocked when the unsupported error flag is enabled.
  • Tests have been added to validate the behavior of both feature flags.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

set composer v1 as deprecated
set composer v1 as unsupported
@github-actions github-actions bot added the L: php:composer Issues and code for Composer label Oct 2, 2024
if parsed_lockfile && parsed_lockfile["plugin-api-version"]
version = Composer::Version.new(parsed_lockfile["plugin-api-version"])
return version.canonical_segments.first == 1 ? "1" : "2"
return version.canonical_segments.first == 1 ? V1 : V2
elsif v1_unsupported
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to change fallback version to v2 when we are not supporting v1 anymore.

end

"2"
# If no conditions are met return V2 by default.
V2
end
Copy link
Contributor Author

@kbukum1 kbukum1 Oct 3, 2024

Choose a reason for hiding this comment

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

Explanation for changes made in composer_version method
We are switching fallback from v1 to v2 if composer v1 is unsupported. As for the other changes we refined the code to improve the logic alongside with comments and reduced duplications for regex checks.

# From composers json-schema: https://getcomposer.org/schema.json
COMPOSER_V2_NAME_REGEX = %r{^[a-z0-9]([_.-]?[a-z0-9]+)*/[a-z0-9](([_.]?|-{0,2})[a-z0-9]+)*$}
COMPOSER_V2_NAME_REGEX = %r{^[a-z0-9]([_.-]?[a-z0-9]++)*/[a-z0-9](([_.]?|-{0,2})[a-z0-9]++)*$}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated quantifiers from + to ++ to prevent backtracking and improve performance, addressing potential ReDoS vulnerabilities flagged by CodeQL.

Resolved CodeQL: #10716 (comment)

@kbukum1 kbukum1 marked this pull request as ready for review October 3, 2024 22:59
@kbukum1 kbukum1 requested a review from a team as a code owner October 3, 2024 22:59
@@ -6,8 +6,13 @@
module Dependabot
module Composer
module Helpers
V1 = "1"
Copy link
Member

Choose a reason for hiding this comment

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

as a follow up; can this file be strongly typed please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I do that in this PR?

Copy link
Member

Choose a reason for hiding this comment

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

A separate PR will be great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sorbet typings another PR will be created.

@@ -1,6 +1,7 @@
# typed: true
Copy link
Member

Choose a reason for hiding this comment

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

in the follow up; let's consider making this strong typing too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I do that in this PR?

Copy link
Member

Choose a reason for hiding this comment

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

It can be a separate PR to avoid blocking this one and for readability too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sorbet typings another PR will be created.

@kbukum1
Copy link
Contributor Author

kbukum1 commented Oct 4, 2024

For sorbet typings another PR will be created.

@kbukum1 kbukum1 merged commit f38ac2f into main Oct 4, 2024
92 checks passed
@kbukum1 kbukum1 deleted the kamil/composer_v1_deprecation_warning_and_unsupported_error branch October 4, 2024 17:53
@kbukum1
Copy link
Contributor Author

kbukum1 commented Oct 5, 2024

FYI @abdulapopoola

Another PR is created for the sorbet typings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: php:composer Issues and code for Composer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants