validate content of develop.watch section

Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
compose-spec · Feb 20, 2024 · ef78476 · ef78476
1 parent 40e24b0
commit ef78476
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 6 deletions.
diff --git a/loader/loader_test.go b/loader/loader_test.go
@@ -2816,6 +2816,7 @@ services:
         # rebuild image and recreate service
         - path: ./proxy/proxy.conf
           action: sync+restart
+          target: /etc/nginx/proxy.conf
 `, nil), func(options *Options) {
 		options.ResolvePaths = false
 		options.SkipValidation = true
@@ -2850,11 +2851,33 @@ services:
 			{
 				Path:   "./proxy/proxy.conf",
 				Action: types.WatchActionSyncRestart,
+				Target: "/etc/nginx/proxy.conf",
 			},
 		},
 	})
 }
 
+func TestBadDevelopConfig(t *testing.T) {
+	_, err := LoadWithContext(context.TODO(), buildConfigDetails(`
+name: load-develop
+services:
+  frontend:
+    image: example/webapp
+    build: ./webapp
+    develop:
+      watch:
+        # sync static content
+        - path: ./webapp/html
+          target: /var/www
+          ignore:
+            - node_modules/
+
+`, nil), func(options *Options) {
+		options.ResolvePaths = false
+	})
+	assert.ErrorContains(t, err, "validating filename0.yml: services.frontend.develop.watch.0 action is required")
+}
+
 func TestBadServiceConfig(t *testing.T) {
 	yaml := `name: scratch
 services:

diff --git a/loader/validate.go b/loader/validate.go
@@ -139,6 +139,15 @@ func checkConsistency(project *types.Project) error {
 			return fmt.Errorf("services.%s: can't set container_name and %s as container name must be unique: %w", attr,
 				s.Name, errdefs.ErrInvalid)
 		}
+
+		if s.Develop != nil && s.Develop.Watch != nil {
+			for _, watch := range s.Develop.Watch {
+				if watch.Action != types.WatchActionRebuild && watch.Target == "" {
+					return fmt.Errorf("services.%s.develop.watch: target is required for non-rebuild actions: %w", s.Name, errdefs.ErrInvalid)
+				}
+			}
+
+		}
 	}
 
 	for name, secret := range project.Secrets {

diff --git a/loader/validate_test.go b/loader/validate_test.go
@@ -279,3 +279,91 @@ func TestValidateContainerName(t *testing.T) {
 	err := checkConsistency(&project)
 	assert.Assert(t, strings.Contains(err.Error(), `container name "mycontainer" is already in use by`))
 }
+
+func TestValidateWatch(t *testing.T) {
+	t.Run("watch valid configuration", func(t *testing.T) {
+		project := types.Project{
+			Services: types.Services{
+				"myservice": {
+					Name:  "myservice",
+					Image: "scratch",
+					Develop: &types.DevelopConfig{
+						Watch: []types.Trigger{
+							{
+								Action: types.WatchActionSync,
+								Path:   "/app",
+								Target: "/container/app",
+							},
+						},
+					},
+				},
+			},
+		}
+		err := checkConsistency(&project)
+		assert.NilError(t, err)
+
+	})
+
+	t.Run("watch missing target for sync action", func(t *testing.T) {
+		project := types.Project{
+			Services: types.Services{
+				"myservice": {
+					Name:  "myservice",
+					Image: "scratch",
+					Develop: &types.DevelopConfig{
+						Watch: []types.Trigger{
+							{
+								Action: types.WatchActionSync,
+								Path:   "/app",
+							},
+						},
+					},
+				},
+			},
+		}
+		err := checkConsistency(&project)
+		assert.Error(t, err, "services.myservice.develop.watch: target is required for non-rebuild actions: invalid compose project")
+	})
+
+	t.Run("watch missing target for sync+restart action", func(t *testing.T) {
+		project := types.Project{
+			Services: types.Services{
+				"myservice": {
+					Name:  "myservice",
+					Image: "scratch",
+					Develop: &types.DevelopConfig{
+						Watch: []types.Trigger{
+							{
+								Action: types.WatchActionSyncRestart,
+								Path:   "/app",
+							},
+						},
+					},
+				},
+			},
+		}
+		err := checkConsistency(&project)
+		assert.Error(t, err, "services.myservice.develop.watch: target is required for non-rebuild actions: invalid compose project")
+	})
+
+	t.Run("watch config valid with missing target for rebuild action", func(t *testing.T) {
+		project := types.Project{
+			Services: types.Services{
+				"myservice": {
+					Name:  "myservice",
+					Image: "scratch",
+					Develop: &types.DevelopConfig{
+						Watch: []types.Trigger{
+							{
+								Action: types.WatchActionRebuild,
+								Path:   "/app",
+							},
+						},
+					},
+				},
+			},
+		}
+		err := checkConsistency(&project)
+		assert.NilError(t, err)
+	})
+}
diff --git a/schema/compose-spec.json b/schema/compose-spec.json
@@ -455,14 +455,14 @@
           "type": "array",
           "items": {
             "type": "object",
+            "required": ["path", "action"],
             "properties": {
               "ignore": {"type": "array", "items": {"type": "string"}},
               "path": {"type": "string"},
               "action": {"type": "string", "enum": ["rebuild", "sync", "sync+restart"]},
               "target": {"type": "string"}
             }
           },
-          "required": ["path", "action"],
           "additionalProperties": false,
           "patternProperties": {"^x-": {}}
         }

diff --git a/types/develop.go b/types/develop.go
@@ -17,7 +17,7 @@
 package types
 
 type DevelopConfig struct {
-	Watch []Trigger `json:"watch,omitempty"`
+	Watch []Trigger `yaml:"watch,omitempty" json:"watch,omitempty"`
 
 	Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"`
 }
@@ -31,8 +31,8 @@ const (
 )
 
 type Trigger struct {
-	Path   string      `json:"path,omitempty"`
-	Action WatchAction `json:"action,omitempty"`
-	Target string      `json:"target,omitempty"`
-	Ignore []string    `json:"ignore,omitempty"`
+	Path   string      `yaml:"path" json:"path"`
+	Action WatchAction `yaml:"action" json:"action"`
+	Target string      `yaml:"target,omitempty" json:"target,omitempty"`
+	Ignore []string    `yaml:"ignore,omitempty" json:"ignore,omitempty"`
 }