Add line, col to types used in batch-expire (#18747)

cmd/batch-expire.go

@@ -36,6 +36,7 @@ import (
 	"github.com/minio/pkg/v2/env"
 	"github.com/minio/pkg/v2/wildcard"
 	"github.com/minio/pkg/v2/workers"
+	"gopkg.in/yaml.v3"
 )
 
 // expire: # Expire objects that match a condition
@@ -80,19 +81,41 @@ import (
 
 // BatchJobExpirePurge type accepts non-negative versions to be retained
 type BatchJobExpirePurge struct {
+	line, col      int
 	RetainVersions int `yaml:"retainVersions" json:"retainVersions"`
 }
 
+var _ yaml.Unmarshaler = &BatchJobExpirePurge{}
+
+// UnmarshalYAML - BatchJobExpirePurge extends unmarshal to extract line, col
+func (p *BatchJobExpirePurge) UnmarshalYAML(val *yaml.Node) error {
+	type purge BatchJobExpirePurge
+	var tmp purge
+	err := val.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+
+	*p = BatchJobExpirePurge(tmp)
+	p.line, p.col = val.Line, val.Column
+	return nil
+}
+
 // Validate returns nil if value is valid, ie > 0.
 func (p BatchJobExpirePurge) Validate() error {
 	if p.RetainVersions < 0 {
-		return errors.New("retainVersions must be >= 0")
+		return BatchJobYamlErr{
+			line: p.line,
+			col:  p.col,
+			msg:  "retainVersions must be >= 0",
+		}
 	}
 	return nil
 }
 
 // BatchJobExpireFilter holds all the filters currently supported for batch replication
 type BatchJobExpireFilter struct {
+	line, col     int
 	OlderThan     time.Duration       `yaml:"olderThan,omitempty" json:"olderThan"`
 	CreatedBefore *time.Time          `yaml:"createdBefore,omitempty" json:"createdBefore"`
 	Tags          []BatchJobKV        `yaml:"tags,omitempty" json:"tags"`
@@ -103,6 +126,22 @@ type BatchJobExpireFilter struct {
 	Purge         BatchJobExpirePurge `yaml:"purge" json:"purge"`
 }
 
+var _ yaml.Unmarshaler = &BatchJobExpireFilter{}
+
+// UnmarshalYAML - BatchJobExpireFilter extends unmarshal to extract line, col
+// information
+func (ef *BatchJobExpireFilter) UnmarshalYAML(value *yaml.Node) error {
+	type expFilter BatchJobExpireFilter
+	var tmp expFilter
+	err := value.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+	*ef = BatchJobExpireFilter(tmp)
+	ef.line, ef.col = value.Line, value.Column
+	return err
+}
+
 // Matches returns true if obj matches the filter conditions specified in ef.
 func (ef BatchJobExpireFilter) Matches(obj ObjectInfo, now time.Time) bool {
 	switch ef.Type {
@@ -194,10 +233,18 @@ func (ef BatchJobExpireFilter) Validate() error {
 	case BatchJobExpireObject:
 	case BatchJobExpireDeleted:
 		if len(ef.Tags) > 0 || len(ef.Metadata) > 0 {
-			return errors.New("invalid batch-expire rule filter")
+			return BatchJobYamlErr{
+				line: ef.line,
+				col:  ef.col,
+				msg:  "delete type filter can't have tags or metadata",
+			}
 		}
 	default:
-		return errors.New("invalid batch-expire type")
+		return BatchJobYamlErr{
+			line: ef.line,
+			col:  ef.col,
+			msg:  "invalid batch-expire type",
+		}
 	}
 
 	for _, tag := range ef.Tags {
@@ -218,7 +265,11 @@ func (ef BatchJobExpireFilter) Validate() error {
 		return err
 	}
 	if ef.CreatedBefore != nil && !ef.CreatedBefore.Before(time.Now()) {
-		return errors.New("CreatedBefore is in the future")
+		return BatchJobYamlErr{
+			line: ef.line,
+			col:  ef.col,
+			msg:  "CreatedBefore is in the future",
+		}
 	}
 	return nil
 }
@@ -226,6 +277,7 @@ func (ef BatchJobExpireFilter) Validate() error {
 // BatchJobExpire represents configuration parameters for a batch expiration
 // job typically supplied in yaml form
 type BatchJobExpire struct {
+	line, col       int
 	APIVersion      string                 `yaml:"apiVersion" json:"apiVersion"`
 	Bucket          string                 `yaml:"bucket" json:"bucket"`
 	Prefix          string                 `yaml:"prefix" json:"prefix"`
@@ -234,6 +286,22 @@ type BatchJobExpire struct {
 	Rules           []BatchJobExpireFilter `yaml:"rules" json:"rules"`
 }
 
+var _ yaml.Unmarshaler = &BatchJobExpire{}
+
+// UnmarshalYAML - BatchJobExpire extends default unmarshal to extract line, col information.
+func (r *BatchJobExpire) UnmarshalYAML(val *yaml.Node) error {
+	type expireJob BatchJobExpire
+	var tmp expireJob
+	err := val.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+
+	*r = BatchJobExpire(tmp)
+	r.line, r.col = val.Line, val.Column
+	return nil
+}
+
 // Notify notifies notification endpoint if configured regarding job failure or success.
 func (r BatchJobExpire) Notify(ctx context.Context, body io.Reader) error {
 	if r.NotificationCfg.Endpoint == "" {

cmd/batch-handlers.go

@@ -52,7 +52,7 @@ import (
 	"github.com/minio/pkg/v2/env"
 	"github.com/minio/pkg/v2/policy"
 	"github.com/minio/pkg/v2/workers"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 )
 
 var globalBatchConfig batch.Config
@@ -1564,7 +1564,7 @@ func (a adminAPIHandlers) StartBatchJob(w http.ResponseWriter, r *http.Request)
 	}
 
 	job := &BatchJobRequest{}
-	if err = yaml.UnmarshalStrict(buf, job); err != nil {
+	if err = yaml.Unmarshal(buf, job); err != nil {
 		writeErrorResponseJSON(ctx, w, toAPIError(ctx, err), r.URL)
 		return
 	}

cmd/batch-job-common-types.go

@@ -18,26 +18,66 @@
 package cmd
 
 import (
-	"errors"
+	"fmt"
 	"strings"
 	"time"
 
 	"github.com/dustin/go-humanize"
 	"github.com/minio/pkg/v2/wildcard"
+	"gopkg.in/yaml.v3"
 )
 
 //go:generate msgp -file $GOFILE
+//msgp:ignore BatchJobYamlErr
+
+// BatchJobYamlErr can be used to return yaml validation errors with line,
+// column information guiding user to fix syntax errors
+type BatchJobYamlErr struct {
+	line, col int
+	msg       string
+}
+
+// message returns the error message excluding line, col information.
+// Intended to be used in unit tests.
+func (b BatchJobYamlErr) message() string {
+	return b.msg
+}
+
+// Error implements Error interface
+func (b BatchJobYamlErr) Error() string {
+	return fmt.Sprintf("%s\n Hint: error near line: %d, col: %d", b.msg, b.line, b.col)
+}
 
 // BatchJobKV is a key-value data type which supports wildcard matching
 type BatchJobKV struct {
-	Key   string `yaml:"key" json:"key"`
-	Value string `yaml:"value" json:"value"`
+	line, col int
+	Key       string `yaml:"key" json:"key"`
+	Value     string `yaml:"value" json:"value"`
+}
+
+var _ yaml.Unmarshaler = &BatchJobKV{}
+
+// UnmarshalYAML - BatchJobKV extends default unmarshal to extract line, col information.
+func (kv *BatchJobKV) UnmarshalYAML(val *yaml.Node) error {
+	type jobKV BatchJobKV
+	var tmp jobKV
+	err := val.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+	*kv = BatchJobKV(tmp)
+	kv.line, kv.col = val.Line, val.Column
+	return nil
 }
 
 // Validate returns an error if key is empty
 func (kv BatchJobKV) Validate() error {
 	if kv.Key == "" {
-		return errInvalidArgument
+		return BatchJobYamlErr{
+			line: kv.line,
+			col:  kv.col,
+			msg:  "key can't be empty",
+		}
 	}
 	return nil
 }
@@ -61,24 +101,66 @@ func (kv BatchJobKV) Match(ikv BatchJobKV) bool {
 // BatchJobNotification stores notification endpoint and token information.
 // Used by batch jobs to notify of their status.
 type BatchJobNotification struct {
-	Endpoint string `yaml:"endpoint" json:"endpoint"`
-	Token    string `yaml:"token" json:"token"`
+	line, col int
+	Endpoint  string `yaml:"endpoint" json:"endpoint"`
+	Token     string `yaml:"token" json:"token"`
+}
+
+var _ yaml.Unmarshaler = &BatchJobNotification{}
+
+// UnmarshalYAML - BatchJobNotification extends unmarshal to extract line, column information
+func (b *BatchJobNotification) UnmarshalYAML(val *yaml.Node) error {
+	type notification BatchJobNotification
+	var tmp notification
+	err := val.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+
+	*b = BatchJobNotification(tmp)
+	b.line, b.col = val.Line, val.Column
+	return nil
 }
 
 // BatchJobRetry stores retry configuration used in the event of failures.
 type BatchJobRetry struct {
-	Attempts int           `yaml:"attempts" json:"attempts"` // number of retry attempts
-	Delay    time.Duration `yaml:"delay" json:"delay"`       // delay between each retries
+	line, col int
+	Attempts  int           `yaml:"attempts" json:"attempts"` // number of retry attempts
+	Delay     time.Duration `yaml:"delay" json:"delay"`       // delay between each retries
+}
+
+var _ yaml.Unmarshaler = &BatchJobRetry{}
+
+// UnmarshalYAML - BatchJobRetry extends unmarshal to extract line, column information
+func (r *BatchJobRetry) UnmarshalYAML(val *yaml.Node) error {
+	type retry BatchJobRetry
+	var tmp retry
+	err := val.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+
+	*r = BatchJobRetry(tmp)
+	r.line, r.col = val.Line, val.Column
+	return nil
 }
 
 // Validate validates input replicate retries.
 func (r BatchJobRetry) Validate() error {
 	if r.Attempts < 0 {
-		return errInvalidArgument
+		return BatchJobYamlErr{
+			line: r.line,
+			col:  r.col,
+			msg:  "Invalid arguments specified",
+		}
 	}
 
 	if r.Delay < 0 {
-		return errInvalidArgument
+		return BatchJobYamlErr{
+			line: r.line,
+			col:  r.col,
+			msg:  "Invalid arguments specified",
+		}
 	}
 
 	return nil
@@ -96,6 +178,7 @@ func (r BatchJobRetry) Validate() error {
 
 // BatchJobSnowball describes the snowball feature when replicating objects from a local source to a remote target
 type BatchJobSnowball struct {
+	line, col   int
 	Disable     *bool   `yaml:"disable" json:"disable"`
 	Batch       *int    `yaml:"batch" json:"batch"`
 	InMemory    *bool   `yaml:"inmemory" json:"inmemory"`
@@ -104,21 +187,60 @@ type BatchJobSnowball struct {
 	SkipErrs    *bool   `yaml:"skipErrs" json:"skipErrs"`
 }
 
+var _ yaml.Unmarshaler = &BatchJobSnowball{}
+
+// UnmarshalYAML - BatchJobSnowball extends unmarshal to extract line, column information
+func (b *BatchJobSnowball) UnmarshalYAML(val *yaml.Node) error {
+	type snowball BatchJobSnowball
+	var tmp snowball
+	err := val.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+
+	*b = BatchJobSnowball(tmp)
+	b.line, b.col = val.Line, val.Column
+	return nil
+}
+
 // Validate the snowball parameters in the job description
 func (b BatchJobSnowball) Validate() error {
 	if *b.Batch <= 0 {
-		return errors.New("batch number should be non positive zero")
+		return BatchJobYamlErr{
+			line: b.line,
+			col:  b.col,
+			msg:  "batch number should be non positive zero",
+		}
 	}
 	_, err := humanize.ParseBytes(*b.SmallerThan)
-	return err
+	return BatchJobYamlErr{
+		line: b.line,
+		col:  b.col,
+		msg:  err.Error(),
+	}
 }
 
 // BatchJobSizeFilter supports size based filters - LesserThan and GreaterThan
 type BatchJobSizeFilter struct {
+	line, col  int
 	UpperBound BatchJobSize `yaml:"lessThan" json:"lessThan"`
 	LowerBound BatchJobSize `yaml:"greaterThan" json:"greaterThan"`
 }
 
+// UnmarshalYAML - BatchJobSizeFilter extends unmarshal to extract line, column information
+func (sf *BatchJobSizeFilter) UnmarshalYAML(val *yaml.Node) error {
+	type sizeFilter BatchJobSizeFilter
+	var tmp sizeFilter
+	err := val.Decode(&tmp)
+	if err != nil {
+		return err
+	}
+
+	*sf = BatchJobSizeFilter(tmp)
+	sf.line, sf.col = val.Line, val.Column
+	return nil
+}
+
 // InRange returns true in the following cases and false otherwise,
 // - sf.LowerBound < sz, when sf.LowerBound alone is specified
 // - sz < sf.UpperBound, when sf.UpperBound alone is specified
@@ -134,12 +256,14 @@ func (sf BatchJobSizeFilter) InRange(sz int64) bool {
 	return true
 }
 
-var errInvalidBatchJobSizeFilter = errors.New("invalid batch-job size filter")
-
 // Validate checks if sf is a valid batch-job size filter
 func (sf BatchJobSizeFilter) Validate() error {
 	if sf.LowerBound > 0 && sf.UpperBound > 0 && sf.LowerBound >= sf.UpperBound {
-		return errInvalidBatchJobSizeFilter
+		return BatchJobYamlErr{
+			line: sf.line,
+			col:  sf.col,
+			msg:  "invalid batch-job size filter",
+		}
 	}
 	return nil
 }

cmd/batch-job-common-types_test.go

@@ -83,6 +83,10 @@ func TestBatchJobSizeInRange(t *testing.T) {
 }
 
 func TestBatchJobSizeValidate(t *testing.T) {
+	errInvalidBatchJobSizeFilter := BatchJobYamlErr{
+		msg: "invalid batch-job size filter",
+	}
+
 	tests := []struct {
 		sizeFilter BatchJobSizeFilter
 		err        error
@@ -128,8 +132,16 @@ func TestBatchJobSizeValidate(t *testing.T) {
 	}
 	for i, test := range tests {
 		t.Run(fmt.Sprintf("test-%d", i+1), func(t *testing.T) {
-			if err := test.sizeFilter.Validate(); err != test.err {
-				t.Fatalf("Expected %v but got %v", test.err, err)
+			err := test.sizeFilter.Validate()
+			if err != nil {
+				gotErr := err.(BatchJobYamlErr)
+				testErr := test.err.(BatchJobYamlErr)
+				if gotErr.message() != testErr.message() {
+					t.Fatalf("Expected %v but got %v", test.err, err)
+				}
+			}
+			if err == nil && test.err != nil {
+				t.Fatalf("Expected %v but got nil", test.err)
 			}
 		})
 	}

go.mod

@@ -98,6 +98,7 @@ require (
 	golang.org/x/time v0.5.0
 	google.golang.org/api v0.154.0
 	gopkg.in/yaml.v2 v2.4.0
+	gopkg.in/yaml.v3 v3.0.1
 )
 
 require (
@@ -258,5 +259,4 @@ require (
 	gopkg.in/h2non/filetype.v1 v1.0.5 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/square/go-jose.v2 v2.6.0 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
 )