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

border effect when reading data : redis::GetByPackName - breaks on a redis cluster with RO slaves #210

Closed
tramora opened this issue Feb 24, 2022 · 2 comments · Fixed by #220
Labels

Comments

@tramora
Copy link
Contributor

tramora commented Feb 24, 2022

Context

We are running vuls and goval-dictionary:v0.6.1 successfully in PRODUCTION. Thanx ;)

Yesterday on our staging environment we tried to bump to the latest stable version : goval-dictionary:v0.7.1.1 and faced a regression.

A quick glance to the code showed that since goval-dictionary:v0.7.0 the redis::GetByPackName has a border effect that breaks in our specific environment :

  • our redis cluster has WR master and RO slaves and it is not expected at all that a write is performed on the slaves.

Is it possible to rework the change to avoid this border effect or if not possible suggest an architecture when using a redis cluster with RO slaves ?

What did you expect to happen?

A Get data without any border effect that fits well with our architecture.

What happened instead?

A write attempt (SREM) on the redis DB is performed.

"READONLY You can't write against a read only replica" :

time="Feb 24 13:31:48" level=error msg="Failed to detect Pkg CVE: Failed to detect CVE with OVAL:\n    github.com/future-architect/vuls/detector.DetectPkgCves\n        /go/src/github.com/future-architect/vuls/detector/detector.go:220\n  - Failed to get Definitions from DB. err:\n    github.com/future-architect/vuls/oval.Ubuntu.fillWithOval\n        /go/src/github.com/future-architect/vuls/oval/debian.go:474\n  - Failed to get ubuntu OVAL info by package: oval.request{packName:\"binutils\", versionRelease:\"2.34-6ubuntu1.1\", newVersionRelease:\"\", arch:\"\", binaryPackNames:[]string(nil), isSrcPack:false, modularityLabel:\"\"}, err:\n    github.com/future-architect/vuls/oval.getDefsByPackNameFromOvalDB\n        /go/src/github.com/future-architect/vuls/oval/util.go:282\n  - Failed to exec pipeline. err:\n    github.com/vulsio/goval-dictionary/db.(*RedisDriver).GetByPackName\n        /go/pkg/mod/github.com/vulsio/goval-dictionary@v0.7.1-0.20220215081041-a472884d0afa/db/redis.go:212\n  - READONLY You can't write against a read only replica."

Steps to reproduce the behaviour

  • use a redis cluster with RO slaves.
  • perform a CVE scan under the special condition of missing defIDs (log : "Failed to HMGet. err: Some fields do not exist. continue scanning")

Configuration (MUST fill this out):

  • goval-dictionary environment:

goval-dictionary:v0.7.1.1

It seems that the change was introducted at the end of last year 2021 (Dec 28th) in version goval-dictionary:v0.7.0 :
9c671c7

@tramora tramora added the bug label Feb 24, 2022
@tramora
Copy link
Contributor Author

tramora commented Mar 16, 2022

@MaineK00n : as you pushed the change do you have any thoughts on this matter please ?

@MaineK00n
Copy link
Collaborator

MaineK00n commented Mar 17, 2022

@tramora

As for me, like this PR(#220), if this error (defstr==nil), I think it should be error, because it is likely that the Redis releation is broken.
Please read some of the background to the current situation in the proposed PR.

Until it is merged, I think it is better to check out my branch and use the binary that I built or use the one from a previous release to get around it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants