From 36e90303af51fda9bba51f4611b86cdf0224d5e9 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 9 Apr 2021 23:19:16 +0300 Subject: [PATCH] feat: validate the machine configuration in the installer container Talos validates machine configuration at boot time, and refuses to boot if machine configuration is invalid. As machine configuration validation rules might change over time, we need to prevent a scenario when after an upgrade machine configuration becomes invalid, as there's no way to roll back properly. Machine configuration is submitted over stdin to the installer container, and installer container validates it using the new version of Talos (which is going to be installed). If the config is not sent over stdin, installer assumes old version of Talos and proceeds. This should be backported to 0.9 to allow config validation on upgrade to 0.10. Fixes #3419 Signed-off-by: Andrey Smirnov --- .../app/machined/internal/install/install.go | 18 +++++++-- .../v1alpha1/v1alpha1_sequencer_tasks.go | 13 +++++++ .../system/runner/containerd/containerd.go | 32 +++------------- .../pkg/system/runner/containerd/stdin.go | 38 +++++++++++++++++++ 4 files changed, 71 insertions(+), 30 deletions(-) create mode 100644 internal/app/machined/pkg/system/runner/containerd/stdin.go diff --git a/internal/app/machined/internal/install/install.go b/internal/app/machined/internal/install/install.go index 6763c0ccbad..ee778479488 100644 --- a/internal/app/machined/internal/install/install.go +++ b/internal/app/machined/internal/install/install.go @@ -5,6 +5,7 @@ package install import ( + "bytes" "context" "fmt" "log" @@ -21,6 +22,7 @@ import ( "golang.org/x/sys/unix" "github.com/talos-systems/talos/internal/app/machined/pkg/runtime" + containerdrunner "github.com/talos-systems/talos/internal/app/machined/pkg/system/runner/containerd" "github.com/talos-systems/talos/internal/pkg/containers/image" "github.com/talos-systems/talos/internal/pkg/kmsg" machineapi "github.com/talos-systems/talos/pkg/machinery/api/machine" @@ -31,7 +33,7 @@ import ( // RunInstallerContainer performs an installation via the installer container. // //nolint:gocyclo -func RunInstallerContainer(disk, platform, ref string, reg config.Registries, opts ...Option) error { +func RunInstallerContainer(disk, platform, ref string, configBytes []byte, reg config.Registries, opts ...Option) error { options := DefaultInstallOptions() for _, opt := range opts { @@ -40,7 +42,10 @@ func RunInstallerContainer(disk, platform, ref string, reg config.Registries, op } } - ctx := namespaces.WithNamespace(context.Background(), constants.SystemContainerdNamespace) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + ctx = namespaces.WithNamespace(ctx, constants.SystemContainerdNamespace) client, err := containerd.New(constants.SystemContainerdAddress) if err != nil { @@ -134,13 +139,20 @@ func RunInstallerContainer(disk, platform, ref string, reg config.Registries, op w := &kmsg.Writer{KmsgWriter: f} - creator := cio.NewCreator(cio.WithStreams(nil, w, w)) + configR := &containerdrunner.StdinCloser{ + Stdin: bytes.NewReader(configBytes), + Closer: make(chan struct{}), + } + + creator := cio.NewCreator(cio.WithStreams(configR, w, w)) t, err := container.NewTask(ctx, creator) if err != nil { return err } + go configR.WaitAndClose(ctx, t) + defer t.Delete(ctx) //nolint:errcheck if err = t.Start(ctx); err != nil { diff --git a/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go b/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go index c50fb96db98..5632a84278a 100644 --- a/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go +++ b/internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go @@ -1369,11 +1369,17 @@ func Upgrade(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFunc, logger.Printf("performing upgrade via %q", in.GetImage()) + configBytes, err := r.Config().Bytes() + if err != nil { + return fmt.Errorf("error marshaling configuration: %w", err) + } + // We pull the installer image when we receive an upgrade request. No need // to pull it again. err = install.RunInstallerContainer( devname, r.State().Platform().Name(), in.GetImage(), + configBytes, r.Config().Machine().Registries(), install.OptionsFromUpgradeRequest(r, in)..., ) @@ -1596,6 +1602,11 @@ func UnmountEphemeralPartition(seq runtime.Sequence, data interface{}) (runtime. // Install mounts or installs the system partitions. func Install(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFunc, string) { return func(ctx context.Context, logger *log.Logger, r runtime.Runtime) (err error) { + configBytes, err := r.Config().Bytes() + if err != nil { + return fmt.Errorf("error marshaling configuration: %w", err) + } + switch { case !r.State().Machine().Installed(): installerImage := r.Config().Machine().Install().Image() @@ -1614,6 +1625,7 @@ func Install(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFunc, disk, r.State().Platform().Name(), installerImage, + configBytes, r.Config().Machine().Registries(), install.WithForce(true), install.WithZero(r.Config().Machine().Install().Zero()), @@ -1639,6 +1651,7 @@ func Install(seq runtime.Sequence, data interface{}) (runtime.TaskExecutionFunc, err = install.RunInstallerContainer( devname, r.State().Platform().Name(), r.State().Machine().StagedInstallImageRef(), + configBytes, r.Config().Machine().Registries(), install.WithOptions(options), ) diff --git a/internal/app/machined/pkg/system/runner/containerd/containerd.go b/internal/app/machined/pkg/system/runner/containerd/containerd.go index 0acd1d8bd9c..f51accb5512 100644 --- a/internal/app/machined/pkg/system/runner/containerd/containerd.go +++ b/internal/app/machined/pkg/system/runner/containerd/containerd.go @@ -34,7 +34,7 @@ type containerdRunner struct { client *containerd.Client ctx context.Context container containerd.Container - stdinCloser *stdinCloser + stdinCloser *StdinCloser } // NewRunner creates runner.Runner that runs a container in containerd. @@ -156,15 +156,7 @@ func (c *containerdRunner) Run(eventSink events.Recorder) error { if r != nil { // See https://github.com/containerd/containerd/issues/4489. - go func() { - select { - case <-c.ctx.Done(): - return - case <-c.stdinCloser.closer: - //nolint:errcheck - task.CloseIO(c.ctx, containerd.WithStdinCloser) - } - }() + go c.stdinCloser.WaitAndClose(c.ctx, task) } defer task.Delete(c.ctx) //nolint:errcheck @@ -291,24 +283,10 @@ func (c *containerdRunner) StdinReader() (io.Reader, error) { return nil, err } - c.stdinCloser = &stdinCloser{ - stdin: bytes.NewReader(contents), - closer: make(chan struct{}), + c.stdinCloser = &StdinCloser{ + Stdin: bytes.NewReader(contents), + Closer: make(chan struct{}), } return c.stdinCloser, nil } - -type stdinCloser struct { - stdin io.Reader - closer chan struct{} -} - -func (s *stdinCloser) Read(p []byte) (int, error) { - n, err := s.stdin.Read(p) - if err == io.EOF { - close(s.closer) - } - - return n, err -} diff --git a/internal/app/machined/pkg/system/runner/containerd/stdin.go b/internal/app/machined/pkg/system/runner/containerd/stdin.go new file mode 100644 index 00000000000..d24bcf82288 --- /dev/null +++ b/internal/app/machined/pkg/system/runner/containerd/stdin.go @@ -0,0 +1,38 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package containerd + +import ( + "context" + "io" + + "github.com/containerd/containerd" +) + +// StdinCloser wraps io.Reader providing a signal when reader is read till EOF. +type StdinCloser struct { + Stdin io.Reader + Closer chan struct{} +} + +func (s *StdinCloser) Read(p []byte) (int, error) { + n, err := s.Stdin.Read(p) + if err == io.EOF { + close(s.Closer) + } + + return n, err +} + +// WaitAndClose closes containerd task stdin when StdinCloser is exhausted. +func (s *StdinCloser) WaitAndClose(ctx context.Context, task containerd.Task) { + select { + case <-ctx.Done(): + return + case <-s.Closer: + //nolint:errcheck + task.CloseIO(ctx, containerd.WithStdinCloser) + } +}