fix(labels): allow setting 'uncloud.' prefixed labels on services using a client but not CLI or Compose

pkg/api/service.go

@@ -78,8 +78,8 @@ type ServiceSpec struct {
 	Configs []ConfigSpec
 	// Container defines the desired state of each container in the service.
 	Container ContainerSpec
-	// Labels are user-defined key-value metadata attached to the service. Unlike container labels, they're not
-	// applied to Docker containers. Keys must not use the reserved "uncloud." prefix.
+	// Labels are key-value metadata attached to the service. Unlike container labels, they're not applied to Docker
+	// containers. The "uncloud." key prefix is reserved for labels set by Uncloud.
 	Labels map[string]string
 	// Mode is the replication mode of the service. Default is ServiceModeReplicated if empty.
 	Mode string
@@ -161,7 +161,9 @@ func (s *ServiceSpec) Validate() error {
 	if err := s.Container.Validate(); err != nil {
 		return err
 	}
-	if err := ValidateLabels(s.Labels); err != nil {
+	// We want to allow reserved uncloud. labels in the service spec, because we can't distinguish between user-defined
+	// in a Compose file and those set by a uc command.
+	if err := ValidateLabelKeys(s.Labels); err != nil {
 		return fmt.Errorf("invalid service labels: %w", err)
 	}
 
@@ -444,9 +446,8 @@ func (s *ContainerSpec) Clone() ContainerSpec {
 	return spec
 }
 
-// ValidateLabels checks that label keys are non-empty, don't contain whitespace, and don't use the reserved Uncloud
-// prefixes.
-func ValidateLabels(labels map[string]string) error {
+// ValidateLabelKeys checks that label keys are non-empty and don't contain whitespace.
+func ValidateLabelKeys(labels map[string]string) error {
 	for k := range labels {
 		if strings.TrimSpace(k) == "" {
 			return fmt.Errorf("label key must not be empty")
@@ -454,6 +455,17 @@ func ValidateLabels(labels map[string]string) error {
 		if strings.ContainsFunc(k, unicode.IsSpace) {
 			return fmt.Errorf("label '%s' must not contain whitespace", k)
 		}
+	}
+	return nil
+}
+
+// ValidateLabels checks that label keys are valid and don't use the reserved Uncloud prefixes. Use it for labels
+// provided by users and for container labels.
+func ValidateLabels(labels map[string]string) error {
+	if err := ValidateLabelKeys(labels); err != nil {
+		return err
+	}
+	for k := range labels {
 		if strings.HasPrefix(k, LabelReservedPrefix) || strings.HasPrefix(k, LabelDaemonReservedPrefix) {
 			return fmt.Errorf("label '%s' cannot use the reserved prefix '%s' or '%s'",
 				k, LabelReservedPrefix, LabelDaemonReservedPrefix)

pkg/api/service_test.go

@@ -82,8 +82,6 @@ func TestServiceSpec_Validate_Labels(t *testing.T) {
 		{name: "trailing space in key", labels: map[string]string{"tier ": "bar"}, wantErr: "must not contain whitespace"},
 		{name: "tab in key", labels: map[string]string{"my\tlabel": "bar"}, wantErr: "must not contain whitespace"},
 		{name: "newline in key", labels: map[string]string{"my\nlabel": "bar"}, wantErr: "must not contain whitespace"},
-		{name: "reserved prefix", labels: map[string]string{LabelManaged: ""}, wantErr: "reserved prefix"},
-		{name: "reserved daemon prefix", labels: map[string]string{"uncloudd.foo": "bar"}, wantErr: "reserved prefix"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name+"/service", func(t *testing.T) {
@@ -111,6 +109,38 @@ func TestServiceSpec_Validate_Labels(t *testing.T) {
 	}
 }
 
+func TestServiceSpec_Validate_ReservedLabels(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name   string
+		labels map[string]string
+	}{
+		{name: "reserved prefix", labels: map[string]string{LabelManaged: ""}},
+		{name: "reserved daemon prefix", labels: map[string]string{"uncloudd.foo": "bar"}},
+		{name: "managed service", labels: map[string]string{
+			LabelReservedPrefix + "service.managed": "",
+			LabelReservedPrefix + "custom.key":      "value",
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			// Uncloud sets its own labels on services, so a service spec can have them.
+			spec := ServiceSpec{Container: ContainerSpec{Image: "nginx:latest"}, Labels: tt.labels}
+			require.NoError(t, spec.Validate())
+
+			// Container labels and labels provided by users can't use the reserved prefixes.
+			spec = ServiceSpec{Container: ContainerSpec{Image: "nginx:latest", Labels: tt.labels}}
+			err := spec.Validate()
+			require.ErrorContains(t, err, "invalid container labels")
+			require.ErrorContains(t, err, "reserved prefix")
+			require.ErrorContains(t, ValidateLabels(tt.labels), "reserved prefix")
+		})
+	}
+}
+
 func TestServiceSpec_Validate_CaddyAndPorts(t *testing.T) {
 	tests := []struct {
 		name    string

pkg/client/compose/service.go

@@ -130,6 +130,9 @@ func ServiceSpecFromCompose(project *types.Project, serviceName string) (api.Ser
 		}
 
 		if len(service.Deploy.Labels) > 0 {
+			if err = api.ValidateLabels(service.Deploy.Labels); err != nil {
+				return spec, fmt.Errorf("invalid deploy.labels: %w", err)
+			}
 			spec.Labels = maps.Clone(service.Deploy.Labels)
 		}
 	}

pkg/client/compose/service_test.go

@@ -950,6 +950,42 @@ services:
 	}
 }
 
+func TestServiceSpecFromCompose_InvalidServiceLabels(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		label   string
+		wantErr string
+	}{
+		{name: "reserved prefix", label: api.LabelManaged, wantErr: "reserved prefix"},
+		{name: "reserved daemon prefix", label: "uncloudd.foo", wantErr: "reserved prefix"},
+		{name: "managed service", label: api.LabelReservedPrefix + "service.managed", wantErr: "reserved prefix"},
+		{name: "whitespace in key", label: "my label", wantErr: "must not contain whitespace"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			composeYAML := `
+services:
+  web:
+    image: nginx
+    deploy:
+      labels:
+        - ` + tt.label + `=value
+`
+			project, err := LoadProjectFromContent(context.Background(), composeYAML)
+			require.NoError(t, err)
+
+			_, err = ServiceSpecFromCompose(project, "web")
+			require.ErrorContains(t, err, "invalid deploy.labels")
+			require.ErrorContains(t, err, tt.wantErr)
+		})
+	}
+}
+
 func TestServiceSpecFromCompose_UpdateConfig(t *testing.T) {
 	t.Parallel()