Merge pull request #5457 from kobergj/PostprocessingConfiguration

Better Configuration for Postprocessing Service

changelog/unreleased/postprocessing-config.md

@@ -0,0 +1,5 @@
+Enhancement: Better config for postprocessing service
+
+The postporcessing service is now individually configurable. This is achieved by allowing a list of postprocessing steps that are processed in order of their appearance in the `POSTPROCESSING_STEPS` envvar.
+
+https://github.com/owncloud/ocis/pull/5457

services/postprocessing/README.md

@@ -20,12 +20,25 @@ When postprocessing has been enabled, configuring any postprocessing step will r
 
 ## Postprocessing Steps
 
-As of now, `ocis` allows two different postprocessing steps to be enabled via an environment variable.
+The postporcessing service is individually configurable. This is achieved by allowing a list of postprocessing steps that are processed in order of their appearance in the `POSTPROCESSING_STEPS` envvar. This envvar expects a comma separated list of steps that will be executed. Currently known steps to the system are `virusscan` and `delay`. Custom steps can be added but need an existing target for processing.
 
 ### Virus Scanning
 
-To enable virus scanning as a postprocessing step after uploading a file, the environment variable  `POSTPROCESSING_VIRUSSCAN` needs to be set to ` true`. As a result, each uploaded file gets virus scanned as part of the postprocessing steps. Note that the `antivirus` service is required to be enabled and configured for this to work.
+To enable virus scanning as a postprocessing step after uploading a file, the environment variable `POSTPROCESSING_STEPS` needs to contain the word `virusscan` at one location in the list of steps. As a result, each uploaded file gets virus scanned as part of the postprocessing steps. Note that the `antivirus` service is required to be enabled and configured for this to work.
 
 ### Delay
 
-Though this is for development purposes only and NOT RECOMMENDED on production systems, setting the environment variable `POSTPROCESSING_DELAY` to a duration not equal to zero will add a delay step with the configured amount of time. ocis will continue postprocessing the file after the configured delay.
+Though this is for development purposes only and NOT RECOMMENDED on production systems, setting the environment variable `POSTPROCESSING_DELAY` to a duration not equal to zero will add a delay step with the configured amount of time. ocis will continue postprocessing the file after the configured delay. Use the enviroment variable `POSTPROCESSING_STEPS` and the keyword `delay` if you have multiple postprocessing steps and want to define their order. If `POSTPROCESSING_DELAY` is set but the keyword `delay` is not contained in `POSTPROCESSING_STEPS`, it will be processed as last postprocessing step without being listed there. In this case, a log entry will be written on service startup to notify the admin about that situation. That log entry can be avoided by adding the keyword `delay` to `POSTPROCESSING_STEPS`.
+
+### Custom Postprocessing Steps
+By using the envvar `POSTPROCESSING_STEPS`, custom postprocessing steps can be added. Any word can be used as step name but be careful not to conflict with exising keywords like `virusscan` and `delay`. In addition, if a keyword is misspelled or the corresponding service does either not exist or does not follow the necessary event communication, the postprocessing service will wait forever getting the required response to proceed and does not continue any other processing.
+
+#### Prerequisites
+For using custom postprocessing steps you need a custom service listening to the configured event system (see `General Prerequisites`)
+
+#### Workflow
+When setting a custom postprocessing step (eg. `"customstep"`) the postprocessing service will eventually sent an event during postprocessing. The event will be of type `StartPostprocessingStep` with its field `StepToStart` set to `"customstep"`. When the custom service receives this event it can savely execute its actions, postprocessing service will wait until it has finished its work. The event contains further information (filename, executing user, size, ...) and also required tokens and urls to download the file in case byte inspection is necessary.
+
+Once the custom service has finished its work, it should sent an event of type `PostprocessingFinished` via the configured events system. This event needs to contain a `FinishedStep` field set to `"customstep"`. It also must contain the outcome of the step, which can be one of "delete" (abort postprocessing, delete the file), "abort" (abort postprocessing, keep the file) and "continue" (continue postprocessing, this is the success case).
+
+See https://github.com/cs3org/reva/blob/edge/pkg/events/postprocessing.go for up-to-date information of reserved step names and event definitons.

services/postprocessing/pkg/config/config.go

@@ -20,11 +20,12 @@ type Config struct {
 	Context context.Context `yaml:"-"`
 }
 
