cluster: bug fixes to kubeconfig gen (#6459)

- treat failure to write kubeconfigs as a cluster connection failure - if runtime dir is not available, write kubeconfigs in a state dir instead (spec says that if the runtime dir doesn't exist, it's up to the implementer to find a better dir, but isn't opinionated about what dir we use) Signed-off-by: Nick Santos <nick.santos@docker.com>
tilt-dev · Nov 4, 2024 · 9622112 · 9622112
1 parent 5285ead
commit 9622112
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 22 deletions.
diff --git a/internal/controllers/core/cluster/reconciler.go b/internal/controllers/core/cluster/reconciler.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sync"
 	"time"
 
 	"github.com/docker/docker/client"
@@ -39,6 +40,8 @@ import (
 
 const ArchUnknown string = "unknown"
 
+var runtimeDirWarning = sync.Once{}
+
 const (
 	clientInitBackoff        = 30 * time.Second
 	clientHealthPollInterval = 15 * time.Second
@@ -369,7 +372,11 @@ func (r *Reconciler) populateK8sMetadata(ctx context.Context, clusterNN types.Na
 			k8sStatus.Namespace = context.Namespace
 			k8sStatus.Cluster = context.Cluster
 		}
-		k8sStatus.ConfigPath = r.writeFrozenKubeConfig(ctx, clusterNN, apiConfig)
+		configPath, err := r.writeFrozenKubeConfig(ctx, clusterNN, apiConfig)
+		if err != nil {
+			conn.initError = err.Error()
+		}
+		k8sStatus.ConfigPath = configPath
 
 		conn.connStatus = &v1alpha1.ClusterConnectionStatus{
 			Kubernetes: k8sStatus,
@@ -384,49 +391,68 @@ func (r *Reconciler) populateK8sMetadata(ctx context.Context, clusterNN types.Na
 	}
 }
 
-func (r *Reconciler) writeFrozenKubeConfig(ctx context.Context, nn types.NamespacedName, config *api.Config) string {
+func (r *Reconciler) openFrozenKubeConfigFile(ctx context.Context, nn types.NamespacedName) (string, *os.File, error) {
+	path, err := r.base.RuntimeFile(
+		filepath.Join(string(r.apiServerName), "cluster", fmt.Sprintf("%s.yml", nn.Name)))
+	if err == nil {
+		var f *os.File
+		f, err = os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
+		if err == nil {
+			return path, f, nil
+		}
+	}
+
+	// From the spec: emit a warning if the runtime dir isn't available.
+	// https://specifications.freedesktop.org/basedir-spec/latest/
+	runtimeDirWarning.Do(func() {
+		logger.Get(ctx).Warnf(
+			"XDG Runtime directory not available. Storing temp kubeconfigs in: %s. Error: %v", path, err)
+	})
+
+	path, err = r.base.StateFile(
+		filepath.Join(string(r.apiServerName), "cluster", fmt.Sprintf("%s.yml", nn.Name)))
+	if err != nil {
+		return "", nil, fmt.Errorf("storing temp kubeconfigs: %v", err)
+	}
+
+	f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
+	if err != nil {
+		return "", nil, fmt.Errorf("storing temp kubeconfigs: %v", err)
+	}
+	return path, f, nil
+}
+
+func (r *Reconciler) writeFrozenKubeConfig(ctx context.Context, nn types.NamespacedName, config *api.Config) (string, error) {
 	config = config.DeepCopy()
 	err := api.MinifyConfig(config)
 	if err != nil {
-		logger.Get(ctx).Warnf("Minifying Kubernetes config: %v", err)
-		return ""
+		return "", fmt.Errorf("minifying Kubernetes config: %v", err)
 	}
 
 	err = api.FlattenConfig(config)
 	if err != nil {
-		logger.Get(ctx).Warnf("Flattening Kubernetes config: %v", err)
-		return ""
+		return "", fmt.Errorf("flattening Kubernetes config: %v", err)
 	}
 
 	obj, err := latest.Scheme.ConvertToVersion(config, latest.ExternalVersion)
 	if err != nil {
-		logger.Get(ctx).Warnf("Converting Kubernetes config: %v", err)
-		return ""
+		return "", fmt.Errorf("converting Kubernetes config: %v", err)
 	}
 
 	printer := printers.YAMLPrinter{}
-	path, err := r.base.RuntimeFile(
-		filepath.Join(string(r.apiServerName), "cluster", fmt.Sprintf("%s.yml", nn.Name)))
-	if err != nil {
-		logger.Get(ctx).Warnf("Writing Kubernetes config: %v", err)
-		return ""
-	}
-
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
+	path, f, err := r.openFrozenKubeConfigFile(ctx, nn)
 	if err != nil {
-		logger.Get(ctx).Warnf("Writing Kubernetes config: %v", err)
-		return ""
+		return "", err
 	}
 	defer func() {
 		_ = f.Close()
 	}()
 
 	err = printer.PrintObj(obj, f)
 	if err != nil {
-		logger.Get(ctx).Warnf("Writing Kubernetes config: %v", err)
-		return ""
+		return "", fmt.Errorf("writing kubeconfig: %v", err)
 	}
-	return path
+	return path, nil
 }
 
 func (r *Reconciler) populateDockerMetadata(ctx context.Context, conn *connection) {

diff --git a/internal/controllers/core/cluster/reconciler_test.go b/internal/controllers/core/cluster/reconciler_test.go
@@ -3,6 +3,7 @@ package cluster
 import (
 	"errors"
 	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -260,13 +261,67 @@ func TestDockerArch(t *testing.T) {
 	}
 }
 
+func TestKubeconfig_RuntimeDirImmutable(t *testing.T) {
+	f := newFixture(t)
+	cluster := &v1alpha1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{Name: "default"},
+		Spec: v1alpha1.ClusterSpec{
+			Connection: &v1alpha1.ClusterConnection{
+				Kubernetes: &v1alpha1.KubernetesClusterConnection{},
+			},
+		},
+	}
+
+	p, err := f.base.RuntimeFile(filepath.Join("tilt-default", "cluster", "default.yml"))
+	require.NoError(t, err)
+	runtimeFile, _ := os.OpenFile(p, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0400)
+	_ = runtimeFile.Close()
+
+	nn := types.NamespacedName{Name: "default"}
+	f.Create(cluster)
+	f.MustGet(nn, cluster)
+
+	configPath := cluster.Status.Connection.Kubernetes.ConfigPath
+	require.NotEqual(t, configPath, "")
+}
+
+func TestKubeconfig_RuntimeAndStateDirImmutable(t *testing.T) {
+	f := newFixture(t)
+	cluster := &v1alpha1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{Name: "default"},
+		Spec: v1alpha1.ClusterSpec{
+			Connection: &v1alpha1.ClusterConnection{
+				Kubernetes: &v1alpha1.KubernetesClusterConnection{},
+			},
+		},
+	}
+
+	p, err := f.base.RuntimeFile(filepath.Join("tilt-default", "cluster", "default.yml"))
+	require.NoError(t, err)
+	runtimeFile, _ := os.OpenFile(p, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0400)
+	_ = runtimeFile.Close()
+
+	p, err = f.base.StateFile(filepath.Join("tilt-default", "cluster", "default.yml"))
+	require.NoError(t, err)
+	stateFile, _ := os.OpenFile(p, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0400)
+	_ = stateFile.Close()
+
+	nn := types.NamespacedName{Name: "default"}
+	f.Create(cluster)
+	f.MustGet(nn, cluster)
+
+	require.Equal(t, cluster.Status.Connection.Kubernetes.ConfigPath, "")
+	require.Contains(t, cluster.Status.Error, "storing temp kubeconfigs")
+}
+
 type fixture struct {
 	*fake.ControllerFixture
 	r            *Reconciler
 	ma           *analytics.MemoryAnalytics
 	clock        clockwork.FakeClock
 	k8sClient    *k8s.FakeK8sClient
 	dockerClient *docker.FakeClient
+	base         xdg.FakeBase
 	requeues     <-chan indexer.RequeueForTestResult
 }
 
@@ -299,6 +354,7 @@ func newFixture(t *testing.T) *fixture {
 		k8sClient:         k8sClient,
 		dockerClient:      dockerClient,
 		requeues:          requeueChan,
+		base:              base,
 	}
 }
 

diff --git a/internal/xdg/fake.go b/internal/xdg/fake.go
@@ -13,7 +13,7 @@ type FakeBase struct {
 var _ Base = FakeBase{}
 
 func (b FakeBase) createPath(prefix, relPath string) (string, error) {
-	p := filepath.Join(b.Dir, "cache", relPath)
+	p := filepath.Join(b.Dir, prefix, relPath)
 	dir := filepath.Dir(p)
 	err := os.MkdirAll(dir, os.ModeDir|0700)
 	if err != nil {