Skip to content

Commit

Permalink
bib: fix injecting default "build" arg into root for flags like --local
Browse files Browse the repository at this point in the history
In PR#608 we dropped the container entrypoint and moved the logic
to detect what command to run into the cobra parsing.

However it seems certain situations like:
```
bootc-image-builder  --type qcow2 --tls-verify=true  --local quay.io/centos-bootc/centos-bootc:stream10
```
are not detected correctly (thanka to Chunfu Wen for reporting!).

This commit detect this usage and injects the build arg then too.

Closes #614
  • Loading branch information
mvo5 authored and ondrejbudai committed Aug 27, 2024
1 parent dcd8831 commit 8f80115
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 6 deletions.
1 change: 1 addition & 0 deletions bib/cmd/bootc-image-builder/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var (
UpdateFilesystemSizes = updateFilesystemSizes
GenPartitionTable = genPartitionTable
CreateRand = createRand
BuildCobraCmdline = buildCobraCmdline
)

func MockOsGetuid(new func() int) (restore func()) {
Expand Down
31 changes: 25 additions & 6 deletions bib/cmd/bootc-image-builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"golang.org/x/exp/slices"

"github.com/osbuild/images/pkg/arch"
Expand Down Expand Up @@ -565,7 +566,7 @@ func cmdVersion(_ *cobra.Command, _ []string) error {
return nil
}

func run() error {
func buildCobraCmdline() (*cobra.Command, error) {
rootCmd := &cobra.Command{
Use: "bootc-image-builder",
Long: "create a bootable image from an ostree native container",
Expand Down Expand Up @@ -612,7 +613,7 @@ func run() error {
// default from users.
manifestCmd.Flags().String("config", "", "build config file; /config.json will be used if present")
if err := manifestCmd.Flags().MarkHidden("config"); err != nil {
return fmt.Errorf("cannot hide 'config' :%w", err)
return nil, fmt.Errorf("cannot hide 'config' :%w", err)
}

buildCmd.Flags().AddFlagSet(manifestCmd.Flags())
Expand All @@ -626,11 +627,11 @@ func run() error {
// flag rules
for _, dname := range []string{"output", "store", "rpmmd"} {
if err := buildCmd.MarkFlagDirname(dname); err != nil {
return err
return nil, err
}
}
if err := buildCmd.MarkFlagFilename("config"); err != nil {
return err
return nil, err
}
buildCmd.MarkFlagsRequiredTogether("aws-region", "aws-bucket", "aws-ami-name")

Expand All @@ -640,11 +641,29 @@ func run() error {
// "quay.io" will create an "err != nil". Ideally we could check err
// for something like cobra.UnknownCommandError but cobra just gives
// us an error string
_, _, err := rootCmd.Find(os.Args[1:])
if err != nil && !slices.Contains([]string{"help", "completion"}, os.Args[1]) {
cmd, _, err := rootCmd.Find(os.Args[1:])
injectBuildArg := func() {
args := append([]string{buildCmd.Name()}, os.Args[1:]...)
rootCmd.SetArgs(args)
}
// command not known, i.e. happens for "bib quay.io/centos/..."
if err != nil && !slices.Contains([]string{"help", "completion"}, os.Args[1]) {
injectBuildArg()
}
// command appears valid, e.g. "bib --local quay.io/centos" but this
// is the parser just assuming "quay.io" is an argument for "--local" :(
if err == nil && cmd.Use == rootCmd.Use && cmd.Flags().Parse(os.Args[1:]) != pflag.ErrHelp {
injectBuildArg()
}

return rootCmd, nil
}

func run() error {
rootCmd, err := buildCobraCmdline()
if err != nil {
return err
}
return rootCmd.Execute()
}

Expand Down
69 changes: 69 additions & 0 deletions bib/cmd/bootc-image-builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import (
"errors"
"fmt"
"os"
"strings"
"testing"

"github.com/spf13/cobra"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -577,3 +580,69 @@ func TestBuildType(t *testing.T) {
})
}
}

func mockOsArgs(new []string) (restore func()) {
saved := os.Args
os.Args = append([]string{"argv0"}, new...)
return func() {
os.Args = saved
}
}

func addRunLog(rootCmd *cobra.Command, runeCall *string) {
for _, cmd := range rootCmd.Commands() {
cmd.RunE = func(cmd *cobra.Command, args []string) error {
callStr := fmt.Sprintf("<%v>: %v", cmd.Name(), strings.Join(args, ","))
if *runeCall != "" {
panic(fmt.Sprintf("runE called with %v but already called before: %v", callStr, *runeCall))
}
*runeCall = callStr
return nil
}
}
}

func TestCobraCmdline(t *testing.T) {
for _, tc := range []struct {
cmdline []string
expectedCall string
}{
// trivial: cmd is given explicitly
{
[]string{"manifest", "quay.io..."},
"<manifest>: quay.io...",
},
{
[]string{"build", "quay.io..."},
"<build>: quay.io...",
},
{
[]string{"version", "quay.io..."},
"<version>: quay.io...",
},
// implicit: no cmd like build/manifest defaults to build
{
[]string{"--local", "quay.io..."},
"<build>: quay.io...",
},
{
[]string{"quay.io..."},
"<build>: quay.io...",
},
} {
var runeCall string

restore := mockOsArgs(tc.cmdline)
defer restore()

rootCmd, err := main.BuildCobraCmdline()
assert.NoError(t, err)
addRunLog(rootCmd, &runeCall)

t.Run(tc.expectedCall, func(t *testing.T) {
err = rootCmd.Execute()
assert.NoError(t, err)
assert.Equal(t, runeCall, tc.expectedCall)
})
}
}

0 comments on commit 8f80115

Please sign in to comment.