-// Postprocessing definces the config options for the postprocessing service.
+// Postprocessing defines the config options for the postprocessing service.
 type Postprocessing struct {
 	Events          Events        `yaml:"events"`
-	Virusscan       bool          `yaml:"virusscan" env:"POSTPROCESSING_VIRUSSCAN" desc:"After uploading a file but before making it available for download, virus scanning the file can be enabled. Needs as prerequisite the antivirus service to be enabled and configured."`
-	Delayprocessing time.Duration `yaml:"delayprocessing" env:"POSTPROCESSING_DELAY" desc:"After uploading a file but before making it available for download, a delay step can be added. Intended for developing purposes only. The duration can be set as number followed by a unit identifier like s, m or h."`
+	Steps           []string      `yaml:"steps" env:"POSTPROCESSING_STEPS" desc:"A comma separated list of postprocessing steps, processed in order of their appearance. Currently supported values by the system are: 'virusscan' and 'delay'. Custom steps are allowed. See the documentation for instructions."`
+	Virusscan       bool          `yaml:"virusscan" env:"POSTPROCESSING_VIRUSSCAN" desc:"After uploading a file but before making it available for download, virus scanning the file can be enabled. Needs as prerequisite the antivirus service to be enabled and configured." deprecationVersion:"master" removalVersion:"master" deprecationInfo:"POSTPROCESSING_VIRUSSCAN is not longer necessary and is replaced by POSTPROCESSING_STEPS which also holds information about the order of steps" deprecationReplacement:"POSTPROCESSING_STEPS"`
+	Delayprocessing time.Duration `yaml:"delayprocessing" env:"POSTPROCESSING_DELAY" desc:"After uploading a file but before making it available for download, a delay step can be added. Intended for developing purposes only. The duration can be set as number followed by a unit identifier like s, m or h. If a duration is set but the keyword 'delay' is not explicitely added to 'POSTPROCESSING_STEPS', the delay step will be processed as last step. In such a case, a log entry will be written on service startup to remind the admin about that situation."`
 }
 
 // Events combines the configuration options for the event bus.

services/postprocessing/pkg/config/parser/parse.go

