Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-19.0] Replace uses of os.Create with os2.Create within backup/restore workflows (#17648) #17664

Merged
merged 3 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions go/os2/file.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2025 The Vitess 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 os2

import (
"io/fs"
"os"
)

const (
// PermFile is a FileMode for regular files without world permission bits.
PermFile fs.FileMode = 0660
// PermDirectory is a FileMode for directories without world permission bits.
PermDirectory fs.FileMode = 0770
)

// Create is identical to os.Create except uses 0660 permission
// rather than 0666, to exclude world read/write bit.
func Create(name string) (*os.File, error) {
return os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, PermFile)
}

// WriteFile is identical to os.WriteFile except permission of 0660 is used.
func WriteFile(name string, data []byte) error {
return os.WriteFile(name, data, PermFile)

Check warning on line 39 in go/os2/file.go

View check run for this annotation

Codecov / codecov/patch

go/os2/file.go#L38-L39

Added lines #L38 - L39 were not covered by tests
}

// Mkdir is identical to os.Mkdir except permission of 0770 is used.
func Mkdir(path string) error {
return os.Mkdir(path, PermDirectory)
}

// MkdirAll is identical to os.MkdirAll except permission of 0770 is used.
func MkdirAll(path string) error {
return os.MkdirAll(path, PermDirectory)
}
12 changes: 12 additions & 0 deletions go/test/endtoend/backup/vtctlbackup/backup_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,18 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDe
return vterrors.Errorf(vtrpc.Code_UNKNOWN, "test failure: %s", test.name)
}
}

t.Run("check for files created with global permissions", func(t *testing.T) {
t.Logf("Confirming that none of the MySQL data directories that we've created have files with global permissions")
for _, ks := range localCluster.Keyspaces {
for _, shard := range ks.Shards {
for _, tablet := range shard.Vttablets {
tablet.VttabletProcess.ConfirmDataDirHasNoGlobalPerms(t)
}
}
}
})

return nil
}

Expand Down
60 changes: 60 additions & 0 deletions go/test/endtoend/cluster/vttablet_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"net/http"
"os"
"os/exec"
Expand All @@ -35,6 +36,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

"vitess.io/vitess/go/constants/sidecar"
"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/sqltypes"
Expand Down Expand Up @@ -677,6 +680,63 @@ func (vttablet *VttabletProcess) IsShutdown() bool {
return vttablet.proc == nil
}

// ConfirmDataDirHasNoGlobalPerms confirms that no files in the tablet's data directory
// have any global/world/other permissions enabled.
func (vttablet *VttabletProcess) ConfirmDataDirHasNoGlobalPerms(t *testing.T) {
datadir := vttablet.Directory
if _, err := os.Stat(datadir); errors.Is(err, os.ErrNotExist) {
t.Logf("Data directory %s no longer exists, skipping permissions check", datadir)
return
}

var allowedFiles = []string{
// These are intentionally created with the world/other read bit set by mysqld itself
// during the --initialize[-insecure] step.
// See: https://dev.mysql.com/doc/mysql-security-excerpt/en/creating-ssl-rsa-files-using-mysql.html
// "On Unix and Unix-like systems, the file access mode is 644 for certificate files
// (that is, world readable) and 600 for key files (that is, accessible only by the
// account that runs the server)."
path.Join("data", "ca.pem"),
path.Join("data", "client-cert.pem"),
path.Join("data", "public_key.pem"),
path.Join("data", "server-cert.pem"),
// The domain socket must have global perms for anyone to use it.
"mysql.sock",
// These files are created by xtrabackup.
path.Join("tmp", "xtrabackup_checkpoints"),
path.Join("tmp", "xtrabackup_info"),
}

var matches []string
fsys := os.DirFS(datadir)
err := fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, _ error) error {
// first check if the file should be skipped
for _, name := range allowedFiles {
if strings.HasSuffix(p, name) {
return nil
}
}

info, err := d.Info()
if err != nil {
return err
}

// check if any global bit is on the filemode
if info.Mode()&0007 != 0 {
matches = append(matches, fmt.Sprintf(
"%s (%s)",
path.Join(datadir, p),
info.Mode(),
))
}
return nil
})

require.NoError(t, err, "Error walking directory")
require.Empty(t, matches, "Found files with global permissions: %s\n", strings.Join(matches, "\n"))
}

