diff --git a/README.md b/README.md index 77ffe32..f79d5f5 100644 --- a/README.md +++ b/README.md @@ -279,10 +279,13 @@ hops local start --backend dory On start/activate, hops: - merges stock `~/.kube/dory-config` into `~/.kube/config` as context **`hops-dory`** - (override with `--name ` or `HOPS_DORY_NAME`; persisted in `~/.hops/local/dory-name`) + (override with `--dory-name ` or `HOPS_DORY_NAME`; persisted in `~/.hops/local/dory-name`) - runs `kubectl config use-context hops-dory` - creates/uses a docker context of the same name → `unix://$HOME/.dory/dory.sock` +`--dory-name` is intentionally **not** `--name`. Workspace commands use `--name` for the +Kubernetes namespace (`hops local up|down|status|open|gitops worktree --name alice`). + So you should **not** need: ```bash @@ -291,8 +294,9 @@ export DOCKER_HOST=unix://$HOME/.dory/dory.sock ``` ```bash -hops local start --backend dory # name defaults to hops-dory -hops local start --backend dory --name mine # custom kube+docker context name +hops local start --backend dory # dory name defaults to hops-dory +hops local start --backend dory --dory-name mine # custom kube+docker context name +hops local up ./gitops/envs/local --name alice # workspace ns only; does not rename Dory kubectl get nodes # context hops-dory docker info # context hops-dory diff --git a/src/commands/local/backend/dory.rs b/src/commands/local/backend/dory.rs index 48efcc5..1773a01 100644 --- a/src/commands/local/backend/dory.rs +++ b/src/commands/local/backend/dory.rs @@ -483,7 +483,7 @@ pub const DEFAULT_CONTEXT_NAME: &str = "hops-dory"; const NAME_FILE: &str = "dory-name"; const NAME_ENV: &str = "HOPS_DORY_NAME"; -/// Resolved context name: `--name` (persisted) > `HOPS_DORY_NAME` > file > `hops-dory`. +/// Resolved context name: `--dory-name` (persisted) > `HOPS_DORY_NAME` > file > `hops-dory`. pub fn context_name() -> String { if let Ok(v) = std::env::var(NAME_ENV) { let t = v.trim(); @@ -515,12 +515,12 @@ pub fn persist_context_name(name: &str) -> Result<(), Box> { fn validate_context_name(name: &str) -> Result> { let name = name.trim(); if name.is_empty() { - return Err("--name must not be empty".into()); + return Err("--dory-name must not be empty".into()); } // kubectl context names: keep it simple (no path separators / whitespace). if name.contains(['/', '\\', ' ', '\t', '\n', ':']) { return Err(format!( - "invalid --name '{name}': use a simple token (e.g. hops-dory)" + "invalid --dory-name '{name}': use a simple token (e.g. hops-dory)" ) .into()); } @@ -545,7 +545,7 @@ pub fn desktop_integration_enabled() -> bool { /// Wire Dory into the normal developer desktop: /// - merge stock `~/.kube/dory-config` into `~/.kube/config` as context **`hops-dory`** -/// (or `--name` / `HOPS_DORY_NAME`) +/// (or `--dory-name` / `HOPS_DORY_NAME`) /// - `kubectl config use-context ` /// - ensure/use a docker context of the same name pointing at `~/.dory/dory.sock` /// diff --git a/src/commands/local/backend/mod.rs b/src/commands/local/backend/mod.rs index e0b2098..bfa9ce8 100644 --- a/src/commands/local/backend/mod.rs +++ b/src/commands/local/backend/mod.rs @@ -230,7 +230,7 @@ impl FromStr for Backend { const BACKEND_FILE: &str = "backend"; -/// Persist the Dory desktop context name (`--name`, default hops-dory). +/// Persist the Dory desktop context name (`--dory-name`, default hops-dory). pub fn persist_dory_context_name(name: &str) -> Result<(), Box> { dory::persist_context_name(name) } diff --git a/src/commands/local/mod.rs b/src/commands/local/mod.rs index 0974414..bbd92d6 100644 --- a/src/commands/local/mod.rs +++ b/src/commands/local/mod.rs @@ -115,11 +115,14 @@ pub struct LocalArgs { #[arg(long, global = true, value_enum)] pub backend: Option, - /// Name for Dory desktop integration (kube context + docker context). + /// Dory desktop integration name (kube context + docker context). /// Defaults to `hops-dory`. Persisted under `~/.hops/local/dory-name`. /// Only used with `--backend dory` (or a persisted dory backend). - #[arg(long, global = true, value_name = "NAME")] - pub name: Option, + /// + /// Named `--dory-name` (not `--name`) so it never collides with workspace + /// `--name` on `hops local up|down|status|open|gitops worktree`. + #[arg(long = "dory-name", global = true, value_name = "NAME")] + pub dory_name: Option, } #[derive(Subcommand, Debug)] @@ -163,7 +166,12 @@ pub enum LocalCommands { } pub fn run(args: &LocalArgs) -> Result<(), Box> { - if let Some(name) = args.name.as_deref().map(str::trim).filter(|s| !s.is_empty()) { + if let Some(name) = args + .dory_name + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + { backend::persist_dory_context_name(name)?; } @@ -431,4 +439,71 @@ mod tests { vec!["repo", "update", "crossplane-stable"] ); } + + /// Regression: workspace `--name` must not populate Dory's `--dory-name`. + /// Dual workspaces (`up --name alice` then `up --name bob`) used to rewrite + /// the desktop kube/docker context and delete the real `dory` context. + #[test] + fn workspace_name_does_not_set_dory_name() { + use clap::Parser; + + #[derive(Parser, Debug)] + #[command(name = "hops-local-test")] + struct Cli { + #[command(flatten)] + local: LocalArgs, + } + + let parsed = Cli::try_parse_from([ + "hops-local-test", + "up", + "./gitops/envs/local", + "--name", + "alice", + "--once", + "--no-cluster", + ]) + .expect("parse up --name alice"); + assert!( + parsed.local.dory_name.is_none(), + "workspace --name must not set dory_name; got {:?}", + parsed.local.dory_name + ); + match parsed.local.command { + LocalCommands::Up(up) => { + assert_eq!(up.name.as_deref(), Some("alice")); + assert!(up.once); + assert!(up.no_cluster); + } + other => panic!("expected Up, got {other:?}"), + } + } + + #[test] + fn dory_name_flag_is_distinct_from_workspace_name() { + use clap::Parser; + + #[derive(Parser, Debug)] + #[command(name = "hops-local-test")] + struct Cli { + #[command(flatten)] + local: LocalArgs, + } + + let parsed = Cli::try_parse_from([ + "hops-local-test", + "--dory-name", + "mine", + "up", + "./env", + "--name", + "bob", + ]) + .expect("parse --dory-name mine up --name bob"); + assert_eq!(parsed.local.dory_name.as_deref(), Some("mine")); + match parsed.local.command { + LocalCommands::Up(up) => assert_eq!(up.name.as_deref(), Some("bob")), + other => panic!("expected Up, got {other:?}"), + } + } } diff --git a/src/commands/local/workbench/reconcile.rs b/src/commands/local/workbench/reconcile.rs index 554c1a1..10b0337 100644 --- a/src/commands/local/workbench/reconcile.rs +++ b/src/commands/local/workbench/reconcile.rs @@ -443,7 +443,75 @@ fn sanitize_release_name(name: &str) -> String { s.trim_matches('-').chars().take(53).collect() } -fn ensure_namespace_on_docs(yaml: &str, namespace: &str) -> Result> { +/// Kinds that are cluster-scoped (never get a namespace stamp). +fn is_cluster_scoped_kind(kind: &str) -> bool { + matches!( + kind, + "Namespace" + | "ClusterRole" + | "ClusterRoleBinding" + | "CustomResourceDefinition" + | "ClusterProviderConfig" + | "ClusterSecretStore" + | "PriorityClass" + | "StorageClass" + | "PersistentVolume" + | "MutatingWebhookConfiguration" + | "ValidatingWebhookConfiguration" + ) +} + +/// Workload / app kinds that live in the worktree app namespace when the chart +/// did not set `metadata.namespace`. Shared identity MRs (Project, Role, +/// HumanUser, …) set their own namespace in the chart and are **not** rewritten. +fn is_app_workload_kind(api_version: &str, kind: &str) -> bool { + // Core / apps workloads + if matches!( + kind, + "Deployment" + | "StatefulSet" + | "DaemonSet" + | "ReplicaSet" + | "Job" + | "CronJob" + | "Service" + | "ServiceAccount" + | "ConfigMap" + | "Secret" + | "PersistentVolumeClaim" + | "NetworkPolicy" + | "Ingress" + | "HorizontalPodAutoscaler" + | "PodDisruptionBudget" + | "Role" + | "RoleBinding" + ) { + // k8s rbac Role/RoleBinding use rbac.authorization.k8s.io — not Zitadel Role + if kind == "Role" || kind == "RoleBinding" { + return api_version.starts_with("rbac.authorization.k8s.io/"); + } + return true; + } + // External Secrets (worktree OIDC secret materialization) + if kind == "ExternalSecret" { + return true; + } + // Worktree-scoped Zitadel OIDC app + instance Features (Login V2 for this UI) + if kind == "Oidc" && api_version.contains("application.zitadel") { + return true; + } + if kind == "Features" && api_version.contains("instance.zitadel") { + return true; + } + false +} + +/// Stamp worktree namespace only onto app workloads that lack `metadata.namespace`. +/// +/// Shared identity resources (e.g. Project/Role/HumanUser with +/// `projectNamespace: default`) keep the namespace declared by the chart. +/// Never overwrites an already-set namespace. +pub fn ensure_namespace_on_docs(yaml: &str, namespace: &str) -> Result> { let mut out = Vec::new(); for doc in split_yaml_docs_owned(yaml) { if doc.trim().is_empty() { @@ -454,18 +522,27 @@ fn ensure_namespace_on_docs(yaml: &str, namespace: &str) -> Result