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.

Sign up for GitHub

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

Jump to bottom

Added Missing Regex for Allowlist Dependency Files #10389

Merged
merged 23 commits into from
Aug 14, 2024
Merged

Added Missing Regex for Allowlist Dependency Files #10389

merged 23 commits into from
Aug 14, 2024

Conversation

honeyankit
Copy link
Contributor

@honeyankit honeyankit commented Aug 7, 2024 •
edited
Loading

What are you trying to accomplish?

This pull request introduces improvements to the updated_files_regex method across all file updaters, enhancing their file matching capabilities. The updated_files_regex method within the FileUpdater class will now ensure that only the intended files are updated for each specific ecosystem. Additionally, this PR includes test cases for the updated_files_regex method across all 18 ecosystems.

Anything you want to highlight for special attention from reviewers?

  • This PR includes changes across all ecosystems. For a smoother review process, please refer to my commit messages as they provide clear information. Request to look the regex carefully for any missing manifest files which needs to be added to the updated_files_regex.

  • Regex support for vendor files has been added only for the go_modules and bundler ecosystems, as these are the ones currently supported. [doc]

  • Q. How changes to updated_files_regex will work ?
    Before creating the PR, the updated_files_regex will validate that the files being updated are appropriate for the ecosystem. If any files are invalid, the PR will not be created and an error will be raised.

How will you know you've accomplished your goal?

This PR should not cause any PR creation failures, except in cases where files from different ecosystems are being updated simultaneously. In those situations, an error will be raised to ensure that the updates are handled correctly.

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.

@honeyankit honeyankit self-assigned this Aug 7, 2024
@honeyankit honeyankit requested a review from a team as a code owner August 7, 2024 22:24
@abdulapopoola
Copy link
Member

@amazimbe , FYI I think this is related to the the issue you noticed today.

@abdulapopoola
Copy link
Member

@honeyankit , is there a way to add tests so we can catch this earlier going forward?

@honeyankit
Copy link
Contributor Author

honeyankit commented Aug 8, 2024 •
edited
Loading

@honeyankit , is there a way to add tests so we can catch this earlier going forward?

@abdulapopoola This PR needs more work as updated_files_regex is not used anywhere in the Core as regex for every ecosystem is not up to date. I will add test that going forward adding updated_files_regex is compulsory else test will fail. This way we can keep track of the missing updated_files_regex in new ecosystem.

@github-actions github-actions bot added L: ruby:bundler RubyGems via bundler L: go:modules Golang modules L: rust:cargo Rust crates via cargo L: php:composer Issues and code for Composer L: devcontainers L: docker Docker containers L: java:gradle Maven packages via Gradle L: github:actions GitHub Actions L: elm Elm packages L: elixir:hex Elixir packages via hex labels Aug 8, 2024
@honeyankit honeyankit requested a review from a team as a code owner August 9, 2024 22:28
@github-actions github-actions bot added L: dotnet:nuget NuGet packages via nuget or dotnet L: java:maven Maven packages via Maven L: javascript L: terraform Terraform packages L: dart:pub Dart packages via pub L: swift Swift packages labels Aug 9, 2024
@honeyankit honeyankit force-pushed the honeyankit/allowlist-dependency-files branch from 71735ce to 6a7b0cd Compare August 9, 2024 23:29
@abdulapopoola
Copy link
Member

Thanks for this @honeyankit , a few generic questions:

  1. Does this impact multi-dir in any way? Or grouping scenarios?
  2. What is our testing / rollout plan to catch any regressions?

kbukum1
kbukum1 approved these changes Aug 12, 2024
View reviewed changes
Copy link
Contributor

@kbukum1 kbukum1 left a comment

Choose a reason for hiding this comment

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

Great job.

