Repository: lima-vm/lima
Pull request #2334 (2024-05-10 09:44:46)

Add unittest for pkg/limayaml/default.yaml
Make sure that it is possible to parse default.yaml to the expected LimaYAML structure

Add some minor changes, to make the internal representation (before defaults) "pretty"

Left to do is to remove the default list of architectures, from the default cpuType list:

* #2333

After that, the default YAML is simply `images: []` - since that value is still mandatory.

```yaml
images: null
```

With some additional default values and "omitempty", it also matches an empty template.

```yaml
vmType: null
os: null
arch: null
images: []
cpus: null
memory: null
disk: null
mountType: null
mountInotify: null
additionalDisks: null
ssh:
  localPort: null
  loadDotSSHPubKeys: null
  forwardAgent: null
  forwardX11: null
  forwardX11Trusted: null
caCerts:
  removeDefaults: null
  files: null
  certs: null
upgradePackages: null
containerd:
  system: null
  user: null
cpuType: null
rosetta:
  enabled: null
  binfmt: null
timezone: null
firmware:
  legacyBIOS: null
audio:
  device: null
video:
  display: null
  vnc:
    display: null
networks: null
propagateProxyEnv: null
hostResolver:
  enabled: null
  ipv6: null
  hosts: null
guestInstallPrefix: null
plain: null
```

afbjorklund: The unmarshal and marshal functions should probably be refactored into separate files.

pkg/limayaml/load.go

pkg/limayaml/limayaml_test.go
afbjorklund: Attempt to add separate "Save" functionality:

* #2338

Also added basic test for "Validate" function:

* #2337 
AkihiroSuda: Could you rebase and squash the commits?

Patch:
diff --git a/examples/default.yaml b/examples/default.yaml
index b471f4fe..36d04ac9 100644
--- a/examples/default.yaml
+++ b/examples/default.yaml
@@ -125,7 +125,7 @@ ssh:
   # 🟢 Builtin default: 0 (automatically assigned to a free port)
   # NOTE: when the instance name is "default", the builtin default value is set to
   # 60022 for backward compatibility.
-  localPort: 0
+  localPort: null
   # Load ~/.ssh/*.pub in addition to $LIMA_HOME/_config/user.pub .
   # This option is useful when you want to use other SSH-based
   # applications such as rsync with the Lima instance.
diff --git a/pkg/downloader/downloader.go b/pkg/downloader/downloader.go
index aa79c68f..10c6c5f8 100644
--- a/pkg/downloader/downloader.go
+++ b/pkg/downloader/downloader.go
@@ -13,7 +13,6 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"github.com/cheggaaa/pb/v3"
 	"github.com/containerd/continuity/fs"
@@ -45,8 +44,6 @@ const (
 type Result struct {
 	Status          Status
 	CachePath       string // "/Users/foo/Library/Caches/lima/download/by-url-sha256/<SHA256_OF_URL>/data"
-	LastModified    time.Time
-	ContentType     string
 	ValidatedDigest bool
 }
 
@@ -121,38 +118,6 @@ func WithExpectedDigest(expectedDigest digest.Digest) Opt {
 	}
 }
 
-func readFile(path string) string {
-	if path == "" {
-		return ""
-	}
-	if _, err := os.Stat(path); err != nil {
-		return ""
-	}
-	b, err := os.ReadFile(path)
-	if err != nil {
-		return ""
-	}
-	return string(b)
-}
-
-func readTime(path string) time.Time {
-	if path == "" {
-		return time.Time{}
-	}
-	if _, err := os.Stat(path); err != nil {
-		return time.Time{}
-	}
-	b, err := os.ReadFile(path)
-	if err != nil {
-		return time.Time{}
-	}
-	t, err := time.Parse(http.TimeFormat, string(b))
-	if err != nil {
-		return time.Time{}
-	}
-	return t
-}
-
 // Download downloads the remote resource into the local path.
 //
 // Download caches the remote resource if WithCache or WithCacheDir option is specified.
