[lxc-devel] [distrobuilder/master] Fix filtering

monstermunchkin on Github lxc-bot at linuxcontainers.org
Tue Oct 15 07:10:21 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 310 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191015/703db16d/attachment-0001.bin>
-------------- next part --------------
From 7c1528f7dec027b3a02ffbee2710587099ff9966 Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thomas.hipp at canonical.com>
Date: Tue, 15 Oct 2019 08:42:07 +0200
Subject: [PATCH 1/3] shared/definition: Fix filter for repositories

Signed-off-by: Thomas Hipp <thomas.hipp at canonical.com>
---
 shared/definition.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/shared/definition.go b/shared/definition.go
index 9d3180a..c3488bc 100644
--- a/shared/definition.go
+++ b/shared/definition.go
@@ -30,11 +30,11 @@ type DefinitionPackagesSet struct {
 
 // A DefinitionPackagesRepository contains data of a specific repository
 type DefinitionPackagesRepository struct {
-	DefinitionFilter
-	Name string `yaml:"name"`           // Name of the repository
-	URL  string `yaml:"url"`            // URL (may differ based on manager)
-	Type string `yaml:"type,omitempty"` // For distros that have more than one repository manager
-	Key  string `yaml:"key,omitempty"`  // GPG armored keyring
+	DefinitionFilter `yaml:",inline"`
+	Name             string `yaml:"name"`           // Name of the repository
+	URL              string `yaml:"url"`            // URL (may differ based on manager)
+	Type             string `yaml:"type,omitempty"` // For distros that have more than one repository manager
+	Key              string `yaml:"key,omitempty"`  // GPG armored keyring
 }
 
 // CustomManagerCmd represents a command for a custom manager.

From bc42da2b22f034c2ab215a72afc713c32d6f6ab4 Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thomas.hipp at canonical.com>
Date: Tue, 15 Oct 2019 08:57:25 +0200
Subject: [PATCH 2/3] distrobuilder: Pass entire definition to managePackages

ManagePackages handles the repositories which can be templated. We need
to pass the entire definition the managePackages in order for the
template to render properly.

Signed-off-by: Thomas Hipp <thomas.hipp at canonical.com>
---
 distrobuilder/chroot.go | 33 ++++++++++++++++-----------------
 distrobuilder/main.go   |  4 +---
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/distrobuilder/chroot.go b/distrobuilder/chroot.go
index 43912db..97d2fd9 100644
--- a/distrobuilder/chroot.go
+++ b/distrobuilder/chroot.go
@@ -9,36 +9,35 @@ import (
 	"github.com/lxc/distrobuilder/shared"
 )
 
-func managePackages(def shared.DefinitionPackages, actions []shared.DefinitionAction,
-	release string, architecture string, variant string) error {
+func managePackages(def *shared.Definition) error {
 	var err error
 	var manager *managers.Manager
 
-	if def.Manager != "" {
-		manager = managers.Get(def.Manager)
+	if def.Packages.Manager != "" {
+		manager = managers.Get(def.Packages.Manager)
 		if manager == nil {
 			return fmt.Errorf("Couldn't get manager")
 		}
 	} else {
-		manager = managers.GetCustom(*def.CustomManager)
+		manager = managers.GetCustom(*def.Packages.CustomManager)
 	}
 
 	// Handle repositories actions
-	if def.Repositories != nil && len(def.Repositories) > 0 {
+	if def.Packages.Repositories != nil && len(def.Packages.Repositories) > 0 {
 		if manager.RepoHandler == nil {
 			return fmt.Errorf("No repository handler present")
 		}
 
-		for _, repo := range def.Repositories {
-			if len(repo.Releases) > 0 && !lxd.StringInSlice(release, repo.Releases) {
+		for _, repo := range def.Packages.Repositories {
+			if len(repo.Releases) > 0 && !lxd.StringInSlice(def.Image.Release, repo.Releases) {
 				continue
 			}
 
-			if len(repo.Architectures) > 0 && !lxd.StringInSlice(architecture, repo.Architectures) {
+			if len(repo.Architectures) > 0 && !lxd.StringInSlice(def.Image.ArchitectureMapped, repo.Architectures) {
 				continue
 			}
 
-			if len(repo.Variants) > 0 && !lxd.StringInSlice(variant, repo.Variants) {
+			if len(repo.Variants) > 0 && !lxd.StringInSlice(def.Image.Variant, repo.Variants) {
 				continue
 			}
 
@@ -60,14 +59,14 @@ func managePackages(def shared.DefinitionPackages, actions []shared.DefinitionAc
 		return err
 	}
 
-	if def.Update {
+	if def.Packages.Update {
 		err = manager.Update()
 		if err != nil {
 			return err
 		}
 
 		// Run post update hook
-		for _, action := range actions {
+		for _, action := range def.GetRunnableActions("post-update") {
 			err = shared.RunScript(action.Action)
 			if err != nil {
 				return fmt.Errorf("Failed to run post-update: %s", err)
@@ -77,16 +76,16 @@ func managePackages(def shared.DefinitionPackages, actions []shared.DefinitionAc
 
 	var validSets []shared.DefinitionPackagesSet
 
-	for _, set := range def.Sets {
-		if len(set.Releases) > 0 && !lxd.StringInSlice(release, set.Releases) {
+	for _, set := range def.Packages.Sets {
+		if len(set.Releases) > 0 && !lxd.StringInSlice(def.Image.Release, set.Releases) {
 			continue
 		}
 
-		if len(set.Architectures) > 0 && !lxd.StringInSlice(architecture, set.Architectures) {
+		if len(set.Architectures) > 0 && !lxd.StringInSlice(def.Image.ArchitectureMapped, set.Architectures) {
 			continue
 		}
 
-		if len(set.Variants) > 0 && !lxd.StringInSlice(variant, set.Variants) {
+		if len(set.Variants) > 0 && !lxd.StringInSlice(def.Image.Variant, set.Variants) {
 			continue
 		}
 
@@ -104,7 +103,7 @@ func managePackages(def shared.DefinitionPackages, actions []shared.DefinitionAc
 		}
 	}
 
-	if def.Cleanup {
+	if def.Packages.Cleanup {
 		err = manager.Clean()
 		if err != nil {
 			return err
diff --git a/distrobuilder/main.go b/distrobuilder/main.go
index 0142f58..9c2ad0e 100644
--- a/distrobuilder/main.go
+++ b/distrobuilder/main.go
@@ -258,9 +258,7 @@ func (c *cmdGlobal) preRunBuild(cmd *cobra.Command, args []string) error {
 	}
 
 	// Install/remove/update packages
-	err = managePackages(c.definition.Packages,
-		c.definition.GetRunnableActions("post-update"), c.definition.Image.Release,
-		c.definition.Image.ArchitectureMapped, c.definition.Image.Variant)
+	err = managePackages(c.definition)
 	if err != nil {
 		return fmt.Errorf("Failed to manage packages: %s", err)
 	}

From 9e8f2578bb89024ad844efbd073481cd4d1d239f Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thomas.hipp at canonical.com>
Date: Tue, 15 Oct 2019 09:06:21 +0200
Subject: [PATCH 3/3] *: Make filtering easier

Signed-off-by: Thomas Hipp <thomas.hipp at canonical.com>
---
 distrobuilder/chroot.go         | 22 ++-----------
 distrobuilder/main_build-dir.go | 15 ++-------
 distrobuilder/main_lxc.go       |  9 +-----
 distrobuilder/main_lxd.go       |  7 +----
 shared/definition.go            | 55 ++++++++++++++++++++++-----------
 shared/definition_test.go       | 11 +++++++
 6 files changed, 54 insertions(+), 65 deletions(-)

diff --git a/distrobuilder/chroot.go b/distrobuilder/chroot.go
index 97d2fd9..f251316 100644
--- a/distrobuilder/chroot.go
+++ b/distrobuilder/chroot.go
@@ -3,8 +3,6 @@ package main
 import (
 	"fmt"
 
-	lxd "github.com/lxc/lxd/shared"
-
 	"github.com/lxc/distrobuilder/managers"
 	"github.com/lxc/distrobuilder/shared"
 )
@@ -29,15 +27,7 @@ func managePackages(def *shared.Definition) error {
 		}
 
 		for _, repo := range def.Packages.Repositories {
-			if len(repo.Releases) > 0 && !lxd.StringInSlice(def.Image.Release, repo.Releases) {
-				continue
-			}
-
-			if len(repo.Architectures) > 0 && !lxd.StringInSlice(def.Image.ArchitectureMapped, repo.Architectures) {
-				continue
-			}
-
-			if len(repo.Variants) > 0 && !lxd.StringInSlice(def.Image.Variant, repo.Variants) {
+			if !shared.ApplyFilter(&repo, def.Image.Release, def.Image.ArchitectureMapped, def.Image.Variant) {
 				continue
 			}
 
@@ -77,15 +67,7 @@ func managePackages(def *shared.Definition) error {
 	var validSets []shared.DefinitionPackagesSet
 
 	for _, set := range def.Packages.Sets {
-		if len(set.Releases) > 0 && !lxd.StringInSlice(def.Image.Release, set.Releases) {
-			continue
-		}
-
-		if len(set.Architectures) > 0 && !lxd.StringInSlice(def.Image.ArchitectureMapped, set.Architectures) {
-			continue
-		}
-
-		if len(set.Variants) > 0 && !lxd.StringInSlice(def.Image.Variant, set.Variants) {
+		if !shared.ApplyFilter(&set, def.Image.Release, def.Image.ArchitectureMapped, def.Image.Variant) {
 			continue
 		}
 
diff --git a/distrobuilder/main_build-dir.go b/distrobuilder/main_build-dir.go
index de44d61..3f8a3a0 100644
--- a/distrobuilder/main_build-dir.go
+++ b/distrobuilder/main_build-dir.go
@@ -3,10 +3,10 @@ package main
 import (
 	"fmt"
 
-	lxd "github.com/lxc/lxd/shared"
 	"github.com/spf13/cobra"
 
 	"github.com/lxc/distrobuilder/generators"
+	"github.com/lxc/distrobuilder/shared"
 )
 
 type cmdBuildDir struct {
@@ -28,18 +28,7 @@ func (c *cmdBuildDir) command() *cobra.Command {
 					return fmt.Errorf("Unknown generator '%s'", file.Generator)
 				}
 
-				if len(file.Releases) > 0 && !lxd.StringInSlice(
-					c.global.definition.Image.Release, file.Releases) {
-					continue
-				}
-
-				if len(file.Architectures) > 0 && !lxd.StringInSlice(
-					c.global.definition.Image.ArchitectureMapped, file.Architectures) {
-					continue
-				}
-
-				if len(file.Variants) > 0 && !lxd.StringInSlice(
-					c.global.definition.Image.Variant, file.Variants) {
+				if !shared.ApplyFilter(&file, c.global.definition.Image.Release, c.global.definition.Image.ArchitectureMapped, c.global.definition.Image.Variant) {
 					continue
 				}
 
diff --git a/distrobuilder/main_lxc.go b/distrobuilder/main_lxc.go
index f95c369..0b8fb77 100644
--- a/distrobuilder/main_lxc.go
+++ b/distrobuilder/main_lxc.go
@@ -3,7 +3,6 @@ package main
 import (
 	"fmt"
 
-	lxd "github.com/lxc/lxd/shared"
 	"github.com/spf13/cobra"
 
 	"github.com/lxc/distrobuilder/generators"
@@ -49,13 +48,7 @@ func (c *cmdLXC) run(cmd *cobra.Command, args []string) error {
 			return fmt.Errorf("Unknown generator '%s'", file.Generator)
 		}
 
-		if len(file.Releases) > 0 && !lxd.StringInSlice(
-			c.global.definition.Image.Release, file.Releases) {
-			continue
-		}
-
-		if len(file.Variants) > 0 && !lxd.StringInSlice(
-			c.global.definition.Image.Variant, file.Variants) {
+		if !shared.ApplyFilter(&file, c.global.definition.Image.Release, c.global.definition.Image.ArchitectureMapped, c.global.definition.Image.Variant) {
 			continue
 		}
 
diff --git a/distrobuilder/main_lxd.go b/distrobuilder/main_lxd.go
index 808673e..9aea6dc 100644
--- a/distrobuilder/main_lxd.go
+++ b/distrobuilder/main_lxd.go
@@ -68,12 +68,7 @@ func (c *cmdLXD) run(cmd *cobra.Command, args []string) error {
 		c.global.flagCacheDir, *c.global.definition)
 
 	for _, file := range c.global.definition.Files {
-		if len(file.Releases) > 0 && !lxd.StringInSlice(c.global.definition.Image.Release,
-			file.Releases) {
-			continue
-		}
-
-		if len(file.Variants) > 0 && !lxd.StringInSlice(c.global.definition.Image.Variant, file.Variants) {
+		if !shared.ApplyFilter(&file, c.global.definition.Image.Release, c.global.definition.Image.ArchitectureMapped, c.global.definition.Image.Variant) {
 			continue
 		}
 
diff --git a/shared/definition.go b/shared/definition.go
index c3488bc..e31725f 100644
--- a/shared/definition.go
+++ b/shared/definition.go
@@ -12,6 +12,12 @@ import (
 	lxdarch "github.com/lxc/lxd/shared/osarch"
 )
 
+type Filter interface {
+	GetReleases() []string
+	GetArchitectures() []string
+	GetVariants() []string
+}
+
 // A DefinitionFilter defines filters for various actions.
 type DefinitionFilter struct {
 	Releases      []string `yaml:"releases,omitempty"`
@@ -19,6 +25,18 @@ type DefinitionFilter struct {
 	Variants      []string `yaml:"variants,omitempty"`
 }
 
+func (d *DefinitionFilter) GetReleases() []string {
+	return d.Releases
+}
+
+func (d *DefinitionFilter) GetArchitectures() []string {
+	return d.Architectures
+}
+
+func (d *DefinitionFilter) GetVariants() []string {
+	return d.Variants
+}
+
 // A DefinitionPackagesSet is a set of packages which are to be installed
 // or removed.
 type DefinitionPackagesSet struct {
@@ -421,15 +439,7 @@ func (d *Definition) GetRunnableActions(trigger string) []DefinitionAction {
 			continue
 		}
 
-		if len(action.Releases) > 0 && !shared.StringInSlice(d.Image.Release, action.Releases) {
-			continue
-		}
-
-		if len(action.Architectures) > 0 && !shared.StringInSlice(d.Image.ArchitectureMapped, action.Architectures) {
-			continue
-		}
-
-		if len(action.Variants) > 0 && !shared.StringInSlice(d.Image.Variant, action.Variants) {
+		if !ApplyFilter(&action, d.Image.Release, d.Image.ArchitectureMapped, d.Image.Variant) {
 			continue
 		}
 
@@ -448,15 +458,7 @@ func (d *Definition) GetEarlyPackages(action string) []string {
 			continue
 		}
 
-		if len(set.Releases) > 0 && !shared.StringInSlice(d.Image.Release, set.Releases) {
-			continue
-		}
-
-		if len(set.Architectures) > 0 && !shared.StringInSlice(d.Image.ArchitectureMapped, set.Architectures) {
-			continue
-		}
-
-		if len(set.Variants) > 0 && !shared.StringInSlice(d.Image.Variant, set.Variants) {
+		if !ApplyFilter(&set, d.Image.Release, d.Image.ArchitectureMapped, d.Image.Variant) {
 			continue
 		}
 
@@ -530,3 +532,20 @@ func getFieldByTag(v reflect.Value, t reflect.Type, tag string) (reflect.Value,
 	// Return its value if it's a primitive type
 	return v, nil
 }
+
+// ApplyFilter returns true if the filter matches.
+func ApplyFilter(filter Filter, release string, architecture string, variant string) bool {
+	if len(filter.GetReleases()) > 0 && !shared.StringInSlice(release, filter.GetReleases()) {
+		return false
+	}
+
+	if len(filter.GetArchitectures()) > 0 && !shared.StringInSlice(architecture, filter.GetArchitectures()) {
+		return false
+	}
+
+	if len(filter.GetVariants()) > 0 && !shared.StringInSlice(variant, filter.GetVariants()) {
+		return false
+	}
+
+	return true
+}
diff --git a/shared/definition_test.go b/shared/definition_test.go
index 7b069f3..6186501 100644
--- a/shared/definition_test.go
+++ b/shared/definition_test.go
@@ -503,3 +503,14 @@ func TestDefinitionFilter(t *testing.T) {
 	require.Contains(t, def.Packages.Sets[0].Packages, "foo")
 	require.Contains(t, def.Packages.Sets[0].Architectures, "amd64")
 }
+
+func TestApplyFilter(t *testing.T) {
+	repo := DefinitionPackagesRepository{}
+
+	repo.Variants = []string{"default"}
+	repo.Architectures = []string{"amd64", "i386"}
+	repo.Releases = []string{"foo"}
+
+	require.True(t, ApplyFilter(&repo, "foo", "amd64", "default"))
+	require.False(t, ApplyFilter(&repo, "", "arm64", "default"))
+}


More information about the lxc-devel mailing list