From 65590669d79e7bfc63a4ecb44dcc02a46badee52 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Wed, 21 Aug 2024 08:10:42 +0200 Subject: [PATCH] Don't treat kine data sources as URLs They aren't (necessarily) valid URLs, hence url.Parse is inappropriate. Introuduce the pkg/config/kine package to provide some utility functions. Signed-off-by: Tom Wieczorek --- pkg/apis/k0s/v1beta1/storage.go | 39 +++++++------- pkg/backup/manager_unix.go | 27 +++++++--- pkg/backup/sqlitedb_unix.go | 40 ++++---------- pkg/component/controller/kine.go | 36 +++++++------ pkg/config/kine/datasource.go | 91 ++++++++++++++++++++++++++++++++ 5 files changed, 163 insertions(+), 70 deletions(-) create mode 100644 pkg/config/kine/datasource.go diff --git a/pkg/apis/k0s/v1beta1/storage.go b/pkg/apis/k0s/v1beta1/storage.go index 200067e0f3d8..15f966d9e33e 100644 --- a/pkg/apis/k0s/v1beta1/storage.go +++ b/pkg/apis/k0s/v1beta1/storage.go @@ -26,6 +26,7 @@ import ( "strings" "github.com/k0sproject/k0s/internal/pkg/iface" + "github.com/k0sproject/k0s/pkg/config/kine" "github.com/k0sproject/k0s/pkg/constant" "k8s.io/apimachinery/pkg/util/validation/field" @@ -180,37 +181,37 @@ func (e *EtcdConfig) GetNodeName() (string, error) { // DefaultKineConfig creates KineConfig with sane defaults func DefaultKineConfig(dataDir string) *KineConfig { - // https://www.sqlite.org/c3ref/open.html#urifilenamesinsqlite3open return &KineConfig{ - DataSource: (&url.URL{ - Scheme: "sqlite", + // https://www.sqlite.org/c3ref/open.html#urifilenamesinsqlite3open + DataSource: fmt.Sprintf("sqlite://%s", &url.URL{ + Scheme: "file", OmitHost: true, Path: filepath.ToSlash(filepath.Join(dataDir, "db", "state.db")), RawQuery: "mode=rwc&_journal=WAL&cache=shared", - }).String(), + }), } } func (k *KineConfig) IsJoinable() bool { - if scheme, _, found := strings.Cut(k.DataSource, ":"); found { - switch scheme { - case "", "sqlite": - return false - - case "nats": - if u, err := url.Parse(k.DataSource); err == nil { - if q, err := url.ParseQuery(u.RawQuery); err == nil { - return q.Has("noEmbed") - } - } - return false + backend, dsn, err := kine.SplitDataSource(k.DataSource) + if err != nil { + return false + } - default: - return true + switch backend { + case "sqlite": + return false + + case "nats": + if u, err := url.Parse(dsn); err == nil { + if q, err := url.ParseQuery(u.RawQuery); err == nil { + return q.Has("noEmbed") + } } + return false } - return false + return true } // GetEndpointsAsString returns comma-separated list of external cluster endpoints if exist diff --git a/pkg/backup/manager_unix.go b/pkg/backup/manager_unix.go index 752abd2dd99a..bbe73219272f 100644 --- a/pkg/backup/manager_unix.go +++ b/pkg/backup/manager_unix.go @@ -24,7 +24,6 @@ import ( "os" "path" "path/filepath" - "strings" "github.com/sirupsen/logrus" @@ -32,6 +31,7 @@ import ( "github.com/k0sproject/k0s/internal/pkg/file" "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" "github.com/k0sproject/k0s/pkg/config" + "github.com/k0sproject/k0s/pkg/config/kine" ) // Manager hold configuration for particular backup-restore process @@ -80,13 +80,26 @@ func (bm *Manager) RunBackup(nodeSpec *v1beta1.ClusterSpec, vars *config.CfgVars } func (bm *Manager) discoverSteps(configFilePath string, nodeSpec *v1beta1.ClusterSpec, vars *config.CfgVars, action string, restoredConfigPath string, out io.Writer) { - if nodeSpec.Storage.Type == v1beta1.EtcdStorageType && !nodeSpec.Storage.Etcd.IsExternalClusterUsed() { - bm.Add(newEtcdStep(bm.tmpDir, vars.CertRootDir, vars.EtcdCertDir, nodeSpec.Storage.Etcd.PeerAddress, vars.EtcdDataDir)) - } else if nodeSpec.Storage.Type == v1beta1.KineStorageType && strings.HasPrefix(nodeSpec.Storage.Kine.DataSource, "sqlite:") { - bm.Add(newSqliteStep(bm.tmpDir, nodeSpec.Storage.Kine.DataSource)) - } else { - logrus.Warnf("only internal etcd and sqlite %s are supported. Other storage backends must be backed-up/restored manually.", action) + switch nodeSpec.Storage.Type { + case v1beta1.EtcdStorageType: + if nodeSpec.Storage.Etcd.IsExternalClusterUsed() { + logrus.Warnf("%s is not supported for an external etcd cluster, it must be done manually", action) + } else { + bm.Add(newEtcdStep(bm.tmpDir, vars.CertRootDir, vars.EtcdCertDir, nodeSpec.Storage.Etcd.PeerAddress, vars.EtcdDataDir)) + } + + case v1beta1.KineStorageType: + if backend, dsn, err := kine.SplitDataSource(nodeSpec.Storage.Kine.DataSource); err != nil { + logrus.WithError(err).Warnf("cannot %s kine data source, it must be done manually", action) + } else if backend != "sqlite" { + logrus.Warnf("%s is not supported for %q kine data sources, it must be done manually", action, backend) + } else if dbPath, err := kine.GetSQLiteFilePath(vars.DataDir, dsn); err != nil { + logrus.WithError(err).Warnf("cannot %s SQLite database file, it must be done manually", action) + } else { + bm.Add(newSqliteStep(bm.tmpDir, dbPath)) + } } + bm.dataDir = vars.DataDir for _, path := range []string{ vars.CertRootDir, diff --git a/pkg/backup/sqlitedb_unix.go b/pkg/backup/sqlitedb_unix.go index 8651be2ef30b..ca8fdc661d4e 100644 --- a/pkg/backup/sqlitedb_unix.go +++ b/pkg/backup/sqlitedb_unix.go @@ -20,7 +20,6 @@ package backup import ( "fmt" - "net/url" "os" "path/filepath" @@ -35,28 +34,23 @@ import ( const kineBackup = "kine-state-backup.db" type sqliteStep struct { - dataSource string - tmpDir string + dbPath string + tmpDir string } -func newSqliteStep(tmpDir string, dataSource string) *sqliteStep { +func newSqliteStep(tmpDir string, dbPath string) *sqliteStep { return &sqliteStep{ - tmpDir: tmpDir, - dataSource: dataSource, + tmpDir: tmpDir, + dbPath: dbPath, } } func (s *sqliteStep) Name() string { - dbPath, _ := s.getKineDBPath() - return fmt.Sprintf("sqlite db path %s", dbPath) + return fmt.Sprintf("sqlite db path %s", s.dbPath) } func (s *sqliteStep) Backup() (StepResult, error) { - dbPath, err := s.getKineDBPath() - if err != nil { - return StepResult{}, err - } - kineDB, err := db.Open(dbPath) + kineDB, err := db.Open(s.dbPath) if err != nil { return StepResult{}, err } @@ -80,27 +74,15 @@ func (s *sqliteStep) Restore(restoreFrom string, _ string) error { if !file.Exists(snapshotPath) { return fmt.Errorf("sqlite snapshot not found at %s", snapshotPath) } - dbPath, err := s.getKineDBPath() - if err != nil { - return err - } // make sure DB dir exists. if not, create it. - dbPathDir := filepath.Dir(dbPath) - if err = dir.Init(dbPathDir, constant.KineDBDirMode); err != nil { + dbPathDir := filepath.Dir(s.dbPath) + if err := dir.Init(dbPathDir, constant.KineDBDirMode); err != nil { return err } - logrus.Infof("restoring sqlite db to `%s`", dbPath) - if err := file.Copy(snapshotPath, dbPath); err != nil { + logrus.Infof("restoring sqlite db to `%s`", s.dbPath) + if err := file.Copy(snapshotPath, s.dbPath); err != nil { logrus.Errorf("failed to restore snapshot from disk: %v", err) } return nil } - -func (s *sqliteStep) getKineDBPath() (string, error) { - u, err := url.Parse(s.dataSource) - if err != nil { - return "", fmt.Errorf("failed to parse Kind datasource string: %w", err) - } - return u.Path, nil -} diff --git a/pkg/component/controller/kine.go b/pkg/component/controller/kine.go index 9a7785af9e9d..3f6cffff00c0 100644 --- a/pkg/component/controller/kine.go +++ b/pkg/component/controller/kine.go @@ -18,6 +18,7 @@ package controller import ( "context" + "errors" "fmt" "net/url" "os" @@ -27,6 +28,7 @@ import ( "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" "github.com/k0sproject/k0s/pkg/component/manager" "github.com/k0sproject/k0s/pkg/config" + "github.com/k0sproject/k0s/pkg/config/kine" "github.com/k0sproject/k0s/pkg/etcd" clientv3 "go.etcd.io/etcd/client/v3" @@ -70,22 +72,26 @@ func (k *Kine) Init(_ context.Context) error { logrus.Warn("failed to chown ", kineSocketDir) } - dsURL, err := url.Parse(k.Config.DataSource) - if err != nil { - return err - } - if dsURL.Scheme == "sqlite" { - // Make sure the db basedir exists - err = dir.Init(filepath.Dir(dsURL.Path), constant.KineDBDirMode) + if backend, dsn, err := kine.SplitDataSource(k.Config.DataSource); err != nil { + return fmt.Errorf("unsupported kine data source: %w", err) + } else if backend == "sqlite" { + dbPath, err := kine.GetSQLiteFilePath(k.K0sVars.DataDir, dsn) if err != nil { - return fmt.Errorf("failed to create dir %s: %w", filepath.Dir(dsURL.Path), err) - } - err = os.Chown(filepath.Dir(dsURL.Path), k.uid, k.gid) - if err != nil && os.Geteuid() == 0 { - return fmt.Errorf("failed to chown dir %s: %w", filepath.Dir(dsURL.Path), err) - } - if err := os.Chown(dsURL.Path, k.uid, k.gid); err != nil && os.Geteuid() == 0 { - logrus.Warningf("datasource file %s does not exist", dsURL.Path) + logrus.WithError(err).Debug("Skipping SQLite database file initialization") + } else { + // Make sure the db basedir exists + dbDir := filepath.Dir(dbPath) + err = dir.Init(dbDir, constant.KineDBDirMode) + if err != nil { + return fmt.Errorf("failed to initialize SQLite database directory: %w", err) + } + err = os.Chown(dbDir, k.uid, k.gid) + if err != nil && os.Geteuid() == 0 { + return fmt.Errorf("failed to change ownership of SQLite database directory: %w", err) + } + if err := os.Chown(dbPath, k.uid, k.gid); err != nil && !errors.Is(err, os.ErrNotExist) && os.Geteuid() == 0 { + logrus.WithError(err).Warn("Failed to change ownership of SQLite database file") + } } } diff --git a/pkg/config/kine/datasource.go b/pkg/config/kine/datasource.go new file mode 100644 index 000000000000..aa9195a16802 --- /dev/null +++ b/pkg/config/kine/datasource.go @@ -0,0 +1,91 @@ +/* +Copyright 2024 k0s authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kine + +import ( + "errors" + "net/url" + "path/filepath" + "strings" +) + +// Splits a kine data source string into the backend and DSN parts. +// +// Kine data sources are of the form "://". +// They look like URLs, but they aren't, so don't try to use [net/url.Parse]. +func SplitDataSource(dataSource string) (backend, dsn string, _ error) { + backend, dsn, ok := strings.Cut(dataSource, "://") + + // Kine behaves weirdly if the infix isn't found or the backend is empty: It + // defaults to SQLite with an empty DSN, no matter what the data source is. + // Let's not duplicate this in k0s, but insist on something with an infix. + + if !ok { + return "", "", errors.New("failed to find infix between driver and DSN") + } + + switch backend { + case "": + return "", "", errors.New("no backend specified") + case "nats": + dsn = dataSource + case "http", "https": + backend = "etcd3" + } + + return backend, dsn, nil +} + +// Gets the file system path of the SQLite database file from an SQLite DSN. +func GetSQLiteFilePath(workingDir, dsn string) (string, error) { + // The DSN is preprocessed by kine's SQLite Go database driver, and not + // passed to the SQLite library as is: + if pos := strings.IndexByte(dsn, '?'); pos >= 1 { + if !strings.HasPrefix(dsn, "file:") { + dsn = dsn[:pos] + } + } + + // Now rely on the SQLite library's URI semantics. + // + // https://www.sqlite.org/c3ref/open.html + // https://www.sqlite.org/uri.html + + // The DSN is treated as the file name if it's not a file URI. + fileName := dsn + uri, err := url.Parse(dsn) + if err == nil && uri.Scheme == "file" { + fileName = filepath.FromSlash(uri.Path) + } + + switch fileName { + case "": + return "", errors.New("private temporary on-disk database") + case ":memory:": + return "", errors.New("in-memory database") + } + + // Kine/SQLite should treat relative paths as relative to their working + // directory, which _should_ be k0s's data dir. + if !filepath.IsAbs(fileName) { + return filepath.Join(workingDir, dsn), nil + } + + // Clean the file path. This is to be consistent with filepath.Join which + // cleans the path internally, too. + return filepath.Clean(fileName), nil +}