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

chore: address snyk vulnerabilities #2579

Merged
merged 25 commits into from
Oct 26, 2022
Merged

chore: address snyk vulnerabilities #2579

merged 25 commits into from
Oct 26, 2022

Conversation

Vikas26021999
Copy link
Contributor

@Vikas26021999 Vikas26021999 commented Oct 17, 2022

Description

With the changes of this PR contains and ignoring vulnerabilities not applicable to us (etcd server mostly), we reached to zero:

Screenshot 2022-10-26 at 12 19 42 PM

How

  1. If we could upgrade a dependency to its vulnerable dependencies, we did so.
  2. Otherwise, replace was used to force the correct version for dependencies that are not in our control

Notion Ticket

https://www.notion.so/rudderstacks/rudder-server-snyk-vulnerabilities-56d33002b9c54385abbfed434124a43f

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@lvrach lvrach changed the title Fix.snyk vul rudder server chore: fix snyk vul rudder server Oct 17, 2022
@codecov
Copy link

codecov bot commented Oct 17, 2022

Codecov Report

Base: 44.08% // Head: 44.04% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (04e007f) compared to base (f13ea33).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2579      +/-   ##
==========================================
- Coverage   44.08%   44.04%   -0.05%     
==========================================
  Files         187      187              
  Lines       39255    39255              
==========================================
- Hits        17305    17288      -17     
- Misses      20845    20862      +17     
  Partials     1105     1105              
Impacted Files Coverage Δ
services/rsources/handler.go 69.72% <0.00%> (-1.39%) ⬇️
processor/processor.go 71.23% <0.00%> (-0.83%) ⬇️
warehouse/schema.go 48.98% <0.00%> (+1.15%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@chrikar chrikar left a comment

Choose a reason for hiding this comment

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

LGTM! Let's check with server team for this as well.

@lvrach lvrach force-pushed the fix.snyk_vul_rudder_server branch from cd7b3d6 to 56a8894 Compare October 26, 2022 07:12
@lvrach lvrach force-pushed the fix.snyk_vul_rudder_server branch from 3df2e97 to 245256d Compare October 26, 2022 07:45
@lvrach lvrach force-pushed the fix.snyk_vul_rudder_server branch from c48a910 to 568c073 Compare October 26, 2022 08:27
@lvrach lvrach force-pushed the fix.snyk_vul_rudder_server branch from b9439d3 to 6b3527b Compare October 26, 2022 08:55
@lvrach lvrach changed the title chore: fix snyk vul rudder server chore: address snyk vulnerabilities Oct 26, 2022
@lvrach lvrach requested a review from a team October 26, 2022 09:23
"fmt"
"io"
"log"
"net/http"
"strconv"
"time"

"github.com/buger/jsonparser"
Copy link
Member

Choose a reason for hiding this comment

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

buger/jsonparser was conflicting with migrate library, it was preventing the upgrade of migrate library.

Since we could easily use json.Unmarshal to parse it I've removed it as a dependency.

@lvrach lvrach requested review from chrikar and a team October 26, 2022 09:34
Copy link
Contributor

@atzoum atzoum left a comment

Choose a reason for hiding this comment

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

one minor comment with regards to replacements

@@ -2,9 +2,20 @@ module github.com/rudderlabs/rudder-server

go 1.18

replace (
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be useful to add a comment here to remind us that at some point, once we upgrade the relevant libraries, these replacements can become outdated and should be removed?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, eventually, we should do that, and we also need to keep the replacement versions up to date.

I am not fun of replace solution, and I tried to use it as little as possible. I am considering ways we can avoid it or make it easier to maintain.

go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants