Skip to content

Conversation

umbynos
Copy link
Contributor

@umbynos umbynos commented Sep 26, 2023

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • Tests for the changes have been added (for bug fixes / features)
  • What kind of change does this PR introduce?

removal of indexes endpoint ("/v2/pkgs/indexes")

  • What is the current behavior?

The indexes endpoint is not used and is a security risk, we are going to remove it

  • What is the new behavior?

Index endpoint is not there anymore.

I took the opportunity to refactor a bit how the "package_staging_index.json" is treated. Now it's handled in the index package and is used by both the old implementation and the new GOA one.

  • Does this PR introduce a breaking change?

Yes, but the endpoint should not be used

  • Other information:

@umbynos umbynos added type: enhancement Proposed improvement topic: code Related to content of the project itself topic: security Related to the protection of user data labels Sep 26, 2023
@umbynos umbynos self-assigned this Sep 26, 2023
@umbynos umbynos force-pushed the remove-indexes-v2-endpoint branch 2 times, most recently from 6ebc0e2 to 55e50a3 Compare September 26, 2023 16:44
@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2023

Codecov Report

Attention: 101 lines in your changes are missing coverage. Please review.

Comparison is base (bf32bad) 18.37% compared to head (5f0a64c) 19.09%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #838      +/-   ##
==========================================
+ Coverage   18.37%   19.09%   +0.71%     
==========================================
  Files          53       46       -7     
  Lines        4120     3635     -485     
==========================================
- Hits          757      694      -63     
+ Misses       3257     2850     -407     
+ Partials      106       91      -15     
Flag Coverage Δ
unit 19.09% <45.69%> (+0.71%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
index/hidefile_default.go 100.00% <100.00%> (ø)
tools/shell_default.go 0.00% <ø> (ø)
v2/http.go 81.48% <100.00%> (-6.02%) ⬇️
v2/pkgs/tools.go 72.51% <97.43%> (+3.69%) ⬆️
main.go 2.30% <0.00%> (+<0.01%) ⬆️
tools/download.go 35.90% <0.00%> (+6.46%) ⬆️
tools/tools.go 0.00% <0.00%> (ø)
index/index.go 52.50% <52.50%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@umbynos umbynos force-pushed the remove-indexes-v2-endpoint branch 2 times, most recently from b22e469 to 53afd39 Compare September 27, 2023 09:57
@umbynos umbynos mentioned this pull request Sep 27, 2023
2 tasks
Copy link
Contributor

@alessio-perugini alessio-perugini left a comment

Choose a reason for hiding this comment

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

I've tested on Linux and it works flawlessly. 🚀
Do you mind testing it on Windows? (Just to be sure that the hideFile function still behaves correctly.)

Great job! Slowly making the agent great again! 😎

@umbynos umbynos force-pushed the remove-indexes-v2-endpoint branch 2 times, most recently from 8cf8573 to 4f358e4 Compare September 28, 2023 15:25
Copy link
Contributor

@alessio-perugini alessio-perugini left a comment

Choose a reason for hiding this comment

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

LGTM.
nit: remove redundant getters methods from the tools struct

@umbynos umbynos force-pushed the remove-indexes-v2-endpoint branch 2 times, most recently from 252a80f to 3f05681 Compare September 29, 2023 17:20
@umbynos
Copy link
Contributor Author

umbynos commented Sep 29, 2023

Just rebased to include changes from #836

@umbynos umbynos requested a review from rhpco October 5, 2023 12:19
@umbynos umbynos force-pushed the remove-indexes-v2-endpoint branch from 3f05681 to 5f0a64c Compare October 6, 2023 15:24
@umbynos umbynos merged commit b8c59a7 into main Oct 9, 2023
@umbynos umbynos deleted the remove-indexes-v2-endpoint branch October 9, 2023 13:13
@umbynos umbynos restored the remove-indexes-v2-endpoint branch October 9, 2023 13:14
@umbynos umbynos deleted the remove-indexes-v2-endpoint branch October 9, 2023 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself topic: security Related to the protection of user data type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants