diff --git a/cmd/ubuntu-image/main.go b/cmd/ubuntu-image/main.go index dfa2f42a..8ddf15c2 100644 --- a/cmd/ubuntu-image/main.go +++ b/cmd/ubuntu-image/main.go @@ -25,7 +25,7 @@ var imageType string = "" var stateMachineLongDesc = `Options for controlling the internal state machine. Other than -w, these options are mutually exclusive. When -u or -t is given, the state machine can be resumed later with -r, but -w must be given in that -case since the state is saved in a ubuntu-image.gob file in the working directory.` +case since the state is saved in a ubuntu-image.json file in the working directory.` func executeStateMachine(commonOpts *commands.CommonOpts, stateMachineOpts *commands.StateMachineOpts, ubuntuImageCommand *commands.UbuntuImageCommand) { // Set up the state machine diff --git a/internal/helper/helper_test.go b/internal/helper/helper_test.go index a9c621d3..58532fa9 100644 --- a/internal/helper/helper_test.go +++ b/internal/helper/helper_test.go @@ -7,7 +7,9 @@ import ( "testing" "github.com/google/uuid" + "github.com/invopop/jsonschema" "github.com/snapcore/snapd/osutil" + "github.com/xeipuuv/gojsonschema" ) // define some mocked versions of go package functions @@ -286,7 +288,51 @@ func TestSetDefaults(t *testing.T) { } else { asserter.AssertErrContains(err, tc.expectedError) } + }) + } +} +// TestCheckEmptyFields unit tests the CheckEmptyFields function +func TestCheckEmptyFields(t *testing.T) { + // define the struct we will use to test + type testStruct struct { + A string `yaml:"a" json:"fieldA,required"` + B string `yaml:"b" json:"fieldB"` + C string `yaml:"c" json:"fieldC,omitempty"` + } + + // generate the schema for our testStruct + var jsonReflector jsonschema.Reflector + schema := jsonReflector.Reflect(&testStruct{}) + + // now run CheckEmptyFields with a variety of test data + // to ensure the correct return values + testCases := []struct { + name string + structData testStruct + shouldPass bool + }{ + {"success", testStruct{A: "foo", B: "bar", C: "baz"}, true}, + {"missing_explicitly_required", testStruct{B: "bar", C: "baz"}, false}, + {"missing_implicitly_required", testStruct{A: "foo", C: "baz"}, false}, + {"missing_omitempty", testStruct{A: "foo", B: "bar"}, true}, + } + for i, tc := range testCases { + t.Run("test_check_empty_fields_"+tc.name, func(t *testing.T) { + asserter := Asserter{T: t} + + result := new(gojsonschema.Result) + err := CheckEmptyFields(&testCases[i].structData, result, schema) + asserter.AssertErrNil(err, false) + schema.Required = append(schema.Required, "fieldA") + + // make sure validation will fail only when expected + if tc.shouldPass && !result.Valid() { + t.Error("CheckEmptyFields added errors when it should not have") + } + if !tc.shouldPass && result.Valid() { + t.Error("CheckEmptyFields did NOT add errors when it should have") + } }) } } diff --git a/internal/imagedefinition/README.rst b/internal/imagedefinition/README.rst index 31207b90..db35ed91 100644 --- a/internal/imagedefinition/README.rst +++ b/internal/imagedefinition/README.rst @@ -58,6 +58,8 @@ The following specification defines what is supported in the YAML: target: (optional) # A path to a model assertion to use when pre-seeding snaps # in the image. Must be a local file URI beginning with file:// + # The given path will be interpreted as relative to the path of + # the image definition file if is not absolute. model-assertion: (optional) # Defines parameters needed to build the rootfs for a classic # image. Currently only building from a seed is supported. @@ -115,7 +117,8 @@ The following specification defines what is supported in the YAML: # following compression types: bzip2, gzip, xz, zstd. tarball: (exactly 1 of archive-tasks, seed or tarball must be specified) # The path to the tarball. Currently only local paths beginning with - # file:// are supported + # file:// are supported. The given path will be interpreted as relative + # to the path of the image definition file if is not absolute. url: (required if tarball dict is specified) # URL to the gpg signature to verify the tarball against. gpg: (optional) @@ -202,6 +205,8 @@ The following specification defines what is supported in the YAML: copy-file: (optional) - # The path to the file to copy. + # The given path will be interpreted as relative to the + # path of the image definition file if is not absolute. source: # The path to use as a destination for the copied # file. The location of the rootfs will be prepended diff --git a/internal/statemachine/classic.go b/internal/statemachine/classic.go index 38db9177..b7fa9b84 100644 --- a/internal/statemachine/classic.go +++ b/internal/statemachine/classic.go @@ -42,6 +42,10 @@ func (classicStateMachine *ClassicStateMachine) Setup() error { // set the beginning states that will be used by all classic image builds classicStateMachine.states = startingClassicStates + if err := classicStateMachine.setConfDefDir(classicStateMachine.parent.(*ClassicStateMachine).Args.ImageDefinition); err != nil { + return err + } + // do the validation common to all image types if err := classicStateMachine.validateInput(); err != nil { return err diff --git a/internal/statemachine/classic_states.go b/internal/statemachine/classic_states.go index 7ddd27e6..817af9b5 100644 --- a/internal/statemachine/classic_states.go +++ b/internal/statemachine/classic_states.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "io" - "net/url" "os" "os/exec" "path" @@ -400,17 +399,18 @@ func (stateMachine *StateMachine) buildGadgetTree() error { } sourceDir = gadgetDir case "directory": - // no need to check error here as the validity of the URL - // has been confirmed by the schema validation - sourceURL, _ := url.Parse(classicStateMachine.ImageDef.Gadget.GadgetURL) + gadgetTreePath := strings.TrimPrefix(classicStateMachine.ImageDef.Gadget.GadgetURL, "file://") + if !filepath.IsAbs(gadgetTreePath) { + gadgetTreePath = filepath.Join(stateMachine.ConfDefPath, gadgetTreePath) + } // copy the source tree to the workdir - files, err := osReadDir(sourceURL.Path) + files, err := osReadDir(gadgetTreePath) if err != nil { return fmt.Errorf("Error reading gadget tree: %s", err.Error()) } for _, gadgetFile := range files { - srcFile := filepath.Join(sourceURL.Path, gadgetFile.Name()) + srcFile := filepath.Join(gadgetTreePath, gadgetFile.Name()) if err := osutilCopySpecialFile(srcFile, gadgetDir); err != nil { return fmt.Errorf("Error copying gadget source: %s", err.Error()) } @@ -821,7 +821,7 @@ func (stateMachine *StateMachine) extractRootfsTar() error { // has been confirmed by the schema validation tarPath := strings.TrimPrefix(classicStateMachine.ImageDef.Rootfs.Tarball.TarballURL, "file://") if !filepath.IsAbs(tarPath) { - tarPath, _ = filepath.Abs(tarPath) + tarPath = filepath.Join(stateMachine.ConfDefPath, tarPath) } // if the sha256 sum of the tarball is provided, make sure it matches @@ -999,38 +999,29 @@ func (stateMachine *StateMachine) manualCustomization() error { return fmt.Errorf("Error setting up /etc/resolv.conf in the chroot: \"%s\"", err.Error()) } - type customizationHandler struct { - inputData interface{} - handlerFunc func(interface{}, string, bool) error + err = manualCopyFile(classicStateMachine.ImageDef.Customization.Manual.CopyFile, classicStateMachine.ConfDefPath, stateMachine.tempDirs.chroot, stateMachine.commonFlags.Debug) + if err != nil { + return err } - customizationHandlers := []customizationHandler{ - { - inputData: classicStateMachine.ImageDef.Customization.Manual.CopyFile, - handlerFunc: manualCopyFile, - }, - { - inputData: classicStateMachine.ImageDef.Customization.Manual.Execute, - handlerFunc: manualExecute, - }, - { - inputData: classicStateMachine.ImageDef.Customization.Manual.TouchFile, - handlerFunc: manualTouchFile, - }, - { - inputData: classicStateMachine.ImageDef.Customization.Manual.AddGroup, - handlerFunc: manualAddGroup, - }, - { - inputData: classicStateMachine.ImageDef.Customization.Manual.AddUser, - handlerFunc: manualAddUser, - }, + + err = manualExecute(classicStateMachine.ImageDef.Customization.Manual.Execute, stateMachine.tempDirs.chroot, stateMachine.commonFlags.Debug) + if err != nil { + return err } - for _, customization := range customizationHandlers { - err := customization.handlerFunc(customization.inputData, stateMachine.tempDirs.chroot, stateMachine.commonFlags.Debug) - if err != nil { - return err - } + err = manualTouchFile(classicStateMachine.ImageDef.Customization.Manual.TouchFile, stateMachine.tempDirs.chroot, stateMachine.commonFlags.Debug) + if err != nil { + return err + } + + err = manualAddGroup(classicStateMachine.ImageDef.Customization.Manual.AddGroup, stateMachine.tempDirs.chroot, stateMachine.commonFlags.Debug) + if err != nil { + return err + } + + err = manualAddUser(classicStateMachine.ImageDef.Customization.Manual.AddUser, stateMachine.tempDirs.chroot, stateMachine.commonFlags.Debug) + if err != nil { + return err } return nil @@ -1095,10 +1086,10 @@ func (stateMachine *StateMachine) prepareClassicImage() error { // are also set to be installed. Note we only do this for snaps that are // seeded. Users are expected to specify all base and content provider // snaps in the image definition. + snapStore := store.New(nil, nil) + snapContext := context.Background() for _, seededSnap := range imageOpts.Snaps { - snapStore := store.New(nil, nil) snapSpec := store.SnapSpec{Name: seededSnap} - snapContext := context.TODO() //context can be empty, just not nil snapInfo, err := snapStore.SnapInfo(snapContext, snapSpec, nil) if err != nil { return fmt.Errorf("Error getting info for snap %s: \"%s\"", @@ -1133,8 +1124,18 @@ func (stateMachine *StateMachine) prepareClassicImage() error { } } + modelAssertionPath := strings.TrimPrefix(classicStateMachine.ImageDef.ModelAssertion, "file://") + // if no explicit model assertion was given, keep empty ModelFile to let snapd fallback to default + // model assertion + if len(modelAssertionPath) != 0 { + if !filepath.IsAbs(modelAssertionPath) { + imageOpts.ModelFile = filepath.Join(stateMachine.ConfDefPath, modelAssertionPath) + } else { + imageOpts.ModelFile = modelAssertionPath + } + } + imageOpts.Classic = true - imageOpts.ModelFile = strings.TrimPrefix(classicStateMachine.ImageDef.ModelAssertion, "file://") imageOpts.Architecture = classicStateMachine.ImageDef.Architecture imageOpts.PrepareDir = classicStateMachine.tempDirs.chroot imageOpts.Customizations = *new(image.Customizations) diff --git a/internal/statemachine/classic_test.go b/internal/statemachine/classic_test.go index 82d83c3e..90d4a661 100644 --- a/internal/statemachine/classic_test.go +++ b/internal/statemachine/classic_test.go @@ -18,7 +18,6 @@ import ( "strings" "testing" - "github.com/invopop/jsonschema" "github.com/pkg/xattr" "github.com/snapcore/snapd/image" "github.com/snapcore/snapd/osutil" @@ -303,6 +302,34 @@ func TestPrintStates(t *testing.T) { }) } +// TestClassicStateMachine_Setup_Fail_setConfDefDir tests a failure in the Setup() function when setting the configuration definition directory +func TestClassicStateMachine_Setup_Fail_setConfDefDir(t *testing.T) { + t.Run("test_failed_set_conf_dir", func(t *testing.T) { + asserter := helper.Asserter{T: t} + saveCWD := helper.SaveCWD() + defer saveCWD() + + var stateMachine ClassicStateMachine + stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() + + tmpDirPath := filepath.Join("/tmp", "test_failed_set_conf_dir") + err := os.Mkdir(tmpDirPath, 0755) + t.Cleanup(func() { + os.RemoveAll(tmpDirPath) + }) + asserter.AssertErrNil(err, true) + + err = os.Chdir(tmpDirPath) + asserter.AssertErrNil(err, true) + + _ = os.RemoveAll(tmpDirPath) + + err = stateMachine.Setup() + asserter.AssertErrContains(err, "unable to determine the configuration definition directory") + os.RemoveAll(stateMachine.stateMachineFlags.WorkDir) + }) +} + // TestFailedValidateInputClassic tests a failure in the Setup() function when validating common input func TestFailedValidateInputClassic(t *testing.T) { t.Run("test_failed_validate_input", func(t *testing.T) { @@ -861,55 +888,64 @@ func TestExtractRootfsTar(t *testing.T) { expectedFiles []string }{ { - "vanilla_tar", - filepath.Join(wd, "testdata", "rootfs_tarballs", "rootfs.tar"), - "ec01fd8488b0f35d2ca69e6f82edfaecef5725da70913bab61240419ce574918", - []string{ + name: "vanilla_tar", + rootfsTar: filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar"), + SHA256sum: "ec01fd8488b0f35d2ca69e6f82edfaecef5725da70913bab61240419ce574918", + expectedFiles: []string{ "test_tar1", "test_tar2", }, }, { - "vanilla_tar_relative_path", - filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar"), - "ec01fd8488b0f35d2ca69e6f82edfaecef5725da70913bab61240419ce574918", - []string{ + name: "vanilla_tar respecting absolute path", + rootfsTar: filepath.Join(wd, "testdata", "rootfs_tarballs", "rootfs.tar"), + SHA256sum: "ec01fd8488b0f35d2ca69e6f82edfaecef5725da70913bab61240419ce574918", + expectedFiles: []string{ "test_tar1", "test_tar2", }, }, { - "gz", - filepath.Join(wd, "testdata", "rootfs_tarballs", "rootfs.tar.gz"), - "29152fd9cadbc92f174815ec642ab3aea98f08f902a4f317ec037f8fe60e40c3", - []string{ + name: "vanilla_tar relative path even with dot dot", + rootfsTar: filepath.Join("testdata", "../..", filepath.Base(wd), "testdata", "rootfs_tarballs", "rootfs.tar"), + SHA256sum: "ec01fd8488b0f35d2ca69e6f82edfaecef5725da70913bab61240419ce574918", + expectedFiles: []string{ + "test_tar1", + "test_tar2", + }, + }, + { + name: "gz", + rootfsTar: filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar.gz"), + SHA256sum: "29152fd9cadbc92f174815ec642ab3aea98f08f902a4f317ec037f8fe60e40c3", + expectedFiles: []string{ "test_tar_gz1", "test_tar_gz2", }, }, { - "xz", - filepath.Join(wd, "testdata", "rootfs_tarballs", "rootfs.tar.xz"), - "e3708f1d98ccea0e0c36843d9576580505ee36d523bfcf78b0f73a035ae9a14e", - []string{ + name: "xz", + rootfsTar: filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar.xz"), + SHA256sum: "e3708f1d98ccea0e0c36843d9576580505ee36d523bfcf78b0f73a035ae9a14e", + expectedFiles: []string{ "test_tar_xz1", "test_tar_xz2", }, }, { - "bz2", - filepath.Join(wd, "testdata", "rootfs_tarballs", "rootfs.tar.bz2"), - "a1180a73b652d85d7330ef21d433b095363664f2f808363e67f798fae15abf0c", - []string{ + name: "bz2", + rootfsTar: filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar.bz2"), + SHA256sum: "a1180a73b652d85d7330ef21d433b095363664f2f808363e67f798fae15abf0c", + expectedFiles: []string{ "test_tar_bz1", "test_tar_bz2", }, }, { - "zst", - filepath.Join(wd, "testdata", "rootfs_tarballs", "rootfs.tar.zst"), - "5fb00513f84e28225a3155fd78c59a6a923b222e1c125aab35bbfd4091281829", - []string{ + name: "zst", + rootfsTar: filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar.zst"), + SHA256sum: "5fb00513f84e28225a3155fd78c59a6a923b222e1c125aab35bbfd4091281829", + expectedFiles: []string{ "test_tar_zstd1", "test_tar_zstd2", }, @@ -918,8 +954,8 @@ func TestExtractRootfsTar(t *testing.T) { for _, tc := range testCases { t.Run("test_extract_rootfs_tar_"+tc.name, func(t *testing.T) { asserter := helper.Asserter{T: t} - saveCWD := helper.SaveCWD() - defer saveCWD() + restoreCWD := helper.SaveCWD() + defer restoreCWD() var stateMachine ClassicStateMachine stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() @@ -934,8 +970,11 @@ func TestExtractRootfsTar(t *testing.T) { }, } + err := stateMachine.setConfDefDir(filepath.Join(wd, "image_definition.yaml")) + asserter.AssertErrNil(err, true) + // need workdir set up for this - err := stateMachine.makeTemporaryDirectories() + err = stateMachine.makeTemporaryDirectories() asserter.AssertErrNil(err, true) err = stateMachine.extractRootfsTar() @@ -962,21 +1001,20 @@ func TestFailedExtractRootfsTar(t *testing.T) { var stateMachine ClassicStateMachine stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() stateMachine.parent = &stateMachine - absTarPath, err := filepath.Abs(filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar")) - asserter.AssertErrNil(err, true) + tarPath := filepath.Join("testdata", "rootfs_tarballs", "rootfs.tar") stateMachine.ImageDef = imagedefinition.ImageDefinition{ Architecture: getHostArch(), Series: getHostSuite(), Rootfs: &imagedefinition.Rootfs{ Tarball: &imagedefinition.Tarball{ - TarballURL: fmt.Sprintf("file://%s", absTarPath), + TarballURL: fmt.Sprintf("file://%s", tarPath), SHA256sum: "fail", }, }, } // need workdir set up for this - err = stateMachine.makeTemporaryDirectories() + err := stateMachine.makeTemporaryDirectories() asserter.AssertErrNil(err, true) // mock os.Mkdir @@ -1339,12 +1377,12 @@ chpasswd: } } -// TestManualCustomization unit tests the manualCustomization function -func TestManualCustomization(t *testing.T) { +// TestStateMachine_manualCustomization unit tests the manualCustomization function +func TestStateMachine_manualCustomization(t *testing.T) { t.Run("test_manual_customization", func(t *testing.T) { asserter := helper.Asserter{T: t} - saveCWD := helper.SaveCWD() - defer saveCWD() + restoreCWD := helper.SaveCWD() + defer restoreCWD() var stateMachine ClassicStateMachine stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() @@ -1391,8 +1429,12 @@ func TestManualCustomization(t *testing.T) { }, } + d, _ := os.Getwd() + err := stateMachine.setConfDefDir(filepath.Join(d, "image_definition.yaml")) + asserter.AssertErrNil(err, true) + // need workdir set up for this - err := stateMachine.makeTemporaryDirectories() + err = stateMachine.makeTemporaryDirectories() asserter.AssertErrNil(err, true) // also create chroot @@ -1431,53 +1473,142 @@ func TestManualCustomization(t *testing.T) { }) } -// TestFailedManualCustomization tests failures in the manualCustomization function -func TestFailedManualCustomization(t *testing.T) { +// TestStateMachine_manualCustomization_fail tests failures in the manualCustomization function +func TestStateMachine_manualCustomization_fail(t *testing.T) { t.Run("test_failed_manual_customization", func(t *testing.T) { asserter := helper.Asserter{T: t} - saveCWD := helper.SaveCWD() - defer saveCWD() + restoreCWD := helper.SaveCWD() + t.Cleanup(restoreCWD) var stateMachine ClassicStateMachine stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() stateMachine.parent = &stateMachine - stateMachine.ImageDef = imagedefinition.ImageDefinition{ - Customization: &imagedefinition.Customization{ - Manual: &imagedefinition.Manual{ - TouchFile: []*imagedefinition.TouchFile{ - { - TouchPath: filepath.Join("this", "path", "does", "not", "exist"), - }, - }, - }, - }, - } - // need workdir set up for this err := stateMachine.makeTemporaryDirectories() asserter.AssertErrNil(err, true) - // create an /etc/resolv.conf in the chroot - err = os.MkdirAll(filepath.Join(stateMachine.tempDirs.chroot, "etc"), 0755) - asserter.AssertErrNil(err, true) - _, err = os.Create(filepath.Join(stateMachine.tempDirs.chroot, "etc", "resolv.conf")) - asserter.AssertErrNil(err, true) - - // now test the failed touch file customization - err = stateMachine.manualCustomization() - asserter.AssertErrContains(err, "no such file or directory") - os.RemoveAll(stateMachine.stateMachineFlags.WorkDir) - // mock helper.BackupAndCopyResolvConf helperBackupAndCopyResolvConf = mockBackupAndCopyResolvConf - defer func() { + t.Cleanup(func() { helperBackupAndCopyResolvConf = helper.BackupAndCopyResolvConf - }() + }) err = stateMachine.manualCustomization() asserter.AssertErrContains(err, "Error setting up /etc/resolv.conf") - helperBackupAndCopyResolvConf = helper.BackupAndCopyResolvConf }) + + tests := []struct { + name string + expectedErr string + manualCustomizations *imagedefinition.Manual + }{ + { + name: "failing manualCopyFile", + expectedErr: "cp: cannot stat 'this/path/does/not/exist'", + manualCustomizations: &imagedefinition.Manual{ + CopyFile: []*imagedefinition.CopyFile{ + { + Source: filepath.Join("this", "path", "does", "not", "exist"), + Dest: filepath.Join("this", "path", "does", "not", "exist"), + }, + }, + }, + }, + { + name: "failing manualExecute", + expectedErr: "chroot: failed to run command", + manualCustomizations: &imagedefinition.Manual{ + Execute: []*imagedefinition.Execute{ + { + ExecutePath: filepath.Join("this", "path", "does", "not", "exist"), + }, + }, + }, + }, + { + name: "failing manualTouchFile", + expectedErr: "no such file or directory", + manualCustomizations: &imagedefinition.Manual{ + TouchFile: []*imagedefinition.TouchFile{ + { + TouchPath: filepath.Join("this", "path", "does", "not", "exist"), + }, + }, + }, + }, + { + name: "failing manualAddGroup", + expectedErr: "group 'root' already exists", + manualCustomizations: &imagedefinition.Manual{ + AddGroup: []*imagedefinition.AddGroup{ + { + GroupName: "root", + GroupID: "0", + }, + }, + }, + }, + { + name: "failing manualAddUser", + expectedErr: "user 'root' already exists", + manualCustomizations: &imagedefinition.Manual{ + AddUser: []*imagedefinition.AddUser{ + { + UserName: "root", + UserID: "0", + }, + }, + }, + }, + } + asserter := helper.Asserter{T: t} + + var stateMachine ClassicStateMachine + stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() + stateMachine.parent = &stateMachine + stateMachine.ImageDef = imagedefinition.ImageDefinition{ + Architecture: getHostArch(), + Series: getHostSuite(), + Rootfs: &imagedefinition.Rootfs{ + Archive: "ubuntu", + }, + } + + // need workdir set up for this + err := stateMachine.makeTemporaryDirectories() + asserter.AssertErrNil(err, true) + + // also create chroot + err = stateMachine.createChroot() + asserter.AssertErrNil(err, true) + + // create an /etc/resolv.conf in the chroot + err = os.MkdirAll(filepath.Join(stateMachine.tempDirs.chroot, "etc"), 0755) + asserter.AssertErrNil(err, true) + _, err = os.Create(filepath.Join(stateMachine.tempDirs.chroot, "etc", "resolv.conf")) + asserter.AssertErrNil(err, true) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + asserter := helper.Asserter{T: t} + saveCWD := helper.SaveCWD() + t.Cleanup(saveCWD) + + stateMachine.ImageDef.Customization = &imagedefinition.Customization{ + Manual: tc.manualCustomizations, + } + + err = stateMachine.manualCustomization() + + if len(tc.expectedErr) == 0 { + asserter.AssertErrNil(err, true) + } else { + asserter.AssertErrContains(err, tc.expectedErr) + } + + }) + } + os.RemoveAll(stateMachine.stateMachineFlags.WorkDir) } // TestPrepareClassicImage unit tests the prepareClassicImage function @@ -2232,52 +2363,6 @@ func TestSuccessfulClassicRun(t *testing.T) { }) } -// TestCheckEmptyFields unit tests the helper.CheckEmptyFields function -func TestCheckEmptyFields(t *testing.T) { - // define the struct we will use to test - type testStruct struct { - A string `yaml:"a" json:"fieldA,required"` - B string `yaml:"b" json:"fieldB"` - C string `yaml:"c" json:"fieldC,omitempty"` - } - - // generate the schema for our testStruct - var jsonReflector jsonschema.Reflector - schema := jsonReflector.Reflect(&testStruct{}) - - // now run CheckEmptyFields with a variety of test data - // to ensure the correct return values - testCases := []struct { - name string - structData testStruct - shouldPass bool - }{ - {"success", testStruct{A: "foo", B: "bar", C: "baz"}, true}, - {"missing_explicitly_required", testStruct{B: "bar", C: "baz"}, false}, - {"missing_implicitly_required", testStruct{A: "foo", C: "baz"}, false}, - {"missing_omitempty", testStruct{A: "foo", B: "bar"}, true}, - } - for i, tc := range testCases { - t.Run("test_check_empty_fields_"+tc.name, func(t *testing.T) { - asserter := helper.Asserter{T: t} - - result := new(gojsonschema.Result) - err := helper.CheckEmptyFields(&testCases[i].structData, result, schema) - asserter.AssertErrNil(err, false) - schema.Required = append(schema.Required, "fieldA") - - // make sure validation will fail only when expected - if tc.shouldPass && !result.Valid() { - t.Error("CheckEmptyFields added errors when it should not have") - } - if !tc.shouldPass && result.Valid() { - t.Error("CheckEmptyFields did NOT add errors when it should have") - } - - }) - } -} - // TestGerminate tests the germinate state and ensures some necessary packages are included func TestGerminate(t *testing.T) { testCases := []struct { @@ -2531,6 +2616,8 @@ func TestBuildGadgetTreeDirectory(t *testing.T) { gitCloneCommand := *exec.Command( "git", "clone", + "--depth", + "1", "--branch", "classic", "https://github.com/snapcore/pc-gadget", @@ -2566,6 +2653,123 @@ func TestBuildGadgetTreeDirectory(t *testing.T) { }) } +func TestStateMachine_buildGadgetTree_paths(t *testing.T) { + asserter := helper.Asserter{T: t} + // git clone the gadget into a /tmp dir + originGadgetDir, err := os.MkdirTemp("", "pc-gadget-") + asserter.AssertErrNil(err, true) + t.Cleanup(func() { + err = os.RemoveAll(originGadgetDir) + if err != nil { + t.Error(err) + } + }) + gitCloneCommand := *exec.Command( + "git", + "clone", + "--depth", + "1", + "--branch", + "classic", + "https://github.com/snapcore/pc-gadget", + originGadgetDir, + ) + err = gitCloneCommand.Run() + asserter.AssertErrNil(err, true) + + tmpDir, err := os.MkdirTemp("", "") + t.Cleanup(func() { + err := osRemoveAll(tmpDir) + if err != nil { + t.Error(err) + } + }) + + testCases := []struct { + name string + gadgetDir string + }{ + { + name: "gadget URL poiting to an absolute dir", + gadgetDir: originGadgetDir, + }, + { + name: "gadget URL pointing to an absolute sub dir", + gadgetDir: filepath.Join(tmpDir, "a", "b"), + }, + { + name: "gadget URL pointing to a relative sub dir", + gadgetDir: filepath.Join("a", "b"), + }, + } + + for _, tc := range testCases { + t.Run("test_build_gadget_tree_paths_"+tc.name, func(t *testing.T) { + asserter := helper.Asserter{T: t} + restoreCWD := helper.SaveCWD() + defer restoreCWD() + + var stateMachine ClassicStateMachine + stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() + stateMachine.parent = &stateMachine + + // need workdir set up for this + err := stateMachine.makeTemporaryDirectories() + asserter.AssertErrNil(err, true) + t.Cleanup(func() { + err := os.RemoveAll(stateMachine.stateMachineFlags.WorkDir) + if err != nil { + t.Error(err) + } + }) + + // move the original gadget dir to the desire location to test it will be found + if originGadgetDir != tc.gadgetDir { + fullGadgetDir := tc.gadgetDir + if !filepath.IsAbs(tc.gadgetDir) { + fullGadgetDir = filepath.Join(tmpDir, tc.gadgetDir) + } + + err = os.MkdirAll(filepath.Dir(fullGadgetDir), 0777) + asserter.AssertErrNil(err, true) + + err = os.Rename(originGadgetDir, fullGadgetDir) + asserter.AssertErrNil(err, true) + // move it back once the test is done + t.Cleanup(func() { + err := os.Rename(fullGadgetDir, originGadgetDir) + if err != nil { + t.Error(err) + } + }) + } + + // now set up the image definition to build from this directory + stateMachine.ImageDef = imagedefinition.ImageDefinition{ + Architecture: getHostArch(), + Series: getHostSuite(), + Gadget: &imagedefinition.Gadget{ + GadgetURL: fmt.Sprintf("file://%s", tc.gadgetDir), + GadgetType: "directory", + }, + } + + err = stateMachine.setConfDefDir(filepath.Join(tmpDir, "image_definition.yaml")) + asserter.AssertErrNil(err, true) + + err = stateMachine.buildGadgetTree() + asserter.AssertErrNil(err, true) + + // now make sure the gadget.yaml is in the expected location + // this was a bug reported by the CPC team + err = stateMachine.prepareGadgetTree() + asserter.AssertErrNil(err, true) + err = stateMachine.loadGadgetYaml() + asserter.AssertErrNil(err, true) + }) + } +} + // TestGadgetGadgetTargets tests using alternate make targets with gadget builds func TestGadgetGadgetTargets(t *testing.T) { testCases := []struct { diff --git a/internal/statemachine/helper.go b/internal/statemachine/helper.go index 82368920..51ef8107 100644 --- a/internal/statemachine/helper.go +++ b/internal/statemachine/helper.go @@ -57,6 +57,16 @@ func (stateMachine *StateMachine) validateInput() error { return nil } +func (stateMachine *StateMachine) setConfDefDir(confFileArg string) error { + path, err := filepath.Abs(filepath.Dir(confFileArg)) + if err != nil { + return fmt.Errorf("unable to determine the configuration definition directory: %w", err) + } + stateMachine.ConfDefPath = path + + return nil +} + // validateUntilThru validates that the the state passed as --until // or --thru exists in the state machine's list of states func (stateMachine *StateMachine) validateUntilThru() error { @@ -795,30 +805,25 @@ func mountTempFS(targetDir, scratchDir, mountpoint string) (mountCmds, umountCmd } // manualCopyFile copies a file into the chroot -func manualCopyFile(copyFileInterfaces interface{}, targetDir string, debug bool) error { - copyFileSlice := reflect.ValueOf(copyFileInterfaces) - for i := 0; i < copyFileSlice.Len(); i++ { - copyFile := copyFileSlice.Index(i).Interface().(*imagedefinition.CopyFile) - - // Copy the file into the specified location in the chroot - dest := filepath.Join(targetDir, copyFile.Dest) +func manualCopyFile(customizations []*imagedefinition.CopyFile, confDefPath string, targetDir string, debug bool) error { + for _, c := range customizations { + source := filepath.Join(confDefPath, c.Source) + dest := filepath.Join(targetDir, c.Dest) if debug { - fmt.Printf("Copying file \"%s\" to \"%s\"\n", copyFile.Source, dest) + fmt.Printf("Copying file \"%s\" to \"%s\"\n", source, dest) } - if err := osutilCopySpecialFile(copyFile.Source, dest); err != nil { + if err := osutilCopySpecialFile(source, dest); err != nil { return fmt.Errorf("Error copying file \"%s\" into chroot: %s", - copyFile.Source, err.Error()) + source, err.Error()) } } return nil } -// manualExecute executes an executable file in the chroot -func manualExecute(executeInterfaces interface{}, targetDir string, debug bool) error { - executeSlice := reflect.ValueOf(executeInterfaces) - for i := 0; i < executeSlice.Len(); i++ { - execute := executeSlice.Index(i).Interface().(*imagedefinition.Execute) - executeCmd := execCommand("chroot", targetDir, execute.ExecutePath) +// manualExecute executes executable files in the chroot +func manualExecute(customizations []*imagedefinition.Execute, targetDir string, debug bool) error { + for _, c := range customizations { + executeCmd := execCommand("chroot", targetDir, c.ExecutePath) if debug { fmt.Printf("Executing command \"%s\"\n", executeCmd.String()) } @@ -832,12 +837,10 @@ func manualExecute(executeInterfaces interface{}, targetDir string, debug bool) return nil } -// manualTouchFile touches a file in the chroot -func manualTouchFile(touchFileInterfaces interface{}, targetDir string, debug bool) error { - touchFileSlice := reflect.ValueOf(touchFileInterfaces) - for i := 0; i < touchFileSlice.Len(); i++ { - touchFile := touchFileSlice.Index(i).Interface().(*imagedefinition.TouchFile) - fullPath := filepath.Join(targetDir, touchFile.TouchPath) +// manualTouchFile touches files in the chroot +func manualTouchFile(customizations []*imagedefinition.TouchFile, targetDir string, debug bool) error { + for _, c := range customizations { + fullPath := filepath.Join(targetDir, c.TouchPath) if debug { fmt.Printf("Creating empty file \"%s\"\n", fullPath) } @@ -849,16 +852,14 @@ func manualTouchFile(touchFileInterfaces interface{}, targetDir string, debug bo return nil } -// manualAddGroup adds a group in the chroot -func manualAddGroup(addGroupInterfaces interface{}, targetDir string, debug bool) error { - addGroupSlice := reflect.ValueOf(addGroupInterfaces) - for i := 0; i < addGroupSlice.Len(); i++ { - addGroup := addGroupSlice.Index(i).Interface().(*imagedefinition.AddGroup) - addGroupCmd := execCommand("chroot", targetDir, "groupadd", addGroup.GroupName) - debugStatement := fmt.Sprintf("Adding group \"%s\"\n", addGroup.GroupName) - if addGroup.GroupID != "" { - addGroupCmd.Args = append(addGroupCmd.Args, []string{"--gid", addGroup.GroupID}...) - debugStatement = fmt.Sprintf("%s with GID %s\n", strings.TrimSpace(debugStatement), addGroup.GroupID) +// manualAddGroup adds groups in the chroot +func manualAddGroup(customizations []*imagedefinition.AddGroup, targetDir string, debug bool) error { + for _, c := range customizations { + addGroupCmd := execCommand("chroot", targetDir, "groupadd", c.GroupName) + debugStatement := fmt.Sprintf("Adding group \"%s\"\n", c.GroupName) + if c.GroupID != "" { + addGroupCmd.Args = append(addGroupCmd.Args, []string{"--gid", c.GroupID}...) + debugStatement = fmt.Sprintf("%s with GID %s\n", strings.TrimSpace(debugStatement), c.GroupID) } if debug { fmt.Print(debugStatement) @@ -873,16 +874,14 @@ func manualAddGroup(addGroupInterfaces interface{}, targetDir string, debug bool return nil } -// manualAddUser adds a group in the chroot -func manualAddUser(addUserInterfaces interface{}, targetDir string, debug bool) error { - addUserSlice := reflect.ValueOf(addUserInterfaces) - for i := 0; i < addUserSlice.Len(); i++ { - addUser := addUserSlice.Index(i).Interface().(*imagedefinition.AddUser) - addUserCmd := execCommand("chroot", targetDir, "useradd", addUser.UserName) - debugStatement := fmt.Sprintf("Adding user \"%s\"\n", addUser.UserName) - if addUser.UserID != "" { - addUserCmd.Args = append(addUserCmd.Args, []string{"--uid", addUser.UserID}...) - debugStatement = fmt.Sprintf("%s with UID %s\n", strings.TrimSpace(debugStatement), addUser.UserID) +// manualAddUser adds users in the chroot +func manualAddUser(customizations []*imagedefinition.AddUser, targetDir string, debug bool) error { + for _, c := range customizations { + addUserCmd := execCommand("chroot", targetDir, "useradd", c.UserName) + debugStatement := fmt.Sprintf("Adding user \"%s\"\n", c.UserName) + if c.UserID != "" { + addUserCmd.Args = append(addUserCmd.Args, []string{"--uid", c.UserID}...) + debugStatement = fmt.Sprintf("%s with UID %s\n", strings.TrimSpace(debugStatement), c.UserID) } if debug { fmt.Print(debugStatement) diff --git a/internal/statemachine/helper_test.go b/internal/statemachine/helper_test.go index 62cbd05f..c24d526b 100644 --- a/internal/statemachine/helper_test.go +++ b/internal/statemachine/helper_test.go @@ -795,7 +795,7 @@ func TestFailedManualCopyFile(t *testing.T) { Source: "/test/does/not/exist", }, } - err := manualCopyFile(copyFiles, "/fakedir", true) + err := manualCopyFile(copyFiles, "/tmp", "/fakedir", true) asserter.AssertErrContains(err, "Error copying file") }) } @@ -1414,3 +1414,76 @@ func TestFailedUpdateGrub(t *testing.T) { execCommand = exec.Command }) } + +func TestStateMachine_setConfDefDir(t *testing.T) { + tests := []struct { + name string + confFileArg string + expectedError string + wantPath string + absBroken bool + }{ + { + name: "simple case", + confFileArg: "ubuntu-server.yaml", + wantPath: "/tmp/simple_case", + }, + { + name: "conf in subdir", + confFileArg: "subdir/ubuntu-server.yaml", + wantPath: "/tmp/conf_in_subdir/subdir", + }, + { + name: "conf in parent", + confFileArg: "../ubuntu-server.yaml", + wantPath: "/tmp", + }, + { + name: "conf at root", + confFileArg: "../../../../../../../../../../..//ubuntu-server.yaml", + wantPath: "/", + }, + { + name: "fail to get conf directory", + confFileArg: "ubuntu-server.yaml", + wantPath: "", + absBroken: true, + expectedError: "unable to determine the configuration definition directory", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + asserter := helper.Asserter{T: t} + tName := strings.ReplaceAll(tc.name, " ", "_") + + tmpDirPath := filepath.Join("/tmp", tName) + saveCWD := helper.SaveCWD() + defer saveCWD() + + err := os.Mkdir(tmpDirPath, 0755) + t.Cleanup(func() { + os.RemoveAll(tmpDirPath) + }) + asserter.AssertErrNil(err, true) + + err = os.Chdir(tmpDirPath) + asserter.AssertErrNil(err, true) + + if tc.absBroken { + os.RemoveAll(tmpDirPath) + } + + stateMachine := &StateMachine{} + err = stateMachine.setConfDefDir(tc.confFileArg) + if len(tc.expectedError) == 0 { + asserter.AssertErrNil(err, true) + } else { + asserter.AssertErrContains(err, tc.expectedError) + } + + if tc.wantPath != stateMachine.ConfDefPath { + t.Errorf("Expected \"%s\" but got \"%s\"", tc.wantPath, stateMachine.ConfDefPath) + } + }) + } +} diff --git a/internal/statemachine/snap.go b/internal/statemachine/snap.go index 2d60e198..3a8c57c8 100644 --- a/internal/statemachine/snap.go +++ b/internal/statemachine/snap.go @@ -37,6 +37,10 @@ func (snapStateMachine *SnapStateMachine) Setup() error { // set the states that will be used for this image type snapStateMachine.states = snapStates + if err := snapStateMachine.setConfDefDir(snapStateMachine.parent.(*SnapStateMachine).Args.ModelAssertion); err != nil { + return err + } + // do the validation common to all image types if err := snapStateMachine.validateInput(); err != nil { return err diff --git a/internal/statemachine/snap_test.go b/internal/statemachine/snap_test.go index ae8d0b5a..2884a3e8 100644 --- a/internal/statemachine/snap_test.go +++ b/internal/statemachine/snap_test.go @@ -21,6 +21,35 @@ import ( "github.com/canonical/ubuntu-image/internal/helper" ) +// TestSnapStateMachine_Setup_Fail_setConfDefDir tests a failure in the Setup() function when setting the configuration definition directory +func TestSnapStateMachine_Setup_Fail_setConfDefDir(t *testing.T) { + t.Run("test_failed_set_conf_dir", func(t *testing.T) { + asserter := helper.Asserter{T: t} + saveCWD := helper.SaveCWD() + defer saveCWD() + + var stateMachine SnapStateMachine + stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() + stateMachine.parent = &stateMachine + + tmpDirPath := filepath.Join("/tmp", "test_failed_set_conf_dir") + err := os.Mkdir(tmpDirPath, 0755) + t.Cleanup(func() { + os.RemoveAll(tmpDirPath) + }) + asserter.AssertErrNil(err, true) + + err = os.Chdir(tmpDirPath) + asserter.AssertErrNil(err, true) + + _ = os.RemoveAll(tmpDirPath) + + err = stateMachine.Setup() + asserter.AssertErrContains(err, "unable to determine the configuration definition directory") + os.RemoveAll(stateMachine.stateMachineFlags.WorkDir) + }) +} + // TestFailedValidateInputSnap tests a failure in the Setup() function when validating common input func TestFailedSnapSetup(t *testing.T) { testCases := []struct { diff --git a/internal/statemachine/state_machine.go b/internal/statemachine/state_machine.go index 21f1215f..cd75097f 100644 --- a/internal/statemachine/state_machine.go +++ b/internal/statemachine/state_machine.go @@ -97,7 +97,8 @@ type StateMachine struct { cleanWorkDir bool // whether or not to clean up the workDir CurrentStep string // tracks the current progress of the state machine StepsTaken int // counts the number of steps taken - YamlFilePath string // the location for the yaml file + ConfDefPath string // directory holding the model assertion / image definition file + YamlFilePath string // the location for the gadget yaml file IsSeeded bool // core 20 images are seeded rootfsVolName string // volume on which the rootfs is located rootfsPartNum int // rootfs partition number diff --git a/internal/statemachine/testdata/metadata/reference_successful_write.json b/internal/statemachine/testdata/metadata/reference_successful_write.json index 4c7309df..22f01773 100644 --- a/internal/statemachine/testdata/metadata/reference_successful_write.json +++ b/internal/statemachine/testdata/metadata/reference_successful_write.json @@ -1 +1 @@ -{"CurrentStep":"","StepsTaken":2,"YamlFilePath":"/tmp/ubuntu-image-2329554237/unpack/gadget/meta/gadget.yaml","IsSeeded":true,"SectorSize":512,"RootfsSize":775915520,"GadgetInfo":{"Volumes":{"pc":{"schema":"gpt","bootloader":"grub","id":"","structure":[{"name":"mbr","filesystem-label":"","offset":0,"offset-write":null,"min-size":440,"size":440,"type":"mbr","role":"mbr","id":"","filesystem":"","content":[{"source":"","target":"","image":"pc-boot.img","offset":null,"size":0,"unpack":false}],"update":{"edition":1,"preserve":null}}]}},"Defaults":null,"Connections":null,"KernelCmdline":{"Allow":null}},"ImageSizes":{"pc":3155165184},"VolumeOrder":["pc"],"VolumeNames":{"pc":"pc.img"}} \ No newline at end of file +{"CurrentStep":"","StepsTaken":2,"ConfDefPath":"","YamlFilePath":"/tmp/ubuntu-image-2329554237/unpack/gadget/meta/gadget.yaml","IsSeeded":true,"SectorSize":512,"RootfsSize":775915520,"GadgetInfo":{"Volumes":{"pc":{"schema":"gpt","bootloader":"grub","id":"","structure":[{"name":"mbr","filesystem-label":"","offset":0,"offset-write":null,"min-size":440,"size":440,"type":"mbr","role":"mbr","id":"","filesystem":"","content":[{"source":"","target":"","image":"pc-boot.img","offset":null,"size":0,"unpack":false}],"update":{"edition":1,"preserve":null}}]}},"Defaults":null,"Connections":null,"KernelCmdline":{"Allow":null}},"ImageSizes":{"pc":3155165184},"VolumeOrder":["pc"],"VolumeNames":{"pc":"pc.img"}} \ No newline at end of file