From 3cb808dff406b561935bf94431f490fd1c0eb1be Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Mon, 9 Sep 2024 10:30:16 +0200 Subject: [PATCH 01/11] Warnings that k8s service may not work KU-1475 --- src/k8s/cmd/k8s/k8s_bootstrap.go | 2 +- src/k8s/cmd/k8sd/k8sd.go | 5 +- src/k8s/cmd/util/hooks.go | 92 ++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 src/k8s/cmd/util/hooks.go diff --git a/src/k8s/cmd/k8s/k8s_bootstrap.go b/src/k8s/cmd/k8s/k8s_bootstrap.go index b4243d824..67b2c917a 100644 --- a/src/k8s/cmd/k8s/k8s_bootstrap.go +++ b/src/k8s/cmd/k8s/k8s_bootstrap.go @@ -45,7 +45,7 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { Use: "bootstrap", Short: "Bootstrap a new Kubernetes cluster", Long: "Generate certificates, configure service arguments and start the Kubernetes services.", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat)), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookVerifyResources()), Run: func(cmd *cobra.Command, args []string) { if opts.interactive && opts.configFile != "" { cmd.PrintErrln("Error: --interactive and --file flags cannot be set at the same time.") diff --git a/src/k8s/cmd/k8sd/k8sd.go b/src/k8s/cmd/k8sd/k8sd.go index dedfafdf7..452d9770d 100644 --- a/src/k8s/cmd/k8sd/k8sd.go +++ b/src/k8s/cmd/k8sd/k8sd.go @@ -33,8 +33,9 @@ func addCommands(root *cobra.Command, group *cobra.Group, commands ...*cobra.Com func NewRootCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ - Use: "k8sd", - Short: "Canonical Kubernetes orchestrator and clustering daemon", + Use: "k8sd", + Short: "Canonical Kubernetes orchestrator and clustering daemon", + PreRun: cmdutil.HookVerifyResources(), Run: func(cmd *cobra.Command, args []string) { // configure logging log.Configure(log.Options{ diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go new file mode 100644 index 000000000..ac0ea42f1 --- /dev/null +++ b/src/k8s/cmd/util/hooks.go @@ -0,0 +1,92 @@ +package cmdutil + +import ( + "errors" + "fmt" + "os" + "os/exec" + "strings" + "syscall" + + "github.com/spf13/cobra" +) + +// paths to validate if root in the owner +var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} + +// HookVerifyResources checks ownership of dirs required for k8s to run. +// HookVerifyResources validates AppArmor configurations. +// If potential issue found pops up warning. +func HookVerifyResources() func(*cobra.Command, []string) { + return func(cmd *cobra.Command, args []string) { + var warnList []string + for _, path := range pathsOwnershipCheck { + if msg, err := validateRootOwnership(path); err != nil { + cmd.PrintErrf(err.Error()) + } else { + warnList = append(warnList, msg) + } + } + + if armor, err := checkAppArmor(); err != nil { + cmd.PrintErr(err.Error()) + } else if len(armor) > 0 { + warnList = append(warnList, armor) + } + + if len(warnList) > 0 { + cmd.PrintErrf("Warning: k8s may not run correctly due to reasons:\n%s"+ + "If runnung inside LXD container refer to "+ + "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/.\n", + strings.Join(warnList, "")) + } + } +} + +// validateRootOwnership checks if given path owner root and root group. +func validateRootOwnership(path string) (string, error) { + + info, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) { + return fmt.Sprintf("%s do not exist\n", path), nil + } else { + return "", err + } + } + var UID int + var GID int + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + UID = int(stat.Uid) + GID = int(stat.Gid) + } else { + return "", errors.New(fmt.Sprintf("cannot access path %s", path)) + } + var warnList string + if UID != 0 { + warnList += fmt.Sprintf("owner of %s is user with UID %d expected 0\n", path, UID) + } + if GID != 0 { + warnList += fmt.Sprintf("owner of %s is group with GID %d expected 0\n", path, GID) + } + return warnList, nil +} + +// checkAppArmor checks AppArmor status. +func checkAppArmor() (string, error) { + cmd := exec.Command("journalctl", "-u", "apparmor") + out, err := cmd.CombinedOutput() + if err != nil { + return "", err + } + output := string(out) + // AppArmor configured for container or service not present + if strings.Contains(output, "Not starting AppArmor in container") || strings.Contains(output, "-- No entries --") { + return "", nil + // cannot read status of AppArmor + } else if strings.Contains(output, "Users in groups 'adm', 'systemd-journal' can see all messages.") { + return "could not validate AppArmor status\n", nil + } + + return "AppArmor may block hosting of nested containers\n", nil +} From 9b6bb633eb01f30432cf62cec68400a6a8d51dd7 Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Mon, 9 Sep 2024 15:53:57 +0200 Subject: [PATCH 02/11] Added validation for running in lxd --- src/k8s/cmd/util/hooks.go | 44 +++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go index ac0ea42f1..f343c6e87 100644 --- a/src/k8s/cmd/util/hooks.go +++ b/src/k8s/cmd/util/hooks.go @@ -11,6 +11,8 @@ import ( "github.com/spf13/cobra" ) +const initialProcesEnvironmentVariables = "/proc/1/environ" + // paths to validate if root in the owner var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} @@ -20,36 +22,60 @@ var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} func HookVerifyResources() func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { var warnList []string + + // check ownership of required dirs for _, path := range pathsOwnershipCheck { if msg, err := validateRootOwnership(path); err != nil { cmd.PrintErrf(err.Error()) - } else { + } else if len(msg) > 0 { warnList = append(warnList, msg) } } + // check App Armor if armor, err := checkAppArmor(); err != nil { cmd.PrintErr(err.Error()) } else if len(armor) > 0 { warnList = append(warnList, armor) } + // check LXD + if lxd, err := validateLXD(); err != nil { + cmd.PrintErr(err.Error()) + } else if len(warnList) > 0 && len(lxd) > 0 { + warnList = append(warnList, lxd) + + } + + // generate report if len(warnList) > 0 { - cmd.PrintErrf("Warning: k8s may not run correctly due to reasons:\n%s"+ - "If runnung inside LXD container refer to "+ - "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/.\n", + cmd.PrintErrf("Warning: k8s may not run correctly due to reasons:\n%s", strings.Join(warnList, "")) } } } +// validateLXD checks if k8s runs in lxd container if so returns link to documentation +func validateLXD() (string, error) { + dat, err := os.ReadFile(initialProcesEnvironmentVariables) + if err != nil { + return "", err + } + env := string(dat) + if strings.Contains(env, "container=lxc") { + return "For running k8s inside LXD container refer to " + + "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/.\n", nil + } + return "", nil +} + // validateRootOwnership checks if given path owner root and root group. func validateRootOwnership(path string) (string, error) { info, err := os.Stat(path) if err != nil { if os.IsNotExist(err) { - return fmt.Sprintf("%s do not exist\n", path), nil + return fmt.Sprintf("%s do not exist.\n", path), nil } else { return "", err } @@ -64,10 +90,10 @@ func validateRootOwnership(path string) (string, error) { } var warnList string if UID != 0 { - warnList += fmt.Sprintf("owner of %s is user with UID %d expected 0\n", path, UID) + warnList += fmt.Sprintf("owner of %s is user with UID %d expected 0.\n", path, UID) } if GID != 0 { - warnList += fmt.Sprintf("owner of %s is group with GID %d expected 0\n", path, GID) + warnList += fmt.Sprintf("owner of %s is group with GID %d expected 0.\n", path, GID) } return warnList, nil } @@ -85,8 +111,8 @@ func checkAppArmor() (string, error) { return "", nil // cannot read status of AppArmor } else if strings.Contains(output, "Users in groups 'adm', 'systemd-journal' can see all messages.") { - return "could not validate AppArmor status\n", nil + return "could not validate AppArmor status.\n", nil } - return "AppArmor may block hosting of nested containers\n", nil + return "AppArmor may block hosting of nested containers.\n", nil } From 727fb4a70267ece73318a67b12288770f96743a1 Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Wed, 11 Sep 2024 20:35:40 +0200 Subject: [PATCH 03/11] moved validation to app --- src/k8s/cmd/k8s/k8s_bootstrap.go | 2 +- src/k8s/cmd/k8sd/k8sd.go | 5 +- src/k8s/cmd/util/hooks.go | 118 ------------------------ src/k8s/pkg/k8sd/app/hooks_bootstrap.go | 2 + src/k8s/pkg/k8sd/app/hooks_join.go | 2 + src/k8s/pkg/utils/file.go | 108 ++++++++++++++++++++++ 6 files changed, 115 insertions(+), 122 deletions(-) delete mode 100644 src/k8s/cmd/util/hooks.go diff --git a/src/k8s/cmd/k8s/k8s_bootstrap.go b/src/k8s/cmd/k8s/k8s_bootstrap.go index 67b2c917a..b4243d824 100644 --- a/src/k8s/cmd/k8s/k8s_bootstrap.go +++ b/src/k8s/cmd/k8s/k8s_bootstrap.go @@ -45,7 +45,7 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { Use: "bootstrap", Short: "Bootstrap a new Kubernetes cluster", Long: "Generate certificates, configure service arguments and start the Kubernetes services.", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookVerifyResources()), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat)), Run: func(cmd *cobra.Command, args []string) { if opts.interactive && opts.configFile != "" { cmd.PrintErrln("Error: --interactive and --file flags cannot be set at the same time.") diff --git a/src/k8s/cmd/k8sd/k8sd.go b/src/k8s/cmd/k8sd/k8sd.go index 452d9770d..dedfafdf7 100644 --- a/src/k8s/cmd/k8sd/k8sd.go +++ b/src/k8s/cmd/k8sd/k8sd.go @@ -33,9 +33,8 @@ func addCommands(root *cobra.Command, group *cobra.Group, commands ...*cobra.Com func NewRootCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ - Use: "k8sd", - Short: "Canonical Kubernetes orchestrator and clustering daemon", - PreRun: cmdutil.HookVerifyResources(), + Use: "k8sd", + Short: "Canonical Kubernetes orchestrator and clustering daemon", Run: func(cmd *cobra.Command, args []string) { // configure logging log.Configure(log.Options{ diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go deleted file mode 100644 index f343c6e87..000000000 --- a/src/k8s/cmd/util/hooks.go +++ /dev/null @@ -1,118 +0,0 @@ -package cmdutil - -import ( - "errors" - "fmt" - "os" - "os/exec" - "strings" - "syscall" - - "github.com/spf13/cobra" -) - -const initialProcesEnvironmentVariables = "/proc/1/environ" - -// paths to validate if root in the owner -var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} - -// HookVerifyResources checks ownership of dirs required for k8s to run. -// HookVerifyResources validates AppArmor configurations. -// If potential issue found pops up warning. -func HookVerifyResources() func(*cobra.Command, []string) { - return func(cmd *cobra.Command, args []string) { - var warnList []string - - // check ownership of required dirs - for _, path := range pathsOwnershipCheck { - if msg, err := validateRootOwnership(path); err != nil { - cmd.PrintErrf(err.Error()) - } else if len(msg) > 0 { - warnList = append(warnList, msg) - } - } - - // check App Armor - if armor, err := checkAppArmor(); err != nil { - cmd.PrintErr(err.Error()) - } else if len(armor) > 0 { - warnList = append(warnList, armor) - } - - // check LXD - if lxd, err := validateLXD(); err != nil { - cmd.PrintErr(err.Error()) - } else if len(warnList) > 0 && len(lxd) > 0 { - warnList = append(warnList, lxd) - - } - - // generate report - if len(warnList) > 0 { - cmd.PrintErrf("Warning: k8s may not run correctly due to reasons:\n%s", - strings.Join(warnList, "")) - } - } -} - -// validateLXD checks if k8s runs in lxd container if so returns link to documentation -func validateLXD() (string, error) { - dat, err := os.ReadFile(initialProcesEnvironmentVariables) - if err != nil { - return "", err - } - env := string(dat) - if strings.Contains(env, "container=lxc") { - return "For running k8s inside LXD container refer to " + - "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/.\n", nil - } - return "", nil -} - -// validateRootOwnership checks if given path owner root and root group. -func validateRootOwnership(path string) (string, error) { - - info, err := os.Stat(path) - if err != nil { - if os.IsNotExist(err) { - return fmt.Sprintf("%s do not exist.\n", path), nil - } else { - return "", err - } - } - var UID int - var GID int - if stat, ok := info.Sys().(*syscall.Stat_t); ok { - UID = int(stat.Uid) - GID = int(stat.Gid) - } else { - return "", errors.New(fmt.Sprintf("cannot access path %s", path)) - } - var warnList string - if UID != 0 { - warnList += fmt.Sprintf("owner of %s is user with UID %d expected 0.\n", path, UID) - } - if GID != 0 { - warnList += fmt.Sprintf("owner of %s is group with GID %d expected 0.\n", path, GID) - } - return warnList, nil -} - -// checkAppArmor checks AppArmor status. -func checkAppArmor() (string, error) { - cmd := exec.Command("journalctl", "-u", "apparmor") - out, err := cmd.CombinedOutput() - if err != nil { - return "", err - } - output := string(out) - // AppArmor configured for container or service not present - if strings.Contains(output, "Not starting AppArmor in container") || strings.Contains(output, "-- No entries --") { - return "", nil - // cannot read status of AppArmor - } else if strings.Contains(output, "Users in groups 'adm', 'systemd-journal' can see all messages.") { - return "could not validate AppArmor status.\n", nil - } - - return "AppArmor may block hosting of nested containers.\n", nil -} diff --git a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go index 4144a0f84..941775174 100644 --- a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go +++ b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go @@ -263,6 +263,8 @@ func (a *App) onBootstrapControlPlane(ctx context.Context, s state.State, bootst log := log.FromContext(ctx).WithValues("hook", "bootstrap") + utils.ReportRequiredResources() + cfg, err := types.ClusterConfigFromBootstrapConfig(bootstrapConfig) if err != nil { return fmt.Errorf("invalid bootstrap config: %w", err) diff --git a/src/k8s/pkg/k8sd/app/hooks_join.go b/src/k8s/pkg/k8sd/app/hooks_join.go index 0e164858f..ab73d2e02 100644 --- a/src/k8s/pkg/k8sd/app/hooks_join.go +++ b/src/k8s/pkg/k8sd/app/hooks_join.go @@ -26,6 +26,8 @@ func (a *App) onPostJoin(ctx context.Context, s state.State, initConfig map[stri // NOTE: Set the notBefore certificate time to the current time. notBefore := time.Now() + utils.ReportRequiredResources() + // NOTE(neoaggelos): context timeout is passed over configuration, so that hook failures are propagated to the client ctx, cancel := context.WithCancel(ctx) defer cancel() diff --git a/src/k8s/pkg/utils/file.go b/src/k8s/pkg/utils/file.go index c4d1dc79f..21a9c054d 100644 --- a/src/k8s/pkg/utils/file.go +++ b/src/k8s/pkg/utils/file.go @@ -9,11 +9,13 @@ import ( "io" "io/fs" "os" + "os/exec" "path" "path/filepath" "slices" "sort" "strings" + "syscall" "github.com/moby/sys/mountinfo" @@ -258,3 +260,109 @@ func CreateTarball(tarballPath string, rootDir string, walkDir string, excludeFi return nil } + +func getOwnership(path string) (int, int, error) { + info, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) { + return 0, 0, fmt.Errorf("%s do not exist", path) + } else { + return 0, 0, err + } + } + + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + return int(stat.Uid), int(stat.Gid), nil + } else { + return 0, 0, fmt.Errorf("cannot access %s", path) + } +} + +// validateRootOwnership checks if given path owner root and root group. +func validateRootOwnership(path string) error { + + UID, GID, err := getOwnership(path) + if err != nil { + return err + } + if UID != 0 { + + return fmt.Errorf("owner of %s is user with UID %d expected 0", path, UID) + } + if GID != 0 { + return fmt.Errorf("owner of %s is group with GID %d expected 0", path, GID) + } + return nil +} + +const initialProcesEnvironmentVariables = "/proc/1/environ" + +// validateLXD checks if k8s runs in lxd container if so returns link to documentation +func validateLXD() (bool, error) { + // can be replaced by Snap.OnLXD()? + dat, err := os.ReadFile(initialProcesEnvironmentVariables) + if err != nil { + if os.IsPermission(err) { + return false, fmt.Errorf("permission denied to %s", initialProcesEnvironmentVariables) + } + return false, err + } + env := string(dat) + if strings.Contains(env, "container=lxc") { + + return true, nil + } + return false, nil +} + +// checkAppArmor checks AppArmor status. +func checkAppArmor() error { + //todo move to proper file + cmd := exec.Command("journalctl", "-u", "apparmor") + out, err := cmd.CombinedOutput() + if err != nil { + return err + } + output := string(out) + // AppArmor configured for container or service not present + if strings.Contains(output, "Not starting AppArmor in container") || strings.Contains(output, "-- No entries --") { + return nil + // cannot read status of AppArmor + } else if strings.Contains(output, "Users in groups 'adm', 'systemd-journal' can see all messages.") { + return fmt.Errorf("could not validate AppArmor status") + } + + return fmt.Errorf("AppArmor may block hosting of nested containers") +} + +var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} + +func ReportRequiredResources() { + var errMsg []string + + // check ownership of required dirs + for _, pathToCheck := range pathsOwnershipCheck { + if err := validateRootOwnership(pathToCheck); err != nil { + errMsg = append(errMsg, err.Error()) + } + } + + // check App Armor + if err := checkAppArmor(); err != nil { + errMsg = append(errMsg, err.Error()) + } + + // printing report + if len(errMsg) > 0 { + if lxd, err := validateLXD(); err != nil { + errMsg = append(errMsg, err.Error()) + } else if lxd { + errMsg = append(errMsg, "For running k8s inside LXD container refer to "+ + "https://documentation.ubuntu.com/canonica-l-kubernetes/latest/snap/howto/install/lxd/") + } + for _, msg := range errMsg { + log.L().Info(fmt.Sprintf("Warning: %s", msg)) + } + } + +} From 925ccd5dde31f27cc2ba0ae52d13b135752829c1 Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Wed, 11 Sep 2024 22:43:30 +0200 Subject: [PATCH 04/11] k8s on lxd url fix --- src/k8s/pkg/utils/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/k8s/pkg/utils/file.go b/src/k8s/pkg/utils/file.go index 21a9c054d..e602317ee 100644 --- a/src/k8s/pkg/utils/file.go +++ b/src/k8s/pkg/utils/file.go @@ -358,7 +358,7 @@ func ReportRequiredResources() { errMsg = append(errMsg, err.Error()) } else if lxd { errMsg = append(errMsg, "For running k8s inside LXD container refer to "+ - "https://documentation.ubuntu.com/canonica-l-kubernetes/latest/snap/howto/install/lxd/") + "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/") } for _, msg := range errMsg { log.L().Info(fmt.Sprintf("Warning: %s", msg)) From 7b122e17cf42a6580cc88e0301c77f08541e94cc Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Thu, 12 Sep 2024 15:44:39 +0200 Subject: [PATCH 05/11] revert changes --- src/k8s/cmd/k8s/k8s_bootstrap.go | 2 +- src/k8s/cmd/k8s/k8s_join_cluster.go | 2 +- src/k8s/cmd/k8sd/k8sd.go | 5 +- src/k8s/cmd/util/hooks.go | 102 ++++++++++++++++++++++ src/k8s/pkg/k8sd/app/hooks_bootstrap.go | 2 - src/k8s/pkg/k8sd/app/hooks_join.go | 2 - src/k8s/pkg/utils/file.go | 108 ------------------------ 7 files changed, 107 insertions(+), 116 deletions(-) create mode 100644 src/k8s/cmd/util/hooks.go diff --git a/src/k8s/cmd/k8s/k8s_bootstrap.go b/src/k8s/cmd/k8s/k8s_bootstrap.go index b4243d824..c7fd521f5 100644 --- a/src/k8s/cmd/k8s/k8s_bootstrap.go +++ b/src/k8s/cmd/k8s/k8s_bootstrap.go @@ -45,7 +45,7 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { Use: "bootstrap", Short: "Bootstrap a new Kubernetes cluster", Long: "Generate certificates, configure service arguments and start the Kubernetes services.", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat)), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookVerifyResources(false)), Run: func(cmd *cobra.Command, args []string) { if opts.interactive && opts.configFile != "" { cmd.PrintErrln("Error: --interactive and --file flags cannot be set at the same time.") diff --git a/src/k8s/cmd/k8s/k8s_join_cluster.go b/src/k8s/cmd/k8s/k8s_join_cluster.go index 7507fedcb..ce6492253 100644 --- a/src/k8s/cmd/k8s/k8s_join_cluster.go +++ b/src/k8s/cmd/k8s/k8s_join_cluster.go @@ -32,7 +32,7 @@ func newJoinClusterCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ Use: "join-cluster ", Short: "Join a cluster using the provided token", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat)), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookVerifyResources(false)), Args: cmdutil.ExactArgs(env, 1), Run: func(cmd *cobra.Command, args []string) { token := args[0] diff --git a/src/k8s/cmd/k8sd/k8sd.go b/src/k8s/cmd/k8sd/k8sd.go index dedfafdf7..2481d02da 100644 --- a/src/k8s/cmd/k8sd/k8sd.go +++ b/src/k8s/cmd/k8sd/k8sd.go @@ -33,8 +33,9 @@ func addCommands(root *cobra.Command, group *cobra.Group, commands ...*cobra.Com func NewRootCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ - Use: "k8sd", - Short: "Canonical Kubernetes orchestrator and clustering daemon", + Use: "k8sd", + Short: "Canonical Kubernetes orchestrator and clustering daemon", + PreRun: cmdutil.HookVerifyResources(true), Run: func(cmd *cobra.Command, args []string) { // configure logging log.Configure(log.Options{ diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go new file mode 100644 index 000000000..8319a591d --- /dev/null +++ b/src/k8s/cmd/util/hooks.go @@ -0,0 +1,102 @@ +package cmdutil + +import ( + "fmt" + "os" + "strings" + "syscall" + + "github.com/spf13/cobra" +) + +const initialProcesEnvironmentVariables = "/proc/1/environ" + +// paths to validate if root in the owner +var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} + +// HookVerifyResources checks ownership of dirs required for k8s to run. +// HookVerifyResources if verbose true prints out list of potenital issues. +// If potential issue found pops up warning for user. +func HookVerifyResources(verbose bool) func(*cobra.Command, []string) { + return func(cmd *cobra.Command, args []string) { + errMsgs := VerifyPaths() + + // printing report + if len(errMsgs) > 0 { + if verbose { + cmd.PrintErrln("Warning: When validating required resources potential issues found:") + for _, errMsg := range errMsgs { + cmd.PrintErrln("\t", errMsg) + } + } + if lxd, err := validateLXD(); lxd { + cmd.PrintErrln("The lxc profile for MicroK8s might be missing.") + cmd.PrintErrln("For running k8s inside LXD container refer to " + + "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/") + } else if err != nil { + cmd.PrintErrf(err.Error()) + + } + } + } +} + +func VerifyPaths() []string { + var errMsg []string + // check ownership of required dirs + for _, pathToCheck := range pathsOwnershipCheck { + if err := validateRootOwnership(pathToCheck); err != nil { + errMsg = append(errMsg, err.Error()) + } + } + return errMsg +} + +func getOwnership(path string) (int, int, error) { + info, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) { + return 0, 0, fmt.Errorf("%s do not exist", path) + } else { + return 0, 0, err + } + } + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + return int(stat.Uid), int(stat.Gid), nil + } else { + return 0, 0, fmt.Errorf("cannot access %s", path) + } +} + +// validateRootOwnership checks if given path owner root and root group. +func validateRootOwnership(path string) error { + UID, GID, err := getOwnership(path) + if err != nil { + return err + } + if UID != 0 { + return fmt.Errorf("owner of %s is user with UID %d expected 0", path, UID) + } + if GID != 0 { + return fmt.Errorf("owner of %s is group with GID %d expected 0", path, GID) + } + return nil +} + +// validateLXD checks if k8s runs in lxd container if so returns link to documentation +func validateLXD() (bool, error) { + dat, err := os.ReadFile(initialProcesEnvironmentVariables) + if err != nil { + // if permission to file is missing we still want to display info about lxd + if os.IsPermission(err) { + return true, err + } + return false, err + } + env := string(dat) + if strings.Contains(env, "container=lxc") { + + return true, nil + } + return false, nil +} diff --git a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go index 941775174..4144a0f84 100644 --- a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go +++ b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go @@ -263,8 +263,6 @@ func (a *App) onBootstrapControlPlane(ctx context.Context, s state.State, bootst log := log.FromContext(ctx).WithValues("hook", "bootstrap") - utils.ReportRequiredResources() - cfg, err := types.ClusterConfigFromBootstrapConfig(bootstrapConfig) if err != nil { return fmt.Errorf("invalid bootstrap config: %w", err) diff --git a/src/k8s/pkg/k8sd/app/hooks_join.go b/src/k8s/pkg/k8sd/app/hooks_join.go index ab73d2e02..0e164858f 100644 --- a/src/k8s/pkg/k8sd/app/hooks_join.go +++ b/src/k8s/pkg/k8sd/app/hooks_join.go @@ -26,8 +26,6 @@ func (a *App) onPostJoin(ctx context.Context, s state.State, initConfig map[stri // NOTE: Set the notBefore certificate time to the current time. notBefore := time.Now() - utils.ReportRequiredResources() - // NOTE(neoaggelos): context timeout is passed over configuration, so that hook failures are propagated to the client ctx, cancel := context.WithCancel(ctx) defer cancel() diff --git a/src/k8s/pkg/utils/file.go b/src/k8s/pkg/utils/file.go index e602317ee..c4d1dc79f 100644 --- a/src/k8s/pkg/utils/file.go +++ b/src/k8s/pkg/utils/file.go @@ -9,13 +9,11 @@ import ( "io" "io/fs" "os" - "os/exec" "path" "path/filepath" "slices" "sort" "strings" - "syscall" "github.com/moby/sys/mountinfo" @@ -260,109 +258,3 @@ func CreateTarball(tarballPath string, rootDir string, walkDir string, excludeFi return nil } - -func getOwnership(path string) (int, int, error) { - info, err := os.Stat(path) - if err != nil { - if os.IsNotExist(err) { - return 0, 0, fmt.Errorf("%s do not exist", path) - } else { - return 0, 0, err - } - } - - if stat, ok := info.Sys().(*syscall.Stat_t); ok { - return int(stat.Uid), int(stat.Gid), nil - } else { - return 0, 0, fmt.Errorf("cannot access %s", path) - } -} - -// validateRootOwnership checks if given path owner root and root group. -func validateRootOwnership(path string) error { - - UID, GID, err := getOwnership(path) - if err != nil { - return err - } - if UID != 0 { - - return fmt.Errorf("owner of %s is user with UID %d expected 0", path, UID) - } - if GID != 0 { - return fmt.Errorf("owner of %s is group with GID %d expected 0", path, GID) - } - return nil -} - -const initialProcesEnvironmentVariables = "/proc/1/environ" - -// validateLXD checks if k8s runs in lxd container if so returns link to documentation -func validateLXD() (bool, error) { - // can be replaced by Snap.OnLXD()? - dat, err := os.ReadFile(initialProcesEnvironmentVariables) - if err != nil { - if os.IsPermission(err) { - return false, fmt.Errorf("permission denied to %s", initialProcesEnvironmentVariables) - } - return false, err - } - env := string(dat) - if strings.Contains(env, "container=lxc") { - - return true, nil - } - return false, nil -} - -// checkAppArmor checks AppArmor status. -func checkAppArmor() error { - //todo move to proper file - cmd := exec.Command("journalctl", "-u", "apparmor") - out, err := cmd.CombinedOutput() - if err != nil { - return err - } - output := string(out) - // AppArmor configured for container or service not present - if strings.Contains(output, "Not starting AppArmor in container") || strings.Contains(output, "-- No entries --") { - return nil - // cannot read status of AppArmor - } else if strings.Contains(output, "Users in groups 'adm', 'systemd-journal' can see all messages.") { - return fmt.Errorf("could not validate AppArmor status") - } - - return fmt.Errorf("AppArmor may block hosting of nested containers") -} - -var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} - -func ReportRequiredResources() { - var errMsg []string - - // check ownership of required dirs - for _, pathToCheck := range pathsOwnershipCheck { - if err := validateRootOwnership(pathToCheck); err != nil { - errMsg = append(errMsg, err.Error()) - } - } - - // check App Armor - if err := checkAppArmor(); err != nil { - errMsg = append(errMsg, err.Error()) - } - - // printing report - if len(errMsg) > 0 { - if lxd, err := validateLXD(); err != nil { - errMsg = append(errMsg, err.Error()) - } else if lxd { - errMsg = append(errMsg, "For running k8s inside LXD container refer to "+ - "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/") - } - for _, msg := range errMsg { - log.L().Info(fmt.Sprintf("Warning: %s", msg)) - } - } - -} From 3a0b8354a56bbcecd11030585b1e1a8ad9fd0d80 Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Fri, 13 Sep 2024 10:27:19 +0200 Subject: [PATCH 06/11] review fixes --- src/k8s/cmd/k8s/k8s_bootstrap.go | 2 +- src/k8s/cmd/k8s/k8s_join_cluster.go | 2 +- src/k8s/cmd/k8sd/k8sd.go | 2 +- src/k8s/cmd/util/hooks.go | 35 +++++++++++++---------------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/k8s/cmd/k8s/k8s_bootstrap.go b/src/k8s/cmd/k8s/k8s_bootstrap.go index c7fd521f5..ad74c95e6 100644 --- a/src/k8s/cmd/k8s/k8s_bootstrap.go +++ b/src/k8s/cmd/k8s/k8s_bootstrap.go @@ -45,7 +45,7 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { Use: "bootstrap", Short: "Bootstrap a new Kubernetes cluster", Long: "Generate certificates, configure service arguments and start the Kubernetes services.", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookVerifyResources(false)), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookCheckLXD(false)), Run: func(cmd *cobra.Command, args []string) { if opts.interactive && opts.configFile != "" { cmd.PrintErrln("Error: --interactive and --file flags cannot be set at the same time.") diff --git a/src/k8s/cmd/k8s/k8s_join_cluster.go b/src/k8s/cmd/k8s/k8s_join_cluster.go index ce6492253..3e54cae68 100644 --- a/src/k8s/cmd/k8s/k8s_join_cluster.go +++ b/src/k8s/cmd/k8s/k8s_join_cluster.go @@ -32,7 +32,7 @@ func newJoinClusterCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ Use: "join-cluster ", Short: "Join a cluster using the provided token", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookVerifyResources(false)), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookCheckLXD(false)), Args: cmdutil.ExactArgs(env, 1), Run: func(cmd *cobra.Command, args []string) { token := args[0] diff --git a/src/k8s/cmd/k8sd/k8sd.go b/src/k8s/cmd/k8sd/k8sd.go index 2481d02da..7e78a6d74 100644 --- a/src/k8s/cmd/k8sd/k8sd.go +++ b/src/k8s/cmd/k8sd/k8sd.go @@ -35,7 +35,7 @@ func NewRootCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ Use: "k8sd", Short: "Canonical Kubernetes orchestrator and clustering daemon", - PreRun: cmdutil.HookVerifyResources(true), + PreRun: cmdutil.HookCheckLXD(true), Run: func(cmd *cobra.Command, args []string) { // configure logging log.Configure(log.Options{ diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go index 8319a591d..52dbe11ec 100644 --- a/src/k8s/cmd/util/hooks.go +++ b/src/k8s/cmd/util/hooks.go @@ -14,29 +14,26 @@ const initialProcesEnvironmentVariables = "/proc/1/environ" // paths to validate if root in the owner var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} -// HookVerifyResources checks ownership of dirs required for k8s to run. -// HookVerifyResources if verbose true prints out list of potenital issues. +// HookCheckLXD checks ownership of dirs required for k8s to run. +// HookCheckLXD if verbose true prints out list of potenital issues. // If potential issue found pops up warning for user. -func HookVerifyResources(verbose bool) func(*cobra.Command, []string) { +func HookCheckLXD(verbose bool) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { - errMsgs := VerifyPaths() - - // printing report - if len(errMsgs) > 0 { - if verbose { - cmd.PrintErrln("Warning: When validating required resources potential issues found:") - for _, errMsg := range errMsgs { - cmd.PrintErrln("\t", errMsg) + if lxd, err := inLXDContainer(); lxd { + errMsgs := VerifyPaths() + if len(errMsgs) > 0 { + if verbose { + cmd.PrintErrln("Warning: When validating required resources potential issues found:") + for _, errMsg := range errMsgs { + cmd.PrintErrln("\t", errMsg) + } } - } - if lxd, err := validateLXD(); lxd { - cmd.PrintErrln("The lxc profile for MicroK8s might be missing.") + cmd.PrintErrln("The lxc profile for Canonical Kubernetes might be missing.") cmd.PrintErrln("For running k8s inside LXD container refer to " + "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/") - } else if err != nil { - cmd.PrintErrf(err.Error()) - } + } else if err != nil { + cmd.PrintErrf(err.Error()) } } } @@ -83,8 +80,8 @@ func validateRootOwnership(path string) error { return nil } -// validateLXD checks if k8s runs in lxd container if so returns link to documentation -func validateLXD() (bool, error) { +// inLXDContainer checks if k8s runs in lxd container if so returns link to documentation +func inLXDContainer() (bool, error) { dat, err := os.ReadFile(initialProcesEnvironmentVariables) if err != nil { // if permission to file is missing we still want to display info about lxd From 061ed37431a444f182e6abf85c09884dd59dd25c Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Fri, 13 Sep 2024 14:30:58 +0200 Subject: [PATCH 07/11] review fixes --- src/k8s/cmd/k8s/hooks.go | 89 ++++++++++++++++++++++++++ src/k8s/cmd/k8s/k8s_bootstrap.go | 2 +- src/k8s/cmd/k8s/k8s_join_cluster.go | 2 +- src/k8s/cmd/k8sd/k8sd.go | 5 +- src/k8s/cmd/util/hooks.go | 99 ----------------------------- 5 files changed, 93 insertions(+), 104 deletions(-) delete mode 100644 src/k8s/cmd/util/hooks.go diff --git a/src/k8s/cmd/k8s/hooks.go b/src/k8s/cmd/k8s/hooks.go index 481d63df6..e89f94472 100644 --- a/src/k8s/cmd/k8s/hooks.go +++ b/src/k8s/cmd/k8s/hooks.go @@ -1,10 +1,22 @@ package k8s import ( + "fmt" + "os" + "strings" + "syscall" + cmdutil "github.com/canonical/k8s/cmd/util" + "github.com/spf13/cobra" ) +// initialProcessEnvironmentVariables environment variables of initial process +const initialProcessEnvironmentVariables = "/proc/1/environ" + +// pathsOwnershipCheck paths to validate root is the ownership +var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} + func chainPreRunHooks(hooks ...func(*cobra.Command, []string)) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { for _, hook := range hooks { @@ -34,3 +46,80 @@ func hookInitializeFormatter(env cmdutil.ExecutionEnvironment, format *string) f } } } + +// hookCheckLXD checks ownership of directories required for k8s to run. +// If potential issue found pops up warning for user. +func hookCheckLXD() func(*cobra.Command, []string) { + return func(cmd *cobra.Command, args []string) { + if lxd, err := inLXDContainer(); lxd { + var errMsgs []string + for _, pathToCheck := range pathsOwnershipCheck { + if err2 := validateRootOwnership(pathToCheck); err2 != nil { + errMsgs = append(errMsgs, err2.Error()) + } + } + if len(errMsgs) > 0 { + if debug, _ := cmd.Flags().GetBool("debug"); debug { + cmd.PrintErrln("Warning: When validating required resources potential issues found:") + for _, errMsg := range errMsgs { + cmd.PrintErrln("\t", errMsg) + } + } + cmd.PrintErrln("The lxc profile for Canonical Kubernetes might be missing.") + cmd.PrintErrln("For running k8s inside LXD container refer to " + + "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/") + } + } else if err != nil { + cmd.PrintErrf(err.Error()) + } + } +} + +// getOwnership given path of file returns UID, GID and error. +func getOwnership(path string) (int, int, error) { + info, err := os.Stat(path) + if err != nil { + if os.IsNotExist(err) { + return 0, 0, fmt.Errorf("%s do not exist", path) + } else { + return 0, 0, err + } + } + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + return int(stat.Uid), int(stat.Gid), nil + } else { + return 0, 0, fmt.Errorf("cannot access %s", path) + } +} + +// validateRootOwnership checks if given path owner root and root group. +func validateRootOwnership(path string) error { + UID, GID, err := getOwnership(path) + if err != nil { + return err + } + if UID != 0 { + return fmt.Errorf("owner of %s is user with UID %d expected 0", path, UID) + } + if GID != 0 { + return fmt.Errorf("owner of %s is group with GID %d expected 0", path, GID) + } + return nil +} + +// inLXDContainer checks if k8s runs in lxd container if so returns link to documentation +func inLXDContainer() (bool, error) { + content, err := os.ReadFile(initialProcessEnvironmentVariables) + if err != nil { + // if permission to file is missing we still want to display info about lxd + if os.IsPermission(err) { + return true, err + } + return false, err + } + env := string(content) + if strings.Contains(env, "container=lxc") { + return true, nil + } + return false, nil +} diff --git a/src/k8s/cmd/k8s/k8s_bootstrap.go b/src/k8s/cmd/k8s/k8s_bootstrap.go index ad74c95e6..5510ed34e 100644 --- a/src/k8s/cmd/k8s/k8s_bootstrap.go +++ b/src/k8s/cmd/k8s/k8s_bootstrap.go @@ -45,7 +45,7 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { Use: "bootstrap", Short: "Bootstrap a new Kubernetes cluster", Long: "Generate certificates, configure service arguments and start the Kubernetes services.", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookCheckLXD(false)), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), hookCheckLXD()), Run: func(cmd *cobra.Command, args []string) { if opts.interactive && opts.configFile != "" { cmd.PrintErrln("Error: --interactive and --file flags cannot be set at the same time.") diff --git a/src/k8s/cmd/k8s/k8s_join_cluster.go b/src/k8s/cmd/k8s/k8s_join_cluster.go index 3e54cae68..4cd5bfe6d 100644 --- a/src/k8s/cmd/k8s/k8s_join_cluster.go +++ b/src/k8s/cmd/k8s/k8s_join_cluster.go @@ -32,7 +32,7 @@ func newJoinClusterCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ Use: "join-cluster ", Short: "Join a cluster using the provided token", - PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), cmdutil.HookCheckLXD(false)), + PreRun: chainPreRunHooks(hookRequireRoot(env), hookInitializeFormatter(env, &opts.outputFormat), hookCheckLXD()), Args: cmdutil.ExactArgs(env, 1), Run: func(cmd *cobra.Command, args []string) { token := args[0] diff --git a/src/k8s/cmd/k8sd/k8sd.go b/src/k8s/cmd/k8sd/k8sd.go index 7e78a6d74..dedfafdf7 100644 --- a/src/k8s/cmd/k8sd/k8sd.go +++ b/src/k8s/cmd/k8sd/k8sd.go @@ -33,9 +33,8 @@ func addCommands(root *cobra.Command, group *cobra.Group, commands ...*cobra.Com func NewRootCmd(env cmdutil.ExecutionEnvironment) *cobra.Command { cmd := &cobra.Command{ - Use: "k8sd", - Short: "Canonical Kubernetes orchestrator and clustering daemon", - PreRun: cmdutil.HookCheckLXD(true), + Use: "k8sd", + Short: "Canonical Kubernetes orchestrator and clustering daemon", Run: func(cmd *cobra.Command, args []string) { // configure logging log.Configure(log.Options{ diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go deleted file mode 100644 index 52dbe11ec..000000000 --- a/src/k8s/cmd/util/hooks.go +++ /dev/null @@ -1,99 +0,0 @@ -package cmdutil - -import ( - "fmt" - "os" - "strings" - "syscall" - - "github.com/spf13/cobra" -) - -const initialProcesEnvironmentVariables = "/proc/1/environ" - -// paths to validate if root in the owner -var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} - -// HookCheckLXD checks ownership of dirs required for k8s to run. -// HookCheckLXD if verbose true prints out list of potenital issues. -// If potential issue found pops up warning for user. -func HookCheckLXD(verbose bool) func(*cobra.Command, []string) { - return func(cmd *cobra.Command, args []string) { - if lxd, err := inLXDContainer(); lxd { - errMsgs := VerifyPaths() - if len(errMsgs) > 0 { - if verbose { - cmd.PrintErrln("Warning: When validating required resources potential issues found:") - for _, errMsg := range errMsgs { - cmd.PrintErrln("\t", errMsg) - } - } - cmd.PrintErrln("The lxc profile for Canonical Kubernetes might be missing.") - cmd.PrintErrln("For running k8s inside LXD container refer to " + - "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/") - } - } else if err != nil { - cmd.PrintErrf(err.Error()) - } - } -} - -func VerifyPaths() []string { - var errMsg []string - // check ownership of required dirs - for _, pathToCheck := range pathsOwnershipCheck { - if err := validateRootOwnership(pathToCheck); err != nil { - errMsg = append(errMsg, err.Error()) - } - } - return errMsg -} - -func getOwnership(path string) (int, int, error) { - info, err := os.Stat(path) - if err != nil { - if os.IsNotExist(err) { - return 0, 0, fmt.Errorf("%s do not exist", path) - } else { - return 0, 0, err - } - } - if stat, ok := info.Sys().(*syscall.Stat_t); ok { - return int(stat.Uid), int(stat.Gid), nil - } else { - return 0, 0, fmt.Errorf("cannot access %s", path) - } -} - -// validateRootOwnership checks if given path owner root and root group. -func validateRootOwnership(path string) error { - UID, GID, err := getOwnership(path) - if err != nil { - return err - } - if UID != 0 { - return fmt.Errorf("owner of %s is user with UID %d expected 0", path, UID) - } - if GID != 0 { - return fmt.Errorf("owner of %s is group with GID %d expected 0", path, GID) - } - return nil -} - -// inLXDContainer checks if k8s runs in lxd container if so returns link to documentation -func inLXDContainer() (bool, error) { - dat, err := os.ReadFile(initialProcesEnvironmentVariables) - if err != nil { - // if permission to file is missing we still want to display info about lxd - if os.IsPermission(err) { - return true, err - } - return false, err - } - env := string(dat) - if strings.Contains(env, "container=lxc") { - - return true, nil - } - return false, nil -} From 4eb2a79407b0809b9949187d3baef78eb96bc67a Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Fri, 13 Sep 2024 14:32:52 +0200 Subject: [PATCH 08/11] review fixes --- src/k8s/cmd/k8s/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/k8s/cmd/k8s/hooks.go b/src/k8s/cmd/k8s/hooks.go index e89f94472..ad37d1767 100644 --- a/src/k8s/cmd/k8s/hooks.go +++ b/src/k8s/cmd/k8s/hooks.go @@ -107,7 +107,7 @@ func validateRootOwnership(path string) error { return nil } -// inLXDContainer checks if k8s runs in lxd container if so returns link to documentation +// inLXDContainer checks if k8s runs in lxd container. func inLXDContainer() (bool, error) { content, err := os.ReadFile(initialProcessEnvironmentVariables) if err != nil { From 00ef3250ad8c7d0f810544d264efadcf3af74ca3 Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Tue, 17 Sep 2024 09:33:46 +0200 Subject: [PATCH 09/11] review fixes --- src/k8s/cmd/k8s/hooks.go | 78 ++++++--------------------------------- src/k8s/cmd/util/hooks.go | 57 ++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 66 deletions(-) create mode 100644 src/k8s/cmd/util/hooks.go diff --git a/src/k8s/cmd/k8s/hooks.go b/src/k8s/cmd/k8s/hooks.go index ad37d1767..b597cba42 100644 --- a/src/k8s/cmd/k8s/hooks.go +++ b/src/k8s/cmd/k8s/hooks.go @@ -1,22 +1,11 @@ package k8s import ( - "fmt" - "os" - "strings" - "syscall" - cmdutil "github.com/canonical/k8s/cmd/util" "github.com/spf13/cobra" ) -// initialProcessEnvironmentVariables environment variables of initial process -const initialProcessEnvironmentVariables = "/proc/1/environ" - -// pathsOwnershipCheck paths to validate root is the ownership -var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} - func chainPreRunHooks(hooks ...func(*cobra.Command, []string)) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { for _, hook := range hooks { @@ -47,14 +36,21 @@ func hookInitializeFormatter(env cmdutil.ExecutionEnvironment, format *string) f } } -// hookCheckLXD checks ownership of directories required for k8s to run. -// If potential issue found pops up warning for user. +// hookCheckLXD verifies the ownership of directories needed for Kubernetes to function. +// If a potential issue is detected, it displays a warning to the user. func hookCheckLXD() func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { - if lxd, err := inLXDContainer(); lxd { + // pathsOwnershipCheck paths to validate root is the owner + var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} + inLXD, err := cmdutil.InLXDContainer() + if err != nil { + cmd.PrintErrf("Failed to check if running inside LXD container: %w", err) + return + } + if inLXD { var errMsgs []string for _, pathToCheck := range pathsOwnershipCheck { - if err2 := validateRootOwnership(pathToCheck); err2 != nil { + if err2 := cmdutil.ValidateRootOwnership(pathToCheck); err2 != nil { errMsgs = append(errMsgs, err2.Error()) } } @@ -69,57 +65,7 @@ func hookCheckLXD() func(*cobra.Command, []string) { cmd.PrintErrln("For running k8s inside LXD container refer to " + "https://documentation.ubuntu.com/canonical-kubernetes/latest/snap/howto/install/lxd/") } - } else if err != nil { - cmd.PrintErrf(err.Error()) } + return } } - -// getOwnership given path of file returns UID, GID and error. -func getOwnership(path string) (int, int, error) { - info, err := os.Stat(path) - if err != nil { - if os.IsNotExist(err) { - return 0, 0, fmt.Errorf("%s do not exist", path) - } else { - return 0, 0, err - } - } - if stat, ok := info.Sys().(*syscall.Stat_t); ok { - return int(stat.Uid), int(stat.Gid), nil - } else { - return 0, 0, fmt.Errorf("cannot access %s", path) - } -} - -// validateRootOwnership checks if given path owner root and root group. -func validateRootOwnership(path string) error { - UID, GID, err := getOwnership(path) - if err != nil { - return err - } - if UID != 0 { - return fmt.Errorf("owner of %s is user with UID %d expected 0", path, UID) - } - if GID != 0 { - return fmt.Errorf("owner of %s is group with GID %d expected 0", path, GID) - } - return nil -} - -// inLXDContainer checks if k8s runs in lxd container. -func inLXDContainer() (bool, error) { - content, err := os.ReadFile(initialProcessEnvironmentVariables) - if err != nil { - // if permission to file is missing we still want to display info about lxd - if os.IsPermission(err) { - return true, err - } - return false, err - } - env := string(content) - if strings.Contains(env, "container=lxc") { - return true, nil - } - return false, nil -} diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go new file mode 100644 index 000000000..4c9095b49 --- /dev/null +++ b/src/k8s/cmd/util/hooks.go @@ -0,0 +1,57 @@ +package cmdutil + +import ( + "fmt" + "os" + "strings" + "syscall" +) + +// getFileOwnerAndGroup retrieves the UID and GID of a file. +func getFileOwnerAndGroup(filePath string) (uid, gid uint32, err error) { + // Get file info using os.Stat + fileInfo, err := os.Stat(filePath) + if err != nil { + return 0, 0, fmt.Errorf("error getting file info: %w", err) + } + // Convert the fileInfo.Sys() to syscall.Stat_t to access UID and GID + stat, ok := fileInfo.Sys().(*syscall.Stat_t) + if !ok { + return 0, 0, fmt.Errorf("failed to cast to syscall.Stat_t") + } + // Return the UID and GID + return stat.Uid, stat.Gid, nil +} + +// ValidateRootOwnership checks if given path owner root and root group. +func ValidateRootOwnership(path string) (err error) { + UID, GID, err := getFileOwnerAndGroup(path) + if err != nil { + return err + } + if UID != 0 { + return fmt.Errorf("owner of %s is user with UID %d expected 0", path, UID) + } + if GID != 0 { + return fmt.Errorf("owner of %s is group with GID %d expected 0", path, GID) + } + return nil +} + +// InLXDContainer checks if k8s runs in lxd container. +func InLXDContainer() (isLXD bool, err error) { + + initialProcessEnvironmentVariables := "/proc/1/environ" + content, err := os.ReadFile(initialProcessEnvironmentVariables) + if err != nil { + // if the permission to file is missing we still want to display info about lxd + if os.IsPermission(err) { + return true, fmt.Errorf("cannnot access %s to check if runing in LXD container: %w", initialProcessEnvironmentVariables, err) + } + return false, fmt.Errorf("cannnot read %s to check if runing in LXD container: %w", initialProcessEnvironmentVariables, err) + } + if strings.Contains(string(content), "container=lxc") { + return true, nil + } + return false, nil +} From 50ef96091dd173367010a87b9549b30210ca5964 Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Tue, 17 Sep 2024 11:58:07 +0200 Subject: [PATCH 10/11] review fixes --- src/k8s/cmd/k8s/hooks.go | 4 ++-- src/k8s/cmd/util/hooks.go | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/k8s/cmd/k8s/hooks.go b/src/k8s/cmd/k8s/hooks.go index b597cba42..08782a092 100644 --- a/src/k8s/cmd/k8s/hooks.go +++ b/src/k8s/cmd/k8s/hooks.go @@ -50,8 +50,8 @@ func hookCheckLXD() func(*cobra.Command, []string) { if inLXD { var errMsgs []string for _, pathToCheck := range pathsOwnershipCheck { - if err2 := cmdutil.ValidateRootOwnership(pathToCheck); err2 != nil { - errMsgs = append(errMsgs, err2.Error()) + if err = cmdutil.ValidateRootOwnership(pathToCheck); err != nil { + errMsgs = append(errMsgs, err.Error()) } } if len(errMsgs) > 0 { diff --git a/src/k8s/cmd/util/hooks.go b/src/k8s/cmd/util/hooks.go index 4c9095b49..a02dc64c3 100644 --- a/src/k8s/cmd/util/hooks.go +++ b/src/k8s/cmd/util/hooks.go @@ -23,7 +23,7 @@ func getFileOwnerAndGroup(filePath string) (uid, gid uint32, err error) { return stat.Uid, stat.Gid, nil } -// ValidateRootOwnership checks if given path owner root and root group. +// ValidateRootOwnership checks if the specified path is owned by the root user and root group. func ValidateRootOwnership(path string) (err error) { UID, GID, err := getFileOwnerAndGroup(path) if err != nil { @@ -38,9 +38,8 @@ func ValidateRootOwnership(path string) (err error) { return nil } -// InLXDContainer checks if k8s runs in lxd container. +// InLXDContainer checks if k8s runs in a lxd container. func InLXDContainer() (isLXD bool, err error) { - initialProcessEnvironmentVariables := "/proc/1/environ" content, err := os.ReadFile(initialProcessEnvironmentVariables) if err != nil { From 7b140546f2f0ed2a9ea10b4197bc4d0c3106f5c4 Mon Sep 17 00:00:00 2001 From: Maciej Golaszewski Date: Tue, 17 Sep 2024 16:06:17 +0200 Subject: [PATCH 11/11] vet fix --- src/k8s/cmd/k8s/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/k8s/cmd/k8s/hooks.go b/src/k8s/cmd/k8s/hooks.go index 08782a092..6b97bd16d 100644 --- a/src/k8s/cmd/k8s/hooks.go +++ b/src/k8s/cmd/k8s/hooks.go @@ -44,7 +44,7 @@ func hookCheckLXD() func(*cobra.Command, []string) { var pathsOwnershipCheck = []string{"/sys", "/proc", "/dev/kmsg"} inLXD, err := cmdutil.InLXDContainer() if err != nil { - cmd.PrintErrf("Failed to check if running inside LXD container: %w", err) + cmd.PrintErrf("Failed to check if running inside LXD container: %s", err.Error()) return } if inLXD {