From b4ae1662a74531e9e05b979ce15fa78184dc7d47 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Mon, 8 Apr 2024 17:34:15 +0200 Subject: [PATCH 1/8] Explicit usage of users.GetUID This allows for a more obvious fallback handling in case certain users don't exist. Add a special RootUID constant that can be used to explicitly assign a fallback UID. Also add log statements to every fallback assignment. Signed-off-by: Tom Wieczorek --- cmd/controller/certificates.go | 54 +++++++++++++------ cmd/kubeconfig/create.go | 3 +- cmd/kubeconfig/kubeconfig_test.go | 3 +- internal/pkg/file/file.go | 5 +- internal/pkg/users/users.go | 2 + pkg/certificate/manager.go | 17 +++--- pkg/component/controller/apiserver.go | 3 +- pkg/component/controller/controllermanager.go | 3 +- pkg/component/controller/cplb_unix.go | 3 +- pkg/component/controller/etcd.go | 25 +++++++-- pkg/component/controller/kine.go | 3 +- pkg/component/controller/konnectivity.go | 3 +- pkg/component/controller/scheduler.go | 3 +- 13 files changed, 84 insertions(+), 43 deletions(-) diff --git a/cmd/controller/certificates.go b/cmd/controller/certificates.go index 890c2ecc1857..994eb6d2a968 100644 --- a/cmd/controller/certificates.go +++ b/cmd/controller/certificates.go @@ -25,6 +25,7 @@ import ( "path/filepath" "github.com/k0sproject/k0s/internal/pkg/file" + "github.com/k0sproject/k0s/internal/pkg/users" "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" "github.com/k0sproject/k0s/pkg/certificate" "github.com/k0sproject/k0s/pkg/config" @@ -65,6 +66,13 @@ func (c *Certificates) Init(ctx context.Context) error { c.CACert = string(cert) // Changing the URL here also requires changes in the "k0s kubeconfig admin" subcommand. kubeConfigAPIUrl := fmt.Sprintf("https://localhost:%d", c.ClusterSpec.API.Port) + + apiServerUID, err := users.GetUID(constant.ApiserverUser) + if err != nil { + apiServerUID = users.RootUID + logrus.WithError(err).Warn("Files with key material for kube-apiserver user will be owned by root") + } + eg.Go(func() error { // Front proxy CA if err := c.CertManager.EnsureCA("front-proxy-ca", "kubernetes-front-proxy-ca"); err != nil { @@ -80,7 +88,7 @@ func (c *Certificates) Init(ctx context.Context) error { CACert: proxyCertPath, CAKey: proxyCertKey, } - _, err := c.CertManager.EnsureCertificate(proxyClientReq, constant.ApiserverUser) + _, err := c.CertManager.EnsureCertificate(proxyClientReq, apiServerUID) return err }) @@ -94,16 +102,16 @@ func (c *Certificates) Init(ctx context.Context) error { CACert: caCertPath, CAKey: caCertKey, } - adminCert, err := c.CertManager.EnsureCertificate(adminReq, "root") + adminCert, err := c.CertManager.EnsureCertificate(adminReq, users.RootUID) if err != nil { return err } - if err := kubeConfig(c.K0sVars.AdminKubeConfigPath, kubeConfigAPIUrl, c.CACert, adminCert.Cert, adminCert.Key, "root"); err != nil { + if err := kubeConfig(c.K0sVars.AdminKubeConfigPath, kubeConfigAPIUrl, c.CACert, adminCert.Cert, adminCert.Key, users.RootUID); err != nil { return err } - return c.CertManager.CreateKeyPair("sa", c.K0sVars, constant.ApiserverUser) + return c.CertManager.CreateKeyPair("sa", c.K0sVars, apiServerUID) }) eg.Go(func() error { @@ -115,11 +123,19 @@ func (c *Certificates) Init(ctx context.Context) error { CACert: caCertPath, CAKey: caCertKey, } - konnectivityCert, err := c.CertManager.EnsureCertificate(konnectivityReq, constant.KonnectivityServerUser) + + uid, err := users.GetUID(constant.KonnectivityServerUser) + if err != nil { + uid = users.RootUID + logrus.WithError(err).Warn("Files with key material for konnectivity-server user will be owned by root") + } + + konnectivityCert, err := c.CertManager.EnsureCertificate(konnectivityReq, uid) if err != nil { return err } - return kubeConfig(c.K0sVars.KonnectivityKubeConfigPath, kubeConfigAPIUrl, c.CACert, konnectivityCert.Cert, konnectivityCert.Key, constant.KonnectivityServerUser) + + return kubeConfig(c.K0sVars.KonnectivityKubeConfigPath, kubeConfigAPIUrl, c.CACert, konnectivityCert.Cert, konnectivityCert.Key, uid) }) eg.Go(func() error { @@ -130,12 +146,12 @@ func (c *Certificates) Init(ctx context.Context) error { CACert: caCertPath, CAKey: caCertKey, } - ccmCert, err := c.CertManager.EnsureCertificate(ccmReq, constant.ApiserverUser) + ccmCert, err := c.CertManager.EnsureCertificate(ccmReq, apiServerUID) if err != nil { return err } - return kubeConfig(filepath.Join(c.K0sVars.CertRootDir, "ccm.conf"), kubeConfigAPIUrl, c.CACert, ccmCert.Cert, ccmCert.Key, constant.ApiserverUser) + return kubeConfig(filepath.Join(c.K0sVars.CertRootDir, "ccm.conf"), kubeConfigAPIUrl, c.CACert, ccmCert.Cert, ccmCert.Key, apiServerUID) }) eg.Go(func() error { @@ -146,12 +162,19 @@ func (c *Certificates) Init(ctx context.Context) error { CACert: caCertPath, CAKey: caCertKey, } - schedulerCert, err := c.CertManager.EnsureCertificate(schedulerReq, constant.SchedulerUser) + + uid, err := users.GetUID(constant.SchedulerUser) + if err != nil { + uid = users.RootUID + logrus.WithError(err).Warn("Files with key material for kube-scheduler user will be owned by root") + } + + schedulerCert, err := c.CertManager.EnsureCertificate(schedulerReq, uid) if err != nil { return err } - return kubeConfig(filepath.Join(c.K0sVars.CertRootDir, "scheduler.conf"), kubeConfigAPIUrl, c.CACert, schedulerCert.Cert, schedulerCert.Key, constant.SchedulerUser) + return kubeConfig(filepath.Join(c.K0sVars.CertRootDir, "scheduler.conf"), kubeConfigAPIUrl, c.CACert, schedulerCert.Cert, schedulerCert.Key, uid) }) eg.Go(func() error { @@ -162,7 +185,7 @@ func (c *Certificates) Init(ctx context.Context) error { CACert: caCertPath, CAKey: caCertKey, } - _, err := c.CertManager.EnsureCertificate(kubeletClientReq, constant.ApiserverUser) + _, err := c.CertManager.EnsureCertificate(kubeletClientReq, apiServerUID) return err }) @@ -212,8 +235,7 @@ func (c *Certificates) Init(ctx context.Context) error { CAKey: caCertKey, Hostnames: hostnames, } - _, err = c.CertManager.EnsureCertificate(serverReq, constant.ApiserverUser) - + _, err = c.CertManager.EnsureCertificate(serverReq, apiServerUID) return err }) @@ -227,7 +249,7 @@ func (c *Certificates) Init(ctx context.Context) error { Hostnames: hostnames, } // TODO Not sure about the user... - _, err := c.CertManager.EnsureCertificate(apiReq, constant.ApiserverUser) + _, err := c.CertManager.EnsureCertificate(apiReq, apiServerUID) return err }) @@ -262,7 +284,7 @@ func detectLocalIPs(ctx context.Context) ([]string, error) { return localIPs, nil } -func kubeConfig(dest, url, caCert, clientCert, clientKey, owner string) error { +func kubeConfig(dest, url, caCert, clientCert, clientKey string, ownerID int) error { // We always overwrite the kubeconfigs as the certs might be regenerated at startup const ( clusterName = "local" @@ -295,5 +317,5 @@ func kubeConfig(dest, url, caCert, clientCert, clientKey, owner string) error { return err } - return file.Chown(dest, owner, constant.CertSecureMode) + return file.Chown(dest, ownerID, constant.CertSecureMode) } diff --git a/cmd/kubeconfig/create.go b/cmd/kubeconfig/create.go index e78f44c205fd..29d8c6e89388 100644 --- a/cmd/kubeconfig/create.go +++ b/cmd/kubeconfig/create.go @@ -25,6 +25,7 @@ import ( "os" "path" + "github.com/k0sproject/k0s/internal/pkg/users" "github.com/k0sproject/k0s/pkg/certificate" "github.com/k0sproject/k0s/pkg/config" "github.com/sirupsen/logrus" @@ -104,7 +105,7 @@ Note: A certificate once signed cannot be revoked for a particular user`, certManager := certificate.Manager{ K0sVars: opts.K0sVars, } - userCert, err := certManager.EnsureCertificate(userReq, "root") + userCert, err := certManager.EnsureCertificate(userReq, users.RootUID) if err != nil { return err } diff --git a/cmd/kubeconfig/kubeconfig_test.go b/cmd/kubeconfig/kubeconfig_test.go index 4817ee8704e8..a2156f83bf5c 100644 --- a/cmd/kubeconfig/kubeconfig_test.go +++ b/cmd/kubeconfig/kubeconfig_test.go @@ -23,6 +23,7 @@ import ( "path/filepath" "testing" + "github.com/k0sproject/k0s/internal/pkg/users" "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" "github.com/k0sproject/k0s/pkg/certificate" "github.com/k0sproject/k0s/pkg/config" @@ -118,7 +119,7 @@ yJm2KSue0toWmkBFK8WMTjAvmAw3Z/qUhJRKoqCu3k6Mf8DNl6t+Uw== s.Require().NoError(os.MkdirAll(k0sVars.CertRootDir, 0755)) - userCert, err := certManager.EnsureCertificate(userReq, "root") + userCert, err := certManager.EnsureCertificate(userReq, users.RootUID) s.Require().NoError(err) clusterAPIURL := cfg.Spec.API.APIAddressURL() diff --git a/internal/pkg/file/file.go b/internal/pkg/file/file.go index 630225962052..d31ccef3d935 100644 --- a/internal/pkg/file/file.go +++ b/internal/pkg/file/file.go @@ -21,8 +21,6 @@ import ( "fmt" "io" "os" - - "github.com/k0sproject/k0s/internal/pkg/users" ) // Exists checks if a file exists and is not a directory before we @@ -36,9 +34,8 @@ func Exists(fileName string) bool { } // Chown changes file/dir mode -func Chown(file, owner string, permissions os.FileMode) error { +func Chown(file string, uid int, permissions os.FileMode) error { // Chown the file properly for the owner - uid, _ := users.GetUID(owner) err := os.Chown(file, uid, -1) if err != nil && os.Geteuid() == 0 { return err diff --git a/internal/pkg/users/users.go b/internal/pkg/users/users.go index 2013a9fb2718..419f0447b727 100644 --- a/internal/pkg/users/users.go +++ b/internal/pkg/users/users.go @@ -24,6 +24,8 @@ import ( "strings" ) +const RootUID = 0 // User ID of the root user + // GetUID returns uid of given username and logs a warning if its missing func GetUID(name string) (int, error) { entry, err := user.Lookup(name) diff --git a/pkg/certificate/manager.go b/pkg/certificate/manager.go index 6283116d69db..8795503087ce 100644 --- a/pkg/certificate/manager.go +++ b/pkg/certificate/manager.go @@ -38,7 +38,6 @@ import ( "github.com/k0sproject/k0s/internal/pkg/file" "github.com/k0sproject/k0s/internal/pkg/stringslice" - "github.com/k0sproject/k0s/internal/pkg/users" "github.com/k0sproject/k0s/pkg/config" "github.com/k0sproject/k0s/pkg/constant" ) @@ -100,13 +99,11 @@ func (m *Manager) EnsureCA(name, cn string) error { } // EnsureCertificate creates the specified certificate if it does not already exist -func (m *Manager) EnsureCertificate(certReq Request, ownerName string) (Certificate, error) { +func (m *Manager) EnsureCertificate(certReq Request, ownerID int) (Certificate, error) { keyFile := filepath.Join(m.K0sVars.CertRootDir, fmt.Sprintf("%s.key", certReq.Name)) certFile := filepath.Join(m.K0sVars.CertRootDir, fmt.Sprintf("%s.crt", certReq.Name)) - uid, _ := users.GetUID(ownerName) - // if regenerateCert returns true, it means we need to create the certs if m.regenerateCert(certReq, keyFile, certFile) { logrus.Debugf("creating certificate %s", certFile) @@ -160,11 +157,11 @@ func (m *Manager) EnsureCertificate(certReq Request, ownerName string) (Certific return Certificate{}, err } - err = os.Chown(keyFile, uid, -1) + err = os.Chown(keyFile, ownerID, -1) if err != nil && os.Geteuid() == 0 { return Certificate{}, err } - err = os.Chown(certFile, uid, -1) + err = os.Chown(certFile, ownerID, -1) if err != nil && os.Geteuid() == 0 { return Certificate{}, err } @@ -173,8 +170,8 @@ func (m *Manager) EnsureCertificate(certReq Request, ownerName string) (Certific } // certs exist, let's just verify their permissions - _ = os.Chown(keyFile, uid, -1) - _ = os.Chown(certFile, uid, -1) + _ = os.Chown(keyFile, ownerID, -1) + _ = os.Chown(certFile, ownerID, -1) cert, err := os.ReadFile(certFile) if err != nil { @@ -230,12 +227,12 @@ func isManagedByK0s(cert *certinfo.Certificate) bool { return false } -func (m *Manager) CreateKeyPair(name string, k0sVars *config.CfgVars, owner string) error { +func (m *Manager) CreateKeyPair(name string, k0sVars *config.CfgVars, ownerID int) error { keyFile := filepath.Join(k0sVars.CertRootDir, fmt.Sprintf("%s.key", name)) pubFile := filepath.Join(k0sVars.CertRootDir, fmt.Sprintf("%s.pub", name)) if file.Exists(keyFile) && file.Exists(pubFile) { - return file.Chown(keyFile, owner, constant.CertSecureMode) + return file.Chown(keyFile, ownerID, constant.CertSecureMode) } reader := rand.Reader diff --git a/pkg/component/controller/apiserver.go b/pkg/component/controller/apiserver.go index 38c5511139df..9e87ade014d8 100644 --- a/pkg/component/controller/apiserver.go +++ b/pkg/component/controller/apiserver.go @@ -90,7 +90,8 @@ func (a *APIServer) Init(_ context.Context) error { var err error a.uid, err = users.GetUID(constant.ApiserverUser) if err != nil { - logrus.Warn("running kube-apiserver as root: ", err) + a.uid = users.RootUID + logrus.WithError(err).Warn("Running Kubernetes API server as root") } return assets.Stage(a.K0sVars.BinDir, kubeAPIComponentName, constant.BinDirMode) } diff --git a/pkg/component/controller/controllermanager.go b/pkg/component/controller/controllermanager.go index f57a5aff1376..cb30fd191dd7 100644 --- a/pkg/component/controller/controllermanager.go +++ b/pkg/component/controller/controllermanager.go @@ -69,7 +69,8 @@ func (a *Manager) Init(_ context.Context) error { // controller manager running as api-server user as they both need access to same sa.key a.uid, err = users.GetUID(constant.ApiserverUser) if err != nil { - logrus.Warn("running kube-controller-manager as root: ", err) + a.uid = users.RootUID + logrus.WithError(err).Warn("Running Kubernetes controller manager as root") } // controller manager should be the only component that needs access to diff --git a/pkg/component/controller/cplb_unix.go b/pkg/component/controller/cplb_unix.go index 4a845a03d778..83182222ed84 100644 --- a/pkg/component/controller/cplb_unix.go +++ b/pkg/component/controller/cplb_unix.go @@ -69,7 +69,8 @@ func (k *Keepalived) Init(_ context.Context) error { var err error k.uid, err = users.GetUID(constant.KeepalivedUser) if err != nil { - k.log.Warnf("Unable to get %s UID running keepalived as root: %v", constant.KeepalivedUser, err) + k.uid = users.RootUID + k.log.WithError(err).Warn("Running keepalived as root") } k.configFilePath = filepath.Join(k.K0sVars.RunDir, "keepalived.conf") diff --git a/pkg/component/controller/etcd.go b/pkg/component/controller/etcd.go index 760276340284..50f526b1dd6f 100644 --- a/pkg/component/controller/etcd.go +++ b/pkg/component/controller/etcd.go @@ -71,7 +71,8 @@ func (e *Etcd) Init(_ context.Context) error { e.uid, err = users.GetUID(constant.EtcdUser) if err != nil { - logrus.Warn("running etcd as root: ", err) + e.uid = users.RootUID + logrus.WithError(err).Warn("Running etcd as root") } err = dir.Init(e.K0sVars.EtcdDataDir, constant.EtcdDataDirMode) // https://docs.datadoghq.com/security_monitoring/default_rules/cis-kubernetes-1.5.1-1.1.11/ @@ -263,6 +264,12 @@ func (e *Etcd) setupCerts(ctx context.Context) error { return fmt.Errorf("failed to create etcd ca: %w", err) } + etcdUID, err := users.GetUID(constant.EtcdUser) + if err != nil { + etcdUID = users.RootUID + logrus.WithError(err).Warn("Files with key material for etcd user will be owned by root") + } + eg, _ := errgroup.WithContext(ctx) eg.Go(func() error { @@ -278,7 +285,14 @@ func (e *Etcd) setupCerts(ctx context.Context) error { "localhost", }, } - _, err := e.CertManager.EnsureCertificate(etcdCertReq, constant.ApiserverUser) + + uid, err := users.GetUID(constant.ApiserverUser) + if err != nil { + uid = users.RootUID + logrus.WithError(err).Warn("Files with key material for kube-apiserver user will be owned by root") + } + + _, err = e.CertManager.EnsureCertificate(etcdCertReq, uid) return err }) @@ -295,7 +309,8 @@ func (e *Etcd) setupCerts(ctx context.Context) error { "localhost", }, } - _, err := e.CertManager.EnsureCertificate(etcdCertReq, constant.EtcdUser) + + _, err = e.CertManager.EnsureCertificate(etcdCertReq, etcdUID) return err }) @@ -310,12 +325,12 @@ func (e *Etcd) setupCerts(ctx context.Context) error { e.Config.PeerAddress, }, } - _, err := e.CertManager.EnsureCertificate(etcdPeerCertReq, constant.EtcdUser) + _, err := e.CertManager.EnsureCertificate(etcdPeerCertReq, etcdUID) return err }) eg.Go(func() error { - return e.CertManager.CreateKeyPair("etcd/jwt", e.K0sVars, constant.EtcdUser) + return e.CertManager.CreateKeyPair("etcd/jwt", e.K0sVars, etcdUID) }) return eg.Wait() diff --git a/pkg/component/controller/kine.go b/pkg/component/controller/kine.go index 3f6cffff00c0..8462f85a4c4a 100644 --- a/pkg/component/controller/kine.go +++ b/pkg/component/controller/kine.go @@ -60,7 +60,8 @@ func (k *Kine) Init(_ context.Context) error { var err error k.uid, err = users.GetUID(constant.KineUser) if err != nil { - logrus.Warn("running kine as root: ", err) + k.uid = users.RootUID + logrus.WithError(err).Warn("Running kine as root") } kineSocketDir := filepath.Dir(k.K0sVars.KineSocketPath) diff --git a/pkg/component/controller/konnectivity.go b/pkg/component/controller/konnectivity.go index 7a74cf9eb79d..bc397e3051da 100644 --- a/pkg/component/controller/konnectivity.go +++ b/pkg/component/controller/konnectivity.go @@ -66,8 +66,9 @@ func (k *Konnectivity) Init(ctx context.Context) error { var err error k.uid, err = users.GetUID(constant.KonnectivityServerUser) if err != nil { + k.uid = users.RootUID k.EmitWithPayload("error getting UID for", err) - logrus.Warn("running konnectivity as root: ", err) + logrus.WithError(err).Warn("Running konnectivity as root") } err = dir.Init(k.K0sVars.KonnectivitySocketDir, 0755) if err != nil { diff --git a/pkg/component/controller/scheduler.go b/pkg/component/controller/scheduler.go index 9e414b006f24..3fd99a97af94 100644 --- a/pkg/component/controller/scheduler.go +++ b/pkg/component/controller/scheduler.go @@ -53,7 +53,8 @@ func (a *Scheduler) Init(_ context.Context) error { var err error a.uid, err = users.GetUID(constant.SchedulerUser) if err != nil { - logrus.Warn("running kube-scheduler as root: ", err) + a.uid = users.RootUID + logrus.WithError(err).Warn("Running kube-scheduler as root") } return assets.Stage(a.K0sVars.BinDir, kubeSchedulerComponentName, constant.BinDirMode) } From b3064ce235d7345809744768fdfb019a950e4ea5 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 9 Apr 2024 15:00:49 +0200 Subject: [PATCH 2/8] Use testify in GetUID test Signed-off-by: Tom Wieczorek --- internal/pkg/users/users_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/pkg/users/users_test.go b/internal/pkg/users/users_test.go index 6c9a1649f5ae..6098887a9658 100644 --- a/internal/pkg/users/users_test.go +++ b/internal/pkg/users/users_test.go @@ -17,8 +17,11 @@ limitations under the License. package users import ( + "os/user" "runtime" "testing" + + "github.com/stretchr/testify/assert" ) func TestGetUID(t *testing.T) { @@ -27,15 +30,13 @@ func TestGetUID(t *testing.T) { } uid, err := GetUID("root") - if err != nil { - t.Errorf("failed to find uid for root: %v", err) - } - if uid != 0 { - t.Errorf("root uid is not 0. It is %d", uid) + if assert.NoError(t, err, "Failed to get UID for root user") { + assert.Equal(t, 0, uid, "root's UID is not 0?") } uid, err = GetUID("some-non-existing-user") - if err == nil { - t.Errorf("unexpectedly got uid for some-non-existing-user: %d", uid) + if assert.Error(t, err, "Got a UID for some-non-existing-user?") { + assert.ErrorIs(t, err, user.UnknownUserError("some-non-existing-user")) + assert.Zero(t, uid) } } From 6320960a67abdca6169c810a66383d9852d1f959 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 9 Apr 2024 16:05:24 +0200 Subject: [PATCH 3/8] More context for errors in GetUID There are quite some failure modes for GetUID. Sort them out by providing hopefully helpful error messages. Signed-off-by: Tom Wieczorek --- internal/pkg/users/users.go | 40 ++++++++++++++++++++++++-------- internal/pkg/users/users_test.go | 3 +++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/internal/pkg/users/users.go b/internal/pkg/users/users.go index 419f0447b727..7e07a0f579ed 100644 --- a/internal/pkg/users/users.go +++ b/internal/pkg/users/users.go @@ -17,27 +17,47 @@ limitations under the License. package users import ( + "bytes" "errors" + "fmt" "os/exec" "os/user" "strconv" - "strings" ) const RootUID = 0 // User ID of the root user // GetUID returns uid of given username and logs a warning if its missing func GetUID(name string) (int, error) { - entry, err := user.Lookup(name) - if err == nil { - return strconv.Atoi(entry.Uid) - } - if errors.Is(err, user.UnknownUserError(name)) { + var uid string + + if entry, err := user.Lookup(name); err != nil { + if !errors.Is(err, user.UnknownUserError(name)) { + return 0, err + } + // fallback to call external `id` in case NSS is used - out, err := exec.Command("/usr/bin/id", "-u", name).CombinedOutput() - if err == nil { - return strconv.Atoi(strings.TrimSuffix(string(out), "\n")) + out, idErr := exec.Command("/usr/bin/id", "-u", name).Output() + if idErr != nil { + var exitErr *exec.ExitError + if errors.As(idErr, &exitErr) { + return 0, fmt.Errorf("%w (%w: %s)", err, idErr, bytes.TrimSpace(exitErr.Stderr)) + } + return 0, fmt.Errorf("%w (%w)", err, idErr) } + + uid = string(bytes.TrimSpace(out)) + } else { + uid = entry.Uid } - return 0, err + + parsedUID, err := strconv.Atoi(uid) + if err != nil { + return 0, fmt.Errorf("UID %q is not a decimal integer: %w", uid, err) + } + if parsedUID < 0 { + return 0, fmt.Errorf("UID is negative: %d", parsedUID) + } + + return parsedUID, nil } diff --git a/internal/pkg/users/users_test.go b/internal/pkg/users/users_test.go index 6098887a9658..ed6e51998f94 100644 --- a/internal/pkg/users/users_test.go +++ b/internal/pkg/users/users_test.go @@ -17,6 +17,7 @@ limitations under the License. package users import ( + "os/exec" "os/user" "runtime" "testing" @@ -37,6 +38,8 @@ func TestGetUID(t *testing.T) { uid, err = GetUID("some-non-existing-user") if assert.Error(t, err, "Got a UID for some-non-existing-user?") { assert.ErrorIs(t, err, user.UnknownUserError("some-non-existing-user")) + var exitErr *exec.ExitError + assert.ErrorAs(t, err, &exitErr, "expected external `id` to return an error") assert.Zero(t, uid) } } From 2ce5800d9c9b0b388f50bbebf40ae5f6ca1a2ecf Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 9 Apr 2024 16:17:12 +0200 Subject: [PATCH 4/8] Use id program from PATH The `id` program is not always installed to /usr/bin. Rely on k0s's PATH to find the right executable. Signed-off-by: Tom Wieczorek --- docs/external-runtime-deps.md | 4 ++-- internal/pkg/users/users.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/external-runtime-deps.md b/docs/external-runtime-deps.md index 81d28b21d068..943a2e08cbc1 100644 --- a/docs/external-runtime-deps.md +++ b/docs/external-runtime-deps.md @@ -204,8 +204,8 @@ are not detected. #### id -External `/usr/bin/id` will be executed as a fallback if local user lookup -fails, in case NSS is used. +External `id` will be executed as a fallback if local user lookup fails, in case +NSS is used. ## Windows specific