-
Notifications
You must be signed in to change notification settings - Fork 48
Add uname package #204
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
base: main
Are you sure you want to change the base?
Add uname package #204
Conversation
|
This was previously discussed a bit in:
Currently, multiple implementations of this functionality exist, and they all tend Alternatively, we can try add something like this to golang.org/x/sys/unix, |
|
Speaking somewhat selfishly, I do kind of prefer the slice-based approach I have in cyphar/filepath-securejoin#64 now because it works for pre-3.0 kernels ("2.6" is not a useful version check) but it also seems incredibly unlikely anyone is going to be using this package for pre-3.0 kernels (though, programs might be run in the linux26 personality -- but version checks there are kind of useless for >5.0 kernels). For cyphar/filepath-securejoin we also have an additional restriction -- the code needs to work on Go 1.18 so that people can backport cyphar/filepath-securejoin to old release branches without needing to upgrade their Go compiler (we ran into this issue last year with podman). |
You're right, I totally missed that this code here doesn't allow to check for x.y.z versions, I guess because there was no need to do so.
This is not a problem, I pushed the updated version which works with Go 1.18 (a few other packages in this repo also support Go 1.18). |
|
I guess I can also change to a slice instead of two numbers. |
|
@cyphar I guess I like your implementation better, feel free to submit it for inclusion here (maybe with an "internal/compat" package, too?) |
|
I recall we had some packages in moby as well for kernel versions (although they went beyond just Linux); https://github.com/moby/moby/blob/v2.0.0-beta.0/pkg/parsers/kernel/kernel.go And a minimal (non-exported) implementation in the profiles package for seccomp; we could also consider exposing it as a package in that / those modules? https://github.com/moby/profiles/blob/seccomp/v0.1.0/seccomp/kernel_linux.go Wondering if there's any interfaces that Go consumes nowadays with their work on generics (e.g. the version defined as a string with a |
|
@thaJeztah FYI the one in runc came from containerd which came from the Docker seccomp one. Personally I agree with @kolyshkin that the one we have right now in runc is overengineered (and uses incredibly confusing terminology). The seccomp one you linked is very similar to the runc/containerd one. The new Docker one is a little more useful in some areas but I still think it's a bit overcomplicated. I don't think you generally need to know the trailing As for generics, there is |
|
Yes, I agree that some of the existing implementations took it too far; perhaps it seemed like a good idea at some point, but the whole KernelVersion structs as a requirement in various places made it just cumbersome to use; one of the reasons I moved it internal to try to prevent further spreading. And, yes, for the Linux kernel, I think major.minor (for better words, ISTR they're not strictly named that, which is where I think the What if we would make the return a typed integer? type Version intThen bit-shift the major so that we have a single, comparable int for the version? return Version(values[0]<<16 | values[1])For convenience we could add Major and Minor methods; func (v Version) Major() int {
return int(v) >> 16
}
func (v Version) Minor() int {
return int(v) & 0xFFFF
}Quick write up of that idea; diff --git a/uname/kernel_version_ge.go b/uname/kernel_version_ge.go
index 78e2ff0..d2633c9 100644
--- a/uname/kernel_version_ge.go
+++ b/uname/kernel_version_ge.go
@@ -5,7 +5,5 @@ package uname
// KernelVersionGE checks if the running kernel version
// is greater than or equal to the provided version.
func KernelVersionGE(x, y int) bool {
- xx, yy := KernelVersion()
-
- return xx > x || (xx == x && yy >= y)
+ return KernelVersion() >= Version(x<<16|y)
}
diff --git a/uname/kernel_version_ge_test.go b/uname/kernel_version_ge_test.go
index 4d53950..133e63f 100644
--- a/uname/kernel_version_ge_test.go
+++ b/uname/kernel_version_ge_test.go
@@ -7,7 +7,8 @@ import (
)
func TestKernelVersionGE(t *testing.T) {
- major, minor := KernelVersion()
+ v := KernelVersion()
+ major, minor := v.Major(), v.Minor()
t.Logf("Running on kernel %d.%d", major, minor)
tests := []struct {
@@ -55,6 +56,7 @@ func TestKernelVersionGE(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
+
got := KernelVersionGE(tt.x, tt.y)
if got != tt.want {
t.Errorf("KernelVersionGE(%d, %d): got %v, want %v", tt.x, tt.y, got, tt.want)
diff --git a/uname/kernel_version_linux.go b/uname/kernel_version_linux.go
index 6b0bb01..4cb5aa5 100644
--- a/uname/kernel_version_linux.go
+++ b/uname/kernel_version_linux.go
@@ -8,13 +8,23 @@ import (
"syscall"
)
+type Version int
+
+func (v Version) Major() int {
+ return int(v) >> 16
+}
+
+func (v Version) Minor() int {
+ return int(v) & 0xFFFF
+}
+
// KernelVersion returns major and minor kernel version numbers
// parsed from the [syscall.Uname] Release field, or (0, 0) if
// the version can't be obtained or parsed.
-func KernelVersion() (major, minor int) {
+func KernelVersion() Version {
var uname syscall.Utsname
if err := syscall.Uname(&uname); err != nil {
- return
+ return 0
}
var (
@@ -36,5 +46,5 @@ func KernelVersion() (major, minor int) {
}
}
- return values[0], values[1]
+ return Version(values[0]<<16 | values[1])
}
diff --git a/uname/kernel_version_other.go b/uname/kernel_version_other.go
index e190481..481dfd6 100644
--- a/uname/kernel_version_other.go
+++ b/uname/kernel_version_other.go
@@ -6,6 +6,6 @@
package uname
-func KernelVersion() (major int, minor int) {
- return 0, 0
+func KernelVersion() Version {
+ return 0
}
diff --git a/uname/kernel_version_test.go b/uname/kernel_version_test.go
index c38fa45..b515bfd 100644
--- a/uname/kernel_version_test.go
+++ b/uname/kernel_version_test.go
@@ -7,7 +7,8 @@ import (
)
func TestKernelVersion(t *testing.T) {
- x, y := KernelVersion()
+ v := KernelVersion()
+ x, y := v.Major(), v.Minor()
t.Logf("KernelVersion: %d.%d (GOOS: %s)", x, y, runtime.GOOS)
switch runtime.GOOS {
case "linux": |
|
The single version number approach is what a lot of C programs use in general for version checks (though usually they multiply by 100 or something rather than bit-shifting, to make it easier to debug values). I think (like most C patterns) it's a little too easy to shoot yourself in the foot, but at a push I wouldn't be against it either. (Not sure if the interface would be super nice to use either.) |
OTOH this code is extremely simple, it's just a screenful of code and no extra dependencies (other than x/sys/unix), while #205 has some additional ones for testing, and compat layer for generics, which feels like a lot already. I am aiming for something extremely simple here (but don't want to go with bit shifting or Any opinions? |
uname/kernel_version_ge.go
Outdated
| // Copyright 2025 The uname Authors. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we add copyright headers (which we really should start doing in other modules as well), should we also start adding a license header to go with it?
I think in this case, the Module will be Apache 2 licensed, with the forked code being relicensed.
For the Apache 2 licenses I'm always a bit sad about the amount of boilerplating that results from it; more so if code gets moved around between projects (so now accumulating a whole bunch of headers).
While the Apache 2 guidelines still mentions "take this template and apply it to each file", and/or add a Notice file I THINK (but IANAL), SPDX headers are generally acceptable now, which ... definitely could make things much less verbose;
So for "new" files, probably something like:
// SPDX-FileCopyrightText: 2025 The uname Authors.
// SPDX-License-Identifier: Apache-2.0And for the files we forked from Go;
// SPDX-FileCopyrightText: 2022 The Go Authors
// SPDX-FileCopyrightText: 2025 The uname Authors.
// SPDX-License-Identifier: Apache-2.0
// Copyright 2022 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE.BSD file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added proper copyright headers (KernelVersionGE is now included so it's also copyright by Go authors).
| // KernelVersionGE checks if the running kernel version | ||
| // is greater than or equal to the provided version. | ||
| func KernelVersionGE(x, y int) bool { | ||
| xx, yy := KernelVersion() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how much changes we want to make to the forked files, but if we're open to making changes, then I usually like calling non-exported functions when used internally, and have a single exported "implementation" (which can be backed by a non-exported platform-specific one); it can make it more clear / discoverable what code is used, and what not., and having a single exported function gives a good place to maintain the GoDoc.
So along these lines;
// KernelVersionGE checks if the running kernel version
// is greater than or equal to the provided version.
func KernelVersionGE(x, y int) bool {
xx, yy := kernelVersion()
// etc.The platform-agnostic file would have the exported
// KernelVersion returns major and minor kernel version numbers
// parsed from the [syscall.Uname] Release field, or (0, 0) if
// the version can't be obtained or parsed.
func KernelVersion() (major, minor int) {
return kernelVersion()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
72abf7b to
a63f9d5
Compare
uname/kernel_version_ge_test.go
Outdated
| @@ -0,0 +1,72 @@ | |||
| // SPDX-FileCopyrightText: 2025 The Go Authors | |||
| // SPDX-FileCopyrightText: 2025 The uname Authors. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks like you picked uname here, and moby/sys for some of the other ones 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad; fixed
This is a tiny package to check the Linux kernel version. It is taken from the golang sources (src/internal/syscall/unix), with the following changes: - removed irrelevant parts, including FreeBSD and Solaris kernelVersion implementations; - added a tiny test case for KernelVersion; - added some documentation; - added proper copyright headers. The code is covered by the "BSD 3-clause" license, which is compatible with Apache license. The history of the code before the fork can be seen here (oldest first): - https://go-review.googlesource.com/c/go/+/424896 - https://go-review.googlesource.com/c/go/+/427675 - https://go-review.googlesource.com/c/go/+/427676 - https://go-review.googlesource.com/c/go/+/700796 This is aimed to replace the relevant containerd package (github.com/containerd/containerd/pkg/kernelversion) and its forks. Signed-off-by: Kir Kolyshkin <[email protected]>
|
maybe we should rename |
|
Yeah; was considering that as well. It could be I recall we also have a package in docker / moby, which provides this, which also supports other OSes (Windows, macOS); so if such functionality would be OK to include in future, then making the name more generic would definitely be something to consider. https://pkg.go.dev/github.com/docker/[email protected]+incompatible/pkg/parsers/kernel |
This is a tiny package to check the Linux kernel version. It is taken
from the golang sources (src/internal/syscall/unix), with the irrelevant
parts, including FreeBSD and Solaris implementations, removed.
The code is covered by the "BSD 3-clause" license, which is compatible
with Apache license.
The history of the code before the fork can be seen here (oldest first):
On top of that, this PR includes:
KernelVersion;KernelVersionGEfunction (same as inhttps://go-review.googlesource.com/c/go/+/700796).
This is aimed to replace the relevant containerd package
(github.com/containerd/containerd/pkg/kernelversion) and its forks.
The difference is smaller code and simpler API.
Usage example: