From a5b712b2758091dd4c3a661771b3b9c8b29a182f Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 10 Feb 2026 16:35:08 -0800 Subject: [PATCH] confutil: fix toml rewriting in new package Seems marshalling the typed struct causes empty fields e.g. gcpolicy = [] that (old versions of?) BuildKit do no allow. Signed-off-by: Tonis Tiigi --- tests/inspect.go | 4 +-- util/confutil/container.go | 61 ++++++++++++++++++++++++-------- util/confutil/container_test.go | 62 +++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 17 deletions(-) create mode 100644 util/confutil/container_test.go diff --git a/tests/inspect.go b/tests/inspect.go index 47a5c4373..682a68128 100644 --- a/tests/inspect.go +++ b/tests/inspect.go @@ -131,10 +131,10 @@ insecure-entitlements = [ "network.host", "security.insecure" ] ` expectedContent := `debug = true -insecure-entitlements = ["network.host", "security.insecure"] +insecure-entitlements = ['network.host', 'security.insecure'] [log] - format = "text" +format = 'text' ` var builderName string diff --git a/util/confutil/container.go b/util/confutil/container.go index 9fc1cd97d..35321664c 100644 --- a/util/confutil/container.go +++ b/util/confutil/container.go @@ -4,6 +4,7 @@ import ( "io" "os" "path" + "path/filepath" "regexp" buildkitdconfig "github.com/moby/buildkit/cmd/buildkitd/config" @@ -30,6 +31,10 @@ func LoadConfigFiles(bkconfig string) (map[string][]byte, error) { } else if err != nil { return nil, errors.Wrapf(err, "invalid buildkit configuration file: %s", bkconfig) } + dt, err := readFile(bkconfig) + if err != nil { + return nil, errors.Wrapf(err, "failed to read buildkit configuration file: %s", bkconfig) + } cfg, err := buildkitdconfig.LoadFile(bkconfig) if err != nil { @@ -37,6 +42,18 @@ func LoadConfigFiles(bkconfig string) (map[string][]byte, error) { } m := make(map[string][]byte) + // unmarshal the config file to a map because marshalling the struct back can cause errors with empty fields on buildkit side + var conf map[string]any + if err := toml.Unmarshal(dt, &conf); err != nil { + return nil, errors.Wrapf(err, "failed to parse buildkit configuration file: %s", bkconfig) + } + if conf == nil { + conf = map[string]any{} + } + registry, hadRegistry := conf["registry"].(map[string]any) + if registry == nil { + registry = map[string]any{} + } // Iterate through registry config to copy certs and update // BuildKit config with the underlying certs' path in the container. @@ -58,12 +75,18 @@ func LoadConfigFiles(bkconfig string) (map[string][]byte, error) { // cert="/etc/buildkit/certs/myregistry.io/cert.pem" if cfg.Registries != nil { for regName, regConf := range cfg.Registries { + regOut, ok := registry[regName].(map[string]any) + if !ok { + return nil, errors.Errorf("invalid registry config for %q", regName) + } + pfx := path.Join("certs", reInvalidCertsDir.ReplaceAllString(regName, "_")) if regCAs := regConf.RootCAs; len(regCAs) > 0 { - var cas []string + cas := make([]string, 0, len(regCAs)) for _, ca := range regCAs { - fp := path.Join(pfx, path.Base(ca)) - cas = append(cas, path.Join(DefaultBuildKitConfigDir, fp)) + fp := path.Join(pfx, filepath.Base(ca)) + dst := path.Join(DefaultBuildKitConfigDir, fp) + cas = append(cas, dst) dt, err := readFile(ca) if err != nil { @@ -71,14 +94,17 @@ func LoadConfigFiles(bkconfig string) (map[string][]byte, error) { } m[fp] = dt } - regConf.RootCAs = cas + regOut["ca"] = cas } if regKeyPairs := regConf.KeyPairs; len(regKeyPairs) > 0 { - for i, kp := range regKeyPairs { + keypairs := make([]map[string]any, 0, len(regKeyPairs)) + for _, kp := range regKeyPairs { + kpv := map[string]any{} key := kp.Key if len(key) > 0 { - fp := path.Join(pfx, path.Base(key)) - kp.Key = path.Join(DefaultBuildKitConfigDir, fp) + fp := path.Join(pfx, filepath.Base(key)) + dst := path.Join(DefaultBuildKitConfigDir, fp) + kpv["key"] = dst dt, err := readFile(key) if err != nil { return nil, errors.Wrapf(err, "failed to read key file: %s", key) @@ -87,26 +113,31 @@ func LoadConfigFiles(bkconfig string) (map[string][]byte, error) { } cert := kp.Certificate if len(cert) > 0 { - fp := path.Join(pfx, path.Base(cert)) - kp.Certificate = path.Join(DefaultBuildKitConfigDir, fp) + fp := path.Join(pfx, filepath.Base(cert)) + dst := path.Join(DefaultBuildKitConfigDir, fp) + kpv["cert"] = dst dt, err := readFile(cert) if err != nil { return nil, errors.Wrapf(err, "failed to read cert file: %s", cert) } m[fp] = dt } - regConf.KeyPairs[i] = kp + keypairs = append(keypairs, kpv) } + regOut["keypair"] = keypairs } - cfg.Registries[regName] = regConf + registry[regName] = regOut } } - dt, err := toml.Marshal(cfg) - if err != nil { - return nil, err + if hadRegistry || len(registry) > 0 { + conf["registry"] = registry } - m["buildkitd.toml"] = dt + out, err := toml.Marshal(conf) + if err != nil { + return nil, errors.Wrap(err, "failed to marshal buildkit configuration file") + } + m["buildkitd.toml"] = out return m, nil } diff --git a/util/confutil/container_test.go b/util/confutil/container_test.go new file mode 100644 index 000000000..9b9aa37ad --- /dev/null +++ b/util/confutil/container_test.go @@ -0,0 +1,62 @@ +package confutil + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestLoadConfigFilesMirrorOnlyPreservesTOML(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "buildkitd.toml") + cfg := `[registry."docker.io"] +mirrors=["127.0.0.1:5000"] +` + require.NoError(t, os.WriteFile(cfgPath, []byte(cfg), 0644)) + + m, err := LoadConfigFiles(cfgPath) + require.NoError(t, err) + + got := string(m["buildkitd.toml"]) + require.Equal(t, `[registry] +[registry.'docker.io'] +mirrors = ['127.0.0.1:5000'] +`, got) +} + +func TestLoadConfigFilesRewritesRegistryCertPaths(t *testing.T) { + dir := t.TempDir() + caPath := filepath.Join(dir, "myca.pem") + keyPath := filepath.Join(dir, "mykey.pem") + certPath := filepath.Join(dir, "mycert.pem") + require.NoError(t, os.WriteFile(caPath, []byte("ca"), 0644)) + require.NoError(t, os.WriteFile(keyPath, []byte("key"), 0644)) + require.NoError(t, os.WriteFile(certPath, []byte("cert"), 0644)) + + cfgPath := filepath.Join(dir, "buildkitd.toml") + cfg := `[registry."myregistry.io"] +ca=['` + caPath + `'] +[[registry."myregistry.io".keypair]] +key='` + keyPath + `' +cert='` + certPath + `' +` + require.NoError(t, os.WriteFile(cfgPath, []byte(cfg), 0644)) + + m, err := LoadConfigFiles(cfgPath) + require.NoError(t, err) + + got := string(m["buildkitd.toml"]) + require.Equal(t, `[registry] +[registry.'myregistry.io'] +ca = ['/etc/buildkit/certs/myregistry.io/myca.pem'] + +[[registry.'myregistry.io'.keypair]] +cert = '/etc/buildkit/certs/myregistry.io/mycert.pem' +key = '/etc/buildkit/certs/myregistry.io/mykey.pem' +`, got) + require.Equal(t, []byte("ca"), m["certs/myregistry.io/myca.pem"]) + require.Equal(t, []byte("key"), m["certs/myregistry.io/mykey.pem"]) + require.Equal(t, []byte("cert"), m["certs/myregistry.io/mycert.pem"]) +}