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

Fix CPUVulnerabilities() reporting from sysfs #532

Merged

Conversation

mwasilew2
Copy link
Contributor

@mwasilew2 mwasilew2 commented Jun 2, 2023

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
sysfs/vulnerability.go Outdated Show resolved Hide resolved
@mwasilew2 mwasilew2 marked this pull request as ready for review June 2, 2023 15:22
Copy link
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.

sysfs/vulnerability_test.go Outdated Show resolved Hide resolved
sysfs/vulnerability.go Show resolved Hide resolved
Copy link
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.

@mwasilew2
Copy link
Contributor Author

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

Copy link
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

mwasilew2 and others added 10 commits June 15, 2023 11:05
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Co-authored-by: Ben Kochie <superq@gmail.com>
Signed-off-by: Michal <mwasilewski@gmx.com>
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
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 <mwasilewski@gmx.com>
@mwasilew2 mwasilew2 force-pushed the mwasilew2/vulnerability-state-parsing branch from 0b132c1 to 3197304 Compare June 15, 2023 09:06
@mwasilew2
Copy link
Contributor Author

mwasilew2 commented Jun 15, 2023

rebased onto upstream

edit: due to unrelated failures in the CI

@mwasilew2
Copy link
Contributor Author

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

Here's the job definition:

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

@SuperQ
Copy link
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
Fix CPUVulnerabilities() reporting from sysfs
---------

Signed-off-by: Michal Wasilewski <mwasilewski@gmx.com>
Signed-off-by: Michal <mwasilewski@gmx.com>
Co-authored-by: Ben Kochie <superq@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CPU vulnerability reporting from sysfs is broken
3 participants