Skip to content

Fix CPUVulnerabilities() reporting from sysfs#532

Merged
SuperQ merged 10 commits intoprometheus:masterfrom
mwasilew2:mwasilew2/vulnerability-state-parsing
Jun 15, 2023
Merged

Fix CPUVulnerabilities() reporting from sysfs#532
SuperQ merged 10 commits intoprometheus:masterfrom
mwasilew2:mwasilew2/vulnerability-state-parsing

Conversation

@mwasilew2
Copy link
Copy Markdown
Contributor

@mwasilew2 mwasilew2 commented Jun 2, 2023 •
edited
Loading

closes #531

This PR contains a few changes (each in a separate commit):

  • adds fixtures and tests for parsing cpu vulnerabilities from sysfs, fixes the typo in the vulnerability string
  • switches to using a map of vulnerabilities instead of a slice (makes working with the result easier, e.g. when testing)
  • switches to using encoding of the state
  • adds license in the test file
  • adds linux build flags
  • adds test cases for vulnerable state, brings state string to lower cases (to try and make it case insensitive)

@mwasilew2 mwasilew2 marked this pull request as draft June 2, 2023 14:22
SuperQ
SuperQ reviewed Jun 2, 2023
View reviewed changes
@mwasilew2 mwasilew2 marked this pull request as ready for review June 2, 2023 15:22
SuperQ
SuperQ requested changes Jun 3, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Minor nits, otherwise great.

SuperQ
SuperQ requested changes Jun 3, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Minor nits, otherwise great.

discordianfish
discordianfish reviewed Jun 9, 2023
View reviewed changes
discordianfish
discordianfish reviewed Jun 9, 2023
View reviewed changes
@mwasilew2
Copy link
Copy Markdown
Contributor Author

mwasilew2 commented Jun 12, 2023

@discordianfish @SuperQ is there anything else I can help with here?

SuperQ
SuperQ approved these changes Jun 15, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

@SuperQ SuperQ requested a review from discordianfish June 15, 2023 06:02
mwasilew2 and others added 10 commits June 15, 2023 11:05
@mwasilew2
add tests for sysfs vulnerabilities
03b4c5e 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2
use a vulnerability indexed map instead of a slice
fd0c9fd 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2
switch to encoding for vuln state instead of plain strings
a297895 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2
add missing license in the test file
254efce 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2
add missing build tags
9c26b1d 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2
add test cases for vulnerable, unify char cases when parsing state
1faaea2 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2 @SuperQ
Update sysfs/vulnerability_test.go
953794a 
Co-authored-by: Ben Kochie <[email protected]>
Signed-off-by: Michal <[email protected]>
@mwasilew2
minor update to the readme
3096943 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2
use switch instead of multiple ifs (no semantic changes)
fbbf002 
Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2
Explain purpose of VulnerabilityHumanEncoding
3197304 
adds a go doc explaining the purpose of the map containing mapping from
int encoded state to human friendly strings

Signed-off-by: Michal Wasilewski <[email protected]>
@mwasilew2 mwasilew2 force-pushed the mwasilew2/vulnerability-state-parsing branch from 0b132c1 to 3197304 Compare June 15, 2023 09:06
@mwasilew2
Copy link
Copy Markdown
Contributor Author

mwasilew2 commented Jun 15, 2023 •
edited
Loading

rebased onto upstream

edit: due to unrelated failures in the CI

@mwasilew2
Copy link
Copy Markdown
Contributor Author

mwasilew2 commented Jun 15, 2023

the codespell job is failing, but it's unrelated to this change

Here's the job definition:

procfs/.circleci/config.yml

Lines 38 to 45 in abbfda0

codespell:
docker:
- image: circleci/python
steps:
- checkout
- run: sudo pip install codespell
- run: codespell --skip=".git,./vendor,ttar,fixtures.ttar,./fixtures,go.mod,go.sum" -I scripts/codespell_ignore.txt

the docker image hasn't changed recently: https://hub.docker.com/r/circleci/python/tags

codespell pip package was updated yesterday: https://pypi.org/project/codespell/#history

I think this is unrelated and should be handled in a separate issue/PR. I'll open a separate one

@mwasilew2 mwasilew2 mentioned this pull request Jun 15, 2023
Closed
@SuperQ
Copy link
Copy Markdown
Member

SuperQ commented Jun 15, 2023

Going to ignore the codespell, it's fixed in HEAD.

@SuperQ SuperQ merged commit a76f400 into prometheus:master Jun 15, 2023
jritter pushed a commit to jritter/procfs that referenced this pull request Jul 15, 2024
@mwasilew2 @SuperQ @jritter
Fix CPUVulnerabilities() reporting from sysfs (prometheus#532)
658d0c0 
Fix CPUVulnerabilities() reporting from sysfs
---------

Signed-off-by: Michal Wasilewski <[email protected]>
Signed-off-by: Michal <[email protected]>
Co-authored-by: Ben Kochie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@discordianfish discordianfish discordianfish approved these changes

@SuperQ SuperQ SuperQ approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CPU vulnerability reporting from sysfs is broken

3 participants

@mwasilew2 @SuperQ @discordianfish