// VttabletProcessInstance returns a VttabletProcess handle for vttablet process
// configured with the given Config.
// The process must be manually started by calling setup()
Expand Down
3 changes: 2 additions & 1 deletion go/vt/mysqlctl/backupengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (

"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/mysql/replication"
"vitess.io/vitess/go/os2"
"vitess.io/vitess/go/vt/logutil"
"vitess.io/vitess/go/vt/mysqlctl/backupstats"
"vitess.io/vitess/go/vt/mysqlctl/backupstorage"
Expand Down Expand Up @@ -654,7 +655,7 @@ func createStateFile(cnf *Mycnf) error {
// rename func to openStateFile
// change to return a *File
fname := filepath.Join(cnf.TabletDir(), RestoreState)
fd, err := os.Create(fname)
fd, err := os2.Create(fname)
if err != nil {
return fmt.Errorf("unable to create file: %v", err)
}
Expand Down
5 changes: 3 additions & 2 deletions go/vt/mysqlctl/builtinbackupengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"vitess.io/vitess/go/ioutil"
"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/mysql/replication"
"vitess.io/vitess/go/os2"
"vitess.io/vitess/go/protoutil"
"vitess.io/vitess/go/vt/concurrency"
"vitess.io/vitess/go/vt/log"
Expand Down Expand Up @@ -199,10 +200,10 @@ func (fe *FileEntry) open(cnf *Mycnf, readOnly bool) (*os.File, error) {
}
} else {
dir := path.Dir(name)
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
if err := os2.MkdirAll(dir); err != nil {
return nil, vterrors.Wrapf(err, "cannot create destination directory %v", dir)
}
if fd, err = os.Create(name); err != nil {
if fd, err = os2.Create(name); err != nil {
return nil, vterrors.Wrapf(err, "cannot create destination file %v", name)
}
}
Expand Down
7 changes: 4 additions & 3 deletions go/vt/mysqlctl/filebackupstorage/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/spf13/pflag"

"vitess.io/vitess/go/ioutil"
"vitess.io/vitess/go/os2"
"vitess.io/vitess/go/vt/concurrency"
stats "vitess.io/vitess/go/vt/mysqlctl/backupstats"
"vitess.io/vitess/go/vt/mysqlctl/backupstorage"
Expand Down Expand Up @@ -110,7 +111,7 @@ func (fbh *FileBackupHandle) AddFile(ctx context.Context, filename string, files
return nil, fmt.Errorf("AddFile cannot be called on read-only backup")
}
p := path.Join(FileBackupStorageRoot, fbh.dir, fbh.name, filename)
f, err := os.Create(p)
f, err := os2.Create(p)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -186,13 +187,13 @@ func (fbs *FileBackupStorage) ListBackups(ctx context.Context, dir string) ([]ba
func (fbs *FileBackupStorage) StartBackup(ctx context.Context, dir, name string) (backupstorage.BackupHandle, error) {
// Make sure the directory exists.
p := path.Join(FileBackupStorageRoot, dir)
if err := os.MkdirAll(p, os.ModePerm); err != nil {
if err := os2.MkdirAll(p); err != nil {
return nil, err
}

// Create the subdirectory for this named backup.
p = path.Join(p, name)
if err := os.Mkdir(p, os.ModePerm); err != nil {
if err := os2.Mkdir(p); err != nil {
return nil, err
}

Expand Down
11 changes: 6 additions & 5 deletions go/vt/mysqlctl/mysqld.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"vitess.io/vitess/config"
"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/mysql/sqlerror"
"vitess.io/vitess/go/os2"
"vitess.io/vitess/go/protoutil"
"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/dbconfigs"
Expand Down Expand Up @@ -838,7 +839,7 @@
return err
}

return os.WriteFile(outFile, []byte(configData), 0o664)
return os2.WriteFile(outFile, []byte(configData))

Check warning on line 842 in go/vt/mysqlctl/mysqld.go

View check run for this annotation

Codecov / codecov/patch

go/vt/mysqlctl/mysqld.go#L842

Added line #L842 was not covered by tests
}

func (mysqld *Mysqld) getMycnfTemplate() string {
Expand Down Expand Up @@ -982,7 +983,7 @@
func (mysqld *Mysqld) createDirs(cnf *Mycnf) error {
tabletDir := cnf.TabletDir()
log.Infof("creating directory %s", tabletDir)
if err := os.MkdirAll(tabletDir, os.ModePerm); err != nil {
if err := os2.MkdirAll(tabletDir); err != nil {

Check warning on line 986 in go/vt/mysqlctl/mysqld.go

View check run for this annotation

Codecov / codecov/patch

go/vt/mysqlctl/mysqld.go#L986

Added line #L986 was not covered by tests
return err
}
for _, dir := range TopLevelDirs() {
Expand All @@ -992,7 +993,7 @@
}
for _, dir := range cnf.directoryList() {
log.Infof("creating directory %s", dir)
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
if err := os2.MkdirAll(dir); err != nil {

Check warning on line 996 in go/vt/mysqlctl/mysqld.go

View check run for this annotation

Codecov / codecov/patch

go/vt/mysqlctl/mysqld.go#L996

Added line #L996 was not covered by tests
return err
}
// FIXME(msolomon) validate permissions?
Expand All @@ -1016,14 +1017,14 @@
if os.IsNotExist(err) {
topdir := path.Join(tabletDir, dir)
log.Infof("creating directory %s", topdir)
return os.MkdirAll(topdir, os.ModePerm)
return os2.MkdirAll(topdir)

Check warning on line 1020 in go/vt/mysqlctl/mysqld.go

View check run for this annotation

Codecov / codecov/patch

go/vt/mysqlctl/mysqld.go#L1020

Added line #L1020 was not covered by tests
}
return err
}
linkto := path.Join(target, vtname)
source := path.Join(tabletDir, dir)
log.Infof("creating directory %s", linkto)
err = os.MkdirAll(linkto, os.ModePerm)
err = os2.MkdirAll(linkto)

Check warning on line 1027 in go/vt/mysqlctl/mysqld.go

View check run for this annotation

Codecov / codecov/patch

go/vt/mysqlctl/mysqld.go#L1027

Added line #L1027 was not covered by tests
if err != nil {
return err
}
Expand Down
Loading