[bugfix, cleanup] Finish moving plugin services to process compose (#1150)

## Summary
Removes `services` stanza from the plugin's json definition, in favor of
using `process-compose.yaml`.

* Fixes bug where, in doing `devbox services ls`, we would only return
the _first_ service defined in a plugin, and would call it the name of
the plugin (instead of the name of the service). This had some
downstream effects as well.
* Removes creation of wrappers of the form `<svc>-start|stop-service`.
They were using the json-based definitions of the services, but `devbox
service start <svc>` was using the process-compose definition, so they
could diverge. Some wrappers were not being generated either, due to the
bug above.
* Deletes obsolete code.
* Updates plugins that define services and bump their versions. Since
plugins are packaged in the CLI, and the process-compose yamls were
already being written before this change, I think this change is
backwards-compatible when it comes to plugins.

Reviewers, please advise on whether this change would affect service
port-forwarding.

## How was it tested?
Ran `devbox services ls|start|stop|up` with different plugins installed
and verified they worked as expected.

## Backwards-compatible?
NO, because we're removing the service wrappers (e.g.
`redis-start-service`). I don't think they're being used, but
@mikeland86 please confirm? If needed, I can replace them with `devbox
services start redis` or something like that.

Author: ipince

docs/app/docs/devbox_examples/languages/php.md

@@ -53,7 +53,7 @@ Use `devbox services start|stop php-fpm` to start PHP-FPM in the background.
 ```bash
 PHPFPM_PORT=8082
 PHPFPM_ERROR_LOG_FILE={PROJECT_DIR}/.devbox/virtenv/php/php-fpm.log
-PHPFPM_PID_FILE={PROJECT_DIR}/.devbox/virtenv/php/php-fpm.log
+PHPFPM_PID_FILE={PROJECT_DIR}/.devbox/virtenv/php/php-fpm.pid
 PHPRC={PROJECT_DIR}/devbox.d/php/php.ini
 ```
 
@@ -62,4 +62,4 @@ PHPRC={PROJECT_DIR}/devbox.d/php/php.ini
 * {PROJECT_DIR}/devbox.d/php81/php-fpm.conf
 * {PROJECT_DIR}/devbox.d/php81/php.ini
 
-You can modify these file to configure PHP or your PHP-FPM server
+You can modify these files to configure PHP or your PHP-FPM server

internal/impl/devbox.go

@@ -501,7 +501,7 @@ func (d *Devbox) Services() (services.Services, error) {
 		return nil, err
 	}
 
-	userSvcs := services.FromProcessComposeYaml(d.projectDir)
+	userSvcs := services.FromUserProcessCompose(d.projectDir)
 
 	svcSet := lo.Assign(pluginSvcs, userSvcs)
 	keys := make([]string, 0, len(svcSet))
@@ -516,7 +516,6 @@ func (d *Devbox) Services() (services.Services, error) {
 	}
 
 	return result, nil
