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

feat: If not already existant, create ftl-project.toml and/or module config or secret #1273

Merged
merged 8 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 2 additions & 0 deletions common/configuration/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/alecthomas/assert/v2"
"github.com/alecthomas/types/optional"
"github.com/zalando/go-keyring"

"github.com/TBD54566975/ftl/internal/log"
Expand Down Expand Up @@ -54,6 +55,7 @@ func TestManager(t *testing.T) {
{Ref: Ref{Name: "baz"}, Accessor: URL("envar://baz")},
{Ref: Ref{Name: "foo"}, Accessor: URL("inline://ImJhciI")},
{Ref: Ref{Name: "mutable"}, Accessor: URL("inline://ImhlbGxvIg")},
{Ref: Ref{Module: optional.Some[string]("echo"), Name: "default"}, Accessor: URL("inline://ImFub255bW91cyI")},
})
})
}
Expand Down
14 changes: 12 additions & 2 deletions common/configuration/projectconfig_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ func (p ProjectConfigResolver[R]) getMapping(config pc.Config, module optional.O
get := func(dest pc.ConfigAndSecrets) map[string]*pc.URL {
switch any(k).(type) {
case Configuration:
return dest.Config
return emptyMapIfNil(dest.Config)
case Secrets:
return dest.Secrets
return emptyMapIfNil(dest.Secrets)
default:
panic("unsupported kind")
}
Expand All @@ -121,6 +121,13 @@ func (p ProjectConfigResolver[R]) getMapping(config pc.Config, module optional.O
return mapping, nil
}

func emptyMapIfNil(mapping map[string]*pc.URL) map[string]*pc.URL {
if mapping == nil {
return map[string]*pc.URL{}
}
return mapping
}

func (p ProjectConfigResolver[R]) setMapping(config pc.Config, module optional.Option[string], mapping map[string]*pc.URL) error {
var k R
set := func(dest *pc.ConfigAndSecrets, mapping map[string]*pc.URL) {
Expand All @@ -143,5 +150,8 @@ func (p ProjectConfigResolver[R]) setMapping(config pc.Config, module optional.O
set(&config.Global, mapping)
}
configPaths := pc.ConfigPaths(p.Config)
if len(configPaths) == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually unnecessary - pc.ConfigPaths() will return pc.GetDefaultConfigPath() if len(p.Config) == 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that! #1276

return pc.CreateAndSave(config)
}
return pc.Save(configPaths[len(configPaths)-1], config)
}
60 changes: 60 additions & 0 deletions common/configuration/projectconfig_resolver_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package configuration

import (
"context"
"net/url"
"os"
"path/filepath"
"testing"

"github.com/alecthomas/assert/v2"
"github.com/alecthomas/types/optional"

"github.com/TBD54566975/ftl/common/projectconfig"
"github.com/TBD54566975/ftl/internal/log"
)

func TestSet(t *testing.T) {
defaultPath := projectconfig.GetDefaultConfigPath()
origConfigBytes, err := os.ReadFile(defaultPath)
assert.NoError(t, err)

config := filepath.Join(t.TempDir(), "ftl-project.toml")
existing, err := os.ReadFile("testdata/ftl-project.toml")
assert.NoError(t, err)
err = os.WriteFile(config, existing, 0600)
assert.NoError(t, err)

t.Run("ExistingModule", func(t *testing.T) {
setAndAssert(t, "echo", []string{config})
})
t.Run("NewModule", func(t *testing.T) {
setAndAssert(t, "echooo", []string{config})
})
t.Run("MissingTOMLFile", func(t *testing.T) {
err := os.Remove(config)
assert.NoError(t, err)
setAndAssert(t, "echooooo", []string{})
err = os.WriteFile(defaultPath, origConfigBytes, 0600)
assert.NoError(t, err)
})
}

func setAndAssert(t *testing.T, module string, config []string) {

Check failure on line 43 in common/configuration/projectconfig_resolver_test.go

View workflow job for this annotation

GitHub Actions / Lint

test helper function should start from t.Helper() (thelper)
ctx := log.ContextWithNewDefaultLogger(context.Background())

cf, err := New(ctx,
ProjectConfigResolver[Configuration]{Config: config},
[]Provider[Configuration]{
EnvarProvider[Configuration]{},
InlineProvider[Configuration]{Inline: true}, // Writer
})
assert.NoError(t, err)

var got *url.URL
want := URL("inline://asdfasdf")
cf.Set(ctx, Ref{Module: optional.Some[string](module), Name: "default"}, want)

Check failure on line 56 in common/configuration/projectconfig_resolver_test.go

View workflow job for this annotation

GitHub Actions / Lint

Error return value of `cf.Set` is not checked (errcheck)
err = cf.Get(ctx, Ref{Module: optional.Some[string](module), Name: "default"}, &got)
assert.NoError(t, err)
assert.Equal(t, want, got)
}
5 changes: 5 additions & 0 deletions common/configuration/testdata/ftl-project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,8 @@ mutable = "keychain://mutable"
baz = "envar://baz"
foo = "inline://ImJhciI"
mutable = "inline://ImhlbGxvIg"

[modules]
[modules.echo]
[modules.echo.configuration]
default = "inline://ImFub255bW91cyI"
22 changes: 21 additions & 1 deletion common/projectconfig/projectconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package projectconfig

import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -42,14 +43,18 @@ func ConfigPaths(input []string) []string {
if len(input) > 0 {
return input
}
path := filepath.Join(internal.GitRoot(""), "ftl-project.toml")
path := GetDefaultConfigPath()
_, err := os.Stat(path)
if err == nil {
return []string{path}
}
return []string{}
}

func GetDefaultConfigPath() string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea.

return filepath.Join(internal.GitRoot(""), "ftl-project.toml")
}

func LoadConfig(ctx context.Context, input []string) (Config, error) {
logger := log.FromContext(ctx)
configPaths := ConfigPaths(input)
Expand Down Expand Up @@ -92,6 +97,21 @@ func loadFile(path string) (Config, error) {
return config, nil
}

func CreateAndSave(config Config) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this adds over Save(), which will always recreate the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh thanks for catching that - I only made this to call GetDefaultConfigPath(), so I just moved that up to the resolver instead so it could call Save() directly.

path := GetDefaultConfigPath()
_, err := os.Stat(path)
// Only create a new file if there isn't one already defined at this location
if err != nil && errors.Is(err, os.ErrNotExist) {
if err = os.WriteFile(path, []byte{}, 0600); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per above, I think this function is redundant, but just FTR it's always better to write to a temp file and rename it, rather than overwrite a file directly, because if the write fails halfway through it will result in a corrupted config. With the rename temp file approach, the rename is atomic, so the file is either old or new, never "half written".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's very good to know. Thanks!

return fmt.Errorf("failed to create file at path %q due to error: %w", path, err)
}
}
if err != nil {
return fmt.Errorf("failed to create file at path %q due to error: %w", path, err)
}
return Save(path, config)
}

// Save project config atomically to a file.
func Save(path string, config Config) error {
w, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path))
Expand Down
Loading