rcm: introduce rpmmd member of the api structure

This is needed for unit tests, because it wasn't possible to mock the
rpmmd module before. This also requires that the checksum is moved to
the compose request and evaluated in the endpoint handler instead of
push compose. I think it makes sense to have the checksum in the compose
request directly.

Also a "module platform ID" is required now, but we don't have the
"global" distribution any more, so this patch introduces mapping from a
distribution to the module platform ID.
This commit is contained in:
Martin Sehnoutka 2020-02-19 11:50:38 +01:00 committed by Tom Gundersen
parent d1c766abe7
commit 923a0b0b97
5 changed files with 72 additions and 20 deletions

View file

@ -18,6 +18,16 @@ func (err *CustomJsonConversionError) Error() string {
return err.reason
}
// CustomTypeError is thrown when parsing strings into enumerations
type CustomTypeError struct {
reason string
}
// Error returns the error as a string
func (err *CustomTypeError) Error() string {
return err.reason
}
// Since Go has no generics, this is the only way to write common code for all the types present in this package.
// It uses weakly typed maps to convert between strings and integers which are then wrapped into type aliases.
// Specific implementations are bellow each type.
@ -243,6 +253,7 @@ const (
// getArchMapping is a helper function that defines the conversion from JSON string value
// to Distribution.
func getDistributionMapping() map[string]int {
// Don't forget to add module-platform-id mapping below
mapping := map[string]int{
"fedora-30": int(Fedora30),
"fedora-31": int(Fedora31),
@ -252,6 +263,22 @@ func getDistributionMapping() map[string]int {
return mapping
}
func (dist *Distribution) ModulePlatformID() (string, error) {
mapping := map[Distribution]string{
// TODO: this could be refactored so we don't have these strings hard coded in two places
Fedora30: "platform:f30",
Fedora31: "platform:f31",
Fedora32: "platform:f32",
RHEL82: "platform:el8",
}
id, exists := mapping[*dist]
if !exists {
return "", &CustomTypeError{reason: "Distribution does not have a module platform ID assigned"}
} else {
return id, nil
}
}
func ListDistributions() []string {
return listHelper(getDistributionMapping())
}
@ -325,5 +352,6 @@ type ComposeRequest struct {
Distro Distribution `json:"distro"`
Arch Architecture `json:"arch"`
Repositories []rpmmd.RepoConfig `json:"repositories"`
Checksums map[string]string `json:"checksums"`
RequestedImages []ImageRequest `json:"requested_images"`
}

View file

@ -23,14 +23,18 @@ type API struct {
logger *log.Logger
store *store.Store
router *httprouter.Router
// rpmMetadata is an interface to dnf-json and we include it here so that we can
// mock it in the unit tests
rpmMetadata rpmmd.RPMMD
}
// New creates new RCM API
func New(logger *log.Logger, store *store.Store) *API {
func New(logger *log.Logger, store *store.Store, rpmMetadata rpmmd.RPMMD) *API {
api := &API{
logger: logger,
store: store,
router: httprouter.New(),
logger: logger,
store: store,
router: httprouter.New(),
rpmMetadata: rpmMetadata,
}
api.router.RedirectTrailingSlash = false
@ -152,6 +156,25 @@ func (api *API) submit(writer http.ResponseWriter, request *http.Request, _ http
})
}
modulePlatformID, err := composeRequest.Distribution.ModulePlatformID()
if err != nil {
writer.WriteHeader(http.StatusBadRequest)
_, err := writer.Write([]byte(err.Error()))
if err != nil {
panic("Failed to write response")
}
}
// map( repo-id => checksum )
_, checksums, err := api.rpmMetadata.FetchMetadata(repoConfigs, modulePlatformID)
if err != nil {
writer.WriteHeader(http.StatusBadRequest)
_, err := writer.Write([]byte(err.Error()))
if err != nil {
panic("Failed to write response")
}
}
// Push the requested compose to the store
composeUUID := uuid.New()
// nil is used as an upload target, because LocalTarget is already used in the PushCompose function
@ -161,6 +184,7 @@ func (api *API) submit(writer http.ResponseWriter, request *http.Request, _ http
Distro: composeRequest.Distribution,
Arch: composeRequest.Architectures[0],
Repositories: repoConfigs,
Checksums: checksums,
RequestedImages: requestedImages,
})
if err != nil {

View file

@ -2,14 +2,16 @@ package rcm_test
import (
"bytes"
"encoding/json"
"github.com/google/uuid"
"net/http"
"net/http/httptest"
"regexp"
"testing"
"github.com/google/uuid"
"github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
distro_mock "github.com/osbuild/osbuild-composer/internal/mocks/distro"
rpmmd_mock "github.com/osbuild/osbuild-composer/internal/mocks/rpmmd"
"github.com/osbuild/osbuild-composer/internal/rcm"
"github.com/osbuild/osbuild-composer/internal/store"
)
@ -49,7 +51,7 @@ func TestBasicRcmAPI(t *testing.T) {
registry := distro_mock.NewRegistry()
distroStruct := fedoratest.New()
api := rcm.New(nil, store.New(nil, distroStruct, *registry))
api := rcm.New(nil, store.New(nil, distroStruct, *registry), rpmmd_mock.NewRPMMDMock(rpmmd_mock.BaseFixture()))
for _, c := range cases {
resp := internalRequest(api, c.Method, c.Path, c.Body, c.ContentType)
@ -71,9 +73,9 @@ func TestBasicRcmAPI(t *testing.T) {
func TestSubmitCompose(t *testing.T) {
// Test the most basic use case: Submit a new job and get its status.
registry := distro_mock.NewRegistry()
distroStruct := fedoratest.New()
api := rcm.New(nil, store.New(nil, distroStruct, *registry))
registry := distro_mock.NewRegistry()
api := rcm.New(nil, store.New(nil, distroStruct, *registry), rpmmd_mock.NewRPMMDMock(rpmmd_mock.BaseFixture()))
var submit_reply struct {
UUID uuid.UUID `json:"compose_id"`
@ -92,7 +94,12 @@ func TestSubmitCompose(t *testing.T) {
{
"POST",
"/v1/compose",
`{"distribution": "fedora-30", "image_types": ["qcow2"], "architectures":["x86_64"], "repositories": [{"url": "aaa"}]}`,
`{"distribution": "fedora-30",
"image_types": ["qcow2"],
"architectures":["x86_64"],
"repositories": [{
"url": "http://download.fedoraproject.org/pub/fedora/linux/releases/30/Everything/x86_64/os/"
}]}`,
"application/json",
},
}
@ -100,7 +107,6 @@ func TestSubmitCompose(t *testing.T) {
for n, c := range cases {
// Submit job
t.Logf("RCM API submit compose, case %d\n", n)
//resp := internalRequest(api, "POST", "/v1/compose", `{"distribution": "fedora-30", "image_types": ["test_output"], "architectures":["test_arch"], "repositories": [{"url": "aaa", "checksum": "bbb"}]}`, "application/json")
resp := internalRequest(api, c.Method, c.Path, c.Body, c.ContentType)
if resp.StatusCode != http.StatusOK {
t.Fatal("Failed to call /v1/compose, HTTP status code:", resp.StatusCode)

View file

@ -15,6 +15,7 @@ import (
"log"
"os"
"path/filepath"
"reflect"
"sort"
"strconv"
"strings"
@ -586,7 +587,7 @@ func (s *Store) PushComposeRequest(request common.ComposeRequest) error {
}
distroStruct := s.distroRegistry.GetDistro(distroString)
if distroStruct == nil {
if distroStruct == nil || (reflect.ValueOf(distroStruct).Kind() == reflect.Ptr && reflect.ValueOf(distroStruct).IsNil()) {
panic("fatal error, distro should exist but it is not in the registry")
}
@ -599,13 +600,6 @@ func (s *Store) PushComposeRequest(request common.ComposeRequest) error {
panic("Multiple image requests are not yet properly implemented")
}
// map( repo-id => checksum )
rpmMetadata := rpmmd.NewRPMMD()
_, checksums, err := rpmMetadata.FetchMetadata(request.Repositories, distroStruct.ModulePlatformID())
if err != nil {
return err
}
for imageBuildID, imageRequest := range request.RequestedImages {
// TODO: handle custom upload targets
// TODO: this requires changes in the Compose Request struct
@ -614,7 +608,7 @@ func (s *Store) PushComposeRequest(request common.ComposeRequest) error {
if !exists {
panic("fatal error, image type should exist but it does not")
}
pipelineStruct, err := distroStruct.Pipeline(&request.Blueprint, request.Repositories, nil, nil, checksums, arch, imgTypeCompatStr, 0)
pipelineStruct, err := distroStruct.Pipeline(&request.Blueprint, request.Repositories, nil, nil, request.Checksums, arch, imgTypeCompatStr, 0)
if err != nil {
return err
}