Merge pull request #144 from canonical/path-relative-to-image-definition

FR-5403 - Pathname for manual customization not relative to image definition file

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

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")
+			}
 		})
 	}
 }

internal/imagedefinition/README.rst

@@ -58,6 +58,8 @@ The following specification defines what is supported in the YAML:
          target: <string> (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: <string> (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: <string> (required if tarball dict is specified)
              # URL to the gpg signature to verify the tarball against.
              gpg: <string> (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: <string>
                # The path to use as a destination for the copied
                # file. The location of the rootfs will be prepended

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

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)