Put Worker.actor fields into an "Assignment" message (agent-substrate#363)

This is a cleanup I noticed as I was reviewing other PRs. I tried to
keep the commits clean, so you can see them one by one.

Author: thockin

.golangci.yaml

@@ -53,7 +53,10 @@ linters:
         - (k8s.io/client-go/tools/cache.SharedIndexInformer).AddIndexers
 
     misspell:
-      mode: default
+      locale: US
+      mode: restricted # comments only
+      ignore-rules:
+        - cancelled
 
   exclusions:
     # Built-in presets that suppress the most common idiomatic false

cmd/ateapi/internal/controlapi/create_actor.go

@@ -18,15 +18,15 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"strings"
 
 	"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
 	"github.com/agent-substrate/substrate/internal/resources"
 	"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/util/validation"
+	"k8s.io/apimachinery/pkg/api/validate/content"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest) (*ateapipb.CreateActorResponse, error) {
@@ -79,42 +79,59 @@ func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequ
 }
 
 func validateCreateActorRequest(req *ateapipb.CreateActorRequest) error {
-	if req.GetActorTemplateNamespace() == "" {
-		return status.Error(codes.InvalidArgument, "actor_template_namespace is required")
-	}
-	if req.GetActorTemplateName() == "" {
-		return status.Error(codes.InvalidArgument, "actor_template_name is required")
-	}
-	if req.GetActorRef().GetName() == "" {
-		return status.Error(codes.InvalidArgument, "actor_id is required")
+	var fldPath *field.Path
+	var errs field.ErrorList
+
+	if val, fldPath := req.ActorTemplateNamespace, fldPath.Child("actor_template_namespace"); val == "" {
+		errs = append(errs, field.Required(fldPath, ""))
+	} else {
+		for _, msg := range content.IsDNS1123Label(val) {
+			errs = append(errs, field.Invalid(fldPath, val, msg))
+		}
 	}
-	if err := resources.ValidateActorID(req.GetActorRef().GetName()); err != nil {
-		return status.Error(codes.InvalidArgument, err.Error())
+
+	if val, fldPath := req.ActorTemplateName, fldPath.Child("actor_template_name"); val == "" {
+		errs = append(errs, field.Required(fldPath, ""))
+	} else {
+		for _, msg := range content.IsDNS1123Subdomain(val) {
+			errs = append(errs, field.Invalid(fldPath, val, msg))
+		}
 	}
-	if req.GetActorRef().GetAtespace() == "" {
-		return status.Error(codes.InvalidArgument, "atespace is required")
+
+	if val, fldPath := req.ActorRef, fldPath.Child("actor_ref"); val == nil {
+		errs = append(errs, field.Required(fldPath, ""))
+	} else {
+		errs = append(errs, resources.ValidateActorRef(val, fldPath)...)
 	}
-	if err := resources.ValidateAtespace(req.GetActorRef().GetAtespace()); err != nil {
-		return status.Error(codes.InvalidArgument, err.Error())
+
+	if val := req.WorkerSelector; val != nil {
+		errs = append(errs, validateSelector(val, fldPath.Child("worker_selector"))...)
 	}
-	if err := validateSelector(req.GetWorkerSelector()); err != nil {
-		return status.Error(codes.InvalidArgument, err.Error())
+
+	if len(errs) > 0 {
+		return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
 	}
 	return nil
 }
 
-func validateSelector(sel *ateapipb.Selector) error {
-	const maxSelectorMatchLabels = 10
-	if n := len(sel.GetMatchLabels()); n > maxSelectorMatchLabels {
-		return fmt.Errorf("worker_selector has %d match_labels entries, exceeding the limit of %d", n, maxSelectorMatchLabels)
-	}
-	for k, v := range sel.GetMatchLabels() {
-		if errs := validation.IsQualifiedName(k); len(errs) > 0 {
-			return fmt.Errorf("invalid worker_selector label key %q: %s", k, strings.Join(errs, "; "))
+func validateSelector(sel *ateapipb.Selector, fldPath *field.Path) field.ErrorList {
+	var errs field.ErrorList
+
+	if sel.MatchLabels != nil {
+		const maxSelectorMatchLabels = 10
+		if n := len(sel.MatchLabels); n > maxSelectorMatchLabels {
+			return field.ErrorList{field.TooMany(fldPath.Child("match_labels"), n, maxSelectorMatchLabels)}
 		}
-		if errs := validation.IsValidLabelValue(v); len(errs) > 0 {
-			return fmt.Errorf("invalid worker_selector label value %q for key %q: %s", v, k, strings.Join(errs, "; "))
+
+		for k, v := range sel.MatchLabels {
+			for _, msg := range content.IsLabelKey(k) {
+				errs = append(errs, field.Invalid(fldPath.Child("match_labels").Key(k), k, msg))
+			}
+			for _, msg := range content.IsLabelValue(v) {
+				errs = append(errs, field.Invalid(fldPath.Child("match_labels").Key(k), v, msg))
+			}
 		}
 	}
-	return nil
+
+	return errs
 }

cmd/ateapi/internal/controlapi/delete_actor.go

@@ -24,6 +24,7 @@ import (
 	"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequest) (*ateapipb.DeleteActorResponse, error) {
@@ -52,14 +53,17 @@ func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequ
 }
 
 func validateDeleteActorRequest(req *ateapipb.DeleteActorRequest) error {
-	if req.GetActorRef().GetName() == "" {
-		return status.Error(codes.InvalidArgument, "actor_id is required")
-	}
-	if err := resources.ValidateActorID(req.GetActorRef().GetName()); err != nil {
-		return status.Error(codes.InvalidArgument, err.Error())
+	var fldPath *field.Path
+	var errs field.ErrorList
+
+	if val, fldPath := req.ActorRef, fldPath.Child("actor_ref"); val == nil {
+		errs = append(errs, field.Required(fldPath, ""))
+	} else {
+		errs = append(errs, resources.ValidateActorRef(val, fldPath)...)
 	}
-	if req.GetActorRef().GetAtespace() == "" {
-		return status.Error(codes.InvalidArgument, "atespace is required")
+
+	if len(errs) > 0 {
+		return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
 	}
 	return nil
 }