scylladb / scylla-tools-java

Apache Cassandra, supplying tools for Scylla
Apache License 2.0
53 stars 85 forks source link

Update `Netty` , `Guava` and `Logback` dependencies #366

Closed yaronkaikov closed 11 months ago

yaronkaikov commented 11 months ago

This PR updates several dependencies in scylla-tools-java: Netty , Guava and Logback. Before the change, security scanners (such as Trivy and the one we have in the docker hub) reported that those dependencies were vulnerable to several "HIGH", "MEDIUM" and "LOW" severity CVEs. Those issues are fixed in newer versions of those libraries and after this PR the security scanner doesn't report any problems related to the updated dependencies.

Fixes: https://github.com/scylladb/scylla-tools-java/issues/363 Fixes: https://github.com/scylladb/scylla-tools-java/issues/364 Fixes: https://github.com/scylladb/scylla-tools-java/issues/365

yaronkaikov commented 11 months ago

@scylladb/scylla-maint Can we merge this and update submodule?

mykaul commented 11 months ago

How do we know it's not DOA with those updates?

denesb commented 11 months ago

How do we know it's not DOA with those updates?

What is DOA? Should I hold on updating the submodule in scylladb.git?

yaronkaikov commented 11 months ago

How do we know it's not DOA with those updates?

I have built a custom docker image with those changes and Scylla is working.

[yaronkaikov@spider8 ~]$ podman run -d localhost/yaronkaikov/scylla:6.6.9
f9bbb18eea68ada5690ecc5c519d63d430f2ec37cd49b067b7ea00e1181b791c
[yaronkaikov@spider8 ~]$ podman exec -it sharp_williamson nodetool status
Datacenter: datacenter1
=======================
Status=Up/Down
|/ State=Normal/Leaving/Joining/Moving
--  Address     Load       Tokens       Owns    Host ID                               Rack
UN  10.0.2.100  218 KB     256          ?       876ef9dd-6326-48bd-bbdc-9c3399cb1d1a  rack1

Note: Non-system keyspaces don't have the same replication settings, effective ownership information is meaningless
denesb commented 11 months ago

Submodule update here: scylladb/scylladb@a11144cdc40f1de268f79d4cc4d0769206d3f3b6

mykaul commented 11 months ago

How do we know it's not DOA with those updates?

What is DOA? Should I hold on updating the submodule in scylladb.git?

Dead On Arrival - that it doesn't work at all with this deps updated. Since there's hardly / at all any CI in this repo, we have no idea if those changes work or break even basic functionality.

benipeled commented 11 months ago

How do we know it's not DOA with those updates?

What is DOA? Should I hold on updating the submodule in scylladb.git?

Dead On Arrival - that it doesn't work at all with this deps updated. Since there's hardly / at all any CI in this repo, we have no idea if those changes work or break even basic functionality.

Dunno about CI for this repo, but a submodule update for scylladb should go through a pull request to verify that nothing is broken on scylladb, otherwise we'll catch it only on next, as in this case.

mykaul commented 11 months ago

@benipeled - I kinda expect this repo to have a Docker compose for example picking latest Scylla (nightly?) and adding a build from a PR, and then running the smallest sanity on it.

benipeled commented 11 months ago
  1. I'm not sure what's "smallest sanity" and if it's always enough (probably not) to mark the change as verified for scylladb
  2. Instead of implementing another CI here, scylla-ci should cover the required - we can try auto-trigger scylla-ci for PRs on this repo
mykaul commented 11 months ago
  1. I'm not sure what's "smallest sanity" and if it's always enough (probably not) to mark the change as verified for scylladb

nodetool basic commands, cassandra-stress (which for some reason we package), etc.

  1. Instead of implementing another CI here, scylla-ci should cover the required - we can try auto-trigger scylla-ci for PRs on this repo

That's fine too.