@@ -2,7 +2,10 @@ package parser
 
 import (
 	"errors"
+	"fmt"
+	"strings"
 
+	"github.com/cs3org/reva/v2/pkg/events"
 	ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config"
 	"github.com/owncloud/ocis/v2/services/postprocessing/pkg/config"
 	"github.com/owncloud/ocis/v2/services/postprocessing/pkg/config/defaults"
@@ -32,6 +35,33 @@ func ParseConfig(cfg *config.Config) error {
 	return Validate(cfg)
 }
 
+// Validate validates the config
 func Validate(cfg *config.Config) error {
+	if cfg.Postprocessing.Virusscan {
+		if !contains(cfg.Postprocessing.Steps, events.PPStepAntivirus) {
+			cfg.Postprocessing.Steps = append(cfg.Postprocessing.Steps, string(events.PPStepAntivirus))
+			fmt.Printf("ATTENTION: POSTPROCESSING_VIRUSSCAN is deprecated. Use `POSTPROCESSING_STEPS=%v` in the future\n", strings.Join(cfg.Postprocessing.Steps, ","))
+		}
+	}
+
+	if cfg.Postprocessing.Delayprocessing != 0 {
+		if !contains(cfg.Postprocessing.Steps, events.PPStepDelay) {
+			if len(cfg.Postprocessing.Steps) > 0 {
+				s := strings.Join(append(cfg.Postprocessing.Steps, string(events.PPStepDelay)), ",")
+				fmt.Printf("Added delay step to the list of postprocessing steps. NOTE: Use envvar `POSTPROCESSING_STEPS=%s` to suppress this message and choose the order of postprocessing steps.\n", s)
+			}
+
+			cfg.Postprocessing.Steps = append(cfg.Postprocessing.Steps, string(events.PPStepDelay))
+		}
+	}
 	return nil
 }
+
+func contains(all []string, candidate events.Postprocessingstep) bool {
+	for _, s := range all {
+		if s == string(candidate) {
+			return true
+		}
+	}
+	return false
+}

services/postprocessing/pkg/postprocessing/postprocessing.go

@@ -6,7 +6,6 @@ import (
 	user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/events"
-	"github.com/owncloud/ocis/v2/services/postprocessing/pkg/config"
 )
 
 // Postprocessing handles postprocessing of a file
@@ -18,22 +17,22 @@ type Postprocessing struct {
 	filename   string
 	filesize   uint64
 	resourceID *provider.ResourceId
-	c          config.Postprocessing
 	steps      []events.Postprocessingstep
+	delay      time.Duration
 }
 
 // New returns a new postprocessing instance
-func New(uploadID string, uploadURL string, user *user.User, filename string, filesize uint64, resourceID *provider.ResourceId, c config.Postprocessing) *Postprocessing {
+func New(uploadID string, uploadURL string, user *user.User, filename string, filesize uint64, resourceID *provider.ResourceId, steps []events.Postprocessingstep, delay time.Duration) *Postprocessing {
 	return &Postprocessing{
 		id:         uploadID,
 		url:        uploadURL,
 		u:          user,
 		m:          make(map[events.Postprocessingstep]interface{}),
-		c:          c,
 		filename:   filename,
 		filesize:   filesize,
 		resourceID: resourceID,
-		steps:      getSteps(c),
+		steps:      steps,
+		delay:      delay,
 	}
 }
 
@@ -48,13 +47,13 @@ func (pp *Postprocessing) Init(ev events.BytesReceived) interface{} {
 	return pp.nextStep(pp.steps[0])
 }
 
-// Virusscan is the virusscanning step of the postprocessing
-func (pp *Postprocessing) Virusscan(ev events.VirusscanFinished) interface{} {
-	pp.m[events.PPStepAntivirus] = ev
+// NextStep returns the next postprocessing step
+func (pp *Postprocessing) NextStep(ev events.PostprocessingStepFinished) interface{} {
+	pp.m[ev.FinishedStep] = ev
 
 	switch ev.Outcome {
 	case events.PPOutcomeContinue:
-		return pp.next(events.PPStepAntivirus)
+		return pp.next(ev.FinishedStep)
 	default:
 		return pp.finished(ev.Outcome)
 
@@ -64,7 +63,7 @@ func (pp *Postprocessing) Virusscan(ev events.VirusscanFinished) interface{} {
 // Delay will sleep the configured time then continue
 func (pp *Postprocessing) Delay(ev events.StartPostprocessingStep) interface{} {
 	pp.m[events.PPStepDelay] = ev
-	time.Sleep(pp.c.Delayprocessing)
+	time.Sleep(pp.delay)
 	return pp.next(events.PPStepDelay)
 }
 
@@ -99,20 +98,3 @@ func (pp *Postprocessing) finished(outcome events.PostprocessingOutcome) events.
 		Outcome:       outcome,
 	}
 }
-
-func getSteps(c config.Postprocessing) []events.Postprocessingstep {
-	// NOTE: first version only contains very basic configuration options
-	// But we aim for a system where postprocessing steps and their order can be configured per space
-	// ideally by the spaceadmin itself
-	// We need to iterate over configuring PP service when we see fit
-	var steps []events.Postprocessingstep
-	if c.Delayprocessing != 0 {
-		steps = append(steps, events.PPStepDelay)
-	}
-
-	if c.Virusscan {
-		steps = append(steps, events.PPStepAntivirus)
-	}
-
-	return steps
-}

services/postprocessing/pkg/service/service.go

@@ -12,6 +12,7 @@ type PostprocessingService struct {
 	log    log.Logger
 	events <-chan interface{}
 	pub    events.Publisher
+	steps  []events.Postprocessingstep
 	c      config.Postprocessing
 }
 
@@ -31,6 +32,7 @@ func NewPostprocessingService(stream events.Stream, logger log.Logger, c config.
 		log:    logger,
 		events: evs,
 		pub:    stream,
+		steps:  getSteps(c),
 		c:      c,
 	}, nil
 }
@@ -42,16 +44,16 @@ func (pps *PostprocessingService) Run() error {
 		var next interface{}
 		switch ev := e.(type) {
 		case events.BytesReceived:
-			pp := postprocessing.New(ev.UploadID, ev.URL, ev.ExecutingUser, ev.Filename, ev.Filesize, ev.ResourceID, pps.c)
+			pp := postprocessing.New(ev.UploadID, ev.URL, ev.ExecutingUser, ev.Filename, ev.Filesize, ev.ResourceID, pps.steps, pps.c.Delayprocessing)
 			current[ev.UploadID] = pp
 			next = pp.Init(ev)
-		case events.VirusscanFinished:
+		case events.PostprocessingStepFinished:
 			pp := current[ev.UploadID]
 			if pp == nil {
 				// no current upload - this was an on demand scan
 				continue
 			}
-			next = pp.Virusscan(ev)
+			next = pp.NextStep(ev)
 		case events.StartPostprocessingStep:
 			if ev.StepToStart != events.PPStepDelay {
 				continue
@@ -73,3 +75,15 @@ func (pps *PostprocessingService) Run() error {
 	}
 	return nil
 }
+
+func getSteps(c config.Postprocessing) []events.Postprocessingstep {
+	// NOTE: improved version only allows configuring order of postprocessing steps
+	// But we aim for a system where postprocessing steps can be configured per space, ideally by the spaceadmin itself
+	// We need to iterate over configuring PP service when we see fit
+	var steps []events.Postprocessingstep
+	for _, s := range c.Steps {
+		steps = append(steps, events.Postprocessingstep(s))
+	}
+
+	return steps
+}