Rework Create endpoints to align with API style guide.

* Take resource message in the request.
* Return resource message.

Author: juli4n

cmd/ateapi/internal/controlapi/create_actor.go

@@ -29,37 +29,44 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
-func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest) (*ateapipb.CreateActorResponse, error) {
+func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest) (*ateapipb.Actor, error) {
 	if err := validateCreateActorRequest(req); err != nil {
 		return nil, err
 	}
-	_, err := s.actorTemplateLister.ActorTemplates(req.GetActorTemplateNamespace()).Get(req.GetActorTemplateName())
+
+	in := req.GetActor()
+	templateNamespace := in.GetActorTemplateNamespace()
+	templateName := in.GetActorTemplateName()
+
+	_, err := s.actorTemplateLister.ActorTemplates(templateNamespace).Get(templateName)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
-			return nil, status.Errorf(codes.FailedPrecondition, "ActorTemplate %s/%s not found", req.GetActorTemplateNamespace(), req.GetActorTemplateName())
+			return nil, status.Errorf(codes.FailedPrecondition, "ActorTemplate %s/%s not found", templateNamespace, templateName)
 		}
 		return nil, fmt.Errorf("while getting ActorTemplate: %w", err)
 	}
 
+	atespace := in.GetMetadata().GetAtespace()
+	name := in.GetMetadata().GetName()
+
 	// The atespace must already exist.
-	exists, err := s.persistence.AtespaceExists(ctx, req.GetActor().GetAtespace())
+	exists, err := s.persistence.AtespaceExists(ctx, atespace)
 	if err != nil {
 		return nil, fmt.Errorf("while checking atespace: %w", err)
 	}
 	if !exists {
-		return nil, status.Errorf(codes.FailedPrecondition, "Atespace %s not found", req.GetActor().GetAtespace())
+		return nil, status.Errorf(codes.FailedPrecondition, "Atespace %s not found", atespace)
 	}
 
-	name := req.GetActor().GetName()
 	actor := &ateapipb.Actor{
 		Metadata: &ateapipb.ResourceMetadata{
-			Atespace: req.GetActor().GetAtespace(),
+			Atespace: atespace,
 			Name:     name,
 		},
 		Status:                 ateapipb.Actor_STATUS_SUSPENDED,
-		ActorTemplateNamespace: req.GetActorTemplateNamespace(),
-		ActorTemplateName:      req.GetActorTemplateName(),
-		WorkerSelector:         req.GetWorkerSelector(),
+		ActorTemplateNamespace: templateNamespace,
+		ActorTemplateName:      templateName,
+		WorkerSelector:         in.GetWorkerSelector(),
 	}
 	stored, err := s.persistence.CreateActor(ctx, actor)
 	if err != nil {
@@ -69,39 +76,49 @@ func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequ
 		return nil, fmt.Errorf("while recording actor: %w", err)
 	}
 
-	return &ateapipb.CreateActorResponse{
-		Actor: stored,
-	}, nil
+	return stored, nil
 }
 
 func validateCreateActorRequest(req *ateapipb.CreateActorRequest) error {
 	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, ""))
+	actor := req.GetActor()
+	actorPath := fldPath.Child("actor")
+	if actor == nil {
+		errs = append(errs, field.Required(actorPath, ""))
+		return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
+	}
+
+	metaPath := actorPath.Child("metadata")
+	if val, p := actor.GetMetadata().GetAtespace(), metaPath.Child("atespace"); val == "" {
+		errs = append(errs, field.Required(p, ""))
 	} else {
-		for _, msg := range content.IsDNS1123Label(val) {
-			errs = append(errs, field.Invalid(fldPath, val, msg))
-		}
+		errs = append(errs, resources.ValidateResourceName(val, p)...)
+	}
+	if val, p := actor.GetMetadata().GetName(), metaPath.Child("name"); val == "" {
+		errs = append(errs, field.Required(p, ""))
+	} else {
+		errs = append(errs, resources.ValidateResourceName(val, p)...)
 	}
 
