Update atenet component to use actor *name* instead of *id*.

Author: juli4n

cmd/ateapi/internal/controlapi/list_actors.go

@@ -48,10 +48,8 @@ func (s *Service) ListActors(ctx context.Context, req *ateapipb.ListActorsReques
 func validateListActorsRequest(req *ateapipb.ListActorsRequest) error {
 	// An empty atespace is allowed here and means "all atespaces"(used by `kubectl ate get actors -A`).
 	// A non-empty atespace is validated and scopes the listing to that atespace.
-	if req.GetAtespace() != "" {
-		if err := resources.ValidateAtespace(req.GetAtespace()); err != nil {
-			return err
-		}
+	if req.GetAtespace() != "" && !resources.IsValidResourceName(req.GetAtespace()) {
+		return fmt.Errorf("invalid atespace %q: must be a valid resource name", req.GetAtespace())
 	}
 	pageSize := req.GetPageSize()
 	if pageSize < 0 {

cmd/atenet/internal/dns/corefile.go

@@ -39,12 +39,12 @@ func buildTemplate() string {
 	directives = append(directives, "ready :8181")
 	directives = append(directives, "reload")
 
-	// Construct match pattern for <ActorID>.<atespace>.<dnsDomain>. Both the
-	// actor id and the atespace are DNS-1123 labels (same regex).
+	// Construct match pattern for <ActorName>.<atespace>.<dnsDomain>. Both the
+	// actor name and the atespace are DNS-1123 labels (same regex).
 	directives = append(directives, fmt.Sprintf("template IN A %s {", resources.ActorDNSSuffix))
 	// Escape the suffix's dots so they match literally; the final \. matches the FQDN's trailing dot.
 	escapedSuffix := strings.ReplaceAll(resources.ActorDNSSuffix, ".", `\.`)
-	directives = append(directives, fmt.Sprintf(`  match "^%s\.%s\.%s\.$"`, resources.ActorIDRegexPattern, resources.ActorIDRegexPattern, escapedSuffix))
+	directives = append(directives, fmt.Sprintf(`  match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix))
 	// Note the %s -- this will be filled with the router IP.
 	directives = append(directives, `  answer "{{ .Name }} 60 IN A %s"`)
 	directives = append(directives, "}")

cmd/atenet/internal/dns/corefile_test.go

@@ -38,7 +38,7 @@ func TestMakeCoreFile(t *testing.T) {
 				"ready :8181",
 				"reload",
 				"template IN A actors.resources.substrate.ate.dev {",
-				`match "^` + resources.ActorIDRegexPattern + `\.` + resources.ActorIDRegexPattern + `\.actors\.resources\.substrate\.ate\.dev\.$"`,
+				`match "^` + resources.ResourceNameRegexPattern + `\.` + resources.ResourceNameRegexPattern + `\.actors\.resources\.substrate\.ate\.dev\.$"`,
 				`answer "{{ .Name }} 60 IN A 10.240.0.10"`,
 			},
 		},

cmd/atenet/internal/router/errors.go

@@ -32,8 +32,8 @@ func newReqError(code envoy_type.StatusCode, format string, args ...any) error {
 }
 
 // actorNotFoundErr returns a 404 reqError identifying the missing actor.
-func actorNotFoundErr(actorID string) error {
-	return newReqError(envoy_type.StatusCode_NotFound, "actor %q not found", actorID)
+func actorNotFoundErr(actorName string) error {
+	return newReqError(envoy_type.StatusCode_NotFound, "actor %q not found", actorName)
 }
 
 // invalidHostErr returns a 404 reqError explaining why the request host was
@@ -53,7 +53,7 @@ func invalidHostErr(host string, cause error) error {
 //
 // Unrecognized errors collapse to 500 with a generic body to avoid leaking
 // server-side detail (stack traces, internal IDs) to clients.
-func mapResumeError(actorID string, err error) error {
+func mapResumeError(actorName string, err error) error {
 	if err == nil {
 		return nil
 	}
@@ -62,31 +62,31 @@ func mapResumeError(actorID string, err error) error {
 	switch status.Code(err) {
 	case codes.NotFound:
 		re.statusCode = int(envoy_type.StatusCode_NotFound)
-		re.msg = fmt.Sprintf("actor %q not found", actorID)
+		re.msg = fmt.Sprintf("actor %q not found", actorName)
 	case codes.FailedPrecondition:
 		// Preserve the gRPC description for FailedPrecondition only: it carries
 		// actionable client-facing context (e.g. "no free workers available")
 		// and is not security-sensitive.
 		re.statusCode = int(envoy_type.StatusCode_ServiceUnavailable)
-		re.msg = fmt.Sprintf("actor %q unavailable: %s", actorID, status.Convert(err).Message())
+		re.msg = fmt.Sprintf("actor %q unavailable: %s", actorName, status.Convert(err).Message())
 	case codes.Unavailable:
 		re.statusCode = int(envoy_type.StatusCode_ServiceUnavailable)
-		re.msg = fmt.Sprintf("actor %q unavailable", actorID)
+		re.msg = fmt.Sprintf("actor %q unavailable", actorName)
 	case codes.DeadlineExceeded:
 		re.statusCode = int(envoy_type.StatusCode_GatewayTimeout)
-		re.msg = fmt.Sprintf("actor %q request timed out", actorID)
+		re.msg = fmt.Sprintf("actor %q request timed out", actorName)
 	case codes.PermissionDenied:
 		re.statusCode = int(envoy_type.StatusCode_Forbidden)
-		re.msg = fmt.Sprintf("actor %q access denied", actorID)
+		re.msg = fmt.Sprintf("actor %q access denied", actorName)
 	case codes.Unauthenticated:
 		re.statusCode = int(envoy_type.StatusCode_Unauthorized)
-		re.msg = fmt.Sprintf("actor %q authentication required", actorID)
+		re.msg = fmt.Sprintf("actor %q authentication required", actorName)
 	case codes.ResourceExhausted:
 		re.statusCode = int(envoy_type.StatusCode_TooManyRequests)
-		re.msg = fmt.Sprintf("actor %q rate limited", actorID)
+		re.msg = fmt.Sprintf("actor %q rate limited", actorName)
 	default:
 		re.statusCode = int(envoy_type.StatusCode_InternalServerError)
-		re.msg = fmt.Sprintf("error resuming actor %q", actorID)
+		re.msg = fmt.Sprintf("error resuming actor %q", actorName)
 	}
 	return re
 }

cmd/atenet/internal/router/errors_test.go

@@ -82,7 +82,7 @@ func TestInvalidHostErr(t *testing.T) {
 func TestMapResumeError(t *testing.T) {
 	t.Parallel()
 
-	const actorID = "ctr6"
+	const actorName = "ctr6"
 
 	tests := []struct {
 		name     string
@@ -150,7 +150,7 @@ func TestMapResumeError(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			got := mapResumeError(actorID, tc.err)
+			got := mapResumeError(actorName, tc.err)
 			if got == nil {
 				t.Fatal("mapResumeError returned nil")
 			}

cmd/atenet/internal/router/extproc.go

@@ -142,16 +142,16 @@ func (s *ExtProcServer) handleRequestHeaders(
 	ctx, span := otel.Tracer(routerServiceName).Start(ctx, "ExtProc.RequestHeaders")
 	defer span.End()
 
-	atespace, actorID, err := parseActorRef(metadata.host)
+	atespace, actorName, err := parseActorRef(metadata.host)
 	if err != nil {
 		// Host is invalid, respond with 404.
 		return nil, metadata, "", "", "", invalidHostErr(metadata.host, err)
 	}
 
-	slog.InfoContext(ctx, "ResumeActor", slog.String("atespace", atespace), slog.String("actorID", actorID))
-	actor, err := s.resumer.ResumeActor(ctx, atespace, actorID)
+	slog.InfoContext(ctx, "ResumeActor", slog.String("atespace", atespace), slog.String("actor", actorName))
+	actor, err := s.resumer.ResumeActor(ctx, atespace, actorName)
 	if err != nil {
-		return nil, metadata, "", "", "", mapResumeError(actorID, err)
+		return nil, metadata, "", "", "", mapResumeError(actorName, err)
 	}
 
 	// Actor template identity, used as low-cardinality route-latency metric
@@ -162,19 +162,19 @@ func (s *ExtProcServer) handleRequestHeaders(
 	workerIP := actor.GetAteomPodIp()
 	slog.InfoContext(ctx, "ResumeActor result",
 		slog.String("atespace", atespace),
-		slog.String("actorID", actorID),
+		slog.String("actor", actorName),
 		slog.String("status", actor.GetStatus().String()),
 		slog.String("workerIP", workerIP))
 
 	if ip := net.ParseIP(workerIP); ip == nil {
 		return nil, metadata, "", tmplNs, tmplName, newReqError(envoy_type.StatusCode_InternalServerError,
-			"actor %q routing failed", actorID)
+			"actor %q routing failed", actorName)
 	}
 
 	// TODO(bowei) -- handle more than port 80 on the actor.
 	targetAddr := net.JoinHostPort(workerIP, "80")
 
-	slog.InfoContext(ctx, "Route ok", slog.String("actorID", actorID), slog.String("targetAddr", targetAddr))
+	slog.InfoContext(ctx, "Route ok", slog.String("actor", actorName), slog.String("targetAddr", targetAddr))
 
 	// Route by rewriting the :authority header.
 	mutation := &extprocv3.HeaderMutation{}

cmd/atenet/internal/router/extproc_in.go

@@ -56,12 +56,12 @@ func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata {
 	}
 }
 
-// parseActorRef extracts the (atespace, actor id) an incoming request is
+// parseActorRef extracts the (atespace, actor name) an incoming request is
 // addressed to from its Host/:authority, which has the form
-// "<actor_id>.<atespace>.actors.resources.substrate.ate.dev" (optionally with a
-// port). The atespace is required because an actor id is only unique within its
+// "<actor_name>.<atespace>.actors.resources.substrate.ate.dev" (optionally with a
+// port). The atespace is required because an actor name is only unique within its
 // atespace.
-func parseActorRef(host string) (atespace, actorID string, err error) {
+func parseActorRef(host string) (atespace, actorName string, err error) {
 	if strings.Contains(host, ":") {
 		host, _, err = net.SplitHostPort(host)
 		if err != nil {

cmd/atenet/internal/router/resumer.go

@@ -42,16 +42,16 @@ func NewActorResumer(apiClient ateapipb.ControlClient) *ActorResumer {
 
 // ResumeActor ensures the requested actor is running. It deduplicates concurrent
 // requests within the process and retries when needed. The actor is addressed by
-// (atespace, actorID) since an actor id is only unique within its atespace.
-func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorID string) (*ateapipb.Actor, error) {
+// (atespace, actorName) since an actor name is only unique within its atespace.
+func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorName string) (*ateapipb.Actor, error) {
 	ctx, span := otel.Tracer(routerServiceName).Start(ctx, "ResumeActor",
 		trace.WithAttributes(
 			attribute.String("atespace", atespace),
-			attribute.String("actor_id", actorID),
+			attribute.String("actor", actorName),
 		))
 	defer span.End()
 
-	ch := r.flight.DoChan(atespace+"/"+actorID, func() (interface{}, error) {
+	ch := r.flight.DoChan(atespace+"/"+actorName, func() (interface{}, error) {
 		// We detach the context from the first caller using a fixed background timeout.
 		// This guarantees that if Caller 1 disconnects or times out, the underlying
 		// resume operation continues running for Caller 2 and Caller 3 without failing.
@@ -70,7 +70,7 @@ func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorID string
 		err := wait.ExponentialBackoffWithContext(bgCtx, backoff, func(ctx context.Context) (bool, error) {
 			var err error
 			resumeResp, err = r.apiClient.ResumeActor(ctx, &ateapipb.ResumeActorRequest{
-				Actor: &ateapipb.ObjectRef{Atespace: atespace, Name: actorID},
+				Actor: &ateapipb.ObjectRef{Atespace: atespace, Name: actorName},
 			})
 			if err == nil {
 				return true, nil

cmd/atenet/internal/router/resumer_test.go

@@ -39,7 +39,7 @@ func (m *resumerMockClient) ResumeActor(ctx context.Context, in *ateapipb.Resume
 }
 
 func TestActorResumer_ResumeActor(t *testing.T) {
-	const testActorID = "actor-a"
+	const testActorName = "actor-a"
 	const testAtespace = "team-a"
 	const expectedIP = "10.0.0.52"
 
@@ -50,7 +50,7 @@ func TestActorResumer_ResumeActor(t *testing.T) {
 				resumeCalled++
 				return &ateapipb.ResumeActorResponse{
 					Actor: &ateapipb.Actor{
-						Metadata:   &ateapipb.ResourceMetadata{Name: testActorID},
+						Metadata:   &ateapipb.ResourceMetadata{Name: testActorName},
 						Status:     ateapipb.Actor_STATUS_RUNNING,
 						AteomPodIp: expectedIP,
 					},
@@ -59,7 +59,7 @@ func TestActorResumer_ResumeActor(t *testing.T) {
 		}
 
 		resumer := NewActorResumer(mock)
-		actor, err := resumer.ResumeActor(context.Background(), testAtespace, testActorID)
+		actor, err := resumer.ResumeActor(context.Background(), testAtespace, testActorName)
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
@@ -81,7 +81,7 @@ func TestActorResumer_ResumeActor(t *testing.T) {
 				}
 				return &ateapipb.ResumeActorResponse{
 					Actor: &ateapipb.Actor{
-						Metadata:   &ateapipb.ResourceMetadata{Name: testActorID},
+						Metadata:   &ateapipb.ResourceMetadata{Name: testActorName},
 						Status:     ateapipb.Actor_STATUS_RUNNING,
 						AteomPodIp: expectedIP,
 					},
@@ -90,7 +90,7 @@ func TestActorResumer_ResumeActor(t *testing.T) {
 		}
 
 		resumer := NewActorResumer(mock)
-		actor, err := resumer.ResumeActor(context.Background(), testAtespace, testActorID)
+		actor, err := resumer.ResumeActor(context.Background(), testAtespace, testActorName)
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
@@ -110,7 +110,7 @@ func TestActorResumer_ResumeActor(t *testing.T) {
 		}
 
 		resumer := NewActorResumer(mock)
-		_, err := resumer.ResumeActor(context.Background(), testAtespace, testActorID)
+		_, err := resumer.ResumeActor(context.Background(), testAtespace, testActorName)
 		if got := status.Code(err); got != codes.NotFound {
 			t.Errorf("expected gRPC code NotFound, got %v (err=%v)", got, err)
 		}
@@ -128,7 +128,7 @@ func TestActorResumer_ResumeActor(t *testing.T) {
 				time.Sleep(20 * time.Millisecond)
 				return &ateapipb.ResumeActorResponse{
 					Actor: &ateapipb.Actor{
-						Metadata:   &ateapipb.ResourceMetadata{Name: testActorID},
+						Metadata:   &ateapipb.ResourceMetadata{Name: testActorName},
 						Status:     ateapipb.Actor_STATUS_RUNNING,
 						AteomPodIp: expectedIP,
 					},
@@ -147,7 +147,7 @@ func TestActorResumer_ResumeActor(t *testing.T) {
 		for i := 0; i < concurrentRequests; i++ {
 			go func(idx int) {
 				defer wg.Done()
-				results[idx], errs[idx] = resumer.ResumeActor(context.Background(), testAtespace, testActorID)
+				results[idx], errs[idx] = resumer.ResumeActor(context.Background(), testAtespace, testActorName)
 			}(i)
 		}
 		wg.Wait()

internal/resources/actor.go

@@ -16,80 +16,46 @@ package resources
 
 import (
 	"fmt"
-	"regexp"
 	"strings"
 )
 
 const (
-	// ActorIDRegexPattern is the regular expression pattern for matching valid actor IDs.
-	ActorIDRegexPattern = `[a-z0-9]([-a-z0-9]*[a-z0-9])?`
+	// ResourceNameRegexPattern is the regular expression pattern for a valid
+	// Substrate resource name.
+	ResourceNameRegexPattern = `[a-z0-9]([-a-z0-9]*[a-z0-9])?`
 	// ActorDNSSuffix is suffix to the DNS name for direct access to Actor
-	// "<actor id>.<atespace>.actors.resources.substrate.ate.dev."
+	// "<actor_name>.<atespace>.actors.resources.substrate.ate.dev."
 	ActorDNSSuffix = "actors.resources.substrate.ate.dev"
 	// GoldenActorAtespace is the reserved system atespace that per-template golden
 	// actors live in.
 	GoldenActorAtespace = "ate-golden"
 )
 
-var actorIDRegex = regexp.MustCompile("^" + ActorIDRegexPattern + "$")
-
-// TODO: unify actor/atespace validation across the control API RPCs — some only
-// reject empty strings (get/pause/resume/suspend), others run the full validator.
-
-// ValidateActorID validates whether the provided actor ID is valid or not.
-// Actor IDs must be valid DNS-1123 labels.
-//
-// 1. Must be between 1 and 63 characters in length.
-// 2. Must start with a lower-case alphanumeric character (a-z, 0-9).
-// 3. Must contain only lower-case alphanumeric characters and hyphens (a-z, 0-9, -).
-// 4. Must end with a lower-case alphanumeric character (cannot end with a hyphen).
-func ValidateActorID(id string) error {
-	if len(id) > 63 {
-		return fmt.Errorf("invalid actor_id: must be no more than 63 characters")
-	}
-	if !actorIDRegex.MatchString(id) {
-		return fmt.Errorf("invalid actor_id: must start and end with a lower case alphanumeric character, and consist only of lower case alphanumeric characters or '-'")
-	}
-	return nil
-}
-
-// ValidateAtespace validates whether the provided atespace name is valid. An
-// atespace must be a valid DNS-1123 label (same rules as an actor ID above).
-func ValidateAtespace(atespace string) error {
-	if len(atespace) > 63 {
-		return fmt.Errorf("invalid atespace: must be no more than 63 characters")
-	}
-	if !actorIDRegex.MatchString(atespace) {
-		return fmt.Errorf("invalid atespace: must start and end with a lower case alphanumeric character, and consist only of lower case alphanumeric characters or '-'")
-	}
-	return nil
-}
-
 // ActorDNSName returns the mesh DNS name an actor is reachable at:
-// "<actor_id>.<atespace>.actors.resources.substrate.ate.dev". The atespace is
-// part of the name because an actor id is only unique within its atespace.
-func ActorDNSName(atespace, actorID string) string {
-	return actorID + "." + atespace + "." + ActorDNSSuffix
+// "<actor_name>.<atespace>.actors.resources.substrate.ate.dev". The atespace is
+// part of the name because an actor name is only unique within its atespace.
+func ActorDNSName(atespace, actorName string) string {
+	return actorName + "." + atespace + "." + ActorDNSSuffix
 }
 
 // ParseActorDNSName parses a mesh DNS name of the form
-// "<actor_id>.<atespace>.actors.resources.substrate.ate.dev" (a trailing dot is
-// tolerated) into its atespace and actor id, validating both. It does not accept
-// a host:port; callers must strip the port first.
-func ParseActorDNSName(name string) (atespace, actorID string, err error) {
+// "<actor_name>.<atespace>.actors.resources.substrate.ate.dev" (a trailing dot
+// is tolerated) into its atespace and actor name, validating both. It does not
+// accept a host:port; callers must strip the port first.
+func ParseActorDNSName(name string) (atespace, actorName string, err error) {
 	rest, found := strings.CutSuffix(strings.TrimSuffix(name, "."), "."+ActorDNSSuffix)
 	if !found {
 		return "", "", fmt.Errorf("invalid actor DNS name: must end with %s, got %q", ActorDNSSuffix, name)
 	}
-	actorID, atespace, found = strings.Cut(rest, ".")
+	actorName, atespace, found = strings.Cut(rest, ".")
 	if !found {
-		return "", "", fmt.Errorf("invalid actor DNS name: expected <actor_id>.<atespace>.%s, got %q", ActorDNSSuffix, name)
+		return "", "", fmt.Errorf("invalid actor DNS name: expected <actor_name>.<atespace>.%s, got %q", ActorDNSSuffix, name)
 	}
-	if err := ValidateActorID(actorID); err != nil {
-		return "", "", err
+	if !IsValidResourceName(actorName) {
+		return "", "", fmt.Errorf("invalid actor DNS name %q: %q is not a valid actor name", name, actorName)
 	}
-	if err := ValidateAtespace(atespace); err != nil {
-		return "", "", err
+	if !IsValidResourceName(atespace) {
+		return "", "", fmt.Errorf("invalid actor DNS name %q: %q is not a valid atespace", name, atespace)
 	}
-	return atespace, actorID, nil
+	return atespace, actorName, nil
 }