From f446613d4a0b690c6c6f98fb30cbb87daffde963 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Mon, 7 Sep 2020 12:12:02 +0100 Subject: [PATCH] upload/koji: use CGInitBuild and clarify metadata structs Move to requiring CGInitBuild to be called before CGImport. In the future we could make the former optional again, but for now we want to allow the caller to have done CGInitBuild and for composer only to do the CGImport using the passed in build_id and token. Also rename and document some struct fields in the metadata struct to make them more specific to our use-case and hopefully easier to read. Signed-off-by: Tom Gundersen --- cmd/osbuild-koji/main.go | 26 ++++++--- cmd/osbuild-worker/main.go | 31 ++++++----- internal/kojiapi/server.go | 38 ++++++++++++- internal/target/koji_target.go | 3 + internal/upload/koji/koji.go | 93 ++++++++++++++++++++----------- internal/upload/koji/koji_test.go | 23 +++++--- 6 files changed, 149 insertions(+), 65 deletions(-) diff --git a/cmd/osbuild-koji/main.go b/cmd/osbuild-koji/main.go index 5a865a8c4..1fe8d44f6 100644 --- a/cmd/osbuild-koji/main.go +++ b/cmd/osbuild-koji/main.go @@ -57,7 +57,7 @@ func main() { return } - build := koji.Build{ + build := koji.ImageBuild{ Name: name, Version: version, Release: release, @@ -79,11 +79,11 @@ func main() { Type: "nspawn", Arch: arch, }, - Tools: []koji.Tool{}, - Components: []koji.Component{}, + Tools: []koji.Tool{}, + RPMs: []koji.RPM{}, }, } - output := []koji.Output{ + output := []koji.Image{ { BuildRootID: 1, Filename: path.Base(filename), @@ -92,20 +92,28 @@ func main() { ChecksumType: "md5", MD5: hash, Type: "image", - Components: []koji.Component{}, - Extra: koji.OutputExtra{ - Image: koji.OutputExtraImageInfo{ + RPMs: []koji.RPM{}, + Extra: koji.ImageExtra{ + Info: koji.ImageExtraInfo{ Arch: arch, }, }, }, } - result, err := k.CGImport(build, buildRoots, output, dir) + initResult, err := k.CGInitBuild(nil, build.Name, build.Version, build.Release) if err != nil { println(err.Error()) return } - fmt.Printf("Success, build id: %d\n", result.BuildID) + build.BuildID = uint64(initResult.BuildID) + + importResult, err := k.CGImport(build, buildRoots, output, dir, initResult.Token) + if err != nil { + println(err.Error()) + return + } + + fmt.Printf("Success, build id: %d\n", importResult.BuildID) } diff --git a/cmd/osbuild-worker/main.go b/cmd/osbuild-worker/main.go index 635b9e552..270e7e170 100644 --- a/cmd/osbuild-worker/main.go +++ b/cmd/osbuild-worker/main.go @@ -86,11 +86,15 @@ func RunJob(job worker.Job, store string) (*osbuild.Result, error) { return nil, err } + start_time := time.Now() + result, err := RunOSBuild(manifest, store, outputDirectory, os.Stderr) if err != nil { return nil, err } + end_time := time.Now() + var r []error for _, t := range targets { @@ -199,12 +203,13 @@ func RunJob(job worker.Job, store string) (*osbuild.Result, error) { continue } - build := koji.Build{ - Name: job.Id.String(), - Version: "1", - Release: "1", - StartTime: time.Now().Unix(), - EndTime: time.Now().Unix(), + build := koji.ImageBuild{ + BuildID: options.BuildID, + Name: options.Name, + Version: options.Version, + Release: options.Release, + StartTime: start_time.Unix(), + EndTime: end_time.Unix(), } buildRoots := []koji.BuildRoot{ { @@ -221,11 +226,11 @@ func RunJob(job worker.Job, store string) (*osbuild.Result, error) { Type: "nspawn", Arch: "noarch", }, - Tools: []koji.Tool{}, - Components: []koji.Component{}, + Tools: []koji.Tool{}, + RPMs: []koji.RPM{}, }, } - output := []koji.Output{ + output := []koji.Image{ { BuildRootID: 1, Filename: options.Filename, @@ -234,16 +239,16 @@ func RunJob(job worker.Job, store string) (*osbuild.Result, error) { ChecksumType: "md5", MD5: hash, Type: "image", - Components: []koji.Component{}, - Extra: koji.OutputExtra{ - Image: koji.OutputExtraImageInfo{ + RPMs: []koji.RPM{}, + Extra: koji.ImageExtra{ + Info: koji.ImageExtraInfo{ Arch: "noarch", }, }, }, } - _, err = k.CGImport(build, buildRoots, output, options.UploadDirectory) + _, err = k.CGImport(build, buildRoots, output, options.UploadDirectory, options.Token) if err != nil { r = append(r, err) continue diff --git a/internal/kojiapi/server.go b/internal/kojiapi/server.go index ac77ee0e6..5d97d2ef6 100644 --- a/internal/kojiapi/server.go +++ b/internal/kojiapi/server.go @@ -4,8 +4,10 @@ package kojiapi import ( + "crypto/tls" "encoding/json" "fmt" + "log" "net" "net/http" @@ -15,6 +17,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/osbuild/osbuild-composer/internal/target" + "github.com/osbuild/osbuild-composer/internal/upload/koji" "github.com/osbuild/osbuild-composer/internal/worker" ) @@ -126,10 +129,40 @@ func (server *Server) PostCompose(w http.ResponseWriter, r *http.Request) { http.Error(w, "Only single-image composes are currently supported", http.StatusBadRequest) return } + + // Koji for some reason needs TLS renegotiation enabled. + // Clone the default http transport and enable renegotiation. + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{ + Renegotiation: tls.RenegotiateOnceAsClient, + } + + k, err := koji.NewFromGSSAPI(request.Koji.Server, &koji.GSSAPICredentials{}, transport) + if err != nil { + http.Error(w, fmt.Sprintf("Could not log into Koji: %v", err), http.StatusBadRequest) + return + } + + defer func() { + err := k.Logout() + if err != nil { + log.Printf("koji logout failed: %v", err) + } + }() + + buildInfo, err := k.CGInitBuild(&request.Koji.TaskId, request.Name, request.Version, request.Release) + if err != nil { + http.Error(w, fmt.Sprintf("Could not initialize build with koji: %v", err), http.StatusBadRequest) + return + } + id, err := server.workers.Enqueue(ir.manifest, []*target.Target{ target.NewKojiTarget(&target.KojiTargetOptions{ - BuildID: uint64(request.Koji.BuildId), - Token: request.Koji.Token, + BuildID: uint64(buildInfo.BuildID), + Token: buildInfo.Token, + Name: request.Name, + Version: request.Version, + Release: request.Release, Filename: ir.filename, UploadDirectory: "osbuild-composer-koji-" + uuid.New().String(), Server: request.Koji.Server, @@ -142,6 +175,7 @@ func (server *Server) PostCompose(w http.ResponseWriter, r *http.Request) { var response ComposeResponse response.Id = id.String() + response.KojiBuildId = buildInfo.BuildID w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusCreated) err = json.NewEncoder(w).Encode(response) diff --git a/internal/target/koji_target.go b/internal/target/koji_target.go index 312405779..535429124 100644 --- a/internal/target/koji_target.go +++ b/internal/target/koji_target.go @@ -3,6 +3,9 @@ package target type KojiTargetOptions struct { BuildID uint64 `json:"build_id"` Token string `json:"token"` + Name string `json:"name"` + Version string `json:"version"` + Release string `json:"release"` Filename string `json:"filename"` UploadDirectory string `json:"upload_directory"` Server string `json:"server"` diff --git a/internal/upload/koji/koji.go b/internal/upload/koji/koji.go index e14ca6054..94af9761c 100644 --- a/internal/upload/koji/koji.go +++ b/internal/upload/koji/koji.go @@ -23,18 +23,19 @@ type Koji struct { transport http.RoundTripper } -type BuildExtra struct { +type ImageBuildExtra struct { Image interface{} `json:"image"` // No extra info tracked at build level. } -type Build struct { - Name string `json:"name"` - Version string `json:"version"` - Release string `json:"release"` - Source string `json:"source"` - StartTime int64 `json:"start_time"` - EndTime int64 `json:"end_time"` - Extra BuildExtra `json:"extra"` +type ImageBuild struct { + BuildID uint64 `json:"build_id"` + Name string `json:"name"` + Version string `json:"version"` + Release string `json:"release"` + Source string `json:"source"` + StartTime int64 `json:"start_time"` + EndTime int64 `json:"end_time"` + Extra ImageBuildExtra `json:"extra"` } type Host struct { @@ -43,7 +44,7 @@ type Host struct { } type ContentGenerator struct { - Name string `json:"name"` + Name string `json:"name"` // Must be 'osbuild'. Version string `json:"version"` } @@ -57,12 +58,12 @@ type Tool struct { Version string `json:"version"` } -type Component struct { +type RPM struct { Type string `json:"type"` // must be 'rpm' Name string `json:"name"` Version string `json:"version"` Release string `json:"release"` - Epoch *uint64 `json:"epoch"` + Epoch *string `json:"epoch,omitempty"` Arch string `json:"arch"` Sigmd5 string `json:"sigmd5"` Signature *string `json:"signature"` @@ -74,35 +75,40 @@ type BuildRoot struct { ContentGenerator ContentGenerator `json:"content_generator"` Container Container `json:"container"` Tools []Tool `json:"tools"` - Components []Component `json:"components"` + RPMs []RPM `json:"components"` } -type OutputExtraImageInfo struct { +type ImageExtraInfo struct { // TODO: Ideally this is where the pipeline would be passed. Arch string `json:"arch"` // TODO: why? } -type OutputExtra struct { - Image OutputExtraImageInfo `json:"image"` +type ImageExtra struct { + Info ImageExtraInfo `json:"image"` } -type Output struct { - BuildRootID uint64 `json:"buildroot_id"` - Filename string `json:"filename"` - FileSize uint64 `json:"filesize"` - Arch string `json:"arch"` - ChecksumType string `json:"checksum_type"` // must be 'md5' - MD5 string `json:"checksum"` - Type string `json:"type"` - Components []Component `json:"component"` - Extra OutputExtra `json:"extra"` +type Image struct { + BuildRootID uint64 `json:"buildroot_id"` + Filename string `json:"filename"` + FileSize uint64 `json:"filesize"` + Arch string `json:"arch"` + ChecksumType string `json:"checksum_type"` // must be 'md5' + MD5 string `json:"checksum"` + Type string `json:"type"` + RPMs []RPM `json:"component"` + Extra ImageExtra `json:"extra"` } type Metadata struct { MetadataVersion int `json:"metadata_version"` // must be '0' - Build Build `json:"build"` + ImageBuild ImageBuild `json:"build"` BuildRoots []BuildRoot `json:"buildroots"` - Output []Output `json:"output"` + Images []Image `json:"output"` +} + +type CGInitBuildResult struct { + BuildID int `xmlrpc:"build_id"` + Token string `xmlrpc:"token"` } type CGImportResult struct { @@ -209,13 +215,36 @@ func (k *Koji) Logout() error { return nil } +// CGInitBuild reserves a build ID and initializes a build +func (k *Koji) CGInitBuild(taskID *int, name, version, release string) (*CGInitBuildResult, error) { + var buildInfo struct { + TaskID *int `xmlrpc:"task_id,omitempty"` + Name string `xmlrpc:"name"` + Version string `xmlrpc:"version"` + Release string `xmlrpc:"release"` + } + + buildInfo.TaskID = taskID + buildInfo.Name = name + buildInfo.Version = version + buildInfo.Release = release + + var result CGInitBuildResult + err := k.xmlrpc.Call("CGInitBuild", []interface{}{"osbuild", buildInfo}, &result) + if err != nil { + return nil, err + } + + return &result, nil +} + // CGImport imports previously uploaded content, by specifying its metadata, and the temporary // directory where it is located. -func (k *Koji) CGImport(build Build, buildRoots []BuildRoot, output []Output, directory string) (*CGImportResult, error) { +func (k *Koji) CGImport(build ImageBuild, buildRoots []BuildRoot, images []Image, directory, token string) (*CGImportResult, error) { m := &Metadata{ - Build: build, + ImageBuild: build, BuildRoots: buildRoots, - Output: output, + Images: images, } metadata, err := json.Marshal(m) if err != nil { @@ -223,7 +252,7 @@ func (k *Koji) CGImport(build Build, buildRoots []BuildRoot, output []Output, di } var result CGImportResult - err = k.xmlrpc.Call("CGImport", []interface{}{string(metadata), directory}, &result) + err = k.xmlrpc.Call("CGImport", []interface{}{string(metadata), directory, token}, &result) if err != nil { return nil, err } diff --git a/internal/upload/koji/koji_test.go b/internal/upload/koji/koji_test.go index b179005c4..e812548a6 100644 --- a/internal/upload/koji/koji_test.go +++ b/internal/upload/koji/koji_test.go @@ -81,7 +81,7 @@ func TestKojiImport(t *testing.T) { require.NoError(t, err) // Import the build - build := koji.Build{ + build := koji.ImageBuild{ Name: buildName, Version: "1", Release: "1", @@ -103,11 +103,11 @@ func TestKojiImport(t *testing.T) { Type: "nspawn", Arch: "noarch", }, - Tools: []koji.Tool{}, - Components: []koji.Component{}, + Tools: []koji.Tool{}, + RPMs: []koji.RPM{}, }, } - output := []koji.Output{ + output := []koji.Image{ { BuildRootID: 1, Filename: filename, @@ -116,16 +116,21 @@ func TestKojiImport(t *testing.T) { ChecksumType: "md5", MD5: hash, Type: "image", - Components: []koji.Component{}, - Extra: koji.OutputExtra{ - Image: koji.OutputExtraImageInfo{ + RPMs: []koji.RPM{}, + Extra: koji.ImageExtra{ + Info: koji.ImageExtraInfo{ Arch: "noarch", }, }, }, } - result, err := k.CGImport(build, buildRoots, output, uploadDirectory) + initResult, err := k.CGInitBuild(nil, build.Name, build.Version, build.Release) + require.NoError(t, err) + + build.BuildID = uint64(initResult.BuildID) + + importResult, err := k.CGImport(build, buildRoots, output, uploadDirectory, initResult.Token) require.NoError(t, err) // check if the build is really there: @@ -136,7 +141,7 @@ func TestKojiImport(t *testing.T) { "--keytab", credentials.KeyTab, "--principal", credentials.Principal, "list-builds", - "--buildid", strconv.Itoa(result.BuildID), + "--buildid", strconv.Itoa(importResult.BuildID), ) // sample output: