From 076bbc5456f918b0cf47c3c7ce75b0118dc3817d Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Fri, 23 Jul 2021 14:53:32 +0200 Subject: [PATCH] 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 --- cmd/osbuild-composer/composer.go | 4 +- cmd/osbuild-composer/config.go | 3 + cmd/osbuild-composer/config_test.go | 2 + cmd/osbuild-composer/main.go | 2 +- cmd/osbuild-composer/testdata/test.toml | 3 + .../unreleased/weldr-image-type-denylist.md | 16 ++ internal/client/unit_test.go | 2 +- internal/weldr/api.go | 70 ++++-- internal/weldr/api_test.go | 231 +++++++++++++++++- 9 files changed, 306 insertions(+), 27 deletions(-) create mode 100644 docs/news/unreleased/weldr-image-type-denylist.md diff --git a/cmd/osbuild-composer/composer.go b/cmd/osbuild-composer/composer.go index f668e6eec..78b42b984 100644 --- a/cmd/osbuild-composer/composer.go +++ b/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 } diff --git a/cmd/osbuild-composer/config.go b/cmd/osbuild-composer/config.go index 07b781800..746f8a866 100644 --- a/cmd/osbuild-composer/config.go +++ b/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) { diff --git a/cmd/osbuild-composer/config_test.go b/cmd/osbuild-composer/config_test.go index 909fc52e4..3fecf9da2 100644 --- a/cmd/osbuild-composer/config_test.go +++ b/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")) diff --git a/cmd/osbuild-composer/main.go b/cmd/osbuild-composer/main.go index f10eaa821..f7c34eaeb 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]) + err = composer.InitWeldr(repositoryConfigs, l[0], config.WeldrAPI.ImageTypeDenylist) 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 5ba153dbf..4974c8897 100644 --- a/cmd/osbuild-composer/testdata/test.toml +++ b/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" ] diff --git a/docs/news/unreleased/weldr-image-type-denylist.md b/docs/news/unreleased/weldr-image-type-denylist.md new file mode 100644 index 000000000..63f4e9dfb --- /dev/null +++ b/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" ] +``` diff --git a/internal/client/unit_test.go b/internal/client/unit_test.go index ae596486e..a0ba48b2f 100644 --- a/internal/client/unit_test.go +++ b/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() diff --git a/internal/weldr/api.go b/internal/weldr/api.go index bbd044590..556dbd57d 100644 --- a/internal/weldr/api.go +++ b/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}) } diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go index 8bce68ce8..1c1adfba0 100644 --- a/internal/weldr/api_test.go +++ b/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) + } + } +}