refactor(agent): built-in per-type env mapping; fix start timeout, orphan, guards

Replace the user-facing env_map mistake with a built-in env convention keyed by resource type + provider — users never see/edit env names; they only fill plain resource fields. Supports git (gitea/github/gitlab), vm, database (mysql/pg/redis/mongo, with alias normalisation), storage (azure blob / bucket). Lenient: missing optional fields are skipped; only unsupported type or a KV read failure errors. Other issues found in review and fixed: - start timeout: template-agent start now uses a longer timeout (default 60s, AGENT_RUNTIME_START_TIMEOUT_SECONDS) since AM provisions synchronously — 5s would time out. amTemplateDo takes a per-call timeout. - orphan agent: if AM start succeeds but the Manager record fails to persist, the orphan is rolled back (best-effort amDeleteTemplateAgent). - findUserTemplateAgent now guards template_id<>'' so the new endpoints can't touch a legacy task deployment. - binding_ids defaults to [] (not null). - removed ResourceBinding.EnvMap field entirely. Tests rewritten for the built-in convention (blob metadata-only, db provider prefixes incl pg/mg aliases, git provider-agnostic names, ownership, unsupported type, empty); adapter round-trip + router tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-03 22:48:29 +08:00
parent 65a71f6b35
commit 19fd4f3326
5 changed files with 247 additions and 102 deletions
@@ -8,27 +8,138 @@ import (
 	"github.com/heicode/manager/model"
 )

-// envMapEntry describes how one environment variable is sourced from a resource
-// binding when starting a template agent.
+// envField maps one environment variable a template agent receives from a bound
+// resource. Source is envMeta (non-secret, from the binding's Metadata JSON) or
+// envSecret (resolved from Key Vault at start only). Key is the field name
+// inside that JSON.
 //
-//	Source == "metadata" -> value taken from the binding's non-secret Metadata JSON
-//	Source == "secret"   -> value resolved from Key Vault (binding.SecretRef) at
-//	                        start time only; never persisted in plaintext.
+// These env names are the HM<->AM contract: the agent template reads them.
+// Users NEVER see or edit env names — they only fill plain resource fields
+// (repo url, token, host, password, ...) in the web console.
+type envField struct {
+	Env    string
+	Source string
+	Key    string
+}
+
+const (
+	envMeta   = "metadata"
+	envSecret = "secret"
+)
+
+// normalizeProvider folds provider aliases to a canonical key.
+func normalizeProvider(p string) string {
+	switch strings.ToLower(strings.TrimSpace(p)) {
+	case "postgres", "postgresql", "pg":
+		return "pg"
+	case "mysql", "mariadb":
+		return "mysql"
+	case "redis":
+		return "redis"
+	case "mongo", "mongodb", "mg":
+		return "mongo"
+	case "github":
+		return "github"
+	case "gitea":
+		return "gitea"
+	case "gitlab":
+		return "gitlab"
+	case "blob", "azureblob", "azure-blob":
+		return "blob"
+	case "bucket", "s3", "oss", "minio", "cos":
+		return "bucket"
+	default:
+		return strings.ToLower(strings.TrimSpace(p))
+	}
+}
+
+func dbEnvSpec(prefix string) []envField {
+	return []envField{
+		{prefix + "_HOST", envMeta, "host"},
+		{prefix + "_PORT", envMeta, "port"},
+		{prefix + "_DATABASE", envMeta, "database"},
+		{prefix + "_USER", envMeta, "user"},
+		{prefix + "_PASSWORD", envSecret, "password"},
+	}
+}
+
+// builtinEnvSpec returns the fixed env mapping for a binding, keyed by resource
+// type and provider. Built into HM; not user-configurable.
 //
