From fd3923d25b5fd3920dc62994dba4d71629e4d075 Mon Sep 17 00:00:00 2001 From: Norio Nomura Date: Mon, 16 Sep 2024 11:41:53 +0900 Subject: [PATCH 1/5] yqutil: fix to preserve line breaks. `yamlfmt` works by replacing line breaks with place holder strings to avoid their loss when using `gopkg.in/yaml.v3`. By replacing line breaks with place holder strings before `yqlib` uses `gopkg.in/yaml.v3`, we can ultimately prevent the loss of line breaks through `yamlfmt`. Signed-off-by: Norio Nomura --- pkg/yqutil/yqutil.go | 45 ++++++++++++++++++++++++++++++++++++++- pkg/yqutil/yqutil_test.go | 1 + 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/pkg/yqutil/yqutil.go b/pkg/yqutil/yqutil.go index 139d9fcbce3..8787797f40b 100644 --- a/pkg/yqutil/yqutil.go +++ b/pkg/yqutil/yqutil.go @@ -1,6 +1,7 @@ package yqutil import ( + "bufio" "bytes" "fmt" "os" @@ -15,13 +16,17 @@ import ( // EvaluateExpression evaluates the yq expression, and returns the modified yaml. func EvaluateExpression(expression string, content []byte) ([]byte, error) { logrus.Debugf("Evaluating yq expression: %q", expression) + contentModified, err := replaceLineBreaksWithMagicString(content) + if err != nil { + return nil, err + } tmpYAMLFile, err := os.CreateTemp("", "lima-yq-*.yaml") if err != nil { return nil, err } tmpYAMLPath := tmpYAMLFile.Name() defer os.RemoveAll(tmpYAMLPath) - _, err = tmpYAMLFile.Write(content) + _, err = tmpYAMLFile.Write(contentModified) if err != nil { tmpYAMLFile.Close() return nil, err @@ -94,3 +99,41 @@ func yamlfmt(content []byte) ([]byte, error) { } return formatter.Format(content) } + +const yamlfmtLineBreakPlaceholder = "#magic___^_^___line" + +type paddinger struct { + strings.Builder +} + +func (p *paddinger) adjust(txt string) { + var indentSize int + for i := 0; i < len(txt) && txt[i] == ' '; i++ { // yaml only allows space to indent. + indentSize++ + } + // Grows if the given size is larger than us and always return the max padding. + for diff := indentSize - p.Len(); diff > 0; diff-- { + p.WriteByte(' ') + } +} + +func replaceLineBreaksWithMagicString(content []byte) ([]byte, error) { + // hotfix: yq does not support line breaks in the middle of a string. + var buf bytes.Buffer + reader := bytes.NewReader(content) + scanner := bufio.NewScanner(reader) + var padding paddinger + for scanner.Scan() { + txt := scanner.Text() + padding.adjust(txt) + if strings.TrimSpace(txt) == "" { // line break or empty space line. + buf.WriteString(padding.String()) // prepend some padding incase literal multiline strings. + buf.WriteString(yamlfmtLineBreakPlaceholder) + buf.WriteString("\n") + } else { + buf.WriteString(txt) + buf.WriteString("\n") + } + } + return buf.Bytes(), scanner.Err() +} diff --git a/pkg/yqutil/yqutil_test.go b/pkg/yqutil/yqutil_test.go index fcf1260ec24..287c51c6be2 100644 --- a/pkg/yqutil/yqutil_test.go +++ b/pkg/yqutil/yqutil_test.go @@ -19,6 +19,7 @@ memory: null expected := ` # CPUs cpus: 2 + # Memory size memory: 2GiB ` From cc577be69695e86bb35140994e919afbe9f257b4 Mon Sep 17 00:00:00 2001 From: Norio Nomura Date: Tue, 17 Sep 2024 18:01:53 +0900 Subject: [PATCH 2/5] examples/*.yaml: format templates by using `limactl edit --set 'del(.nothing)'` `find examples -name '*.yaml' -exec limactl edit --set 'del(.nothing)' {} \;` Signed-off-by: Norio Nomura --- examples/buildkit.yaml | 10 +++++----- examples/default.yaml | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/examples/buildkit.yaml b/examples/buildkit.yaml index 6d27c80ae6c..125c33bef80 100644 --- a/examples/buildkit.yaml +++ b/examples/buildkit.yaml @@ -5,11 +5,11 @@ # $ export BUILDKIT_HOST=$(limactl list buildkit --format 'unix://{{.Dir}}/sock/buildkitd.sock') # $ buildctl debug workers message: | - To run `buildkit` on the host (assumes buildctl is installed), run the following commands: - ------- - export BUILDKIT_HOST="unix://{{.Dir}}/sock/buildkitd.sock" - buildctl debug workers - ------- + To run `buildkit` on the host (assumes buildctl is installed), run the following commands: + ------- + export BUILDKIT_HOST="unix://{{.Dir}}/sock/buildkitd.sock" + buildctl debug workers + ------- images: # Try to use release-yyyyMMdd image if available. Note that release-yyyyMMdd will be removed after several months. - location: "https://cloud-images.ubuntu.com/releases/24.04/release-20240821/ubuntu-24.04-server-cloudimg-amd64.img" diff --git a/examples/default.yaml b/examples/default.yaml index 5b269ff7e25..d8bb61ec5da 100644 --- a/examples/default.yaml +++ b/examples/default.yaml @@ -264,10 +264,10 @@ containerd: # Setting of instructions is supported like this: "qemu64,+ssse3". # 🟢 Builtin default: hard-coded arch map with type (see the output of `limactl info | jq .defaultTemplate.cpuType`) cpuType: - # aarch64: "cortex-a72" # (or "host" when running on aarch64 host) - # armv7l: "cortex-a7" # (or "host" when running on armv7l host) - # riscv64: "rv64" # (or "host" when running on riscv64 host) - # x86_64: "qemu64" # (or "host,-pdpe1gb" when running on x86_64 host) +# aarch64: "cortex-a72" # (or "host" when running on aarch64 host) +# armv7l: "cortex-a7" # (or "host" when running on armv7l host) +# riscv64: "rv64" # (or "host" when running on riscv64 host) +# x86_64: "qemu64" # (or "host,-pdpe1gb" when running on x86_64 host) rosetta: # Enable Rosetta for Linux (EXPERIMENTAL; will graduate from experimental in Lima v1.0). @@ -456,8 +456,8 @@ hostResolver: # predefined to specify the gateway address to the host. # 🟢 Builtin default: null hosts: - # guest.name: 127.1.1.1 - # host.name: host.lima.internal + # guest.name: 127.1.1.1 + # host.name: host.lima.internal # If hostResolver.enabled is false, then the following rules apply for configuring dns: # Explicitly set DNS addresses for qemu user-mode networking. By default qemu picks *one* From beeeccf2d35ef1f7dd71e31303fe7f1724733c9c Mon Sep 17 00:00:00 2001 From: Norio Nomura Date: Tue, 17 Sep 2024 18:46:44 +0900 Subject: [PATCH 3/5] test.yaml: verify templates match `limactl edit` format Signed-off-by: Norio Nomura --- .github/workflows/test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f503fd79f10..7d012d7351a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -90,6 +90,10 @@ jobs: run: make - name: Install run: sudo make install + - name: Verify templates match `limactl edit` format + run: | + find examples -name '*.yaml' -exec limactl edit --set 'del(.nothing)' {} \; + git diff-index --exit-code HEAD - name: Uninstall run: sudo make uninstall From bf44aa538ec5f45cd982dfb21f8f97bbe92d71d3 Mon Sep 17 00:00:00 2001 From: Norio Nomura Date: Wed, 18 Sep 2024 06:44:42 +0900 Subject: [PATCH 4/5] default.yaml: apply review Signed-off-by: Norio Nomura --- examples/default.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/default.yaml b/examples/default.yaml index d8bb61ec5da..d071d88974d 100644 --- a/examples/default.yaml +++ b/examples/default.yaml @@ -264,10 +264,10 @@ containerd: # Setting of instructions is supported like this: "qemu64,+ssse3". # 🟢 Builtin default: hard-coded arch map with type (see the output of `limactl info | jq .defaultTemplate.cpuType`) cpuType: -# aarch64: "cortex-a72" # (or "host" when running on aarch64 host) -# armv7l: "cortex-a7" # (or "host" when running on armv7l host) -# riscv64: "rv64" # (or "host" when running on riscv64 host) -# x86_64: "qemu64" # (or "host,-pdpe1gb" when running on x86_64 host) +# aarch64: "cortex-a72" # (or "host" when running on aarch64 host) +# armv7l: "cortex-a7" # (or "host" when running on armv7l host) +# riscv64: "rv64" # (or "host" when running on riscv64 host) +# x86_64: "qemu64" # (or "host,-pdpe1gb" when running on x86_64 host) rosetta: # Enable Rosetta for Linux (EXPERIMENTAL; will graduate from experimental in Lima v1.0). @@ -456,8 +456,8 @@ hostResolver: # predefined to specify the gateway address to the host. # 🟢 Builtin default: null hosts: - # guest.name: 127.1.1.1 - # host.name: host.lima.internal + # guest.name: 127.1.1.1 + # host.name: host.lima.internal # If hostResolver.enabled is false, then the following rules apply for configuring dns: # Explicitly set DNS addresses for qemu user-mode networking. By default qemu picks *one* From f2d6a55612d8c75f43552833dbc1522ffef16bfd Mon Sep 17 00:00:00 2001 From: Norio Nomura Date: Wed, 18 Sep 2024 14:00:58 +0900 Subject: [PATCH 5/5] yqutil: stop adapting internal codes from `yamlfmt` Signed-off-by: Norio Nomura --- pkg/yqutil/yqutil.go | 69 +++++++++++++++----------------------------- 1 file changed, 23 insertions(+), 46 deletions(-) diff --git a/pkg/yqutil/yqutil.go b/pkg/yqutil/yqutil.go index 8787797f40b..ea7ef9c811c 100644 --- a/pkg/yqutil/yqutil.go +++ b/pkg/yqutil/yqutil.go @@ -1,12 +1,12 @@ package yqutil import ( - "bufio" "bytes" "fmt" "os" "strings" + "github.com/google/yamlfmt" "github.com/google/yamlfmt/formatters/basic" "github.com/mikefarah/yq/v4/pkg/yqlib" "github.com/sirupsen/logrus" @@ -16,7 +16,16 @@ import ( // EvaluateExpression evaluates the yq expression, and returns the modified yaml. func EvaluateExpression(expression string, content []byte) ([]byte, error) { logrus.Debugf("Evaluating yq expression: %q", expression) - contentModified, err := replaceLineBreaksWithMagicString(content) + formatter, err := yamlfmtBasicFormatter() + if err != nil { + return nil, err + } + // `ApplyFeatures()` is being called directly before passing content to `yqlib`. + // This results in `ApplyFeatures()` being called twice with `FeatureApplyBefore`: + // once here and once inside `formatter.Format`. + // Currently, calling `ApplyFeatures()` with `FeatureApplyBefore` twice is not an issue, + // but future changes to `yamlfmt` might cause problems if it is called twice. + contentModified, err := formatter.Features.ApplyFeatures(content, yamlfmt.FeatureApplyBefore) if err != nil { return nil, err } @@ -75,7 +84,7 @@ func EvaluateExpression(expression string, content []byte) ([]byte, error) { return nil, err } - return yamlfmt(out.Bytes()) + return formatter.Format(out.Bytes()) } func Join(yqExprs []string) string { @@ -85,55 +94,23 @@ func Join(yqExprs []string) string { return strings.Join(yqExprs, " | ") } -func yamlfmt(content []byte) ([]byte, error) { +func yamlfmtBasicFormatter() (*basic.BasicFormatter, error) { factory := basic.BasicFormatterFactory{} config := map[string]interface{}{ - "indentless_arrays": true, - "line_ending": "lf", // prefer LF even on Windows - "pad_line_comments": 2, - "retain_line_breaks": true, // does not affect to the output because yq removes empty lines before formatting + "indentless_arrays": true, + "line_ending": "lf", // prefer LF even on Windows + "pad_line_comments": 2, + "retain_line_breaks": true, + "retain_line_breaks_single": false, } + formatter, err := factory.NewFormatter(config) if err != nil { return nil, err } - return formatter.Format(content) -} - -const yamlfmtLineBreakPlaceholder = "#magic___^_^___line" - -type paddinger struct { - strings.Builder -} - -func (p *paddinger) adjust(txt string) { - var indentSize int - for i := 0; i < len(txt) && txt[i] == ' '; i++ { // yaml only allows space to indent. - indentSize++ - } - // Grows if the given size is larger than us and always return the max padding. - for diff := indentSize - p.Len(); diff > 0; diff-- { - p.WriteByte(' ') - } -} - -func replaceLineBreaksWithMagicString(content []byte) ([]byte, error) { - // hotfix: yq does not support line breaks in the middle of a string. - var buf bytes.Buffer - reader := bytes.NewReader(content) - scanner := bufio.NewScanner(reader) - var padding paddinger - for scanner.Scan() { - txt := scanner.Text() - padding.adjust(txt) - if strings.TrimSpace(txt) == "" { // line break or empty space line. - buf.WriteString(padding.String()) // prepend some padding incase literal multiline strings. - buf.WriteString(yamlfmtLineBreakPlaceholder) - buf.WriteString("\n") - } else { - buf.WriteString(txt) - buf.WriteString("\n") - } + basicFormatter, ok := formatter.(*basic.BasicFormatter) + if !ok { + return nil, fmt.Errorf("unexpected formatter type: %T", formatter) } - return buf.Bytes(), scanner.Err() + return basicFormatter, nil }