From b55ce0439114b52c0c8266d28c1a14dc5354267f Mon Sep 17 00:00:00 2001 From: AtomicFS Date: Fri, 24 Jan 2025 16:01:38 +0100 Subject: [PATCH 1/2] chore(cmd): remove interactivity via SSH - Dagger since v0.12 supports much better interactive interface than our make-shift SSH solutions provides - as such we are dropping the SSH feature Signed-off-by: AtomicFS --- cmd/firmware-action/container/ssh.go | 190 ------------------ cmd/firmware-action/container/ssh_test.go | 151 -------------- cmd/firmware-action/main.go | 7 +- cmd/firmware-action/recipes/config.go | 2 +- cmd/firmware-action/recipes/coreboot.go | 20 +- cmd/firmware-action/recipes/coreboot_test.go | 8 +- cmd/firmware-action/recipes/edk2.go | 12 +- cmd/firmware-action/recipes/edk2_test.go | 2 +- cmd/firmware-action/recipes/linux.go | 18 +- cmd/firmware-action/recipes/linux_test.go | 2 +- cmd/firmware-action/recipes/recipes.go | 20 +- cmd/firmware-action/recipes/recipes_test.go | 13 +- cmd/firmware-action/recipes/stitching.go | 31 ++- cmd/firmware-action/recipes/stitching_test.go | 2 +- cmd/firmware-action/recipes/universal.go | 10 +- cmd/firmware-action/recipes/universal_test.go | 2 +- cmd/firmware-action/recipes/uroot.go | 10 +- cmd/firmware-action/recipes/uroot_test.go | 2 +- 18 files changed, 66 insertions(+), 436 deletions(-) delete mode 100644 cmd/firmware-action/container/ssh.go delete mode 100644 cmd/firmware-action/container/ssh_test.go diff --git a/cmd/firmware-action/container/ssh.go b/cmd/firmware-action/container/ssh.go deleted file mode 100644 index 89b2d2b6..00000000 --- a/cmd/firmware-action/container/ssh.go +++ /dev/null @@ -1,190 +0,0 @@ -// SPDX-License-Identifier: MIT - -// Package container for dealing with containers via dagger -package container - -import ( - "context" - "errors" - "fmt" - "log" - "math/rand" - "strings" - "sync" - - "dagger.io/dagger" -) - -// ErrParseAddress is raised when containers IP address could not be parsed -var ErrParseAddress = errors.New("could not parse address string") - -// OptsOpenSSH stores options for SSH tunnel for OpenSSH function -type OptsOpenSSH struct { - WaitFunc func() // Waiting function holding container with SSH running - Password string // Filled in by OpenSSH function - IPv4 string // Filled in by OpenSSH function - Port string // Filled in by OpenSSH function - MutexData *sync.Mutex // Mutex for modifying data in this struct - - // We could do with single channel here, but for clarity and less mental overhead there are 2 - TunnelClose chan (bool) // Channel to signal that SSH tunnel is ready - TunnelReady chan (bool) // Channel to signal that SSH tunnel is not longer needed and can be closed -} - -// Wait calls WaitFunc -func (s OptsOpenSSH) Wait() { - s.WaitFunc() -} - -// Address function parses provided string and populates IPv4 and Port (port defaults to 22 if not found) -func (s *OptsOpenSSH) Address(address string) error { - s.MutexData.Lock() - var err error - - sshAddressSplit := strings.Split(address, ":") - switch len(sshAddressSplit) { - case 1: - // IP address but no port - s.IPv4 = sshAddressSplit[0] - s.Port = "22" - case 2: - // Possibly both IP address and port - s.IPv4 = sshAddressSplit[0] - if s.IPv4 == "" { - err = fmt.Errorf("%w: '%s'", ErrParseAddress, address) - } - s.Port = sshAddressSplit[1] - if s.Port == "" { - s.Port = "22" - } - default: - err = fmt.Errorf("%w: '%s'", ErrParseAddress, address) - } - - s.MutexData.Unlock() - return err -} - -// SettingsSSH is for functional option pattern -type SettingsSSH func(*OptsOpenSSH) - -// WithWaitPressEnter is one possible function to pass into OpenSSH -// It will wait until user presses ENTER key to shutdown the container -func WithWaitPressEnter() SettingsSSH { - return func(s *OptsOpenSSH) { - s.WaitFunc = func() { - <-s.TunnelReady - fmt.Print("Press ENTER to stop container ") - fmt.Scanln() // nolint:errcheck - s.TunnelClose <- true - } - } -} - -// WithWaitNone is one possible function to pass into OpenSSH -// It will not wait -func WithWaitNone() SettingsSSH { - return func(s *OptsOpenSSH) { - s.WaitFunc = func() { - fmt.Println("Skipping waiting") - } - } -} - -// NewSettingsSSH returns a SettingsSSH -func NewSettingsSSH(opts ...SettingsSSH) *OptsOpenSSH { - // Defaults - var m sync.Mutex - s := &OptsOpenSSH{ - MutexData: &m, - TunnelClose: make(chan (bool)), - TunnelReady: make(chan (bool)), - } - WithWaitPressEnter()(s) - - for _, opt := range opts { - opt(s) - } - return s -} - -// OpenSSH takes a container and starts SSH server with port exposed to the host -func OpenSSH( - ctx context.Context, - client *dagger.Client, - container *dagger.Container, - workdir string, - opts *OptsOpenSSH, -) error { - // Example in docs: - // https://docs.dagger.io/cookbook/#expose-service-containers-to-host - // This feature is untested and instead relies on tears, blood and sweat produced during - // it's development to work. - // UPDATE: After more tears, blood and sweat we also have some testing! Yippee! - - if container == nil { - log.Println("skipping SSH because no container was given") - return nil - } - - if workdir == "" { - workdir = "/" - } - - // Generate a password for the root user - opts.MutexData.Lock() - opts.Password = generatePassword(16) - opts.MutexData.Unlock() - - // Prepare the container - container = container. - WithExec([]string{"bash", "-c", fmt.Sprintf("echo 'root:%s' | chpasswd", opts.Password)}). - WithExec([]string{"bash", "-c", fmt.Sprintf("echo 'cd %s' >> /root/.bashrc", workdir)}). - WithExec([]string{"/usr/sbin/sshd", "-D"}) - - // ANCHOR: ContainerAsService - // Convert container to service with exposed SSH port - const sshPort = 22 - sshServiceDoc := container.WithExposedPort(sshPort).AsService() - - // Expose the SSH server to the host - sshServiceTunnel, err := client.Host().Tunnel(sshServiceDoc).Start(ctx) - if err != nil { - fmt.Println("Problem getting tunnel up") - return err - } - defer sshServiceTunnel.Stop(ctx) // nolint:errcheck - // ANCHOR_END: ContainerAsService - - // Get and print instructions on how to connect - sshAddress, err := sshServiceTunnel.Endpoint(ctx) - errAddr := opts.Address(sshAddress) - if err != nil || errAddr != nil { - fmt.Println("problem getting address") - return errors.Join(err, errAddr) - } - fmt.Println("Container was reverted back into a state before failed command was executed") - fmt.Printf("Connect into the container with:\n ssh root@%s -p %s -o PreferredAuthentications=password\n", opts.IPv4, opts.Port) - fmt.Printf("Password is:\n %s\n", opts.Password) - fmt.Println("SSH up and running") - - // Wait for user to press key - go opts.Wait() - opts.TunnelReady <- true - <-opts.TunnelClose - - fmt.Println("DONE") - return nil -} - -func generatePassword(length int) string { - // I suppose we could use crypto/rand, but this seems simpler - // Also, it is meant only as temporary password for a temporary container which gets - // shut-down / removed afterwards. I think it is good enough. - characters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789") - pass := make([]rune, length) - for i := range pass { - pass[i] = characters[rand.Intn(len(characters))] - } - return string(pass) -} diff --git a/cmd/firmware-action/container/ssh_test.go b/cmd/firmware-action/container/ssh_test.go deleted file mode 100644 index 5390006f..00000000 --- a/cmd/firmware-action/container/ssh_test.go +++ /dev/null @@ -1,151 +0,0 @@ -// SPDX-License-Identifier: MIT - -// Package container -package container - -import ( - "bytes" - "context" - "fmt" - "os" - "testing" - "time" - - "dagger.io/dagger" - "github.com/stretchr/testify/assert" - "golang.org/x/crypto/ssh" -) - -func TestAddress(t *testing.T) { - defaultIPv4 := "192.168.0.1" - defaultPort := "22" - - testCases := []struct { - name string - address string - ipv4 string - port string - wantErr error - }{ - { - name: "classic", - address: fmt.Sprintf("%s:%s", defaultIPv4, defaultPort), - ipv4: defaultIPv4, - port: defaultPort, - wantErr: nil, - }, - { - name: "no port", - address: defaultIPv4, - ipv4: defaultIPv4, - port: defaultPort, - wantErr: nil, - }, - { - name: "no port but with colon", - address: fmt.Sprintf("%s:", defaultIPv4), - ipv4: defaultIPv4, - port: defaultPort, - wantErr: nil, - }, - { - name: "no IP but port", - address: fmt.Sprintf(":%s", defaultPort), - ipv4: defaultIPv4, - port: defaultPort, - wantErr: ErrParseAddress, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - opts := NewSettingsSSH() - err := opts.Address(tc.address) - assert.ErrorIs(t, err, tc.wantErr) - if err != nil { - // no need to check if errored - return - } - assert.Equal(t, tc.ipv4, opts.IPv4) - assert.Equal(t, tc.port, opts.Port) - }) - } -} - -func TestOpenSSH(t *testing.T) { - if testing.Short() { - t.Skip("skipping test in short mode") - } - - ctx := context.Background() - client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stdout)) - assert.NoError(t, err) - defer client.Close() - - testCases := []struct { - name string - opts SetupOpts - optsSSH *OptsOpenSSH - }{ - { - name: "hello world", - opts: SetupOpts{ - ContainerURL: "ghcr.io/9elements/firmware-action/linux_6.1.45:main", - MountContainerDir: "/src", - MountHostDir: ".", - WorkdirContainer: "/src", - }, - optsSSH: NewSettingsSSH(WithWaitNone()), - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - myContainer, err := Setup(ctx, client, &tc.opts, "") - assert.NoError(t, err) - - // Open the SSH - go func() { - err := OpenSSH(ctx, client, myContainer, tc.opts.WorkdirContainer, tc.optsSSH) - if err != nil { - tc.optsSSH.TunnelClose <- true - } - assert.NoError(t, err) - }() - // Wait until SSH server is ready - ticker := time.NewTicker(5 * time.Second) - select { - case <-tc.optsSSH.TunnelReady: - ticker.Stop() - case <-ticker.C: - ticker.Stop() - ctx.Done() - assert.FailNow(t, "timeout waiting for SSH server to be ready") - } - - // Connect with client - config := &ssh.ClientConfig{ - User: "root", - Auth: []ssh.AuthMethod{ - ssh.Password(tc.optsSSH.Password), - }, - HostKeyCallback: ssh.InsecureIgnoreHostKey(), - } - client, err := ssh.Dial("tcp", fmt.Sprintf("%s:%s", tc.optsSSH.IPv4, tc.optsSSH.Port), config) - assert.NoError(t, err) - defer client.Close() - - // Open session - session, err := client.NewSession() - assert.NoError(t, err) - defer session.Close() - - // Run simple command to test functionality - var b bytes.Buffer - session.Stdout = &b - err = session.Run("/usr/bin/whoami") - assert.NoError(t, err) - assert.Equal(t, "root\n", b.String()) - - tc.optsSSH.TunnelClose <- true - }) - } -} diff --git a/cmd/firmware-action/main.go b/cmd/firmware-action/main.go index 1b8374cf..20f8e2a9 100644 --- a/cmd/firmware-action/main.go +++ b/cmd/firmware-action/main.go @@ -45,9 +45,8 @@ var CLI struct { Config string `type:"path" required:"" default:"${config_file}" help:"Path to configuration file"` Build struct { - Target string `required:"" help:"Select which target to build, use ID from configuration file"` - Recursive bool `help:"Build recursively with all dependencies and payloads"` - Interactive bool `help:"Open interactive SSH into container if build fails"` + Target string `required:"" help:"Select which target to build, use ID from configuration file"` + Recursive bool `help:"Build recursively with all dependencies and payloads"` } `cmd:"build" help:"Build a target defined in configuration file"` GenerateConfig struct{} `cmd:"generate-config" help:"Generate empty configuration file"` @@ -80,7 +79,6 @@ func run(ctx context.Context) error { slog.String("input/config", CLI.Config), slog.String("input/target", CLI.Build.Target), slog.Bool("input/recursive", CLI.Build.Recursive), - slog.Bool("input/interactive", CLI.Build.Interactive), ) // Check if submodules were initialized @@ -129,7 +127,6 @@ submodule_out: ctx, CLI.Build.Target, CLI.Build.Recursive, - CLI.Build.Interactive, myConfig, recipes.Execute, ) diff --git a/cmd/firmware-action/recipes/config.go b/cmd/firmware-action/recipes/config.go index 2f64d850..427975d3 100644 --- a/cmd/firmware-action/recipes/config.go +++ b/cmd/firmware-action/recipes/config.go @@ -222,7 +222,7 @@ type FirmwareModule interface { GetContainerOutputFiles() []string GetOutputDir() string GetSources() []string - buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) (*dagger.Container, error) + buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) error } // =========== diff --git a/cmd/firmware-action/recipes/coreboot.go b/cmd/firmware-action/recipes/coreboot.go index 89474b88..9c8249cd 100644 --- a/cmd/firmware-action/recipes/coreboot.go +++ b/cmd/firmware-action/recipes/coreboot.go @@ -245,7 +245,7 @@ func corebootProcessBlobs(opts CorebootBlobs) ([]BlobDef, error) { } // buildFirmware builds coreboot with all blobs and stuff -func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) (*dagger.Container, error) { +func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) error { // Spin up container containerOpts := container.SetupOpts{ ContainerURL: opts.SdkURL, @@ -262,7 +262,7 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien "Failed to start a container", slog.Any("error", err), ) - return nil, err + return err } // Copy over the defconfig file @@ -275,7 +275,7 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien slog.String("suggestion", logging.ThisShouldNotHappenMessage), slog.Any("error", err), ) - return nil, err + return err } myContainer = myContainer.WithFile( filepath.Join(ContainerWorkDir, defconfigBasename), @@ -285,7 +285,6 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien // Get value of CONFIG_MAINBOARD_DIR / MAINBOARD_DIR variable from dotconfig // to extract value of 'CONFIG_MAINBOARD_DIR', there must be '.config' generateDotConfigCmd := []string{"make", fmt.Sprintf("KBUILD_DEFCONFIG=%s", defconfigBasename), "defconfig"} - myContainerPrevious := myContainer mainboardDir, err := myContainer. WithExec(generateDotConfigCmd). WithExec([]string{"./util/scripts/config", "-s", "CONFIG_MAINBOARD_DIR"}). @@ -295,7 +294,7 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien "Failed to get value of MAINBOARD_DIR from .config", slog.Any("error", err), ) - return myContainerPrevious, err + return err } // strip newline from mainboardDir mainboardDir = strings.Replace(mainboardDir, "\n", "", -1) @@ -319,7 +318,7 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien "Failed to process all blobs", slog.Any("error", err), ) - return nil, err + return err } for blob := range blobs { // Path to local file on host @@ -340,7 +339,7 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien slog.String("suggestion", "blobs are copied into container separately from 'input_files' and 'input_dirs', the path should point to files on your host"), slog.Any("error", err), ) - return nil, err + return err } if blobs[blob].IsDirectory { // Directory @@ -386,7 +385,7 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien "Failed to extract environment variables from current environment", slog.Any("error", err), ) - return myContainerPrevious, fmt.Errorf("coreboot build failed: %w", err) + return fmt.Errorf("coreboot build failed: %w", err) } for key, value := range envVars { myContainer = myContainer.WithEnvVariable(key, value) @@ -394,7 +393,6 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien // Build for step := range buildSteps { - myContainerPrevious := myContainer myContainer, err = myContainer. WithExec(buildSteps[step]). Sync(ctx) @@ -403,12 +401,12 @@ func (opts CorebootOpts) buildFirmware(ctx context.Context, client *dagger.Clien "Failed to build coreboot", slog.Any("error", err), ) - return myContainerPrevious, fmt.Errorf("coreboot build failed: %w", err) + return fmt.Errorf("coreboot build failed: %w", err) } } // Extract artifacts - return myContainer, container.GetArtifacts(ctx, myContainer, opts.CommonOpts.GetArtifacts()) + return container.GetArtifacts(ctx, myContainer, opts.CommonOpts.GetArtifacts()) } func corebootPassEnvVars(repoPath string) (map[string]string, error) { diff --git a/cmd/firmware-action/recipes/coreboot_test.go b/cmd/firmware-action/recipes/coreboot_test.go index e3bf443b..8bbbf158 100644 --- a/cmd/firmware-action/recipes/coreboot_test.go +++ b/cmd/firmware-action/recipes/coreboot_test.go @@ -300,7 +300,7 @@ func TestCorebootBuild(t *testing.T) { assert.NoError(t, err) } // Try to build coreboot - _, err = tc.corebootOptions.buildFirmware(ctx, client, "") + err = tc.corebootOptions.buildFirmware(ctx, client, "") assert.ErrorIs(t, err, tc.wantErr) // Check artifacts @@ -311,7 +311,7 @@ func TestCorebootBuild(t *testing.T) { if tc.wantErr == nil { // Check coreboot version - _, err = tc.universalOptions.buildFirmware(ctx, client, "") + err = tc.universalOptions.buildFirmware(ctx, client, "") assert.ErrorIs(t, err, tc.wantErr) // Check file with coreboot version exists @@ -592,7 +592,7 @@ func TestCorebootSubmodule(t *testing.T) { } // Try to build coreboot - _, err = tc.corebootOptions.buildFirmware(ctx, client, "") + err = tc.corebootOptions.buildFirmware(ctx, client, "") assert.ErrorIs(t, err, tc.wantErr) // Check artifacts @@ -602,7 +602,7 @@ func TestCorebootSubmodule(t *testing.T) { } // Check coreboot version - _, err = tc.universalOptions.buildFirmware(ctx, client, "") + err = tc.universalOptions.buildFirmware(ctx, client, "") assert.ErrorIs(t, err, tc.wantErr) // Check file with coreboot version exists diff --git a/cmd/firmware-action/recipes/edk2.go b/cmd/firmware-action/recipes/edk2.go index cd6d9030..d98fb57e 100644 --- a/cmd/firmware-action/recipes/edk2.go +++ b/cmd/firmware-action/recipes/edk2.go @@ -74,7 +74,7 @@ func (opts Edk2Opts) GetArtifacts() *[]container.Artifacts { } // buildFirmware builds edk2 or Intel FSP -func (opts Edk2Opts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) (*dagger.Container, error) { +func (opts Edk2Opts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) error { envVars := map[string]string{ "WORKSPACE": ContainerWorkDir, "EDK_TOOLS_PATH": "/tools/Edk2/BaseTools", @@ -97,7 +97,7 @@ func (opts Edk2Opts) buildFirmware(ctx context.Context, client *dagger.Client, d "Failed to start a container", slog.Any("error", err), ) - return nil, err + return err } // Setup environment variables in the container @@ -112,7 +112,7 @@ func (opts Edk2Opts) buildFirmware(ctx context.Context, client *dagger.Client, d if _, err := os.Stat(opts.DefconfigPath); !errors.Is(err, os.ErrNotExist) { defconfigFileArgs, err = os.ReadFile(opts.DefconfigPath) if err != nil { - return nil, err + return err } } else { slog.Warn( @@ -133,9 +133,7 @@ func (opts Edk2Opts) buildFirmware(ctx context.Context, client *dagger.Client, d buildSteps = append(buildSteps, []string{"bash", "-c", fmt.Sprintf("%s %s", opts.BuildCommand, string(defconfigFileArgs))}) // Build - var myContainerPrevious *dagger.Container for step := range buildSteps { - myContainerPrevious = myContainer myContainer, err = myContainer. WithExec(buildSteps[step]). Sync(ctx) @@ -144,10 +142,10 @@ func (opts Edk2Opts) buildFirmware(ctx context.Context, client *dagger.Client, d "Failed to build edk2", slog.Any("error", err), ) - return myContainerPrevious, fmt.Errorf("edk2 build failed: %w", err) + return fmt.Errorf("edk2 build failed: %w", err) } } // Extract artifacts - return myContainer, container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) + return container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) } diff --git a/cmd/firmware-action/recipes/edk2_test.go b/cmd/firmware-action/recipes/edk2_test.go index b5a2fed9..0fc9bb75 100644 --- a/cmd/firmware-action/recipes/edk2_test.go +++ b/cmd/firmware-action/recipes/edk2_test.go @@ -107,7 +107,7 @@ func TestEdk2(t *testing.T) { tc.edk2Options.OutputDir = outputPath // Try to build edk2 - _, err = tc.edk2Options.buildFirmware(ctx, client, dockerfilePath) + err = tc.edk2Options.buildFirmware(ctx, client, dockerfilePath) assert.NoError(t, err) // Check artifacts diff --git a/cmd/firmware-action/recipes/linux.go b/cmd/firmware-action/recipes/linux.go index c4ba93ba..d686f5be 100644 --- a/cmd/firmware-action/recipes/linux.go +++ b/cmd/firmware-action/recipes/linux.go @@ -72,7 +72,7 @@ func (opts LinuxOpts) GetArtifacts() *[]container.Artifacts { // buildFirmware builds linux kernel // // docs: https://www.kernel.org/doc/html/latest/kbuild/index.html -func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) (*dagger.Container, error) { +func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) error { // Spin up container containerOpts := container.SetupOpts{ ContainerURL: opts.SdkURL, @@ -89,7 +89,7 @@ func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, "Failed to start a container", slog.Any("error", err), ) - return nil, err + return err } // Copy over the defconfig file @@ -102,7 +102,7 @@ func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, // make: *** [Makefile:704: linux.defconfig] Error 2 // but defconfigBasename="linux_defconfig" works fine // Don't know why, just return error and let user deal with it. - return nil, fmt.Errorf( + return fmt.Errorf( "filename '%s' specified by defconfig_path must not contain '.defconfig'", defconfigBasename, ) @@ -111,7 +111,7 @@ func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, if !defconfigRegex.MatchString(defconfigBasename) { // 'make $defconfigBasename' will fail for Linux kernel if the file // does not end with 'defconfig' - return nil, fmt.Errorf( + return fmt.Errorf( "filename '%s' specified by defconfig_path must end with 'defconfig'", defconfigBasename, ) @@ -124,7 +124,7 @@ func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, slog.String("suggestion", logging.ThisShouldNotHappenMessage), slog.Any("error", err), ) - return nil, err + return err } myContainer = myContainer.WithFile( filepath.Join(ContainerWorkDir, defconfigBasename), @@ -154,7 +154,7 @@ func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, slog.String("target_architecture", opts.Arch), slog.Any("error", err), ) - return nil, err + return err } if val != "" { envVars["CROSS_COMPILE"] = val @@ -183,9 +183,7 @@ func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, } // Execute build commands - var myContainerPrevious *dagger.Container for step := range buildSteps { - myContainerPrevious = myContainer myContainer, err = myContainer. WithExec(buildSteps[step]). Sync(ctx) @@ -194,10 +192,10 @@ func (opts LinuxOpts) buildFirmware(ctx context.Context, client *dagger.Client, "Failed to build linux", slog.Any("error", err), ) - return myContainerPrevious, fmt.Errorf("linux build failed: %w", err) + return fmt.Errorf("linux build failed: %w", err) } } // Extract artifacts - return myContainer, container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) + return container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) } diff --git a/cmd/firmware-action/recipes/linux_test.go b/cmd/firmware-action/recipes/linux_test.go index 9daffd44..77aa1c85 100644 --- a/cmd/firmware-action/recipes/linux_test.go +++ b/cmd/firmware-action/recipes/linux_test.go @@ -151,7 +151,7 @@ func TestLinux(t *testing.T) { myLinuxOpts.OutputDir = outputPath // Try to build linux kernel - _, err = myLinuxOpts.buildFirmware(ctx, client, dockerfilePath) + err = myLinuxOpts.buildFirmware(ctx, client, dockerfilePath) assert.ErrorIs(t, err, tc.wantErr) // Check artifacts diff --git a/cmd/firmware-action/recipes/recipes.go b/cmd/firmware-action/recipes/recipes.go index 6a90d22e..6d71112e 100644 --- a/cmd/firmware-action/recipes/recipes.go +++ b/cmd/firmware-action/recipes/recipes.go @@ -15,7 +15,6 @@ import ( "sync" "dagger.io/dagger" - "github.com/9elements/firmware-action/container" "github.com/9elements/firmware-action/filesystem" "github.com/heimdalr/dag" ) @@ -61,9 +60,8 @@ func Build( ctx context.Context, target string, recursive bool, - interactive bool, config *Config, - executor func(context.Context, string, *Config, bool) error, + executor func(context.Context, string, *Config) error, ) ([]BuildResults, error) { dependencyForest := dag.NewDAG() dependencies := [][]string{} @@ -128,7 +126,7 @@ func Build( for _, item := range queue { slog.Info(fmt.Sprintf("Building: %s", item)) - err = executor(ctx, item, config, interactive) + err = executor(ctx, item, config) builds = append(builds, BuildResults{item, err}) if err != nil && !errors.Is(err, ErrBuildUpToDate) { @@ -139,7 +137,7 @@ func Build( // else build only the target slog.Info(fmt.Sprintf("Building '%s' NOT recursively", target)) - err = executor(ctx, target, config, interactive) + err = executor(ctx, target, config) builds = append(builds, BuildResults{target, err}) } @@ -174,8 +172,8 @@ func IsDirEmpty(path string) (bool, error) { } // Execute a build step -// func Execute(ctx context.Context, target string, config *Config, interactive bool, bulldozeMode bool) error { -func Execute(ctx context.Context, target string, config *Config, interactive bool) error { +// func Execute(ctx context.Context, target string, config *Config, bulldozeMode bool) error { +func Execute(ctx context.Context, target string, config *Config) error { // Prep err := os.MkdirAll(TimestampsDir, os.ModePerm) if err != nil { @@ -246,17 +244,11 @@ func Execute(ctx context.Context, target string, config *Config, interactive boo defer client.Close() // Build the module - myContainer, err := modules[target].buildFirmware(ctx, client, "") + err = modules[target].buildFirmware(ctx, client, "") if err == nil { // On success update the timestamp _ = filesystem.SaveCurrentRunTime(timestampFile) } - if err != nil && interactive { - // If error, try to open SSH - opts := container.NewSettingsSSH(container.WithWaitPressEnter()) - sshErr := container.OpenSSH(ctx, client, myContainer, ContainerWorkDir, opts) - return errors.Join(err, sshErr) - } return err } return ErrTargetMissing diff --git a/cmd/firmware-action/recipes/recipes_test.go b/cmd/firmware-action/recipes/recipes_test.go index e0b0613e..9f1fd21c 100644 --- a/cmd/firmware-action/recipes/recipes_test.go +++ b/cmd/firmware-action/recipes/recipes_test.go @@ -11,7 +11,6 @@ import ( ) func TestExecute(t *testing.T) { - const interactive = false ctx := context.Background() client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stdout)) assert.NoError(t, err) @@ -39,14 +38,13 @@ func TestExecute(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err = Execute(ctx, tc.target, &tc.config, interactive) + err = Execute(ctx, tc.target, &tc.config) assert.ErrorIs(t, err, tc.wantErr) }) } } func TestExecuteSkipAndMissing(t *testing.T) { - const interactive = false ctx := context.Background() client, err := dagger.Connect(ctx, dagger.WithLogOutput(os.Stdout)) assert.NoError(t, err) @@ -91,18 +89,18 @@ func TestExecuteSkipAndMissing(t *testing.T) { // Files from the 2nd modules are missing // This should fail since the 2nd module is in Depends - err = Execute(ctx, target, &myConfig, interactive) + err = Execute(ctx, target, &myConfig) assert.ErrorIs(t, err, ErrDependencyOutputMissing) // Create the output directory // Should build because the directory is empty err = os.Mkdir(outputDir, os.ModePerm) assert.NoError(t, err) - err = Execute(ctx, target, &myConfig, interactive) + err = Execute(ctx, target, &myConfig) assert.ErrorIs(t, err, ErrDependencyOutputMissing) } -func executeDummy(_ context.Context, _ string, _ *Config, _ bool) error { +func executeDummy(_ context.Context, _ string, _ *Config) error { return nil } @@ -246,14 +244,12 @@ func TestBuild(t *testing.T) { }, } - const interactive = false for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { _, err := Build( ctx, tc.target, tc.recursive, - interactive, &tc.config, executeDummy, ) @@ -266,7 +262,6 @@ func TestBuild(t *testing.T) { ctx, "pizza", recursive, - interactive, &testConfigDependencyHell, executeDummy, ) diff --git a/cmd/firmware-action/recipes/stitching.go b/cmd/firmware-action/recipes/stitching.go index 5726b8ea..ac87c2c7 100644 --- a/cmd/firmware-action/recipes/stitching.go +++ b/cmd/firmware-action/recipes/stitching.go @@ -161,7 +161,7 @@ func ifdtoolCmd(platform string, arguments []string) []string { } // buildFirmware builds coreboot with all blobs and stuff -func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) (*dagger.Container, error) { +func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) error { // Check that all files have unique filenames (they are copied into the same dir) copiedFiles := map[string]string{} for _, entry := range opts.IfdtoolEntries { @@ -172,7 +172,7 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag slog.String("suggestion", "Each file must have a unique name because they get copied into single directory"), slog.Any("error", os.ErrExist), ) - return nil, os.ErrExist + return os.ErrExist } copiedFiles[filename] = entry.Path } @@ -190,7 +190,7 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag "Failed to start a container", slog.Any("error", err), ) - return nil, err + return err } // Copy all the files into container @@ -201,7 +201,7 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag slog.String("suggestion", logging.ThisShouldNotHappenMessage), slog.Any("error", err), ) - return nil, err + return err } newBaseFilePath := filepath.Join(ContainerWorkDir, filepath.Base(opts.BaseFilePath)) myContainer = myContainer.WithFile( @@ -236,20 +236,19 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag slog.String("suggestion", "Double check provided path to file"), slog.Any("error", err), ) - return nil, err + return err } } // Get the size of image (total size) cmd := ifdtoolCmd(opts.Platform, []string{"--dump", opts.BaseFilePath}) - myContainerPrevious := myContainer ifdtoolStdout, err := myContainer.WithExec(cmd).Stdout(ctx) if err != nil { slog.Error( "Failed to dump Intel Firmware Descriptor (IFD)", slog.Any("error", err), ) - return myContainerPrevious, err + return err } size, err := ExtractSizeFromString(ifdtoolStdout) if err != nil { @@ -257,7 +256,7 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag "Failed extract size from Intel Firmware Descriptor (IFD)", slog.Any("error", err), ) - return nil, err + return err } var totalSize uint64 for _, i := range size { @@ -270,7 +269,7 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag // Read the base file baseFile, err := os.ReadFile(oldBaseFilePath) if err != nil { - return nil, err + return err } baseFileSize := uint64(len(baseFile)) slog.Info( @@ -286,7 +285,7 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag ), slog.Any("error", err), ) - return nil, err + return err } // Take baseFile content and expand it to correct size @@ -310,11 +309,11 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag ) firmwareImageFile, err := os.Create(imageFilename) if err != nil { - return nil, err + return err } _, err = firmwareImageFile.Write(firmwareImage) if err != nil { - return nil, err + return err } firmwareImageFile.Close() myContainer = myContainer.WithFile( @@ -351,26 +350,24 @@ func (opts FirmwareStitchingOpts) buildFirmware(ctx context.Context, client *dag imageFilename, }, ) - myContainerPrevious = myContainer myContainer, err = myContainer.WithExec(cmd).Sync(ctx) if err != nil { slog.Error("Failed to inject region") - return myContainerPrevious, err + return err } // ifdtool makes a new file '.new', so let's rename back to original name imageFilenameNew := fmt.Sprintf("%s.new", imageFilename) cmd = []string{"mv", "--force", imageFilenameNew, imageFilename} - myContainerPrevious = myContainer myContainer, err = myContainer.WithExec(cmd).Sync(ctx) if err != nil { slog.Error( fmt.Sprintf("Failed to rename '%s' to '%s'", imageFilenameNew, imageFilename), ) - return myContainerPrevious, err + return err } } // Extract artifacts - return myContainer, container.GetArtifacts(ctx, myContainer, opts.CommonOpts.GetArtifacts()) + return container.GetArtifacts(ctx, myContainer, opts.CommonOpts.GetArtifacts()) } diff --git a/cmd/firmware-action/recipes/stitching_test.go b/cmd/firmware-action/recipes/stitching_test.go index 6a75506f..bebc381f 100644 --- a/cmd/firmware-action/recipes/stitching_test.go +++ b/cmd/firmware-action/recipes/stitching_test.go @@ -364,7 +364,7 @@ func TestStitching(t *testing.T) { assert.NoError(t, err) defer client.Close() - _, err = tc.stitchingOpts.buildFirmware(ctx, client, "") + err = tc.stitchingOpts.buildFirmware(ctx, client, "") assert.ErrorIs(t, err, tc.wantErr) if tc.wantErr != nil { return diff --git a/cmd/firmware-action/recipes/universal.go b/cmd/firmware-action/recipes/universal.go index b79a7d30..71ec4f5c 100644 --- a/cmd/firmware-action/recipes/universal.go +++ b/cmd/firmware-action/recipes/universal.go @@ -49,7 +49,7 @@ func (opts UniversalOpts) GetArtifacts() *[]container.Artifacts { } // buildFirmware builds (or rather executes) universal command module -func (opts UniversalOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) (*dagger.Container, error) { +func (opts UniversalOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) error { // Spin up container containerOpts := container.SetupOpts{ ContainerURL: opts.SdkURL, @@ -66,7 +66,7 @@ func (opts UniversalOpts) buildFirmware(ctx context.Context, client *dagger.Clie "Failed to start a container", slog.Any("error", err), ) - return nil, err + return err } // Assemble commands to build @@ -79,9 +79,7 @@ func (opts UniversalOpts) buildFirmware(ctx context.Context, client *dagger.Clie } // Execute build commands - var myContainerPrevious *dagger.Container for step := range buildSteps { - myContainerPrevious = myContainer myContainer, err = myContainer. WithExec(buildSteps[step]). Sync(ctx) @@ -90,10 +88,10 @@ func (opts UniversalOpts) buildFirmware(ctx context.Context, client *dagger.Clie "Failed to build universal", slog.Any("error", err), ) - return myContainerPrevious, fmt.Errorf("universal build failed: %w", err) + return fmt.Errorf("universal build failed: %w", err) } } // Extract artifacts - return myContainer, container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) + return container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) } diff --git a/cmd/firmware-action/recipes/universal_test.go b/cmd/firmware-action/recipes/universal_test.go index 6384145e..df19fa29 100644 --- a/cmd/firmware-action/recipes/universal_test.go +++ b/cmd/firmware-action/recipes/universal_test.go @@ -73,7 +73,7 @@ func TestUniversal(t *testing.T) { myUniversalOpts.OutputDir = outputPath // Try to build universal - _, err = myUniversalOpts.buildFirmware(ctx, client, "") + err = myUniversalOpts.buildFirmware(ctx, client, "") assert.ErrorIs(t, err, tc.wantErr) // Check artifacts diff --git a/cmd/firmware-action/recipes/uroot.go b/cmd/firmware-action/recipes/uroot.go index 5d332b48..877b963b 100644 --- a/cmd/firmware-action/recipes/uroot.go +++ b/cmd/firmware-action/recipes/uroot.go @@ -49,7 +49,7 @@ func (opts URootOpts) GetArtifacts() *[]container.Artifacts { } // buildFirmware builds u-root -func (opts URootOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) (*dagger.Container, error) { +func (opts URootOpts) buildFirmware(ctx context.Context, client *dagger.Client, dockerfileDirectoryPath string) error { // Spin up container containerOpts := container.SetupOpts{ ContainerURL: opts.SdkURL, @@ -66,7 +66,7 @@ func (opts URootOpts) buildFirmware(ctx context.Context, client *dagger.Client, "Failed to start a container", slog.Any("error", err), ) - return nil, err + return err } // Assemble commands to build @@ -76,9 +76,7 @@ func (opts URootOpts) buildFirmware(ctx context.Context, client *dagger.Client, } // Execute build commands - var myContainerPrevious *dagger.Container for step := range buildSteps { - myContainerPrevious = myContainer myContainer, err = myContainer. WithExec(buildSteps[step]). Sync(ctx) @@ -87,10 +85,10 @@ func (opts URootOpts) buildFirmware(ctx context.Context, client *dagger.Client, "Failed to build u-root", slog.Any("error", err), ) - return myContainerPrevious, fmt.Errorf("u-root build failed: %w", err) + return fmt.Errorf("u-root build failed: %w", err) } } // Extract artifacts - return myContainer, container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) + return container.GetArtifacts(ctx, myContainer, opts.GetArtifacts()) } diff --git a/cmd/firmware-action/recipes/uroot_test.go b/cmd/firmware-action/recipes/uroot_test.go index c2f89654..fdae40e7 100644 --- a/cmd/firmware-action/recipes/uroot_test.go +++ b/cmd/firmware-action/recipes/uroot_test.go @@ -98,7 +98,7 @@ func TestURoot(t *testing.T) { myURootOpts.OutputDir = outputPath // Try to build u-root initramfs - _, err = myURootOpts.buildFirmware(ctx, client, "") + err = myURootOpts.buildFirmware(ctx, client, "") assert.ErrorIs(t, err, tc.wantErr) // Check artifacts From f1960ae496e861512f36813a9939bfa17fbacfe1 Mon Sep 17 00:00:00 2001 From: AtomicFS Date: Fri, 24 Jan 2025 16:36:37 +0100 Subject: [PATCH 2/2] docs: update notes on interactive debugging Signed-off-by: AtomicFS --- cmd/firmware-action/main.go | 2 +- docs/src/SUMMARY.md | 2 +- .../get_started/get_started.md | 2 +- docs/src/firmware-action/interactive.md | 54 ++++++++++++------- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/cmd/firmware-action/main.go b/cmd/firmware-action/main.go index 20f8e2a9..8db17856 100644 --- a/cmd/firmware-action/main.go +++ b/cmd/firmware-action/main.go @@ -47,7 +47,7 @@ var CLI struct { Build struct { Target string `required:"" help:"Select which target to build, use ID from configuration file"` Recursive bool `help:"Build recursively with all dependencies and payloads"` - } `cmd:"build" help:"Build a target defined in configuration file"` + } `cmd:"build" help:"Build a target defined in configuration file. For interactive debugging preface the command with 'dagger run --interactive', for example 'dagger run --interactive $(which firmware-action) build --config=...'. To install dagger follow instructions at https://dagger.io/"` GenerateConfig struct{} `cmd:"generate-config" help:"Generate empty configuration file"` Version struct{} `cmd:"version" help:"Print version and exit"` diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index 4a187028..b41493f7 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -14,7 +14,7 @@ - [Get firmware-action](firmware-action/get_started/04_get_firmware_action.md) - [Run firmware-action locally](firmware-action/get_started/05_run_firmware_action.md) - [Run firmware-action in CI](firmware-action/get_started/06_run_in_ci.md) - - [Interactive mode](firmware-action/interactive.md) + - [Interactive debugging](firmware-action/interactive.md) - [TLDR; Usage](firmware-action/usage.md) - [Local system](firmware-action/usage_local.md) - [GitHub CI](firmware-action/usage_github.md) diff --git a/docs/src/firmware-action/get_started/get_started.md b/docs/src/firmware-action/get_started/get_started.md index 9c0d59e1..ed2c7646 100644 --- a/docs/src/firmware-action/get_started/get_started.md +++ b/docs/src/firmware-action/get_started/get_started.md @@ -15,5 +15,5 @@ The code from this example is available in [firmware-action-example](https://git - installed [Docker](https://wiki.archlinux.org/title/Docker) - installed git +- installed [dagger](https://docs.dagger.io/install) (optional, needed for interactive debugging) - installed [taskfile](https://taskfile.dev/) (optional) - diff --git a/docs/src/firmware-action/interactive.md b/docs/src/firmware-action/interactive.md index 0ec89bde..f5651dca 100644 --- a/docs/src/firmware-action/interactive.md +++ b/docs/src/firmware-action/interactive.md @@ -1,35 +1,53 @@ -# Interactive mode +# Interactive debugging ```admonish note title="A little bit of backstory" -While I was playing around with firmware-action I found early on that debugging what is going on inside the docker container is rather lengthy and annoying process. This was the moment when the idea of some interactive option was born. +While I was playing around with `firmware-action` I found early on that debugging what is going on inside the docker container is rather lengthy and annoying process. This was the moment when the idea of some interactive option was born. ``` -```admonish bug title="Issue [#269](https://github.com/9elements/firmware-action/issues/269)" +```admonish done collapsible=true title="Dropping the SSH feature in favor of Dagger build-in debugging" Dagger [since v0.12](https://dagger.io/blog/dagger-0-12) supports new built-in interactive debugging. -We are already planning to re-write `firmware-action` to use this new feature instead of the `ssh` solution we are currently using. For more details see issue [269](https://github.com/9elements/firmware-action/issues/269). -``` +~~We are already planning to re-write `firmware-action` to use this new feature instead of the `ssh` solution we are currently using. For more details see issue [269](https://github.com/9elements/firmware-action/issues/269).~~ + +**UPDATE:** It is possible now to use the new and shiny feature of dagger for interactive debugging! As a result we have dropped the SSH feature. -On build failure open `ssh` server in the container and let user connect into it to debug the problem. To enable this feature user has to pass argument `--interactive`. User can ssh into the container with a randomly generated password. +Related: +- Issue [#269](https://github.com/9elements/firmware-action/issues/269) +- Pull Request [#522](https://github.com/9elements/firmware-action/pull/522) -The container will keep running until user presses `ENTER` key in the terminal with firmware-action running. +Supplementary dagger documentation: +- Blog post [Dagger 0.12: interactive debugging](https://dagger.io/blog/dagger-0-12) +- Documentation for [Interactive Debugging](https://docs.dagger.io/features/debugging) +- Documentation for [Custom applications](https://docs.dagger.io/api/sdk/#custom-applications) +- Documentation for [Interactive Terminal](https://docs.dagger.io/api/terminal/) +``` +To leverage the use of interactive debugging, you have to install [dagger CLI](https://docs.dagger.io/install). -```admonish attention -The container is launched in the interactive mode before the failed command was started. +Then when using `firmware-action`, simply prepend the command with `dagger run --interactive`. -This reverting behavior is out of technical necessity. +Instead of: +```bash +firmware-action build --config=firmware-action.json --target=coreboot-example +``` + Execute this: +```bash +dagger run --interactive firmware-action build --config=firmware-action.json --target=coreboot-example ``` -```admonish note -The containers in dagger (at the time of writing) are single-use non-interactive containers. Dagger has a pipeline (command queue for each container) which starts executing only when specific functions such as [Sync()](https://pkg.go.dev/dagger.io/dagger#Container.Sync) are called which trigger evaluation of the pipeline inside the container. - -To start a `ssh` service and wait for user to log-in, the container has to be converted into a [service](https://pkg.go.dev/dagger.io/dagger#Container.AsService) which also forces evaluation of the pipeline. And if any of the commands should fail, it would fail to start the `service` container. +If you are using `Taskfile` to abstract away some of the complexity that comes with larger projects, simply prepend the whole `Taskfile` command. -As a workaround, when the evaluation of pipeline fails in the container, the container from previous step is converted into a `service` container with everything as it was just before the failing command was executed. In essence, when you connect, you end up in pristine environment. +Instead of: +```bash +task build:coreboot-example +``` -~~~go -{{#include ../../../cmd/firmware-action/container/ssh.go:ContainerAsService}} -~~~ + Execute this: +```bash +dagger run --interactive task build:coreboot-example ``` + +On build failure you will be dropped into the container and can debug the problem. + +To exit the container run command `exit` or press `CTRL+D`.