From 83a343c67dc4ecde80f40a88816eeb6a2616e540 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Tue, 24 Oct 2023 10:26:30 +0200 Subject: [PATCH] Remove hooks scripts They were not used, and removing them can make the code a bit cleaner. --- cmd/serve.go | 3 - cozy.example.yaml | 3 - docs/config.md | 60 -------------------- model/app/installer.go | 37 +++++-------- model/instance/lifecycle/create.go | 17 ------ model/instance/lifecycle/destroy.go | 9 --- pkg/config/config/config.go | 2 - pkg/hooks/hooks.go | 54 ------------------ pkg/hooks/hooks_test.go | 80 --------------------------- pkg/hooks/testdata/post-success | 3 - pkg/hooks/testdata/pre-failure | 2 - pkg/hooks/testdata/pre-success | 3 - scripts/hooks/post-install-app.sample | 17 ------ scripts/hooks/pre-install-app.sample | 15 ----- 14 files changed, 15 insertions(+), 290 deletions(-) delete mode 100644 pkg/hooks/hooks.go delete mode 100644 pkg/hooks/hooks_test.go delete mode 100755 pkg/hooks/testdata/post-success delete mode 100755 pkg/hooks/testdata/pre-failure delete mode 100755 pkg/hooks/testdata/pre-success delete mode 100755 scripts/hooks/post-install-app.sample delete mode 100755 scripts/hooks/pre-install-app.sample diff --git a/cmd/serve.go b/cmd/serve.go index f07ae35a2c7..6bfb672dc32 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -193,9 +193,6 @@ func init() { flags.StringSlice("flagship-apple-app-ids", []string{"3AKXFMV43J.io.cozy.drive.mobile", "3AKXFMV43J.io.cozy.flagship.mobile"}, "App ID of the flagship app on iOS") checkNoErr(viper.BindPFlag("flagship.apple_app_ids", flags.Lookup("flagship-apple-app-ids"))) - flags.String("hooks", ".", "define the directory used for hook scripts") - checkNoErr(viper.BindPFlag("hooks", flags.Lookup("hooks"))) - flags.String("geodb", ".", "define the location of the database for IP -> City lookups") checkNoErr(viper.BindPFlag("geodb", flags.Lookup("geodb"))) diff --git a/cozy.example.yaml b/cozy.example.yaml index 61911e89498..525ef483d81 100644 --- a/cozy.example.yaml +++ b/cozy.example.yaml @@ -224,9 +224,6 @@ mail: username: {{.Env.COZY_BETA_MAIL_USERNAME}} password: {{.Env.COZY_BETA_MAIL_PASSWORD}} -# directory with the hooks scripts - flags: --hooks -hooks: ./scripts/hooks - # location of the database for IP -> City lookups - flags: --geodb # See https://dev.maxmind.com/geoip/geoip2/geolite2/ geodb: "" diff --git a/docs/config.md b/docs/config.md index 3817a4adadd..edca730d382 100644 --- a/docs/config.md +++ b/docs/config.md @@ -147,66 +147,6 @@ couchdb: 2. each instance document will keep the list index of the CouchDB cluster used for its databases, so don't remove a cluster in the middle of the list! -## Hooks - -Cozy-stack can run scripts on some events to customize it. The scripts must be -in the hooks directory defined in the config, have a predefined name, and be -executable. Then, they should be fired automatically. Let's the available hooks. - -The `pre-add-instance` hook is run just before creating an instance. It can -prevent the command from running by exiting with non-zero status. It can be used -to check the domain for example. It is called with the following parameter: - -1. the domain of the instance that will be created. - -The `post-add-instance` hook is run just after an instance has been created. -installed. It can be used to setup DNS for this instance for example. It is -called with the following parameter: - -1. the domain of the instance that has been created. - -The `pre-remove-instance` hook is run just before destroying an instance. It can -prevent the command from running by exiting with non-zero status. It can be used -to make a backup of the instance before destroying it. It is called with the -following parameter: - -1. the domain of the instance that will be destroyed. - -The `post-remove-instance` hook is run just after an instance has been -destroyed. It can be used to do some cleanup. It is called with the following -parameter: - -1. the domain of the instance that has been destroyed. - -The `pre-install-app` hook is run just before installing an application, and can -prevent the command from running by exiting with non-zero status. It is called -with the following parameters: - -1. the instance on which the application will be installed -2. the application name that will be installed. - -The `post-install-app` hook is run just after an application has been installed. -It can be used for logging, notification, statistics, etc. It's also a good -place to add a vhost for an application in the reverse-proxy configuration, with -a TLS certificate. It is called with the following parameters: - -1. the instance on which the application has been installed -2. the application name that has been installed. - -The `pre-uninstall-app` hook is run just before uninstalling an application, and -can prevent the command from running by exiting with non-zero status. It is -called with the following parameters: - -1. the instance on which the application will be uninstalled -2. the application name that will be uninstalled. - -The `post-uninstall-app` hook is run just after an application has been -uninstalled. It can be used for cleaning the configuration of the reverse- proxy -for example. It is called with the following parameters: - -1. the instance on which the application has been uninstalled -2. the application name that has been uninstalled. - ## OnlyOffice An integration between Cozy and OnlyOffice has been made. It allows the diff --git a/model/app/installer.go b/model/app/installer.go index 4ca886b10ed..54b308dd75a 100644 --- a/model/app/installer.go +++ b/model/app/installer.go @@ -18,7 +18,6 @@ import ( "github.com/cozy/cozy-stack/pkg/config/config" "github.com/cozy/cozy-stack/pkg/consts" "github.com/cozy/cozy-stack/pkg/couchdb" - "github.com/cozy/cozy-stack/pkg/hooks" "github.com/cozy/cozy-stack/pkg/logger" "github.com/cozy/cozy-stack/pkg/prefixer" "github.com/cozy/cozy-stack/pkg/realtime" @@ -309,23 +308,20 @@ func (i *Installer) run() (err error) { // Note that the fetched manifest is returned even if an error occurred while // upgrading. func (i *Installer) install() error { - args := []string{i.db.DomainName(), i.slug} - return hooks.Execute("install-app", args, func() error { - newManifest, err := i.ReadManifest(Installing) - if err != nil { - i.log.Debugf("Could not read manifest") - return err - } - i.man = newManifest - i.sendRealtimeEvent() - i.notifyChannel() - if err := i.fetcher.Fetch(i.src, i.fs, i.man); err != nil { - i.log.Debugf("Could not fetch tarball") - return err - } - i.man.SetState(i.endState) - return i.man.Create(i.db) - }) + newManifest, err := i.ReadManifest(Installing) + if err != nil { + i.log.Debugf("Could not read manifest") + return err + } + i.man = newManifest + i.sendRealtimeEvent() + i.notifyChannel() + if err := i.fetcher.Fetch(i.src, i.fs, i.man); err != nil { + i.log.Debugf("Could not fetch tarball") + return err + } + i.man.SetState(i.endState) + return i.man.Create(i.db) } // checkSkipPermissions checks if the instance contexts is configured to skip @@ -535,10 +531,7 @@ func (i *Installer) delete() error { if err := i.checkState(i.man); err != nil { return err } - args := []string{i.db.DomainName(), i.slug} - return hooks.Execute("uninstall-app", args, func() error { - return i.man.Delete(i.db) - }) + return i.man.Delete(i.db) } // checkState returns whether or not the manifest is in the right state to diff --git a/model/instance/lifecycle/create.go b/model/instance/lifecycle/create.go index 37526c38403..f646abd3a32 100644 --- a/model/instance/lifecycle/create.go +++ b/model/instance/lifecycle/create.go @@ -17,7 +17,6 @@ import ( "github.com/cozy/cozy-stack/pkg/consts" "github.com/cozy/cozy-stack/pkg/couchdb" "github.com/cozy/cozy-stack/pkg/crypto" - "github.com/cozy/cozy-stack/pkg/hooks" "github.com/cozy/cozy-stack/pkg/logger" "github.com/cozy/cozy-stack/pkg/prefixer" "github.com/cozy/cozy-stack/pkg/utils" @@ -74,22 +73,6 @@ func (opts *Options) trace(name string, do func()) { // Create builds an instance and initializes it func Create(opts *Options) (*instance.Instance, error) { - domain, err := validateDomain(opts.Domain) - if err != nil { - return nil, err - } - var inst *instance.Instance - err = hooks.Execute("add-instance", []string{domain}, func() error { - var err2 error - inst, err2 = CreateWithoutHooks(opts) - return err2 - }) - return inst, err -} - -// CreateWithoutHooks builds an instance and initializes it. The difference -// with Create is that script hooks are not executed for this function. -func CreateWithoutHooks(opts *Options) (*instance.Instance, error) { domain := opts.Domain var err error opts.trace("validate domain", func() { diff --git a/model/instance/lifecycle/destroy.go b/model/instance/lifecycle/destroy.go index 3d7b8f64f88..d3f991d07b0 100644 --- a/model/instance/lifecycle/destroy.go +++ b/model/instance/lifecycle/destroy.go @@ -13,7 +13,6 @@ import ( "github.com/cozy/cozy-stack/pkg/config/config" "github.com/cozy/cozy-stack/pkg/consts" "github.com/cozy/cozy-stack/pkg/couchdb" - "github.com/cozy/cozy-stack/pkg/hooks" "github.com/cozy/cozy-stack/pkg/logger" "github.com/cozy/cozy-stack/pkg/mail" "github.com/labstack/echo/v4" @@ -56,14 +55,6 @@ func Destroy(domain string) error { if err != nil { return err } - return hooks.Execute("remove-instance", []string{domain}, func() error { - return destroyWithoutHooks(domain) - }) -} - -// destroyWithoutHooks is used to remove the instance. The difference with -// Destroy is that scripts hooks are not executed for this function. -func destroyWithoutHooks(domain string) error { inst, err := instance.GetFromCouch(domain) if err != nil { return err diff --git a/pkg/config/config/config.go b/pkg/config/config/config.go index ca4763abe70..3306596b113 100644 --- a/pkg/config/config/config.go +++ b/pkg/config/config/config.go @@ -105,7 +105,6 @@ type Config struct { NoReplyAddr string NoReplyName string ReplyTo string - Hooks string GeoDB string PasswordResetInterval time.Duration @@ -761,7 +760,6 @@ func UseViper(v *viper.Viper) error { NoReplyAddr: v.GetString("mail.noreply_address"), NoReplyName: v.GetString("mail.noreply_name"), ReplyTo: v.GetString("mail.reply_to"), - Hooks: v.GetString("hooks"), GeoDB: v.GetString("geodb"), PasswordResetInterval: v.GetDuration("password_reset_interval"), diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go deleted file mode 100644 index 6d4c517a8dd..00000000000 --- a/pkg/hooks/hooks.go +++ /dev/null @@ -1,54 +0,0 @@ -package hooks - -import ( - "errors" - "fmt" - "os" - "os/exec" - - "github.com/cozy/cozy-stack/pkg/config/config" - "github.com/cozy/cozy-stack/pkg/logger" -) - -// ErrHookFailed is used when an hook script exits with a non-zero status -var ErrHookFailed = errors.New("Hook exited with non-zero status") - -// Execute runs a pre-hook, then calls te function, and finally run the -// post-hook. -func Execute(name string, args []string, fn func() error) error { - if err := runHook("pre", name, args); err != nil { - return err - } - if err := fn(); err != nil { - return err - } - return runHook("post", name, args) -} - -func runHook(prefix, name string, args []string) error { - dir := config.GetConfig().Hooks - script := fmt.Sprintf("%s/%s-%s", dir, prefix, name) - if !isExecutable(script) { - return nil - } - log := logger.WithNamespace("hooks") - log.Infof("Execute %s with %v", script, args) - cmd := exec.Command(script, args...) - out, err := cmd.CombinedOutput() - if len(out) > 0 { - log.Infof("Output: %s", out) - } - if err != nil { - log.Errorf("Execution failed: %s", err) - return ErrHookFailed - } - return nil -} - -func isExecutable(script string) bool { - stat, err := os.Stat(script) - if err != nil { - return false - } - return stat.Mode()&0100 > 0 -} diff --git a/pkg/hooks/hooks_test.go b/pkg/hooks/hooks_test.go deleted file mode 100644 index 60aafc6f4b2..00000000000 --- a/pkg/hooks/hooks_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package hooks - -import ( - "errors" - "os" - "path" - "testing" - - "github.com/cozy/cozy-stack/pkg/config/config" - "github.com/stretchr/testify/assert" -) - -func TestHooks(t *testing.T) { - if testing.Short() { - t.Skip("an instance is required for this test: test skipped due to the use of --short flag") - } - - config.UseTestFile(t) - config.GetConfig().Hooks = "./testdata" - - t.Run("ExecuteSuccess", func(t *testing.T) { - tmp := path.Join(t.TempDir(), "out.log") - args := []string{tmp, "bar"} - executed := false - fn := func() error { executed = true; return nil } - err := Execute("success", args, fn) - assert.NoError(t, err) - assert.True(t, executed) - content, err := os.ReadFile(tmp) - assert.NoError(t, err) - assert.Equal(t, "post-bar\n", string(content)) - }) - - t.Run("ExecutePreFails", func(t *testing.T) { - executed := false - fn := func() error { executed = true; return nil } - err := Execute("failure", []string{}, fn) - assert.Equal(t, ErrHookFailed, err) - assert.False(t, executed) - }) - - t.Run("ExecuteFunctionFails", func(t *testing.T) { - tmp := path.Join(t.TempDir(), "out.log") - args := []string{tmp, "bar"} - executed := false - e := errors.New("fn fails") - fn := func() error { executed = true; return e } - err := Execute("success", args, fn) - assert.Equal(t, e, err) - assert.True(t, executed) - content, err := os.ReadFile(tmp) - assert.NoError(t, err) - assert.Equal(t, "pre-bar\n", string(content)) - }) - - t.Run("RunHooks", func(t *testing.T) { - tmp := path.Join(t.TempDir(), "out.log") - args := []string{tmp, "bar"} - err := runHook("pre", "success", args) - assert.NoError(t, err) - content, err := os.ReadFile(tmp) - assert.NoError(t, err) - assert.Equal(t, "pre-bar\n", string(content)) - err = runHook("post", "success", args) - assert.NoError(t, err) - content, err = os.ReadFile(tmp) - assert.NoError(t, err) - assert.Equal(t, "post-bar\n", string(content)) - err = runHook("pre", "no-hook", args) - assert.NoError(t, err) - err = runHook("pre", "failure", args) - assert.Error(t, err) - }) - - t.Run("IsExecutable", func(t *testing.T) { - assert.False(t, isExecutable("no/such/file")) - assert.False(t, isExecutable("../../tests/fixtures/logos.zip")) - assert.True(t, isExecutable("./testdata/pre-success")) - }) -} diff --git a/pkg/hooks/testdata/post-success b/pkg/hooks/testdata/post-success deleted file mode 100755 index 65c7610b844..00000000000 --- a/pkg/hooks/testdata/post-success +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/bash -echo "post-$2" > $1 -exit 0 diff --git a/pkg/hooks/testdata/pre-failure b/pkg/hooks/testdata/pre-failure deleted file mode 100755 index afe8ade3e26..00000000000 --- a/pkg/hooks/testdata/pre-failure +++ /dev/null @@ -1,2 +0,0 @@ -#!/bin/bash -exit 1 diff --git a/pkg/hooks/testdata/pre-success b/pkg/hooks/testdata/pre-success deleted file mode 100755 index f285d347bfc..00000000000 --- a/pkg/hooks/testdata/pre-success +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/bash -echo "pre-$2" > $1 -exit 0 diff --git a/scripts/hooks/post-install-app.sample b/scripts/hooks/post-install-app.sample deleted file mode 100755 index 145568e5efa..00000000000 --- a/scripts/hooks/post-install-app.sample +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash -# -# The "post-install-app" hook is run just after an application has been -# installed. It can be used for logging, notification, statistics, etc. -# It's also a good place to add a vhost for an application in the -# reverse-proxy configuration, with a TLS certificate. -# -# The hook is called with the following parameters: -# -# $1 -- the instance on which the application has been installed -# $2 -- the application name that has been installed. -# -# To enable this hook, rename this file to "post-install-app". -# -# This sample shows how to log the install. - -echo "$1 $2" >> /var/log/cozy/installs.log diff --git a/scripts/hooks/pre-install-app.sample b/scripts/hooks/pre-install-app.sample deleted file mode 100755 index d40c4dd4de2..00000000000 --- a/scripts/hooks/pre-install-app.sample +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/bash -# -# The "pre-install-app" hook is run just before installing an application, -# and can prevent the command from running by exiting with non-zero status. -# -# The hook is called with the following parameters: -# -# $1 -- the instance on which the application will be installed -# $2 -- the application name that will be installed. -# -# To enable this hook, rename this file to "pre-install-app". -# -# This sample shows how to block install randomly. - -exit $[ $RANDOM % 2 ]