store/ImageBuild: make Manifest a required property

Any valid ImageBuild must contain a Manifest, so don't allow this to be
nil, simplifying the code a bit in the process.

Signed-off-by: Tom Gundersen <teg@jklm.no>
This commit is contained in:
Tom Gundersen 2020-05-31 17:57:30 +02:00
parent d606c5195b
commit 4aced4e749
6 changed files with 50 additions and 70 deletions

View file

@ -23,7 +23,7 @@ func (ste *StateTransitionError) Error() string {
type ImageBuild struct {
ID int
ImageType distro.ImageType
Manifest *osbuild.Manifest
Manifest osbuild.Manifest
Targets []*target.Target
JobCreated time.Time
JobStarted time.Time
@ -38,11 +38,6 @@ type ImageBuild struct {
// DeepCopy creates a copy of the ImageBuild structure
func (ib *ImageBuild) DeepCopy() ImageBuild {
var newManifestPtr *osbuild.Manifest = nil
if ib.Manifest != nil {
manifestCopy := *ib.Manifest
newManifestPtr = &manifestCopy
}
var newTargets []*target.Target
for _, t := range ib.Targets {
newTarget := *t
@ -53,7 +48,7 @@ func (ib *ImageBuild) DeepCopy() ImageBuild {
ID: ib.ID,
QueueStatus: ib.QueueStatus,
ImageType: ib.ImageType,
Manifest: newManifestPtr,
Manifest: ib.Manifest,
Targets: newTargets,
JobCreated: ib.JobCreated,
JobStarted: ib.JobStarted,

View file

@ -6,8 +6,8 @@ import (
"github.com/google/uuid"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/target"
)
@ -57,49 +57,53 @@ func FixtureBase() *Store {
if err != nil {
panic("invalid image type qcow2 for x86_64 @ fedoratest")
}
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil)
if err != nil {
panic("could not create manifest")
}
s := New(nil, arch, nil)
s.blueprints[bName] = b
s.composes = map[uuid.UUID]Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000000"): Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000000"): {
Blueprint: &b,
ImageBuild: ImageBuild{
QueueStatus: common.IBWaiting,
ImageType: imgType,
Manifest: &osbuild.Manifest{},
Manifest: *manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
},
},
uuid.MustParse("30000000-0000-0000-0000-000000000001"): Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000001"): {
Blueprint: &b,
ImageBuild: ImageBuild{
QueueStatus: common.IBRunning,
ImageType: imgType,
Manifest: &osbuild.Manifest{},
Manifest: *manifest,
Targets: []*target.Target{localTarget},
JobCreated: date,
JobStarted: date,
},
},
uuid.MustParse("30000000-0000-0000-0000-000000000002"): Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000002"): {
Blueprint: &b,
ImageBuild: ImageBuild{
QueueStatus: common.IBFinished,
ImageType: imgType,
Manifest: &osbuild.Manifest{},
Manifest: *manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
JobStarted: date,
JobFinished: date,
},
},
uuid.MustParse("30000000-0000-0000-0000-000000000003"): Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000003"): {
Blueprint: &b,
ImageBuild: ImageBuild{
QueueStatus: common.IBFailed,
ImageType: imgType,
Manifest: &osbuild.Manifest{},
Manifest: *manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
JobStarted: date,
@ -156,34 +160,41 @@ func FixtureFinished() *Store {
if err != nil {
panic("invalid image type qcow2 for x86_64 @ fedoratest")
}
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil)
if err != nil {
panic("could not create manifest")
}
s := New(nil, arch, nil)
s.blueprints[bName] = b
s.composes = map[uuid.UUID]Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000000"): Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000000"): {
Blueprint: &b,
ImageBuild: ImageBuild{
QueueStatus: common.IBFinished,
ImageType: imgType,
Manifest: *manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
},
},
uuid.MustParse("30000000-0000-0000-0000-000000000001"): Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000001"): {
Blueprint: &b,
ImageBuild: ImageBuild{
QueueStatus: common.IBFinished,
ImageType: imgType,
Manifest: *manifest,
Targets: []*target.Target{localTarget},
JobCreated: date,
JobStarted: date,
},
},
uuid.MustParse("30000000-0000-0000-0000-000000000003"): Compose{
uuid.MustParse("30000000-0000-0000-0000-000000000003"): {
Blueprint: &b,
ImageBuild: ImageBuild{
QueueStatus: common.IBFailed,
ImageType: imgType,
Manifest: *manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
JobStarted: date,

View file

@ -41,7 +41,7 @@ type composesV0 map[uuid.UUID]composeV0
type imageBuildV0 struct {
ID int `json:"id"`
ImageType string `json:"image_type"`
Manifest *json.RawMessage `json:"manifest"`
Manifest json.RawMessage `json:"manifest"`
Targets []*target.Target `json:"targets"`
JobCreated time.Time `json:"job_created"`
JobStarted time.Time `json:"job_started"`
@ -118,15 +118,11 @@ func newImageBuildFromV0(imageBuildStruct imageBuildV0, arch distro.Arch) (Image
// on upgrades.
return ImageBuild{}, errors.New("invalid Image Type string")
}
var manifestPtr *osbuild.Manifest
if imageBuildStruct.Manifest != nil {
var manifest osbuild.Manifest
err := json.Unmarshal(*imageBuildStruct.Manifest, &manifest)
if err != nil {
// The JSON object is not a valid manifest, this may happen on upgrades.
return ImageBuild{}, errors.New("invalid manifest")
}
manifestPtr = &manifest
var manifest osbuild.Manifest
err := json.Unmarshal(imageBuildStruct.Manifest, &manifest)
if err != nil {
// The JSON object is not a valid manifest, this may happen on upgrades.
return ImageBuild{}, errors.New("invalid manifest")
}
// Backwards compatibility: fail all builds that are queued or
// running. Jobs status is now handled outside of the store
@ -140,7 +136,7 @@ func newImageBuildFromV0(imageBuildStruct imageBuildV0, arch distro.Arch) (Image
return ImageBuild{
ID: imageBuildStruct.ID,
ImageType: imgType,
Manifest: manifestPtr,
Manifest: manifest,
Targets: imageBuildStruct.Targets,
JobCreated: imageBuildStruct.JobCreated,
JobStarted: imageBuildStruct.JobStarted,
@ -266,22 +262,18 @@ func newWorkspaceV0(workspace map[string]blueprint.Blueprint) workspaceV0 {
func newComposeV0(compose Compose) composeV0 {
bp := compose.Blueprint.DeepCopy()
var rawManifestPtr *json.RawMessage
if compose.ImageBuild.Manifest != nil {
manifest, err := json.Marshal(compose.ImageBuild.Manifest)
if err != nil {
panic(err)
}
rawManifest := json.RawMessage(manifest)
rawManifestPtr = &rawManifest
manifest, err := json.Marshal(compose.ImageBuild.Manifest)
if err != nil {
panic(err)
}
rawManifest := json.RawMessage(manifest)
return composeV0{
Blueprint: &bp,
ImageBuilds: []imageBuildV0{
{
ID: compose.ImageBuild.ID,
ImageType: imageTypeToCompatString(compose.ImageBuild.ImageType),
Manifest: rawManifestPtr,
Manifest: rawManifest,
Targets: compose.ImageBuild.Targets,
JobCreated: compose.ImageBuild.JobCreated,
JobStarted: compose.ImageBuild.JobStarted,

View file

@ -332,7 +332,7 @@ func (s *Store) GetAllComposes() map[uuid.UUID]Compose {
return composes
}
func (s *Store) PushCompose(composeID uuid.UUID, manifest *osbuild.Manifest, imageType distro.ImageType, bp *blueprint.Blueprint, size uint64, targets []*target.Target, jobId uuid.UUID) error {
func (s *Store) PushCompose(composeID uuid.UUID, manifest osbuild.Manifest, imageType distro.ImageType, bp *blueprint.Blueprint, size uint64, targets []*target.Target, jobId uuid.UUID) error {
if _, exists := s.GetCompose(composeID); exists {
panic("a compose with this id already exists")
}
@ -362,7 +362,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, manifest *osbuild.Manifest, ima
// PushTestCompose is used for testing
// Set testSuccess to create a fake successful compose, otherwise it will create a failed compose
// It does not actually run a compose job
func (s *Store) PushTestCompose(composeID uuid.UUID, manifest *osbuild.Manifest, imageType distro.ImageType, bp *blueprint.Blueprint, size uint64, targets []*target.Target, testSuccess bool) error {
func (s *Store) PushTestCompose(composeID uuid.UUID, manifest osbuild.Manifest, imageType distro.ImageType, bp *blueprint.Blueprint, size uint64, targets []*target.Target, testSuccess bool) error {
if targets == nil {
targets = []*target.Target{}
}

View file

@ -1670,16 +1670,16 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request
testMode := q.Get("test")
if testMode == "1" {
// Create a failed compose
err = api.store.PushTestCompose(composeID, manifest, imageType, bp, size, targets, false)
err = api.store.PushTestCompose(composeID, *manifest, imageType, bp, size, targets, false)
} else if testMode == "2" {
// Create a successful compose
err = api.store.PushTestCompose(composeID, manifest, imageType, bp, size, targets, true)
err = api.store.PushTestCompose(composeID, *manifest, imageType, bp, size, targets, true)
} else {
var jobId uuid.UUID
jobId, err = api.workers.Enqueue(manifest, targets)
if err == nil {
err = api.store.PushCompose(composeID, manifest, imageType, bp, size, targets, jobId)
err = api.store.PushCompose(composeID, *manifest, imageType, bp, size, targets, jobId)
}
}
@ -2068,16 +2068,7 @@ func (api *API) composeMetadataHandler(writer http.ResponseWriter, request *http
return
}
// Return the Manifest, if it exists
if compose.ImageBuild.Manifest == nil {
errors := responseError{
ID: "EmptyManifest",
Msg: fmt.Sprintf("Manifest unexpectedly empty."),
}
statusResponseError(writer, http.StatusBadRequest, errors)
return
}
metadata, err := json.Marshal(compose.ImageBuild.Manifest)
metadata, err := json.Marshal(&compose.ImageBuild.Manifest)
common.PanicOnError(err)
writer.Header().Set("Content-Disposition", "attachment; filename="+uuid.String()+"-metadata.tar")
@ -2138,16 +2129,7 @@ func (api *API) composeResultsHandler(writer http.ResponseWriter, request *http.
return
}
// Return the Manifest, if it exists
if compose.ImageBuild.Manifest == nil {
errors := responseError{
ID: "EmptyManifest",
Msg: fmt.Sprintf("Manifest unexpectedly empty."),
}
statusResponseError(writer, http.StatusBadRequest, errors)
return
}
metadata, err := json.Marshal(compose.ImageBuild.Manifest)
metadata, err := json.Marshal(&compose.ImageBuild.Manifest)
common.PanicOnError(err)
writer.Header().Set("Content-Disposition", "attachment; filename="+uuid.String()+".tar")

View file

@ -13,6 +13,7 @@ import (
"time"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/target"
"github.com/osbuild/osbuild-composer/internal/blueprint"
@ -533,12 +534,11 @@ func TestCompose(t *testing.T) {
require.NotNilf(t, composeStruct.ImageBuild.Manifest, "%s: the compose in the store did not contain a blueprint", c.Path)
// TODO: find some (reasonable) way to verify the contents of the pipeline
composeStruct.ImageBuild.Manifest = nil
composeStruct.ImageBuild.Manifest = osbuild.Manifest{}
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)
}
}
}
@ -639,10 +639,10 @@ func TestComposeLogs(t *testing.T) {
}{
{"/api/v0/compose/logs/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-logs.tar", "application/x-tar", "logs/osbuild.log", "The compose result is empty.\n"},
{"/api/v1/compose/logs/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-logs.tar", "application/x-tar", "logs/osbuild.log", "The compose result is empty.\n"},
{"/api/v0/compose/metadata/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-metadata.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":null,\"pipeline\":{}}"},
{"/api/v1/compose/metadata/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-metadata.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":null,\"pipeline\":{}}"},
{"/api/v0/compose/results/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":null,\"pipeline\":{}}"},
{"/api/v1/compose/results/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":null,\"pipeline\":{}}"},
{"/api/v0/compose/metadata/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-metadata.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":{},\"pipeline\":{}}"},
{"/api/v1/compose/metadata/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002-metadata.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":{},\"pipeline\":{}}"},
{"/api/v0/compose/results/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":{},\"pipeline\":{}}"},
{"/api/v1/compose/results/30000000-0000-0000-0000-000000000002", "attachment; filename=30000000-0000-0000-0000-000000000002.tar", "application/x-tar", "30000000-0000-0000-0000-000000000002.json", "{\"sources\":{},\"pipeline\":{}}"},
}
for _, c := range successCases {