@@ -210,7 +175,7 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
 	}
 
 	if o.cacheDir == "" {
-		if err := downloadHTTP(ctx, localPath, "", "", remote, o.description, o.expectedDigest); err != nil {
+		if err := downloadHTTP(ctx, localPath, remote, o.description, o.expectedDigest); err != nil {
 			return nil, err
 		}
 		res := &Result{
@@ -222,8 +187,6 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
 
 	shad := cacheDirectoryPath(o.cacheDir, remote)
 	shadData := filepath.Join(shad, "data")
-	shadTime := filepath.Join(shad, "time")
-	shadType := filepath.Join(shad, "type")
 	shadDigest, err := cacheDigestPath(shad, o.expectedDigest)
 	if err != nil {
 		return nil, err
@@ -247,8 +210,6 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
 		res := &Result{
 			Status:          StatusUsedCache,
 			CachePath:       shadData,
-			LastModified:    readTime(shadTime),
-			ContentType:     readFile(shadType),
 			ValidatedDigest: o.expectedDigest != "",
 		}
 		return res, nil
@@ -263,7 +224,7 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
 	if err := os.WriteFile(shadURL, []byte(remote), 0o644); err != nil {
 		return nil, err
 	}
-	if err := downloadHTTP(ctx, shadData, shadTime, shadType, remote, o.description, o.expectedDigest); err != nil {
+	if err := downloadHTTP(ctx, shadData, remote, o.description, o.expectedDigest); err != nil {
 		return nil, err
 	}
 	// no need to pass the digest to copyLocal(), as we already verified the digest
@@ -278,8 +239,6 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
 	res := &Result{
 		Status:          StatusDownloaded,
 		CachePath:       shadData,
-		LastModified:    readTime(shadTime),
-		ContentType:     readFile(shadType),
 		ValidatedDigest: o.expectedDigest != "",
 	}
 	return res, nil
@@ -307,8 +266,6 @@ func Cached(remote string, opts ...Opt) (*Result, error) {
 
 	shad := cacheDirectoryPath(o.cacheDir, remote)
 	shadData := filepath.Join(shad, "data")
-	shadTime := filepath.Join(shad, "time")
-	shadType := filepath.Join(shad, "type")
 	shadDigest, err := cacheDigestPath(shad, o.expectedDigest)
 	if err != nil {
 		return nil, err
@@ -328,8 +285,6 @@ func Cached(remote string, opts ...Opt) (*Result, error) {
 	res := &Result{
 		Status:          StatusUsedCache,
 		CachePath:       shadData,
-		LastModified:    readTime(shadTime),
-		ContentType:     readFile(shadType),
 		ValidatedDigest: o.expectedDigest != "",
 	}
 	return res, nil
@@ -338,8 +293,6 @@ func Cached(remote string, opts ...Opt) (*Result, error) {
 // cacheDirectoryPath returns the cache subdirectory path.
 //   - "url" file contains the url
 //   - "data" file contains the data
-//   - "time" file contains the time (Last-Modified header)
-//   - "type" file contains the type (Content-Type header)
 func cacheDirectoryPath(cacheDir, remote string) string {
 	return filepath.Join(cacheDir, "download", "by-url-sha256", fmt.Sprintf("%x", sha256.Sum256([]byte(remote))))
 }
@@ -517,7 +470,7 @@ func validateLocalFileDigest(localPath string, expectedDigest digest.Digest) err
 	return nil
 }
 
-func downloadHTTP(ctx context.Context, localPath, lastModified, contentType, url, description string, expectedDigest digest.Digest) error {
+func downloadHTTP(ctx context.Context, localPath, url, description string, expectedDigest digest.Digest) error {
 	if localPath == "" {
 		return fmt.Errorf("downloadHTTP: got empty localPath")
 	}
@@ -536,18 +489,6 @@ func downloadHTTP(ctx context.Context, localPath, lastModified, contentType, url
 	if err != nil {
 		return err
 	}
-	if lastModified != "" {
-		lm := resp.Header.Get("Last-Modified")
-		if err := os.WriteFile(lastModified, []byte(lm), 0o644); err != nil {
-			return err
-		}
-	}
-	if contentType != "" {
-		ct := resp.Header.Get("Content-Type")
-		if err := os.WriteFile(contentType, []byte(ct), 0o644); err != nil {
-			return err
-		}
-	}
 	defer resp.Body.Close()
 	bar, err := progressbar.New(resp.ContentLength)
 	if err != nil {
diff --git a/pkg/limayaml/limayaml.go b/pkg/limayaml/limayaml.go
index 1a2e5ce5..a6761b6c 100644
--- a/pkg/limayaml/limayaml.go
+++ b/pkg/limayaml/limayaml.go
@@ -72,8 +72,8 @@ const (
 )
 
 type Rosetta struct {
-	Enabled *bool `yaml:"enabled" json:"enabled"`
-	BinFmt  *bool `yaml:"binfmt" json:"binfmt"`
+	Enabled *bool `yaml:"enabled,omitempty" json:"enabled,omitempty"`
+	BinFmt  *bool `yaml:"binfmt,omitempty" json:"binfmt,omitempty"`
 }
 
 type File struct {


Test patch:
diff --git a/pkg/downloader/downloader_test.go b/pkg/downloader/downloader_test.go
index d2111200..e5de0d65 100644
--- a/pkg/downloader/downloader_test.go
+++ b/pkg/downloader/downloader_test.go
@@ -10,7 +10,6 @@ import (
 	"runtime"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/opencontainers/go-digest"
 	"gotest.tools/v3/assert"
@@ -26,8 +25,6 @@ func TestDownloadRemote(t *testing.T) {
 	t.Cleanup(ts.Close)
 	dummyRemoteFileURL := ts.URL + "/downloader.txt"
 	const dummyRemoteFileDigest = "sha256:380481d26f897403368be7cb86ca03a4bc14b125bfaf2b93bff809a5a2ad717e"
-	dummyRemoteFileStat, err := os.Stat(filepath.Join("testdata", "downloader.txt"))
-	assert.NilError(t, err)
 
 	t.Run("without cache", func(t *testing.T) {
 		t.Run("without digest", func(t *testing.T) {
@@ -108,17 +105,6 @@ func TestDownloadRemote(t *testing.T) {
 		_, err = Cached(dummyRemoteFileURL, WithExpectedDigest(wrongDigest), WithCacheDir(cacheDir))
 		assert.ErrorContains(t, err, "expected digest")
 	})
-	t.Run("metadata", func(t *testing.T) {
-		_, err := Cached(dummyRemoteFileURL, WithExpectedDigest(dummyRemoteFileDigest))
-		assert.ErrorContains(t, err, "cache directory to be specified")
-
-		cacheDir := filepath.Join(t.TempDir(), "cache")
-		r, err := Download(context.Background(), "", dummyRemoteFileURL, WithExpectedDigest(dummyRemoteFileDigest), WithCacheDir(cacheDir))
-		assert.NilError(t, err)
-		assert.Equal(t, StatusDownloaded, r.Status)
-		assert.Equal(t, dummyRemoteFileStat.ModTime().Truncate(time.Second).UTC(), r.LastModified)
-		assert.Equal(t, "text/plain; charset=utf-8", r.ContentType)
-	})
 }
 
 func TestDownloadLocal(t *testing.T) {
diff --git a/pkg/limayaml/limayaml_test.go b/pkg/limayaml/limayaml_test.go
new file mode 100644
index 00000000..b75e9e0b
--- /dev/null
+++ b/pkg/limayaml/limayaml_test.go
@@ -0,0 +1,43 @@
+package limayaml
+
+import (
+	"encoding/json"
+	"os"
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func dumpJSON(t *testing.T, d interface{}) string {
+	b, err := json.Marshal(d)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return string(b)
+}
+
+const emptyYAML = "images: []\n"
+
+func TestEmptyYAML(t *testing.T) {
+	var y LimaYAML
+	t.Log(dumpJSON(t, y))
+	b, err := marshalYAML(y)
+	assert.NilError(t, err)
+	assert.Equal(t, string(b), emptyYAML)
+}
+
+const defaultYAML = "images: []\n"
+
+func TestDefaultYAML(t *testing.T) {
+	bytes, err := os.ReadFile("default.yaml")
+	assert.NilError(t, err)
+	var y LimaYAML
+	err = unmarshalYAML(bytes, &y, "")
+	assert.NilError(t, err)
+	y.Images = nil // remove default images
+	y.Mounts = nil // remove default mounts
+	t.Log(dumpJSON(t, y))
+	b, err := marshalYAML(y)
+	assert.NilError(t, err)
+	assert.Equal(t, string(b), defaultYAML)
+}