@honeyankit honeyankit force-pushed the honeyankit/allowlist-dependency-files branch from fcc801f to 449d26b Compare August 12, 2024 18:00
@honeyankit honeyankit force-pushed the honeyankit/allowlist-dependency-files branch from c5dec55 to 49d1d46 Compare August 14, 2024 04:50
@honeyankit
added submodules regex
afe53b8
@honeyankit
added missing poetry.lock in updated_files_regex
7ee527f
@honeyankit
remove unwanted comma
caf26f9
@honeyankit
added better comment
e3d72c5
@honeyankit
fixed lint error
671cd93
@honeyankit
added regex to support vendor files for bundler and go ecosystem
042f61a
@honeyankit
nested regex and test case added for bundler
5939eab
@honeyankit
regex test case added for cargo
642dab3
@honeyankit
regex test case added for composer
e354a99
@honeyankit
regex test case added for devcontainers
fc11c86
@honeyankit
fixed lint issue for bundler #updated_files_regex test
4c132a3
@honeyankit
regex test case added for docker
8dbc389
@honeyankit
regex test case added for elm, submodules, go, gradle
ed9b544
@honeyankit
regex test case added for hex and fix for gradle and github_actions
0fcb6b5
@honeyankit
regex test case added for npm, yarn, nuget, maven and fix for hex
2ea9fdc
@honeyankit
regex test case added for pub, python, swift and regexfix for python
ad7f5c1
@honeyankit
fixed nuget regex error and python lint error
971fc22
@honeyankit
fixed lint issue in gradle ecosystem
b7e1d94
@honeyankit
added nuget.config and NuGet.Config to test for not updating it
053f27c
@honeyankit
fix the nuget.config case
7875456
@honeyankit
added feature flag for production rollout
5222cfe
@honeyankit honeyankit force-pushed the honeyankit/allowlist-dependency-files branch from 34da5e8 to 5222cfe Compare August 14, 2024 16:55
@honeyankit honeyankit merged commit 134a545 into main Aug 14, 2024
139 of 142 checks passed
@honeyankit honeyankit deleted the honeyankit/allowlist-dependency-files branch August 14, 2024 20:58
@honeyankit honeyankit mentioned this pull request Aug 16, 2024
Merged
5 tasks
imajes pushed a commit to imajes/dependabot-core that referenced this pull request Sep 27, 2024
@honeyankit @imajes
Added Missing Regex for Allowlist Dependency Files (dependabot#10389)
bba7c24 
* added submodules regex

* added missing poetry.lock in updated_files_regex

* remove unwanted comma

* added better comment

* fixed lint error

* added regex to support vendor files for bundler and go ecosystem

* nested regex and test case added for bundler

* regex test case added for cargo

* regex test case added for composer

* regex test case added for devcontainers

* fixed lint issue for bundler #updated_files_regex test

* regex test case added for docker

* regex test case added for elm, submodules, go, gradle

* regex test case added for hex and fix for gradle and github_actions

* regex test case added for npm, yarn, nuget, maven and fix for hex

* regex test case added for pub, python, swift and regexfix for python

* fixed nuget regex error and python lint error

* fixed lint issue in gradle ecosystem

* added nuget.config and NuGet.Config to test for not updating it

* fix the nuget.config case

* added feature flag for production rollout

* replace ff to use boolean variable passed via api

* fixed lint issue in hex and nuget rspec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbukum1 kbukum1 kbukum1 approved these changes

@brettfo brettfo brettfo approved these changes

@jakecoffman jakecoffman jakecoffman approved these changes

@pavera pavera pavera approved these changes

Assignees

@honeyankit honeyankit

Labels
L: dart:pub Dart packages via pub L: devcontainers L: docker Docker containers L: dotnet:nuget NuGet packages via nuget or dotnet L: elixir:hex Elixir packages via hex L: elm Elm packages L: git:submodules Git submodules L: github:actions GitHub Actions L: go:modules Golang modules L: java:gradle Maven packages via Gradle L: java:maven Maven packages via Maven L: javascript L: php:composer Issues and code for Composer L: python L: ruby:bundler RubyGems via bundler L: rust:cargo Rust crates via cargo L: swift Swift packages L: terraform Terraform packages
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@honeyankit @abdulapopoola @pavera @jakecoffman @brettfo @kbukum1