From cc0c9e5e75afdf139f9224d2ebbbb2c84724adf5 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Wed, 26 Jun 2024 13:26:33 +1000 Subject: [PATCH] chore: make using ModuleConfig paths more consistent (#1877) `.Abs()` will return a clone with all paths made absolute. This avoids a bunch of manual and error prone joining. --- buildengine/build.go | 11 +++---- buildengine/build_kotlin.go | 2 +- buildengine/build_test.go | 2 +- buildengine/deploy.go | 20 ++++++------ common/moduleconfig/moduleconfig.go | 47 ++++++++++++++++++++++++++++- go-runtime/compile/build.go | 4 +-- 6 files changed, 64 insertions(+), 22 deletions(-) diff --git a/buildengine/build.go b/buildengine/build.go index 2df71fa7b9..fd51cf0b4f 100644 --- a/buildengine/build.go +++ b/buildengine/build.go @@ -36,7 +36,7 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module, filesTr ctx = log.ContextWithLogger(ctx, logger) // clear the deploy directory before extracting schema - if err := os.RemoveAll(module.Config.AbsDeployDir()); err != nil { + if err := os.RemoveAll(module.Config.Abs().DeployDir); err != nil { return fmt.Errorf("failed to clear errors: %w", err) } @@ -55,7 +55,7 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module, filesTr errs = append(errs, err) } // read runtime-specific build errors from the build directory - errorList, err := loadProtoErrors(module.Config) + errorList, err := loadProtoErrors(module.Config.Abs()) if err != nil { return fmt.Errorf("failed to read build errors for module: %w", err) } @@ -71,13 +71,12 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module, filesTr return nil } -func loadProtoErrors(config moduleconfig.ModuleConfig) (*schema.ErrorList, error) { - f := filepath.Join(config.AbsDeployDir(), config.Errors) - if _, err := os.Stat(f); errors.Is(err, os.ErrNotExist) { +func loadProtoErrors(config moduleconfig.AbsModuleConfig) (*schema.ErrorList, error) { + if _, err := os.Stat(config.Errors); errors.Is(err, os.ErrNotExist) { return &schema.ErrorList{Errors: make([]*schema.Error, 0)}, nil } - content, err := os.ReadFile(f) + content, err := os.ReadFile(config.Errors) if err != nil { return nil, err } diff --git a/buildengine/build_kotlin.go b/buildengine/build_kotlin.go index 433565525b..da8764c61a 100644 --- a/buildengine/build_kotlin.go +++ b/buildengine/build_kotlin.go @@ -94,7 +94,7 @@ func SetPOMProperties(ctx context.Context, baseDir string) error { } func prepareFTLRoot(module Module) error { - buildDir := module.Config.AbsDeployDir() + buildDir := module.Config.Abs().DeployDir if err := os.MkdirAll(buildDir, 0700); err != nil { return err } diff --git a/buildengine/build_test.go b/buildengine/build_test.go index 863b9d4d3a..83a05b6c09 100644 --- a/buildengine/build_test.go +++ b/buildengine/build_test.go @@ -122,7 +122,7 @@ func assertBuildProtoErrors(msgs ...string) assertion { t.Helper() config, err := moduleconfig.LoadModuleConfig(bctx.moduleDir) assert.NoError(t, err, "Error loading module config") - errorList, err := loadProtoErrors(config) + errorList, err := loadProtoErrors(config.Abs()) assert.NoError(t, err, "Error loading proto errors") expected := make([]*schema.Error, 0, len(msgs)) diff --git a/buildengine/deploy.go b/buildengine/deploy.go index c0c5440bde..c8362224e4 100644 --- a/buildengine/deploy.go +++ b/buildengine/deploy.go @@ -40,14 +40,14 @@ func Deploy(ctx context.Context, module Module, replicas int32, waitForDeployOnl ctx = log.ContextWithLogger(ctx, logger) logger.Infof("Deploying module") - deployDir := module.Config.AbsDeployDir() - files, err := findFiles(deployDir, module.Config.Deploy) + moduleConfig := module.Config.Abs() + files, err := findFiles(moduleConfig) if err != nil { - logger.Errorf(err, "failed to find files in %s", deployDir) + logger.Errorf(err, "failed to find files in %s", moduleConfig) return err } - filesByHash, err := hashFiles(deployDir, files) + filesByHash, err := hashFiles(moduleConfig.DeployDir, files) if err != nil { return err } @@ -57,7 +57,7 @@ func Deploy(ctx context.Context, module Module, replicas int32, waitForDeployOnl return err } - moduleSchema, err := loadProtoSchema(deployDir, module.Config, replicas) + moduleSchema, err := loadProtoSchema(moduleConfig, replicas) if err != nil { return fmt.Errorf("failed to load protobuf schema from %q: %w", module.Config.Schema, err) } @@ -130,9 +130,8 @@ func terminateModuleDeployment(ctx context.Context, client ftlv1connect.Controll return err } -func loadProtoSchema(deployDir string, config moduleconfig.ModuleConfig, replicas int32) (*schemapb.Module, error) { - schema := filepath.Join(deployDir, config.Schema) - content, err := os.ReadFile(schema) +func loadProtoSchema(config moduleconfig.AbsModuleConfig, replicas int32) (*schemapb.Module, error) { + content, err := os.ReadFile(config.Schema) if err != nil { return nil, err } @@ -155,10 +154,9 @@ func loadProtoSchema(deployDir string, config moduleconfig.ModuleConfig, replica return module, nil } -func findFiles(base string, files []string) ([]string, error) { +func findFiles(moduleConfig moduleconfig.AbsModuleConfig) ([]string, error) { var out []string - for _, file := range files { - file = filepath.Join(base, file) + for _, file := range moduleConfig.Deploy { info, err := os.Stat(file) if err != nil { return nil, err diff --git a/common/moduleconfig/moduleconfig.go b/common/moduleconfig/moduleconfig.go index b7a6aa1e2a..ba9bfd7067 100644 --- a/common/moduleconfig/moduleconfig.go +++ b/common/moduleconfig/moduleconfig.go @@ -10,6 +10,8 @@ import ( "github.com/BurntSushi/toml" "golang.org/x/mod/modfile" + + "github.com/TBD54566975/ftl/internal/slices" ) // ModuleGoConfig is language-specific configuration for Go modules. @@ -22,7 +24,7 @@ type ModuleKotlinConfig struct{} // // Module config files are currently TOML. type ModuleConfig struct { - // Dir is the root of the module. + // Dir is the absolute path to the root of the module. Dir string `toml:"-"` Language string `toml:"language"` @@ -45,6 +47,11 @@ type ModuleConfig struct { Kotlin ModuleKotlinConfig `toml:"kotlin,optional"` } +// AbsModuleConfig is a ModuleConfig with all paths made absolute. +// +// This is a type alias to prevent accidental use of the wrong type. +type AbsModuleConfig ModuleConfig + // LoadModuleConfig from a directory. func LoadModuleConfig(dir string) (ModuleConfig, error) { path := filepath.Join(dir, "ftl.toml") @@ -60,6 +67,44 @@ func LoadModuleConfig(dir string) (ModuleConfig, error) { return config, nil } +func (c ModuleConfig) String() string { + return fmt.Sprintf("%s (%s)", c.Module, c.Dir) +} + +// Abs creates a clone of ModuleConfig with all paths made absolute. +// +// This function will panic if any paths are not beneath the module directory. +// This should never happen under normal use, as LoadModuleConfig performs this +// validation separately. This is just a sanity check. +func (c ModuleConfig) Abs() AbsModuleConfig { + clone := c + clone.Dir = filepath.Clean(clone.Dir) + clone.DeployDir = filepath.Clean(filepath.Join(clone.Dir, clone.DeployDir)) + if !strings.HasPrefix(clone.DeployDir, clone.Dir) { + panic(fmt.Sprintf("deploy-dir %q is not beneath module directory %q", clone.DeployDir, clone.Dir)) + } + clone.Schema = filepath.Clean(filepath.Join(clone.DeployDir, clone.Schema)) + if !strings.HasPrefix(clone.Schema, clone.DeployDir) { + panic(fmt.Sprintf("schema %q is not beneath deploy directory %q", clone.Schema, clone.DeployDir)) + } + clone.Errors = filepath.Clean(filepath.Join(clone.DeployDir, clone.Errors)) + if !strings.HasPrefix(clone.Errors, clone.DeployDir) { + panic(fmt.Sprintf("errors %q is not beneath deploy directory %q", clone.Errors, clone.DeployDir)) + } + clone.Deploy = slices.Map(clone.Deploy, func(p string) string { + out := filepath.Clean(filepath.Join(clone.DeployDir, p)) + if !strings.HasPrefix(out, clone.DeployDir) { + panic(fmt.Sprintf("deploy path %q is not beneath deploy directory %q", out, clone.DeployDir)) + } + return out + }) + // Watch paths are allowed to be outside the deploy directory. + clone.Watch = slices.Map(clone.Watch, func(p string) string { + return filepath.Clean(filepath.Join(clone.Dir, p)) + }) + return AbsModuleConfig(clone) +} + // AbsDeployDir returns the absolute path to the deploy directory. func (c ModuleConfig) AbsDeployDir() string { return filepath.Join(c.Dir, c.DeployDir) diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go index 68bd35ef4b..b3559490fe 100644 --- a/go-runtime/compile/build.go +++ b/go-runtime/compile/build.go @@ -539,7 +539,7 @@ func writeSchema(config moduleconfig.ModuleConfig, module *schema.Module) error if err != nil { return fmt.Errorf("failed to marshal schema: %w", err) } - return os.WriteFile(filepath.Join(config.AbsDeployDir(), "schema.pb"), schemaBytes, 0600) + return os.WriteFile(config.Abs().Schema, schemaBytes, 0600) } func writeSchemaErrors(config moduleconfig.ModuleConfig, errors []*schema.Error) error { @@ -550,7 +550,7 @@ func writeSchemaErrors(config moduleconfig.ModuleConfig, errors []*schema.Error) if err != nil { return fmt.Errorf("failed to marshal errors: %w", err) } - return os.WriteFile(filepath.Join(config.AbsDeployDir(), config.Errors), elBytes, 0600) + return os.WriteFile(config.Abs().Errors, elBytes, 0600) } func getSumTypes(module *schema.Module, sch *schema.Schema, nativeNames NativeNames) []goSumType {