RSDK-7848 remove side-by-side meta.json support (#5008)

Author: abe-winter

config/module.go

@@ -27,6 +27,8 @@ const (
 	defaultFirstRunTimeout = 1 * time.Hour
 )
 
+var errLocalTarballEntrypoint = errors.New("local tarballs must contain a meta.json with the 'entrypoint' field")
+
 // Module represents an external resource module, with a path to the binary module file.
 type Module struct {
 	// Name is an arbitrary name used to identify the module, and is used to name it's socket as well.
@@ -197,8 +199,7 @@ func parseJSONFile[T any](path string) (*T, error) {
 
 // EvaluateExePath returns absolute ExePath from one of three sources (in order of precedence):
 // 1. if there is a meta.json in the exe dir, use that, except in local non-tarball case.
-// 2. if this is a local tarball and there's a meta.json next to the tarball, use that.
-// 3. otherwise use the exe path from config, or fail if this is a local tarball.
+// 2. otherwise use the exe path from config, or fail if this is a local tarball.
 // Note: the working directory must be the unpacked tarball directory or local exec directory.
 func (m Module) EvaluateExePath(packagesDir string) (string, error) {
 	path, err := utils.ExpandHomeDir(m.ExePath)
@@ -233,24 +234,10 @@ func (m Module) EvaluateExePath(packagesDir string) (string, error) {
 			}
 			return filepath.Abs(entrypoint)
 		}
-	}
-	if m.NeedsSyntheticPackage() {
-		// this is case 2, side-by-side
-		// TODO(RSDK-7848): remove this case once java sdk supports internal meta.json.
-		metaPath, err := utils.SafeJoinDir(filepath.Dir(path), "meta.json")
-		if err != nil {
-			return "", err
+		if m.NeedsSyntheticPackage() {
+			// registry modules can use configured ExePath, but for local tarballs it is wrong, throw an error.
+			return "", errLocalTarballEntrypoint
 		}
-		meta, err := parseJSONFile[JSONManifest](metaPath)
-		if err != nil {
-			// note: this error deprecates the side-by-side case because the side-by-side case is deprecated.
-			return "", fmt.Errorf("couldn't find meta.json inside tarball %s (or next to it): %w", path, err)
-		}
-		entrypoint, err := utils.SafeJoinDir(exeDir, meta.Entrypoint)
-		if err != nil {
-			return "", err
-		}
-		return filepath.Abs(entrypoint)
 	}
 	return path, nil
 }
@@ -464,27 +451,6 @@ func (m Module) getJSONManifest(unpackedModDir string, env map[string]string) (*
 		}
 	}
 
-	var exeDir string
-	var localTarballErr error
-
-	// TODO(RSDK-7848): remove this case once java sdk supports internal meta.json.
-	// case 3: local AND tarball
-	if m.NeedsSyntheticPackage() {
-		exeDir = filepath.Dir(m.ExePath)
-
-		var meta *JSONManifest
-		meta, localTarballErr = findMetaJSONFile(exeDir)
-		if localTarballErr != nil {
-			if !os.IsNotExist(localTarballErr) {
-				return nil, "", fmt.Errorf("local tarball: %w", localTarballErr)
-			}
-		}
-
-		if meta != nil {
-			return meta, exeDir, nil
-		}
-	}
-
 	if online {
 		if !ok {
 			return nil, "", fmt.Errorf("registry module: failed to find meta.json. VIAM_MODULE_ROOT not set: %w", registryTarballErr)
@@ -494,7 +460,7 @@ func (m Module) getJSONManifest(unpackedModDir string, env map[string]string) (*
 	}
 
 	if !localNonTarball {
-		return nil, "", fmt.Errorf("local tarball: failed to find meta.json: %w", errors.Join(registryTarballErr, localTarballErr))
+		return nil, "", fmt.Errorf("local tarball: failed to find meta.json: %w", registryTarballErr)
 	}
 
 	return nil, "", errors.New("local non-tarball: did not search for meta.json")

config/module_test.go

@@ -27,22 +27,31 @@ func testChdir(t *testing.T, dir string) {
 func TestInternalMeta(t *testing.T) {
 	tmp := t.TempDir()
 	testChdir(t, tmp)
-	testWriteJSON(t, "meta.json", JSONManifest{Entrypoint: "entry"})
 	packagesDir := filepath.Join(tmp, "packages")
+
 	t.Run("local-tarball", func(t *testing.T) {
 		mod := Module{
 			Type:    ModuleTypeLocal,
 			ExePath: filepath.Join(tmp, "whatever.tar.gz"),
 		}
-		exePath, err := mod.EvaluateExePath(packagesDir)
-		test.That(t, err, test.ShouldBeNil)
-		exeDir, err := mod.exeDir(packagesDir)
-		test.That(t, err, test.ShouldBeNil)
-		// "entry" is from meta.json.
-		test.That(t, exePath, test.ShouldEqual, filepath.Join(exeDir, "entry"))
+		t.Run("meta.json missing", func(t *testing.T) {
+			_, err := mod.EvaluateExePath(packagesDir)
+			test.That(t, err, test.ShouldEqual, errLocalTarballEntrypoint)
+		})
+
+		t.Run("meta.json present", func(t *testing.T) {
+			exeDir, err := mod.exeDir(packagesDir)
+			test.That(t, err, test.ShouldBeNil)
+			manifest := JSONManifest{Entrypoint: "entry"}
+			testWriteJSON(t, filepath.Join(exeDir, "meta.json"), manifest)
+			exePath, err := mod.EvaluateExePath(packagesDir)
+			test.That(t, err, test.ShouldBeNil)
+			test.That(t, exePath, test.ShouldResemble, filepath.Join(exeDir, manifest.Entrypoint))
+		})
 	})
 
 	t.Run("non-tarball", func(t *testing.T) {
+		testWriteJSON(t, "meta.json", JSONManifest{Entrypoint: "entry"})
 		mod := Module{
 			Type:    ModuleTypeLocal,
 			ExePath: filepath.Join(tmp, "whatever"),
@@ -94,11 +103,8 @@ func TestSyntheticModule(t *testing.T) {
 		testWriteJSON(t, filepath.Join(tmp, "meta.json"), &meta)
 
 		// local tarball case
-		syntheticPath, err := modNeedsSynthetic.EvaluateExePath(tmp)
-		test.That(t, err, test.ShouldBeNil)
-		exeDir, err := modNeedsSynthetic.exeDir(tmp)
-		test.That(t, err, test.ShouldBeNil)
-		test.That(t, syntheticPath, test.ShouldEqual, filepath.Join(exeDir, meta.Entrypoint))
+		_, err := modNeedsSynthetic.EvaluateExePath(tmp)
+		test.That(t, err, test.ShouldEqual, errLocalTarballEntrypoint)
 
 		// vanilla case
 		notTarPath, err := modNotTar.EvaluateExePath(tmp)
@@ -289,7 +295,6 @@ func TestGetJSONManifest(t *testing.T) {
 
 		exePath := filepath.Join(tmp, "module.tgz")
 		exeDir := filepath.Dir(exePath)
-		exeMetaJSONFilepath := filepath.Join(exeDir, "meta.json")
 		unpackedModDir := filepath.Join(tmp, "unpacked-mod-dir")
 		unpackedModMetaJSONFilepath := filepath.Join(unpackedModDir, "meta.json")
 		env := map[string]string{}
@@ -308,26 +313,6 @@ func TestGetJSONManifest(t *testing.T) {
 		test.That(t, err.Error(), test.ShouldContainSubstring, unpackedModDir)
 		test.That(t, err.Error(), test.ShouldContainSubstring, exeDir)
 
-		// meta.json found in executable directory; parsing fails
-		exeMetaJSONFile, err := os.Create(exeMetaJSONFilepath)
-		test.That(t, err, test.ShouldBeNil)
-		defer exeMetaJSONFile.Close()
-
-		meta, moduleWorkingDirectory, err = modLocalTar.getJSONManifest(unpackedModDir, env)
-		test.That(t, meta, test.ShouldBeNil)
-		test.That(t, moduleWorkingDirectory, test.ShouldBeEmpty)
-		test.That(t, err, test.ShouldNotBeNil)
-		test.That(t, err.Error(), test.ShouldContainSubstring, "local tarball")
-		test.That(t, errors.Is(err, os.ErrNotExist), test.ShouldBeFalse)
-
-		// meta.json found in executable directory; parsing succeeds
-		testWriteJSON(t, exeMetaJSONFilepath, validJSONManifest)
-
-		meta, moduleWorkingDirectory, err = modLocalTar.getJSONManifest(unpackedModDir, env)
-		test.That(t, *meta, test.ShouldResemble, validJSONManifest)
-		test.That(t, moduleWorkingDirectory, test.ShouldEqual, exeDir)
-		test.That(t, err, test.ShouldBeNil)
-
 		// meta.json found in unpacked modular directory; parsing fails
 		unpackedModMetaJSONFile, err := os.Create(unpackedModMetaJSONFilepath)
 		test.That(t, err, test.ShouldBeNil)
@@ -425,6 +410,8 @@ func TestMergeEnvVars(t *testing.T) {
 // testWriteJSON is a t.Helper that serializes `value` to `path` as json.
 func testWriteJSON(t *testing.T, path string, value any) {
 	t.Helper()
+	err := os.MkdirAll(filepath.Dir(path), 0o700)
+	test.That(t, err, test.ShouldBeNil)
 	file, err := os.Create(path)
 	test.That(t, err, test.ShouldBeNil)
 	defer file.Close()