From 3700aea9c4284e511b7b952724b99ac9c5635b58 Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Thu, 25 Aug 2022 01:01:57 +0300 Subject: [PATCH 1/9] charm: improved output handling, add warning textbox variant --- commands/charm/charm.go | 37 +++++++++++++++++++++++++------ commands/charm/textbox/textbox.go | 5 +++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/commands/charm/charm.go b/commands/charm/charm.go index f1ba3cb70..fd5d3da32 100644 --- a/commands/charm/charm.go +++ b/commands/charm/charm.go @@ -2,6 +2,7 @@ package charm import ( "fmt" + "io" "os" "github.com/charmbracelet/lipgloss" @@ -14,12 +15,13 @@ var ( // Style is a styled component. type Style struct { - style lipgloss.Style + style lipgloss.Style + output io.Writer } // NewStyle creates a new styled component. func NewStyle(style lipgloss.Style) Style { - return Style{style.Copy()} + return Style{style: style.Copy()} } // Lipgloss returns a copy of the underlying lipgloss.Style. @@ -29,7 +31,10 @@ func (s Style) Lipgloss() lipgloss.Style { // Copy returns a copy of the style. func (s Style) Copy() Style { - return Style{style: s.style.Copy()} + return Style{ + style: s.style.Copy(), + output: s.output, + } } // Inherit returns a copy of the original style with the properties from another style inherited. @@ -67,15 +72,20 @@ func (s Style) S(str any) string { return s.Sprint(str) } -// Printf formats the specified text with the style applied and prints it to stdout. +// Print applies the style to the specified text and prints it to the output writer. +func (s Style) Print(str any) (int, error) { + return fmt.Fprint(s, str) +} + +// Printf formats the specified text with the style applied and prints it to the output writer. func (s Style) Printf(format string, a ...any) (int, error) { - return fmt.Fprint(os.Stdout, s.Sprintf(format, a...)) + return fmt.Fprintf(s, format, a...) } -// Write implements the io.Writer interface and prints to stdout. +// Write implements the io.Writer interface and prints to the output writer. func (s Style) Write(b []byte) (n int, err error) { n = len(b) - _, err = s.Printf(string(b)) + _, err = fmt.Fprint(s.writer(), s.Sprint(string(b))) return } @@ -86,7 +96,20 @@ func (s Style) WithString(str string) Style { return c } +// WithOutput sets the output writer. +func (s Style) WithOutput(output io.Writer) Style { + s.output = output + return s +} + // String implements the fmt.Stringer interface. func (s Style) String() string { return s.style.String() } + +func (s Style) writer() io.Writer { + if s.output != nil { + return s.output + } + return os.Stdout +} diff --git a/commands/charm/textbox/textbox.go b/commands/charm/textbox/textbox.go index 4c3fd329f..6630652c7 100644 --- a/commands/charm/textbox/textbox.go +++ b/commands/charm/textbox/textbox.go @@ -30,3 +30,8 @@ func (t TextBox) Error() TextBox { t.Style = t.Style.InheritLipgloss(lipgloss.NewStyle().BorderForeground(charm.Colors.Error)) return t } + +func (t TextBox) Warning() TextBox { + t.Style = t.Style.InheritLipgloss(lipgloss.NewStyle().BorderForeground(charm.Colors.Warning)) + return t +} From 5408a2bb2fe7f8a97134074411754b248d967171 Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Thu, 25 Aug 2022 01:02:58 +0300 Subject: [PATCH 2/9] add config interfaces and types --- doit.go | 23 +++-- internal/apps/config/config.go | 116 +++++++++++++++++++++ internal/apps/config/config_test.go | 152 ++++++++++++++++++++++++++++ internal/apps/config/doctl.go | 45 ++++++++ internal/apps/config/doctl_test.go | 38 +++++++ 5 files changed, 364 insertions(+), 10 deletions(-) create mode 100644 internal/apps/config/config.go create mode 100644 internal/apps/config/config_test.go create mode 100644 internal/apps/config/doctl.go create mode 100644 internal/apps/config/doctl_test.go diff --git a/doit.go b/doit.go index 2930e4c87..75bc10fa9 100644 --- a/doit.go +++ b/doit.go @@ -396,7 +396,10 @@ func (c *LiveConfig) GetDuration(ns, key string) (time.Duration, error) { } func nskey(ns, key string) string { - return fmt.Sprintf("%s.%s", ns, key) + if ns != "" { + key = fmt.Sprintf("%s.%s", ns, key) + } + return key } func isRequired(key string) bool { @@ -452,7 +455,7 @@ func (c *TestConfig) Listen(url *url.URL, token string, schemaFunc listen.Schema // Set sets a config key. func (c *TestConfig) Set(ns, key string, val interface{}) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) c.v.Set(nskey, val) c.IsSetMap[key] = true } @@ -465,21 +468,21 @@ func (c *TestConfig) IsSet(key string) bool { // GetString returns the string value for the key in the given namespace. Because // this is a mock implementation, and error will never be returned. func (c *TestConfig) GetString(ns, key string) (string, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) return c.v.GetString(nskey), nil } // GetInt returns the int value for the key in the given namespace. Because // this is a mock implementation, and error will never be returned. func (c *TestConfig) GetInt(ns, key string) (int, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) return c.v.GetInt(nskey), nil } // GetIntPtr returns the int value for the key in the given namespace. Because // this is a mock implementation, and error will never be returned. func (c *TestConfig) GetIntPtr(ns, key string) (*int, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) if !c.v.IsSet(nskey) { return nil, nil } @@ -491,7 +494,7 @@ func (c *TestConfig) GetIntPtr(ns, key string) (*int, error) { // namespace. Because this is a mock implementation, and error will never be // returned. func (c *TestConfig) GetStringSlice(ns, key string) ([]string, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) return c.v.GetStringSlice(nskey), nil } @@ -499,21 +502,21 @@ func (c *TestConfig) GetStringSlice(ns, key string) ([]string, error) { // given namespace. Because this is a mock implementation, and error will never // be returned. func (c *TestConfig) GetStringMapString(ns, key string) (map[string]string, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) return c.v.GetStringMapString(nskey), nil } // GetBool returns the bool value for the key in the given namespace. Because // this is a mock implementation, and error will never be returned. func (c *TestConfig) GetBool(ns, key string) (bool, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) return c.v.GetBool(nskey), nil } // GetBoolPtr returns the bool value for the key in the given namespace. Because // this is a mock implementation, and error will never be returned. func (c *TestConfig) GetBoolPtr(ns, key string) (*bool, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) if !c.v.IsSet(nskey) { return nil, nil } @@ -524,7 +527,7 @@ func (c *TestConfig) GetBoolPtr(ns, key string) (*bool, error) { // GetDuration returns the duration value for the key in the given namespace. Because // this is a mock implementation, and error will never be returned. func (c *TestConfig) GetDuration(ns, key string) (time.Duration, error) { - nskey := fmt.Sprintf("%s-%s", ns, key) + nskey := nskey(ns, key) return c.v.GetDuration(nskey), nil } diff --git a/internal/apps/config/config.go b/internal/apps/config/config.go new file mode 100644 index 000000000..78d3db109 --- /dev/null +++ b/internal/apps/config/config.go @@ -0,0 +1,116 @@ +package config + +import ( + "strings" + "time" +) + +// ConfigSource is a config source. +type ConfigSource interface { + IsSet(key string) bool + GetString(key string) string + GetBool(key string) bool + GetDuration(key string) time.Duration +} + +type mutatingConfigSource struct { + cs ConfigSource + mutateKey func(key string) string + mutateIsSet bool // some config sources except IsSet to _not_ include the namespace +} + +func (s *mutatingConfigSource) key(key string) string { + if s.mutateKey != nil { + key = s.mutateKey(key) + } + return key +} + +func (s *mutatingConfigSource) IsSet(key string) bool { + if s.mutateIsSet { + key = s.key(key) + } + return s.cs.IsSet(key) +} + +func (s *mutatingConfigSource) GetString(key string) string { + return s.cs.GetString(s.key(key)) +} + +func (s *mutatingConfigSource) GetBool(key string) bool { + return s.cs.GetBool(s.key(key)) +} + +func (s *mutatingConfigSource) GetDuration(key string) time.Duration { + return s.cs.GetDuration(s.key(key)) +} + +func MutatingConfigSource(cs ConfigSource, mutateKey func(key string) string) ConfigSource { + return &mutatingConfigSource{ + cs: cs, + mutateKey: mutateKey, + mutateIsSet: true, + } +} + +// NamespacedConfigSource accepts a ConfigSource and configures a default namespace that is prefixed to the key on all +// Get* calls. +func NamespacedConfigSource(cs ConfigSource, ns string) ConfigSource { + var mutateKey func(string) string + if ns != "" { + mutateKey = func(key string) string { + return nsKey(ns, key) + } + } + return MutatingConfigSource(cs, mutateKey) +} + +// Multi returns a config source that wraps multiple config sources. +// Each source is evaluated in order and the first match is returned. +func Multi(sources ...ConfigSource) ConfigSource { + return &multiConfigSource{sources} +} + +type multiConfigSource struct { + sources []ConfigSource +} + +func (s *multiConfigSource) IsSet(key string) bool { + for _, s := range s.sources { + if s != nil && s.IsSet(key) { + return true + } + } + return false +} + +func (s *multiConfigSource) GetString(key string) string { + for _, s := range s.sources { + if s != nil && s.IsSet(key) { + return s.GetString(key) + } + } + return "" +} + +func (s *multiConfigSource) GetBool(key string) bool { + for _, s := range s.sources { + if s != nil && s.IsSet(key) { + return s.GetBool(key) + } + } + return false +} + +func (s *multiConfigSource) GetDuration(key string) time.Duration { + for _, s := range s.sources { + if s != nil && s.IsSet(key) { + return s.GetDuration(key) + } + } + return 0 +} + +func nsKey(parts ...string) string { + return strings.Join(parts, ".") +} diff --git a/internal/apps/config/config_test.go b/internal/apps/config/config_test.go new file mode 100644 index 000000000..ef5173287 --- /dev/null +++ b/internal/apps/config/config_test.go @@ -0,0 +1,152 @@ +package config + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestMutatingConfigSource(t *testing.T) { + // cs - keyed by word + cs := NewTestConfigSource(map[string]any{ + "one": true, + "two": time.Minute, + "three": "hello", + }) + for _, k := range []string{"one", "two", "three"} { + assert.True(t, cs.IsSet(k), k) + } + for _, k := range []string{"1", "2", "3"} { + assert.False(t, cs.IsSet(k), k) + } + + // mcs - keyed by number + mcs := MutatingConfigSource(cs, func(key string) string { + // translate number to word + return map[string]string{ + "1": "one", + "2": "two", + "3": "three", + }[key] + }) + for _, k := range []string{"one", "two", "three"} { + assert.False(t, mcs.IsSet(k), k) + } + for _, k := range []string{"1", "2", "3"} { + assert.True(t, mcs.IsSet(k), k) + } + + assert.Equal(t, true, mcs.GetBool("1")) + assert.Equal(t, time.Minute, mcs.GetDuration("2")) + assert.Equal(t, "hello", mcs.GetString("3")) +} + +func TestNamespacedConfigSource(t *testing.T) { + cs := NewTestConfigSource(map[string]any{ + "config.key1": true, + "config.key2": time.Minute, + "config.key3": "hello", + }) + for i := 1; i <= 3; i++ { + assert.True(t, cs.IsSet(fmt.Sprintf("config.key%d", i))) + assert.False(t, cs.IsSet(fmt.Sprintf("key%d", i))) + } + + mcs := NamespacedConfigSource(cs, "config") + for i := 1; i <= 3; i++ { + assert.False(t, mcs.IsSet(fmt.Sprintf("config.key%d", i))) + assert.True(t, mcs.IsSet(fmt.Sprintf("key%d", i))) + } + assert.Equal(t, true, mcs.GetBool("key1")) + assert.Equal(t, time.Minute, mcs.GetDuration("key2")) + assert.Equal(t, "hello", mcs.GetString("key3")) +} + +func TestMulti(t *testing.T) { + items1 := map[string]any{ + "name": "Bufo", + "ttl": 3 * time.Hour, + "verify": true, + } + c1 := NewTestConfigSource(items1) + items2 := map[string]any{ + "slug": "bufo", + "verify": false, + } + c2 := NewTestConfigSource(items2) + + // c2 should be evaluated before c1 + multi := Multi(c2, c1) + for k := range items1 { + assert.True(t, c1.IsSet(k), k) + assert.True(t, multi.IsSet(k), k) + } + for k := range items2 { + assert.True(t, c2.IsSet(k), k) + assert.True(t, multi.IsSet(k), k) + } + + assert.Equal(t, "Bufo", multi.GetString("name")) // c1 + assert.Equal(t, 3*time.Hour, multi.GetDuration("ttl")) // c1 + assert.Equal(t, false, multi.GetBool("verify")) // c2 overrides c1 + assert.Equal(t, "bufo", multi.GetString("slug")) // c2 +} + +func NewTestConfigSource(vals map[string]any) *TestConfigSource { + m := make(map[string]any) + for k, v := range vals { + m[k] = v + } + return &TestConfigSource{m} +} + +type TestConfigSource struct { + vals map[string]any +} + +func (s *TestConfigSource) Set(key string, value any) { + s.vals[key] = value +} + +func (s *TestConfigSource) IsSet(key string) bool { + _, ok := s.vals[key] + return ok +} + +func (s *TestConfigSource) GetString(key string) string { + v, ok := s.vals[key] + if !ok { + return "" + } + vv, ok := v.(string) + if !ok { + return "" + } + return vv +} + +func (s *TestConfigSource) GetBool(key string) bool { + v, ok := s.vals[key] + if !ok { + return false + } + vv, ok := v.(bool) + if !ok { + return false + } + return vv +} + +func (s *TestConfigSource) GetDuration(key string) time.Duration { + v, ok := s.vals[key] + if !ok { + return 0 + } + vv, ok := v.(time.Duration) + if !ok { + return 0 + } + return vv +} diff --git a/internal/apps/config/doctl.go b/internal/apps/config/doctl.go new file mode 100644 index 000000000..2ed8de537 --- /dev/null +++ b/internal/apps/config/doctl.go @@ -0,0 +1,45 @@ +package config + +import ( + "time" + + "github.com/digitalocean/doctl" +) + +// doctlConfigSource wraps doctl.Config to implement the ConfigSource interface. +type doctlConfigSource struct { + config doctl.Config +} + +func (c *doctlConfigSource) IsSet(key string) bool { + return c.config.IsSet(key) +} + +func (c *doctlConfigSource) GetString(key string) string { + v, _ := c.config.GetString("", key) + return v +} + +func (c *doctlConfigSource) GetBool(key string) bool { + v, _ := c.config.GetBool("", key) + return v +} + +func (c *doctlConfigSource) GetDuration(key string) time.Duration { + v, _ := c.config.GetDuration("", key) + return v +} + +// DoctlConfigSource converts a doctl.Config into a ConfigSource with an optional default namespace. +func DoctlConfigSource(config doctl.Config, ns string) ConfigSource { + var mutateKey func(string) string + if ns != "" { + mutateKey = func(key string) string { + return nsKey(ns, key) + } + } + return &mutatingConfigSource{ + cs: &doctlConfigSource{config}, + mutateKey: mutateKey, + } +} diff --git a/internal/apps/config/doctl_test.go b/internal/apps/config/doctl_test.go new file mode 100644 index 000000000..bbe811e0a --- /dev/null +++ b/internal/apps/config/doctl_test.go @@ -0,0 +1,38 @@ +package config + +import ( + "testing" + "time" + + "github.com/digitalocean/doctl" + "github.com/stretchr/testify/assert" +) + +func TestDoctlConfigSource(t *testing.T) { + doctlConfig := doctl.NewTestConfig() + doctlConfig.Set("", "interactive", true) + doctlConfig.Set("last-seen-ago", "bufo", time.Minute) + + // doctl's IsSet implementation pretends namespaces don't exist + t.Run("no namespace", func(t *testing.T) { + cs := DoctlConfigSource(doctlConfig, "") + assert.True(t, cs.IsSet("interactive")) + assert.False(t, cs.IsSet("last-seen-ago.bufo")) // namespace not considered by IsSet + assert.True(t, cs.IsSet("bufo")) + + assert.Equal(t, true, cs.GetBool("interactive")) + assert.Equal(t, time.Minute, cs.GetDuration("last-seen-ago.bufo")) + assert.Equal(t, time.Duration(0), cs.GetDuration("bufo")) // does not exist + }) + + t.Run("yes namespace", func(t *testing.T) { + cs := DoctlConfigSource(doctlConfig, "last-seen-ago") + assert.True(t, cs.IsSet("interactive")) + assert.False(t, cs.IsSet("last-seen-ago.bufo")) // namespace not considered by IsSet + assert.True(t, cs.IsSet("bufo")) + + assert.Equal(t, false, cs.GetBool("interactive")) // does not exist + assert.Equal(t, time.Duration(0), cs.GetDuration("last-seen-ago.bufo")) // does not exist + assert.Equal(t, time.Minute, cs.GetDuration("bufo")) + }) +} From c3efa280c9121c8747064116f48cbc73f0df5629 Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Thu, 25 Aug 2022 01:04:09 +0300 Subject: [PATCH 3/9] use new config interfaces, add support for per-component config --- args.go | 4 +- commands/apps_dev.go | 174 ++++++++++++++---------- commands/apps_dev_config.go | 153 +++------------------ commands/apps_dev_config_test.go | 73 +++++----- commands/apps_dev_test.go | 26 +++- internal/apps/config/appdev.go | 223 +++++++++++++++++++++++++++++++ 6 files changed, 403 insertions(+), 250 deletions(-) create mode 100644 internal/apps/config/appdev.go diff --git a/args.go b/args.go index 3c8517209..24371491e 100644 --- a/args.go +++ b/args.go @@ -353,8 +353,8 @@ const ( // ArgReadWrite indicates a generated token should be read/write. ArgReadWrite = "read-write" - // ArgRegistryName indicates the name of the registry. - ArgRegistryName = "registry-name" + // ArgRegistry indicates the name of the registry. + ArgRegistry = "registry" // ArgRegistryExpirySeconds indicates the length of time the token will be valid in seconds. ArgRegistryExpirySeconds = "expiry-seconds" // ArgSubscriptionTier is a subscription tier slug. diff --git a/commands/apps_dev.go b/commands/apps_dev.go index 38237c097..d0dd9c5bf 100644 --- a/commands/apps_dev.go +++ b/commands/apps_dev.go @@ -7,6 +7,7 @@ import ( "io" "io/fs" "os" + "strings" "sync" "github.com/MakeNowJust/heredoc" @@ -15,9 +16,10 @@ import ( "github.com/digitalocean/doctl/commands/charm/list" "github.com/digitalocean/doctl/commands/charm/pager" "github.com/digitalocean/doctl/commands/charm/template" + "github.com/digitalocean/doctl/commands/charm/text" "github.com/digitalocean/doctl/commands/charm/textbox" - "github.com/digitalocean/doctl/commands/displayers" "github.com/digitalocean/doctl/internal/apps/builder" + "github.com/digitalocean/doctl/internal/apps/config" "github.com/digitalocean/godo" "github.com/joho/godotenv" @@ -27,6 +29,8 @@ import ( const ( // AppsDevDefaultSpecPath is the default spec path for an app. AppsDevDefaultSpecPath = ".do/app.yaml" + // AppsDevDefaultEnvFile is the default env file path. + AppsDevDefaultEnvFile = ".env" ) // AppsDev creates the apps dev command subtree. @@ -54,7 +58,6 @@ func AppsDev() *Command { ), Writer, aliasOpt("b"), - displayerType(&displayers.Apps{}), ) build.DisableFlagsInUseLine = true @@ -95,7 +98,7 @@ func AppsDev() *Command { ) AddStringFlag( - build, doctl.ArgRegistryName, + build, doctl.ArgRegistry, "", os.Getenv("APP_DEV_REGISTRY"), "Registry name to build use for the component build.", ) @@ -106,33 +109,36 @@ func AppsDev() *Command { // RunAppsDevBuild builds an app component locally. func RunAppsDevBuild(c *CmdConfig) error { ctx := context.Background() - noCache, err := c.Doit.GetBool(c.NS, doctl.ArgNoCache) - if err != nil { - return err - } - timeout, err := c.Doit.GetDuration(c.NS, doctl.ArgTimeout) + + conf, err := newAppDevConfig(c) if err != nil { - return err + return fmt.Errorf("initializing config: %w", err) } + + var ( + // doctlConfig is the CLI config source. + doctlConfig = config.DoctlConfigSource(c.Doit, c.NS) + // appsDevConfig is the dev-config.yaml config source. + appsDevConfig = appsDevFlagConfigCompat(conf) + // globalConfig contains global config vars with cli flags as the first priority. + globalConfig = config.Multi( + doctlConfig, + appsDevConfig, + ) + ) + + timeout := globalConfig.GetDuration(doctl.ArgTimeout) if timeout > 0 { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, timeout) defer cancel() } - conf, err := newAppDevConfig(c) - if err != nil { - return fmt.Errorf("initializing config: %w", err) - } - var ( spec *godo.AppSpec cnbVersioning *godo.AppBuildConfigCNBVersioning ) - appID, err := conf.GetString(doctl.ArgApp) - if err != nil { - return err - } + appID := globalConfig.GetString(doctl.ArgApp) // TODO: if this is the user's first time running dev build, ask them if they'd like to // link an existing app. @@ -146,16 +152,10 @@ func RunAppsDevBuild(c *CmdConfig) error { cnbVersioning = app.GetBuildConfig().GetCNBVersioning() } - appSpecPath, err := conf.GetString(doctl.ArgAppSpec) - if err != nil { - return err - } - - if spec == nil && appSpecPath == "" { - if _, err := os.Stat(AppsDevDefaultSpecPath); err == nil { - appSpecPath = AppsDevDefaultSpecPath - template.Print(`{{success checkmark}} using app spec at {{highlight .}}{{nl}}`, AppsDevDefaultSpecPath) - } + appSpecPath := globalConfig.GetString(doctl.ArgAppSpec) + if spec == nil && appSpecPath == "" && fileExists(AppsDevDefaultSpecPath) { + appSpecPath = AppsDevDefaultSpecPath + template.Print(`{{success checkmark}} using app spec at {{highlight .}}{{nl}}`, AppsDevDefaultSpecPath) } if appSpecPath != "" { spec, err = readAppSpec(os.Stdin, appSpecPath) @@ -221,15 +221,9 @@ func RunAppsDevBuild(c *CmdConfig) error { return err } - buildingComponentLine := template.String( - `building {{lower (snakeToTitle .GetType)}} {{highlight .GetName}}`, - componentSpec, - ) - template.Print(`{{success checkmark}} {{.}}{{nl 2}}`, buildingComponentLine) - if componentSpec.GetSourceDir() != "" { sd := componentSpec.GetSourceDir() - stat, err := os.Stat(sd) + stat, err := os.Stat(conf.ContextPath(sd)) if err != nil { if errors.Is(err, os.ErrNotExist) { return fmt.Errorf("source dir %s does not exist. please make sure you are running doctl in your app directory", sd) @@ -241,29 +235,49 @@ func RunAppsDevBuild(c *CmdConfig) error { } } + var ( + // componentConfig is the per-component config source. + componentConfig = appsDevFlagConfigCompat(conf.Components(component)) + // componentGlobalConfig is per-component config that can be overridden at the global/app level as well as via cli flags. + componentGlobalConfig = config.Multi( + doctlConfig, + componentConfig, + appsDevFlagConfigCompat(conf), + ) + // componentArgConfig is per-component config that can be overridden via cli flags. + componentArgConfig = config.Multi( + doctlConfig, + componentConfig, + ) + ) + var envs map[string]string - envFile, err := conf.GetString(doctl.ArgEnvFile) - if err != nil { - return err + envFile := componentGlobalConfig.GetString(doctl.ArgEnvFile) + if envFile == "" && Interactive && fileExists(conf.ContextPath(AppsDevDefaultEnvFile)) { + choice, err := confirm.New( + template.String(`{{highlight .}} exists, use it for env var values?`, AppsDevDefaultEnvFile), + confirm.WithDefaultChoice(confirm.No), + ).Prompt() + if err != nil { + return err + } + if choice == confirm.Yes { + envFile = conf.ContextPath(AppsDevDefaultEnvFile) + } + } else if envFile != "" { + envFile = conf.ContextPath(envFile) } if envFile != "" { envs, err = godotenv.Read(envFile) if err != nil { - return err + return fmt.Errorf("reading env file: %w", err) } } - registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistryName) - if err != nil { - return err - } - - buildOverrride, err := conf.GetString(doctl.ArgAppDevBuildCommand) - if err != nil { - return err - } - + registryName := globalConfig.GetString(doctl.ArgRegistry) + noCache := globalConfig.GetBool(doctl.ArgNoCache) if noCache { + template.Render(text.Warning, `{{crossmark}} build caching disabled{{nl}}`, nil) err = conf.ClearCacheDir(ctx, component) if err != nil { return err @@ -274,19 +288,19 @@ func RunAppsDevBuild(c *CmdConfig) error { return err } - if Interactive { - choice, err := confirm.New( - "start build?", - confirm.WithDefaultChoice(confirm.Yes), - confirm.WithPersistPrompt(confirm.PersistPromptIfNo), - ).Prompt() - if err != nil { - return err - } - if choice != confirm.Yes { - return fmt.Errorf("cancelled") - } - } + // if Interactive { + // choice, err := confirm.New( + // "start build?", + // confirm.WithDefaultChoice(confirm.Yes), + // confirm.WithPersistPrompt(confirm.PersistPromptIfNo), + // ).Prompt() + // if err != nil { + // return err + // } + // if choice != confirm.Yes { + // return fmt.Errorf("cancelled") + // } + // } cli, err := c.Doit.GetDockerEngineClient() if err != nil { @@ -296,6 +310,12 @@ func RunAppsDevBuild(c *CmdConfig) error { ctx, cancel := context.WithCancel(ctx) defer cancel() + buildingComponentLine := template.String( + `building {{lower (snakeToTitle .GetType)}} {{highlight .GetName}}`, + componentSpec, + ) + template.Print(`{{success checkmark}} {{.}}{{nl 2}}`, buildingComponentLine) + var ( wg sync.WaitGroup logWriter io.Writer @@ -325,13 +345,13 @@ func RunAppsDevBuild(c *CmdConfig) error { err = func() error { defer cancel() - builder, err := c.componentBuilderFactory.NewComponentBuilder(cli, conf.contextDir, spec, builder.NewBuilderOpts{ + builder, err := c.componentBuilderFactory.NewComponentBuilder(cli, conf.ContextDir(), spec, builder.NewBuilderOpts{ Component: component, LocalCacheDir: conf.CacheDir(component), NoCache: noCache, Registry: registryName, EnvOverride: envs, - BuildCommandOverride: buildOverrride, + BuildCommandOverride: componentArgConfig.GetString(doctl.ArgAppDevBuildCommand), LogWriter: logWriter, Versioning: builder.Versioning{CNB: cnbVersioning}, }) @@ -342,7 +362,11 @@ func RunAppsDevBuild(c *CmdConfig) error { if err != nil { _, isSnap := os.LookupEnv("SNAP") if errors.Is(err, fs.ErrPermission) && isSnap { - template.Buffered(logWriter, `{{warning "Using the doctl Snap? Grant access to the doctl:app-dev-build plug to use this command with: sudo snap connect doctl:app-dev-build docker:docker-daemon"}}{{nl}}`, nil) + template.Buffered( + textbox.New().Warning().WithOutput(logWriter), + `Using the doctl Snap? Grant doctl access to Docker by running {{highlight "sudo snap connect doctl:app-dev-build docker:docker-daemon"}}`, + nil, + ) return err } return err @@ -365,7 +389,7 @@ func RunAppsDevBuild(c *CmdConfig) error { } else if res.ExitCode == 0 { template.Buffered( textbox.New().Success(), - `{{success checkmark}} successfully built {{success .img}} in {{warning (duration .dur)}}`, + `{{success checkmark}} successfully built {{success .img}} in {{highlight (duration .dur)}}`, map[string]any{ "img": res.Image, "dur": res.BuildDuration, @@ -374,7 +398,7 @@ func RunAppsDevBuild(c *CmdConfig) error { } else { template.Buffered( textbox.New().Error(), - `{{error crossmark}} build container exited with code {{highlight .code}} after {{warning (duration .dur)}}`, + `{{error crossmark}} build container exited with code {{error .code}} after {{highlight (duration .dur)}}`, map[string]any{ "code": res.ExitCode, "dur": res.BuildDuration, @@ -384,3 +408,19 @@ func RunAppsDevBuild(c *CmdConfig) error { fmt.Print("\n") return nil } + +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} + +func dashToUnderscore(key string) string { + return strings.ReplaceAll(key, "-", "_") +} + +// appsDevFlagConfigCompat replaces dashes with underscores in the key to keep compatibility with doctl.Arg* keys +// while keeping the config file keys consistent with the app spec naming convention. +// for example: --no-cache on the CLI will map to no_cache in the config file. +func appsDevFlagConfigCompat(cs config.ConfigSource) config.ConfigSource { + return config.MutatingConfigSource(cs, dashToUnderscore) +} diff --git a/commands/apps_dev_config.go b/commands/apps_dev_config.go index 0aee37661..c8060dea3 100644 --- a/commands/apps_dev_config.go +++ b/commands/apps_dev_config.go @@ -2,20 +2,17 @@ package commands import ( "bytes" - "context" "errors" - "fmt" "io/ioutil" "os" "path/filepath" "regexp" - "sort" "strings" "github.com/digitalocean/doctl" - "github.com/digitalocean/doctl/commands/displayers" + "github.com/digitalocean/doctl/commands/charm/template" + "github.com/digitalocean/doctl/internal/apps/config" "github.com/spf13/cobra" - "github.com/spf13/viper" ) const ( @@ -23,14 +20,6 @@ const ( DefaultDevConfigFile = "dev-config.yaml" ) -type appDevUnknownKeyErr struct { - key string -} - -func (e *appDevUnknownKeyErr) Error() string { - return fmt.Sprintf("unknown key: %s\nvalid keys: %s", e.key, outputValidAppDevKeys()) -} - // AppsDevConfig creates the apps dev config command subtree. func AppsDevConfig() *Command { cmd := &Command{ @@ -47,12 +36,12 @@ func AppsDevConfig() *Command { RunAppsDevConfigSet, "set KEY=VALUE...", "Set dev configuration settings.", - fmt.Sprintf(`Set dev configuration settings for a build. + "Set dev configuration settings for a build.", + // fmt.Sprintf(`Set dev configuration settings for a build. -Valid Keys: %s -`, outputValidAppDevKeys()), + // Valid Keys: %s + // `, config.ValidAppDevKeys()), Writer, - displayerType(&displayers.Apps{}), ) AddStringFlag( @@ -66,12 +55,12 @@ Valid Keys: %s RunAppsDevConfigUnset, "unset KEY...", "Unset dev configuration settings.", - fmt.Sprintf(`Unset dev configuration settings for a build. - -Valid Keys: %s -`, outputValidAppDevKeys()), + "Unset dev configuration settings for a build.", + // fmt.Sprintf(`Unset dev configuration settings for a build. + + // Valid Keys: %s + // `, config.ValidAppDevKeys()), Writer, - displayerType(&displayers.Apps{}), ) AddStringFlag( @@ -103,6 +92,7 @@ func RunAppsDevConfigSet(c *CmdConfig) error { if err != nil { return err } + template.Print(`{{success checkmark}} set new value for {{highlight .}}{{nl}}`, split[0]) } err = dev.WriteConfig() @@ -113,7 +103,7 @@ func RunAppsDevConfigSet(c *CmdConfig) error { return nil } -// RunAppsDevConfigUnset runs the set configuration command. +// RunAppsDevConfigUnset runs the unset configuration command. func RunAppsDevConfigUnset(c *CmdConfig) error { if len(c.Args) == 0 { return errors.New("you must provide at least one argument") @@ -129,133 +119,24 @@ func RunAppsDevConfigUnset(c *CmdConfig) error { if err != nil { return err } - } - - err = dev.WriteConfig() - if err != nil { - return err - } - - return nil -} -var validAppDevKeys = map[string]bool{ - doctl.ArgApp: true, - doctl.ArgAppSpec: true, - doctl.ArgEnvFile: true, - doctl.ArgRegistryName: true, - doctl.ArgAppDevBuildCommand: true, -} - -func outputValidAppDevKeys() string { - keys := []string{} - for k := range validAppDevKeys { - keys = append(keys, k) + template.Print(`{{success checkmark}} unset {{highlight .}}{{nl}}`, arg) } - sort.Strings(keys) - return strings.Join(keys, ", ") -} - -type appDevConfig struct { - contextDir string - cmdConfig *CmdConfig - viper *viper.Viper -} - -func (c *appDevConfig) CacheDir(component string) string { - return filepath.Join(c.contextDir, ".do", "cache", component) -} -func (c *appDevConfig) EnsureCacheDir(ctx context.Context, component string) error { - err := os.MkdirAll(filepath.Join(c.contextDir, ".do", "cache", component), os.ModePerm) + err = dev.WriteConfig() if err != nil { return err } - return ensureStringInFile(filepath.Join(c.contextDir, ".do", ".gitignore"), "/cache") -} - -func (c *appDevConfig) ClearCacheDir(ctx context.Context, component string) error { - return os.RemoveAll(filepath.Join(c.contextDir, ".do", "cache", component)) -} - -func (c *appDevConfig) WriteConfig() error { - return c.viper.WriteConfig() -} - -func (c *appDevConfig) Set(key string, value string) error { - if !validAppDevKeys[key] { - return &appDevUnknownKeyErr{key} - } - c.viper.Set(key, value) return nil } -func (c *appDevConfig) GetString(key string) (string, error) { - if !validAppDevKeys[key] { - return "", &appDevUnknownKeyErr{key} - } - if c.cmdConfig != nil { - if v, err := c.cmdConfig.Doit.GetString(c.cmdConfig.NS, key); v != "" { - return v, nil - } else if err != nil { - return "", err - } - } - return c.viper.GetString(key), nil -} - -func newAppDevConfig(cmdConfig *CmdConfig) (*appDevConfig, error) { - config := &appDevConfig{ - cmdConfig: cmdConfig, - viper: viper.New(), - } - +func newAppDevConfig(cmdConfig *CmdConfig) (*config.AppDev, error) { devConfigFilePath, err := cmdConfig.Doit.GetString(cmdConfig.NS, doctl.ArgAppDevConfig) if err != nil { return nil, err } - - config.contextDir, err = os.Getwd() - if err != nil { - return nil, err - } - gitRoot, err := findTopLevelGitDir(config.contextDir) - if err != nil && !errors.Is(err, errNoGitRepo) { - return nil, err - } - if gitRoot != "" { - config.contextDir = gitRoot - } - - if devConfigFilePath == "" { - if gitRoot == "" { - return nil, errNoGitRepo - } - configDir := filepath.Join(gitRoot, ".do") - err = os.MkdirAll(configDir, os.ModePerm) - if err != nil { - return nil, err - } - devConfigFilePath = filepath.Join(configDir, DefaultDevConfigFile) - if err := ensureStringInFile(devConfigFilePath, ""); err != nil { - return nil, err - } - if err := ensureStringInFile(filepath.Join(configDir, ".gitignore"), DefaultDevConfigFile); err != nil { - return nil, err - } - } else if _, err := os.Stat(devConfigFilePath); err != nil { - return nil, err - } - - config.viper.SetConfigType("yaml") - config.viper.SetConfigFile(devConfigFilePath) - - if err := config.viper.ReadInConfig(); err != nil { - return nil, err - } - - return config, nil + return config.New(devConfigFilePath) } func ensureStringInFile(file string, val string) error { diff --git a/commands/apps_dev_config_test.go b/commands/apps_dev_config_test.go index 819442e4c..b16fed390 100644 --- a/commands/apps_dev_config_test.go +++ b/commands/apps_dev_config_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/digitalocean/doctl" + "github.com/digitalocean/doctl/internal/apps/config" "github.com/stretchr/testify/require" ) @@ -18,12 +19,12 @@ func TestRunAppsDevConfigSet(t *testing.T) { os.Remove(file.Name()) }) - withTestClient(t, func(config *CmdConfig, tm *tcMocks) { + withTestClient(t, func(cmdConfig *CmdConfig, tm *tcMocks) { tcs := []struct { name string args []string expectErr error - expect func(*testing.T, *appDevConfig) + expect func(*testing.T, *config.AppDev) }{ { name: "no args", @@ -35,39 +36,34 @@ func TestRunAppsDevConfigSet(t *testing.T) { args: []string{"only-key"}, expectErr: errors.New("unexpected arg: only-key"), }, - { - name: "unknown key", - args: []string{"unknown=value"}, - expectErr: &appDevUnknownKeyErr{"unknown"}, - }, { name: "single key", args: []string{"app=12345"}, - expect: func(t *testing.T, c *appDevConfig) { - require.Equal(t, "12345", c.viper.Get("app"), "app-id") + expect: func(t *testing.T, c *config.AppDev) { + require.Equal(t, "12345", c.GetString("app"), "app-id") }, }, { name: "multiple keys", args: []string{"app=value1", "spec=value2"}, - expect: func(t *testing.T, c *appDevConfig) { - require.Equal(t, "value1", c.viper.Get("app"), "app-id") - require.Equal(t, "value2", c.viper.Get("spec"), "spec") + expect: func(t *testing.T, c *config.AppDev) { + require.Equal(t, "value1", c.GetString("app"), "app-id") + require.Equal(t, "value2", c.GetString("spec"), "spec") }, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - config.Args = tc.args - config.Doit.Set(config.NS, doctl.ArgAppDevConfig, file.Name()) - err = RunAppsDevConfigSet(config) + cmdConfig.Args = tc.args + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file.Name()) + err = RunAppsDevConfigSet(cmdConfig) if tc.expectErr != nil { require.EqualError(t, err, tc.expectErr.Error()) return } require.NoError(t, err, "running command") - devConf, err := newAppDevConfig(config) + devConf, err := newAppDevConfig(cmdConfig) require.NoError(t, err, "getting dev config") if tc.expect != nil { tc.expect(t, devConf) @@ -85,63 +81,58 @@ func TestRunAppsDevConfigUnset(t *testing.T) { os.Remove(file.Name()) }) - withTestClient(t, func(config *CmdConfig, tm *tcMocks) { + withTestClient(t, func(cmdConfig *CmdConfig, tm *tcMocks) { tcs := []struct { name string args []string - pre func(*testing.T, *appDevConfig) + pre func(*testing.T, *config.AppDev) expectErr error - expect func(*testing.T, *appDevConfig) + expect func(*testing.T, *config.AppDev) }{ { name: "no args", args: []string{}, expectErr: errors.New("you must provide at least one argument"), }, - { - name: "unknown key", - args: []string{"unknown"}, - expectErr: &appDevUnknownKeyErr{"unknown"}, - }, { name: "single key", args: []string{"app"}, - pre: func(t *testing.T, c *appDevConfig) { - c.viper.Set("app", "value") - err := c.viper.WriteConfig() + pre: func(t *testing.T, c *config.AppDev) { + c.Set("app", "value") + err := c.WriteConfig() require.NoError(t, err, "setting up default values") }, - expect: func(t *testing.T, c *appDevConfig) { - require.Equal(t, "", c.viper.Get("app"), "app-id") + expect: func(t *testing.T, c *config.AppDev) { + require.Equal(t, "", c.GetString("app"), "app-id") }, }, { name: "multiple keys", args: []string{"app", "spec"}, - pre: func(t *testing.T, c *appDevConfig) { - c.viper.Set("app", "value") - c.viper.Set("spec", "value") - err := c.viper.WriteConfig() + pre: func(t *testing.T, c *config.AppDev) { + c.Set("app", "value") + c.Set("spec", "value") + err := c.WriteConfig() require.NoError(t, err, "setting up default values") }, - expect: func(t *testing.T, c *appDevConfig) { - require.Equal(t, "", c.viper.Get("app"), "app-id") - require.Equal(t, "", c.viper.Get("spec"), "spec") + expect: func(t *testing.T, c *config.AppDev) { + require.Equal(t, "", c.GetString("app"), "app-id") + require.Equal(t, "", c.GetString("spec"), "spec") }, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - devConf, err := newAppDevConfig(config) + devConf, err := newAppDevConfig(cmdConfig) require.NoError(t, err, "getting dev config") if tc.pre != nil { tc.pre(t, devConf) } - config.Args = tc.args - config.Doit.Set(config.NS, doctl.ArgAppDevConfig, file.Name()) - err = RunAppsDevConfigUnset(config) + cmdConfig.Args = tc.args + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file.Name()) + err = RunAppsDevConfigUnset(cmdConfig) if tc.expectErr != nil { require.EqualError(t, err, tc.expectErr.Error()) return @@ -149,7 +140,7 @@ func TestRunAppsDevConfigUnset(t *testing.T) { require.NoError(t, err, "running command") if tc.expect != nil { - devConf, err = newAppDevConfig(config) + devConf, err = newAppDevConfig(cmdConfig) require.NoError(t, err, "getting dev config") tc.expect(t, devConf) } diff --git a/commands/apps_dev_test.go b/commands/apps_dev_test.go index 73dc55c21..47735c9d0 100644 --- a/commands/apps_dev_test.go +++ b/commands/apps_dev_test.go @@ -2,6 +2,8 @@ package commands import ( "encoding/json" + "os" + "path/filepath" "testing" "github.com/digitalocean/doctl" @@ -25,20 +27,22 @@ func TestRunAppsDevBuild(t *testing.T) { t.Run("with local app spec", func(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { + setTempWorkingDir(t) + specJSON, err := json.Marshal(sampleSpec) require.NoError(t, err, "marshalling sample spec") specFile := testTempFile(t, []byte(specJSON)) config.Args = append(config.Args, component) config.Doit.Set(config.NS, doctl.ArgAppSpec, specFile) - config.Doit.Set(config.NS, doctl.ArgRegistryName, registryName) + config.Doit.Set(config.NS, doctl.ArgRegistry, registryName) config.Doit.Set(config.NS, doctl.ArgInteractive, false) conf, err := newAppDevConfig(config) require.NoError(t, err) tm.appBuilder.EXPECT().Build(gomock.Any()).Return(builder.ComponentBuilderResult{}, nil) - tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), conf.contextDir, sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) + tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), conf.ContextDir(), sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) err = RunAppsDevBuild(config) require.NoError(t, err) @@ -47,9 +51,11 @@ func TestRunAppsDevBuild(t *testing.T) { t.Run("with appID", func(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { + setTempWorkingDir(t) + conf, err := newAppDevConfig(config) require.NoError(t, err) - tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), conf.contextDir, sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) + tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), conf.ContextDir(), sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) tm.appBuilder.EXPECT().Build(gomock.Any()).Return(builder.ComponentBuilderResult{}, nil) tm.apps.EXPECT().Get(appID).Times(1).Return(&godo.App{ @@ -58,7 +64,7 @@ func TestRunAppsDevBuild(t *testing.T) { config.Args = append(config.Args, component) config.Doit.Set(config.NS, doctl.ArgApp, appID) - config.Doit.Set(config.NS, doctl.ArgRegistryName, registryName) + config.Doit.Set(config.NS, doctl.ArgRegistry, registryName) config.Doit.Set(config.NS, doctl.ArgInteractive, false) err = RunAppsDevBuild(config) @@ -66,3 +72,15 @@ func TestRunAppsDevBuild(t *testing.T) { }) }) } + +func setTempWorkingDir(t *testing.T) { + tmp := t.TempDir() + err := os.Mkdir(filepath.Join(tmp, ".git"), os.ModePerm) + require.NoError(t, err) + oldCwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { + os.Chdir(oldCwd) + }) + os.Chdir(tmp) +} diff --git a/internal/apps/config/appdev.go b/internal/apps/config/appdev.go new file mode 100644 index 000000000..d52593c6e --- /dev/null +++ b/internal/apps/config/appdev.go @@ -0,0 +1,223 @@ +package config + +import ( + "bytes" + "context" + "errors" + "io/ioutil" + "os" + "path/filepath" + "regexp" + "time" + + "github.com/spf13/viper" +) + +const ( + // DefaultDevConfigFile is the name of the default dev configuration file. + DefaultDevConfigFile = "dev-config.yaml" + + // nsComponents is the namespace of the component-specific config tree. + nsComponents = "components" +) + +// type appDevUnknownKeyErr struct { +// key string +// } + +// func (e *appDevUnknownKeyErr) Error() string { +// return fmt.Sprintf("unknown key: %s\nvalid keys: %s", e.key, ValidAppDevKeys()) +// } + +// var validAppDevKeys = map[string]bool{ +// doctl.ArgApp: true, +// doctl.ArgAppSpec: true, +// doctl.ArgEnvFile: true, +// doctl.ArgRegistry: true, +// doctl.ArgAppDevBuildCommand: true, +// } + +// func ValidAppDevKeys() string { +// keys := []string{} +// for k := range validAppDevKeys { +// keys = append(keys, k) +// } +// sort.Strings(keys) +// return strings.Join(keys, ", ") +// } + +type AppDev struct { + contextDir string + doctlConfig ConfigSource + viper *viper.Viper +} + +func (c *AppDev) CacheDir(component string) string { + return c.ContextPath(".do", "cache", component) +} + +func (c *AppDev) EnsureCacheDir(ctx context.Context, component string) error { + err := os.MkdirAll(c.ContextPath(".do", "cache", component), os.ModePerm) + if err != nil { + return err + } + + return ensureStringInFile(c.ContextPath(".do", ".gitignore"), "/cache") +} + +func (c *AppDev) ClearCacheDir(ctx context.Context, component string) error { + return os.RemoveAll(c.ContextPath(".do", "cache", component)) +} + +func (c *AppDev) WriteConfig() error { + return c.viper.WriteConfig() +} + +func (c *AppDev) Set(key string, value any) error { + // if !validAppDevKeys[key] { + // return &appDevUnknownKeyErr{key} + // } + c.viper.Set(key, value) + return nil +} + +func (c *AppDev) Components(component string) ConfigSource { + return NamespacedConfigSource(c, nsKey(nsComponents, component)) +} + +func (c *AppDev) ContextDir() string { + return c.contextDir +} + +func (c *AppDev) ContextPath(path ...string) string { + return filepath.Join(append([]string{c.contextDir}, path...)...) +} + +func (c *AppDev) IsSet(key string) bool { + return c.viper.IsSet(key) +} + +func (c *AppDev) GetString(key string) string { + return c.viper.GetString(key) +} + +func (c *AppDev) GetBool(key string) bool { + return c.viper.GetBool(key) +} + +func (c *AppDev) GetDuration(key string) time.Duration { + return c.viper.GetDuration(key) +} + +func New(path string) (*AppDev, error) { + config := &AppDev{ + viper: viper.New(), + } + + var err error + config.contextDir, err = os.Getwd() + if err != nil { + return nil, err + } + gitRoot, err := findTopLevelGitDir(config.contextDir) + if err != nil && !errors.Is(err, errNoGitRepo) { + return nil, err + } + if gitRoot != "" { + config.contextDir = gitRoot + } + + if path == "" { + configDir := config.ContextPath(".do") + err = os.MkdirAll(configDir, os.ModePerm) + if err != nil { + return nil, err + } + path = filepath.Join(configDir, DefaultDevConfigFile) + if err := ensureStringInFile(path, ""); err != nil { + return nil, err + } + if err := ensureStringInFile(filepath.Join(configDir, ".gitignore"), DefaultDevConfigFile); err != nil { + return nil, err + } + } else if _, err := os.Stat(path); err != nil { + return nil, err + } + + config.viper.SetConfigType("yaml") + config.viper.SetConfigFile(path) + + if err := config.viper.ReadInConfig(); err != nil { + return nil, err + } + + return config, nil +} + +func ensureStringInFile(file string, val string) error { + if _, err := os.Stat(file); errors.Is(err, os.ErrNotExist) { + f, err := os.OpenFile( + file, + os.O_APPEND|os.O_CREATE|os.O_WRONLY, + 0644, + ) + if err != nil { + return err + } + defer f.Close() + _, err = f.WriteString(val) + return err + } else if err != nil { + return err + } + + b, err := ioutil.ReadFile(file) + if err != nil { + return err + } + + if exists, err := regexp.Match(regexp.QuoteMeta(val), b); err != nil { + return err + } else if !exists { + f, err := os.OpenFile( + file, + os.O_APPEND|os.O_WRONLY, + 0644, + ) + if err != nil { + return err + } + defer f.Close() + + if !bytes.HasSuffix(b, []byte("\n")) { + val = "\n" + val + } + + _, err = f.WriteString(val) + return err + } + + return nil +} + +var errNoGitRepo = errors.New("no git repository found") + +// findTopLevelGitDir ... +func findTopLevelGitDir(workingDir string) (string, error) { + dir, err := filepath.Abs(workingDir) + if err != nil { + return "", err + } + + for { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return dir, nil + } + + parent := filepath.Dir(dir) + if parent == dir { + return "", errors.New("no git repository found") + } + dir = parent + } +} From 5b0208c469c3c521953f888b9af588c7ce529f23 Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Thu, 25 Aug 2022 23:47:48 +0300 Subject: [PATCH 4/9] address pr feedback, app dev workspace --- commands/apps_dev.go | 73 +++---- commands/apps_dev_config.go | 75 +------ commands/apps_dev_config_test.go | 139 ++++--------- commands/apps_dev_test.go | 12 +- internal/apps/config/appdev.go | 96 +-------- internal/apps/config/config.go | 48 ++--- internal/apps/config/config_test.go | 28 +-- internal/apps/config/doctl.go | 7 +- internal/apps/workspace/workspace.go | 240 ++++++++++++++++++++++ internal/apps/workspace/workspace_test.go | 79 +++++++ 10 files changed, 437 insertions(+), 360 deletions(-) create mode 100644 internal/apps/workspace/workspace.go create mode 100644 internal/apps/workspace/workspace_test.go diff --git a/commands/apps_dev.go b/commands/apps_dev.go index d0dd9c5bf..1c9acfdb2 100644 --- a/commands/apps_dev.go +++ b/commands/apps_dev.go @@ -7,7 +7,6 @@ import ( "io" "io/fs" "os" - "strings" "sync" "github.com/MakeNowJust/heredoc" @@ -20,6 +19,7 @@ import ( "github.com/digitalocean/doctl/commands/charm/textbox" "github.com/digitalocean/doctl/internal/apps/builder" "github.com/digitalocean/doctl/internal/apps/config" + "github.com/digitalocean/doctl/internal/apps/workspace" "github.com/digitalocean/godo" "github.com/joho/godotenv" @@ -110,23 +110,12 @@ func AppsDev() *Command { func RunAppsDevBuild(c *CmdConfig) error { ctx := context.Background() - conf, err := newAppDevConfig(c) + ws, err := appDevWorkspace(c) if err != nil { - return fmt.Errorf("initializing config: %w", err) + return fmt.Errorf("preparing workspace: %w", err) } - var ( - // doctlConfig is the CLI config source. - doctlConfig = config.DoctlConfigSource(c.Doit, c.NS) - // appsDevConfig is the dev-config.yaml config source. - appsDevConfig = appsDevFlagConfigCompat(conf) - // globalConfig contains global config vars with cli flags as the first priority. - globalConfig = config.Multi( - doctlConfig, - appsDevConfig, - ) - ) - + globalConfig := ws.Config.Global(true) timeout := globalConfig.GetDuration(doctl.ArgTimeout) if timeout > 0 { var cancel context.CancelFunc @@ -223,7 +212,7 @@ func RunAppsDevBuild(c *CmdConfig) error { if componentSpec.GetSourceDir() != "" { sd := componentSpec.GetSourceDir() - stat, err := os.Stat(conf.ContextPath(sd)) + stat, err := os.Stat(ws.Context(sd)) if err != nil { if errors.Is(err, os.ErrNotExist) { return fmt.Errorf("source dir %s does not exist. please make sure you are running doctl in your app directory", sd) @@ -235,25 +224,10 @@ func RunAppsDevBuild(c *CmdConfig) error { } } - var ( - // componentConfig is the per-component config source. - componentConfig = appsDevFlagConfigCompat(conf.Components(component)) - // componentGlobalConfig is per-component config that can be overridden at the global/app level as well as via cli flags. - componentGlobalConfig = config.Multi( - doctlConfig, - componentConfig, - appsDevFlagConfigCompat(conf), - ) - // componentArgConfig is per-component config that can be overridden via cli flags. - componentArgConfig = config.Multi( - doctlConfig, - componentConfig, - ) - ) - + componentConfig, componentGlobalConfig := ws.Config.Component(component, true) var envs map[string]string envFile := componentGlobalConfig.GetString(doctl.ArgEnvFile) - if envFile == "" && Interactive && fileExists(conf.ContextPath(AppsDevDefaultEnvFile)) { + if envFile == "" && Interactive && fileExists(ws.Context(AppsDevDefaultEnvFile)) { choice, err := confirm.New( template.String(`{{highlight .}} exists, use it for env var values?`, AppsDevDefaultEnvFile), confirm.WithDefaultChoice(confirm.No), @@ -262,10 +236,10 @@ func RunAppsDevBuild(c *CmdConfig) error { return err } if choice == confirm.Yes { - envFile = conf.ContextPath(AppsDevDefaultEnvFile) + envFile = ws.Context(AppsDevDefaultEnvFile) } } else if envFile != "" { - envFile = conf.ContextPath(envFile) + envFile = ws.Context(envFile) } if envFile != "" { envs, err = godotenv.Read(envFile) @@ -274,16 +248,16 @@ func RunAppsDevBuild(c *CmdConfig) error { } } - registryName := globalConfig.GetString(doctl.ArgRegistry) + registry := globalConfig.GetString(doctl.ArgRegistry) noCache := globalConfig.GetBool(doctl.ArgNoCache) if noCache { template.Render(text.Warning, `{{crossmark}} build caching disabled{{nl}}`, nil) - err = conf.ClearCacheDir(ctx, component) + err = ws.ClearCacheDir(ctx, component) if err != nil { return err } } - err = conf.EnsureCacheDir(ctx, component) + err = ws.EnsureCacheDir(ctx, component) if err != nil { return err } @@ -345,13 +319,13 @@ func RunAppsDevBuild(c *CmdConfig) error { err = func() error { defer cancel() - builder, err := c.componentBuilderFactory.NewComponentBuilder(cli, conf.ContextDir(), spec, builder.NewBuilderOpts{ + builder, err := c.componentBuilderFactory.NewComponentBuilder(cli, ws.Context(), spec, builder.NewBuilderOpts{ Component: component, - LocalCacheDir: conf.CacheDir(component), + LocalCacheDir: ws.CacheDir(component), NoCache: noCache, - Registry: registryName, + Registry: registry, EnvOverride: envs, - BuildCommandOverride: componentArgConfig.GetString(doctl.ArgAppDevBuildCommand), + BuildCommandOverride: componentConfig.GetString(doctl.ArgAppDevBuildCommand), LogWriter: logWriter, Versioning: builder.Versioning{CNB: cnbVersioning}, }) @@ -414,13 +388,12 @@ func fileExists(path string) bool { return err == nil } -func dashToUnderscore(key string) string { - return strings.ReplaceAll(key, "-", "_") -} +func appDevWorkspace(cmdConfig *CmdConfig) (*workspace.AppDev, error) { + devConfigFilePath, err := cmdConfig.Doit.GetString(cmdConfig.NS, doctl.ArgAppDevConfig) + if err != nil { + return nil, err + } + doctlConfig := config.DoctlConfigSource(cmdConfig.Doit, cmdConfig.NS) -// appsDevFlagConfigCompat replaces dashes with underscores in the key to keep compatibility with doctl.Arg* keys -// while keeping the config file keys consistent with the app spec naming convention. -// for example: --no-cache on the CLI will map to no_cache in the config file. -func appsDevFlagConfigCompat(cs config.ConfigSource) config.ConfigSource { - return config.MutatingConfigSource(cs, dashToUnderscore) + return workspace.NewAppDev(devConfigFilePath, doctlConfig) } diff --git a/commands/apps_dev_config.go b/commands/apps_dev_config.go index c8060dea3..7ea9f8344 100644 --- a/commands/apps_dev_config.go +++ b/commands/apps_dev_config.go @@ -1,17 +1,14 @@ package commands import ( - "bytes" "errors" - "io/ioutil" + "fmt" "os" "path/filepath" - "regexp" "strings" "github.com/digitalocean/doctl" "github.com/digitalocean/doctl/commands/charm/template" - "github.com/digitalocean/doctl/internal/apps/config" "github.com/spf13/cobra" ) @@ -78,9 +75,9 @@ func RunAppsDevConfigSet(c *CmdConfig) error { return errors.New("you must provide at least one argument") } - dev, err := newAppDevConfig(c) + ws, err := appDevWorkspace(c) if err != nil { - return err + return fmt.Errorf("preparing workspace: %w", err) } for _, arg := range c.Args { @@ -88,14 +85,14 @@ func RunAppsDevConfigSet(c *CmdConfig) error { if len(split) != 2 { return errors.New("unexpected arg: " + arg) } - err := dev.Set(split[0], split[1]) + err := ws.Config.Set(split[0], split[1]) if err != nil { return err } template.Print(`{{success checkmark}} set new value for {{highlight .}}{{nl}}`, split[0]) } - err = dev.WriteConfig() + err = ws.Config.Write() if err != nil { return err } @@ -109,13 +106,13 @@ func RunAppsDevConfigUnset(c *CmdConfig) error { return errors.New("you must provide at least one argument") } - dev, err := newAppDevConfig(c) + ws, err := appDevWorkspace(c) if err != nil { - return err + return fmt.Errorf("preparing workspace: %w", err) } for _, arg := range c.Args { - err = dev.Set(arg, "") + err = ws.Config.Set(arg, "") if err != nil { return err } @@ -123,65 +120,11 @@ func RunAppsDevConfigUnset(c *CmdConfig) error { template.Print(`{{success checkmark}} unset {{highlight .}}{{nl}}`, arg) } - err = dev.WriteConfig() - if err != nil { - return err - } - - return nil -} - -func newAppDevConfig(cmdConfig *CmdConfig) (*config.AppDev, error) { - devConfigFilePath, err := cmdConfig.Doit.GetString(cmdConfig.NS, doctl.ArgAppDevConfig) - if err != nil { - return nil, err - } - return config.New(devConfigFilePath) -} - -func ensureStringInFile(file string, val string) error { - if _, err := os.Stat(file); errors.Is(err, os.ErrNotExist) { - f, err := os.OpenFile( - file, - os.O_APPEND|os.O_CREATE|os.O_WRONLY, - 0644, - ) - if err != nil { - return err - } - defer f.Close() - _, err = f.WriteString(val) - return err - } else if err != nil { - return err - } - - b, err := ioutil.ReadFile(file) + err = ws.Config.Write() if err != nil { return err } - if exists, err := regexp.Match(regexp.QuoteMeta(val), b); err != nil { - return err - } else if !exists { - f, err := os.OpenFile( - file, - os.O_APPEND|os.O_WRONLY, - 0644, - ) - if err != nil { - return err - } - defer f.Close() - - if !bytes.HasSuffix(b, []byte("\n")) { - val = "\n" + val - } - - _, err = f.WriteString(val) - return err - } - return nil } diff --git a/commands/apps_dev_config_test.go b/commands/apps_dev_config_test.go index b16fed390..28b1d6a6f 100644 --- a/commands/apps_dev_config_test.go +++ b/commands/apps_dev_config_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/digitalocean/doctl" - "github.com/digitalocean/doctl/internal/apps/config" + "github.com/digitalocean/doctl/internal/apps/workspace" "github.com/stretchr/testify/require" ) @@ -24,7 +24,7 @@ func TestRunAppsDevConfigSet(t *testing.T) { name string args []string expectErr error - expect func(*testing.T, *config.AppDev) + expect func(*testing.T, *workspace.AppDev) }{ { name: "no args", @@ -39,34 +39,38 @@ func TestRunAppsDevConfigSet(t *testing.T) { { name: "single key", args: []string{"app=12345"}, - expect: func(t *testing.T, c *config.AppDev) { - require.Equal(t, "12345", c.GetString("app"), "app-id") + expect: func(t *testing.T, ws *workspace.AppDev) { + require.Equal(t, "12345", ws.Config.Global(false).GetString("app"), "app-id") }, }, { name: "multiple keys", args: []string{"app=value1", "spec=value2"}, - expect: func(t *testing.T, c *config.AppDev) { - require.Equal(t, "value1", c.GetString("app"), "app-id") - require.Equal(t, "value2", c.GetString("spec"), "spec") + expect: func(t *testing.T, ws *workspace.AppDev) { + require.Equal(t, "value1", ws.Config.Global(false).GetString("app"), "app-id") + require.Equal(t, "value2", ws.Config.Global(false).GetString("spec"), "spec") }, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - cmdConfig.Args = tc.args cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file.Name()) + + ws, err := appDevWorkspace(cmdConfig) + require.NoError(t, err, "getting workspace") + cmdConfig.Args = tc.args err = RunAppsDevConfigSet(cmdConfig) if tc.expectErr != nil { require.EqualError(t, err, tc.expectErr.Error()) return } require.NoError(t, err, "running command") - devConf, err := newAppDevConfig(cmdConfig) - require.NoError(t, err, "getting dev config") + + ws, err = appDevWorkspace(cmdConfig) + require.NoError(t, err, "getting workspace") if tc.expect != nil { - tc.expect(t, devConf) + tc.expect(t, ws) } }) } @@ -85,9 +89,9 @@ func TestRunAppsDevConfigUnset(t *testing.T) { tcs := []struct { name string args []string - pre func(*testing.T, *config.AppDev) + pre func(*testing.T, *workspace.AppDev) expectErr error - expect func(*testing.T, *config.AppDev) + expect func(*testing.T, *workspace.AppDev) }{ { name: "no args", @@ -97,41 +101,42 @@ func TestRunAppsDevConfigUnset(t *testing.T) { { name: "single key", args: []string{"app"}, - pre: func(t *testing.T, c *config.AppDev) { - c.Set("app", "value") - err := c.WriteConfig() + pre: func(t *testing.T, ws *workspace.AppDev) { + ws.Config.Set("app", "value") + err := ws.Config.Write() require.NoError(t, err, "setting up default values") }, - expect: func(t *testing.T, c *config.AppDev) { - require.Equal(t, "", c.GetString("app"), "app-id") + expect: func(t *testing.T, ws *workspace.AppDev) { + require.Equal(t, "", ws.Config.Global(false).GetString("app"), "app-id") }, }, { name: "multiple keys", args: []string{"app", "spec"}, - pre: func(t *testing.T, c *config.AppDev) { - c.Set("app", "value") - c.Set("spec", "value") - err := c.WriteConfig() + pre: func(t *testing.T, ws *workspace.AppDev) { + ws.Config.Set("app", "value") + ws.Config.Set("spec", "value") + err := ws.Config.Write() require.NoError(t, err, "setting up default values") }, - expect: func(t *testing.T, c *config.AppDev) { - require.Equal(t, "", c.GetString("app"), "app-id") - require.Equal(t, "", c.GetString("spec"), "spec") + expect: func(t *testing.T, ws *workspace.AppDev) { + require.Equal(t, "", ws.Config.Global(false).GetString("app"), "app-id") + require.Equal(t, "", ws.Config.Global(false).GetString("spec"), "spec") }, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - devConf, err := newAppDevConfig(cmdConfig) - require.NoError(t, err, "getting dev config") + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file.Name()) + + ws, err := appDevWorkspace(cmdConfig) + require.NoError(t, err, "getting workspace") if tc.pre != nil { - tc.pre(t, devConf) + tc.pre(t, ws) } cmdConfig.Args = tc.args - cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file.Name()) err = RunAppsDevConfigUnset(cmdConfig) if tc.expectErr != nil { require.EqualError(t, err, tc.expectErr.Error()) @@ -139,82 +144,12 @@ func TestRunAppsDevConfigUnset(t *testing.T) { } require.NoError(t, err, "running command") + ws, err = appDevWorkspace(cmdConfig) + require.NoError(t, err, "getting workspace") if tc.expect != nil { - devConf, err = newAppDevConfig(cmdConfig) - require.NoError(t, err, "getting dev config") - tc.expect(t, devConf) + tc.expect(t, ws) } }) } }) } - -func Test_ensureStringInFile(t *testing.T) { - ensureValue := "newvalue" - - tcs := []struct { - name string - pre func(t *testing.T, fname string) - expect []byte - }{ - { - name: "no pre-existing file", - pre: func(t *testing.T, fname string) {}, - expect: []byte(ensureValue), - }, - { - name: "pre-existing file with value", - pre: func(t *testing.T, fname string) { - f, err := os.OpenFile(fname, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) - require.NoError(t, err) - f.WriteString("line1\n" + ensureValue) - f.Close() - }, - expect: []byte("line1\n" + ensureValue), - }, - { - name: "pre-existing file without value", - pre: func(t *testing.T, fname string) { - f, err := os.OpenFile(fname, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) - require.NoError(t, err) - f.WriteString("line1\n") - f.Close() - }, - expect: []byte("line1\n" + ensureValue), - }, - { - name: "pre-existing file without value or newline", - pre: func(t *testing.T, fname string) { - f, err := os.OpenFile(fname, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) - require.NoError(t, err) - f.WriteString("line1") - f.Close() - }, - expect: []byte("line1\n" + ensureValue), - }, - } - - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - file, err := ioutil.TempFile("", "dev-config.*.yaml") - require.NoError(t, err, "creating temp file") - file.Close() - - // allow the test to dictate existence; we just use this - // to get a valid temporary filename that is unique - err = os.Remove(file.Name()) - require.NoError(t, err, "deleting temp file") - - if tc.pre != nil { - tc.pre(t, file.Name()) - } - - err = ensureStringInFile(file.Name(), ensureValue) - require.NoError(t, err, "ensuring string in file") - - b, err := ioutil.ReadFile(file.Name()) - require.NoError(t, err) - require.Equal(t, string(tc.expect), string(b)) - }) - } -} diff --git a/commands/apps_dev_test.go b/commands/apps_dev_test.go index 47735c9d0..ac25ce3e7 100644 --- a/commands/apps_dev_test.go +++ b/commands/apps_dev_test.go @@ -38,11 +38,11 @@ func TestRunAppsDevBuild(t *testing.T) { config.Doit.Set(config.NS, doctl.ArgRegistry, registryName) config.Doit.Set(config.NS, doctl.ArgInteractive, false) - conf, err := newAppDevConfig(config) - require.NoError(t, err) + ws, err := appDevWorkspace(config) + require.NoError(t, err, "getting workspace") tm.appBuilder.EXPECT().Build(gomock.Any()).Return(builder.ComponentBuilderResult{}, nil) - tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), conf.ContextDir(), sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) + tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), ws.Context(), sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) err = RunAppsDevBuild(config) require.NoError(t, err) @@ -53,9 +53,9 @@ func TestRunAppsDevBuild(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { setTempWorkingDir(t) - conf, err := newAppDevConfig(config) - require.NoError(t, err) - tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), conf.ContextDir(), sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) + ws, err := appDevWorkspace(config) + require.NoError(t, err, "getting workspace") + tm.appBuilderFactory.EXPECT().NewComponentBuilder(gomock.Any(), ws.Context(), sampleSpec, gomock.Any()).Return(tm.appBuilder, nil) tm.appBuilder.EXPECT().Build(gomock.Any()).Return(builder.ComponentBuilderResult{}, nil) tm.apps.EXPECT().Get(appID).Times(1).Return(&godo.App{ diff --git a/internal/apps/config/appdev.go b/internal/apps/config/appdev.go index d52593c6e..ddd4dce6d 100644 --- a/internal/apps/config/appdev.go +++ b/internal/apps/config/appdev.go @@ -2,11 +2,9 @@ package config import ( "bytes" - "context" "errors" "io/ioutil" "os" - "path/filepath" "regexp" "time" @@ -47,26 +45,7 @@ const ( // } type AppDev struct { - contextDir string - doctlConfig ConfigSource - viper *viper.Viper -} - -func (c *AppDev) CacheDir(component string) string { - return c.ContextPath(".do", "cache", component) -} - -func (c *AppDev) EnsureCacheDir(ctx context.Context, component string) error { - err := os.MkdirAll(c.ContextPath(".do", "cache", component), os.ModePerm) - if err != nil { - return err - } - - return ensureStringInFile(c.ContextPath(".do", ".gitignore"), "/cache") -} - -func (c *AppDev) ClearCacheDir(ctx context.Context, component string) error { - return os.RemoveAll(c.ContextPath(".do", "cache", component)) + viper *viper.Viper } func (c *AppDev) WriteConfig() error { @@ -82,15 +61,7 @@ func (c *AppDev) Set(key string, value any) error { } func (c *AppDev) Components(component string) ConfigSource { - return NamespacedConfigSource(c, nsKey(nsComponents, component)) -} - -func (c *AppDev) ContextDir() string { - return c.contextDir -} - -func (c *AppDev) ContextPath(path ...string) string { - return filepath.Join(append([]string{c.contextDir}, path...)...) + return MutatingConfigSource(c, KeyNamespaceMutator(nsKey(nsComponents, component)), nil) } func (c *AppDev) IsSet(key string) bool { @@ -110,48 +81,19 @@ func (c *AppDev) GetDuration(key string) time.Duration { } func New(path string) (*AppDev, error) { - config := &AppDev{ - viper: viper.New(), - } - - var err error - config.contextDir, err = os.Getwd() - if err != nil { - return nil, err - } - gitRoot, err := findTopLevelGitDir(config.contextDir) - if err != nil && !errors.Is(err, errNoGitRepo) { + viper := viper.New() + if _, err := os.Stat(path); err != nil { return nil, err } - if gitRoot != "" { - config.contextDir = gitRoot - } - if path == "" { - configDir := config.ContextPath(".do") - err = os.MkdirAll(configDir, os.ModePerm) - if err != nil { - return nil, err - } - path = filepath.Join(configDir, DefaultDevConfigFile) - if err := ensureStringInFile(path, ""); err != nil { - return nil, err - } - if err := ensureStringInFile(filepath.Join(configDir, ".gitignore"), DefaultDevConfigFile); err != nil { - return nil, err - } - } else if _, err := os.Stat(path); err != nil { - return nil, err - } + viper.SetConfigType("yaml") + viper.SetConfigFile(path) - config.viper.SetConfigType("yaml") - config.viper.SetConfigFile(path) - - if err := config.viper.ReadInConfig(); err != nil { + if err := viper.ReadInConfig(); err != nil { return nil, err } - return config, nil + return &AppDev{viper}, nil } func ensureStringInFile(file string, val string) error { @@ -199,25 +141,3 @@ func ensureStringInFile(file string, val string) error { return nil } - -var errNoGitRepo = errors.New("no git repository found") - -// findTopLevelGitDir ... -func findTopLevelGitDir(workingDir string) (string, error) { - dir, err := filepath.Abs(workingDir) - if err != nil { - return "", err - } - - for { - if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { - return dir, nil - } - - parent := filepath.Dir(dir) - if parent == dir { - return "", errors.New("no git repository found") - } - dir = parent - } -} diff --git a/internal/apps/config/config.go b/internal/apps/config/config.go index 78d3db109..adc0c9c0d 100644 --- a/internal/apps/config/config.go +++ b/internal/apps/config/config.go @@ -14,55 +14,55 @@ type ConfigSource interface { } type mutatingConfigSource struct { - cs ConfigSource - mutateKey func(key string) string - mutateIsSet bool // some config sources except IsSet to _not_ include the namespace + cs ConfigSource + mutateKey func(key string) string + // excludeMethods is a list of methods that should receive the original non-mutated input. example: "IsSet". + excludeMethods map[string]bool } -func (s *mutatingConfigSource) key(key string) string { - if s.mutateKey != nil { +func (s *mutatingConfigSource) key(method string, key string) string { + if !s.excludeMethods[method] && s.mutateKey != nil { key = s.mutateKey(key) } return key } func (s *mutatingConfigSource) IsSet(key string) bool { - if s.mutateIsSet { - key = s.key(key) - } - return s.cs.IsSet(key) + return s.cs.IsSet(s.key("IsSet", key)) } func (s *mutatingConfigSource) GetString(key string) string { - return s.cs.GetString(s.key(key)) + return s.cs.GetString(s.key("GetString", key)) } func (s *mutatingConfigSource) GetBool(key string) bool { - return s.cs.GetBool(s.key(key)) + return s.cs.GetBool(s.key("GetBool", key)) } func (s *mutatingConfigSource) GetDuration(key string) time.Duration { - return s.cs.GetDuration(s.key(key)) + return s.cs.GetDuration(s.key("GetDuration", key)) } -func MutatingConfigSource(cs ConfigSource, mutateKey func(key string) string) ConfigSource { +func MutatingConfigSource(cs ConfigSource, mutateKey func(key string) string, excludeMethods []string) ConfigSource { + excludeMethodsMap := make(map[string]bool) + for _, m := range excludeMethods { + excludeMethodsMap[m] = true + } return &mutatingConfigSource{ - cs: cs, - mutateKey: mutateKey, - mutateIsSet: true, + cs: cs, + mutateKey: mutateKey, + excludeMethods: excludeMethodsMap, } } -// NamespacedConfigSource accepts a ConfigSource and configures a default namespace that is prefixed to the key on all -// Get* calls. -func NamespacedConfigSource(cs ConfigSource, ns string) ConfigSource { - var mutateKey func(string) string - if ns != "" { - mutateKey = func(key string) string { - return nsKey(ns, key) +// KeyNamespaceMutator returns a mutator that prefixes a namespace to the key with a `.` delimiter. +func KeyNamespaceMutator(ns string) func(key string) string { + return func(key string) string { + if ns == "" { + return key } + return nsKey(ns, key) } - return MutatingConfigSource(cs, mutateKey) } // Multi returns a config source that wraps multiple config sources. diff --git a/internal/apps/config/config_test.go b/internal/apps/config/config_test.go index ef5173287..43ec93e92 100644 --- a/internal/apps/config/config_test.go +++ b/internal/apps/config/config_test.go @@ -1,7 +1,6 @@ package config import ( - "fmt" "testing" "time" @@ -30,7 +29,7 @@ func TestMutatingConfigSource(t *testing.T) { "2": "two", "3": "three", }[key] - }) + }, nil) for _, k := range []string{"one", "two", "three"} { assert.False(t, mcs.IsSet(k), k) } @@ -43,25 +42,14 @@ func TestMutatingConfigSource(t *testing.T) { assert.Equal(t, "hello", mcs.GetString("3")) } -func TestNamespacedConfigSource(t *testing.T) { - cs := NewTestConfigSource(map[string]any{ - "config.key1": true, - "config.key2": time.Minute, - "config.key3": "hello", - }) - for i := 1; i <= 3; i++ { - assert.True(t, cs.IsSet(fmt.Sprintf("config.key%d", i))) - assert.False(t, cs.IsSet(fmt.Sprintf("key%d", i))) - } +func TestKeyNamespaceMutator(t *testing.T) { + empty := KeyNamespaceMutator("") + assert.Equal(t, "", empty("")) + assert.Equal(t, "key", empty("key")) - mcs := NamespacedConfigSource(cs, "config") - for i := 1; i <= 3; i++ { - assert.False(t, mcs.IsSet(fmt.Sprintf("config.key%d", i))) - assert.True(t, mcs.IsSet(fmt.Sprintf("key%d", i))) - } - assert.Equal(t, true, mcs.GetBool("key1")) - assert.Equal(t, time.Minute, mcs.GetDuration("key2")) - assert.Equal(t, "hello", mcs.GetString("key3")) + ns := KeyNamespaceMutator("namespace") + assert.Equal(t, "namespace.", ns("")) + assert.Equal(t, "namespace.key", ns("key")) } func TestMulti(t *testing.T) { diff --git a/internal/apps/config/doctl.go b/internal/apps/config/doctl.go index 2ed8de537..003cd3efe 100644 --- a/internal/apps/config/doctl.go +++ b/internal/apps/config/doctl.go @@ -38,8 +38,7 @@ func DoctlConfigSource(config doctl.Config, ns string) ConfigSource { return nsKey(ns, key) } } - return &mutatingConfigSource{ - cs: &doctlConfigSource{config}, - mutateKey: mutateKey, - } + + // doctl expects the namespace to be present on all calls except IsSet. + return MutatingConfigSource(&doctlConfigSource{config}, mutateKey, []string{"IsSet"}) } diff --git a/internal/apps/workspace/workspace.go b/internal/apps/workspace/workspace.go new file mode 100644 index 000000000..b1ab2697a --- /dev/null +++ b/internal/apps/workspace/workspace.go @@ -0,0 +1,240 @@ +package workspace + +import ( + "bytes" + "context" + "errors" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/digitalocean/doctl/internal/apps/config" +) + +const ( + // DefaultDevConfigFile is the name of the default dev configuration file. + DefaultDevConfigFile = "dev-config.yaml" +) + +// NewAppDev creates a new AppDev workspace. +// +// If devConfigFilePath is empty, it defaults to /.do/. +func NewAppDev(devConfigFilePath string, doctlConfig config.ConfigSource) (*AppDev, error) { + contextDir, err := findContextDir() + if err != nil { + return nil, err + } + + if devConfigFilePath == "" { + configDir := filepath.Join(contextDir, ".do") + err = os.MkdirAll(configDir, os.ModePerm) + if err != nil { + return nil, err + } + devConfigFilePath = filepath.Join(configDir, DefaultDevConfigFile) + if err := ensureStringInFile(devConfigFilePath, ""); err != nil { + return nil, err + } + if err := ensureStringInFile(filepath.Join(configDir, ".gitignore"), DefaultDevConfigFile); err != nil { + return nil, err + } + } + + c, err := config.New(devConfigFilePath) + if err != nil { + return nil, fmt.Errorf("initializing config: %w", err) + } + + return &AppDev{ + contextDir: contextDir, + Config: &AppDevConfig{ + appDevConfig: c, + doctlConfig: doctlConfig, + }, + }, nil +} + +type AppDev struct { + Config *AppDevConfig + contextDir string +} + +func (c *AppDev) CacheDir(component string) string { + return c.Context(".do", "cache", component) +} + +func (c *AppDev) EnsureCacheDir(ctx context.Context, component string) error { + err := os.MkdirAll(c.CacheDir(component), os.ModePerm) + if err != nil { + return err + } + + return ensureStringInFile(c.Context(".do", ".gitignore"), "/cache") +} + +func (c *AppDev) ClearCacheDir(ctx context.Context, component string) error { + return os.RemoveAll(c.CacheDir(component)) +} + +// Context returns a path relative to the workspace context. +// A call with no arguments returns the workspace context path. +func (ws *AppDev) Context(path ...string) string { + return filepath.Join(append([]string{ws.contextDir}, path...)...) +} + +type AppDevConfig struct { + appDevConfig *config.AppDev + doctlConfig config.ConfigSource +} + +// Write writes the current dev-config.yaml to disk. +func (c *AppDevConfig) Write() error { + return c.appDevConfig.WriteConfig() +} + +// Doctl returns doctl's CLI config. +func (c *AppDevConfig) Doctl() config.ConfigSource { + return c.doctlConfig +} + +// Global returns the dev-config.yaml config with an optional CLI override. +func (c *AppDevConfig) Global(cliOverride bool) config.ConfigSource { + var cliConfig config.ConfigSource + if cliOverride { + cliConfig = c.Doctl() + } + + return config.Multi( + cliConfig, + appsDevFlagConfigCompat(c.appDevConfig), + ) +} + +// Set sets a value in dev-config.yaml. +func (c *AppDevConfig) Set(key string, value any) error { + return c.appDevConfig.Set(key, value) +} + +// Component returns per-component config. +// +// componentOnly: in order of priority: +// 1. CLI config (if requested). +// 2. the component's config. +// componentGlobal: in order of priority: +// 1. CLI config (if requested). +// 2. the component's config. +// 3. global config. +func (c *AppDevConfig) Component(component string, cliOverride bool) (componentOnly, componentGlobal config.ConfigSource) { + var cliConfig config.ConfigSource + if cliOverride { + cliConfig = c.Doctl() + } + + componentOnly = config.Multi( + cliConfig, + c.appDevConfig.Components(component), + ) + componentGlobal = config.Multi( + componentOnly, + c.Global(false), // cliOverride is false because it's already accounted for in componentOnly. + ) + return +} + +func ensureStringInFile(file string, val string) error { + if _, err := os.Stat(file); errors.Is(err, os.ErrNotExist) { + f, err := os.OpenFile( + file, + os.O_APPEND|os.O_CREATE|os.O_WRONLY, + 0644, + ) + if err != nil { + return err + } + defer f.Close() + _, err = f.WriteString(val) + return err + } else if err != nil { + return err + } + + b, err := ioutil.ReadFile(file) + if err != nil { + return err + } + + if exists, err := regexp.Match(regexp.QuoteMeta(val), b); err != nil { + return err + } else if !exists { + f, err := os.OpenFile( + file, + os.O_APPEND|os.O_WRONLY, + 0644, + ) + if err != nil { + return err + } + defer f.Close() + + if !bytes.HasSuffix(b, []byte("\n")) { + val = "\n" + val + } + + _, err = f.WriteString(val) + return err + } + + return nil +} + +func dashToUnderscore(key string) string { + return strings.ReplaceAll(key, "-", "_") +} + +// appsDevFlagConfigCompat replaces dashes with underscores in the key to keep compatibility with doctl.Arg* keys +// while keeping the config file keys consistent with the app spec naming convention. +// for example: --no-cache on the CLI will map to no_cache in the config file. +func appsDevFlagConfigCompat(cs config.ConfigSource) config.ConfigSource { + return config.MutatingConfigSource(cs, dashToUnderscore, nil) +} + +func findContextDir() (string, error) { + contextDir, err := os.Getwd() + if err != nil { + return "", err + } + gitRoot, err := findTopLevelGitDir(contextDir) + if err != nil && !errors.Is(err, errNoGitRepo) { + return "", err + } + if gitRoot != "" { + contextDir = gitRoot + } + + return contextDir, nil +} + +var errNoGitRepo = errors.New("no git repository found") + +// findTopLevelGitDir ... +func findTopLevelGitDir(workingDir string) (string, error) { + dir, err := filepath.Abs(workingDir) + if err != nil { + return "", err + } + + for { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + return dir, nil + } + + parent := filepath.Dir(dir) + if parent == dir { + return "", errNoGitRepo + } + dir = parent + } +} diff --git a/internal/apps/workspace/workspace_test.go b/internal/apps/workspace/workspace_test.go new file mode 100644 index 000000000..e486a6f1e --- /dev/null +++ b/internal/apps/workspace/workspace_test.go @@ -0,0 +1,79 @@ +package workspace + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_ensureStringInFile(t *testing.T) { + ensureValue := "newvalue" + + tcs := []struct { + name string + pre func(t *testing.T, fname string) + expect []byte + }{ + { + name: "no pre-existing file", + pre: func(t *testing.T, fname string) {}, + expect: []byte(ensureValue), + }, + { + name: "pre-existing file with value", + pre: func(t *testing.T, fname string) { + f, err := os.OpenFile(fname, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + require.NoError(t, err) + f.WriteString("line1\n" + ensureValue) + f.Close() + }, + expect: []byte("line1\n" + ensureValue), + }, + { + name: "pre-existing file without value", + pre: func(t *testing.T, fname string) { + f, err := os.OpenFile(fname, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + require.NoError(t, err) + f.WriteString("line1\n") + f.Close() + }, + expect: []byte("line1\n" + ensureValue), + }, + { + name: "pre-existing file without value or newline", + pre: func(t *testing.T, fname string) { + f, err := os.OpenFile(fname, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + require.NoError(t, err) + f.WriteString("line1") + f.Close() + }, + expect: []byte("line1\n" + ensureValue), + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + file, err := ioutil.TempFile("", "dev-config.*.yaml") + require.NoError(t, err, "creating temp file") + file.Close() + + // allow the test to dictate existence; we just use this + // to get a valid temporary filename that is unique + err = os.Remove(file.Name()) + require.NoError(t, err, "deleting temp file") + + if tc.pre != nil { + tc.pre(t, file.Name()) + } + + err = ensureStringInFile(file.Name(), ensureValue) + require.NoError(t, err, "ensuring string in file") + + b, err := ioutil.ReadFile(file.Name()) + require.NoError(t, err) + require.Equal(t, string(tc.expect), string(b)) + }) + } +} From 0ebee27e5706b64f3c56c7c0a05dc4be8478e23e Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Fri, 26 Aug 2022 19:45:57 +0300 Subject: [PATCH 5/9] structured config --- args.go | 4 +- commands/apps.go | 55 +------- commands/apps_dev.go | 126 +++++++---------- commands/apps_dev_config_test.go | 59 ++++---- commands/apps_test.go | 62 --------- internal/apps/apps.go | 59 ++++++++ internal/apps/apps_test.go | 166 ++++++++++++++++++++++ internal/apps/workspace/workspace.go | 197 ++++++++++++++++++++++++--- 8 files changed, 483 insertions(+), 245 deletions(-) create mode 100644 internal/apps/apps.go create mode 100644 internal/apps/apps_test.go diff --git a/args.go b/args.go index 24371491e..1c78c201f 100644 --- a/args.go +++ b/args.go @@ -44,8 +44,8 @@ const ( ArgAppDeployment = "deployment" // ArgAppDevConfig is the path to the app dev link config. ArgAppDevConfig = "dev-config" - // ArgAppDevBuildCommand is an optional build command to set for local development. - ArgAppDevBuildCommand = "build-command" + // ArgBuildCommand is an optional build command to set for local development. + ArgBuildCommand = "build-command" // ArgAppLogFollow follow logs. ArgAppLogFollow = "follow" // ArgAppLogTail tail logs. diff --git a/commands/apps.go b/commands/apps.go index 2dd22b230..759074d63 100644 --- a/commands/apps.go +++ b/commands/apps.go @@ -28,6 +28,7 @@ import ( "github.com/digitalocean/doctl" "github.com/digitalocean/doctl/commands/displayers" "github.com/digitalocean/doctl/do" + "github.com/digitalocean/doctl/internal/apps" "github.com/digitalocean/godo" multierror "github.com/hashicorp/go-multierror" "github.com/spf13/cobra" @@ -237,7 +238,7 @@ func RunAppsCreate(c *CmdConfig) error { return err } - appSpec, err := readAppSpec(os.Stdin, specPath) + appSpec, err := apps.ReadAppSpec(os.Stdin, specPath) if err != nil { return err } @@ -334,7 +335,7 @@ func RunAppsUpdate(c *CmdConfig) error { return err } - appSpec, err := readAppSpec(os.Stdin, specPath) + appSpec, err := apps.ReadAppSpec(os.Stdin, specPath) if err != nil { return err } @@ -635,7 +636,7 @@ func RunAppsPropose(c *CmdConfig) error { return err } - appSpec, err := readAppSpec(os.Stdin, specPath) + appSpec, err := apps.ReadAppSpec(os.Stdin, specPath) if err != nil { return err } @@ -653,52 +654,6 @@ func RunAppsPropose(c *CmdConfig) error { return c.Display(displayers.AppProposeResponse{Res: res}) } -func readAppSpec(stdin io.Reader, path string) (*godo.AppSpec, error) { - var spec io.Reader - if path == "-" { - spec = stdin - } else { - specFile, err := os.Open(path) // guardrails-disable-line - if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("opening app spec: %s does not exist", path) - } - return nil, fmt.Errorf("opening app spec: %w", err) - } - defer specFile.Close() - spec = specFile - } - - byt, err := ioutil.ReadAll(spec) - if err != nil { - return nil, fmt.Errorf("reading app spec: %w", err) - } - - s, err := parseAppSpec(byt) - if err != nil { - return nil, fmt.Errorf("parsing app spec: %w", err) - } - - return s, nil -} - -func parseAppSpec(spec []byte) (*godo.AppSpec, error) { - jsonSpec, err := yaml.YAMLToJSON(spec) - if err != nil { - return nil, err - } - - dec := json.NewDecoder(bytes.NewReader(jsonSpec)) - dec.DisallowUnknownFields() - - var appSpec godo.AppSpec - if err := dec.Decode(&appSpec); err != nil { - return nil, err - } - - return &appSpec, nil -} - func appsSpec() *Command { cmd := &Command{ Command: &cobra.Command{ @@ -778,7 +733,7 @@ func RunAppsSpecValidate(c *CmdConfig) error { } specPath := c.Args[0] - appSpec, err := readAppSpec(os.Stdin, specPath) + appSpec, err := apps.ReadAppSpec(os.Stdin, specPath) if err != nil { return err } diff --git a/commands/apps_dev.go b/commands/apps_dev.go index 1c9acfdb2..a1930838a 100644 --- a/commands/apps_dev.go +++ b/commands/apps_dev.go @@ -7,6 +7,7 @@ import ( "io" "io/fs" "os" + "path/filepath" "sync" "github.com/MakeNowJust/heredoc" @@ -22,13 +23,10 @@ import ( "github.com/digitalocean/doctl/internal/apps/workspace" "github.com/digitalocean/godo" - "github.com/joho/godotenv" "github.com/spf13/cobra" ) const ( - // AppsDevDefaultSpecPath is the default spec path for an app. - AppsDevDefaultSpecPath = ".do/app.yaml" // AppsDevDefaultEnvFile is the default env file path. AppsDevDefaultEnvFile = ".env" ) @@ -86,7 +84,7 @@ func AppsDev() *Command { ) AddStringFlag( - build, doctl.ArgAppDevBuildCommand, + build, doctl.ArgBuildCommand, "", "", "Optional build command override for local development.", ) @@ -115,51 +113,22 @@ func RunAppsDevBuild(c *CmdConfig) error { return fmt.Errorf("preparing workspace: %w", err) } - globalConfig := ws.Config.Global(true) - timeout := globalConfig.GetDuration(doctl.ArgTimeout) - if timeout > 0 { + if ws.Config.Timeout > 0 { var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, timeout) + ctx, cancel = context.WithTimeout(ctx, ws.Config.Timeout) defer cancel() } - var ( - spec *godo.AppSpec - cnbVersioning *godo.AppBuildConfigCNBVersioning - ) - appID := globalConfig.GetString(doctl.ArgApp) - // TODO: if this is the user's first time running dev build, ask them if they'd like to // link an existing app. - if appID != "" { - template.Print(`{{success checkmark}} fetching app details{{nl}}`, AppsDevDefaultSpecPath) - app, err := c.Apps().Get(appID) - if err != nil { - return err - } - spec = app.Spec - cnbVersioning = app.GetBuildConfig().GetCNBVersioning() - } - appSpecPath := globalConfig.GetString(doctl.ArgAppSpec) - if spec == nil && appSpecPath == "" && fileExists(AppsDevDefaultSpecPath) { - appSpecPath = AppsDevDefaultSpecPath - template.Print(`{{success checkmark}} using app spec at {{highlight .}}{{nl}}`, AppsDevDefaultSpecPath) - } - if appSpecPath != "" { - spec, err = readAppSpec(os.Stdin, appSpecPath) - if err != nil { - return err - } - } - - if spec == nil { + if ws.Config.AppSpec == nil { // TODO(ntate); allow app-detect build to remove requirement return errors.New("app spec is required for component build") } var hasBuildableComponents bool - _ = godo.ForEachAppSpecComponent(spec, func(c godo.AppBuildableComponentSpec) error { + _ = godo.ForEachAppSpecComponent(ws.Config.AppSpec, func(c godo.AppBuildableComponentSpec) error { hasBuildableComponents = true return fmt.Errorf("stop") }) @@ -167,19 +136,19 @@ func RunAppsDevBuild(c *CmdConfig) error { return fmt.Errorf("the specified app spec does not contain any buildable components") } - var component string + var componentName string if len(c.Args) >= 1 { - component = c.Args[0] + componentName = c.Args[0] } - if component == "" { + if componentName == "" { var components []list.Item - _ = godo.ForEachAppSpecComponent(spec, func(c godo.AppBuildableComponentSpec) error { + _ = godo.ForEachAppSpecComponent(ws.Config.AppSpec, func(c godo.AppBuildableComponentSpec) error { components = append(components, componentListItem{c}) return nil }) if len(components) == 1 { - component = components[0].(componentListItem).spec.GetName() + componentName = components[0].(componentListItem).spec.GetName() } else if len(components) > 1 && Interactive { list := list.New(components) list.Model().Title = "select a component" @@ -194,22 +163,26 @@ func RunAppsDevBuild(c *CmdConfig) error { if !ok { return fmt.Errorf("unexpected item type %T", selectedComponent) } - component = selectedComponent.spec.GetName() + componentName = selectedComponent.spec.GetName() } } - if component == "" { + if componentName == "" { if !Interactive { return errors.New("component name is required when running non-interactively") } - return errors.New("component name is required") + return errors.New("component is required") } - componentSpec, err := godo.GetAppSpecComponent[godo.AppBuildableComponentSpec](spec, component) - if err != nil { - return err + component := ws.Config.Components[componentName] + if component == nil { + // TODO: add support for building without an app spec via app detection + return fmt.Errorf("component %s does not exist in app spec", componentName) + } + componentSpec, ok := component.Spec.(godo.AppBuildableComponentSpec) + if !ok { + return fmt.Errorf("cannot build component %s", componentName) } - if componentSpec.GetSourceDir() != "" { sd := componentSpec.GetSourceDir() stat, err := os.Stat(ws.Context(sd)) @@ -224,10 +197,10 @@ func RunAppsDevBuild(c *CmdConfig) error { } } - componentConfig, componentGlobalConfig := ws.Config.Component(component, true) - var envs map[string]string - envFile := componentGlobalConfig.GetString(doctl.ArgEnvFile) - if envFile == "" && Interactive && fileExists(ws.Context(AppsDevDefaultEnvFile)) { + if component.EnvFile != "" { + template.Print(`{{success checkmark}} using envs from {{highlight .}}{{nl}}`, component.EnvFile) + } else if Interactive && fileExists(ws.Context(AppsDevDefaultEnvFile)) { + // TODO: persist env file path to dev config choice, err := confirm.New( template.String(`{{highlight .}} exists, use it for env var values?`, AppsDevDefaultEnvFile), confirm.WithDefaultChoice(confirm.No), @@ -236,28 +209,21 @@ func RunAppsDevBuild(c *CmdConfig) error { return err } if choice == confirm.Yes { - envFile = ws.Context(AppsDevDefaultEnvFile) - } - } else if envFile != "" { - envFile = ws.Context(envFile) - } - if envFile != "" { - envs, err = godotenv.Read(envFile) - if err != nil { - return fmt.Errorf("reading env file: %w", err) + err := component.LoadEnvFile(ws.Context(AppsDevDefaultEnvFile)) + if err != nil { + return fmt.Errorf("reading env file: %w", err) + } } } - registry := globalConfig.GetString(doctl.ArgRegistry) - noCache := globalConfig.GetBool(doctl.ArgNoCache) - if noCache { + if ws.Config.NoCache { template.Render(text.Warning, `{{crossmark}} build caching disabled{{nl}}`, nil) - err = ws.ClearCacheDir(ctx, component) + err = ws.ClearCacheDir(ctx, componentName) if err != nil { return err } } - err = ws.EnsureCacheDir(ctx, component) + err = ws.EnsureCacheDir(ctx, componentName) if err != nil { return err } @@ -319,15 +285,15 @@ func RunAppsDevBuild(c *CmdConfig) error { err = func() error { defer cancel() - builder, err := c.componentBuilderFactory.NewComponentBuilder(cli, ws.Context(), spec, builder.NewBuilderOpts{ - Component: component, - LocalCacheDir: ws.CacheDir(component), - NoCache: noCache, - Registry: registry, - EnvOverride: envs, - BuildCommandOverride: componentConfig.GetString(doctl.ArgAppDevBuildCommand), + builder, err := c.componentBuilderFactory.NewComponentBuilder(cli, ws.Context(), ws.Config.AppSpec, builder.NewBuilderOpts{ + Component: componentName, + LocalCacheDir: ws.CacheDir(componentName), + NoCache: ws.Config.NoCache, + Registry: ws.Config.Registry, + EnvOverride: component.Envs, + BuildCommandOverride: component.BuildCommand, LogWriter: logWriter, - Versioning: builder.Versioning{CNB: cnbVersioning}, + Versioning: builder.Versioning{CNB: ws.Config.App.GetBuildConfig().GetCNBVersioning()}, }) if err != nil { return err @@ -383,8 +349,8 @@ func RunAppsDevBuild(c *CmdConfig) error { return nil } -func fileExists(path string) bool { - _, err := os.Stat(path) +func fileExists(path ...string) bool { + _, err := os.Stat(filepath.Join(path...)) return err == nil } @@ -395,5 +361,9 @@ func appDevWorkspace(cmdConfig *CmdConfig) (*workspace.AppDev, error) { } doctlConfig := config.DoctlConfigSource(cmdConfig.Doit, cmdConfig.NS) - return workspace.NewAppDev(devConfigFilePath, doctlConfig) + return workspace.NewAppDev(workspace.NewAppDevOpts{ + DevConfigFilePath: devConfigFilePath, + DoctlConfig: doctlConfig, + AppsService: cmdConfig.Apps(), + }) } diff --git a/commands/apps_dev_config_test.go b/commands/apps_dev_config_test.go index 28b1d6a6f..73bc90d3c 100644 --- a/commands/apps_dev_config_test.go +++ b/commands/apps_dev_config_test.go @@ -2,9 +2,10 @@ package commands import ( "errors" - "io/ioutil" "os" + "path/filepath" "testing" + "time" "github.com/digitalocean/doctl" "github.com/digitalocean/doctl/internal/apps/workspace" @@ -12,13 +13,6 @@ import ( ) func TestRunAppsDevConfigSet(t *testing.T) { - file, err := ioutil.TempFile("", "dev-config.*.yaml") - require.NoError(t, err, "creating temp file") - t.Cleanup(func() { - file.Close() - os.Remove(file.Name()) - }) - withTestClient(t, func(cmdConfig *CmdConfig, tm *tcMocks) { tcs := []struct { name string @@ -38,27 +32,27 @@ func TestRunAppsDevConfigSet(t *testing.T) { }, { name: "single key", - args: []string{"app=12345"}, + args: []string{"registry=docker-registry"}, expect: func(t *testing.T, ws *workspace.AppDev) { - require.Equal(t, "12345", ws.Config.Global(false).GetString("app"), "app-id") + require.Equal(t, "docker-registry", ws.Config.Registry, "registry") }, }, { name: "multiple keys", - args: []string{"app=value1", "spec=value2"}, + args: []string{"registry=docker-registry", "timeout=5m"}, expect: func(t *testing.T, ws *workspace.AppDev) { - require.Equal(t, "value1", ws.Config.Global(false).GetString("app"), "app-id") - require.Equal(t, "value2", ws.Config.Global(false).GetString("spec"), "spec") + require.Equal(t, "docker-registry", ws.Config.Registry, "registry") + require.Equal(t, 5*time.Minute, ws.Config.Timeout, "timeout") }, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file.Name()) - - ws, err := appDevWorkspace(cmdConfig) - require.NoError(t, err, "getting workspace") + file := filepath.Join(t.TempDir(), "dev-config.yaml") + _, err := os.Create(file) + require.NoError(t, err) + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file) cmdConfig.Args = tc.args err = RunAppsDevConfigSet(cmdConfig) if tc.expectErr != nil { @@ -67,7 +61,7 @@ func TestRunAppsDevConfigSet(t *testing.T) { } require.NoError(t, err, "running command") - ws, err = appDevWorkspace(cmdConfig) + ws, err := appDevWorkspace(cmdConfig) require.NoError(t, err, "getting workspace") if tc.expect != nil { tc.expect(t, ws) @@ -78,13 +72,6 @@ func TestRunAppsDevConfigSet(t *testing.T) { } func TestRunAppsDevConfigUnset(t *testing.T) { - file, err := ioutil.TempFile("", "dev-config.*.yaml") - require.NoError(t, err, "creating temp file") - t.Cleanup(func() { - file.Close() - os.Remove(file.Name()) - }) - withTestClient(t, func(cmdConfig *CmdConfig, tm *tcMocks) { tcs := []struct { name string @@ -100,35 +87,39 @@ func TestRunAppsDevConfigUnset(t *testing.T) { }, { name: "single key", - args: []string{"app"}, + args: []string{"registry"}, pre: func(t *testing.T, ws *workspace.AppDev) { - ws.Config.Set("app", "value") + ws.Config.Set("registry", "docker-registry") err := ws.Config.Write() require.NoError(t, err, "setting up default values") }, expect: func(t *testing.T, ws *workspace.AppDev) { - require.Equal(t, "", ws.Config.Global(false).GetString("app"), "app-id") + require.Equal(t, "", ws.Config.Registry, "registry") }, }, { name: "multiple keys", - args: []string{"app", "spec"}, + args: []string{"registry", "timeout"}, pre: func(t *testing.T, ws *workspace.AppDev) { - ws.Config.Set("app", "value") - ws.Config.Set("spec", "value") + ws.Config.Set("registry", "docker-registry") + ws.Config.Set("timeout", 5*time.Minute) err := ws.Config.Write() require.NoError(t, err, "setting up default values") }, expect: func(t *testing.T, ws *workspace.AppDev) { - require.Equal(t, "", ws.Config.Global(false).GetString("app"), "app-id") - require.Equal(t, "", ws.Config.Global(false).GetString("spec"), "spec") + require.Equal(t, "", ws.Config.Registry, "registry") + require.Equal(t, time.Duration(0), ws.Config.Timeout, "timeout") }, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file.Name()) + file := filepath.Join(t.TempDir(), "dev-config.yaml") + _, err := os.Create(file) + require.NoError(t, err) + + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file) ws, err := appDevWorkspace(cmdConfig) require.NoError(t, err, "getting workspace") diff --git a/commands/apps_test.go b/commands/apps_test.go index ea55ec002..fb6913b81 100644 --- a/commands/apps_test.go +++ b/commands/apps_test.go @@ -528,68 +528,6 @@ var validAppSpec = &godo.AppSpec{ }, } -func Test_parseAppSpec(t *testing.T) { - expectedSpec := validAppSpec - - t.Run("json", func(t *testing.T) { - spec, err := parseAppSpec([]byte(validJSONSpec)) - require.NoError(t, err) - assert.Equal(t, expectedSpec, spec) - }) - t.Run("yaml", func(t *testing.T) { - spec, err := parseAppSpec([]byte(validYAMLSpec)) - require.NoError(t, err) - assert.Equal(t, expectedSpec, spec) - }) - t.Run("invalid", func(t *testing.T) { - _, err := parseAppSpec([]byte("invalid spec")) - require.Error(t, err) - }) - t.Run("unknown fields", func(t *testing.T) { - _, err := parseAppSpec([]byte(unknownFieldSpec)) - require.Error(t, err) - }) -} - -func Test_readAppSpec(t *testing.T) { - tcs := []struct { - name string - setup func(t *testing.T) (path string, stdin io.Reader) - - wantSpec *godo.AppSpec - wantErr error - }{ - { - name: "stdin", - setup: func(t *testing.T) (string, io.Reader) { - return "-", bytes.NewBufferString(validYAMLSpec) - }, - wantSpec: validAppSpec, - }, - { - name: "file yaml", - setup: func(t *testing.T) (string, io.Reader) { - return testTempFile(t, []byte(validJSONSpec)), nil - }, - wantSpec: validAppSpec, - }, - } - - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - path, stdin := tc.setup(t) - spec, err := readAppSpec(stdin, path) - if tc.wantErr != nil { - require.Equal(t, tc.wantErr, err) - } else { - require.NoError(t, err) - } - - assert.Equal(t, tc.wantSpec, spec) - }) - } -} - func testTempFile(t *testing.T, data []byte) string { t.Helper() file := t.TempDir() + "/file" diff --git a/internal/apps/apps.go b/internal/apps/apps.go new file mode 100644 index 000000000..986a9ddc7 --- /dev/null +++ b/internal/apps/apps.go @@ -0,0 +1,59 @@ +package apps + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "os" + + "github.com/digitalocean/godo" + "sigs.k8s.io/yaml" +) + +func ReadAppSpec(stdin io.Reader, path string) (*godo.AppSpec, error) { + var spec io.Reader + if path == "-" && stdin != nil { + spec = stdin + } else { + specFile, err := os.Open(path) // guardrails-disable-line + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("opening app spec: %s does not exist", path) + } + return nil, fmt.Errorf("opening app spec: %w", err) + } + defer specFile.Close() + spec = specFile + } + + byt, err := ioutil.ReadAll(spec) + if err != nil { + return nil, fmt.Errorf("reading app spec: %w", err) + } + + s, err := ParseAppSpec(byt) + if err != nil { + return nil, fmt.Errorf("parsing app spec: %w", err) + } + + return s, nil +} + +func ParseAppSpec(spec []byte) (*godo.AppSpec, error) { + jsonSpec, err := yaml.YAMLToJSON(spec) + if err != nil { + return nil, err + } + + dec := json.NewDecoder(bytes.NewReader(jsonSpec)) + dec.DisallowUnknownFields() + + var appSpec godo.AppSpec + if err := dec.Decode(&appSpec); err != nil { + return nil, err + } + + return &appSpec, nil +} diff --git a/internal/apps/apps_test.go b/internal/apps/apps_test.go new file mode 100644 index 000000000..6c5e67c1d --- /dev/null +++ b/internal/apps/apps_test.go @@ -0,0 +1,166 @@ +package apps + +import ( + "bytes" + "io" + "io/ioutil" + "testing" + + "github.com/digitalocean/godo" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseAppSpec(t *testing.T) { + expectedSpec := validAppSpec + + t.Run("json", func(t *testing.T) { + spec, err := ParseAppSpec([]byte(validJSONSpec)) + require.NoError(t, err) + assert.Equal(t, expectedSpec, spec) + }) + t.Run("yaml", func(t *testing.T) { + spec, err := ParseAppSpec([]byte(validYAMLSpec)) + require.NoError(t, err) + assert.Equal(t, expectedSpec, spec) + }) + t.Run("invalid", func(t *testing.T) { + _, err := ParseAppSpec([]byte("invalid spec")) + require.Error(t, err) + }) + t.Run("unknown fields", func(t *testing.T) { + _, err := ParseAppSpec([]byte(unknownFieldSpec)) + require.Error(t, err) + }) +} + +func Test_readAppSpec(t *testing.T) { + tcs := []struct { + name string + setup func(t *testing.T) (path string, stdin io.Reader) + + wantSpec *godo.AppSpec + wantErr error + }{ + { + name: "stdin", + setup: func(t *testing.T) (string, io.Reader) { + return "-", bytes.NewBufferString(validYAMLSpec) + }, + wantSpec: validAppSpec, + }, + { + name: "file yaml", + setup: func(t *testing.T) (string, io.Reader) { + return testTempFile(t, []byte(validJSONSpec)), nil + }, + wantSpec: validAppSpec, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + path, stdin := tc.setup(t) + spec, err := ReadAppSpec(stdin, path) + if tc.wantErr != nil { + require.Equal(t, tc.wantErr, err) + } else { + require.NoError(t, err) + } + + assert.Equal(t, tc.wantSpec, spec) + }) + } +} + +const ( + validJSONSpec = `{ + "name": "test", + "services": [ + { + "name": "web", + "github": { + "repo": "digitalocean/sample-golang", + "branch": "main" + } + } + ], + "static_sites": [ + { + "name": "static", + "git": { + "repo_clone_url": "git@github.com:digitalocean/sample-gatsby.git", + "branch": "main" + }, + "routes": [ + { + "path": "/static" + } + ] + } + ] +}` + validYAMLSpec = `name: test +services: +- github: + branch: main + repo: digitalocean/sample-golang + name: web +static_sites: +- git: + branch: main + repo_clone_url: git@github.com:digitalocean/sample-gatsby.git + name: static + routes: + - path: /static +` + unknownFieldSpec = ` +name: test +bugField: bad +services: +- name: web + github: + repo: digitalocean/sample-golang + branch: main +static_sites: +- name: static + git: + repo_clone_url: git@github.com:digitalocean/sample-gatsby.git + branch: main + routes: + - path: /static +` +) + +var validAppSpec = &godo.AppSpec{ + Name: "test", + Services: []*godo.AppServiceSpec{ + { + Name: "web", + GitHub: &godo.GitHubSourceSpec{ + Repo: "digitalocean/sample-golang", + Branch: "main", + }, + }, + }, + StaticSites: []*godo.AppStaticSiteSpec{ + { + Name: "static", + Git: &godo.GitSourceSpec{ + RepoCloneURL: "git@github.com:digitalocean/sample-gatsby.git", + Branch: "main", + }, + Routes: []*godo.AppRouteSpec{ + {Path: "/static"}, + }, + }, + }, +} + +func testTempFile(t *testing.T, data []byte) string { + t.Helper() + file := t.TempDir() + "/file" + err := ioutil.WriteFile(file, data, 0644) + require.NoError(t, err, "writing temp file") + return file +} diff --git a/internal/apps/workspace/workspace.go b/internal/apps/workspace/workspace.go index b1ab2697a..b00443952 100644 --- a/internal/apps/workspace/workspace.go +++ b/internal/apps/workspace/workspace.go @@ -10,32 +10,49 @@ import ( "path/filepath" "regexp" "strings" + "time" + "github.com/digitalocean/doctl" + "github.com/digitalocean/doctl/commands/charm/template" + "github.com/digitalocean/doctl/do" + "github.com/digitalocean/doctl/internal/apps" "github.com/digitalocean/doctl/internal/apps/config" + "github.com/digitalocean/godo" + "github.com/joho/godotenv" ) const ( // DefaultDevConfigFile is the name of the default dev configuration file. DefaultDevConfigFile = "dev-config.yaml" + // DefaultSpecPath is the default spec path for an app. + DefaultSpecPath = ".do/app.yaml" ) +type NewAppDevOpts struct { + // DevConfigFilePath is an optional path to the config file. Defaults to /.do/. + DevConfigFilePath string + // DoctlConfig is the doctl CLI config source. Use config.DoctlConfigSource(...) to create it. + DoctlConfig config.ConfigSource + // AppsService is the apps API service. + AppsService do.AppsService +} + // NewAppDev creates a new AppDev workspace. // -// If devConfigFilePath is empty, it defaults to /.do/. -func NewAppDev(devConfigFilePath string, doctlConfig config.ConfigSource) (*AppDev, error) { +func NewAppDev(opts NewAppDevOpts) (*AppDev, error) { contextDir, err := findContextDir() if err != nil { return nil, err } - if devConfigFilePath == "" { + if opts.DevConfigFilePath == "" { configDir := filepath.Join(contextDir, ".do") err = os.MkdirAll(configDir, os.ModePerm) if err != nil { return nil, err } - devConfigFilePath = filepath.Join(configDir, DefaultDevConfigFile) - if err := ensureStringInFile(devConfigFilePath, ""); err != nil { + opts.DevConfigFilePath = filepath.Join(configDir, DefaultDevConfigFile) + if err := ensureStringInFile(opts.DevConfigFilePath, ""); err != nil { return nil, err } if err := ensureStringInFile(filepath.Join(configDir, ".gitignore"), DefaultDevConfigFile); err != nil { @@ -43,17 +60,19 @@ func NewAppDev(devConfigFilePath string, doctlConfig config.ConfigSource) (*AppD } } - c, err := config.New(devConfigFilePath) + appDevConfig, err := config.New(opts.DevConfigFilePath) + if err != nil { + return nil, fmt.Errorf("initializing config: %w", err) + } + + config, err := NewAppDevConfig(appDevConfig, opts.DoctlConfig, opts.AppsService) if err != nil { return nil, fmt.Errorf("initializing config: %w", err) } return &AppDev{ contextDir: contextDir, - Config: &AppDevConfig{ - appDevConfig: c, - doctlConfig: doctlConfig, - }, + Config: config, }, nil } @@ -81,13 +100,148 @@ func (c *AppDev) ClearCacheDir(ctx context.Context, component string) error { // Context returns a path relative to the workspace context. // A call with no arguments returns the workspace context path. +// If an absolute path is given it is returned as-is. func (ws *AppDev) Context(path ...string) string { - return filepath.Join(append([]string{ws.contextDir}, path...)...) + p := filepath.Join(path...) + if filepath.IsAbs(p) { + return p + } + return filepath.Join(ws.contextDir, p) } type AppDevConfig struct { + // AppSpec is the app spec for the workspace sourced from either AppSpecPath or AppID. + AppSpec *godo.AppSpec + // AppSpecPath is the path to the app spec on disk. + // It can be empty if only a linked production app's spec is used. + AppSpecPath string + + // AppID is an optional production app id to link the workspace to. + AppID string + // App is the production app resource if AppID is set. + App *godo.App + + Registry string + Timeout time.Duration + NoCache bool + + // Components contains component-specific configuration keyed by component name. + Components map[string]*AppDevConfigComponent + appDevConfig *config.AppDev doctlConfig config.ConfigSource + appsService do.AppsService +} + +type AppDevConfigComponent struct { + Spec godo.AppComponentSpec + EnvFile string + Envs map[string]string + BuildCommand string +} + +// NewAppDevConfig populates an AppDevConfig instance with values sourced from *config.AppDev and doctl.Config. +func NewAppDevConfig(appDevConfig *config.AppDev, doctlConfig config.ConfigSource, appsService do.AppsService) (*AppDevConfig, error) { + c := &AppDevConfig{ + appDevConfig: appDevConfig, + doctlConfig: doctlConfig, + appsService: appsService, + } + + err := c.Load() + if err != nil { + return nil, err + } + + err = c.validate() + if err != nil { + return nil, fmt.Errorf("validating config: %w", err) + } + + return c, nil +} + +// Load loads the config. +func (c *AppDevConfig) Load() error { + // ws - workspace config w/ CLI overrides + ws := c.Workspace(true) + + c.Timeout = ws.GetDuration(doctl.ArgTimeout) + c.AppID = ws.GetString(doctl.ArgApp) + c.AppSpecPath = ws.GetString(doctl.ArgAppSpec) + c.Registry = ws.GetString(doctl.ArgRegistry) + c.NoCache = ws.GetBool(doctl.ArgNoCache) + + err := c.loadAppSpec() + if err != nil { + return err + } + + c.Components = make(map[string]*AppDevConfigComponent) + _ = godo.ForEachAppSpecComponent(c.AppSpec, func(spec godo.AppBuildableComponentSpec) error { + name := spec.GetName() + // component - component config w/ CLI overrides + // componentWS - component config w/ workspace and CLI overrides + component, componentWS := c.Component(name, true) + cc := &AppDevConfigComponent{ + Spec: spec, + BuildCommand: component.GetString(doctl.ArgBuildCommand), + } + cc.LoadEnvFile(componentWS.GetString(doctl.ArgEnvFile)) + + c.Components[name] = cc + return nil + }) + + return nil +} + +// LoadEnvFile loads the given file into the component config. +func (c *AppDevConfigComponent) LoadEnvFile(path string) error { + if path == "" { + return nil + } + + envs, err := godotenv.Read(path) + if err != nil { + return fmt.Errorf("reading env file: %w", err) + } + + c.EnvFile = path + c.Envs = envs + return nil +} + +// loadAppSpec loads the app spec from disk or from godo based on the AppID and AppSpecPath configs. +func (c *AppDevConfig) loadAppSpec() error { + var err error + + if c.AppID != "" { + template.Print(`{{success checkmark}} fetching app details{{nl}}`, nil) + c.App, err = c.appsService.Get(c.AppID) + if err != nil { + return fmt.Errorf("fetching app %s: %w", c.AppID, err) + } + template.Print(`{{success checkmark}} loading config from app {{highlight .}}{{nl}}`, c.App.GetSpec().GetName()) + c.AppSpec = c.App.GetSpec() + } else if c.AppSpecPath == "" && fileExists(DefaultSpecPath) { + c.AppSpecPath = DefaultSpecPath + } + + if c.AppSpecPath != "" { + template.Print(`{{success checkmark}} using app spec at {{highlight .}}{{nl}}`, c.AppSpecPath) + c.AppSpec, err = apps.ReadAppSpec(nil, c.AppSpecPath) + if err != nil { + return err + } + } + + return nil +} + +// validate runs validation checks. +func (c *AppDevConfig) validate() error { + return nil } // Write writes the current dev-config.yaml to disk. @@ -95,16 +249,16 @@ func (c *AppDevConfig) Write() error { return c.appDevConfig.WriteConfig() } -// Doctl returns doctl's CLI config. -func (c *AppDevConfig) Doctl() config.ConfigSource { +// doctl returns doctl's CLI config. +func (c *AppDevConfig) doctl() config.ConfigSource { return c.doctlConfig } -// Global returns the dev-config.yaml config with an optional CLI override. -func (c *AppDevConfig) Global(cliOverride bool) config.ConfigSource { +// Workspace returns the dev-config.yaml config with an optional CLI override. +func (c *AppDevConfig) Workspace(cliOverride bool) config.ConfigSource { var cliConfig config.ConfigSource if cliOverride { - cliConfig = c.Doctl() + cliConfig = c.doctl() } return config.Multi( @@ -130,7 +284,7 @@ func (c *AppDevConfig) Set(key string, value any) error { func (c *AppDevConfig) Component(component string, cliOverride bool) (componentOnly, componentGlobal config.ConfigSource) { var cliConfig config.ConfigSource if cliOverride { - cliConfig = c.Doctl() + cliConfig = c.doctl() } componentOnly = config.Multi( @@ -139,7 +293,7 @@ func (c *AppDevConfig) Component(component string, cliOverride bool) (componentO ) componentGlobal = config.Multi( componentOnly, - c.Global(false), // cliOverride is false because it's already accounted for in componentOnly. + c.Workspace(false), // cliOverride is false because it's already accounted for in componentOnly. ) return } @@ -227,7 +381,7 @@ func findTopLevelGitDir(workingDir string) (string, error) { } for { - if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { + if fileExists(dir, ".git"); err == nil { return dir, nil } @@ -238,3 +392,8 @@ func findTopLevelGitDir(workingDir string) (string, error) { dir = parent } } + +func fileExists(path ...string) bool { + _, err := os.Stat(filepath.Join(path...)) + return err == nil +} From 10430a838b56b1f7df5f4fdaec026d4ce238d988 Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Fri, 26 Aug 2022 20:24:34 +0300 Subject: [PATCH 6/9] unexport Config.AppID and config.AppSpecPath --- internal/apps/workspace/workspace.go | 31 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/internal/apps/workspace/workspace.go b/internal/apps/workspace/workspace.go index b00443952..65fc2ae06 100644 --- a/internal/apps/workspace/workspace.go +++ b/internal/apps/workspace/workspace.go @@ -110,14 +110,13 @@ func (ws *AppDev) Context(path ...string) string { } type AppDevConfig struct { - // AppSpec is the app spec for the workspace sourced from either AppSpecPath or AppID. + // appSpecPath is the path to the app spec on disk. + appSpecPath string + // AppSpec is the app spec for the workspace. AppSpec *godo.AppSpec - // AppSpecPath is the path to the app spec on disk. - // It can be empty if only a linked production app's spec is used. - AppSpecPath string - // AppID is an optional production app id to link the workspace to. - AppID string + // appID is an optional production app id to link the workspace to. + appID string // App is the production app resource if AppID is set. App *godo.App @@ -167,8 +166,8 @@ func (c *AppDevConfig) Load() error { ws := c.Workspace(true) c.Timeout = ws.GetDuration(doctl.ArgTimeout) - c.AppID = ws.GetString(doctl.ArgApp) - c.AppSpecPath = ws.GetString(doctl.ArgAppSpec) + c.appID = ws.GetString(doctl.ArgApp) + c.appSpecPath = ws.GetString(doctl.ArgAppSpec) c.Registry = ws.GetString(doctl.ArgRegistry) c.NoCache = ws.GetBool(doctl.ArgNoCache) @@ -216,21 +215,21 @@ func (c *AppDevConfigComponent) LoadEnvFile(path string) error { func (c *AppDevConfig) loadAppSpec() error { var err error - if c.AppID != "" { + if c.appID != "" { template.Print(`{{success checkmark}} fetching app details{{nl}}`, nil) - c.App, err = c.appsService.Get(c.AppID) + c.App, err = c.appsService.Get(c.appID) if err != nil { - return fmt.Errorf("fetching app %s: %w", c.AppID, err) + return fmt.Errorf("fetching app %s: %w", c.appID, err) } template.Print(`{{success checkmark}} loading config from app {{highlight .}}{{nl}}`, c.App.GetSpec().GetName()) c.AppSpec = c.App.GetSpec() - } else if c.AppSpecPath == "" && fileExists(DefaultSpecPath) { - c.AppSpecPath = DefaultSpecPath + } else if c.appSpecPath == "" && fileExists(DefaultSpecPath) { + c.appSpecPath = DefaultSpecPath } - if c.AppSpecPath != "" { - template.Print(`{{success checkmark}} using app spec at {{highlight .}}{{nl}}`, c.AppSpecPath) - c.AppSpec, err = apps.ReadAppSpec(nil, c.AppSpecPath) + if c.appSpecPath != "" { + template.Print(`{{success checkmark}} using app spec at {{highlight .}}{{nl}}`, c.appSpecPath) + c.AppSpec, err = apps.ReadAppSpec(nil, c.appSpecPath) if err != nil { return err } From a89b9e9824d825db2d4261a25dd31f0f0701db62 Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Sat, 27 Aug 2022 01:26:05 +0300 Subject: [PATCH 7/9] unexport internal method --- commands/apps_dev.go | 1 - internal/apps/workspace/workspace.go | 14 +++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/commands/apps_dev.go b/commands/apps_dev.go index a1930838a..b78748d8e 100644 --- a/commands/apps_dev.go +++ b/commands/apps_dev.go @@ -121,7 +121,6 @@ func RunAppsDevBuild(c *CmdConfig) error { // TODO: if this is the user's first time running dev build, ask them if they'd like to // link an existing app. - if ws.Config.AppSpec == nil { // TODO(ntate); allow app-detect build to remove requirement return errors.New("app spec is required for component build") diff --git a/internal/apps/workspace/workspace.go b/internal/apps/workspace/workspace.go index 65fc2ae06..1dd006f57 100644 --- a/internal/apps/workspace/workspace.go +++ b/internal/apps/workspace/workspace.go @@ -163,7 +163,7 @@ func NewAppDevConfig(appDevConfig *config.AppDev, doctlConfig config.ConfigSourc // Load loads the config. func (c *AppDevConfig) Load() error { // ws - workspace config w/ CLI overrides - ws := c.Workspace(true) + ws := c.workspace(true) c.Timeout = ws.GetDuration(doctl.ArgTimeout) c.appID = ws.GetString(doctl.ArgApp) @@ -181,7 +181,7 @@ func (c *AppDevConfig) Load() error { name := spec.GetName() // component - component config w/ CLI overrides // componentWS - component config w/ workspace and CLI overrides - component, componentWS := c.Component(name, true) + component, componentWS := c.component(name, true) cc := &AppDevConfigComponent{ Spec: spec, BuildCommand: component.GetString(doctl.ArgBuildCommand), @@ -253,8 +253,8 @@ func (c *AppDevConfig) doctl() config.ConfigSource { return c.doctlConfig } -// Workspace returns the dev-config.yaml config with an optional CLI override. -func (c *AppDevConfig) Workspace(cliOverride bool) config.ConfigSource { +// workspace returns the dev-config.yaml config with an optional CLI override. +func (c *AppDevConfig) workspace(cliOverride bool) config.ConfigSource { var cliConfig config.ConfigSource if cliOverride { cliConfig = c.doctl() @@ -271,7 +271,7 @@ func (c *AppDevConfig) Set(key string, value any) error { return c.appDevConfig.Set(key, value) } -// Component returns per-component config. +// component returns per-component config. // // componentOnly: in order of priority: // 1. CLI config (if requested). @@ -280,7 +280,7 @@ func (c *AppDevConfig) Set(key string, value any) error { // 1. CLI config (if requested). // 2. the component's config. // 3. global config. -func (c *AppDevConfig) Component(component string, cliOverride bool) (componentOnly, componentGlobal config.ConfigSource) { +func (c *AppDevConfig) component(component string, cliOverride bool) (componentOnly, componentGlobal config.ConfigSource) { var cliConfig config.ConfigSource if cliOverride { cliConfig = c.doctl() @@ -292,7 +292,7 @@ func (c *AppDevConfig) Component(component string, cliOverride bool) (componentO ) componentGlobal = config.Multi( componentOnly, - c.Workspace(false), // cliOverride is false because it's already accounted for in componentOnly. + c.workspace(false), // cliOverride is false because it's already accounted for in componentOnly. ) return } From 2a09550379e66800f351c31497e7f15dc956601c Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Sat, 27 Aug 2022 01:33:14 +0300 Subject: [PATCH 8/9] fix unclosed file handler in tests --- commands/apps_dev_config_test.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/commands/apps_dev_config_test.go b/commands/apps_dev_config_test.go index 73bc90d3c..0dd074f31 100644 --- a/commands/apps_dev_config_test.go +++ b/commands/apps_dev_config_test.go @@ -49,12 +49,10 @@ func TestRunAppsDevConfigSet(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - file := filepath.Join(t.TempDir(), "dev-config.yaml") - _, err := os.Create(file) - require.NoError(t, err) + file := tempFile(t, "dev-config.yaml") cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file) cmdConfig.Args = tc.args - err = RunAppsDevConfigSet(cmdConfig) + err := RunAppsDevConfigSet(cmdConfig) if tc.expectErr != nil { require.EqualError(t, err, tc.expectErr.Error()) return @@ -115,10 +113,7 @@ func TestRunAppsDevConfigUnset(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - file := filepath.Join(t.TempDir(), "dev-config.yaml") - _, err := os.Create(file) - require.NoError(t, err) - + file := tempFile(t, "dev-config.yaml") cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file) ws, err := appDevWorkspace(cmdConfig) @@ -144,3 +139,11 @@ func TestRunAppsDevConfigUnset(t *testing.T) { } }) } + +func tempFile(t *testing.T, name string) (path string) { + file := filepath.Join(t.TempDir(), name) + f, err := os.Create(file) + require.NoError(t, err) + f.Close() + return file +} From eba81dffde413964d61671bf35be2ec66df8e84a Mon Sep 17 00:00:00 2001 From: Kamal Nasser Date: Wed, 7 Sep 2022 00:48:53 +0300 Subject: [PATCH 9/9] test setting and unsetting component-level settings --- commands/apps_dev_config_test.go | 71 +++++++++++++++++++++++++--- internal/apps/workspace/workspace.go | 8 +++- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/commands/apps_dev_config_test.go b/commands/apps_dev_config_test.go index 0dd074f31..6b76a6b08 100644 --- a/commands/apps_dev_config_test.go +++ b/commands/apps_dev_config_test.go @@ -2,6 +2,7 @@ package commands import ( "errors" + "io/ioutil" "os" "path/filepath" "testing" @@ -9,14 +10,18 @@ import ( "github.com/digitalocean/doctl" "github.com/digitalocean/doctl/internal/apps/workspace" + "github.com/digitalocean/godo" "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" ) func TestRunAppsDevConfigSet(t *testing.T) { withTestClient(t, func(cmdConfig *CmdConfig, tm *tcMocks) { tcs := []struct { - name string - args []string + name string + args []string + // appSpec is an optional app spec for the workspace + appSpec *godo.AppSpec expectErr error expect func(*testing.T, *workspace.AppDev) }{ @@ -45,13 +50,34 @@ func TestRunAppsDevConfigSet(t *testing.T) { require.Equal(t, 5*time.Minute, ws.Config.Timeout, "timeout") }, }, + { + name: "component setting", + appSpec: &godo.AppSpec{ + // Note: the service name intentionally includes a dash to ensure that the appsDevFlagConfigCompat + // mutator works as expected -- i.e. only dashes in config options are mutated but not component names. + // `www-svc` remains `www-svc` but `build-command` becomes `build_command`. + Services: []*godo.AppServiceSpec{{Name: "www-svc"}}, + }, + args: []string{"components.www-svc.build_command=npm run start"}, + expect: func(t *testing.T, ws *workspace.AppDev) { + cc := ws.Config.Components["www-svc"] + require.NotNil(t, cc, "component config exists") + require.Equal(t, "npm run start", cc.BuildCommand) + }, + }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - file := tempFile(t, "dev-config.yaml") - cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file) + configFile := tempFile(t, "dev-config.yaml") + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, configFile) cmdConfig.Args = tc.args + + if tc.appSpec != nil { + appSpecFile := tempAppSpec(t, tc.appSpec) + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppSpec, appSpecFile) + } + err := RunAppsDevConfigSet(cmdConfig) if tc.expectErr != nil { require.EqualError(t, err, tc.expectErr.Error()) @@ -72,8 +98,10 @@ func TestRunAppsDevConfigSet(t *testing.T) { func TestRunAppsDevConfigUnset(t *testing.T) { withTestClient(t, func(cmdConfig *CmdConfig, tm *tcMocks) { tcs := []struct { - name string - args []string + name string + args []string + // appSpec is an optional app spec for the workspace + appSpec *godo.AppSpec pre func(*testing.T, *workspace.AppDev) expectErr error expect func(*testing.T, *workspace.AppDev) @@ -109,6 +137,23 @@ func TestRunAppsDevConfigUnset(t *testing.T) { require.Equal(t, time.Duration(0), ws.Config.Timeout, "timeout") }, }, + { + name: "component setting", + args: []string{"components.www-svc.build_command"}, + appSpec: &godo.AppSpec{ + Services: []*godo.AppServiceSpec{{Name: "www-svc"}}, + }, + pre: func(t *testing.T, ws *workspace.AppDev) { + ws.Config.Set("components.www-svc.build_command", "npm run start") + err := ws.Config.Write() + require.NoError(t, err, "setting up default values") + }, + expect: func(t *testing.T, ws *workspace.AppDev) { + cc := ws.Config.Components["www-svc"] + require.NotNil(t, cc, "component config exists") + require.Equal(t, "", cc.BuildCommand) + }, + }, } for _, tc := range tcs { @@ -116,6 +161,11 @@ func TestRunAppsDevConfigUnset(t *testing.T) { file := tempFile(t, "dev-config.yaml") cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppDevConfig, file) + if tc.appSpec != nil { + appSpecFile := tempAppSpec(t, tc.appSpec) + cmdConfig.Doit.Set(cmdConfig.NS, doctl.ArgAppSpec, appSpecFile) + } + ws, err := appDevWorkspace(cmdConfig) require.NoError(t, err, "getting workspace") if tc.pre != nil { @@ -147,3 +197,12 @@ func tempFile(t *testing.T, name string) (path string) { f.Close() return file } + +func tempAppSpec(t *testing.T, spec *godo.AppSpec) (path string) { + path = tempFile(t, "app.yaml") + specYaml, err := yaml.Marshal(spec) + require.NoError(t, err, "marshaling app spec") + err = ioutil.WriteFile(path, specYaml, 0664) + require.NoError(t, err, "writing app spec to disk") + return +} diff --git a/internal/apps/workspace/workspace.go b/internal/apps/workspace/workspace.go index 1dd006f57..5c976ce2b 100644 --- a/internal/apps/workspace/workspace.go +++ b/internal/apps/workspace/workspace.go @@ -161,6 +161,9 @@ func NewAppDevConfig(appDevConfig *config.AppDev, doctlConfig config.ConfigSourc } // Load loads the config. +// +// Note: the .Components config structure is only loaded for components that are present in the app spec. Configuration +// in dev-config.yaml for components that are not present in the app spec will be ignored. func (c *AppDevConfig) Load() error { // ws - workspace config w/ CLI overrides ws := c.workspace(true) @@ -244,6 +247,8 @@ func (c *AppDevConfig) validate() error { } // Write writes the current dev-config.yaml to disk. +// Note that modifying values in the AppDevConfig struct will not affect the contents of the dev-config.yaml file. Instead, +// use the Set(...) method and then call Write(). func (c *AppDevConfig) Write() error { return c.appDevConfig.WriteConfig() } @@ -267,6 +272,7 @@ func (c *AppDevConfig) workspace(cliOverride bool) config.ConfigSource { } // Set sets a value in dev-config.yaml. +// Note that the configuration must be reloaded for the new values to be populated in AppDevConfig. func (c *AppDevConfig) Set(key string, value any) error { return c.appDevConfig.Set(key, value) } @@ -288,7 +294,7 @@ func (c *AppDevConfig) component(component string, cliOverride bool) (componentO componentOnly = config.Multi( cliConfig, - c.appDevConfig.Components(component), + appsDevFlagConfigCompat(c.appDevConfig.Components(component)), ) componentGlobal = config.Multi( componentOnly,