APP-6696 pass new AgentInfo.PlatformTags in config/reader.go #4469
Conversation
viambot
added
the
safe to test
viambot
added
safe to test
config/reader.go
Outdated
| if runtime.GOOS == "linux" && rutils.PathExists("/etc/os-release") { | ||
| if body, err := os.Open("/etc/os-release"); err != nil { |
There was a problem hiding this comment.
This feels like the long way around for two reasons. First, no need to check the path first. Just check the error from os.Open with !errors.Is(err, os.ErrNotExist)
Second, opening the file, then buffering it into a reader, then parsing it, and deferring a close() is a lot of steps for something that's mostly replaced by "os.ReadFile()" which just gives you back the bytes directly.
That said, nothing inherently wrong here either, so consider this optional.
There was a problem hiding this comment.
simplified it in a532b41
config/reader.go
Outdated
| tags = append(tags, "distro:"+osRelease["ID"]) | ||
| tags = append(tags, "os_version:"+osRelease["VERSION_ID"]) | ||
| tags = append(tags, "codename:"+osRelease["VERSION_CODENAME"]) |
There was a problem hiding this comment.
No guarantee that all of these will be in every os-release file, as the fields are all optional. Many distros populate only some of these. If they're blank, you're gonna end up with odd tags like codename: which may break other stuff down the line when parsing.
Ex: Arch looks like this:
NAME="Arch Linux"
PRETTY_NAME="Arch Linux"
ID=arch
BUILD_ID=rolling
ANSI_COLOR="38;2;23;147;209"
HOME_URL="https://archlinux.org/"
DOCUMENTATION_URL="https://wiki.archlinux.org/"
SUPPORT_URL="https://bbs.archlinux.org/"
BUG_REPORT_URL="https://gitlab.archlinux.org/groups/archlinux/-/issues"
PRIVACY_POLICY_URL="https://terms.archlinux.org/docs/privacy-policy/"
LOGO=archlinux-logo
There was a problem hiding this comment.
changed logic in a532b41 to only set a tag if the value exists
viambot
added
safe to test
DO NOT MERGE
What changed
readExtendedPlatformTagsin config readerWhy
So we can select module binaries based on platform details.