-// EnvMap on a ResourceBinding is a JSON object: {"ENV_NAME": {"source": "...",
-// "key": "<field in metadata or secret json>"}}.
-type envMapEntry struct {
-	Source string `json:"source"`
-	Key    string `json:"key"`
+// Supported: git (gitea/github/gitlab), vm, database (mysql/pg/redis/mongo),
+// storage (azure blob / object-storage bucket).
+func builtinEnvSpec(resourceType, provider string) []envField {
+	rt := strings.ToLower(strings.TrimSpace(resourceType))
+	p := normalizeProvider(provider)
+
+	switch rt {
+	case "git":
+		// Same env names across gitea/github/gitlab; provider passed through.
+		return []envField{
+			{"GIT_PROVIDER", envMeta, "provider"},
+			{"GIT_REPO_URL", envMeta, "repo_url"},
+			{"GIT_BRANCH", envMeta, "default_branch"},
+			{"GIT_API_BASE", envMeta, "api_base"},
+			{"GIT_TOKEN", envSecret, "token"},
+		}
+	case "vm":
+		return []envField{
+			{"VM_HOST", envMeta, "host"},
+			{"VM_PORT", envMeta, "port"},
+			{"VM_USER", envMeta, "user"},
+			{"VM_PASSWORD", envSecret, "password"},
+			{"VM_SSH_KEY", envSecret, "private_key"},
+		}
+	case "database", "db":
+		switch p {
+		case "mysql":
+			return dbEnvSpec("MYSQL")
+		case "pg":
+			return dbEnvSpec("PG")
+		case "mongo":
+			return dbEnvSpec("MONGO")
+		case "redis":
+			return []envField{
+				{"REDIS_HOST", envMeta, "host"},
+				{"REDIS_PORT", envMeta, "port"},
+				{"REDIS_DB", envMeta, "db"},
+				{"REDIS_PASSWORD", envSecret, "password"},
+			}
+		default:
+			return dbEnvSpec("DB")
+		}
+	case "blob", "storage":
+		if p == "bucket" {
+			return []envField{
+				{"BUCKET_ENDPOINT", envMeta, "endpoint"},
+				{"BUCKET_REGION", envMeta, "region"},
+				{"BUCKET_NAME", envMeta, "bucket"},
+				{"BUCKET_ACCESS_KEY_ID", envSecret, "access_key_id"},
+				{"BUCKET_SECRET_ACCESS_KEY", envSecret, "secret_access_key"},
+			}
+		}
+		// Default: Azure Blob.
+		return []envField{
+			{"BLOB_ACCOUNT", envMeta, "account"},
+			{"BLOB_CONTAINER", envMeta, "container"},
+			{"BLOB_KEY", envSecret, "key"},
+		}
+	default:
+		return nil
+	}
 }

 // buildAgentEnvFromBindings resolves the selected resource bindings into a flat
-// environment map for a template agent's .env at start time.
+// environment map for a template agent's .env at start time, using the built-in
+// per-type env convention.
 //
 // Non-secret values come from each binding's Metadata; secret values are read
-// from Key Vault here, at start time, and returned for injection into the agent
-// — they are NOT stored back on the Manager side. Callers must not log the
-// returned map.
+// from Key Vault here, at start time, and returned for injection — they are NOT
+// persisted on the Manager side. Callers must NOT log the returned map.
+//
+// Lenient by design: a missing optional field (e.g. a vm with a password but no
+// ssh key, or a binding with no secret yet) is simply skipped, not an error.
+// Only an unsupported resource type or a Key Vault read failure errors out.
 func buildAgentEnvFromBindings(userID int, bindingIDs []int) (map[string]string, error) {
 	env := map[string]string{}
 	if len(bindingIDs) == 0 {
@@ -47,14 +158,9 @@ func buildAgentEnvFromBindings(userID int, bindingIDs []int) (map[string]string,
 			return nil, fmt.Errorf("resource binding %d not found for user", id)
 		}

-		entries := map[string]envMapEntry{}
-		if strings.TrimSpace(binding.EnvMap) != "" {
-			if err := common.UnmarshalJsonStr(binding.EnvMap, &entries); err != nil {
-				return nil, fmt.Errorf("binding %d env_map is not valid JSON: %w", id, err)
-			}
-		}
-		if len(entries) == 0 {
-			continue
+		spec := builtinEnvSpec(binding.ResourceType, binding.Provider)
+		if len(spec) == 0 {
+			return nil, fmt.Errorf("binding %d: unsupported resource type %q (provider %q)", id, binding.ResourceType, binding.Provider)
 		}

 		metadata := map[string]any{}
@@ -62,45 +168,37 @@ func buildAgentEnvFromBindings(userID int, bindingIDs []int) (map[string]string,
 			_ = common.UnmarshalJsonStr(binding.Metadata, &metadata)
 		}

-		// The secret JSON is fetched lazily, only if some entry needs it.
+		// Resolve the secret JSON once per binding, only if one is bound.
 		var secret map[string]any
-		secretLoaded := false
+		if strings.TrimSpace(binding.SecretRef) != "" {
+			if !storeReady {
+				client, err := newSecretStoreClientFromEnv()
+				if err != nil {
+					return nil, err
+				}
+				store = client
+				storeReady = true
+			}
+			resolved, err := store.getJSONSecret(binding.SecretRef)
+			if err != nil {
+				return nil, fmt.Errorf("binding %d secret read failed: %w", id, err)
+			}
+			secret = resolved
+		}

-		for envName, entry := range entries {
-			name := strings.TrimSpace(envName)
-			if name == "" {
+		for _, f := range spec {
+			src := metadata
+			if f.Source == envSecret {
+				src = secret
+			}
+			if src == nil {
 				continue
 			}
-			switch entry.Source {
-			case "metadata":
-				if value, ok := metadata[entry.Key]; ok {
-					env[name] = fmt.Sprintf("%v", value)
+			if value, ok := src[f.Key]; ok {
+				str := fmt.Sprintf("%v", value)
+				if strings.TrimSpace(str) != "" {
+					env[f.Env] = str
 				}
-			case "secret":
-				if !secretLoaded {
-					if strings.TrimSpace(binding.SecretRef) == "" {
-						return nil, fmt.Errorf("binding %d env_map needs a secret but the binding has no secret_ref", id)
-					}
-					if !storeReady {
-						client, err := newSecretStoreClientFromEnv()
-						if err != nil {
-							return nil, err
-						}
-						store = client
-						storeReady = true
-					}
-					resolved, err := store.getJSONSecret(binding.SecretRef)
-					if err != nil {
-						return nil, fmt.Errorf("binding %d secret read failed: %w", id, err)
-					}
-					secret = resolved
-					secretLoaded = true
-				}
-				if value, ok := secret[entry.Key]; ok {
-					env[name] = fmt.Sprintf("%v", value)
-				}
-			default:
-				return nil, fmt.Errorf("binding %d env_map %q: unknown source %q (want metadata|secret)", id, name, entry.Source)
 			}
 		}
 	}