Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/virtwork/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func runE(cmd *cobra.Command, args []string) error {
var c client.Client
if !cfg.DryRun {
var contextName string
c, contextName, err = cluster.Connect(cfg.KubeconfigPath)
c, contextName, err = cluster.Connect(cluster.ResolveKubeconfigPath(cfg.KubeconfigPath))
if err != nil {
return fmt.Errorf("connecting to cluster: %w", err)
}
Expand Down Expand Up @@ -627,7 +627,7 @@ func cleanupE(cmd *cobra.Command, args []string) error {
}

// Connect to cluster early to capture context for audit
c, contextName, err := cluster.Connect(cfg.KubeconfigPath)
c, contextName, err := cluster.Connect(cluster.ResolveKubeconfigPath(cfg.KubeconfigPath))
if err != nil {
return fmt.Errorf("connecting to cluster: %w", err)
}
Expand Down
19 changes: 17 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ These apply to both `virtwork run` and `virtwork cleanup`.
| Flag | Env Var | YAML Key | Default | Description |
|---|---|---|---|---|
| `--namespace` | `VIRTWORK_NAMESPACE` | `namespace` | `virtwork` | Kubernetes namespace for VMs and supporting resources |
| `--kubeconfig` | `VIRTWORK_KUBECONFIG` | `kubeconfig` | _empty (in-cluster, then `~/.kube/config`)_ | Path to kubeconfig file |
| `--kubeconfig` | `VIRTWORK_KUBECONFIG` | `kubeconfig` | _empty (in-cluster, then `~/.kube/config`)_ | Path to kubeconfig file (see [kubeconfig resolution](#kubeconfig-resolution)) |
| `--config` | — | — | _empty_ | Path to a YAML config file to merge in |
| `--verbose` | `VIRTWORK_VERBOSE` | `verbose` | `false` | Switch the logger from INFO to DEBUG |
| `--audit` | `VIRTWORK_AUDIT` | `audit` | `true` | Enable SQLite audit tracking |
Expand Down Expand Up @@ -80,7 +80,7 @@ Global flags (`--namespace`, `--kubeconfig`, `--audit`, `--no-audit`, `--audit-d
| `VIRTWORK_CPU_CORES` | int | `2` | flag-bound | Default per-VM CPU cores |
| `VIRTWORK_DATA_DISK_SIZE` | string | `10Gi` | flag-bound | Default data disk size |
| `VIRTWORK_DRY_RUN` | bool | `false` | flag-bound | Default for `--dry-run` |
| `VIRTWORK_KUBECONFIG` | string | _empty_ | flag-bound | Kubeconfig path |
| `VIRTWORK_KUBECONFIG` | string | _empty_ | flag-bound | Kubeconfig path (takes precedence over `KUBECONFIG`; see [kubeconfig resolution](#kubeconfig-resolution)) |
| `VIRTWORK_MEMORY` | string | `2Gi` | flag-bound | Default per-VM memory |
| `VIRTWORK_NAMESPACE` | string | `virtwork` | flag-bound | Target namespace |
| `VIRTWORK_SSH_AUTHORIZED_KEYS` | string (csv) | _empty_ | env-only | Comma-separated SSH public keys |
Expand Down Expand Up @@ -255,6 +255,21 @@ flowchart TD
DEF -->|4th| MERGE
```

### Kubeconfig resolution

The `kubeconfig` key has an additional resolution layer handled by `cluster.ResolveKubeconfigPath` before the connection is established:

```
--kubeconfig / VIRTWORK_KUBECONFIG > KUBECONFIG env var > in-cluster service-account > ~/.kube/config
```

1. **Explicit path** — `--kubeconfig` flag or `VIRTWORK_KUBECONFIG` env var (resolved via the standard Viper priority chain above). When set, in-cluster detection is skipped entirely.
2. **`KUBECONFIG` env var** — the standard Kubernetes variable. Checked only when no explicit path is provided.
3. **In-cluster service-account** — `rest.InClusterConfig()` is attempted only when *no* kubeconfig path is resolved from steps 1–2.
4. **Default loading rules** — `~/.kube/config` (via `clientcmd` default loading rules) is the final fallback.

This means setting `VIRTWORK_KUBECONFIG` in a CI/CD pipeline or ConfigMap will always win over `KUBECONFIG`, and any explicit path will bypass in-cluster detection — even when running inside a pod.

How it works in code (`internal/config/config.go` → `LoadConfig`):

1. `SetDefaults(v)` seeds Viper with the built-in defaults.
Expand Down
81 changes: 56 additions & 25 deletions internal/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,40 +26,48 @@ func NewScheme() *runtime.Scheme {
return scheme
}

// Connect creates a controller-runtime client.Client. It first attempts
// in-cluster configuration; on failure it falls back to the kubeconfig at
// the given path (checking the KUBECONFIG env var when the path is empty).
// Both failures produce a wrapped error.
// ResolveKubeconfigPath returns the kubeconfig file path to use given
// the explicit path from Viper (--kubeconfig flag / VIRTWORK_KUBECONFIG).
// Precedence: explicitPath > KUBECONFIG env var > "" (triggers in-cluster
// or default ~/.kube/config resolution in Connect).
func ResolveKubeconfigPath(explicitPath string) string {
if explicitPath != "" {
return explicitPath
}
if v := os.Getenv("KUBECONFIG"); v != "" {
return v
}
return ""
}

// Connect creates a controller-runtime client.Client using the following
// kubeconfig resolution order:
//
// 1. kubeconfigPath (already resolved via ResolveKubeconfigPath: --kubeconfig > VIRTWORK_KUBECONFIG > KUBECONFIG)
// 2. In-cluster service-account token (only attempted when kubeconfigPath is empty)
// 3. Default loading rules (~/.kube/config)
func Connect(kubeconfigPath string) (client.Client, string, error) {
scheme := NewScheme()

var restConfig *rest.Config
var contextName string
if kubeconfigPath == "" {
kubeconfigPath = os.Getenv("KUBECONFIG")
}
var err error

restConfig, err := rest.InClusterConfig()
if err != nil {
// Not in-cluster, load from kubeconfig
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
if kubeconfigPath != "" {
loadingRules.ExplicitPath = kubeconfigPath
}
configOverrides := &clientcmd.ConfigOverrides{}
kubeConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, configOverrides)
restConfig, err = kubeConfig.ClientConfig()
if kubeconfigPath != "" {
restConfig, contextName, err = configFromKubeconfig(kubeconfigPath)
if err != nil {
return nil, "", fmt.Errorf("failed to build kubeconfig from %q: %w", kubeconfigPath, err)
return nil, "", err
}

// Get the current context name
rawConfig, err := kubeConfig.RawConfig()
} else {
restConfig, err = rest.InClusterConfig()
if err == nil {
contextName = rawConfig.CurrentContext
contextName = "in-cluster"
} else {
restConfig, contextName, err = configFromKubeconfig("")
if err != nil {
return nil, "", err
}
}
} else {
// In-cluster - no context name
contextName = "in-cluster"
}

c, err := client.New(restConfig, client.Options{Scheme: scheme})
Expand All @@ -69,3 +77,26 @@ func Connect(kubeconfigPath string) (client.Client, string, error) {

return c, contextName, nil
}

// configFromKubeconfig loads a rest.Config and context name from a kubeconfig
// file. When path is empty, default loading rules (~/.kube/config) apply.
func configFromKubeconfig(path string) (*rest.Config, string, error) {
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
if path != "" {
loadingRules.ExplicitPath = path
}
kubeConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
loadingRules, &clientcmd.ConfigOverrides{})

restConfig, err := kubeConfig.ClientConfig()
if err != nil {
return nil, "", fmt.Errorf("failed to build kubeconfig from %q: %w", path, err)
}

var contextName string
rawConfig, rawErr := kubeConfig.RawConfig()
if rawErr == nil {
contextName = rawConfig.CurrentContext
}
return restConfig, contextName, nil
}
126 changes: 99 additions & 27 deletions internal/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,30 +45,58 @@ var _ = Describe("NewScheme", func() {
})
})

var _ = Describe("ResolveKubeconfigPath", func() {
var origKubeconfig string
var hadKubeconfig bool

BeforeEach(func() {
origKubeconfig, hadKubeconfig = os.LookupEnv("KUBECONFIG")
_ = os.Unsetenv("KUBECONFIG")
})

AfterEach(func() {
if hadKubeconfig {
_ = os.Setenv("KUBECONFIG", origKubeconfig)
} else {
_ = os.Unsetenv("KUBECONFIG")
}
})

It("should return explicit path when provided", func() {
_ = os.Setenv("KUBECONFIG", "/from-env")
result := cluster.ResolveKubeconfigPath("/explicit/path")
Expect(result).To(Equal("/explicit/path"))
})

It("should fall back to KUBECONFIG env when explicit path is empty", func() {
_ = os.Setenv("KUBECONFIG", "/from-kubeconfig-env")
result := cluster.ResolveKubeconfigPath("")
Expect(result).To(Equal("/from-kubeconfig-env"))
})

It("should return empty when no explicit path and no KUBECONFIG", func() {
result := cluster.ResolveKubeconfigPath("")
Expect(result).To(BeEmpty())
})
})

var _ = Describe("Connect", func() {
It("should return error when both in-cluster and kubeconfig fail", func() {
// Ensure we're not running in-cluster by unsetting the env var
origHost := os.Getenv("KUBERNETES_SERVICE_HOST")
// Helper to disable in-cluster detection for tests running outside a pod
disableInCluster := func() func() {
origHost, hadHost := os.LookupEnv("KUBERNETES_SERVICE_HOST")
_ = os.Unsetenv("KUBERNETES_SERVICE_HOST")
defer func() {
if origHost != "" {
return func() {
if hadHost {
_ = os.Setenv("KUBERNETES_SERVICE_HOST", origHost)
} else {
_ = os.Unsetenv("KUBERNETES_SERVICE_HOST")
}
}()

_, _, err := cluster.Connect("/nonexistent/kubeconfig/path")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("kubeconfig"))
})
}
}

It("should use kubeconfig path when provided", func() {
// Create a minimal but invalid kubeconfig to confirm the path is used
writeFakeKubeconfig := func(contextName string) string {
tmpFile, err := os.CreateTemp("", "kubeconfig-*.yaml")
Expect(err).NotTo(HaveOccurred())
defer func() {
_ = os.Remove(tmpFile.Name())
}()

_, err = tmpFile.WriteString(`apiVersion: v1
kind: Config
clusters:
Expand All @@ -79,29 +107,73 @@ contexts:
- context:
cluster: test
user: test
name: test
current-context: test
name: ` + contextName + `
current-context: ` + contextName + `
users:
- name: test
user:
token: fake-token
`)
Expect(err).NotTo(HaveOccurred())
_ = tmpFile.Close()
return tmpFile.Name()
}

// Ensure we're not running in-cluster
origHost := os.Getenv("KUBERNETES_SERVICE_HOST")
_ = os.Unsetenv("KUBERNETES_SERVICE_HOST")
It("should return error when both in-cluster and kubeconfig fail", func() {
restore := disableInCluster()
defer restore()

_, _, err := cluster.Connect("/nonexistent/kubeconfig/path")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("kubeconfig"))
})

It("should use explicit kubeconfig path when provided", func() {
restore := disableInCluster()
defer restore()

path := writeFakeKubeconfig("explicit-ctx")
defer func() { _ = os.Remove(path) }()

c, contextName, err := cluster.Connect(path)
Expect(err).NotTo(HaveOccurred())
Expect(c).NotTo(BeNil())
Expect(contextName).To(Equal("explicit-ctx"))
})

It("should skip in-cluster when explicit path is provided", func() {
path := writeFakeKubeconfig("explicit-over-incluster")
defer func() { _ = os.Remove(path) }()

c, contextName, err := cluster.Connect(path)
Expect(err).NotTo(HaveOccurred())
Expect(c).NotTo(BeNil())
Expect(contextName).To(Equal("explicit-over-incluster"))
})

It("should fall back to default rules when path is empty and not in-cluster", func() {
restore := disableInCluster()
defer restore()

origKubeconfig, had := os.LookupEnv("KUBECONFIG")
defer func() {
if origHost != "" {
_ = os.Setenv("KUBERNETES_SERVICE_HOST", origHost)
if had {
_ = os.Setenv("KUBECONFIG", origKubeconfig)
} else {
_ = os.Unsetenv("KUBECONFIG")
}
}()

// Connect should succeed (client creation doesn't dial the server)
c, contextName, err := cluster.Connect(tmpFile.Name())
path := writeFakeKubeconfig("default-rules-ctx")
defer func() { _ = os.Remove(path) }()
_ = os.Setenv("KUBECONFIG", path)

// ResolveKubeconfigPath("") would pick up KUBECONFIG, but
// Connect("") should use default loading rules which also
// check KUBECONFIG. Verify it works with an empty path.
c, contextName, err := cluster.Connect("")
Expect(err).NotTo(HaveOccurred())
Expect(c).NotTo(BeNil())
Expect(contextName).NotTo(BeEmpty())
Expect(contextName).To(Equal("default-rules-ctx"))
})
})
6 changes: 3 additions & 3 deletions internal/testutil/binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ var (
// 1. VIRTWORK_BINARY environment variable (if set)
// 2. Build from source (on first call, cached thereafter)
//
// The binary is built with CGO_ENABLED=0 for portability and placed in a
// temporary directory. The build finds the module root automatically by
// The binary is built with CGO_ENABLED=1 (required by go-sqlite3) and placed
// in a temporary directory. The build finds the module root automatically by
// walking up from this file's location.
//
// Example:
Expand Down Expand Up @@ -72,7 +72,7 @@ func buildBinary() (string, error) {
//nolint:gosec
cmd := exec.Command("go", "build", "-o", outputPath, "./cmd/virtwork")
cmd.Dir = moduleRoot
cmd.Env = append(os.Environ(), "CGO_ENABLED=0")
cmd.Env = append(os.Environ(), "CGO_ENABLED=1")

var stderr bytes.Buffer
cmd.Stderr = &stderr
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var _ = Describe("virtwork cleanup", Label("slow"), func() {
"cleanup", "--namespace", namespace)
Expect(err).NotTo(HaveOccurred())
Expect(exitCode).To(Equal(0))
Expect(stdout).To(ContainSubstring("1 VMs deleted"))
Expect(stdout).To(ContainSubstring(`"vms_deleted":1`))
})

It("should not delete the namespace by default", func() {
Expand Down Expand Up @@ -79,6 +79,6 @@ var _ = Describe("virtwork cleanup", Label("slow"), func() {
"cleanup", "--namespace", namespace)
Expect(err).NotTo(HaveOccurred())
Expect(exitCode2).To(Equal(0))
Expect(stdout).To(ContainSubstring("0 VMs deleted"))
Expect(stdout).To(ContainSubstring(`"vms_deleted":0`))
})
})
2 changes: 1 addition & 1 deletion tests/e2e/dryrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,6 @@ var _ = Describe("virtwork run --dry-run", func() {
"run", "--dry-run", "--workloads", "nonexistent")
Expect(err).NotTo(HaveOccurred())
Expect(exitCode).NotTo(Equal(0))
Expect(stderr).To(ContainSubstring("unknown workload"))
Expect(stderr).To(ContainSubstring("workload not found"))
})
})
4 changes: 2 additions & 2 deletions tests/e2e/fullcycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ var _ = Describe("Full deployment cycle", Label("slow"), func() {
"cleanup", "--namespace", namespace)
Expect(err).NotTo(HaveOccurred())
Expect(exitCode).To(Equal(0))
Expect(stdout).To(ContainSubstring("1 VMs deleted"))
Expect(stdout).To(ContainSubstring(`"vms_deleted":1`))

// Step 4: Verify resources are gone (KubeVirt finalizers may delay removal)
Eventually(func() int {
Expand Down Expand Up @@ -90,6 +90,6 @@ var _ = Describe("Full deployment cycle", Label("slow"), func() {
"cleanup", "--namespace", namespace)
Expect(err).NotTo(HaveOccurred())
Expect(exitCode).To(Equal(0))
Expect(stdout).To(ContainSubstring("2 VMs deleted"))
Expect(stdout).To(ContainSubstring(`"vms_deleted":2`))
})
})
Loading