Weldr API: introduce Image Type denylist for filtering exposed images

Extend Weldr API to accept a list of denied image types, which should
not be exposed via API for any supported distribution. This
functionality will be needed to not expose image types which can't be
successfully built outside of Red Hat VPN. Example of such images are
the official RHEL EC2 images, which include RHUI client packages not
available publicly.

Image Types are filters when listing available compose types and
creating a new compose using Weldr API.

Extend osbuild-composer configuration to allow specifying the list of
denied Image Types for Weldr API.

Add unit tests for implemented changes.

Add NEWS entry describing the newly introduced functionality.

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

cmd/osbuild-composer/composer.go

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

cmd/osbuild-composer/config.go

@@ -28,6 +28,9 @@ type ComposerConfigFile struct {
 	ComposerAPI struct {
 		IdentityFilter []string `toml:"identity_filter"`
 	} `toml:"composer_api"`
+	WeldrAPI struct {
+		ImageTypeDenylist []string `toml:"image_type_denylist"`
+	} `toml:"weldr_api"`
 }
 
 func LoadConfig(name string) (*ComposerConfigFile, error) {

cmd/osbuild-composer/config_test.go

@@ -36,6 +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, "overwrite-me-db", config.Worker.PGDatabase)
 
 	require.NoError(t, os.Setenv("PGDATABASE", "composer-db"))

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])
+		err = composer.InitWeldr(repositoryConfigs, l[0], config.WeldrAPI.ImageTypeDenylist)
 		if err != nil {
 			log.Fatalf("Error initializing weldr API: %v", err)
 		}

cmd/osbuild-composer/testdata/test.toml

@@ -6,3 +6,6 @@ ca = "/etc/osbuild-composer/ca-crt.pem"
 allowed_domains = [ "osbuild.org" ]
 ca = "/etc/osbuild-composer/ca-crt.pem"
 pg_database = "overwrite-me-db"
+
+[weldr_api]
+image_type_denylist = [ "qcow2", "vmdk" ]

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

@@ -0,0 +1,16 @@
+# 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
+not be exposed via API for any supported distribution. 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:
+```toml
+[weldr_api]
+image_type_denylist = [ "qcow2", "vmdk" ]
+```

internal/client/unit_test.go

@@ -68,7 +68,7 @@ func executeTests(m *testing.M) int {
 	}
 
 	logger := log.New(os.Stdout, "", 0)
-	api := weldr.NewTestAPI(rpm, arch, dr, rr, logger, fixture.Store, fixture.Workers, "")
+	api := weldr.NewTestAPI(rpm, arch, dr, rr, logger, fixture.Store, fixture.Workers, "", nil)
 	server := http.Server{Handler: api}
 	defer server.Close()
 

internal/weldr/api.go

@@ -57,6 +57,9 @@ type API struct {
 	hostDistroName string                   // Name of the host distro
 	distroRegistry *distroregistry.Registry // Available distros
 	distros        []string                 // Supported distro names
+
+	// List of ImageType names, which should not be exposed by the API
+	imageTypeDenylist []string
 }
 
 type ComposeState int
