particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

distro: make the osbuild package internal to the distros

Browse source 
Rather than Manifest() returning an osbuild.Manifest object, introduce a
new distro.Manifest object which represents it as an opaque, JSON
serializable object. This new type has the following properties:

1) its serialization is compatible with the input to osbuild,
2) any valid osbuild input can be deserialized into it, and
3) marshalling and unmarshaling to and from JSON is lossless.

This means that even as we change the subset of valid osbulid manifests
that we support, we can still load any previous state from disk, and it
will continue to work just as before, even though we can no longer
deserialize it into our internal notion of osbuild.Manifest.

This fixes the underlying problem of which #685 was a symptom.

Signed-off-by: Tom Gundersen <teg@jklm.no>
This commit is contained in:
Tom Gundersen 2020-05-31 20:11:06 +02:00
parent 4aced4e749
commit 0417c6d8bb
18 changed files with 101 additions and 86 deletions
Download patch file Download diff file Expand all files Collapse all files

4
cmd/osbuild-worker/osbuild.go
View file

@ -7,7 +7,7 @@ import (
"os/exec"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/distro"
)
type OSBuildError struct {
@ -19,7 +19,7 @@ func (e *OSBuildError) Error() string {
return e.Message
}
func RunOSBuild(manifest *osbuild.Manifest, store, outputDirectory string, errorWriter io.Writer) (*common.ComposeResult, error) {
func RunOSBuild(manifest distro.Manifest, store, outputDirectory string, errorWriter io.Writer) (*common.ComposeResult, error) {
cmd := exec.Command(
"osbuild",
"--store", store,

22
internal/distro/distro.go
View file

@ -2,6 +2,7 @@ package distro
import (
"bufio"
"encoding/json"
"errors"
"fmt"
"io"
@ -9,8 +10,6 @@ import (
"sort"
"strings"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
)
@ -79,7 +78,7 @@ type ImageType interface {
// Returns an osbuild manifest, containing the sources and pipeline necessary
// to build an image, given output format with all packages and customizations
// specified in the given blueprint.
Manifest(b *blueprint.Customizations, options ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec) (*osbuild.Manifest, error)
Manifest(b *blueprint.Customizations, options ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec) (Manifest, error)
}
// The ImageOptions specify options for a specific image build
@ -94,6 +93,23 @@ type OSTreeImageOptions struct {
Parent string
}
// A Manifest is an opaque JSON object, which is a valid input to osbuild
type Manifest []byte
func (m Manifest) MarshalJSON() ([]byte, error) {
return json.RawMessage(m).MarshalJSON()
}
func (m *Manifest) UnmarshalJSON(payload []byte) error {
var raw json.RawMessage
err := (&raw).UnmarshalJSON(payload)
if err != nil {
return err
}
*m = Manifest(raw)
return nil
}
type Registry struct {
distros map[string]Distro
}

11
internal/distro/distro_test_common/distro_test_common.go
View file

@ -7,10 +7,8 @@ import (
"path/filepath"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -40,9 +38,9 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, distr
Packages []rpmmd.PackageSpec `json:"packages"`
}
var tt struct {
ComposeRequest *composeRequest `json:"compose-request"`
RpmMD *rpmMD `json:"rpmmd"`
Manifest *osbuild.Manifest `json:"manifest,omitempty"`
ComposeRequest *composeRequest `json:"compose-request"`
RpmMD *rpmMD `json:"rpmmd"`
Manifest distro.Manifest `json:"manifest,omitempty"`
}
file, err := ioutil.ReadFile(fileName)
assert.NoErrorf(err, "Could not read test-case '%s': %v", fileName, err)
@ -94,8 +92,7 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, distr
return
}
if tt.Manifest != nil {
diff := cmp.Diff(got, tt.Manifest)
assert.Empty(diff, "d.Manifest() different from expected: %v", diff)
require.JSONEq(t, string(tt.Manifest), string(got))
}
})
}

15
internal/distro/fedora31/distro.go
View file

@ -1,6 +1,7 @@
package fedora31
import (
"encoding/json"
"errors"
"sort"
"strconv"
@ -170,16 +171,18 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (*osbuild.Manifest, error) {
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
pipeline, err := t.pipeline(c, repos, packageSpecs, buildPackageSpecs, options.Size)
if err != nil {
return nil, err
return distro.Manifest{}, err
}
return &osbuild.Manifest{
Sources: *sources(append(packageSpecs, buildPackageSpecs...)),
Pipeline: *pipeline,
}, nil
return json.Marshal(
osbuild.Manifest{
Sources: *sources(append(packageSpecs, buildPackageSpecs...)),
Pipeline: *pipeline,
},
)
}
func New() *Fedora31 {

15
internal/distro/fedora32/distro.go
View file

@ -1,6 +1,7 @@
package fedora32
import (
"encoding/json"
"errors"
"fmt"
"sort"
@ -178,16 +179,18 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (*osbuild.Manifest, error) {
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
pipeline, err := t.pipeline(c, options, repos, packageSpecs, buildPackageSpecs)
if err != nil {
return nil, err
return distro.Manifest{}, err
}
return &osbuild.Manifest{
Sources: *sources(append(packageSpecs, buildPackageSpecs...)),
Pipeline: *pipeline,
}, nil
return json.Marshal(
osbuild.Manifest{
Sources: *sources(append(packageSpecs, buildPackageSpecs...)),
Pipeline: *pipeline,
},
)
}
func (d *distribution) Name() string {

14
internal/distro/fedoratest/distro.go
View file

@ -1,6 +1,7 @@
package fedoratest
import (
"encoding/json"
"errors"
"github.com/osbuild/osbuild-composer/internal/blueprint"
@ -94,11 +95,14 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (*osbuild.Manifest, error) {
return &osbuild.Manifest{
Pipeline: osbuild.Pipeline{},
Sources: osbuild.Sources{},
}, nil
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
return json.Marshal(
osbuild.Manifest{
Sources: osbuild.Sources{},
Pipeline: osbuild.Pipeline{},
},
)
}
func New() *FedoraTestDistro {

15
internal/distro/rhel8/distro.go
View file

@ -1,6 +1,7 @@
package rhel8
import (
"encoding/json"
"errors"
"sort"
"strconv"
@ -179,16 +180,18 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (*osbuild.Manifest, error) {
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
pipeline, err := t.pipeline(c, options, repos, packageSpecs, buildPackageSpecs)
if err != nil {
return nil, err
return distro.Manifest{}, err
}
return &osbuild.Manifest{
Sources: *sources(append(packageSpecs, buildPackageSpecs...)),
Pipeline: *pipeline,
}, nil
return json.Marshal(
osbuild.Manifest{
Sources: *sources(append(packageSpecs, buildPackageSpecs...)),
Pipeline: *pipeline,
},
)
}
func (d *distribution) Name() string {

13
internal/distro/test_distro/distro.go
View file

@ -1,6 +1,7 @@
package test_distro
import (
"encoding/json"
"errors"
"github.com/osbuild/osbuild-composer/internal/blueprint"
@ -74,11 +75,13 @@ func (t *TestImageType) BuildPackages() []string {
return nil
}
func (t *TestImageType) Manifest(b *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec) (*osbuild.Manifest, error) {
return &osbuild.Manifest{
Sources: osbuild.Sources{},
Pipeline: osbuild.Pipeline{},
}, nil
func (t *TestImageType) Manifest(b *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
return json.Marshal(
osbuild.Manifest{
Sources: osbuild.Sources{},
Pipeline: osbuild.Pipeline{},
},
)
}
func New() *TestDistro {

3
internal/store/compose.go
View file

@ -7,7 +7,6 @@ import (
"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/osbuild"
"github.com/osbuild/osbuild-composer/internal/target"
)
@ -23,7 +22,7 @@ func (ste *StateTransitionError) Error() string {
type ImageBuild struct {
ID int
ImageType distro.ImageType
Manifest osbuild.Manifest
Manifest distro.Manifest
Targets []*target.Target
JobCreated time.Time
JobStarted time.Time

14
internal/store/fixtures.go
View file

@ -70,7 +70,7 @@ func FixtureBase() *Store {
ImageBuild: ImageBuild{
QueueStatus: common.IBWaiting,
ImageType: imgType,
Manifest: *manifest,
Manifest: manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
},
@ -80,7 +80,7 @@ func FixtureBase() *Store {
ImageBuild: ImageBuild{
QueueStatus: common.IBRunning,
ImageType: imgType,
Manifest: *manifest,
Manifest: manifest,
Targets: []*target.Target{localTarget},
JobCreated: date,
JobStarted: date,
@ -91,7 +91,7 @@ func FixtureBase() *Store {
ImageBuild: ImageBuild{
QueueStatus: common.IBFinished,
ImageType: imgType,
Manifest: *manifest,
Manifest: manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
JobStarted: date,
@ -103,7 +103,7 @@ func FixtureBase() *Store {
ImageBuild: ImageBuild{
QueueStatus: common.IBFailed,
ImageType: imgType,
Manifest: *manifest,
Manifest: manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
JobStarted: date,
@ -173,7 +173,7 @@ func FixtureFinished() *Store {
ImageBuild: ImageBuild{
QueueStatus: common.IBFinished,
ImageType: imgType,
Manifest: *manifest,
Manifest: manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
},
@ -183,7 +183,7 @@ func FixtureFinished() *Store {
ImageBuild: ImageBuild{
QueueStatus: common.IBFinished,
ImageType: imgType,
Manifest: *manifest,
Manifest: manifest,
Targets: []*target.Target{localTarget},
JobCreated: date,
JobStarted: date,
@ -194,7 +194,7 @@ func FixtureFinished() *Store {
ImageBuild: ImageBuild{
QueueStatus: common.IBFailed,
ImageType: imgType,
Manifest: *manifest,
Manifest: manifest,
Targets: []*target.Target{localTarget, awsTarget},
JobCreated: date,
JobStarted: date,

19
internal/store/json.go
View file

@ -1,7 +1,6 @@
package store
import (
"encoding/json"
"errors"
"log"
"sort"
@ -11,7 +10,6 @@ import (
"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/osbuild"
"github.com/osbuild/osbuild-composer/internal/target"
)
@ -41,7 +39,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 distro.Manifest `json:"manifest"`
Targets []*target.Target `json:"targets"`
JobCreated time.Time `json:"job_created"`
JobStarted time.Time `json:"job_started"`
@ -118,12 +116,6 @@ func newImageBuildFromV0(imageBuildStruct imageBuildV0, arch distro.Arch) (Image
// on upgrades.
return ImageBuild{}, errors.New("invalid Image Type string")
}
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
// (and the compose). The fields are kept so that previously
@ -136,7 +128,7 @@ func newImageBuildFromV0(imageBuildStruct imageBuildV0, arch distro.Arch) (Image
return ImageBuild{
ID: imageBuildStruct.ID,
ImageType: imgType,
Manifest: manifest,
Manifest: imageBuildStruct.Manifest,
Targets: imageBuildStruct.Targets,
JobCreated: imageBuildStruct.JobCreated,
JobStarted: imageBuildStruct.JobStarted,
@ -262,18 +254,13 @@ func newWorkspaceV0(workspace map[string]blueprint.Blueprint) workspaceV0 {
func newComposeV0(compose Compose) composeV0 {
bp := compose.Blueprint.DeepCopy()
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: rawManifest,
Manifest: compose.ImageBuild.Manifest,
Targets: compose.ImageBuild.Targets,
JobCreated: compose.ImageBuild.JobCreated,
JobStarted: compose.ImageBuild.JobStarted,

5
internal/store/store.go
View file

@ -15,7 +15,6 @@ import (
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/jsondb"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/common"
@ -332,7 +331,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 distro.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 +361,7 @@ func (s *Store) PushCompose(composeID uuid.UUID, manifest osbuild.Manifest, imag
// 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 distro.Manifest, imageType distro.ImageType, bp *blueprint.Blueprint, size uint64, targets []*target.Target, testSuccess bool) error {
if targets == nil {
targets = []*target.Target{}
}

6
internal/weldr/api.go
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)
}
}

8
internal/weldr/api_test.go
View file

@ -13,7 +13,7 @@ import (
"time"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/target"
"github.com/osbuild/osbuild-composer/internal/blueprint"
@ -438,6 +438,8 @@ func TestCompose(t *testing.T) {
require.NoError(t, err)
imgType, err := arch.GetImageType("qcow2")
require.NoError(t, err)
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil)
require.NoError(t, err)
expectedComposeLocal := &store.Compose{
Blueprint: &blueprint.Blueprint{
Name: "test",
@ -450,6 +452,7 @@ func TestCompose(t *testing.T) {
ImageBuild: store.ImageBuild{
QueueStatus: common.IBWaiting,
ImageType: imgType,
Manifest: manifest,
Targets: []*target.Target{
{
// skip Uuid and Created fields - they are ignored
@ -473,6 +476,7 @@ func TestCompose(t *testing.T) {
ImageBuild: store.ImageBuild{
QueueStatus: common.IBWaiting,
ImageType: imgType,
Manifest: manifest,
Targets: []*target.Target{
{
Name: "org.osbuild.aws",
@ -533,8 +537,6 @@ 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 = 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)

4
internal/worker/client.go
View file

@ -14,7 +14,7 @@ import (
"github.com/google/uuid"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/target"
)
@ -26,7 +26,7 @@ type Client struct {
type Job struct {
Id uuid.UUID
Manifest *osbuild.Manifest
Manifest distro.Manifest
Targets []*target.Target
}

12
internal/worker/json.go
View file

@ -4,7 +4,7 @@ import (
"github.com/google/uuid"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/target"
)
@ -13,8 +13,8 @@ import (
//
type OSBuildJob struct {
Manifest *osbuild.Manifest `json:"manifest"`
Targets []*target.Target `json:"targets,omitempty"`
Manifest distro.Manifest `json:"manifest"`
Targets []*target.Target `json:"targets,omitempty"`
}
type OSBuildJobResult struct {
@ -37,9 +37,9 @@ type addJobRequest struct {
}
type addJobResponse struct {
Id uuid.UUID `json:"id"`
Manifest *osbuild.Manifest `json:"manifest"`
Targets []*target.Target `json:"targets,omitempty"`
Id uuid.UUID `json:"id"`
Manifest distro.Manifest `json:"manifest"`
Targets []*target.Target `json:"targets,omitempty"`
}
type updateJobRequest struct {

4
internal/worker/server.go
View file

@ -16,8 +16,8 @@ import (
"github.com/julienschmidt/httprouter"
"github.com/osbuild/osbuild-composer/internal/common"
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/jobqueue"
"github.com/osbuild/osbuild-composer/internal/osbuild"
"github.com/osbuild/osbuild-composer/internal/target"
)
@ -80,7 +80,7 @@ func (s *Server) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
s.router.ServeHTTP(writer, request)
}
func (s *Server) Enqueue(manifest *osbuild.Manifest, targets []*target.Target) (uuid.UUID, error) {
func (s *Server) Enqueue(manifest distro.Manifest, targets []*target.Target) (uuid.UUID, error) {
job := OSBuildJob{
Manifest: manifest,
Targets: targets,

3
internal/worker/server_test.go
View file

@ -57,12 +57,11 @@ func TestCreate(t *testing.T) {
if err != nil {
t.Fatalf("error getting image type from arch")
}
server := worker.NewServer(nil, testjobqueue.New(), "")
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil)
if err != nil {
t.Fatalf("error creating osbuild manifest")
}
server := worker.NewServer(nil, testjobqueue.New(), "")
id, err := server.Enqueue(manifest, nil)
require.NoError(t, err)