bundle, cmd/build: Do not write manifest if empty

This way it will be obvious if users run `opa build` and do not see
their manifest included. Also, update the `opa build` --revision flag
to not get set to the default empty value unconditionally.

Fixes #3480

Signed-off-by: Torin Sandall <torinsandall@gmail.com>

bundle/bundle.go

@@ -697,6 +697,10 @@ func (w *Writer) writeWasm(tw *tar.Writer, bundle Bundle) error {
 
 func writeManifest(tw *tar.Writer, bundle Bundle) error {
 
+	if bundle.Manifest.Equal(Manifest{}) {
+		return nil
+	}
+
 	var buf bytes.Buffer
 
 	if err := json.NewEncoder(&buf).Encode(bundle.Manifest); err != nil {

bundle/bundle_test.go

@@ -6,10 +6,12 @@
 package bundle
 
 import (
+	"archive/tar"
 	"bytes"
 	"compress/gzip"
 	"encoding/json"
 	"fmt"
+	"io"
 
 	"path/filepath"
 	"reflect"
@@ -907,6 +909,40 @@ func TestWriterUsePath(t *testing.T) {
 	}
 }
 
+func TestWriterSkipEmptyManifest(t *testing.T) {
+
+	bundle := Bundle{
+		Data:     map[string]interface{}{},
+		Manifest: Manifest{},
+	}
+
+	var buf bytes.Buffer
+
+	if err := NewWriter(&buf).Write(bundle); err != nil {
+		t.Fatal("Unexpected error:", err)
+	}
+
+	gr, err := gzip.NewReader(&buf)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tr := tar.NewReader(gr)
+	for {
+		f, err := tr.Next()
+		if err != nil {
+			if err != io.EOF {
+				t.Fatal(err)
+			}
+			break
+		}
+
+		if f.Name != "/data.json" {
+			t.Fatal("expected only /data.json but got:", f.Name)
+		}
+	}
+}
+
 func TestGenerateSignature(t *testing.T) {
 	signatures := SignaturesConfig{Signatures: []string{"some_token"}}
 

cmd/build.go

@@ -30,7 +30,7 @@ type buildParams struct {
 	optimizationLevel  int
 	entrypoints        repeatedStringFlag
 	outputFile         string
-	revision           string
+	revision           stringptrFlag
 	ignore             []string
 	debug              bool
 	algorithm          string
@@ -92,7 +92,9 @@ For more information on bundles see https://www.openpolicyagent.org/docs/latest/
 When -b is specified the 'build' command assumes paths refer to existing bundle files
 or directories following the bundle structure. If multiple bundles are provided, their
 contents are merged. If there are any merge conflicts (e.g., due to conflicting bundle
-roots), the command fails.
+roots), the command fails. When loading an existing bundle file, the .manifest from
+the input bundle will be included in the output bundle. Flags that set .manifest fields
+(such as --revision) override input bundle .manifest fields.
 
 The -O flag controls the optimization level. By default, optimization is disabled (-O=0).
 When optimization is enabled the 'build' command generates a bundle that is semantically
@@ -216,7 +218,7 @@ against OPA v0.22.0:
 	buildCommand.Flags().BoolVarP(&buildParams.debug, "debug", "", false, "enable debug output")
 	buildCommand.Flags().IntVarP(&buildParams.optimizationLevel, "optimize", "O", 0, "set optimization level")
 	buildCommand.Flags().VarP(&buildParams.entrypoints, "entrypoint", "e", "set slash separated entrypoint path")
-	buildCommand.Flags().StringVarP(&buildParams.revision, "revision", "r", "", "set output bundle revision")
+	buildCommand.Flags().VarP(&buildParams.revision, "revision", "r", "set output bundle revision")
 	buildCommand.Flags().StringVarP(&buildParams.outputFile, "output", "o", "bundle.tar.gz", "set the output filename")
 
 	addBundleModeFlag(buildCommand.Flags(), &buildParams.bundleMode, false)
@@ -273,10 +275,13 @@ func dobuild(params buildParams, args []string) error {
 		WithEntrypoints(params.entrypoints.v...).
 		WithPaths(args...).
 		WithFilter(buildCommandLoaderFilter(params.bundleMode, params.ignore)).
-		WithRevision(params.revision).
 		WithBundleVerificationConfig(bvc).
 		WithBundleSigningConfig(bsc)
 
+	if params.revision.isSet {
+		compiler = compiler.WithRevision(*params.revision.v)
+	}
+
 	if params.debug {
 		compiler = compiler.WithDebug(os.Stderr)
 	}

cmd/build_test.go

@@ -1,6 +1,9 @@
 package cmd
 
 import (
+	"archive/tar"
+	"compress/gzip"
+	"io"
 	"os"
 	"path"
 	"path/filepath"
@@ -34,6 +37,33 @@ func TestBuildProducesBundle(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
+
+		// Check that manifest is not written given no input manifest and no other flags
+		f, err := os.Open(params.outputFile)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer f.Close()
+
+		gr, err := gzip.NewReader(f)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		tr := tar.NewReader(gr)
+
+		for {
+			f, err := tr.Next()
+			if err == io.EOF {
+				break
+			} else if err != nil {
+				t.Fatal(err)
+			}
+			if f.Name == "/data.json" || strings.HasSuffix(f.Name, "/test.rego") {
+				continue
+			}
+			t.Fatal("unexpected file:", f.Name)
+		}
 	})
 }
 

cmd/eval.go

@@ -538,6 +538,8 @@ func readInputBytes(params evalCommandParams) ([]byte, error) {
 	return nil, nil
 }
 
+const stringType = "string"
+
 type repeatedStringFlag struct {
 	v     []string
 	isSet bool
@@ -551,7 +553,7 @@ func newrepeatedStringFlag(val []string) repeatedStringFlag {
 }
 
 func (f *repeatedStringFlag) Type() string {
-	return "string"
+	return stringType
 }
 
 func (f *repeatedStringFlag) String() string {

cmd/flags.go

@@ -171,7 +171,7 @@ func newcapabilitiesFlag() *capabilitiesFlag {
 }
 
 func (f *capabilitiesFlag) Type() string {
-	return "string"
+	return stringType
 }
 
 func (f *capabilitiesFlag) String() string {
@@ -188,3 +188,25 @@ func (f *capabilitiesFlag) Set(s string) error {
 	f.C, err = ast.LoadCapabilitiesJSON(fd)
 	return err
 }
+
+type stringptrFlag struct {
+	v     *string
+	isSet bool
+}
+
+func (f *stringptrFlag) Type() string {
+	return stringType
+}
+
+func (f *stringptrFlag) String() string {
+	if f.v == nil {
+		return ""
+	}
+	return *f.v
+}
+
+func (f *stringptrFlag) Set(s string) error {
+	f.v = &s
+	f.isSet = true
+	return nil
+}