@@ -117,26 +120,29 @@ var ValidBlueprintName = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`)
 // NewTestAPI is used for the test framework, sets up a single distro
 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) *API {
+	store *store.Store, workers *worker.Server, compatOutputDir string,
+	imageTypeDenylist []string) *API {
 
 	// Use the first entry as the host distribution
 	hostDistro := dr.GetDistro(dr.List()[0])
 	api := &API{
-		store:           store,
-		workers:         workers,
-		rpmmd:           rpm,
-		arch:            arch,
-		repoRegistry:    rr,
-		logger:          logger,
-		compatOutputDir: compatOutputDir,
-		hostDistroName:  hostDistro.Name(),
-		distroRegistry:  dr,
-		distros:         validDistros(rr, dr, arch.Name(), logger),
+		store:             store,
+		workers:           workers,
+		rpmmd:             rpm,
+		arch:              arch,
+		repoRegistry:      rr,
+		logger:            logger,
+		compatOutputDir:   compatOutputDir,
+		hostDistroName:    hostDistro.Name(),
+		distroRegistry:    dr,
+		distros:           validDistros(rr, dr, arch.Name(), logger),
+		imageTypeDenylist: imageTypeDenylist,
 	}
 	return setupRouter(api)
 }
 
-func New(repoPaths []string, stateDir string, rpm rpmmd.RPMMD, dr *distroregistry.Registry, logger *log.Logger, workers *worker.Server) (*API, error) {
+func New(repoPaths []string, stateDir string, rpm rpmmd.RPMMD, dr *distroregistry.Registry,
+	logger *log.Logger, workers *worker.Server, imageTypeDenylist []string) (*API, error) {
 	if logger == nil {
 		logger = log.New(os.Stdout, "", 0)
 	}
@@ -168,16 +174,17 @@ func New(repoPaths []string, stateDir string, rpm rpmmd.RPMMD, dr *distroregistr
 	compatOutputDir := path.Join(stateDir, "outputs")
 
 	api := &API{
-		store:           store,
-		workers:         workers,
-		rpmmd:           rpm,
-		arch:            hostArch,
-		repoRegistry:    rr,
-		logger:          logger,
-		compatOutputDir: compatOutputDir,
-		hostDistroName:  hostDistro.Name(),
-		distroRegistry:  dr,
-		distros:         validDistros(rr, dr, hostArch.Name(), logger),
+		store:             store,
+		workers:           workers,
+		rpmmd:             rpm,
+		arch:              hostArch,
+		repoRegistry:      rr,
+		logger:            logger,
+		compatOutputDir:   compatOutputDir,
+		hostDistroName:    hostDistro.Name(),
+		distroRegistry:    dr,
+		distros:           validDistros(rr, dr, hostArch.Name(), logger),
+		imageTypeDenylist: imageTypeDenylist,
 	}
 	return setupRouter(api), nil
 }
@@ -378,10 +385,26 @@ 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
+// provided to the API from configuration. If the given ImageType is not allowed the method
+// returns an `error`. Otherwise `nil` is returned.
+func (api *API) checkImageTypeDenylist(imageType string) error {
+	for _, deniedImgType := range api.imageTypeDenylist {
+		if imageType == deniedImgType {
+			return fmt.Errorf("image type denied by configuration: %q", imageType)
+		}
+	}
+	return nil
+}
+
 // getImageType returns the ImageType for the selected distro
 // 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 {
+		return nil, err
+	}
+
 	distro := api.getDistro(distroName)
 	if distro == nil {
 		return nil, fmt.Errorf("GetDistro - unknown distribution: %s", distroName)
@@ -2491,6 +2514,9 @@ func (api *API) composeTypesHandler(writer http.ResponseWriter, request *http.Re
 	}
 
 	for _, format := range arch.ListImageTypes() {
+		if err := api.checkImageTypeDenylist(format); err != nil {
+			continue
+		}
 		reply.Types = append(reply.Types, composeType{format, true})
 	}
 

internal/weldr/api_test.go

@@ -61,7 +61,42 @@ func createWeldrAPI(tempdir string, fixtureGenerator rpmmd_mock.FixtureGenerator
 		panic(err)
 	}
 
-	return NewTestAPI(rpm, arch, dr, rr, nil, fixture.Store, fixture.Workers, ""), fixture.Store
+	return NewTestAPI(rpm, arch, dr, rr, nil, fixture.Store, fixture.Workers, "", nil), fixture.Store
+}
+
+// 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) {
+	fixture := fixtureGenerator(tempdir)
+	rpm := rpmmd_mock.NewRPMMDMock(fixture)
+
+	rr := reporegistry.NewFromDistrosRepoConfigs(rpmmd.DistrosRepoConfigs{
+		test_distro.TestDistroName: {
+			test_distro.TestArch2Name: {
+				{Name: "test-id", BaseURL: "http://example.com/test/os/x86_64", CheckGPG: true},
+			},
+		},
+		test_distro.TestDistro2Name: {
+			test_distro.TestArch2Name: {
+				{Name: "test-id-2", BaseURL: "http://example.com/test-2/os/x86_64", CheckGPG: true},
+			},
+		},
+	})
+
+	distro1 := test_distro.New()
+	arch, err := distro1.GetArch(test_distro.TestArch2Name)
+	if err != nil {
+		panic(err)
+	}
+	distro2 := test_distro.New2()
+
+	dr, err := distroregistry.New(distro1, distro2)
+	if err != nil {
+		panic(err)
+	}
+
+	return NewTestAPI(rpm, arch, dr, rr, nil, fixture.Store, fixture.Workers, "", imageTypeDenylist), fixture.Store
 }
 
 func TestBasic(t *testing.T) {
@@ -736,6 +771,7 @@ func TestCompose(t *testing.T) {
 		{false, "POST", "/api/v1/compose", fmt.Sprintf(`{"blueprint_name": "test","compose_type":"%s","branch":"master","ostree":{"ref":"/bad/ref","parent":"","url":"http://ostree/"}}`, test_distro.TestImageTypeName), http.StatusBadRequest, `{"status":false,"errors":[{"id":"InvalidChars","msg":"Invalid ostree ref"}]}`, expectedComposeOSTreeURL, []string{"build_id"}},
 		{false, "POST", "/api/v1/compose", fmt.Sprintf(`{"blueprint_name": "test-distro-2","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName), http.StatusOK, `{"status": true}`, expectedComposeGoodDistro, []string{"build_id"}},
 		{false, "POST", "/api/v1/compose", fmt.Sprintf(`{"blueprint_name": "test-fedora-1","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName), http.StatusBadRequest, `{"status": false,"errors":[{"id":"DistroError", "msg":"Unknown distribution: fedora-1"}]}`, nil, []string{"build_id"}},
+		{false, "POST", "/api/v1/compose", `{"blueprint_name": "test-distro-2","compose_type": "imaginary_type","branch": "master"}`, http.StatusBadRequest, `{"status": false,"errors":[{"id":"UnknownComposeType", "msg":"Unknown compose type for architecture: imaginary_type"}]}`, nil, []string{"build_id"}},
 	}
 
 	tempdir, err := ioutil.TempDir("", "weldr-tests-")
@@ -1444,3 +1480,196 @@ func TestModulesList(t *testing.T) {
 		test.TestRoute(t, api, true, "GET", c.Path, ``, c.ExpectedStatus, c.ExpectedJSON)
 	}
 }
+
+func TestComposeTypes_ImageTypeDenylist(t *testing.T) {
+	var cases = []struct {
+		Path              string
+		ImageTypeDenylist []string
+		ExpectedStatus    int
+		ExpectedJSON      string
+	}{
+		{
+			"/api/v1/compose/types",
+			[]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{},
+			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},
+			http.StatusOK,
+			fmt.Sprintf(`{"types": [{"enabled":true, "name":%q}]}`, test_distro.TestImageType2Name),
+		},
+		{
+			"/api/v1/compose/types?distro=test-distro-2",
+			[]string{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},
+			http.StatusOK,
+			`{"types": null}`,
+		},
+		{
+			"/api/v1/compose/types?distro=test-distro-2",
+			[]string{test_distro.TestImageTypeName, test_distro.TestImageType2Name},
+			http.StatusOK,
+			`{"types": null}`,
+		},
+	}
+
+	tempdir, err := ioutil.TempDir("", "weldr-tests-")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempdir)
+
+	for _, c := range cases {
+		api, _ := createWeldrAPI2(tempdir, rpmmd_mock.BaseFixture, c.ImageTypeDenylist)
+		test.TestRoute(t, api, true, "GET", c.Path, ``, c.ExpectedStatus, c.ExpectedJSON)
+	}
+}
+
+func TestComposePOST_ImageTypeDenylist(t *testing.T) {
+	arch, err := test_distro.New2().GetArch(test_distro.TestArch2Name)
+	require.NoError(t, err)
+	imgType, err := arch.GetImageType(test_distro.TestImageTypeName)
+	require.NoError(t, err)
+	imgType2, err := arch.GetImageType(test_distro.TestImageType2Name)
+	require.NoError(t, err)
+	manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, 0)
+	require.NoError(t, err)
+
+	expectedComposeLocal := &store.Compose{
+		Blueprint: &blueprint.Blueprint{
+			Name:           "test",
+			Version:        "0.0.0",
+			Packages:       []blueprint.Package{},
+			Modules:        []blueprint.Package{},
+			Groups:         []blueprint.Group{},
+			Customizations: nil,
+		},
+		ImageBuild: store.ImageBuild{
+			QueueStatus: common.IBWaiting,
+			ImageType:   imgType,
+			Manifest:    manifest,
+		},
+		Packages: []rpmmd.PackageSpec{},
+	}
+
+	expectedComposeLocal2 := &store.Compose{
+		Blueprint: &blueprint.Blueprint{
+			Name:           "test",
+			Version:        "0.0.0",
+			Packages:       []blueprint.Package{},
+			Modules:        []blueprint.Package{},
+			Groups:         []blueprint.Group{},
+			Customizations: nil,
+		},
+		ImageBuild: store.ImageBuild{
+			QueueStatus: common.IBWaiting,
+			ImageType:   imgType2,
+			Manifest:    manifest,
+		},
+		Packages: []rpmmd.PackageSpec{},
+	}
+
+	var cases = []struct {
+		Path              string
+		Body              string
+		imageTypeDenylist []string
+		ExpectedStatus    int
+		ExpectedJSON      string
+		ExpectedCompose   *store.Compose
+		IgnoreFields      []string
+	}{
+		{
+			"/api/v1/compose",
+			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
+			[]string{},
+			http.StatusOK,
+			`{"status":true}`,
+			expectedComposeLocal,
+			[]string{"build_id"},
+		},
+		{
+			"/api/v1/compose",
+			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageType2Name),
+			[]string{},
+			http.StatusOK,
+			`{"status": true}`,
+			expectedComposeLocal2,
+			[]string{"build_id"},
+		},
+		{
+			"/api/v1/compose",
+			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
+			[]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.TestImageType2Name),
+			[]string{test_distro.TestImageTypeName},
+			http.StatusOK,
+			`{"status": true}`,
+			expectedComposeLocal2,
+			[]string{"build_id"},
+		},
+		{
+			"/api/v1/compose",
+			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageTypeName),
+			[]string{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,
+			[]string{"build_id"},
+		},
+		{
+			"/api/v1/compose",
+			fmt.Sprintf(`{"blueprint_name": "test","compose_type": "%s","branch": "master"}`, test_distro.TestImageType2Name),
+			[]string{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"},
+		},
+	}
+
+	tempdir, err := ioutil.TempDir("", "weldr-tests-")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempdir)
+
+	for _, c := range cases {
+		api, s := createWeldrAPI2(tempdir, rpmmd_mock.NoComposesFixture, c.imageTypeDenylist)
+		test.TestRoute(t, api, true, "POST", c.Path, c.Body, c.ExpectedStatus, c.ExpectedJSON, c.IgnoreFields...)
+
+		if c.ExpectedStatus != http.StatusOK {
+			continue
+		}
+
+		composes := s.GetAllComposes()
+		require.Equalf(t, 1, len(composes), "%s: bad compose count in store", c.Path)
+
+		var composeStruct store.Compose
+		for _, c := range composes {
+			composeStruct = c
+			break
+		}
+
+		require.NotNilf(t, composeStruct.ImageBuild.Manifest, "%s: the compose in the store did not contain a blueprint", c.Path)
+
+		if diff := cmp.Diff(composeStruct, *c.ExpectedCompose, test.IgnoreDates(), test.IgnoreUuids(), test.Ignore("Targets.Options.Location"), test.CompareImageTypes()); diff != "" {
+			t.Errorf("%s: compose in store isn't the same as expected, diff:\n%s", c.Path, diff)
+		}
+	}
+}