-	if val, fldPath := req.ActorTemplateName, fldPath.Child("actor_template_name"); val == "" {
-		errs = append(errs, field.Required(fldPath, ""))
+	if val, p := actor.GetActorTemplateNamespace(), actorPath.Child("actor_template_namespace"); val == "" {
+		errs = append(errs, field.Required(p, ""))
 	} else {
-		for _, msg := range content.IsDNS1123Subdomain(val) {
-			errs = append(errs, field.Invalid(fldPath, val, msg))
+		for _, msg := range content.IsDNS1123Label(val) {
+			errs = append(errs, field.Invalid(p, val, msg))
 		}
 	}
-
-	if val, fldPath := req.Actor, fldPath.Child("actor"); val == nil {
-		errs = append(errs, field.Required(fldPath, ""))
+	if val, p := actor.GetActorTemplateName(), actorPath.Child("actor_template_name"); val == "" {
+		errs = append(errs, field.Required(p, ""))
 	} else {
-		errs = append(errs, resources.ValidateObjectRef(val, fldPath)...)
+		for _, msg := range content.IsDNS1123Subdomain(val) {
+			errs = append(errs, field.Invalid(p, val, msg))
+		}
 	}
 
-	if val := req.WorkerSelector; val != nil {
-		errs = append(errs, validateSelector(val, fldPath.Child("worker_selector"))...)
+	if val := actor.GetWorkerSelector(); val != nil {
+		errs = append(errs, validateSelector(val, actorPath.Child("worker_selector"))...)
 	}
 
 	if len(errs) > 0 {

cmd/ateapi/internal/controlapi/create_atespace.go

@@ -24,33 +24,55 @@ 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) CreateAtespace(ctx context.Context, req *ateapipb.CreateAtespaceRequest) (*ateapipb.CreateAtespaceResponse, error) {
+func (s *Service) CreateAtespace(ctx context.Context, req *ateapipb.CreateAtespaceRequest) (*ateapipb.Atespace, error) {
 	if err := validateCreateAtespaceRequest(req); err != nil {
 		return nil, err
 	}
 
+	name := req.GetAtespace().GetMetadata().GetName()
 	atespace := &ateapipb.Atespace{
-		Metadata: &ateapipb.ResourceMetadata{Name: req.GetName()},
+		Metadata: &ateapipb.ResourceMetadata{
+			Name: name,
+		},
 	}
 	stored, err := s.persistence.CreateAtespace(ctx, atespace)
 	if err != nil {
 		if errors.Is(err, store.ErrAlreadyExists) {
-			return nil, status.Errorf(codes.AlreadyExists, "Atespace %s already exists", req.GetName())
+			return nil, status.Errorf(codes.AlreadyExists, "Atespace %s already exists", name)
 		}
 		return nil, fmt.Errorf("while recording atespace: %w", err)
 	}
 
-	return &ateapipb.CreateAtespaceResponse{Atespace: stored}, nil
+	return stored, nil
 }
 
 func validateCreateAtespaceRequest(req *ateapipb.CreateAtespaceRequest) error {
-	if req.GetName() == "" {
-		return status.Error(codes.InvalidArgument, "name is required")
+	var fldPath *field.Path
+	var errs field.ErrorList
+
+	atespace := req.GetAtespace()
+	atespacePath := fldPath.Child("atespace")
+	if atespace == nil {
+		errs = append(errs, field.Required(atespacePath, ""))
+		return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
+	}
+
+	// Atespace is global-scoped: metadata.atespace must be empty, name required + valid.
+	metaPath := atespacePath.Child("metadata")
+	if val, p := atespace.GetMetadata().GetAtespace(), metaPath.Child("atespace"); val != "" {
+		errs = append(errs, field.Invalid(p, val, "must be empty for a global-scoped resource"))
 	}
-	if err := resources.ValidateAtespace(req.GetName()); err != nil {
-		return status.Error(codes.InvalidArgument, err.Error())
+	if val, p := atespace.GetMetadata().GetName(), metaPath.Child("name"); val == "" {
+		errs = append(errs, field.Required(p, ""))
+	} else {
+		errs = append(errs, resources.ValidateResourceName(val, p)...)
+	}
+
+	if len(errs) > 0 {
+		return status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
 	}
 	return nil
 }