feat(extensions): auto registration and repo subpaths (#6371)

* test for existing fake nesting behavior Signed-off-by: Alex Vidal <[email protected]> * introduce support for extension_repo.prefix Signed-off-by: Alex Vidal <[email protected]> * implement repo-wide extension paths Signed-off-by: Alex Vidal <[email protected]> * test for extension loading when a repo has a prefix and a subpath Signed-off-by: Alex Vidal <[email protected]> * update path and prefix test to use subdirectories Signed-off-by: Alex Vidal <[email protected]> * chore: make update-codegen Signed-off-by: Alex Vidal <[email protected]> * wip: split up ensureExtension and write tests Signed-off-by: Alex Vidal <[email protected]> * fixup: use correct json/protobuf field name casing Signed-off-by: Alex Vidal <[email protected]> --------- Signed-off-by: Alex Vidal <[email protected]>
tilt-dev · May 20, 2024 · 153ae0d · 153ae0d
1 parent 0a22ed4
commit 153ae0d
Show file tree

Hide file tree

Showing 10 changed files with 992 additions and 409 deletions.
diff --git a/internal/controllers/core/extension/reconciler.go b/internal/controllers/core/extension/reconciler.go
@@ -126,7 +126,11 @@ func (r *Reconciler) apply(ext *v1alpha1.Extension, repo *v1alpha1.ExtensionRepo
 		return v1alpha1.ExtensionStatus{Error: fmt.Sprintf("extension repo not loaded: %s", ext.Spec.RepoName)}
 	}
 
-	absPath := filepath.Join(repo.Status.Path, ext.Spec.RepoPath, "Tiltfile")
+	// The absolute path to an extension's Tiltfile is a combination of:
+	//  - path to the repository on disk (repo.Status.Path)
+	//  - specified path for all extensions within the repository (repo.Spec.GitSubpath)
+	//  - specified path for the extension (ext.Spec.RepoPath)
+	absPath := filepath.Join(repo.Status.Path, repo.Spec.GitSubpath, ext.Spec.RepoPath, "Tiltfile")
 
 	// Make sure the user isn't trying to use path tricks to "escape" the repo.
 	if !ospath.IsChild(repo.Status.Path, absPath) {

diff --git a/internal/controllers/core/extension/reconciler_test.go b/internal/controllers/core/extension/reconciler_test.go
@@ -49,7 +49,51 @@ func TestDefault(t *testing.T) {
 	})
 
 	assert.Equal(t, f.ma.Counts, []analytics.CountEvent{
-		analytics.CountEvent{
+		{
+			Name: "api.extension.load",
+			Tags: map[string]string{
+				"ext_path":      "my-ext",
+				"repo_type":     "file",
+				"repo_url_hash": tiltanalytics.HashSHA1(repo.Spec.URL),
+			},
+			N: 1,
+		},
+	})
+}
+
+// Assert that the repo extension path, if specified, is used for extension location
+func TestRepoSubpath(t *testing.T) {
+	f := newFixture(t)
+	repo := f.setupRepoSubpath("subpath")
+
+	ext := v1alpha1.Extension{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my-repo:my-ext",
+		},
+		Spec: v1alpha1.ExtensionSpec{
+			RepoName: "my-repo",
+			RepoPath: "my-ext",
+			Args:     []string{"--namespaces=foo"},
+		},
+	}
+	f.Create(&ext)
+
+	f.MustGet(types.NamespacedName{Name: "my-repo:my-ext"}, &ext)
+
+	p := f.JoinPath("my-repo", "subpath", "my-ext", "Tiltfile")
+	require.Equal(t, p, ext.Status.Path)
+
+	var tf v1alpha1.Tiltfile
+	f.MustGet(types.NamespacedName{Name: "my-repo:my-ext"}, &tf)
+	require.Equal(t, tf.Spec, v1alpha1.TiltfileSpec{
+		Path:      p,
+		Labels:    map[string]string{"extension.my-repo_my-ext": "extension.my-repo_my-ext"},
+		RestartOn: &v1alpha1.RestartOnSpec{FileWatches: []string{"configs:my-repo:my-ext"}},
+		Args:      []string{"--namespaces=foo"},
+	})
+
+	assert.Equal(t, f.ma.Counts, []analytics.CountEvent{
+		{
 			Name: "api.extension.load",
 			Tags: map[string]string{
 				"ext_path":      "my-ext",
@@ -206,3 +250,23 @@ func (f *fixture) setupRepo() *v1alpha1.ExtensionRepo {
 	f.Create(&repo)
 	return &repo
 }
+
+func (f *fixture) setupRepoSubpath(subpath string) *v1alpha1.ExtensionRepo {
+	p := f.JoinPath("my-repo", subpath, "my-ext", "Tiltfile")
+	f.WriteFile(p, "print('hello-world')")
+
+	repo := v1alpha1.ExtensionRepo{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my-repo",
+		},
+		Spec: v1alpha1.ExtensionRepoSpec{
+			URL:        fmt.Sprintf("file://%s", f.JoinPath("my-repo")),
+			GitSubpath: subpath,
+		},
+		Status: v1alpha1.ExtensionRepoStatus{
+			Path: f.JoinPath("my-repo"),
+		},
+	}
+	f.Create(&repo)
+	return &repo
+}
diff --git a/internal/tiltfile/tiltextension/plugin.go b/internal/tiltfile/tiltextension/plugin.go
@@ -23,8 +23,10 @@ import (
 	"github.com/tilt-dev/tilt/pkg/logger"
 )
 
-const extensionPrefix = "ext://"
-const defaultRepoName = "default"
+const (
+	extensionPrefix = "ext://"
+	defaultRepoName = "default"
+)
 
 type Plugin struct {
 	repoReconciler ExtRepoReconciler
@@ -129,6 +131,75 @@ func (e *Plugin) LocalPath(t *starlark.Thread, arg string) (localPath string, er
 // Otherwise, infers an extension object that points to the default repo.
 func (e *Plugin) ensureExtension(t *starlark.Thread, objSet apiset.ObjectSet, moduleName string) *v1alpha1.Extension {
 	extName := apis.SanitizeName(moduleName)
+
+	extSet := objSet.GetOrCreateTypedSet(&v1alpha1.Extension{})
+	if existing, exists := extSet[extName]; exists {
+		ext := existing.(*v1alpha1.Extension)
+		metav1.SetMetaDataAnnotation(&ext.ObjectMeta, v1alpha1.AnnotationManagedBy, "tiltfile.loader")
+		return ext
+	}
+
+	repoSet := objSet.GetOrCreateTypedSet(&v1alpha1.ExtensionRepo{})
+
+	return e.registerExtension(t, extSet, repoSet, extName, moduleName)
+}
+
+// In cases where an extension is not already registered, this function will search for an extension
+// repo that can satisfy the requested extension, with a fallback to an extension in the default
+// repository.
+func (e *Plugin) registerExtension(t *starlark.Thread, extSet, repoSet apiset.TypedObjectSet, extName, moduleName string) *v1alpha1.Extension {
+	loadHost, extPath, tryRegister := strings.Cut(moduleName, "/")
+
+	// Safety fallback (in case this is called without already previously calling ensureExtension)
+	existing := extSet[extName]
+	if existing != nil {
+		return existing.(*v1alpha1.Extension)
+	}
+
+	// If the supplied module name does not contain a / then there's no point looking for matching
+	// extension repositories. We can just return an extension named extName in the default
+	// repository
+	if !tryRegister {
+		return e.registerDefaultExtension(t, extSet, extName, moduleName)
+	}
+
+	// Otherwise, look for a repository that can satisfy this lookup
+	for _, v := range repoSet {
+		repo := v.(*v1alpha1.ExtensionRepo)
+		if repo.Spec.LoadHost == "" || repo.Spec.LoadHost != loadHost {
+			continue
+		}
+
+		// This repo load_host matches the first component of the module name
+		// So we can register this as an extension on that repo
+		ext := &v1alpha1.Extension{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: extName,
+				Annotations: map[string]string{
+					v1alpha1.AnnotationManagedBy: "tiltfile.loader",
+				},
+			},
+			Spec: v1alpha1.ExtensionSpec{
+				RepoName: repo.Name,
+				RepoPath: extPath,
+			},
+		}
+
+		extSet[extName] = ext
+		return ext
+	}
+
+	return e.registerDefaultExtension(t, extSet, extName, moduleName)
+}
+
+// Registers an extension named moduleName in the default extension repository. Used as a fallback.
+func (e *Plugin) registerDefaultExtension(t *starlark.Thread, extSet apiset.TypedObjectSet, extName, moduleName string) *v1alpha1.Extension {
+	// Safety fallback
+	existing := extSet[extName]
+	if existing != nil {
+		return existing.(*v1alpha1.Extension)
+	}
+
 	defaultExt := &v1alpha1.Extension{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: extName,
@@ -142,15 +213,7 @@ func (e *Plugin) ensureExtension(t *starlark.Thread, objSet apiset.ObjectSet, mo
 		},
 	}
 
-	typedSet := objSet.GetOrCreateTypedSet(defaultExt)
-	existing, exists := typedSet[extName]
-	if exists {
-		ext := existing.(*v1alpha1.Extension)
-		metav1.SetMetaDataAnnotation(&ext.ObjectMeta, v1alpha1.AnnotationManagedBy, "tiltfile.loader")
-		return ext
-	}
-
-	typedSet[extName] = defaultExt
+	extSet[extName] = defaultExt
 	return defaultExt
 }
 
@@ -179,8 +242,10 @@ func (e *Plugin) ensureRepo(t *starlark.Thread, objSet apiset.ObjectSet, repoNam
 	return defaultRepo
 }
 
-var _ starkit.LoadInterceptor = (*Plugin)(nil)
-var _ starkit.StatefulPlugin = (*Plugin)(nil)
+var (
+	_ starkit.LoadInterceptor = (*Plugin)(nil)
+	_ starkit.StatefulPlugin  = (*Plugin)(nil)
+)
 
 func MustState(model starkit.Model) State {
 	state, err := GetState(model)