diff --git a/src/commands/local/doctor.rs b/src/commands/local/doctor.rs index 979a445..e7f8967 100644 --- a/src/commands/local/doctor.rs +++ b/src/commands/local/doctor.rs @@ -1,4 +1,4 @@ -use super::run_cmd_output; +use super::{run_cmd_output, MANAGED_BY_LABEL, PROVIDER_INSTALL_MANAGED_BY}; use std::error::Error; /// `hops local doctor` — verify what `hops local start` set up on the current @@ -124,7 +124,35 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) { cond_detail("Healthy", &healthy), ); - // Drift detector: the Provider must point at its OWN per-provider DRC. + // A provider installed via `hops provider install` (e.g. a fork built from + // source) is labeled and uses the `-runtime` convention instead of + // the bootstrap `local-dev-` DRC. Recognize that so it reads as an + // intentional custom install, not drift — while still verifying cluster-admin. + let custom_install = provider_label(e.provider_name, MANAGED_BY_LABEL).as_deref() + == Some(PROVIDER_INSTALL_MANAGED_BY); + let (exp_drc, exp_binding, exp_sa) = if custom_install { + ( + format!("{}-runtime", e.provider_name), + format!("{}-cluster-admin", e.provider_name), + e.provider_name.to_string(), + ) + } else { + (e.drc.to_string(), e.binding.to_string(), e.sa.to_string()) + }; + d.check( + "install mode", + true, + if custom_install { + format!( + "custom install via `hops provider install` (package: {})", + provider_package(e.provider_name) + ) + } else { + "bootstrap (hops local start)".to_string() + }, + ); + + // The Provider must point at its expected per-provider DRC. let rc = jsonpath(&[ "get", "provider.pkg.crossplane.io", @@ -133,7 +161,7 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) { "jsonpath={.spec.runtimeConfigRef.name}", ]) .unwrap_or_default(); - let rc_ok = rc == e.drc; + let rc_ok = rc == exp_drc; d.check( "runtimeConfigRef pinned to its own DRC", rc_ok, @@ -143,19 +171,19 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) { format!( "runtimeConfigRef is \"{}\" (expected \"{}\") — drifted; provider lacks its cluster-admin ServiceAccount and cannot observe XRs", if rc.is_empty() { "" } else { &rc }, - e.drc + exp_drc ) }, ); - let drc_ok = exists(&["get", "deploymentruntimeconfig", e.drc]); + let drc_ok = exists(&["get", "deploymentruntimeconfig", exp_drc.as_str()]); d.check( "DeploymentRuntimeConfig present", drc_ok, if drc_ok { String::new() } else { - format!("DeploymentRuntimeConfig \"{}\" missing", e.drc) + format!("DeploymentRuntimeConfig \"{}\" missing", exp_drc) }, ); @@ -163,21 +191,21 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) { let role = jsonpath(&[ "get", "clusterrolebinding", - e.binding, + exp_binding.as_str(), "-o", "jsonpath={.roleRef.name}", ]); let subjects = jsonpath(&[ "get", "clusterrolebinding", - e.binding, + exp_binding.as_str(), "-o", "jsonpath={.subjects[?(@.kind==\"ServiceAccount\")].name}", ]); let binding_ok = role.as_deref() == Some("cluster-admin") && subjects .as_deref() - .map(|s| s.split_whitespace().any(|n| n == e.sa)) + .map(|s| s.split_whitespace().any(|n| n == exp_sa.as_str())) .unwrap_or(false); d.check( "cluster-admin binding -> pinned SA", @@ -187,7 +215,7 @@ fn check_provider(d: &mut Doctor, e: &ProviderExpectation) { } else { format!( "ClusterRoleBinding \"{}\" missing or not binding cluster-admin to ServiceAccount \"{}\"", - e.binding, e.sa + exp_binding, exp_sa ) }, ); @@ -229,6 +257,34 @@ fn cond_detail(cond: &str, status: &str) -> String { } } +/// Read a metadata label off a Provider via `-o json` (robust against the dotted +/// label key that jsonpath escaping mishandles). +fn provider_label(provider: &str, key: &str) -> Option { + let json = run_cmd_output( + "kubectl", + &["get", "provider.pkg.crossplane.io", provider, "-o", "json"], + ) + .ok()?; + let v: serde_json::Value = serde_json::from_str(&json).ok()?; + v.get("metadata")? + .get("labels")? + .get(key)? + .as_str() + .map(|s| s.to_string()) +} + +/// The Provider's spec.package (image ref) — shown for custom installs. +fn provider_package(provider: &str) -> String { + jsonpath(&[ + "get", + "provider.pkg.crossplane.io", + provider, + "-o", + "jsonpath={.spec.package}", + ]) + .unwrap_or_default() +} + /// True when `kubectl get ` finds the resource. Uses `--ignore-not-found` /// so a missing resource is `false` rather than an error. fn exists(get_args: &[&str]) -> bool { diff --git a/src/commands/local/mod.rs b/src/commands/local/mod.rs index 0fa9967..0b4ac62 100644 --- a/src/commands/local/mod.rs +++ b/src/commands/local/mod.rs @@ -20,6 +20,14 @@ use std::process::{Command, Stdio}; const LOCAL_STATE_DIR: &str = ".hops/local"; const REPO_CACHE_DIR: &str = "repo-cache"; +/// metadata.labels key recording who manages a resource (k8s recommended label). +pub const MANAGED_BY_LABEL: &str = "app.kubernetes.io/managed-by"; +/// `MANAGED_BY_LABEL` value that `hops provider install` stamps on the Provider, +/// DeploymentRuntimeConfig, and ClusterRoleBinding it manages — so `hops local +/// doctor` can tell an intentional custom install (e.g. a forked provider built +/// from source) apart from accidental drift. +pub const PROVIDER_INSTALL_MANAGED_BY: &str = "hops-provider-install"; + /// Env var checked by kubectl helpers to inject `--context `. pub const HOPS_KUBE_CONTEXT_ENV: &str = "HOPS_KUBE_CONTEXT"; diff --git a/src/commands/provider/install.rs b/src/commands/provider/install.rs index b894fc8..c8ad920 100644 --- a/src/commands/provider/install.rs +++ b/src/commands/provider/install.rs @@ -5,7 +5,7 @@ use crate::commands::local::package_install::{ }; use crate::commands::local::{ kubectl_apply_stdin, run_cmd, run_cmd_output, sync_registry_hosts_entry, - HOPS_KUBE_CONTEXT_ENV, + HOPS_KUBE_CONTEXT_ENV, MANAGED_BY_LABEL, PROVIDER_INSTALL_MANAGED_BY, }; use clap::Args; use serde::Deserialize; @@ -1002,6 +1002,8 @@ fn build_provider_yaml( kind: Provider metadata: name: {name} + labels: + {MANAGED_BY_LABEL}: {PROVIDER_INSTALL_MANAGED_BY} spec: package: {package_ref} packagePullPolicy: Always @@ -1028,6 +1030,8 @@ fn build_runtime_config_yaml( kind: DeploymentRuntimeConfig metadata: name: {name} + labels: + {MANAGED_BY_LABEL}: {PROVIDER_INSTALL_MANAGED_BY} spec: serviceAccountTemplate: metadata: @@ -1056,6 +1060,8 @@ fn build_cluster_role_binding_yaml(name: &str, service_account: &str) -> String kind: ClusterRoleBinding metadata: name: {name} + labels: + {MANAGED_BY_LABEL}: {PROVIDER_INSTALL_MANAGED_BY} subjects: - kind: ServiceAccount name: {service_account} @@ -1084,6 +1090,7 @@ mod tests { assert!(yaml.contains("name: provider-helm")); assert!(yaml.contains("package: registry.example/provider-helm:dev-abc")); assert!(yaml.contains("name: provider-helm-runtime")); + assert!(yaml.contains("app.kubernetes.io/managed-by: hops-provider-install")); assert!(!yaml.contains("skipDependencyResolution")); } @@ -1107,6 +1114,7 @@ mod tests { assert!(!without.contains("package-runtime")); assert!(!without.contains("deploymentTemplate")); assert!(without.contains("serviceAccountTemplate")); + assert!(without.contains("app.kubernetes.io/managed-by: hops-provider-install")); } #[test] @@ -1116,6 +1124,7 @@ mod tests { assert!(yaml.contains("name: p-cluster-admin")); assert!(yaml.contains("name: p\n namespace: crossplane-system")); assert!(yaml.contains("name: cluster-admin")); + assert!(yaml.contains("app.kubernetes.io/managed-by: hops-provider-install")); } const TEST_LOCAL_REGISTRY: &str = "registry.crossplane-system.svc.cluster.local:5000";