[plugins] Remove match and packages from schema (#1296)

mikeland73 · web-flow · commit 2f0e8ebe231f · 2023-07-21T15:37:36.000-07:00
## Summary After chatting with @Lagoja we decided to remove `packages` and `match` from plugin schema before we release custom plugins. The reason being: * `packages` is poorly defined and not a great experience. It was added so we could build plugins that install custom flakes (for php, haskell, etc) and replace planners. It was renamed to `__packages` and will not be externally documented. In very near future we plan on adding `suggested_packages` (final name TBD) which informs the user of any packages needed for a plugin and allows the user to install if needed. * `match` is designed to automatically add built-in plugins if certain packages are installed. This functionality doesn't make sense for custom plugins. This field was moved into a hardcoded map. Another benefit of removing this from schema is that we can put the plugins into the plugin github repo. cc: @Lagoja ## How was it tested? * unit tests * manual smoke testing (installed php)
diff --git a/docs/app/docs/guides/creating_plugins.md b/docs/app/docs/guides/creating_plugins.md
@@ -28,7 +28,6 @@ Plugins are defined as Go JSON Template files, using the following schema:
 {
   "name": "",
   "version": "",
-  "match": "",
   "readme": "",
   "env": {
     "<key>": "<value>"
diff --git a/examples/plugins/builtin/devbox.d/php8/php-fpm.conf b/examples/plugins/builtin/devbox.d/php8/php-fpm.conf
@@ -0,0 +1,17 @@
+[global]
+pid = ${PHPFPM_PID_FILE}
+error_log = ${PHPFPM_ERROR_LOG_FILE}
+daemonize = yes
+
+[www]
+; user = www-data
+; group = www-data
+listen = 127.0.0.1:${PHPFPM_PORT}
+; listen.owner = www-data
+; listen.group = www-data
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 2
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+chdir = /
diff --git a/examples/plugins/builtin/devbox.d/php8/php.ini b/examples/plugins/builtin/devbox.d/php8/php.ini
@@ -0,0 +1,6 @@
+[php]
+
+; Put your php.ini directives here. For the latest default php.ini file, see https://github.com/php/php-src/blob/master/php.ini-production
+
+; memory_limit = 128M
+; expose_php = Off
diff --git a/examples/plugins/builtin/devbox.json b/examples/plugins/builtin/devbox.json
@@ -0,0 +1,16 @@
+{
+  "packages": [],
+  "shell": {
+    "init_hook": [
+      "echo 'Welcome to devbox!' > /dev/null"
+    ],
+    "scripts": {
+      "test": [
+        "test -n \"$PHPRC\" || exit 1"
+      ]
+    }
+  },
+  "include": [
+    "plugin:php8"
+  ]
+}
diff --git a/examples/plugins/builtin/devbox.lock b/examples/plugins/builtin/devbox.lock
@@ -0,0 +1,4 @@
+{
+  "lockfile_version": "1",
+  "packages": {}
+}
diff --git a/examples/stacks/lapp-stack/devbox.lock b/examples/stacks/lapp-stack/devbox.lock
@@ -3,7 +3,7 @@
   "packages": {
     "apache@2.4": {
       "last_modified": "2023-05-01T16:53:22Z",
-      "plugin_version": "0.0.1",
+      "plugin_version": "0.0.2",
       "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#apacheHttpd",
       "version": "2.4.57"
     },
@@ -19,15 +19,15 @@
     },
     "php@8.1": {
       "last_modified": "2023-05-01T16:53:22Z",
-      "plugin_version": "0.0.1",
+      "plugin_version": "0.0.2",
       "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#php",
       "version": "8.1.18"
     },
     "postgresql@14": {
       "last_modified": "2023-05-01T16:53:22Z",
-      "plugin_version": "0.0.1",
+      "plugin_version": "0.0.2",
       "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#postgresql",
       "version": "14.7"
     }
   }
-}
+}
diff --git a/internal/cloud/openssh/config_test.go b/internal/cloud/openssh/config_test.go
@@ -60,7 +60,6 @@ func TestHostOrMatchRegex(t *testing.T) {
 		"\tMatch all":        true,
 
 		"Host":               false,
-		"Match":              false,
 		"Hostname devbox.sh": false,
 		`# Host *.devbox.sh`: true,
 		`# Match all`:        true,
diff --git a/internal/plugin/files.go b/internal/plugin/files.go
@@ -4,9 +4,8 @@
 package plugin
 
 import (
+	"io/fs"
 	"os"
-	"regexp"
-	"strings"
 
 	"github.com/pkg/errors"
 	"go.jetpack.io/devbox/internal/devpkg"
@@ -16,7 +15,7 @@ import (
 func getConfigIfAny(pkg Includable, projectDir string) (*config, error) {
 	switch pkg := pkg.(type) {
 	case *devpkg.Package:
-		return getBuiltinPluginConfig(pkg, projectDir)
+		return getBuiltinPluginConfigIfExists(pkg, projectDir)
 	case *githubPlugin:
 		return pkg.buildConfig(projectDir)
 	case *localPlugin:
@@ -29,34 +28,16 @@ func getConfigIfAny(pkg Includable, projectDir string) (*config, error) {
 	return nil, errors.Errorf("unknown plugin type %T", pkg)
 }
 
-func getBuiltinPluginConfig(pkg Includable, projectDir string) (*config, error) {
-	builtins, err := plugins.Builtins()
+func getBuiltinPluginConfigIfExists(
+	pkg Includable,
+	projectDir string,
+) (*config, error) {
+	content, err := plugins.BuiltInForPackage(pkg.CanonicalName())
+	if errors.Is(err, fs.ErrNotExist) {
+		return nil, nil
+	}
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
-
-	for _, file := range builtins {
-		// We deserialize first so we can check the Match field. If it's there
-		// we use it, otherwise we use the name.
-		// TODO(landau): this is weird, hard to understand code. Fetching the file
-		// content of configs should probably not use FileContent()
-		content, err := pkg.FileContent(file.Name())
-		if err != nil {
-			return nil, errors.WithStack(err)
-		}
-
-		name := pkg.CanonicalName()
-		cfg, err := buildConfig(pkg, projectDir, string(content))
-		if err != nil {
-			return nil, errors.WithStack(err)
-		}
-		// if match regex is set we use it to check. Otherwise we assume it's a
-		// perfect match
-		if (cfg.Match != "" && !regexp.MustCompile(cfg.Match).MatchString(name)) ||
-			(cfg.Match == "" && strings.Split(file.Name(), ".")[0] != name) {
-			continue
-		}
-		return cfg, nil
-	}
-	return nil, nil
+	return buildConfig(pkg, projectDir, string(content))
 }
diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go
@@ -39,9 +39,8 @@ var (
 type config struct {
 	Name        string            `json:"name"`
 	Version     string            `json:"version"`
-	Match       string            `json:"match"`
 	CreateFiles map[string]string `json:"create_files"`
-	Packages    []string          `json:"packages"`
+	Packages    []string          `json:"__packages"`
 	Env         map[string]string `json:"env"`
 	Readme      string            `json:"readme"`
 
diff --git a/plugins/README.md b/plugins/README.md
@@ -25,7 +25,6 @@ Plugins are defined as Go JSON Template files, using the following schema:
 {
   "name": "",
   "version": "",
-  "match": "",
   "readme": "",
   "env": {
     "<key>": "<value>"
diff --git a/plugins/apacheHttpd.json b/plugins/apacheHttpd.json
@@ -1,7 +1,6 @@
 {
   "name": "apache",
   "version": "0.0.2",
-  "match": "^(apache|apacheHttpd)$",
   "readme": "If you with to edit the config file, please copy it out of the .devbox directory.",
   "env": {
     "HTTPD_DEVBOX_CONFIG_DIR": "{{ .DevboxProjectDir }}",
diff --git a/plugins/builtins.go b/plugins/builtins.go
@@ -0,0 +1,53 @@
+package plugins
+
+import (
+	"embed"
+	"io/fs"
+	"regexp"
+	"strings"
+
+	"github.com/samber/lo"
+)
+
+//go:embed *.json */*
+var builtIn embed.FS
+
+func Builtins() ([]fs.DirEntry, error) {
+	entries, err := builtIn.ReadDir(".")
+	if err != nil {
+		return nil, err
+	}
+	return lo.Filter(entries, func(e fs.DirEntry, _ int) bool {
+		return !e.IsDir() && !strings.HasSuffix(e.Name(), ".go")
+	}), nil
+}
+
+type BuiltIn struct {
+}
+
+var builtInMap = map[*regexp.Regexp]string{
+	regexp.MustCompile(`^(apache|apacheHttpd)$`):                       "apacheHttpd",
+	regexp.MustCompile(`^(gradle|gradle_[0-9])$`):                      "gradle",
+	regexp.MustCompile(`^(ghc|haskell\.compiler\.(.*))$`):              "haskell",
+	regexp.MustCompile(`^mariadb(-embedded)?_?[0-9]*$`):                "mariadb",
+	regexp.MustCompile(`^mysql?[0-9]*$`):                               "mysql",
+	regexp.MustCompile(`^php[0-9]*$`):                                  "php",
+	regexp.MustCompile(`^python3[0-9]*Packages.pip$`):                  "pip",
+	regexp.MustCompile(`^postgresql(_[0-9]+)?$`):                       "postgresql",
+	regexp.MustCompile(`^python[0-9]*(Full|Minimal|-full|-minimal)?$`): "python",
+	regexp.MustCompile(`^redis$`):                                      "redis",
+	regexp.MustCompile(`^j?ruby([0-9_]*[0-9]+)?$`):                     "ruby",
+}
+
+func BuiltInForPackage(pkgName string) ([]byte, error) {
+	for re, name := range builtInMap {
+		if re.MatchString(pkgName) {
+			return builtIn.ReadFile(name + ".json")
+		}
+	}
+	return builtIn.ReadFile(pkgName + ".json")
+}
+
+func (f *BuiltIn) FileContent(contentPath string) ([]byte, error) {
+	return builtIn.ReadFile(contentPath)
+}
diff --git a/plugins/builtins_test.go b/plugins/builtins_test.go
@@ -0,0 +1,59 @@
+package plugins
+
+import (
+	"testing"
+)
+
+func TestBuiltInMap(t *testing.T) {
+	testCases := map[string]string{
+		"apache":                                 "apacheHttpd",
+		"apacheHttpd":                            "apacheHttpd",
+		"php":                                    "php",
+		"php81":                                  "php",
+		"php82":                                  "php",
+		"gradle":                                 "gradle",
+		"gradle_7":                               "gradle",
+		"ghc":                                    "haskell",
+		"haskell.compiler.abc":                   "haskell",
+		"haskell.compiler.native-bignum.ghcHEAD": "haskell",
+		"haskell.compiler.native-bignum.ghc962":  "haskell",
+		"mariadb":                                "mariadb",
+		"mariadb_1011":                           "mariadb",
+		"mariadb-embedded":                       "mariadb",
+		"mysql":                                  "mysql",
+		"mysql80":                                "mysql",
+		"python3Packages.pip":                    "pip",
+		"python":                                 "python",
+		"python3":                                "python",
+		"python3Full":                            "python",
+		"python2Minimal":                         "python",
+		"python-full":                            "python",
+		"python-minimal":                         "python",
+		"redis":                                  "redis",
+		"ruby":                                   "ruby",
+		"ruby_21":                                "ruby",
+		"ruby_2_6":                               "ruby",
+		"ruby_2_6_5":                             "ruby",
+		"ruby_2_6_5_1":                           "ruby",
+		"ruby_2_6_5_1_2":                         "ruby",
+		"jruby":                                  "ruby",
+		"ruby_":                                  "",
+		"ruby_abc":                               "",
+		"ruby_2_":                                "",
+	}
+
+	for input, expected := range testCases {
+		matched := false
+		for re, value := range builtInMap {
+			if re.MatchString(input) {
+				matched = true
+				if value != expected {
+					t.Errorf("Regex match failed for input: %s. Expected: %s, Got: %s", input, expected, value)
+				}
+			}
+		}
+		if !matched && expected != "" {
+			t.Errorf("Regex match failed for input: %s. Expected: %s, Got: %s", input, expected, "")
+		}
+	}
+}
diff --git a/plugins/gradle.json b/plugins/gradle.json
@@ -1,11 +1,10 @@
 {
     "name": "gradle",
     "version": "0.0.1",
-    "match": "^(gradle|gradle_[0-9])$",
     "readme": "You can customize which JDK gradle will use by specifying the value of `org.gradle.java.home` in gradle.properties file",
     "shell": {
         "init_hook": [
             "[ -s gradle.properties ] || echo org.gradle.java.home=$JAVA_HOME >> gradle.properties"
         ]
     }
-}
+}
diff --git a/plugins/haskell.json b/plugins/haskell.json
@@ -1,9 +1,8 @@
 {
   "name": "haskell",
   "version": "0.0.1",
-  "match": "^(ghc|haskell\\.compiler\\.(.*))$",
   "readme": "Haskell plugin",
-  "packages": [
+  "__packages": [
     "path:{{ .Virtenv }}"
   ],
   "create_files": {
diff --git a/plugins/mariadb.json b/plugins/mariadb.json
@@ -1,7 +1,6 @@
 {
   "name": "mariadb",
   "version": "0.0.2",
-  "match": "^mariadb_?[0-9]*$",
   "readme": "* This plugin wraps mysqld and mysql_install_db to work in your local project\n* This plugin will create a new database for your project in MYSQL_DATADIR if one doesn't exist on shell init\n* Use mysqld to manually start the server, and `mysqladmin -u root shutdown` to manually stop it",
   "env": {
     "MYSQL_BASEDIR": "{{ .DevboxProfileDefault }}",
@@ -16,12 +15,12 @@
     "{{ .Virtenv }}/setup_db.sh": "mariadb/setup_db.sh",
     "{{ .Virtenv }}/process-compose.yaml": "mariadb/process-compose.yaml"
   },
-  "packages": [
+  "__packages": [
     "path:{{ .Virtenv }}"
   ],
   "shell": {
     "init_hook": [
       "bash {{ .Virtenv }}/setup_db.sh"
     ]
   }
-}
+}
diff --git a/plugins/mysql.json b/plugins/mysql.json
@@ -1,7 +1,6 @@
 {
     "name": "mysql",
     "version": "0.0.1",
-    "match": "^mysql?[0-9]*$",
     "readme": "* This plugin wraps mysqld and mysql_install_db to work in your local project\n* This plugin will create a new database for your project in MYSQL_DATADIR if one doesn't exist on shell init. This DB will be started in `insecure` mode, so be sure to add a root password after creation if needed.\n* Use mysqld to manually start the server, and `mysqladmin -u root shutdown` to manually stop it",
     "env": {
       "MYSQL_BASEDIR": "{{ .DevboxProfileDefault }}",
@@ -16,7 +15,7 @@
       "{{ .Virtenv }}/setup_db.sh": "mysql/setup_db.sh",
       "{{ .Virtenv }}/process-compose.yaml": "mysql/process-compose.yaml"
     },
-    "packages": [
+    "__packages": [
       "path:{{ .Virtenv }}"
     ],
     "shell": {
diff --git a/plugins/php.json b/plugins/php.json
@@ -1,9 +1,8 @@
 {
   "name": "php",
   "version": "0.0.2",
-  "match": "^php[0-9]*$",
   "readme": "PHP is compiled with default extensions. If you would like to use non-default extensions you can add them with devbox add php81Extensions.{extension} . For example, for the memcache extension you can do `devbox add php81Extensions.memcached`.",
-  "packages": [
+  "__packages": [
     "path:{{ .Virtenv }}",
     "path:{{ .Virtenv }}#composer"
   ],
diff --git a/plugins/pip.json b/plugins/pip.json
@@ -1,7 +1,6 @@
 {
     "name": "pip",
     "version": "0.0.1",
-    "match": "^python3[0-9]*Packages.pip$",
     "readme": "This plugin adds a script for automatically creating a virtual environment using `venv` for python3 projects, so you can install packages with pip as normal.\nTo activate the environment, run `source $VENV_DIR/bin/activate` or add it to the init_hook of your devbox.json\nTo change where your virtual environment is created, modify the $VENV_DIR environment variable in your init_hook",
     "env": {
         "VENV_DIR": "{{ .Virtenv }}/.venv"
diff --git a/plugins/plugins.go b/plugins/plugins.go
diff --git a/plugins/postgresql.json b/plugins/postgresql.json
diff --git a/plugins/python.json b/plugins/python.json
diff --git a/plugins/redis.json b/plugins/redis.json
diff --git a/plugins/ruby.json b/plugins/ruby.json

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,6 @@ Plugins are defined as Go JSON Template files, using the following schema:`
`28`	`28`	`{`
`29`	`29`	`"name": "",`
`30`	`30`	`"version": "",`
`31`		`- "match": "",`
`32`	`31`	`"readme": "",`
`33`	`32`	`"env": {`
`34`	`33`	`"<key>": "<value>"`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +  "lockfile_version": "1",
 +  "packages": {}
 +}
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,6 @@ Plugins are defined as Go JSON Template files, using the following schema:`
`25`	`25`	`{`
`26`	`26`	`"name": "",`
`27`	`27`	`"version": "",`
`28`		`- "match": "",`
`29`	`28`	`"readme": "",`
`30`	`29`	`"env": {`
`31`	`30`	`"<key>": "<value>"`