From b150d57c1823a21c9a2efcd66325c67f65b0c1fa Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Mon, 26 Jul 2021 12:55:20 +0200 Subject: [PATCH] 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 --- cmd/osbuild-composer/composer.go | 5 +- cmd/osbuild-composer/config.go | 23 +++++- cmd/osbuild-composer/config_test.go | 16 +++- cmd/osbuild-composer/main.go | 2 +- cmd/osbuild-composer/testdata/test.toml | 5 +- .../unreleased/weldr-image-type-denylist.md | 18 +++-- internal/weldr/api.go | 78 ++++++++++--------- internal/weldr/api_test.go | 75 ++++++++++++++---- 8 files changed, 160 insertions(+), 62 deletions(-) diff --git a/cmd/osbuild-composer/composer.go b/cmd/osbuild-composer/composer.go index 78b42b984..410332ce6 100644 --- a/cmd/osbuild-composer/composer.go +++ b/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) { - c.weldr, err = weldr.New(repoPaths, c.stateDir, c.rpm, c.distros, c.logger, c.workers, imageTypeDenylist) +func (c *Composer) InitWeldr(repoPaths []string, weldrListener net.Listener, + 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 } diff --git a/cmd/osbuild-composer/config.go b/cmd/osbuild-composer/config.go index 746f8a866..1cc737e1a 100644 --- a/cmd/osbuild-composer/config.go +++ b/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 { diff --git a/cmd/osbuild-composer/config_test.go b/cmd/osbuild-composer/config_test.go index 3fecf9da2..4e26a5aa8 100644 --- a/cmd/osbuild-composer/config_test.go +++ b/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()) +} diff --git a/cmd/osbuild-composer/main.go b/cmd/osbuild-composer/main.go index f7c34eaeb..03623b991 100644 --- a/cmd/osbuild-composer/main.go +++ b/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) } diff --git a/cmd/osbuild-composer/testdata/test.toml b/cmd/osbuild-composer/testdata/test.toml index 4974c8897..da6b3a977 100644 --- a/cmd/osbuild-composer/testdata/test.toml +++ b/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" ] diff --git a/docs/news/unreleased/weldr-image-type-denylist.md b/docs/news/unreleased/weldr-image-type-denylist.md index 63f4e9dfb..d56b9e079 100644 --- a/docs/news/unreleased/weldr-image-type-denylist.md +++ b/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 -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 +Extend Weldr API to accept a map of distribution-specific lists of denied +image types, which should not be exposed via API. A special name `*` can be +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" ] ``` diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 556dbd57d..f47c410d4 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -58,8 +58,8 @@ type API struct { distroRegistry *distroregistry.Registry // Available distros distros []string // Supported distro names - // List of ImageType names, which should not be exposed by the API - imageTypeDenylist []string + // List of ImageType names, which should not be exposed by the API + distrosImageTypeDenylist map[string][]string } type ComposeState int @@ -121,28 +121,28 @@ 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]) 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), - imageTypeDenylist: imageTypeDenylist, + 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), + 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) } @@ -174,17 +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), - imageTypeDenylist: imageTypeDenylist, + 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), + distrosImageTypeDenylist: distrosImageTypeDenylist, } return setupRouter(api), nil } @@ -385,13 +385,21 @@ 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) +// checkImageTypeDenylist checks the given ImageType and Distro names against +//the distro-specific 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(distroName, imageType string) error { + 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}) diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go index 1c1adfba0..fb500f456 100644 --- a/internal/weldr/api_test.go +++ b/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-")