Repository: osbuild/osbuild-composer
Pull request #2146 (2021-12-21 13:36:31)

osbuild2: support 'format' and 'root-node' Tar stage options
Bring the Tar stage implementation on par with the current osbuild
schema. Specifically add the 'format' and 'root-node' options to the
stage options structure.

Add stage options validation along with appropriate unit tests.

Signed-off-by: Tomas Hozza <thozza@redhat.com>


This pull request includes:

- [ ] adequate testing for the new functionality or fixed issue
- [ ] adequate documentation informing people about the change such as
  - [ ] submit a PR for the [guides](https://github.com/osbuild/guides) repository if this PR changed any behavior described there: https://www.osbuild.org/guides/

<!--
Thanks for proposing a change to osbuild-composer!

Please don't remove the above check list. These are things that each pull
request must have before it is merged. It helps maintainers to not forget
anything.

If the reason for ticking any of the boxes is ambiguous, please add a short
note explaining why.

In addition, if this pull request fixes a downstream issue, please refer to
test/README.md and add these additional items:

- [ ] 1st commit of any `rhbz#` related PR contains bug reproducer; CI reports FAIL or
- [ ] PR contains automated tests for new functionality and
- [ ] QE has approved reproducer/new tests and
- [ ] Subsequent commits provide bug fixes without modifying the reproducer; CI reports PASS and
- [ ] QE approves this PR; RHBZ status is set to `MODIFIED + Verified=Tested`
-->


thozza: Related to:

- https://github.com/osbuild/osbuild/pull/698
- https://github.com/osbuild/osbuild/pull/744

The `format` option is required for the GCE image.

Patch:
diff --git a/internal/osbuild2/tar_stage.go b/internal/osbuild2/tar_stage.go
index 995de834c..099764f6c 100644
--- a/internal/osbuild2/tar_stage.go
+++ b/internal/osbuild2/tar_stage.go
@@ -1,9 +1,33 @@
 package osbuild2
 
+import "fmt"
+
+type TarArchiveFormat string
+
+// valid values for the 'format' Tar stage option
+const (
+	TarArchiveFormatGnu    TarArchiveFormat = "gnu"
+	TarArchiveFormatOldgnu TarArchiveFormat = "oldngu"
+	TarArchiveFormatPosix  TarArchiveFormat = "posix"
+	TarArchiveFormatUstar  TarArchiveFormat = "ustar"
+	TarArchiveFormatV7     TarArchiveFormat = "v7"
+)
+
+type TarRootNode string
+
+// valid values for the 'root-node' Tar stage option
+const (
+	TarRootNodeInclude TarRootNode = "include"
+	TarRootNodeOmit    TarRootNode = "omit"
+)
+
 type TarStageOptions struct {
 	// Filename for tar archive
 	Filename string `json:"filename"`
 
+	// Archive format to use
+	Format TarArchiveFormat `json:"format,omitempty"`
+
 	// Enable support for POSIX ACLs
 	ACLs bool `json:"acls,omitempty"`
 
@@ -12,10 +36,54 @@ type TarStageOptions struct {
 
 	// Enable support for extended attributes
 	Xattrs bool `json:"xattrs,omitempty"`
+
+	// How to handle the root node: include or omit
+	RootNode TarRootNode `json:"root-node,omitempty"`
 }
 
 func (TarStageOptions) isStageOptions() {}
 
+func (o TarStageOptions) validate() error {
+	if o.Format != "" {
+		allowedArchiveFormatValues := []TarArchiveFormat{
+			TarArchiveFormatGnu,
+			TarArchiveFormatOldgnu,
+			TarArchiveFormatPosix,
+			TarArchiveFormatUstar,
+			TarArchiveFormatV7,
+		}
+		valid := false
+		for _, value := range allowedArchiveFormatValues {
+			if o.Format == value {
+				valid = true
+				break
+			}
+		}
+		if !valid {
+			return fmt.Errorf("'format' option does not allow %q as a value", o.Format)
+		}
+	}
+
+	if o.RootNode != "" {
+		allowedRootNodeValues := []TarRootNode{
+			TarRootNodeInclude,
+			TarRootNodeOmit,
+		}
+		valid := false
+		for _, value := range allowedRootNodeValues {
+			if o.RootNode == value {
+				valid = true
+				break
+			}
+		}
+		if !valid {
+			return fmt.Errorf("'root-node' option does not allow %q as a value", o.RootNode)
+		}
+	}
+
+	return nil
+}
+
 type TarStageInput struct {
 	inputCommon
 	References TarStageReferences `json:"references"`
@@ -36,6 +104,10 @@ func (TarStageReferences) isReferences() {}
 // Assembles a tree into a tar archive. Compression is determined by the suffix
 // (i.e., --auto-compress is used).
 func NewTarStage(options *TarStageOptions, inputs *TarStageInputs) *Stage {
+	if err := options.validate(); err != nil {
+		panic(err)
+	}
+
 	return &Stage{
 		Type:    "org.osbuild.tar",
 		Options: options,


Test patch:
diff --git a/internal/osbuild2/tar_stage_test.go b/internal/osbuild2/tar_stage_test.go
new file mode 100644
index 000000000..93b57a851
--- /dev/null
+++ b/internal/osbuild2/tar_stage_test.go
@@ -0,0 +1,69 @@
+package osbuild2
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewTarStage(t *testing.T) {
+	stageOptions := &TarStageOptions{Filename: "archive.tar.xz"}
+	stageInputs := &TarStageInputs{}
+	expectedStage := &Stage{
+		Type:    "org.osbuild.tar",
+		Options: stageOptions,
+		Inputs:  stageInputs,
+	}
+	actualStage := NewTarStage(stageOptions, stageInputs)
+	assert.Equal(t, expectedStage, actualStage)
+}
+
+func TestTarStageOptionsValidate(t *testing.T) {
+	tests := []struct {
+		name    string
+		options TarStageOptions
+		err     bool
+	}{
+		{
+			name:    "empty-options",
+			options: TarStageOptions{},
+			err:     false,
+		},
+		{
+			name: "invalid-archive-format",
+			options: TarStageOptions{
+				Filename: "archive.tar.xz",
+				Format:   "made-up-format",
+			},
+			err: true,
+		},
+		{
+			name: "invalid-root-node",
+			options: TarStageOptions{
+				Filename: "archive.tar.xz",
+				RootNode: "I-don't-care",
+			},
+			err: true,
+		},
+		{
+			name: "valid-data",
+			options: TarStageOptions{
+				Filename: "archive.tar.xz",
+				Format:   TarArchiveFormatOldgnu,
+				RootNode: TarRootNodeOmit,
+			},
+			err: false,
+		},
+	}
+	for idx, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.err {
+				assert.Errorf(t, tt.options.validate(), "%q didn't return an error [idx: %d]", tt.name, idx)
+				assert.Panics(t, func() { NewTarStage(&tt.options, &TarStageInputs{}) })
+			} else {
+				assert.NoErrorf(t, tt.options.validate(), "%q returned an error [idx: %d]", tt.name, idx)
+				assert.NotPanics(t, func() { NewTarStage(&tt.options, &TarStageInputs{}) })
+			}
+		})
+	}
+}