make bundle a general solution rather than only composer (#484)

code support for bundle extension. Based on previous change, this PR is
pretty simple. Now you could run extension in this way:

```
boe run --extension composer/example-go --extension dns/listener
```

Because the `/` is not allowed in the extension name
(`"^[a-z][.a-z0-9-_]*$"`), it's pretty great to be used as the separator
between bundle and extension name. And easy to understand and use.

If there is a `/`, the first part is bundle (artifact), the second part
the specific extension name. If there is no `/`, we will fallback to use
the extension name as bundle name.

---------

Signed-off-by: wbpcode <wbphub@gmail.com>

Author: wbpcode

cli/cmd/download.go

@@ -65,7 +65,7 @@ func (d *Download) AfterApply(dirs *xdg.Directories, logger *slog.Logger) error
 func (d *Download) Run(ctx context.Context, logger *slog.Logger) error {
 	logger.Debug("handling download command", "cmd", internal.RedactSensitive(d))
 
-	name, tag := splitRef(d.Extension)
+	bundle, name, tag := splitRef(d.Extension)
 	_, _ = fmt.Fprintf(os.Stderr, "→ %sDownloading %s for %s/%s...%s\n",
 		internal.ANSIBold, d.Extension, d.downloader.OS, d.downloader.Arch, internal.ANSIReset)
 
@@ -77,7 +77,7 @@ func (d *Download) Run(ctx context.Context, logger *slog.Logger) error {
 	case extensions.ComposerArtifact, extensions.ComposerArtifactLite, extensions.ComposerArtifactSource:
 		downloaded, err = d.downloader.DownloadComposer(ctx, tag, name)
 	default:
-		downloaded, err = d.downloader.DownloadExtension(ctx, name, tag)
+		downloaded, err = d.downloader.DownloadExtension(ctx, bundle, name, tag)
 	}
 	if err != nil {
 		return fmt.Errorf("failed to download extension: %w", err)

cli/cmd/flags.go

@@ -42,7 +42,7 @@ func (e extensionPositions) sort(manifests []*extensions.Manifest) ([]*extension
 
 	for r, positions := range e.remote {
 		pos := slices.IndexFunc(manifests, func(m *extensions.Manifest) bool {
-			return m.Remote && (m.Name+":"+m.Version == r || m.Name == r)
+			return m.Remote && (m.RemoteRef == r)
 		})
 		if pos == -1 {
 			return nil, fmt.Errorf("failed to find manifest for remote extension with reference %s", r)

cli/cmd/flags_test.go

@@ -31,20 +31,20 @@ func TestExtensionPositionsSort(t *testing.T) {
 			name: "single remote extension",
 			args: []string{"boe", "gen-config", "--extension", "ext1"},
 			manifests: []*extensions.Manifest{
-				{Name: "ext1", Remote: true},
+				{Name: "ext1", Remote: true, RemoteRef: "ext1"},
 			},
 			want: []*extensions.Manifest{
-				{Name: "ext1", Remote: true},
+				{Name: "ext1", Remote: true, RemoteRef: "ext1"},
 			},
 		},
 		{
 			name: "single remote extension with version",
 			args: []string{"boe", "gen-config", "--extension", "ext1:0.1.0"},
 			manifests: []*extensions.Manifest{
-				{Name: "ext1", Version: "0.1.0", Remote: true},
+				{Name: "ext1", Version: "0.1.0", Remote: true, RemoteRef: "ext1:0.1.0"},
 			},
 			want: []*extensions.Manifest{
-				{Name: "ext1", Version: "0.1.0", Remote: true},
+				{Name: "ext1", Version: "0.1.0", Remote: true, RemoteRef: "ext1:0.1.0"},
 			},
 		},
 		{
@@ -61,23 +61,23 @@ func TestExtensionPositionsSort(t *testing.T) {
 			name: "local before remote",
 			args: []string{"boe", "gen-config", "--local", localDir1, "--extension", "ext1"},
 			manifests: []*extensions.Manifest{
-				{Name: "ext1", Remote: true},
+				{Name: "ext1", Remote: true, RemoteRef: "ext1"},
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
 			},
 			want: []*extensions.Manifest{
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
-				{Name: "ext1", Remote: true},
+				{Name: "ext1", Remote: true, RemoteRef: "ext1"},
 			},
 		},
 		{
 			name: "remote before local",
 			args: []string{"boe", "gen-config", "--extension", "ext1", "--local", localDir1},
 			manifests: []*extensions.Manifest{
-				{Name: "ext1", Remote: true},
+				{Name: "ext1", Remote: true, RemoteRef: "ext1"},
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
 			},
 			want: []*extensions.Manifest{
-				{Name: "ext1", Remote: true},
+				{Name: "ext1", Remote: true, RemoteRef: "ext1"},
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
 			},
 		},
@@ -93,19 +93,19 @@ func TestExtensionPositionsSort(t *testing.T) {
 				"--local", localDir1,
 			},
 			manifests: []*extensions.Manifest{
-				{Name: "ext1", Version: "1.0.0", Remote: true},
-				{Name: "ext2", Remote: true},
-				{Name: "ext1", Version: "1.0.0", Remote: true},
+				{Name: "ext1", Version: "1.0.0", Remote: true, RemoteRef: "ext1"},
+				{Name: "ext2", Remote: true, RemoteRef: "ext2"},
+				{Name: "ext1", Version: "1.0.0", Remote: true, RemoteRef: "ext1:1.0.0"},
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
 				{Name: "local2", Path: localDir2 + "/manifest.yaml"},
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
 			},
 			want: []*extensions.Manifest{
-				{Name: "ext1", Version: "1.0.0", Remote: true},
+				{Name: "ext1", Version: "1.0.0", Remote: true, RemoteRef: "ext1"},
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
-				{Name: "ext2", Remote: true},
+				{Name: "ext2", Remote: true, RemoteRef: "ext2"},
 				{Name: "local2", Path: localDir2 + "/manifest.yaml"},
-				{Name: "ext1", Version: "1.0.0", Remote: true},
+				{Name: "ext1", Version: "1.0.0", Remote: true, RemoteRef: "ext1:1.0.0"},
 				{Name: "local1", Path: localDir1 + "/manifest.yaml"},
 			},
 		},

cli/cmd/run.go

@@ -240,7 +240,7 @@ func adminAddressForLocal(port uint32) string {
 func downloadExtensions(ctx context.Context, downloader *extensions.Downloader, refs []string, build bool) ([]*extensions.Manifest, error) {
 	downloaded := make([]*extensions.Manifest, 0, len(refs))
 	for _, ext := range refs {
-		name, tag := splitRef(ext)
+		bundle, name, tag := splitRef(ext)
 
 		// The reserved name "goplugin-loader" is not a downloadable extension: it drives
 		// the composer's goplugin-loader filter directly from the user-supplied --config.
@@ -251,15 +251,19 @@ func downloadExtensions(ctx context.Context, downloader *extensions.Downloader,
 			if err != nil {
 				return nil, err
 			}
+			manifest.Remote = true
+			manifest.RemoteRef = ext
 			downloaded = append(downloaded, manifest)
 			continue
 		}
 
 		_, _ = fmt.Fprintf(os.Stderr, "→ %sFetching %s...%s\n", internal.ANSIBold, name, internal.ANSIReset)
-		artifact, err := downloader.DownloadExtension(ctx, name, tag)
+		artifact, err := downloader.DownloadExtension(ctx, bundle, name, tag)
 		if err != nil {
 			return nil, err
 		}
+		artifact.ExtensionManifest.Remote = true
+		artifact.ExtensionManifest.RemoteRef = ext
 
 		switch artifact.ArtifactType {
 		case extensions.ArtifactBinary:
@@ -319,9 +323,6 @@ func resolveGoPluginLoader(ctx context.Context, downloader *extensions.Downloade
 		Parent:          extensions.ComposerBundle,
 		Version:         version,
 		ComposerVersion: version,
-		// Marked remote so extensionPositions.sort can match it by reference, like
-		// other downloaded extensions (it is fetched from the registry as composer-lite).
-		Remote: true,
 	}
 	manifest.ApplyDefaults()
 	return manifest, nil
@@ -436,20 +437,34 @@ func resolveParent(ctx context.Context, downloader *extensions.Downloader, m *ex
 		dl, err = downloader.DownloadComposer(ctx, version, extensions.ComposerArtifactLite)
 	} else {
 		version := cmp.Or(m.Version, "latest")
-		dl, err = downloader.DownloadExtension(ctx, m.Parent, version)
+		dl, err = downloader.DownloadExtension(ctx, m.Parent, m.Parent, version)
 	}
 	if err != nil {
 		return nil, fmt.Errorf("downloading parent %s: %w", m.Parent, err)
 	}
 	return dl.Manifest, nil
 }
 
-// extractTag extracts the tag from a full OCI reference.
-func splitRef(ref string) (repo string, tag string) {
+// splitRef splits an extension reference into bundle, extension name, and tag.
+// The format is [bundle/]extension[:tag]. If no bundle prefix is given, the extension
+// name is used as the bundle name (standalone extension). If no tag is given, "latest" is used.
+//
+// Examples:
+//
+//	"composer/example-go:v1.0.0" → ("composer", "example-go", "v1.0.0")
+//	"cors:1.0.0"                 → ("cors", "cors", "1.0.0")
+//	"cors"                       → ("cors", "cors", "latest")
+func splitRef(ref string) (bundle string, extension string, tag string) {
+	tag = "latest"
+	name := ref
 	if colonIdx := strings.LastIndex(ref, ":"); colonIdx != -1 {
-		return ref[:colonIdx], ref[colonIdx+1:]
+		name = ref[:colonIdx]
+		tag = ref[colonIdx+1:]
 	}
-	return ref, "latest"
+	if slashIdx := strings.Index(name, "/"); slashIdx != -1 {
+		return name[:slashIdx], name[slashIdx+1:], tag
+	}
+	return name, name, tag
 }
 
 // errInvalidManifest is returned when some extension is not compatible with the requested Envoy version.

cli/cmd/run_test.go

@@ -463,41 +463,54 @@ func TestRunInvalidEnvoyPath(t *testing.T) {
 
 func TestSplitRef(t *testing.T) {
 	tests := []struct {
-		name     string
-		input    string
-		wantRepo string
-		wantTag  string
+		name          string
+		input         string
+		wantBundle    string
+		wantExtension string
+		wantTag       string
 	}{
 		{
-			name:     "simple name without tag",
-			input:    "cors",
-			wantRepo: "cors",
-			wantTag:  "latest",
+			name:          "simple name without tag",
+			input:         "cors",
+			wantBundle:    "cors",
+			wantExtension: "cors",
+			wantTag:       "latest",
+		},
+		{
+			name:          "simple name with tag",
+			input:         "cors:1.0.0",
+			wantBundle:    "cors",
+			wantExtension: "cors",
+			wantTag:       "1.0.0",
 		},
 		{
-			name:     "simple name with tag",
-			input:    "cors:1.0.0",
-			wantRepo: "cors",
-			wantTag:  "1.0.0",
+			name:          "empty string",
+			input:         "",
+			wantBundle:    "",
+			wantExtension: "",
+			wantTag:       "latest",
 		},
 		{
-			name:     "empty string",
-			input:    "",
-			wantRepo: "",
-			wantTag:  "latest",
+			name:          "bundle prefixed name with tag",
+			input:         "composer/example-go:v1.0.0",
+			wantBundle:    "composer",
+			wantExtension: "example-go",
+			wantTag:       "v1.0.0",
 		},
 		{
-			name:     "name with multiple colons takes last",
-			input:    "foo:bar:baz",
-			wantRepo: "foo:bar",
-			wantTag:  "baz",
+			name:          "bundle prefixed name without tag",
+			input:         "composer/example-go",
+			wantBundle:    "composer",
+			wantExtension: "example-go",
+			wantTag:       "latest",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			repo, tag := splitRef(tt.input)
-			require.Equal(t, tt.wantRepo, repo)
+			bundle, extension, tag := splitRef(tt.input)
+			require.Equal(t, tt.wantBundle, bundle)
+			require.Equal(t, tt.wantExtension, extension)
 			require.Equal(t, tt.wantTag, tag)
 		})
 	}

cli/e2e/run_test.go

@@ -13,6 +13,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"os/exec"
+	"regexp"
 	"runtime"
 	"strings"
 	"testing"
@@ -148,6 +149,17 @@ func TestRustLocalExtension(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, 0, status.ExitCode())
 
+	// Overwrite the dependency in Cargo.toml to the stable version of Envoy 1.38 to
+	// avoid potential compatibility issues make the CI flaky.
+	// TODO(wbpcode): remove this if we get the dependency more stable.
+	cargoTomlPath := dataDir + "/rust-e2e/Cargo.toml"
+	// #nosec G304
+	cargoToml, err := os.ReadFile(cargoTomlPath)
+	require.NoError(t, err)
+	cargoToml = regexp.MustCompile(`rev = "[0-9a-f]+"`).
+		ReplaceAll(cargoToml, []byte(`rev = "f1dd21b16c244bda00edfb5ffce577e12d0d2ec2"`))
+	require.NoError(t, os.WriteFile(cargoTomlPath, cargoToml, 0o600))
+
 	// Run the newly created extension
 	ports := internaltesting.FreePorts(t, 2)
 	proxyPort := ports[0]
@@ -397,6 +409,52 @@ func TestGoPluginLoaderRemoteExtension(t *testing.T) {
 		})
 }
 
+// TestComposerBundleExtension exercises the bundle-prefixed extension syntax end to end:
+//  1. build the full composer image (non-lite, single platform) and push it to the local test registry;
+//  2. run `boe run --extension composer/example-go` which downloads the full composer binary
+//     artifact and resolves the example-go child extension manifest from within the bundle.
+func TestComposerBundleExtension(t *testing.T) {
+	t.Setenv("TEST_BOE_RUN_ENVOY_TIMEOUT", "5m")
+
+	const composerDir = "../../extensions/composer"
+
+	manifests, err := extensions.LoadManifests(internaltesting.ExtensionsFS(t), ".", false)
+	require.NoError(t, err)
+	composer, ok := manifests[extensions.ComposerArtifact]
+	require.True(t, ok)
+	version := composer.Version
+
+	t.Setenv("BOE_REGISTRY", registryAddr)
+	t.Setenv("BOE_REGISTRY_INSECURE", "true")
+	t.Setenv("BOE_DATA_HOME", t.TempDir())
+
+	// Build and push the full composer image (single platform) to the local registry.
+	runCmd(t, composerDir, "make", "push_image",
+		"PLATFORMS=linux/"+runtime.GOARCH,
+		"OCI_REGISTRY="+registryAddr,
+	)
+
+	// Run boe with the bundle-prefixed extension reference: composer/example-go.
+	// This downloads the full composer artifact keyed by the "composer" bundle name,
+	// then resolves the "example-go" child extension manifest within it.
+	ports := internaltesting.FreePorts(t, 2)
+	proxyPort := ports[0]
+	args := []string{
+		"--log-level", "dynamic_modules:debug",
+		"--extension", "composer/example-go:" + version,
+	}
+	if strings.HasSuffix(version, "-dev") {
+		args = append(args, "--dev")
+	}
+	internaltesting.RunEnvoy(t, cliBin, proxyPort, ports[1], args...)
+
+	internaltesting.RequireEventuallyGet(t,
+		fmt.Sprintf("http://localhost:%d/status/200", proxyPort),
+		func(r *http.Response) bool {
+			return r.Header.Get("x-example-response-header") == "example-value"
+		})
+}
+
 // runCmd runs name with args (in dir, if non-empty), logging the combined output and failing the
 // test on a non-zero exit.
 func runCmd(t *testing.T, dir, name string, args ...string) {

cli/internal/extensions/downloader.go

@@ -72,9 +72,14 @@ func (d *Downloader) DownloadComposer(ctx context.Context, version string, artif
 }
 
 // DownloadExtension downloads the extension from the specified repository and tag into the downloadDir.
-func (d *Downloader) DownloadExtension(ctx context.Context, name, version string) (DownloadedExtension, error) {
-	d.Logger.Info("downloading extension", "repository", d.Registry, "name", name, "version", version)
-	repository := RepositoryName(d.Registry, name)
+// The bundle parameter is used to construct the OCI repository name for downloading the artifact.
+// The name parameter is used to resolve the specific extension manifest within the downloaded artifact.
+// For standalone extensions, bundle and name are the same. For bundled extensions (e.g. "composer/example-go"),
+// the bundle is the parent artifact and the name is the child whose manifest must have a Parent field
+// matching the bundle name.
+func (d *Downloader) DownloadExtension(ctx context.Context, bundle, name, version string) (DownloadedExtension, error) {
+	d.Logger.Info("downloading extension", "repository", d.Registry, "bundle", bundle, "extension", name, "version", version)
+	repository := RepositoryName(d.Registry, bundle)
 
 	artifact, err := d.download(ctx, repository, version, func(manifest *ocispec.Manifest) string {
 		extensionManifest := ManifestFromOCI(manifest)
@@ -96,6 +101,21 @@ func (d *Downloader) DownloadExtension(ctx context.Context, name, version string
 	if err != nil {
 		return DownloadedExtension{}, fmt.Errorf("failed to resolve extension manifest for %q: %w", name, err)
 	}
+
+	// For bundled extensions (bundle != name), validate that the resolved extension
+	// declares the bundle as its parent.
+	if bundle != name {
+		if extensionManifest.Parent == "" {
+			return DownloadedExtension{},
+				fmt.Errorf("extension %q in bundle %q has no parent set in its manifest", name, bundle)
+		}
+		if extensionManifest.Parent != bundle {
+			return DownloadedExtension{},
+				fmt.Errorf("extension %q declares parent %q but was requested from bundle %q",
+					name, extensionManifest.Parent, bundle)
+		}
+	}
+
 	extensionManifest.Remote = true
 	artifact.ExtensionManifest = extensionManifest
 	return artifact, nil