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

Add os-release gatherer #283

Merged
merged 4 commits into from
Oct 27, 2023
Merged

Add os-release gatherer #283

merged 4 commits into from
Oct 27, 2023

Conversation

rtorrero
Copy link
Contributor

@rtorrero rtorrero commented Oct 25, 2023

This PR adds a new gatherer for the /etc/os-release file. It will take all the values present in the file and export them as a map.

@rtorrero rtorrero force-pushed the os-release-gatherer branch from 247a1c2 to f24d3bb Compare October 26, 2023 08:12
@rtorrero rtorrero marked this pull request as ready for review October 26, 2023 08:14
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Looks good!
Add the gatherer to the list of gatherers please.
No need for a new review

log.Error(err)
return facts, OSReleaseFileError.Wrap(err.Error())
}
defer file.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

return facts, nil
}

func mapToFactValue(inputMap map[string]string) entities.FactValue {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename this to mapOSReleaseToFactValue or similar.
Keep in mind that this function is shared across all the package, and it looks too generic

@@ -0,0 +1,12 @@
NAME="Ubuntu"
VERSION="20.04.3 LTS (Focal Fossa)"
ID=ubuntu
Copy link
Contributor

Choose a reason for hiding this comment

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

I have always seen values wrapped in quotes. Does ubuntu have them without?
It would be nicer to put opensuse 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The quotes are not mandatory AFAIK, see some examples: https://manpages.ubuntu.com/manpages/focal/man5/os-release.5.html

I think they are only required when they include values with spaces in them.

I've added some openSUSE love in the mocked data :-)

Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

Hey man looks good to me! Nice work :)

@rtorrero rtorrero merged commit 8a3a869 into main Oct 27, 2023
9 checks passed
@rtorrero rtorrero deleted the os-release-gatherer branch October 27, 2023 10:41
@CDimonaco CDimonaco added the enhancement New feature or request label Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

4 participants