diff --git a/.github/workflows/on-pr-kind-smoke.yaml b/.github/workflows/on-pr-kind-smoke.yaml index cf710af..a53dc90 100644 --- a/.github/workflows/on-pr-kind-smoke.yaml +++ b/.github/workflows/on-pr-kind-smoke.yaml @@ -83,20 +83,21 @@ jobs: FIXTURE=tests/fixtures/config-smoke test -d "$FIXTURE" test -f "$FIXTURE/upbound.yaml" + CONFIGURATION=config-smoke ./target/debug/hops-cli config install --path "$FIXTURE" --cluster-provider kind --docker-provider docker for i in $(seq 1 90); do - healthy="$(kubectl --context kind-hops get configuration.pkg.crossplane.io hops-ops-config-smoke \ + healthy="$(kubectl --context kind-hops get configuration.pkg.crossplane.io "$CONFIGURATION" \ -o jsonpath='{.status.conditions[?(@.type=="Healthy")].status}' 2>/dev/null || true)" - installed="$(kubectl --context kind-hops get configuration.pkg.crossplane.io hops-ops-config-smoke \ + installed="$(kubectl --context kind-hops get configuration.pkg.crossplane.io "$CONFIGURATION" \ -o jsonpath='{.status.conditions[?(@.type=="Installed")].status}' 2>/dev/null || true)" if [ "$healthy" = "True" ] && [ "$installed" = "True" ]; then - echo "Configuration hops-ops-config-smoke Healthy after ${i} attempts" + echo "Configuration $CONFIGURATION Healthy after ${i} attempts" break fi if [ "$i" -eq 90 ]; then - kubectl --context kind-hops get configuration.pkg.crossplane.io hops-ops-config-smoke -o yaml || true + kubectl --context kind-hops get configuration.pkg.crossplane.io "$CONFIGURATION" -o yaml || true kubectl --context kind-hops get configurationrevisions.pkg.crossplane.io -o wide || true kubectl --context kind-hops get functions.pkg.crossplane.io -o wide || true exit 1 diff --git a/Cargo.lock b/Cargo.lock index 093f5bb..56bf93b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -628,6 +628,7 @@ dependencies = [ "dialoguer", "fern", "flate2", + "libc", "log", "notify", "openssl-sys", diff --git a/Cargo.toml b/Cargo.toml index b11fb8b..798e220 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ rusoto_sts = "0.48.0" tokio = { version = "1.45.1", features = ["rt-multi-thread"] } uuid = { version = "1.17.0", features = ["v4"] } notify = "8.2.0" +libc = "0.2" [features] vendored = ["openssl-sys/vendored"] diff --git a/README.md b/README.md index c3a792b..29c6156 100644 --- a/README.md +++ b/README.md @@ -609,7 +609,7 @@ Notes: - Source-build mode intended for a local control plane because it depends on the local registry flow - Runs `up project build` in `PATH` (defaults to current directory) - Loads generated `.uppkg` artifacts from `/_output` - - Pushes package images to the registry exposed at `127.0.0.1:30500` (IPv4; avoids Docker's `localhost` → `[::1]` path) + - Pushes package images to the backend-specific registry endpoint selected by the CLI: `127.0.0.1:30500` when Docker runs locally, or `{dory-k8s-ip}:30500` for Dory - Applies Crossplane `Configuration` resources pointing at `registry.crossplane-system.svc.cluster.local:5000/...` - Supports `--skip-dependency-resolution` - `config install --repo [--reload]` diff --git a/skills/claude/references/secrets.md b/skills/claude/references/secrets.md index 3c83471..ace8643 100644 --- a/skills/claude/references/secrets.md +++ b/skills/claude/references/secrets.md @@ -3,7 +3,7 @@ ## Overview `hops secrets` manages repo-level secrets using SOPS for encryption and syncs -to AWS Secrets Manager, GitHub repository secrets, or HashiCorp Vault (KV). +to AWS Secrets Manager or GitHub repository secrets. ## Setup @@ -20,11 +20,9 @@ secrets/ # Plaintext (gitignored) aws/ github/ _shared/ - vault/ # → hops secrets sync vault (KV paths) secrets-encrypted/ # SOPS-encrypted (committed) aws/ github/ - vault/ ``` ### Configuration (`.hops.yaml`) @@ -46,18 +44,6 @@ secrets: repos: - repo-a - repo-b - vault: - path: vault - address: http://127.0.0.1:8200 # or $VAULT_ADDR - mount: secret # KV mount - version: v2 - path_prefix: "" # optional prefix on every remote path - token_env: VAULT_TOKEN - kube: # port-forward when address is down - enabled: true - namespace: vault - service: vault - local_port: 8200 ``` ## Encrypt / Decrypt @@ -108,25 +94,3 @@ hops secrets sync github - `.env` files → one secret per `KEY=value` entry - Shared secrets fan out to all repos in `shared_secrets.repos` - Repo-specific values override shared values - -## Sync to HashiCorp Vault (KV) - -```bash -export VAULT_TOKEN=root # local SecretStack dev Vault; never commit -hops secrets sync vault -hops secrets sync vault --secret-path secrets/vault/e2e-ui/dogfood -y -hops secrets sync vault --port-forward # force kubectl tunnel to in-cluster Vault -``` - -### Vault naming rules (same roll-up as AWS) - -| Source | Vault KV path (mount `secret`) | -|--------|--------------------------------| -| `secrets/vault/e2e-ui/dogfood/oidc.json` | `e2e-ui/dogfood/oidc` (JSON object → properties) | -| `secrets/vault/e2e-ui/dogfood/human-passwords/{alice,bob}` | `e2e-ui/dogfood/human-passwords` | -| `secrets/vault/auth/zitadel-masterkey/masterkey` | `auth/zitadel-masterkey` property `masterkey` | - -- Paths match ExternalSecret `remoteRef.key` (no `secret/data/` prefix) -- Writer token is separate from ESO’s read-only kubernetes auth role -- Unchanged remote maps are skipped (compare-before-write) -- Local Vault chart is often in-memory: re-run sync after `vault-0` restarts; keep SOPS plaintext as the durable copy diff --git a/src/commands/local/mod.rs b/src/commands/local/mod.rs index 11e3fa3..a10aced 100644 --- a/src/commands/local/mod.rs +++ b/src/commands/local/mod.rs @@ -300,12 +300,19 @@ pub(crate) fn wait_for_kubernetes() -> Result<(), Box> { log::info!("Waiting for Kubernetes API..."); // ~10 minutes — nested-virt apiserver can stay overloaded after package install. for _ in 0..120 { - let result = run_cmd_output("kubectl", &["get", "--raw", "/readyz"]); - if result.is_ok() { + if kubectl_command(&["get", "--raw", "/readyz"]) + .output() + .map(|output| output.status.success()) + .unwrap_or(false) + { return Ok(()); } // Fall back to a cheap list if /readyz is denied on some setups. - if run_cmd_output("kubectl", &["get", "ns", "default"]).is_ok() { + if kubectl_command(&["get", "ns", "default"]) + .output() + .map(|output| output.status.success()) + .unwrap_or(false) + { return Ok(()); } std::thread::sleep(std::time::Duration::from_secs(5)); @@ -357,19 +364,48 @@ pub fn kubectl_apply_stdin(yaml: &str) -> Result<(), Box> { if !stderr.trim().is_empty() { eprint!("{stderr}"); } + if !is_transient_kubectl_apply_error(&stderr) { + return Err(format!("kubectl apply exited with {last_err}").into()); + } + if attempt == ATTEMPTS { + break; + } log::warn!( "kubectl apply failed (attempt {}/{}, {}); waiting for API...", attempt, ATTEMPTS, output.status ); - let _ = wait_for_kubernetes(); + wait_for_kubernetes().map_err(|e| { + format!("kubectl apply exited with {last_err}; API recovery failed: {e}") + })?; std::thread::sleep(std::time::Duration::from_secs(10)); } Err(format!("kubectl apply exited with {last_err} after retries").into()) } +fn is_transient_kubectl_apply_error(stderr: &str) -> bool { + let lower = stderr.to_ascii_lowercase(); + [ + "unable to connect to the server", + "connection refused", + "connection reset", + "connection timed out", + "i/o timeout", + "tls handshake timeout", + "context deadline exceeded", + "dial tcp", + "no route to host", + "service unavailable", + "gateway timeout", + "server is currently unable to handle the request", + "unexpected eof", + ] + .iter() + .any(|needle| lower.contains(needle)) +} + /// Apply a JSON merge patch with `kubectl patch --type merge`. /// Automatically injects `--context` when configured. pub fn kubectl_patch_merge( diff --git a/src/commands/local/workbench/cluster_dns.rs b/src/commands/local/workbench/cluster_dns.rs index ef4ac4a..24be2ff 100644 --- a/src/commands/local/workbench/cluster_dns.rs +++ b/src/commands/local/workbench/cluster_dns.rs @@ -13,11 +13,13 @@ //! //! Result: `curl http://e2e-ui-api.dogfood.svc.cluster.local:8791` -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::error::Error; -use std::fs; +use std::fs::{self, File, OpenOptions}; +use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::time::{SystemTime, UNIX_EPOCH}; /// Managed block markers in `/etc/hosts` (and the runtime mirror). pub const HOSTS_BEGIN: &str = "# BEGIN hops-local-dns (managed by hops local — do not edit)"; @@ -151,18 +153,33 @@ pub fn merge_hosts_file(existing: &str, all_workspace_lines: &[String]) -> Strin /// Remove the hops-managed block from hosts content. pub fn strip_managed_block(existing: &str) -> String { - let mut out = String::new(); - let mut in_block = false; - for line in existing.lines() { + let lines: Vec<&str> = existing.lines().collect(); + let mut pending_begin = None; + let mut matched_ranges = Vec::new(); + for (index, line) in lines.iter().enumerate() { if line.trim() == HOSTS_BEGIN { - in_block = true; - continue; - } - if line.trim() == HOSTS_END { - in_block = false; - continue; + // A later BEGIN supersedes an unmatched earlier one. This preserves an + // interrupted block as ordinary user content while still recognizing a + // subsequent complete block written by hops. + pending_begin = Some(index); + } else if line.trim() == HOSTS_END { + if let Some(begin) = pending_begin.take() { + matched_ranges.push((begin, index)); + } } - if !in_block { + } + if pending_begin.is_some() { + log::warn!("unterminated hops-local block in /etc/hosts; preserving original content"); + } + if matched_ranges.is_empty() { + return existing.to_string(); + } + let mut out = String::new(); + for (index, line) in lines.into_iter().enumerate() { + if !matched_ranges + .iter() + .any(|(begin, end)| index >= *begin && index <= *end) + { out.push_str(line); out.push('\n'); } @@ -183,12 +200,58 @@ fn alloc_path(state_dir: &Path) -> PathBuf { state_dir.join("runtime").join("dns-ip-alloc.json") } +pub(crate) struct DnsStateLock { + _file: File, +} + +impl Drop for DnsStateLock { + fn drop(&mut self) { + #[cfg(unix)] + unsafe { + use std::os::fd::AsRawFd; + let _ = libc::flock(self._file.as_raw_fd(), libc::LOCK_UN); + } + } +} + +pub(crate) fn acquire_dns_state_lock(state_dir: &Path) -> Result> { + let runtime_dir = state_dir.join("runtime"); + fs::create_dir_all(&runtime_dir)?; + let file = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .truncate(false) + .open(runtime_dir.join("dns-state.lock"))?; + #[cfg(unix)] + unsafe { + use std::os::fd::AsRawFd; + if libc::flock(file.as_raw_fd(), libc::LOCK_EX) != 0 { + return Err(std::io::Error::last_os_error().into()); + } + } + Ok(DnsStateLock { _file: file }) +} + pub fn load_ip_alloc(state_dir: &Path) -> DnsIpAlloc { let path = alloc_path(state_dir); - fs::read_to_string(path) - .ok() - .and_then(|t| serde_json::from_str(&t).ok()) - .unwrap_or_default() + match fs::read_to_string(&path) { + Ok(text) => serde_json::from_str(&text).unwrap_or_else(|error| { + log::warn!( + "ignoring corrupt DNS allocation file {}: {error}", + path.display() + ); + DnsIpAlloc::default() + }), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => DnsIpAlloc::default(), + Err(error) => { + log::warn!( + "could not read DNS allocation file {}: {error}", + path.display() + ); + DnsIpAlloc::default() + } + } } pub fn save_ip_alloc(state_dir: &Path, alloc: &DnsIpAlloc) -> Result<(), Box> { @@ -205,6 +268,15 @@ pub fn sync_alloc_for_namespace( state_dir: &Path, namespace: &str, services: &[super::net::ServiceEndpoint], +) -> Result, Box> { + let _lock = acquire_dns_state_lock(state_dir)?; + sync_alloc_for_namespace_locked(state_dir, namespace, services) +} + +pub(crate) fn sync_alloc_for_namespace_locked( + state_dir: &Path, + namespace: &str, + services: &[super::net::ServiceEndpoint], ) -> Result, Box> { let mut alloc = load_ip_alloc(state_dir); // Drop bindings for services no longer present in this namespace @@ -228,6 +300,27 @@ pub fn sync_alloc_for_namespace( Ok(service_ips) } +pub fn parse_ifconfig_inet_addresses(output: &str) -> BTreeSet { + output + .lines() + .filter_map(|line| { + let mut fields = line.split_whitespace(); + (fields.next() == Some("inet")) + .then(|| fields.next()) + .flatten() + }) + .map(str::to_string) + .collect() +} + +fn loopback_inet_addresses() -> BTreeSet { + Command::new("ifconfig") + .arg("lo0") + .output() + .map(|output| parse_ifconfig_inet_addresses(&String::from_utf8_lossy(&output.stdout))) + .unwrap_or_default() +} + /// Rebuild /etc/hosts managed block from **all** workspace runtimes that use dns mode. /// `workspace_blocks`: list of (namespace, service→ip). pub fn rebuild_hosts_from_blocks( @@ -259,14 +352,10 @@ pub fn dns_os_config_present(hosts_body: &str, loopback_ips: &[String]) -> bool if !macos_resolver_present() { return false; } - let lo0 = Command::new("ifconfig") - .arg("lo0") - .output() - .map(|o| String::from_utf8_lossy(&o.stdout).into_owned()) - .unwrap_or_default(); + let lo0 = loopback_inet_addresses(); loopback_ips .iter() - .all(|ip| ip.is_empty() || ip == "127.0.0.1" || lo0.contains(ip.as_str())) + .all(|ip| ip.is_empty() || ip == "127.0.0.1" || lo0.contains(ip)) } fn macos_resolver_present() -> bool { @@ -306,23 +395,93 @@ fn extract_managed_block(hosts: &str) -> String { pub fn apply_privileged_dns_config( hosts_body: &str, loopback_ips: &[String], +) -> Result<(), Box> { + apply_privileged_dns_config_with_prompt( + hosts_body, + loopback_ips, + PrivilegedPrompt::InteractiveOnce, + ) +} + +pub(crate) fn apply_privileged_dns_config_noprompt(hosts_body: &str) -> Result<(), Box> { + apply_privileged_dns_config_with_prompt(hosts_body, &[], PrivilegedPrompt::Never) +} + +struct StagedHostsFile { + dir: PathBuf, + path: PathBuf, +} + +impl StagedHostsFile { + fn create(hosts_body: &str) -> Result> { + let dir = std::env::temp_dir().join(format!("hops-hosts-{}", uuid::Uuid::new_v4())); + let mut builder = fs::DirBuilder::new(); + #[cfg(unix)] + { + use std::os::unix::fs::DirBuilderExt; + builder.mode(0o700); + } + builder.create(&dir)?; + let path = dir.join("hosts"); + let staged = Self { dir, path }; + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options.open(&staged.path)?; + file.write_all(hosts_body.as_bytes())?; + Ok(staged) + } +} + +impl Drop for StagedHostsFile { + fn drop(&mut self) { + let _ = fs::remove_file(&self.path); + let _ = fs::remove_dir(&self.dir); + } +} + +fn safe_privileged_path(path: &str) -> bool { + !path.is_empty() + && path + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '/' | '.' | '_' | '-')) +} + +fn apply_privileged_dns_config_with_prompt( + hosts_body: &str, + loopback_ips: &[String], + prompt: PrivilegedPrompt, ) -> Result<(), Box> { if dns_os_config_present(hosts_body, loopback_ips) { log::debug!("cluster DNS OS config already present; skipping admin prompt"); return Ok(()); } - let tmp = std::env::temp_dir().join(format!("hops-hosts-{}.tmp", std::process::id())); - fs::write(&tmp, hosts_body)?; - #[cfg(unix)] + let staged = StagedHostsFile::create(hosts_body)?; + let tmp_s = staged.path.to_string_lossy().into_owned(); + if !safe_privileged_path(&tmp_s) { + return Err("temporary hosts path contains unsupported shell metacharacters".into()); + } + if loopback_ips + .iter() + .any(|ip| !ip.is_empty() && ip.parse::().is_err()) { - use std::os::unix::fs::PermissionsExt; - let _ = fs::set_permissions(&tmp, fs::Permissions::from_mode(0o644)); + return Err("invalid loopback IP in DNS configuration".into()); } - let tmp_s = tmp.to_string_lossy().into_owned(); + let timestamp = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); + let backup = format!( + "/etc/hosts.hops-backup-{timestamp}-{}", + uuid::Uuid::new_v4() + ); let mut shell = String::new(); - shell.push_str(&format!("cp '{tmp_s}' /etc/hosts && chmod 644 /etc/hosts")); + shell.push_str(&format!( + "cp /etc/hosts '{backup}' && chmod 600 '{backup}' && cp '{tmp_s}' /etc/hosts && chmod 644 /etc/hosts" + )); if cfg!(target_os = "macos") { for ip in loopback_ips { if ip == "127.0.0.1" || ip.is_empty() { @@ -346,8 +505,10 @@ pub fn apply_privileged_dns_config( log::info!( "Configuring cluster DNS on this machine (admin required once): /etc/hosts + loopback aliases" ); - let result = run_privileged_shell(&shell, PrivilegedPrompt::InteractiveOnce); - let _ = fs::remove_file(&tmp); + let result = run_privileged_shell(&shell, prompt); + if result.is_ok() { + log::info!("backed up /etc/hosts to {backup}"); + } result.map_err(|e| { format!( "cluster DNS needs admin privileges to write /etc/hosts (and lo0 aliases on macOS).\n\ @@ -407,7 +568,9 @@ pub fn run_privileged_shell(script: &str, prompt: PrivilegedPrompt) -> Result<() } if cfg!(target_os = "macos") { - // GUI admin dialog (IDEs / agents without a TTY). Escape for AppleScript. + // GUI admin dialog (IDEs / agents without a TTY). AppleScript escaping alone + // does not neutralize shell substitutions, so interpolated paths are validated + // before this function is called. let escaped = script .replace('\\', "\\\\") .replace('"', "\\\"") @@ -431,16 +594,11 @@ pub fn ensure_loopback_aliases(ips: &[String]) -> Result<(), Box> { if !cfg!(target_os = "macos") { return Ok(()); } + let configured = loopback_inet_addresses(); let missing: Vec = ips .iter() .filter(|ip| !ip.is_empty() && *ip != "127.0.0.1") - .filter(|ip| { - !Command::new("ifconfig") - .arg("lo0") - .output() - .map(|o| String::from_utf8_lossy(&o.stdout).contains(ip.as_str())) - .unwrap_or(false) - }) + .filter(|ip| !configured.contains(*ip)) .cloned() .collect(); if missing.is_empty() { @@ -473,6 +631,15 @@ pub fn remove_loopback_aliases(ips: &[String]) { } } +pub(crate) fn remove_macos_resolver_noprompt() { + if cfg!(target_os = "macos") { + let _ = run_privileged_shell( + "rm -f '/etc/resolver/svc.cluster.local'; dscacheutil -flushcache 2>/dev/null; killall -HUP mDNSResponder 2>/dev/null; true", + PrivilegedPrompt::Never, + ); + } +} + /// Port-forward argv: bind cluster service port on a specific loopback IP. pub fn build_dns_port_forward_args( namespace: &str, @@ -553,6 +720,31 @@ mod tests { assert_eq!(merged.matches(HOSTS_BEGIN).count(), 1); } + #[test] + fn unterminated_managed_block_preserves_original_hosts() { + let existing = format!("127.0.0.1 localhost\n{HOSTS_BEGIN}\nkeep-this-line\n"); + assert_eq!(strip_managed_block(&existing), existing); + } + + #[test] + fn later_complete_block_does_not_consume_unterminated_user_content() { + let existing = format!( + "127.0.0.1 localhost\n{HOSTS_BEGIN}\nkeep-this-line\n{HOSTS_BEGIN}\nmanaged\n{HOSTS_END}\n" + ); + let stripped = strip_managed_block(&existing); + assert!(stripped.contains("keep-this-line")); + assert!(stripped.contains(HOSTS_BEGIN)); + assert!(!stripped.contains("managed\n")); + } + + #[test] + fn ifconfig_parser_matches_complete_inet_addresses() { + let output = "\tinet 127.0.0.1 netmask 0xff000000\n\tinet 127.53.0.25 netmask 0xff000000\n"; + let addresses = parse_ifconfig_inet_addresses(output); + assert!(addresses.contains("127.53.0.25")); + assert!(!addresses.contains("127.53.0.2")); + } + #[test] fn dns_port_forward_binds_address() { let args = build_dns_port_forward_args("x", "api", "127.53.0.2", 8791); diff --git a/src/commands/local/workbench/delivery.rs b/src/commands/local/workbench/delivery.rs index 89f36d5..cbdaea6 100644 --- a/src/commands/local/workbench/delivery.rs +++ b/src/commands/local/workbench/delivery.rs @@ -233,9 +233,7 @@ fn try_docker_node_probe(host_path: &Path) -> Result, Box< if !command_exists("docker") || !command_exists("kubectl") { return Ok(None); } - let nodes_json = Command::new("kubectl") - .args(["get", "nodes", "-o", "json"]) - .output()?; + let nodes_json = kubectl_command(&["get", "nodes", "-o", "json"]).output()?; if !nodes_json.status.success() { return Ok(None); } @@ -349,8 +347,8 @@ spec: ); // Apply - let mut child = Command::new("kubectl") - .args(["apply", "-f", "-"]) + let mut apply = kubectl_command(&["apply", "-f", "-"]); + let mut child = apply .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -360,9 +358,8 @@ spec: } let apply_out = child.wait_with_output()?; if !apply_out.status.success() { - let _ = Command::new("kubectl") - .args(["delete", "pod", &name, "-n", "default", "--wait=false"]) - .output(); + let _ = + kubectl_command(&["delete", "pod", &name, "-n", "default", "--wait=false"]).output(); return Err(format!( "probe pod apply failed: {}", String::from_utf8_lossy(&apply_out.stderr) @@ -376,25 +373,22 @@ spec: let deadline = Instant::now() + Duration::from_secs(15); while Instant::now() < deadline { std::thread::sleep(Duration::from_millis(400)); - let phase_out = Command::new("kubectl") - .args([ - "get", - "pod", - &name, - "-n", - "default", - "-o", - "jsonpath={.status.phase}", - ]) - .output()?; + let phase_out = kubectl_command(&[ + "get", + "pod", + &name, + "-n", + "default", + "-o", + "jsonpath={.status.phase}", + ]) + .output()?; let phase = String::from_utf8_lossy(&phase_out.stdout) .trim() .to_string(); // FailedMount appears in events / container statuses - let desc = Command::new("kubectl") - .args(["describe", "pod", &name, "-n", "default"]) - .output()?; + let desc = kubectl_command(&["describe", "pod", &name, "-n", "default"]).output()?; let desc_s = String::from_utf8_lossy(&desc.stdout); if desc_s.contains("FailedMount") || desc_s.contains("failed to mount") @@ -407,9 +401,7 @@ spec: } if phase == "Succeeded" || phase == "Failed" || phase == "Running" { - let logs = Command::new("kubectl") - .args(["logs", &name, "-n", "default", "--tail=20"]) - .output()?; + let logs = kubectl_command(&["logs", &name, "-n", "default", "--tail=20"]).output()?; let log_s = String::from_utf8_lossy(&logs.stdout); if log_s.contains("HOPS_PATH_VISIBLE") { visible = true; @@ -429,17 +421,16 @@ spec: } } - let _ = Command::new("kubectl") - .args([ - "delete", - "pod", - &name, - "-n", - "default", - "--wait=false", - "--ignore-not-found=true", - ]) - .output(); + let _ = kubectl_command(&[ + "delete", + "pod", + &name, + "-n", + "default", + "--wait=false", + "--ignore-not-found=true", + ]) + .output(); Ok(probe_from_visibility(host_path, visible, detail)) } @@ -947,7 +938,11 @@ while true; do cur=$(find {roots} -type f \ ! -path '*/node_modules/*' ! -path '*/target/*' ! -path '*/.git/*' \ ! -path '*/dist/*' ! -path '*/.svelte-kit/*' \ - -print0 2>/dev/null | xargs -0 stat -f '%m' 2>/dev/null | cksum | awk '{{print $1}}') + -print0 2>/dev/null | xargs -0 sh -c ' + for file do + stat -c "%Y" "$file" 2>/dev/null || stat -f "%m" "$file" 2>/dev/null + done + ' sh | cksum | awk '{{print $1}}') if [ "$cur" != "$prev" ] || needs_marker; then prev="$cur" sync_all diff --git a/src/commands/local/workbench/net.rs b/src/commands/local/workbench/net.rs index 2a8987e..c8b9130 100644 --- a/src/commands/local/workbench/net.rs +++ b/src/commands/local/workbench/net.rs @@ -14,8 +14,10 @@ //! `http://e2e-ui-ui.dogfood.svc.cluster.local:5180` use super::cluster_dns::{ - self, format_dns_url, remove_loopback_aliases, sync_alloc_for_namespace, MACOS_LOCAL_DNS_PORT, + self, acquire_dns_state_lock, format_dns_url, parse_ifconfig_inet_addresses, + remove_loopback_aliases, sync_alloc_for_namespace_locked, MACOS_LOCAL_DNS_PORT, }; +use super::registry::slugify_name; use crate::commands::local::{kubectl_command, HOPS_KUBE_CONTEXT_ENV}; use std::collections::{BTreeMap, BTreeSet}; use std::error::Error; @@ -149,7 +151,7 @@ pub fn host_access_status_line(rt: &HostAccessRuntime) -> String { fn runtime_path(state_dir: &Path, workspace: &str) -> PathBuf { state_dir .join(RUNTIME_SUBDIR) - .join(format!("{workspace}.host-access.json")) + .join(format!("{}.host-access.json", slugify_name(workspace))) } pub fn save_host_access_runtime( @@ -337,10 +339,10 @@ fn regex_lite_cluster_dns(s: &str) -> Vec<(String, String, Option)> { .unwrap_or(0); let host = &s[start..end]; // expect name.namespace - let mut parts: Vec<&str> = host.split('.').collect(); - if parts.len() >= 2 { - let ns = parts.pop().unwrap().to_string(); - let name = parts.join("."); + let parts: Vec<&str> = host.split('.').collect(); + if parts.len() == 2 { + let name = parts[0].to_string(); + let ns = parts[1].to_string(); if !name.is_empty() && !ns.is_empty() { let after = end + marker.len(); let port = if s[after..].starts_with(':') { @@ -377,6 +379,10 @@ pub fn start_host_access( )); } + // Serialize allocation, runtime collection, /etc/hosts, and zone updates across + // worktrees so each update observes the previous workspace's complete state. + let _dns_state_lock = acquire_dns_state_lock(state_dir)?; + // Allocate IPs per namespace group. let mut by_ns: BTreeMap> = BTreeMap::new(); for svc in services { @@ -390,7 +396,7 @@ pub fn start_host_access( let mut ns_blocks: Vec<(String, BTreeMap)> = Vec::new(); for (ns, svcs) in &by_ns { // sync_alloc maps bare service name → ip; rekey to ns/name - let bare = sync_alloc_for_namespace(state_dir, ns, svcs)?; + let bare = sync_alloc_for_namespace_locked(state_dir, ns, svcs)?; let mut bare_for_hosts = BTreeMap::new(); for (name, ip) in bare { bare_for_hosts.insert(name.clone(), ip.clone()); @@ -420,7 +426,9 @@ pub fn start_host_access( let plan = plan_host_access_with_ips(namespace, services, &ip_map); // Zone file for macOS stub DNS (instant *.svc.cluster.local; avoid mDNS). write_zone_file(state_dir, &merged_by_ns)?; - ensure_macos_stub_dns(state_dir)?; + if let Err(error) = ensure_macos_stub_dns(state_dir) { + log::warn!("macOS stub DNS unavailable ({error}); falling back to /etc/hosts resolution"); + } let rt = start_dns_supervisor(&plan, services, state_dir, workspace)?; std::thread::sleep(Duration::from_millis(700)); if !rt.pids.iter().any(|p| pid_is_alive(*p)) { @@ -637,11 +645,11 @@ fn verify_loopback_aliases_ready(ips: &[String]) -> Result<(), Box> { let lo0 = Command::new("ifconfig") .arg("lo0") .output() - .map(|o| String::from_utf8_lossy(&o.stdout).into_owned()) + .map(|o| parse_ifconfig_inet_addresses(&String::from_utf8_lossy(&o.stdout))) .unwrap_or_default(); let missing: Vec<&str> = ips .iter() - .filter(|ip| !ip.is_empty() && *ip != "127.0.0.1" && !lo0.contains(ip.as_str())) + .filter(|ip| !ip.is_empty() && *ip != "127.0.0.1" && !lo0.contains(*ip)) .map(String::as_str) .collect(); if missing.is_empty() { @@ -661,6 +669,7 @@ fn start_dns_supervisor( workspace: &str, ) -> Result> { let _ = stop_host_access_processes_only(state_dir, workspace); + let workspace = slugify_name(workspace); let log_dir = state_dir.join(RUNTIME_SUBDIR); fs::create_dir_all(&log_dir)?; @@ -718,9 +727,16 @@ while true; do pf="$PIDDIR/$safe.pid" pid="" if [ -f "$pf" ]; then pid=$(cat "$pf" 2>/dev/null || true); fi - if [ -n "$pid" ] && kill -0 "$pid" 2>/dev/null; then continue; fi + if [ -n "$pid" ] && kill -0 "$pid" 2>/dev/null; then + state=$(ps -o state= -p "$pid" 2>/dev/null || true) + case "$state" in + Z*|z*) wait "$pid" 2>/dev/null || true; rm -f "$pf" ;; + *) continue ;; + esac + fi if [ -n "$pid" ]; then kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true echo "$(date -u +%Y-%m-%dT%H:%M:%SZ) restart $KEY" >>"$LOG" else echo "$(date -u +%Y-%m-%dT%H:%M:%SZ) start $KEY $IP:$PORT" >>"$LOG" @@ -768,7 +784,7 @@ done service_ports, ip_map: plan.ip_map.clone(), }; - save_host_access_runtime(state_dir, workspace, &runtime)?; + save_host_access_runtime(state_dir, &workspace, &runtime)?; log::info!("host access started (dns supervisor) pid={pid}"); Ok(runtime) } @@ -914,45 +930,74 @@ fn stop_host_access_processes_only( state_dir: &Path, workspace: &str, ) -> Result<(), Box> { - if let Some(rt) = load_host_access_runtime(state_dir, workspace)? { + let workspace = slugify_name(workspace); + if let Some(rt) = load_host_access_runtime(state_dir, &workspace)? { + let supervisor_marker = format!("{workspace}.dns-sup.sh"); for pid in &rt.pids { - let _ = Command::new("kill") - .args(["-TERM", &pid.to_string()]) - .status(); + if pid_command_contains_all(*pid, &[&supervisor_marker]) { + let _ = Command::new("kill") + .args(["-TERM", &pid.to_string()]) + .status(); + } } std::thread::sleep(Duration::from_millis(200)); for pid in &rt.pids { - let _ = Command::new("kill") - .args(["-KILL", &pid.to_string()]) - .status(); + if pid_command_contains_all(*pid, &[&supervisor_marker]) { + let _ = Command::new("kill") + .args(["-KILL", &pid.to_string()]) + .status(); + } } let piddir = state_dir .join(RUNTIME_SUBDIR) .join(format!("{workspace}.dns-pf-pids")); - if let Ok(entries) = fs::read_dir(piddir) { + if let Ok(entries) = fs::read_dir(&piddir) { for ent in entries.flatten() { if let Ok(s) = fs::read_to_string(ent.path()) { if let Ok(pid) = s.trim().parse::() { - let _ = Command::new("kill") - .args(["-TERM", &pid.to_string()]) - .status(); + if pid_command_contains_all(pid, &["kubectl", "port-forward"]) { + let _ = Command::new("kill") + .args(["-TERM", &pid.to_string()]) + .status(); + } } } + let _ = fs::remove_file(ent.path()); } + let _ = fs::remove_dir(&piddir); } } - clear_host_access_runtime(state_dir, workspace); + clear_host_access_runtime(state_dir, &workspace); Ok(()) } +fn pid_command_contains_all(pid: u32, needles: &[&str]) -> bool { + Command::new("ps") + .args(["-p", &pid.to_string(), "-o", "args="]) + .output() + .map(|output| { + let command = String::from_utf8_lossy(&output.stdout); + needles.iter().all(|needle| command.contains(needle)) + }) + .unwrap_or(false) +} + pub fn stop_host_access(state_dir: &Path, workspace: &str) -> Result<(), Box> { + let _dns_state_lock = acquire_dns_state_lock(state_dir)?; let rt = load_host_access_runtime(state_dir, workspace)?; stop_host_access_processes_only(state_dir, workspace)?; if let Some(rt) = rt { if !rt.ip_map.is_empty() { remove_loopback_aliases(&rt.ip_map.values().cloned().collect::>()); if let Ok(blocks) = collect_dns_blocks_from_runtimes(state_dir, Some(workspace)) { + let by_ns: BTreeMap> = + blocks.iter().cloned().collect(); + let _ = write_zone_file(state_dir, &by_ns); let _ = rebuild_hosts_from_blocks_noprompt(&blocks); + if blocks.is_empty() { + stop_macos_stub_dns(state_dir); + cluster_dns::remove_macos_resolver_noprompt(); + } } } } @@ -962,25 +1007,41 @@ pub fn stop_host_access(state_dir: &Path, workspace: &str) -> Result<(), Box)], ) -> Result<(), Box> { - use super::cluster_dns::{ - dns_os_config_present, hosts_lines_for_workspace, merge_hosts_file, run_privileged_shell, - PrivilegedPrompt, - }; + use super::cluster_dns::{hosts_lines_for_workspace, merge_hosts_file}; let mut lines = Vec::new(); for (ns, ips) in blocks { lines.extend(hosts_lines_for_workspace(ns, ips)); } let current = fs::read_to_string("/etc/hosts").unwrap_or_default(); let merged = merge_hosts_file(¤t, &lines); - if dns_os_config_present(&merged, &[]) { - return Ok(()); + cluster_dns::apply_privileged_dns_config_noprompt(&merged) +} + +fn stop_macos_stub_dns(state_dir: &Path) { + if !cfg!(target_os = "macos") { + return; } - let tmp = std::env::temp_dir().join(format!("hops-hosts-down-{}.tmp", std::process::id())); - fs::write(&tmp, &merged)?; - let script = format!("cp '{}' /etc/hosts && chmod 644 /etc/hosts", tmp.display()); - let res = run_privileged_shell(&script, PrivilegedPrompt::Never); - let _ = fs::remove_file(&tmp); - res + let runtime = state_dir.join(RUNTIME_SUBDIR); + let pid_path = runtime.join("macos-stub-dns.pid"); + let script_path = runtime.join("macos-stub-dns.py"); + if let Ok(raw) = fs::read_to_string(&pid_path) { + if let Ok(pid) = raw.trim().parse::() { + let owns_pid = Command::new("ps") + .args(["-p", &pid.to_string(), "-o", "args="]) + .output() + .map(|output| { + String::from_utf8_lossy(&output.stdout) + .contains(&script_path.to_string_lossy().to_string()) + }) + .unwrap_or(false); + if owns_pid { + let _ = Command::new("kill") + .args(["-TERM", &pid.to_string()]) + .status(); + } + } + } + let _ = fs::remove_file(pid_path); } fn collect_dns_blocks_from_runtimes( @@ -988,6 +1049,7 @@ fn collect_dns_blocks_from_runtimes( skip_workspace: Option<&str>, ) -> Result)>, Box> { let dir = state_dir.join(RUNTIME_SUBDIR); + let skip_workspace = skip_workspace.map(slugify_name); let mut by_ns: BTreeMap> = BTreeMap::new(); let Ok(entries) = fs::read_dir(&dir) else { return Ok(Vec::new()); @@ -998,7 +1060,7 @@ fn collect_dns_blocks_from_runtimes( continue; } let ws = name.trim_end_matches(".host-access.json"); - if skip_workspace == Some(ws) { + if skip_workspace.as_deref() == Some(ws) { continue; } let Ok(text) = fs::read_to_string(ent.path()) else { @@ -1058,4 +1120,35 @@ mod tests { vec![("auth".into(), "zitadel-zitadel".into(), Some(8080))] ); } + + #[test] + fn parser_ignores_pod_specific_cluster_dns_names() { + let refs = regex_lite_cluster_dns("http://pod-0.service.ns.svc.cluster.local:8080"); + assert!(refs.is_empty()); + } + + #[test] + fn runtime_path_slugifies_workspace_name() { + let path = runtime_path(Path::new("/state"), "../../My Workspace"); + assert_eq!( + path, + Path::new("/state/runtime/my-workspace.host-access.json") + ); + } + + #[test] + fn runtime_collection_slugifies_skipped_workspace_name() { + let dir = std::env::temp_dir().join(format!("hops-net-test-{}", uuid::Uuid::new_v4())); + let runtime = HostAccessRuntime { + namespace: "my-workspace".into(), + ip_map: BTreeMap::from([("my-workspace/removed-service".into(), "127.53.0.2".into())]), + ..Default::default() + }; + save_host_access_runtime(&dir, "My Workspace", &runtime).unwrap(); + + let blocks = collect_dns_blocks_from_runtimes(&dir, Some("My Workspace")).unwrap(); + + assert!(blocks.is_empty()); + let _ = fs::remove_dir_all(dir); + } } diff --git a/src/commands/local/workbench/reconcile.rs b/src/commands/local/workbench/reconcile.rs index 2579bd8..9c552a3 100644 --- a/src/commands/local/workbench/reconcile.rs +++ b/src/commands/local/workbench/reconcile.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize}; use serde_yaml::Value; use std::collections::{BTreeMap, BTreeSet}; use std::error::Error; +use std::fs::OpenOptions; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -86,6 +87,34 @@ pub trait KubectlApplier { ) -> Result<(), Box>; } +struct TemporaryValuesFile { + path: PathBuf, +} + +impl TemporaryValuesFile { + fn create(contents: &str) -> Result> { + let path = + std::env::temp_dir().join(format!("hops-lwb-values-{}.yaml", uuid::Uuid::new_v4())); + let temporary = Self { path }; + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options.open(&temporary.path)?; + file.write_all(contents.as_bytes())?; + Ok(temporary) + } +} + +impl Drop for TemporaryValuesFile { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + } +} + /// Real helm binary runner. pub struct SystemHelm; @@ -97,12 +126,7 @@ impl HelmRunner for SystemHelm { namespace: &str, values_yaml: &str, ) -> Result> { - let values_path = std::env::temp_dir().join(format!( - "hops-lwb-values-{}-{}.yaml", - std::process::id(), - release - )); - std::fs::write(&values_path, values_yaml)?; + let values_file = TemporaryValuesFile::create(values_yaml)?; let output = Command::new("helm") .args([ "template", @@ -111,10 +135,9 @@ impl HelmRunner for SystemHelm { "--namespace", namespace, "--values", - &values_path.to_string_lossy(), + &values_file.path.to_string_lossy(), ]) .output()?; - let _ = std::fs::remove_file(&values_path); if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); return Err(format!( @@ -151,10 +174,8 @@ impl KubectlApplier for SystemKubectl { // Apply one document at a time so a missing platform CRD (e.g. PSQLCluster // when the pack is not installed) does not prevent core Deploy/Service apply. let mut hard_errors = Vec::new(); - for doc in split_yaml_docs_owned(yaml) { - if doc.trim().is_empty() { - continue; - } + for doc in parse_yaml_docs(yaml)? { + let doc = serde_yaml::to_string(&doc)?; match crate::commands::local::kubectl_apply_stdin(&doc) { Ok(()) => {} Err(e) => { @@ -260,11 +281,7 @@ fn inventory_name(app_name: &str) -> String { fn managed_object_refs(yaml: &str) -> Result, Box> { let mut refs = BTreeSet::new(); - for doc in split_yaml_docs_owned(yaml) { - if doc.trim().is_empty() { - continue; - } - let value: Value = serde_yaml::from_str(&doc)?; + for value in parse_yaml_docs(yaml)? { let Some(root) = value.as_mapping() else { continue; }; @@ -408,23 +425,17 @@ pub fn inject_labels(workspace_name: &str, app_name: &str) -> BTreeMap Vec { - let mut parts = Vec::new(); - let mut buf = String::new(); - for line in s.lines() { - if line.trim() == "---" { - if !buf.trim().is_empty() { - parts.push(std::mem::take(&mut buf)); - } - } else { - buf.push_str(line); - buf.push('\n'); +fn parse_yaml_docs(s: &str) -> Result, Box> { + use serde::Deserialize; + + let mut docs = Vec::new(); + for document in serde_yaml::Deserializer::from_str(s) { + let value = Value::deserialize(document)?; + if !value.is_null() { + docs.push(value); } } - if !buf.trim().is_empty() { - parts.push(buf); - } - parts + Ok(docs) } fn inject_labels_into_value(value: &mut Value, labels: &BTreeMap) { @@ -488,12 +499,9 @@ pub fn render_labels_into_manifests( labels: &BTreeMap, ) -> Result> { let mut out_docs = Vec::new(); - for doc in split_yaml_docs_owned(rendered) { - if doc.trim().is_empty() { - continue; - } - let mut value: Value = serde_yaml::from_str(&doc) - .map_err(|e| format!("parse rendered manifest: {e}\n---\n{doc}"))?; + for mut value in + parse_yaml_docs(rendered).map_err(|e| format!("parse rendered manifest: {e}"))? + { inject_labels_into_value(&mut value, labels); out_docs.push(serde_yaml::to_string(&value)?); } @@ -733,11 +741,7 @@ fn is_app_workload_kind(api_version: &str, kind: &str) -> bool { /// 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() { - continue; - } - let mut value: Value = serde_yaml::from_str(&doc)?; + for mut value in parse_yaml_docs(yaml)? { if let Some(root) = value.as_mapping_mut() { let kind = root .get(Value::String("kind".into())) @@ -856,6 +860,19 @@ spec: ); } + #[test] + fn parser_preserves_document_markers_inside_block_scalars() { + let rendered = "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: sample\ndata:\n script: |\n before\n ---\n after\n---\napiVersion: v1\nkind: Service\nmetadata:\n name: sample\n"; + let labels = inject_labels("ws", "app"); + let out = render_labels_into_manifests(rendered, &labels).unwrap(); + let docs = parse_yaml_docs(&out).unwrap(); + assert_eq!(docs.len(), 2); + assert_eq!( + docs[0]["data"]["script"].as_str(), + Some("before\n---\nafter\n") + ); + } + #[test] fn inventory_diff_prunes_only_removed_exact_objects() { let previous = managed_object_refs(