Weldr API: make Image Type denylist distribution-specific

Change the Image Type denylist in Weldr API from being applied to all
distributions to being distribution-specific. A special name `*`
can be used in the configuration to match any distribution
or any image type.

Modify NEWS entry and unit tests to reflect this change.

Signed-off-by: Tomas Hozza <thozza@redhat.com>

cmd/osbuild-composer/composer.go

@@ -89,8 +89,9 @@ func NewComposer(config *ComposerConfigFile, stateDir, cacheDir string, logger *
 	return &c, nil
 }
 
-func (c *Composer) InitWeldr(repoPaths []string, weldrListener net.Listener, imageTypeDenylist []string) (err error) {
+func (c *Composer) InitWeldr(repoPaths []string, weldrListener net.Listener,
-	c.weldr, err = weldr.New(repoPaths, c.stateDir, c.rpm, c.distros, c.logger, c.workers, imageTypeDenylist)
+	distrosImageTypeDenylist map[string][]string) (err error) {
+	c.weldr, err = weldr.New(repoPaths, c.stateDir, c.rpm, c.distros, c.logger, c.workers, distrosImageTypeDenylist)
 	if err != nil {
 		return err
 	}

cmd/osbuild-composer/config.go

@@ -29,10 +29,28 @@ type ComposerConfigFile struct {
 		IdentityFilter []string `toml:"identity_filter"`
 	} `toml:"composer_api"`
 	WeldrAPI struct {
-		ImageTypeDenylist []string `toml:"image_type_denylist"`
+		DistroConfigs map[string]WeldrDistroConfig `toml:"distros"`
 	} `toml:"weldr_api"`
 }
 
+type WeldrDistroConfig struct {
+	ImageTypeDenyList []string `toml:"image_type_denylist"`
+}
+
+// weldrDistrosImageTypeDenyList returns a map of distro-specific Image Type
+// deny lists for Weldr API.
+func (c *ComposerConfigFile) weldrDistrosImageTypeDenyList() map[string][]string {
+	distrosImageTypeDenyList := map[string][]string{}
+
+	for distro, distroConfig := range c.WeldrAPI.DistroConfigs {
+		if distroConfig.ImageTypeDenyList != nil {
+			distrosImageTypeDenyList[distro] = append([]string{}, distroConfig.ImageTypeDenyList...)
+		}
+	}
+
+	return distrosImageTypeDenyList
+}
+
 func LoadConfig(name string) (*ComposerConfigFile, error) {
 	var c ComposerConfigFile
 	_, err := toml.DecodeFile(name, &c)
@@ -69,6 +87,9 @@ func loadConfigFromEnv(intf interface{}) error {
 		case reflect.Slice:
 			// no-op
 			continue
+		case reflect.Map:
+			// no-op
+			continue
 		case reflect.Struct:
 			err := loadConfigFromEnv(fieldV.Addr().Interface())
 			if err != nil {

cmd/osbuild-composer/config_test.go

@@ -36,7 +36,8 @@ func TestConfig(t *testing.T) {
 	require.Equal(t, config.Worker.AllowedDomains, []string{"osbuild.org"})
 	require.Equal(t, config.Worker.CA, "/etc/osbuild-composer/ca-crt.pem")
 
-	require.Equal(t, config.WeldrAPI.ImageTypeDenylist, []string{"qcow2", "vmdk"})
+	require.Equal(t, []string{"qcow2", "vmdk"}, config.WeldrAPI.DistroConfigs["*"].ImageTypeDenyList)
+	require.Equal(t, []string{"qcow2"}, config.WeldrAPI.DistroConfigs["rhel-84"].ImageTypeDenyList)
 
 	require.Equal(t, "overwrite-me-db", config.Worker.PGDatabase)
 
@@ -46,3 +47,16 @@ func TestConfig(t *testing.T) {
 	require.NotNil(t, config)
 	require.Equal(t, "composer-db", config.Worker.PGDatabase)
 }
+
+func TestWeldrDistrosImageTypeDenyList(t *testing.T) {
+	config, err := LoadConfig("testdata/test.toml")
+	require.NoError(t, err)
+	require.NotNil(t, config)
+
+	expectedWeldrDistrosImageTypeDenyList := map[string][]string{
+		"*":       {"qcow2", "vmdk"},
+		"rhel-84": {"qcow2"},
+	}
+
+	require.Equal(t, expectedWeldrDistrosImageTypeDenyList, config.weldrDistrosImageTypeDenyList())
+}

cmd/osbuild-composer/main.go

@@ -69,7 +69,7 @@ func main() {
 			log.Fatal("The osbuild-composer.socket unit is misconfigured. It should contain only one socket.")
 		}
 
-		err = composer.InitWeldr(repositoryConfigs, l[0], config.WeldrAPI.ImageTypeDenylist)
+		err = composer.InitWeldr(repositoryConfigs, l[0], config.weldrDistrosImageTypeDenyList())
 		if err != nil {
 			log.Fatalf("Error initializing weldr API: %v", err)
 		}

cmd/osbuild-composer/testdata/test.toml

@@ -7,5 +7,8 @@ allowed_domains = [ "osbuild.org" ]
 ca = "/etc/osbuild-composer/ca-crt.pem"
 pg_database = "overwrite-me-db"
 
-[weldr_api]
+[weldr_api.distros."*"]
 image_type_denylist = [ "qcow2", "vmdk" ]
+
+[weldr_api.distros.rhel-84]
+image_type_denylist = [ "qcow2" ]

docs/news/unreleased/weldr-image-type-denylist.md

@@ -1,16 +1,24 @@
 # Weldr API: introduce the ablility to limit exposed Image Types by configuration
 
-Extend Weldr API to accept a list of denied image types, which should
+Extend Weldr API to accept a map of distribution-specific lists of denied
-not be exposed via API for any supported distribution. This functionality is
+image types, which should not be exposed via API. A special name `*` can be
-needed to not expose image types which can't be successfully built outside
+used to match any Distribution or any Image Type. This functionality is needed
+to not expose image types which can't be successfully built outside
 of Red Hat VPN.
 
 The list of denied Image Types is defined in `osbuild-composer` configuration,
 `/etc/osbuild-composer/osbuild-composer.toml`.
 
 Example configuration denying the building of `qcow2` and `vmdk` Image Types
-via Weldr API:
+via Weldr API for any distribution:
 ```toml
-[weldr_api]
+[weldr_api.distros."*"]
+image_type_denylist = [ "qcow2", "vmdk" ]
+```
+
+Example configuration denying the building of `qcow2` and `vmdk` Image Types
+via Weldr API for `rhel-84` distribution:
+```toml
+[weldr_api.distros.rhel-84]
 image_type_denylist = [ "qcow2", "vmdk" ]
 ```

internal/weldr/api.go

@@ -59,7 +59,7 @@ type API struct {
 	distros        []string                 // Supported distro names
 
 	//  List of ImageType names, which should not be exposed by the API
-	imageTypeDenylist []string
+	distrosImageTypeDenylist map[string][]string
 }
 
 type ComposeState int
@@ -121,7 +121,7 @@ var ValidBlueprintName = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`)
 func NewTestAPI(rpm rpmmd.RPMMD, arch distro.Arch, dr *distroregistry.Registry,
 	rr *reporegistry.RepoRegistry, logger *log.Logger,
 	store *store.Store, workers *worker.Server, compatOutputDir string,
-	imageTypeDenylist []string) *API {
+	distrosImageTypeDenylist map[string][]string) *API {
 
 	// Use the first entry as the host distribution
 	hostDistro := dr.GetDistro(dr.List()[0])
@@ -136,13 +136,13 @@ func NewTestAPI(rpm rpmmd.RPMMD, arch distro.Arch, dr *distroregistry.Registry,
 		hostDistroName:           hostDistro.Name(),
 		distroRegistry:           dr,
 		distros:                  validDistros(rr, dr, arch.Name(), logger),
-		imageTypeDenylist: imageTypeDenylist,
+		distrosImageTypeDenylist: distrosImageTypeDenylist,
 	}
 	return setupRouter(api)
 }
 
 func New(repoPaths []string, stateDir string, rpm rpmmd.RPMMD, dr *distroregistry.Registry,
-	logger *log.Logger, workers *worker.Server, imageTypeDenylist []string) (*API, error) {
+	logger *log.Logger, workers *worker.Server, distrosImageTypeDenylist map[string][]string) (*API, error) {
 	if logger == nil {
 		logger = log.New(os.Stdout, "", 0)
 	}
@@ -184,7 +184,7 @@ func New(repoPaths []string, stateDir string, rpm rpmmd.RPMMD, dr *distroregistr
 		hostDistroName:           hostDistro.Name(),
 		distroRegistry:           dr,
 		distros:                  validDistros(rr, dr, hostArch.Name(), logger),
-		imageTypeDenylist: imageTypeDenylist,
+		distrosImageTypeDenylist: distrosImageTypeDenylist,
 	}
 	return setupRouter(api), nil
 }
@@ -385,15 +385,23 @@ func (api *API) openImageFile(composeId uuid.UUID, compose store.Compose) (io.Re
 	return reader, size, nil
 }
 
-// checkImageTypeDenylist checks the given ImageType name against the ImageType Denylist
+// checkImageTypeDenylist checks the given ImageType and Distro names against
-// provided to the API from configuration. If the given ImageType is not allowed the method
+//the distro-specific ImageType Denylist provided to the API from configuration.
-// returns an `error`. Otherwise `nil` is returned.
+// If the given ImageType is not allowed the method returns an `error`.
-func (api *API) checkImageTypeDenylist(imageType string) error {
+// Otherwise `nil` is returned.
-	for _, deniedImgType := range api.imageTypeDenylist {
+func (api *API) checkImageTypeDenylist(distroName, imageType string) error {
-		if imageType == deniedImgType {
+	anyImageType := "*"
+	anyDistro := "*"
+
+	for deniedDistro, deniedImgTypes := range api.distrosImageTypeDenylist {
+		if distroName == deniedDistro || deniedDistro == anyDistro {
+			for _, deniedImgType := range deniedImgTypes {
+				if imageType == deniedImgType || deniedImgType == anyImageType {
 					return fmt.Errorf("image type denied by configuration: %q", imageType)
 				}
 			}
+		}
+	}
 	return nil
 }
 
@@ -401,7 +409,7 @@ func (api *API) checkImageTypeDenylist(imageType string) error {
 // This is necessary because different distros support different image types, and the image
 // type may have a different package set than other distros.
 func (api *API) getImageType(distroName, imageType string) (distro.ImageType, error) {
-	if err := api.checkImageTypeDenylist(imageType); err != nil {
+	if err := api.checkImageTypeDenylist(distroName, imageType); err != nil {
 		return nil, err
 	}
 
@@ -2514,7 +2522,7 @@ func (api *API) composeTypesHandler(writer http.ResponseWriter, request *http.Re
 	}
 
 	for _, format := range arch.ListImageTypes() {
-		if err := api.checkImageTypeDenylist(format); err != nil {
+		if err := api.checkImageTypeDenylist(distroName, format); err != nil {
 			continue
 		}
 		reply.Types = append(reply.Types, composeType{format, true})

internal/weldr/api_test.go

@@ -67,7 +67,7 @@ func createWeldrAPI(tempdir string, fixtureGenerator rpmmd_mock.FixtureGenerator
 // createWeldrAPI2 is an alternative function to createWeldrAPI, using different test architecture
 // with more than a single image type
 func createWeldrAPI2(tempdir string, fixtureGenerator rpmmd_mock.FixtureGenerator,
-	imageTypeDenylist []string) (*API, *store.Store) {
+	distroImageTypeDenylist map[string][]string) (*API, *store.Store) {
 	fixture := fixtureGenerator(tempdir)
 	rpm := rpmmd_mock.NewRPMMDMock(fixture)
 
@@ -96,7 +96,7 @@ func createWeldrAPI2(tempdir string, fixtureGenerator rpmmd_mock.FixtureGenerato
 		panic(err)
 	}
 
-	return NewTestAPI(rpm, arch, dr, rr, nil, fixture.Store, fixture.Workers, "", imageTypeDenylist), fixture.Store
+	return NewTestAPI(rpm, arch, dr, rr, nil, fixture.Store, fixture.Workers, "", distroImageTypeDenylist), fixture.Store
 }
 
 func TestBasic(t *testing.T) {
@@ -1484,46 +1484,71 @@ func TestModulesList(t *testing.T) {
 func TestComposeTypes_ImageTypeDenylist(t *testing.T) {
 	var cases = []struct {
 		Path              string
-		ImageTypeDenylist []string
+		ImageTypeDenylist map[string][]string
 		ExpectedStatus    int
 		ExpectedJSON      string
 	}{
 		{
 			"/api/v1/compose/types",
-			[]string{},
+			map[string][]string{},
 			http.StatusOK,
 			fmt.Sprintf(`{"types": [{"enabled":true, "name":%q},{"enabled":true, "name":%q}]}`, test_distro.TestImageTypeName, test_distro.TestImageType2Name),
 		},
 		{
 			"/api/v1/compose/types?distro=test-distro-2",
-			[]string{},
+			map[string][]string{},
 			http.StatusOK,
 			fmt.Sprintf(`{"types": [{"enabled":true, "name":%q},{"enabled":true, "name":%q}]}`, test_distro.TestImageTypeName, test_distro.TestImageType2Name),
 		},
 		{
 			"/api/v1/compose/types",
-			[]string{test_distro.TestImageTypeName},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName}},
 			http.StatusOK,
 			fmt.Sprintf(`{"types": [{"enabled":true, "name":%q}]}`, test_distro.TestImageType2Name),
 		},
 		{
 			"/api/v1/compose/types?distro=test-distro-2",
-			[]string{test_distro.TestImageTypeName},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName}},
 			http.StatusOK,
 			fmt.Sprintf(`{"types": [{"enabled":true, "name":%q}]}`, test_distro.TestImageType2Name),
 		},
 		{
 			"/api/v1/compose/types",
-			[]string{test_distro.TestImageTypeName, test_distro.TestImageType2Name},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName, test_distro.TestImageType2Name}},
 			http.StatusOK,
 			`{"types": null}`,
 		},
 		{
 			"/api/v1/compose/types?distro=test-distro-2",
-			[]string{test_distro.TestImageTypeName, test_distro.TestImageType2Name},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName, test_distro.TestImageType2Name}},
 			http.StatusOK,
 			`{"types": null}`,
 		},
+		{
+			"/api/v1/compose/types",
+			map[string][]string{"*": {test_distro.TestImageTypeName}},
+			http.StatusOK,
+			fmt.Sprintf(`{"types": [{"enabled":true, "name":%q}]}`, test_distro.TestImageType2Name),
+		},
+		{
+			"/api/v1/compose/types",
+			map[string][]string{"*": {test_distro.TestImageTypeName, test_distro.TestImageType2Name}},
+			http.StatusOK,
+			`{"types": null}`,
+		},
+		{
+			"/api/v1/compose/types",
+			map[string][]string{test_distro.TestDistro2Name: {"*"}},
+			http.StatusOK,
+			`{"types": null}`,
+		},
+		{
+			"/api/v1/compose/types?distro=test-distro-2",
+			map[string][]string{test_distro.TestDistroName: {"*"}},
+			http.StatusOK,
+			fmt.Sprintf(`{"types": [{"enabled":true, "name":%q}, {"enabled":true, "name":%q}]}`,
+				test_distro.TestImageTypeName, test_distro.TestImageType2Name),
+		},
 	}
 
 	tempdir, err := ioutil.TempDir("", "weldr-tests-")
@@ -1583,7 +1608,7 @@ func TestComposePOST_ImageTypeDenylist(t *testing.T) {
 	var cases = []struct {
 		Path              string
 		Body              string
-		imageTypeDenylist []string
+		imageTypeDenylist map[string][]string
 		ExpectedStatus    int
 		ExpectedJSON      string
 		ExpectedCompose   *store.Compose
@@ -1592,7 +1617,7 @@ func TestComposePOST_ImageTypeDenylist(t *testing.T) {
 		{
 			"/api/v1/compose",
 			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
-			[]string{},
+			map[string][]string{},
 			http.StatusOK,
 			`{"status":true}`,
 			expectedComposeLocal,
@@ -1601,7 +1626,7 @@ func TestComposePOST_ImageTypeDenylist(t *testing.T) {
 		{
 			"/api/v1/compose",
 			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageType2Name),
-			[]string{},
+			map[string][]string{},
 			http.StatusOK,
 			`{"status": true}`,
 			expectedComposeLocal2,
@@ -1610,7 +1635,7 @@ func TestComposePOST_ImageTypeDenylist(t *testing.T) {
 		{
 			"/api/v1/compose",
 			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
-			[]string{test_distro.TestImageTypeName},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName}},
 			http.StatusBadRequest,
 			fmt.Sprintf(`{"status":false,"errors":[{"id":"UnknownComposeType","msg":"Unknown compose type for architecture: %s"}]}`, test_distro.TestImageTypeName),
 			expectedComposeLocal,
@@ -1619,7 +1644,7 @@ func TestComposePOST_ImageTypeDenylist(t *testing.T) {
 		{
 			"/api/v1/compose",
 			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageType2Name),
-			[]string{test_distro.TestImageTypeName},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName}},
 			http.StatusOK,
 			`{"status": true}`,
 			expectedComposeLocal2,
@@ -1628,7 +1653,7 @@ func TestComposePOST_ImageTypeDenylist(t *testing.T) {
 		{
 			"/api/v1/compose",
 			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
-			[]string{test_distro.TestImageTypeName, test_distro.TestImageType2Name},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName, test_distro.TestImageType2Name}},
 			http.StatusBadRequest,
 			fmt.Sprintf(`{"status":false,"errors":[{"id":"UnknownComposeType","msg":"Unknown compose type for architecture: %s"}]}`, test_distro.TestImageTypeName),
 			expectedComposeLocal,
@@ -1637,12 +1662,30 @@ func TestComposePOST_ImageTypeDenylist(t *testing.T) {
 		{
 			"/api/v1/compose",
 			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageType2Name),
-			[]string{test_distro.TestImageTypeName, test_distro.TestImageType2Name},
+			map[string][]string{test_distro.TestDistro2Name: {test_distro.TestImageTypeName, test_distro.TestImageType2Name}},
 			http.StatusBadRequest,
 			fmt.Sprintf(`{"status":false,"errors":[{"id":"UnknownComposeType","msg":"Unknown compose type for architecture: %s"}]}`, test_distro.TestImageType2Name),
 			expectedComposeLocal2,
 			[]string{"build_id"},
 		},
+		{
+			"/api/v1/compose",
+			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
+			map[string][]string{"*": {test_distro.TestImageTypeName}},
+			http.StatusBadRequest,
+			fmt.Sprintf(`{"status":false,"errors":[{"id":"UnknownComposeType","msg":"Unknown compose type for architecture: %s"}]}`, test_distro.TestImageTypeName),
+			expectedComposeLocal,
+			[]string{"build_id"},
+		},
+		{
+			"/api/v1/compose",
+			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
+			map[string][]string{test_distro.TestDistro2Name: {"*"}},
+			http.StatusBadRequest,
+			fmt.Sprintf(`{"status":false,"errors":[{"id":"UnknownComposeType","msg":"Unknown compose type for architecture: %s"}]}`, test_distro.TestImageTypeName),
+			expectedComposeLocal,
+			[]string{"build_id"},
+		},
 	}
 
 	tempdir, err := ioutil.TempDir("", "weldr-tests-")