improve the host OS collector and analyzer (#1743)

The OS version analyzer did not allow checking for things like "redhat 8.x" - this equates to >= 8 && < 9 in the new code.

Also, we previously only collected the OS name (like redhat, centos, or ubuntu) not the OS family (which would be rhel, rhel, and debian for the previous OSes) - this greatly reduces the number of cases required in an analyzer.
This commit is contained in:
Andrew Lavery
2025-02-20 16:03:53 -05:00
committed by GitHub
parent 51c3a0c40f
commit fb9ea281cb
6 changed files with 142 additions and 10 deletions

View File

@@ -63,14 +63,14 @@ require (
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/api v0.32.1 // indirect
k8s.io/apiextensions-apiserver v0.32.1 // indirect
k8s.io/apimachinery v0.32.1 // indirect
k8s.io/client-go v0.32.1 // indirect
k8s.io/api v0.32.2 // indirect
k8s.io/apiextensions-apiserver v0.32.2 // indirect
k8s.io/apimachinery v0.32.2 // indirect
k8s.io/client-go v0.32.2 // indirect
k8s.io/klog/v2 v2.130.1 // indirect
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect
sigs.k8s.io/controller-runtime v0.20.1 // indirect
sigs.k8s.io/controller-runtime v0.20.2 // indirect
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.4.2 // indirect
)

View File

@@ -165,12 +165,16 @@ helm.sh/helm/v3 v3.17.1 h1:gzVoAD+qVuoJU6KDMSAeo0xRJ6N1znRxz3wyuXRmJDk=
helm.sh/helm/v3 v3.17.1/go.mod h1:nvreuhuR+j78NkQcLC3TYoprCKStLyw5P4T7E5itv2w=
k8s.io/api v0.32.1 h1:f562zw9cy+GvXzXf0CKlVQ7yHJVYzLfL6JAS4kOAaOc=
k8s.io/api v0.32.1/go.mod h1:/Yi/BqkuueW1BgpoePYBRdDYfjPF5sgTr5+YqDZra5k=
k8s.io/api v0.32.2/go.mod h1:hKlhk4x1sJyYnHENsrdCWw31FEmCijNGPJO5WzHiJ6Y=
k8s.io/apiextensions-apiserver v0.32.1 h1:hjkALhRUeCariC8DiVmb5jj0VjIc1N0DREP32+6UXZw=
k8s.io/apiextensions-apiserver v0.32.1/go.mod h1:sxWIGuGiYov7Io1fAS2X06NjMIk5CbRHc2StSmbaQto=
k8s.io/apiextensions-apiserver v0.32.2/go.mod h1:GPwf8sph7YlJT3H6aKUWtd0E+oyShk/YHWQHf/OOgCA=
k8s.io/apimachinery v0.32.1 h1:683ENpaCBjma4CYqsmZyhEzrGz6cjn1MY/X2jB2hkZs=
k8s.io/apimachinery v0.32.1/go.mod h1:GpHVgxoKlTxClKcteaeuF1Ul/lDVb74KpZcxcmLDElE=
k8s.io/apimachinery v0.32.2/go.mod h1:GpHVgxoKlTxClKcteaeuF1Ul/lDVb74KpZcxcmLDElE=
k8s.io/client-go v0.32.1 h1:otM0AxdhdBIaQh7l1Q0jQpmo7WOFIk5FFa4bg6YMdUU=
k8s.io/client-go v0.32.1/go.mod h1:aTTKZY7MdxUaJ/KiUs8D+GssR9zJZi77ZqtzcGXIiDg=
k8s.io/client-go v0.32.2/go.mod h1:fpZ4oJXclZ3r2nDOv+Ux3XcJutfrwjKTCHz2H3sww94=
k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f h1:GA7//TjRY9yWGy1poLzYYJJ4JRdzg3+O6e8I+e+8T5Y=
@@ -179,6 +183,7 @@ k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 h1:M3sRQVHv7vB20Xc2ybTt7ODCeFj6J
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
sigs.k8s.io/controller-runtime v0.20.1 h1:JbGMAG/X94NeM3xvjenVUaBjy6Ui4Ogd/J5ZtjZnHaE=
sigs.k8s.io/controller-runtime v0.20.1/go.mod h1:BrP3w158MwvB3ZbNpaAcIKkHQ7YGpYnzpoSTZ8E14WU=
sigs.k8s.io/controller-runtime v0.20.2/go.mod h1:xg2XB0K5ShQzAgsoujxuKN4LNXR2LfwwHsPj7Iaw+XY=
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 h1:/Rv+M11QRah1itp8VhT6HoVx1Ray9eB4DBr+K+/sCJ8=
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3/go.mod h1:18nIHnGi6636UCz6m8i4DhaJ65T6EruyzmoQqI2BVDo=
sigs.k8s.io/structured-merge-diff/v4 v4.4.2 h1:MdmvkGuXi/8io6ixD5wud3vOLwc1rj0aNqRlpuvjmwA=

View File

@@ -60,13 +60,46 @@ func (a *AnalyzeHostOS) CheckCondition(when string, data []byte) (bool, error) {
if len(parts) < 3 {
return false, errors.New("when condition must have at least 3 parts")
}
// handle things like "ubuntu == 20.04", but also "ubuntu == 20.04 || < 20.04" or "ubuntu == 20.04 || < 20.04 || >= 20.04" etc
// the number of parts should be a multiple of 3
if len(parts)%3 != 0 {
return false, errors.New("when condition must have a multiple of 3 parts, such as 'ubuntu == 20.04' or 'rhel >= 8 && < 9'")
}
stringToParse := ""
expectedVer := fixVersion(parts[2])
toleratedVer, err := semver.ParseTolerant(expectedVer)
if err != nil {
return false, errors.Wrapf(err, "failed to parse version: %s", expectedVer)
}
when = fmt.Sprintf("%s %v", parts[1], toleratedVer)
whenRange, err := semver.ParseRange(when)
stringToParse = fmt.Sprintf("%s %s", parts[1], toleratedVer.String())
trimmedParts := strings.Split(when, " ")
// read through the next three parts if they exist - this could look like "|| < 20.04" or "&& >= 8"
for len(trimmedParts) > 3 {
trimmedParts = trimmedParts[3:]
expectedVer = fixVersion(trimmedParts[2])
toleratedVer, err = semver.ParseTolerant(expectedVer)
if err != nil {
return false, errors.Wrapf(err, "failed to parse version: %s", expectedVer)
}
// first part is either "||" or "&&"
// if it's "&&", it is assumed by the semver package and should not be included
// second part is the conditional
// third part is the version
if trimmedParts[0] == "||" {
stringToParse = fmt.Sprintf("%s %s %s %s", stringToParse, trimmedParts[0], trimmedParts[1], toleratedVer.String())
} else if trimmedParts[0] == "&&" {
stringToParse = fmt.Sprintf("%s %s %s", stringToParse, trimmedParts[1], toleratedVer.String())
} else {
return false, errors.Errorf("invalid conditional, expected either && or ||, got %q", trimmedParts[0])
}
}
whenRange, err := semver.ParseRange(stringToParse)
if err != nil {
return false, errors.Wrapf(err, "failed to parse version range: %s", when)
}
@@ -97,7 +130,7 @@ func (a *AnalyzeHostOS) CheckCondition(when string, data []byte) (bool, error) {
return true, nil
}
}
} else if platform == osInfo.Platform {
} else if platform == osInfo.Platform || platform == osInfo.PlatformFamily {
fixedDistVer := fixVersion(osInfo.PlatformVersion)
toleratedDistVer, err := semver.ParseTolerant(fixedDistVer)
if err != nil {

View File

@@ -49,6 +49,47 @@ func TestAnalyzeHostOSCheckCondition(t *testing.T) {
expected: true,
expectErr: false,
},
{
name: "centos < 8 when actual is 7.2",
conditional: "centos < 8",
osInfo: collect.HostOSInfo{
Platform: "centos",
PlatformVersion: "7.2",
},
expected: true,
expectErr: false,
},
{
name: "centos < 8 when actual is 8.2",
conditional: "centos < 8",
osInfo: collect.HostOSInfo{
Platform: "centos",
PlatformVersion: "8.2",
},
expected: false,
expectErr: false,
},
{
name: "centos < 8 when actual is rhel 7.2", // this tests that we properly exclude other OSes despite the version matching
conditional: "centos < 8",
osInfo: collect.HostOSInfo{
Platform: "redhat",
PlatformVersion: "7.2",
},
expected: false,
expectErr: false,
},
{
name: "rhel == 8.2 when actual is 8.2", // this tests that we match on either platform or platform family
conditional: "rhel == 8.2",
osInfo: collect.HostOSInfo{
Platform: "centos",
PlatformFamily: "rhel",
PlatformVersion: "8.2",
},
expected: true,
expectErr: false,
},
{
name: "ubuntu == 20.04 when actual is 18.04",
conditional: "ubuntu == 20.04",
@@ -69,6 +110,56 @@ func TestAnalyzeHostOSCheckCondition(t *testing.T) {
expected: false,
expectErr: true,
},
{
name: "multiple conditionals, neither true",
conditional: "ubuntu > 20.04 || <= 18.04",
osInfo: collect.HostOSInfo{
Platform: "ubuntu",
PlatformVersion: "20.04",
},
expected: false,
expectErr: false,
},
{
name: "multiple conditionals, first true",
conditional: "ubuntu > 20.04 || <= 18.04",
osInfo: collect.HostOSInfo{
Platform: "ubuntu",
PlatformVersion: "22.04",
},
expected: true,
expectErr: false,
},
{
name: "multiple conditionals, second true",
conditional: "ubuntu > 20.04 || <= 18.04",
osInfo: collect.HostOSInfo{
Platform: "ubuntu",
PlatformVersion: "18.04",
},
expected: true,
expectErr: false,
},
{
name: "multiple conditionals, between the two",
conditional: "redhat >= 8 && < 9",
osInfo: collect.HostOSInfo{
Platform: "redhat",
PlatformVersion: "8.2",
},
expected: true,
expectErr: false,
},
{
name: "multiple conditionals, outside the two",
conditional: "redhat >= 8 && < 9",
osInfo: collect.HostOSInfo{
Platform: "redhat",
PlatformVersion: "9.2",
},
expected: false,
expectErr: false,
},
}
for _, test := range tests {

View File

@@ -14,6 +14,7 @@ type HostOSInfo struct {
KernelVersion string `json:"kernelVersion"`
PlatformVersion string `json:"platformVersion"`
Platform string `json:"platform"`
PlatformFamily string `json:"platformFamily"`
}
type HostOSInfoNodes struct {
@@ -44,6 +45,7 @@ func (c *CollectHostOS) Collect(progressChan chan<- interface{}) (map[string][]b
}
hostInfo := HostOSInfo{}
hostInfo.Platform = infoStat.Platform
hostInfo.PlatformFamily = infoStat.PlatformFamily
hostInfo.KernelVersion = infoStat.KernelVersion
hostInfo.Name = infoStat.Hostname
hostInfo.PlatformVersion = infoStat.PlatformVersion

View File

@@ -25,9 +25,10 @@ func TestCollectHostOS_Collect(t *testing.T) {
require.NoError(t, err)
// Check if values exist. They will be different on different machines.
assert.Equal(t, 4, len(m))
assert.Equal(t, 5, len(m))
assert.Contains(t, m, "name")
assert.Contains(t, m, "kernelVersion")
assert.Contains(t, m, "platformVersion")
assert.Contains(t, m, "platformVersion")
assert.Contains(t, m, "platform")
assert.Contains(t, m, "platformFamily")
}