Revert "Fixup: Fix nil configured runtimeIDs"

This reverts commit d8d33b6.

go/runtime/bundle/discovery.go

@@ -22,8 +22,6 @@ import (
 	"github.com/oasisprotocol/oasis-core/go/common/logging"
 	cmSync "github.com/oasisprotocol/oasis-core/go/common/sync"
 	"github.com/oasisprotocol/oasis-core/go/config"
-	cmdFlags "github.com/oasisprotocol/oasis-core/go/oasis-node/cmd/common/flags"
-	"github.com/spf13/viper"
 )
 
 const (
@@ -65,7 +63,7 @@ type Discovery struct {
 }
 
 // NewDiscovery creates a new bundle discovery.
-func NewDiscovery(dataDir string, registry Registry) *Discovery {
+func NewDiscovery(dataDir string, registry Registry, runtimeIDsCfg []common.Namespace) *Discovery {
 	logger := logging.GetLogger("runtime/bundle/discovery")
 
 	client := http.Client{
@@ -85,7 +83,7 @@ func NewDiscovery(dataDir string, registry Registry) *Discovery {
 		client:             &client,
 		maxBundleSizeBytes: bundleSize,
 		registry:           registry,
-		runtimeIDsCfg:      make([]common.Namespace, 0),
+		runtimeIDsCfg:      runtimeIDsCfg,
 		logger:             *logger,
 	}
 }
@@ -125,18 +123,10 @@ func (d *Discovery) Init() error {
 		runtimeBaseURLs[runtime.ID] = urls
 	}
 
-	// Store configured runtimeIDs to the discovery. To support
-	// legacy configuration (bundle paths), this is only valid
-	// after d.copyBundles() and d.Discover() is called.
-	runtimeIDsCfg, err := getConfiguredRuntimeIDs(d.registry)
-	if err != nil {
-		return fmt.Errorf("getConfiguredRuntimeIDs: %w", err)
-	}
-
 	// Update discovery.
 	d.mu.Lock()
 	defer d.mu.Unlock()
-	d.runtimeIDsCfg = runtimeIDsCfg
+
 	d.globalBaseURLs = globalBaseURLs
 	d.runtimeBaseURLs = runtimeBaseURLs
 
@@ -180,14 +170,6 @@ func (d *Discovery) run(ctx context.Context) {
 	}
 }
 
-// GetConfiguredRuntimeIDs returns the runtime IDs that discovery is configured
-// for.
-//
-// Warning: this is only valid after d.Init() is called.
-func (d *Discovery) GetConfiguredRuntimeIDs() []common.Namespace {
-	return d.runtimeIDsCfg
-}
-
 // Discover searches for new bundles in the bundle directory and adds them
 // to the bundle registry.
 func (d *Discovery) Discover() error {
@@ -542,53 +524,6 @@ func (d *Discovery) copyBundle(src string) error {
 	return nil
 }
 
-func getConfiguredRuntimeIDs(registry Registry) ([]common.Namespace, error) {
-	// Check if any runtimes are configured to be hosted.
-	runtimes := make(map[common.Namespace]struct{})
-	for _, cfg := range config.GlobalConfig.Runtime.Runtimes {
-		runtimes[cfg.ID] = struct{}{}
-	}
-
-	// Support legacy configurations where runtimes are specified within
-	// configured bundles.
-	for _, manifest := range registry.GetManifests() {
-		runtimes[manifest.ID] = struct{}{}
-	}
-
-	if cmdFlags.DebugDontBlameOasis() && viper.IsSet(CfgDebugMockIDs) {
-		// Allow the mock provisioner to function, as it does not use an actual
-		// runtime. This is only used for the basic node tests.
-		for _, str := range viper.GetStringSlice(CfgDebugMockIDs) {
-			var runtimeID common.Namespace
-			if err := runtimeID.UnmarshalText([]byte(str)); err != nil {
-				return nil, fmt.Errorf("failed to deserialize runtime ID: %w", err)
-			}
-			runtimes[runtimeID] = struct{}{}
-		}
-
-		// Skip validation
-		return slices.Collect(maps.Keys(runtimes)), nil
-	}
-
-	// Validate configured runtimes based on the runtime mode.
-	switch config.GlobalConfig.Mode {
-	case config.ModeValidator, config.ModeSeed:
-		// No runtimes should be configured.
-		if len(runtimes) > 0 && !cmdFlags.DebugDontBlameOasis() {
-			return nil, fmt.Errorf("no runtimes should be configured when in validator or seed modes")
-		}
-	case config.ModeCompute, config.ModeKeyManager, config.ModeStatelessClient:
-		// At least one runtime should be configured.
-		if len(runtimes) == 0 && !cmdFlags.DebugDontBlameOasis() {
-			return nil, fmt.Errorf("at least one runtime must be configured when in compute, keymanager, or client-stateless modes")
-		}
-	default:
-		// In any other mode, runtimes can be optionally configured.
-	}
-
-	return slices.Collect(maps.Keys(runtimes)), nil
-}
-
 // cleanStaleExplodedBundles removes regular and detached bundles exploded bundle subdir,
 // for the runtimes no longer present in the configuration.
 func (d *Discovery) cleanStaleExplodedBundles(ctx context.Context) {
@@ -642,8 +577,6 @@ func (d *Discovery) cleanStaleExplodedBundles(ctx context.Context) {
 				}
 				if stale {
 					explodedDir := filepath.Join(bundlesDir, manifestHash)
-					d.logger.Info("Remvoving exploded bundle dir",
-						"explodedDir", explodedDir)
 					if err = os.RemoveAll(explodedDir); err != nil {
 						d.logger.Error("error removing exploded bundle",
 							"manifestHash", manifestHash,

go/runtime/bundle/discovery_test.go

@@ -20,7 +20,6 @@ var _ Registry = (*mockRegistry)(nil)
 
 type mockRegistry struct {
 	manifestHashes map[hash.Hash]struct{}
-	manifests      []*Manifest
 }
 
 // HasBundle implements Registry.
@@ -47,7 +46,7 @@ func (r *mockRegistry) WatchVersions(common.Namespace) (<-chan version.Version,
 
 // GetManifests implements Registry.
 func (r *mockRegistry) GetManifests() []*Manifest {
-	return r.manifests
+	panic("unimplemented")
 }
 
 // GetName implements Registry.
@@ -68,21 +67,25 @@ func (r *mockRegistry) CleanStaleBundles(context.Context, common.Namespace, vers
 func newMockListener() *mockRegistry {
 	return &mockRegistry{
 		manifestHashes: make(map[hash.Hash]struct{}),
-		manifests:      make([]*Manifest, 0),
 	}
 }
 
 func TestBundleDiscovery(t *testing.T) {
 	// Prepare a temporary directory for storing bundles.
 	dataDir := t.TempDir()
 
+	// Create dummy runtimeID1
+	var runtimeID1 common.Namespace
+	err := runtimeID1.UnmarshalHex("8000000000000000000000000000000000000000000000000000000000000001")
+	require.NoError(t, err)
+
 	// Create discovery.
 	registry := newMockListener()
-	discovery := NewDiscovery(dataDir, registry)
+	discovery := NewDiscovery(dataDir, registry, []common.Namespace{runtimeID1})
 
 	// Get bundle directory.
 	dir := ExplodedPath(dataDir)
-	err := common.Mkdir(dir)
+	err = common.Mkdir(dir)
 	require.NoError(t, err)
 
 	// Create an empty file, which should be ignored by the discovery.
@@ -135,6 +138,7 @@ func TestCleanStaleExplodedBundles(t *testing.T) {
 
 	// Create discovery, that has only runtimeID2 registered.
 	registry := newMockListener()
+	discovery := NewDiscovery(dir, registry, []common.Namespace{runtimeID2})
 
 	version1 := version.Version{Major: 1}
 	version2 := version.Version{Major: 2}
@@ -154,25 +158,14 @@ func TestCleanStaleExplodedBundles(t *testing.T) {
 
 	paths := []string{path0, path1, path2, path3}
 
-	for i, path := range paths {
+	for _, path := range paths {
 		// Explode the bundle.
 		bnd, err := Open(path)
 		require.NoError(t, err)
-		// Add runtimeID1 to the registry
-		if i == 0 {
-			err := registry.AddBundle("mock", bnd.manifestHash)
-			require.NoError(t, err)
-			registry.manifests = append(registry.manifests, bnd.Manifest)
-
-		}
 		_, err = bnd.WriteExploded(dir)
 		require.NoError(t, err)
 	}
 
-	discovery := NewDiscovery(dir, registry)
-	err = discovery.Init()
-	require.NoError(t, err)
-
 	// Ensure bundle were exploded succesfully.
 	entries, err := os.ReadDir(ExplodedPath(dir))
 	require.NoError(t, err)
@@ -185,7 +178,6 @@ func TestCleanStaleExplodedBundles(t *testing.T) {
 
 	// Clean stale bundles.
 	discovery.cleanStaleExplodedBundles(context.Background())
-	fmt.Println(discovery.GetConfiguredRuntimeIDs())
 
 	// All exploded bundles for runtimeID1 should be removed.
 	entries, err = os.ReadDir(ExplodedPath(dir))

go/runtime/registry/config.go

@@ -3,10 +3,14 @@ package registry
 import (
 	"context"
 	"fmt"
+	"maps"
 	"os"
+	"slices"
 	"strings"
 	"time"
 
+	"github.com/spf13/viper"
+
 	"github.com/oasisprotocol/oasis-core/go/common"
 	"github.com/oasisprotocol/oasis-core/go/common/identity"
 	"github.com/oasisprotocol/oasis-core/go/common/persistent"
@@ -15,6 +19,7 @@ import (
 	consensus "github.com/oasisprotocol/oasis-core/go/consensus/api"
 	ias "github.com/oasisprotocol/oasis-core/go/ias/api"
 	cmdFlags "github.com/oasisprotocol/oasis-core/go/oasis-node/cmd/common/flags"
+	"github.com/oasisprotocol/oasis-core/go/runtime/bundle"
 	"github.com/oasisprotocol/oasis-core/go/runtime/bundle/component"
 	rtConfig "github.com/oasisprotocol/oasis-core/go/runtime/config"
 	"github.com/oasisprotocol/oasis-core/go/runtime/history"
@@ -32,6 +37,53 @@ func getLocalConfig(runtimeID common.Namespace) map[string]interface{} {
 	return config.GlobalConfig.Runtime.GetLocalConfig(runtimeID)
 }
 
+func getConfiguredRuntimeIDs(registry bundle.Registry) ([]common.Namespace, error) {
+	// Check if any runtimes are configured to be hosted.
+	runtimes := make(map[common.Namespace]struct{})
+	for _, cfg := range config.GlobalConfig.Runtime.Runtimes {
+		runtimes[cfg.ID] = struct{}{}
+	}
+
+	// Support legacy configurations where runtimes are specified within
+	// configured bundles.
+	for _, manifest := range registry.GetManifests() {
+		runtimes[manifest.ID] = struct{}{}
+	}
+
+	if cmdFlags.DebugDontBlameOasis() && viper.IsSet(bundle.CfgDebugMockIDs) {
+		// Allow the mock provisioner to function, as it does not use an actual
+		// runtime. This is only used for the basic node tests.
+		for _, str := range viper.GetStringSlice(bundle.CfgDebugMockIDs) {
+			var runtimeID common.Namespace
+			if err := runtimeID.UnmarshalText([]byte(str)); err != nil {
+				return nil, fmt.Errorf("failed to deserialize runtime ID: %w", err)
+			}
+			runtimes[runtimeID] = struct{}{}
+		}
+
+		// Skip validation
+		return slices.Collect(maps.Keys(runtimes)), nil
+	}
+
+	// Validate configured runtimes based on the runtime mode.
+	switch config.GlobalConfig.Mode {
+	case config.ModeValidator, config.ModeSeed:
+		// No runtimes should be configured.
+		if len(runtimes) > 0 && !cmdFlags.DebugDontBlameOasis() {
+			return nil, fmt.Errorf("no runtimes should be configured when in validator or seed modes")
+		}
+	case config.ModeCompute, config.ModeKeyManager, config.ModeStatelessClient:
+		// At least one runtime should be configured.
+		if len(runtimes) == 0 && !cmdFlags.DebugDontBlameOasis() {
+			return nil, fmt.Errorf("at least one runtime must be configured when in compute, keymanager, or client-stateless modes")
+		}
+	default:
+		// In any other mode, runtimes can be optionally configured.
+	}
+
+	return slices.Collect(maps.Keys(runtimes)), nil
+}
+
 func createHostInfo(consensus consensus.Backend) (*hostProtocol.HostInfo, error) {
 	cs, err := consensus.GetStatus(context.Background())
 	if err != nil {

go/runtime/registry/registry.go

@@ -685,10 +685,10 @@ func (r *runtimeRegistry) Cleanup() {
 
 // Init initializes the runtime registry by adding runtimes from the global
 // runtime configuration to the registry.
-func (r *runtimeRegistry) Init(ctx context.Context) error {
+func (r *runtimeRegistry) Init(ctx context.Context, runtimeIDs []common.Namespace) error {
 	managed := config.GlobalConfig.Mode != config.ModeKeyManager
 
-	for _, runtimeID := range r.bundleDiscovery.GetConfiguredRuntimeIDs() {
+	for _, runtimeID := range runtimeIDs {
 		if _, err := r.NewRuntime(ctx, runtimeID, managed); err != nil {
 			r.logger.Error("failed to add runtime",
 				"err", err,
@@ -713,7 +713,13 @@ func New(
 	// Create bundle registry and discovery.
 	bundleRegistry := bundle.NewRegistry(dataDir)
 
-	bundleDiscovery := bundle.NewDiscovery(dataDir, bundleRegistry)
+	// Get configured Runtime IDs.
+	runtimeIDs, err := getConfiguredRuntimeIDs(bundleRegistry)
+	if err != nil {
+		return nil, err
+	}
+
+	bundleDiscovery := bundle.NewDiscovery(dataDir, bundleRegistry, runtimeIDs)
 
 	// Create history keeper factory.
 	historyFactory, err := createHistoryFactory()
@@ -758,7 +764,7 @@ func New(
 	}
 
 	// Initialize the runtime registry.
-	if err = r.Init(ctx); err != nil {
+	if err = r.Init(ctx, runtimeIDs); err != nil {
 		return nil, err
 	}