Skip to content

Commit f5da457

Browse files
committed
Fix reference passing when yaml unmarshal
For example: metadata := new(api.ImageMetadata) // type: *api.ImageMetadata err = yaml.Unmarshal(content, &metadata) It works on `yaml.v2` and `yaml.v3` but not work under `goccy/go-yaml`, and `goccy/go-yaml` behavior is correct. So fixed it and add test cases to pretect. Signed-off-by: JUN JIE NAN <[email protected]>
1 parent faa4880 commit f5da457

File tree

7 files changed

+311
-7
lines changed

7 files changed

+311
-7
lines changed

cmd/incus-agent/templates.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func templatesApply(path string) ([]string, error) {
2727
}
2828

2929
metadata := new(api.ImageMetadata)
30-
err = yaml.Unmarshal(content, &metadata)
30+
err = yaml.Unmarshal(content, metadata)
3131
if err != nil {
3232
return nil, fmt.Errorf("Could not parse metadata.yaml: %w", err)
3333
}

cmd/incus-agent/templates_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
const (
10+
testingImageMetadata = `architecture: x86_64
11+
creation_date: 1744625602
12+
expiry_date: 1747217602
13+
properties:
14+
architecture: x86_64
15+
description: openEuler 22.03-LTS-SP4
16+
# no templates
17+
`
18+
)
19+
20+
func PrepareMetadata(t *testing.T) (metaPath string, rb func()) {
21+
t.Helper()
22+
metaPath, err := os.MkdirTemp("", "metadata")
23+
if err != nil {
24+
t.Fatal(err)
25+
}
26+
27+
err = os.WriteFile(filepath.Join(metaPath, "metadata.yaml"), []byte(testingImageMetadata), 0o655)
28+
if err != nil {
29+
t.Fatal(err)
30+
}
31+
32+
rb = func() {
33+
err = os.RemoveAll(metaPath)
34+
if err != nil {
35+
t.Fatal(err)
36+
}
37+
}
38+
39+
return
40+
}
41+
42+
func TestTemplateApply(t *testing.T) {
43+
metaDir, rb := PrepareMetadata(t)
44+
defer rb()
45+
filenames, err := templatesApply(metaDir)
46+
if err != nil {
47+
t.Fatal(err)
48+
}
49+
50+
if len(filenames) != 0 {
51+
t.Fatal(filenames)
52+
}
53+
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
const (
10+
testingInstanceTypes = `aws:
11+
c1.medium:
12+
cpu: 2.0
13+
mem: 1.7
14+
gce:
15+
f1-micro:
16+
cpu: 0.2
17+
mem: 0.6
18+
g1-small:
19+
cpu: 0.5
20+
mem: 1.7
21+
azure:
22+
A10:
23+
cpu: 8.0
24+
mem: 56.0
25+
A11:
26+
cpu: 16.0
27+
mem: 112.0
28+
Standard_H8m:
29+
cpu: 8.0
30+
mem: 112.0
31+
`
32+
)
33+
34+
func PrepareIncusCache(t *testing.T, data []byte) (rb func()) {
35+
t.Helper()
36+
incusDir, err := os.MkdirTemp("", "IncusDir")
37+
if err != nil {
38+
t.Fatal()
39+
}
40+
41+
cacheDir := filepath.Join(incusDir, "cache")
42+
err = os.MkdirAll(cacheDir, 0o755)
43+
if err != nil {
44+
t.Fatal(err)
45+
}
46+
47+
err = os.WriteFile(filepath.Join(cacheDir, "instance_types.yaml"), data, 0o644)
48+
if err != nil {
49+
t.Fatal(err)
50+
}
51+
52+
os.Setenv("INCUS_DIR", incusDir)
53+
rb = func() {
54+
os.Unsetenv("INCUS_DIR")
55+
os.RemoveAll(incusDir)
56+
}
57+
58+
return
59+
}
60+
61+
func TestInstanceLoadCache(t *testing.T) {
62+
rb := PrepareIncusCache(t, []byte(testingInstanceTypes))
63+
defer rb()
64+
if instanceTypes != nil {
65+
t.Fatal(instanceTypes)
66+
}
67+
68+
err := instanceLoadCache()
69+
if err != nil {
70+
t.Fatal(err)
71+
}
72+
73+
if len(instanceTypes) == 0 {
74+
t.Fatal(instanceTypes)
75+
}
76+
77+
aws := len(instanceTypes["aws"])
78+
gce := len(instanceTypes["gce"])
79+
azure := len(instanceTypes["azure"])
80+
if aws != 1 || gce != 2 || azure != 3 {
81+
t.Fatal(aws, gce, azure)
82+
}
83+
}

internal/server/instance/drivers/driver_qemu.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3230,6 +3230,16 @@ func (d *qemu) generateConfigShare() error {
32303230
return nil
32313231
}
32323232

3233+
func loadImageMetadata(content []byte) (*api.ImageMetadata, error) {
3234+
metadata := new(api.ImageMetadata)
3235+
err := yaml.Unmarshal(content, metadata)
3236+
if err != nil {
3237+
return nil, fmt.Errorf("Could not parse %s: %w", string(content), err)
3238+
}
3239+
3240+
return metadata, nil
3241+
}
3242+
32333243
func (d *qemu) templateApplyNow(trigger instance.TemplateTrigger, path string) error {
32343244
// If there's no metadata, just return.
32353245
fname := filepath.Join(d.Path(), "metadata.yaml")
@@ -3242,11 +3252,9 @@ func (d *qemu) templateApplyNow(trigger instance.TemplateTrigger, path string) e
32423252
if err != nil {
32433253
return fmt.Errorf("Failed to read metadata: %w", err)
32443254
}
3245-
3246-
metadata := new(api.ImageMetadata)
3247-
err = yaml.Unmarshal(content, &metadata)
3255+
metadata, err := loadImageMetadata(content)
32483256
if err != nil {
3249-
return fmt.Errorf("Could not parse %s: %w", fname, err)
3257+
return err
32503258
}
32513259

32523260
// Figure out the instance architecture.
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package drivers
2+
3+
import (
4+
"testing"
5+
)
6+
7+
const (
8+
testingImageMetadata01 = `architecture: x86_64
9+
creation_date: 1727609459
10+
expiry_date: 1730201459
11+
properties:
12+
architecture: amd64
13+
description: Ubuntu jammy
14+
name: ubuntu-disco-x86_64
15+
templates:
16+
/etc/hostname:
17+
when:
18+
- create
19+
- copy
20+
create_only: false
21+
template: hostname.tpl
22+
properties: {}
23+
`
24+
testingImageMetadata02 = `architecture: x86_64
25+
creation_date: 1744625602
26+
expiry_date: 1747217602
27+
properties:
28+
architecture: x86_64
29+
description: openEuler 22.03-LTS-SP4
30+
name: openEuler-22.03-LTS-SP4-x86_64-cloud-01
31+
`
32+
)
33+
34+
func TestLoadImageMetadata(t *testing.T) {
35+
tcs := []struct {
36+
name string
37+
data []byte
38+
want01 string
39+
want02 int
40+
}{
41+
{"ubuntu", []byte(testingImageMetadata01), "Ubuntu jammy", 4},
42+
{"openEuler", []byte(testingImageMetadata02), "openEuler 22.03-LTS-SP4", 3},
43+
}
44+
for _, tc := range tcs {
45+
t.Run(tc.name, func(t *testing.T) {
46+
metadata, err := loadImageMetadata(tc.data)
47+
if err != nil {
48+
t.Fatal(err)
49+
}
50+
want01 := metadata.Properties["description"]
51+
want02 := len(metadata.Properties) + len(metadata.Templates)
52+
if want01 != tc.want01 {
53+
t.Fatal(want01, tc.want01)
54+
}
55+
56+
if want02 != tc.want02 {
57+
t.Fatal(want02, tc.want02)
58+
}
59+
})
60+
}
61+
}

shared/cliconfig/file.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func LoadConfig(path string) (*Config, error) {
6464

6565
// Decode the YAML document
6666
c := NewConfig(configDir, false)
67-
err = yaml.Unmarshal(content, &c)
67+
err = yaml.Unmarshal(content, c)
6868
if err != nil {
6969
return nil, fmt.Errorf("Unable to decode the configuration: %w", err)
7070
}
@@ -80,7 +80,7 @@ func LoadConfig(path string) (*Config, error) {
8080
globalConf := NewConfig("", false)
8181
content, err = os.ReadFile(globalConf.GlobalConfigPath("config.yml"))
8282
if err == nil {
83-
err = yaml.Unmarshal(content, &globalConf)
83+
err = yaml.Unmarshal(content, globalConf)
8484
if err != nil {
8585
return nil, fmt.Errorf("Unable to decode the configuration: %w", err)
8686
}

shared/cliconfig/file_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package cliconfig_test
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
8+
"github.com/lxc/incus/v6/shared/cliconfig"
9+
)
10+
11+
const (
12+
testingGlobalContent01 = `default-remote: local
13+
remotes:
14+
images:
15+
addr: https://images.linuxcontainers.org
16+
protocol: simplestreams
17+
public: true
18+
lxd:
19+
addr: unix:///var/snap/lxd/common/lxd/unix.socket
20+
auth_type: file access
21+
protocol: incus
22+
public: false
23+
aliases: {}`
24+
25+
testingLocalContent01 = `default-remote: aarch64
26+
remotes:
27+
aarch64:
28+
addr: https://aarch64.lxd:8443
29+
auth_type: tls
30+
project: default
31+
protocol: incus
32+
public: false
33+
local:
34+
addr: unix://
35+
project: user-1000
36+
protocol: incus
37+
public: false
38+
amd64:
39+
addr: https://amd64.lxd:8443
40+
auth_type: tls
41+
project: default
42+
protocol: incus
43+
public: false
44+
aliases: {}
45+
`
46+
)
47+
48+
func PrepareConfigFile(t *testing.T, gdata, ldata string) (rb func()) {
49+
t.Helper()
50+
configDir, err := os.MkdirTemp("", "TestLoadConfig")
51+
if err != nil {
52+
t.Fatal(err)
53+
}
54+
55+
globalDir := filepath.Join(configDir, "global")
56+
localDir := filepath.Join(configDir, "local")
57+
err = os.MkdirAll(globalDir, 0o755)
58+
if err != nil {
59+
t.Fatal(err)
60+
}
61+
62+
err = os.MkdirAll(localDir, 0o755)
63+
if err != nil {
64+
t.Fatal(err)
65+
}
66+
67+
err = os.WriteFile(filepath.Join(globalDir, "config.yml"), []byte(gdata), 0o644)
68+
if err != nil {
69+
t.Fatal(err)
70+
}
71+
72+
err = os.WriteFile(filepath.Join(localDir, "config.yml"), []byte(ldata), 0o644)
73+
if err != nil {
74+
t.Fatal(err)
75+
}
76+
77+
os.Setenv("INCUS_GLOBAL_CONF", globalDir)
78+
os.Setenv("INCUS_CONF", localDir)
79+
rb = func() {
80+
os.Unsetenv("INCUS_CONF")
81+
os.Unsetenv("INCUS_GLOBAL_CONF")
82+
os.RemoveAll(configDir)
83+
}
84+
85+
return
86+
}
87+
88+
func TestLoadConfig(t *testing.T) {
89+
rb := PrepareConfigFile(t, testingGlobalContent01, testingLocalContent01)
90+
defer rb()
91+
config, err := cliconfig.LoadConfig("")
92+
if err != nil {
93+
t.Fatal(err)
94+
}
95+
96+
if config.DefaultRemote != "aarch64" || len(config.Remotes) != 5 {
97+
t.Fatal(config)
98+
}
99+
}

0 commit comments

Comments
 (0)