Skip to content

Commit

Permalink
improve the host OS collector and analyzer (#1743)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@laverya
laverya authored Feb 20, 2025
1 parent 51c3a0c commit fb9ea28
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 10 deletions.
10 changes: 5 additions & 5 deletions examples/sdk/helm-template/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
5 changes: 5 additions & 0 deletions examples/sdk/helm-template/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand All @@ -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=
Expand Down
39 changes: 36 additions & 3 deletions pkg/analyze/host_os_info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
91 changes: 91 additions & 0 deletions pkg/analyze/host_os_info_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions pkg/collect/host_os_info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions pkg/collect/host_os_info_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

0 comments on commit fb9ea28

Please sign in to comment.