Use current pundle identification

Make use of the current pundle identification to make pundles not
add includes for AllPackages accounting. Also revert some file and
package name blacklisting and fixup tests for cycles and directories
always being in a bundle/pack.

Author: William Douglas

builder/build_validate.go

@@ -493,6 +493,11 @@ func (b *Builder) resolvePkgFiles(pkg *pkgInfo, downloadRetries int) ([]*fileInf
 			continue
 		}
 
+		// Files with blacklisted characters are skipped
+		if swupd.FilenameBlacklisted(filepath.Base(path)) {
+			continue
+		}
+
 		// Directories are omitted from MCA because they may be missed from rpm output.
 		mode := fileMetadata[3]
 		if mode[:1] == "d" {

builder/bundles.go

@@ -404,21 +404,23 @@ func resolvePackagesWithOptions(numWorkers int, set bundleSet, packagerCmd []str
 				return
 			}
 
-			singlePkgMap := make(repoPkgMap)
-			for _, pkgs := range rpm {
-				for _, pkg := range pkgs {
-					if pkg.name == bundle.Name {
-						singlePkgMap[pkg.repo] = append(singlePkgMap[pkg.repo], pkg)
+			if bundle.Header.Maintainer == "pundle" {
+				singlePkgMap := make(repoPkgMap)
+				for _, pkgs := range rpm {
+					for _, pkg := range pkgs {
+						if pkg.name == bundle.Name {
+							singlePkgMap[pkg.repo] = append(singlePkgMap[pkg.repo], pkg)
+							break
+						}
+					}
+					if len(singlePkgMap) > 0 {
 						break
 					}
 				}
 				if len(singlePkgMap) > 0 {
-					break
+					rpm = singlePkgMap
 				}
 			}
-			if len(singlePkgMap) > 0 {
-				rpm = singlePkgMap
-			}
 
 			for _, pkgs := range rpm {
 				// Add packages to bundle's AllPackages

builder/bundleset.go

@@ -44,11 +44,10 @@ type bundleSet map[string]*bundle
 // the following constraints:
 //  1. Completeness. For each bundle in the set, every bundle included by that
 //     bundle is also in the set.
-//  2. Cycle-Free. The set contains no bundle include cycles.
 func validateAndFillBundleSet(bundles bundleSet) error {
 	// Sort the bundles so that all includes and optional (also-add) includes appear
 	// before a bundle, then calculate AllPackages for each bundle.
-	// Cycles and missing bundles are identified as part of sorting the bundles.
+	// Missing bundles are identified as part of sorting the bundles.
 	sortedBundles, err := sortBundles(bundles)
 	if err != nil {
 		return err
@@ -59,6 +58,15 @@ func validateAndFillBundleSet(bundles bundleSet) error {
 			b.AllPackages[k] = v
 		}
 	}
+	for _, b := range sortedBundles {
+		if b.Header.Maintainer != "pundle" {
+			for _, include := range b.DirectIncludes {
+				for k, v := range bundles[include].AllPackages {
+					b.AllPackages[k] = v
+				}
+			}
+		}
+	}
 
 	return nil
 }
@@ -85,6 +93,7 @@ func sortBundles(bundles bundleSet) ([]*bundle, error) {
 	visit = func(b *bundle) error {
 		switch mark[b] {
 		case Visiting:
+			// In a cycle, short circuit
 			return nil
 		case NotVisited:
 			mark[b] = Visiting
@@ -156,14 +165,21 @@ func validateBundleFile(filename string, lvl ValidationLevel) error {
 	}
 
 	// Strict Validation
-	err = validateBundle(b)
-	if err != nil {
-		errText += err.Error() + "\n"
-	}
+	if b.Header.Maintainer == "pundle" {
+		err = validatePundle(b)
+		if err != nil {
+			errText += err.Error() + "\n"
+		}
+	} else {
+		err = validateBundle(b)
+		if err != nil {
+			errText += err.Error() + "\n"
+		}
 
-	name := filepath.Base(filename)
-	if name != b.Header.Title {
-		errText += fmt.Sprintf("Bundle name %q and bundle header Title %q do not match\n", name, b.Header.Title)
+		name := filepath.Base(filename)
+		if name != b.Header.Title {
+			errText += fmt.Sprintf("Bundle name %q and bundle header Title %q do not match\n", name, b.Header.Title)
+		}
 	}
 
 	if errText != "" {
@@ -190,6 +206,31 @@ func validateBundleFilename(filename string) error {
 	return nil
 }
 
+func validatePundle(b *bundle) error {
+	var errText string
+
+	if b.Header.Title != "" {
+		errText = fmt.Sprintf("pundle name %q detected in pundle header Title (should be empty)\n", b.Header.Title)
+	}
+	if b.Header.Description != "" {
+		errText += "Non-empty Description in pundle header\n"
+	}
+	if b.Header.Maintainer != "pundle" {
+		errText += "Non-pundle Maintainer in bundle header\n"
+	}
+	if b.Header.Status == "" {
+		errText += "Non-empty Status in bundle header\n"
+	}
+	if b.Header.Capabilities == "" {
+		errText += "Non-empty Capabilities in bundle header\n"
+	}
+	if errText != "" {
+		return errors.New(strings.TrimSuffix(errText, "\n"))
+	}
+
+	return nil
+}
+
 func validateBundle(b *bundle) error {
 	var errText string
 

builder/bundleset_test.go

@@ -534,11 +534,57 @@ func TestParseBundleSet(t *testing.T) {
 			},
 		},
 
-		{"cyclic error two bundles",
-			FilesMap{"a": "include(b)", "b": "include(a)"}, Error},
+		{
+			"cycles are fine, two bundles",
+			FilesMap{
+				"a": Lines("include(b) A"),
+				"b": Lines("include(a) B"),
+			},
+			CountsMap{
+				"a": 2,
+				"b": 2,
+			},
+		},
+
+		{
+			"cycles are fine, two pundles",
+			FilesMap{
+				"a": "# [MAINTAINER]: pundle\ninclude(b)\nA",
+				"b": "# [MAINTAINER]: pundle\ninclude(a)\nB",
+			},
+			CountsMap{
+				"a": 1,
+				"b": 1,
+			},
+		},
 
-		{"cyclic error three bundles",
-			FilesMap{"a": "include(b)", "b": "include(c)", "c": "include(a)"}, Error},
+		{
+			"cycles are fine, three bundles",
+			FilesMap{
+				"a": Lines("include(c) include(b) A"),
+				"b": Lines("include(c) include(a) B"),
+				"c": Lines("include(b) include(a) C"),
+			},
+			CountsMap{
+				"a": 3,
+				"b": 3,
+				"c": 3,
+			},
+		},
+
+		{
+			"cycles are fine, three pundles",
+			FilesMap{
+				"a": "# [MAINTAINER]: pundle\ninclude(c)\ninclude(b)\nA",
+				"b": "# [MAINTAINER]: pundle\ninclude(c)\ninclude(a)\nB",
+				"c": "# [MAINTAINER]: pundle\ninclude(b)\ninclude(a)\nC",
+			},
+			CountsMap{
+				"a": 1,
+				"b": 1,
+				"c": 1,
+			},
+		},
 
 		{"bundle not available",
 			FilesMap{"a": "include(c)"}, Error},

swupd/filesystem.go

@@ -24,6 +24,13 @@ import (
 	"github.com/clearlinux/mixer-tools/log"
 )
 
+const illegalChars = ";&|*`/<>\\\"'\a"
+
+// FilenameBlacklisted checks for illegal characters in filename
+func FilenameBlacklisted(fname string) bool {
+	return strings.ContainsAny(fname, illegalChars)
+}
+
 // createFileRecord creates a manifest File entry from a file
 func (m *Manifest) createFileRecord(rootPath, path, removePrefix string, fi os.FileInfo) error {
 	file, err := recordFromFile(rootPath, path, removePrefix, fi)
@@ -67,6 +74,10 @@ func recordFromFile(rootPath, path, removePrefix string, fi os.FileInfo) (*File,
 		return nil, nil
 	}
 
+	if FilenameBlacklisted(filepath.Base(fname)) {
+		return nil, fmt.Errorf("%s is a blacklisted file name", fname)
+	}
+
 	file = &File{
 		Name: fname,
 		Info: fi,

swupd/manifest.go

@@ -759,6 +759,7 @@ func (m *Manifest) addManifestFiles(ui UpdateInfo, c config) error {
 		for f := range m.BundleInfo.Files {
 			isIncluded := false
 			for _, inc := range includes {
+				// Handle cycles
 				if inc.Name == m.Name {
 					continue
 				}

swupd/swupd_test.go

@@ -99,7 +99,7 @@ func TestFullRun(t *testing.T) {
 	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle"), ts.path("www/10/pack-test-bundle-from-0.tar"))
 	mustHaveDeltaCount(t, infoTestBundle, 0)
 	// Empty file (bundle file), "foo".
-	mustHaveFullfileCount(t, infoTestBundle, 2)
+	mustHaveFullfileCount(t, infoTestBundle, 3)
 
 	testBundle := ts.parseManifest(10, "test-bundle")
 	checkIncludes(t, testBundle, "os-core")
@@ -137,7 +137,7 @@ func TestFullRunDelta(t *testing.T) {
 
 	ts.createPack("os-core", 0, 10, ts.path("image"))
 	info := ts.createPack("test-bundle", 0, 10, ts.path("image"))
-	mustHaveFullfileCount(t, info, 4) // largefile, foo and foobarbaz and the test-bundle file.
+	mustHaveFullfileCount(t, info, 5) // largefile, foo and foobarbaz and the test-bundle file.
 
 	mustValidateZeroPack(t, ts.path("www/10/Manifest.os-core"), ts.path("www/10/pack-os-core-from-0.tar"))
 	mustValidateZeroPack(t, ts.path("www/10/Manifest.test-bundle"), ts.path("www/10/pack-test-bundle-from-0.tar"))
@@ -164,7 +164,7 @@ func TestFullRunDelta(t *testing.T) {
 
 	ts.createPack("os-core", 0, 20, ts.path("image"))
 	info = ts.createPack("test-bundle", 0, 20, ts.path("image"))
-	mustHaveFullfileCount(t, info, 2) // largefile and the test-bundle file.
+	mustHaveFullfileCount(t, info, 3) // largefile and the test-bundle file.
 
 	mustValidateZeroPack(t, ts.path("www/20/Manifest.os-core"), ts.path("www/20/pack-os-core-from-0.tar"))
 	mustValidateZeroPack(t, ts.path("www/20/Manifest.test-bundle"), ts.path("www/20/pack-test-bundle-from-0.tar"))