GitHub

Original file line numberDiff line numberDiff line change

@@ -11,7 +11,7 @@ import (

1111

yaml "github.com/zrepl/yaml-config"

1212
1313

"github.com/zrepl/zrepl/internal/client/status/viewmodel/stringbuilder"

14-

"github.com/zrepl/zrepl/internal/daemon"

14+

"github.com/zrepl/zrepl/internal/config"

1515

"github.com/zrepl/zrepl/internal/daemon/job"

1616

"github.com/zrepl/zrepl/internal/daemon/pruner"

1717

"github.com/zrepl/zrepl/internal/daemon/snapper"

@@ -85,7 +85,7 @@ func (m *M) Update(p Params) {

8585

// filter out internal jobs

8686

var jobsList []*Job

8787

for _, j := range m.jobsList {

88-

if daemon.IsInternalJobName(j.name) {

88+

if config.IsInternalJobName(j.name) {

8989

continue

9090

}

9191

jobsList = append(jobsList, j)

Original file line numberDiff line numberDiff line change

@@ -6,6 +6,7 @@ import (

66

"os"

77

pathpkg "path"

88

"path/filepath"

9+

"strings"

910

"time"

1011
1112

"github.com/pkg/errors"

@@ -693,9 +694,32 @@ func ParseConfig(path string) (rootConfig *Config, err error) {

693694

return nil, err

694695

}

695696
697+

if err = validateJobNames(rootConfig); err != nil {

698+

return nil, err

699+

}

700+
696701

return rootConfig, err

697702

}

698703
704+

func IsInternalJobName(s string) bool {

705+

return strings.HasPrefix(s, "_")

706+

}

707+
708+

func validateJobNames(config *Config) error {

709+

seen := make(map[string]struct{})

710+

for _, job := range config.Jobs {

711+

name := job.Name()

712+

if IsInternalJobName(name) {

713+

return errors.Errorf("job name %q is reserved for internal use (starts with _)", name)

714+

}

715+

if _, ok := seen[name]; ok {

716+

return errors.Errorf("duplicate job name %q", name)

717+

}

718+

seen[name] = struct{}{}

719+

}

720+

return nil

721+

}

722+
699723

func expandConfigInclude(configPath string, config *Config) (err error) {

700724

var includeConfigPaths []string

701725

for _, path := range config.Include {

Original file line numberDiff line numberDiff line change

@@ -45,6 +45,20 @@ func TestSampleConfigsAreParsedWithoutErrors(t *testing.T) {

4545
4646

}

4747
48+

func TestInvalidSampleConfigsFailToParse(t *testing.T) {

49+

paths, err := filepath.Glob("./samples/invalid/*/zrepl.yml")

50+

require.NoError(t, err, "glob failed")

51+

require.NotEmpty(t, paths, "no invalid sample configs found")

52+
53+

for _, p := range paths {

54+

t.Run(p, func(t *testing.T) {

55+

_, err := ParseConfig(p)

56+

require.Error(t, err, "expected config %s to fail parsing", p)

57+

t.Logf("config %s failed as expected: %v", p, err)

58+

})

59+

}

60+

}

61+
4862

// template must be a template/text template with a single '{{ . }}' as placeholder for val

4963

//

5064

//nolint:deadcode,unused

Original file line numberDiff line numberDiff line change

@@ -0,0 +1,24 @@

1+

jobs:

2+

- type: snap

3+

name: "my_job"

4+

filesystems: {

5+

"<": true,

6+

}

7+

snapshotting:

8+

type: manual

9+

pruning:

10+

keep:

11+

- type: last_n

12+

count: 10

13+
14+

- type: snap

15+

name: "my_job"

16+

filesystems: {

17+

"<": true,

18+

}

19+

snapshotting:

20+

type: manual

21+

pruning:

22+

keep:

23+

- type: last_n

24+

count: 5

Original file line numberDiff line numberDiff line change

@@ -0,0 +1,12 @@

1+

jobs:

2+

- type: snap

3+

name: "my_job"

4+

filesystems: {

5+

"<": true,

6+

}

7+

snapshotting:

8+

type: manual

9+

pruning:

10+

keep:

11+

- type: last_n

12+

count: 5

Original file line numberDiff line numberDiff line change

@@ -0,0 +1,15 @@

1+

jobs:

2+

- type: snap

3+

name: "my_job"

4+

filesystems: {

5+

"<": true,

6+

}

7+

snapshotting:

8+

type: manual

9+

pruning:

10+

keep:

11+

- type: last_n

12+

count: 10

13+
14+

include:

15+

- ./included.yml

Original file line numberDiff line numberDiff line change

@@ -0,0 +1,12 @@

1+

jobs:

2+

- type: snap

3+

name: "_internal_job"

4+

filesystems: {

5+

"<": true,

6+

}

7+

snapshotting:

8+

type: manual

9+

pruning:

10+

keep:

11+

- type: last_n

12+

count: 10

Original file line numberDiff line numberDiff line change

@@ -5,7 +5,6 @@ import (

55

"fmt"

66

"os"

77

"os/signal"

8-

"strings"

98

"sync"

109

"syscall"

1110

"time"

@@ -64,7 +63,7 @@ func Run(ctx context.Context, conf *config.Config) error {

6463

})

6564
6665

for _, job := range confJobs {

67-

if IsInternalJobName(job.Name()) {

66+

if config.IsInternalJobName(job.Name()) {

6867

panic(fmt.Sprintf("internal job name used for config job '%s'", job.Name())) //FIXME

6968

}

7069

}

@@ -211,21 +210,19 @@ const (

211210

jobNameControl = "_control"

212211

)

213212
214-

func IsInternalJobName(s string) bool {

215-

return strings.HasPrefix(s, "_")

216-

}

217-
218213

func (s *jobs) start(ctx context.Context, j job.Job, internal bool) {

219214

s.m.Lock()

220215

defer s.m.Unlock()

221216
222217

ctx = logging.WithInjectedField(ctx, logging.JobField, j.Name())

223218
224219

jobName := j.Name()

225-

if !internal && IsInternalJobName(jobName) {

220+
221+

// package `config` enforces these with clean errors, these are just assertions

222+

if !internal && config.IsInternalJobName(jobName) {

226223

panic(fmt.Sprintf("internal job name used for non-internal job %s", jobName))

227224

}

228-

if internal && !IsInternalJobName(jobName) {

225+

if internal && !config.IsInternalJobName(jobName) {

229226

panic(fmt.Sprintf("internal job does not use internal job name %s", jobName))

230227

}

231228

if _, ok := s.jobs[jobName]; ok {

Read the original on github.com ↗