-
 }
 
 func (d *Devbox) StartServices(ctx context.Context, serviceNames ...string) error {

internal/plugin/files.go

@@ -18,11 +18,11 @@ func getConfigIfAny(pkg *nix.Input, projectDir string) (*config, error) {
 		return nil, errors.WithStack(err)
 	}
 
-	// Try to find perfect match first
 	for _, file := range configFiles {
 		if file.IsDir() || strings.HasSuffix(file.Name(), ".go") {
 			continue
 		}
+		// Try to find perfect match first
 		content, err := plugins.BuiltIn.ReadFile(file.Name())
 		if err != nil {
 			return nil, errors.WithStack(err)

internal/plugin/info.go

@@ -63,15 +63,19 @@ func printReadme(cfg *config, w io.Writer, markdown bool) error {
 }
 
 func printServices(cfg *config, w io.Writer, markdown bool) error {
-	if len(cfg.Services) == 0 {
+	svcs, err := cfg.Services()
+	if err != nil {
+		return errors.WithStack(err)
+	}
+	if len(svcs) == 0 {
 		return nil
 	}
 	services := ""
-	for _, service := range cfg.Services {
+	for _, service := range svcs {
 		services += fmt.Sprintf("* %[1]s\n", service.Name)
 	}
 
-	_, err := fmt.Fprintf(
+	_, err = fmt.Fprintf(
 		w,
 		"%sServices:\n%s\nUse `devbox services start|stop [service]` to interact with services\n\n",
 		lo.Ternary(markdown, "### ", ""),

internal/plugin/plugin.go

@@ -43,7 +43,6 @@ type config struct {
 	Packages    []string          `json:"packages"`
 	Env         map[string]string `json:"env"`
 	Readme      string            `json:"readme"`
-	Services    services.Services `json:"services"`
 
 	Shell struct {
 		// InitHook contains commands that will run at shell startup.
@@ -60,6 +59,13 @@ func (c *config) ProcessComposeYaml() (string, bool) {
 	return "", false
 }
 
+func (c *config) Services() (services.Services, error) {
+	if file, ok := c.ProcessComposeYaml(); ok {
+		return services.FromProcessCompose(file)
+	}
+	return nil, nil
+}
+
 func (m *Manager) Include(included string) error {
 	name, err := m.parseInclude(included)
 	if err != nil {

internal/plugin/services.go

@@ -4,17 +4,18 @@
 package plugin
 
 import (
+	"fmt"
+	"os"
+
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/services"
 )
 
-// TODO: this should have PluginManager as receiver so we can build once with
-// pkgs, includes, etc
 func (m *Manager) GetServices(
 	pkgs []*nix.Input,
 	includes []string,
 ) (services.Services, error) {
-	svcs := services.Services{}
+	allSvcs := services.Services{}
 
 	allPkgs := append([]*nix.Input(nil), pkgs...)
 	for _, include := range includes {
@@ -34,15 +35,15 @@ func (m *Manager) GetServices(
 			continue
 		}
 
-		if file, ok := conf.ProcessComposeYaml(); ok {
-			svc := services.Service{
-				Name:               conf.Name,
-				Env:                conf.Env,
-				ProcessComposePath: file,
-			}
-			svcs[conf.Name] = svc
+		svcs, err := conf.Services()
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "error reading services in plugin \"%s\", skipping", conf.Name)
+			continue
+		}
+		for name, svc := range svcs {
+			allSvcs[name] = svc
 		}
-
 	}
-	return svcs, nil
+
+	return allSvcs, nil
 }

internal/services/config.go

@@ -14,21 +14,21 @@ import (
 	"go.jetpack.io/devbox/internal/cuecfg"
 )
 
-func FromProcessComposeYaml(projectDir string) Services {
-	// TODO need to handle if a filepath is passed in
+func FromUserProcessCompose(projectDir string) Services {
 	processComposeYaml := lookupProcessCompose(projectDir, "")
 	if processComposeYaml == "" {
 		return nil
 	}
-	userSvcs, err := readProcessCompose(processComposeYaml)
+
+	userSvcs, err := FromProcessCompose(processComposeYaml)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "error reading process-compose.yaml: %s, skipping", err)
 		return nil
 	}
 	return userSvcs
 }
 
-func readProcessCompose(path string) (Services, error) {
+func FromProcessCompose(path string) (Services, error) {
 	processCompose := &types.Project{}
 	services := Services{}
 	err := errors.WithStack(cuecfg.ParseFile(path, processCompose))

internal/services/manager.go

@@ -167,9 +167,7 @@ func StartProcessManager(
 	}
 
 	for _, s := range availableServices {
-		if file, hasComposeYaml := s.ProcessComposeYaml(); hasComposeYaml {
-			flags = append(flags, "-f", file)
-		}
+		flags = append(flags, "-f", s.ProcessComposePath)
 	}
 
 	if processComposeBackground {

internal/services/services.go

@@ -1,105 +1,11 @@
 // Copyright 2023 Jetpack Technologies Inc and contributors. All rights reserved.
 // Use of this source code is governed by the license in the LICENSE file.
 
-//lint:file-ignore U1000 Ignore unused function temporarily for debugging
 package services
 
-import (
-	"encoding/json"
-	"fmt"
-	"io"
-	"os"
-
-	"github.com/a8m/envsubst"
-	"github.com/fatih/color"
-	"github.com/pkg/errors"
-
-	"go.jetpack.io/devbox/internal/envir"
-)
-
-type Services map[string]Service
+type Services map[string]Service // name -> Service
 
 type Service struct {
-	Name               string            `json:"name"`
-	Env                map[string]string `json:"-"`
-	RawPort            string            `json:"port"`
-	Start              string            `json:"start"`
-	Stop               string            `json:"stop"`
+	Name               string
 	ProcessComposePath string
 }
-
-// TODO: (john) Since moving to process-compose, our services no longer use the old `toggleServices` function. We'll need to clean a lot of this up in a later PR.
-
-type serviceAction int
-
-const (
-	startService serviceAction = iota
-	stopService
-)
-
-func printProxyURL(w io.Writer, services Services) error { // TODO: remove it?
-	if !envir.IsDevboxCloud() {
-		return nil
-	}
-
-	hostname, err := os.Hostname()
-	if err != nil {
-		return errors.WithStack(err)
-	}
-
-	printGeneric := false
-	for _, service := range services {
-		if port, _ := service.Port(); port != "" {
-			color.New(color.FgHiGreen).Fprintf(
-				w,
-				"To access %s on this vm use: %s-%s.svc.devbox.sh\n",
-				service.Name,
-				hostname,
-				port,
-			)
-		} else {
-			printGeneric = true
-		}
-	}
-
-	if printGeneric {
-		color.New(color.FgHiGreen).Fprintf(
-			w,
-			"To access other services on this vm use: %s-<port>.svc.devbox.sh\n",
-			hostname,
-		)
-	}
-	return nil
-}
-
-func (s *Service) Port() (string, error) {
-	if s.RawPort == "" {
-		return "", nil
-	}
-	return envsubst.String(s.RawPort)
-}
-
-func (s *Service) ProcessComposeYaml() (string, bool) {
-	return s.ProcessComposePath, true
-}
-
-func (s *Service) StartName() string {
-	return fmt.Sprintf("%s-service-start", s.Name)
-}
-
-func (s *Service) StopName() string {
-	return fmt.Sprintf("%s-service-stop", s.Name)
-}
-
-func (s *Services) UnmarshalJSON(b []byte) error {
-	var m map[string]Service
-	if err := json.Unmarshal(b, &m); err != nil {
-		return err
-	}
-	*s = make(Services)
-	for name, svc := range m {
-		svc.Name = name
-		(*s)[name] = svc
-	}
-	return nil
-}

internal/wrapnix/wrapper.go

@@ -16,15 +16,13 @@ import (
 	"go.jetpack.io/devbox/internal/cmdutil"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/plugin"
-	"go.jetpack.io/devbox/internal/services"
 )
 
 type devboxer interface {
 	NixBins(ctx context.Context) ([]string, error)
 	ShellEnvHash(ctx context.Context) (string, error)
 	ShellEnvHashKey() string
 	ProjectDir() string
-	Services() (services.Services, error)
 }
 
 //go:embed wrapper.sh.tmpl
@@ -38,11 +36,6 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 		return err
 	}
 
-	services, err := devbox.Services()
-	if err != nil {
-		return err
-	}
-
 	// Remove all old wrappers
 	_ = os.RemoveAll(filepath.Join(devbox.ProjectDir(), plugin.WrapperPath))
 
@@ -51,28 +44,6 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 	_ = os.MkdirAll(destPath, 0755)
 
 	bashPath := cmdutil.GetPathOrDefault("bash", "/bin/bash")
-	for _, service := range services {
-		if err = createWrapper(&createWrapperArgs{
-			devboxer:     devbox,
-			BashPath:     bashPath,
-			Command:      service.Start,
-			Env:          service.Env,
-			ShellEnvHash: shellEnvHash,
-			destPath:     filepath.Join(destPath, service.StartName()),
-		}); err != nil {
-			return err
-		}
-		if err = createWrapper(&createWrapperArgs{
-			devboxer:     devbox,
-			BashPath:     bashPath,
-			Command:      service.Stop,
-			Env:          service.Env,
-			ShellEnvHash: shellEnvHash,
-			destPath:     filepath.Join(destPath, service.StopName()),
-		}); err != nil {
-			return err
-		}
-	}
 
 	bins, err := devbox.NixBins(ctx)
 	if err != nil {