From 3b5d143dc5da78f9506268334d13d170b048417a Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Mon, 5 Jun 2017 14:00:40 -0700 Subject: [PATCH 1/9] client: add application oem attribute --- omaha/client/client.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/omaha/client/client.go b/omaha/client/client.go index 1cbf163..a4a6fc6 100644 --- a/omaha/client/client.go +++ b/omaha/client/client.go @@ -53,6 +53,7 @@ type AppClient struct { appID string track string version string + oem string } // New creates an omaha client for updating one or more applications. @@ -178,6 +179,12 @@ func (ac *AppClient) SetTrack(track string) error { return nil } +// SetOEM sets the application OEM name. +// This is a update_engine/Core Update protocol extension. +func (ac *AppClient) SetOEM(oem string) { + ac.oem = oem +} + func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { req := ac.newReq() app := req.Apps[0] @@ -265,6 +272,7 @@ func (ac *AppClient) newReq() *omaha.Request { app := req.AddApp(ac.appID, ac.version) app.Track = ac.track + app.OEM = ac.oem // MachineID and BootID are non-standard fields used by CoreOS' // update_engine and Core Update. Copy their values from the From c42bec118e6126789fed261ec7bdbd6ec216aee4 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Mon, 5 Jun 2017 14:12:07 -0700 Subject: [PATCH 2/9] client: support runtime changes of app id --- omaha/client/client.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/omaha/client/client.go b/omaha/client/client.go index a4a6fc6..106a384 100644 --- a/omaha/client/client.go +++ b/omaha/client/client.go @@ -155,6 +155,21 @@ func NewAppClient(serverURL, userID, appID, appVersion string) (*AppClient, erro return ac, nil } +func (ac *AppClient) SetAppID(appID string) error { + if appID == ac.appID { + return nil + } + + if _, ok := ac.apps[appID]; ok { + return fmt.Errorf("omaha: duplicate app %q", appID) + } + + delete(ac.apps, ac.appID) + ac.appID = appID + ac.apps[appID] = ac + return nil +} + // SetVersion changes the application version. func (ac *AppClient) SetVersion(version string) error { if version == "" { From 03222d488c007f3179278e7ef424c510558c8a09 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Tue, 6 Jun 2017 17:03:22 -0700 Subject: [PATCH 3/9] client: support sending events asynchronously Makes blocking and waiting for success/error optional, also allows concurrent use of the rest of the client while events are sent. --- omaha/client/client.go | 38 +++++++++++++++++++++++++------------ omaha/client/client_test.go | 2 +- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/omaha/client/client.go b/omaha/client/client.go index 106a384..8e287e3 100644 --- a/omaha/client/client.go +++ b/omaha/client/client.go @@ -255,25 +255,37 @@ func (ac *AppClient) Ping() error { return nil } -func (ac *AppClient) Event(event *omaha.EventRequest) error { +// Event asynchronously sends the given omaha event. +// Reading the error channel is optional. +func (ac *AppClient) Event(event *omaha.EventRequest) <-chan error { + errc := make(chan error, 1) + url := ac.apiEndpoint req := ac.newReq() app := req.Apps[0] app.Events = append(app.Events, event) - appResp, err := ac.doReq(ac.apiEndpoint, req) - if err != nil { - return err - } + go func() { + appResp, err := ac.doReq(url, req) + if err != nil { + errc <- err + return + } - if len(appResp.Events) == 0 { - return fmt.Errorf("omaha: event status missing from response") - } + if len(appResp.Events) == 0 { + errc <- fmt.Errorf("omaha: event status missing from response") + return + } - if appResp.Events[0].Status != "ok" { - return fmt.Errorf("omaha: event status %s", appResp.Events[0].Status) - } + if appResp.Events[0].Status != "ok" { + errc <- fmt.Errorf("omaha: event status %s", appResp.Events[0].Status) + return + } - return nil + errc <- nil + return + }() + + return errc } func (ac *AppClient) newReq() *omaha.Request { @@ -299,6 +311,8 @@ func (ac *AppClient) newReq() *omaha.Request { return req } +// doReq posts an omaha request. It may be called in its own goroutine so +// it should not touch any mutable data in AppClient, but apiClient is ok. func (ac *AppClient) doReq(url string, req *omaha.Request) (*omaha.AppResponse, error) { if len(req.Apps) != 1 { panic(fmt.Errorf("unexpected number of apps: %d", len(req.Apps))) diff --git a/omaha/client/client_test.go b/omaha/client/client_test.go index 05ee0d0..251f3ff 100644 --- a/omaha/client/client_test.go +++ b/omaha/client/client_test.go @@ -167,7 +167,7 @@ func TestClientEvent(t *testing.T) { Type: omaha.EventTypeDownloadComplete, Result: omaha.EventResultSuccess, } - if err := ac.Event(event); err != nil { + if err := <-ac.Event(event); err != nil { t.Fatal(err) } From 9ed95f2e49e7379fd09bbca564863ab5585ead54 Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Tue, 6 Jun 2017 17:03:32 -0700 Subject: [PATCH 4/9] protocol: fix data type for event errorcode, should be an int --- omaha/protocol.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omaha/protocol.go b/omaha/protocol.go index 8a2d411..ed5e092 100644 --- a/omaha/protocol.go +++ b/omaha/protocol.go @@ -142,7 +142,7 @@ type PingRequest struct { type EventRequest struct { Type EventType `xml:"eventtype,attr"` Result EventResult `xml:"eventresult,attr"` - ErrorCode string `xml:"errorcode,attr,omitempty"` + ErrorCode int `xml:"errorcode,attr,omitempty"` NextVersion string `xml:"nextversion,attr,omitempty"` PreviousVersion string `xml:"previousversion,attr,omitempty"` } From 41143a9d1725ab8e9df5d3d8a07732a7d151d0ba Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 8 Jun 2017 13:50:30 -0700 Subject: [PATCH 5/9] client: define error codes and events used by update_engine Event error codes are application specific but for the sake of simplicity lets re-use the update_engine ones where applicable. update_engine and thus CoreUpdate only use a small subset of possible event types so we can provide pre-defined event structs for them. --- omaha/client/update_engine_events.go | 226 +++++++++++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 omaha/client/update_engine_events.go diff --git a/omaha/client/update_engine_events.go b/omaha/client/update_engine_events.go new file mode 100644 index 0000000..e20a70a --- /dev/null +++ b/omaha/client/update_engine_events.go @@ -0,0 +1,226 @@ +// Copyright 2017 CoreOS, Inc. +// Copyright 2011 The Chromium OS Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package client + +import ( + "fmt" + + "github.com/coreos/go-omaha/omaha" +) + +var ( + // These events are what update_engine sends to CoreUpdate to + // mark different steps in the update process. + EventDownloading = &omaha.EventRequest{ + Type: omaha.EventTypeUpdateDownloadStarted, + Result: omaha.EventResultSuccess, + } + EventDownloaded = &omaha.EventRequest{ + Type: omaha.EventTypeUpdateDownloadFinished, + Result: omaha.EventResultSuccess, + } + EventInstalled = &omaha.EventRequest{ + Type: omaha.EventTypeUpdateComplete, + Result: omaha.EventResultSuccess, + } + EventComplete = &omaha.EventRequest{ + Type: omaha.EventTypeUpdateComplete, + Result: omaha.EventResultSuccessReboot, + } +) + +// ExitCode is used for omaha event error codes derived from update_engine +type ExitCode int + +// These error codes are from CoreOS Container Linux update_engine 0.4.x +// https://github.com/coreos/update_engine/blob/master/src/update_engine/action_processor.h +// The whole list is included for the sake of completeness but lots of these +// are not generally applicable and not even used by update_engine any more. +// Also there are clearly duplicate errors for the same condition. +const ( + ExitCodeSuccess ExitCode = 0 + ExitCodeError ExitCode = 1 + ExitCodeOmahaRequestError ExitCode = 2 + ExitCodeOmahaResponseHandlerError ExitCode = 3 + ExitCodeFilesystemCopierError ExitCode = 4 + ExitCodePostinstallRunnerError ExitCode = 5 + ExitCodeSetBootableFlagError ExitCode = 6 + ExitCodeInstallDeviceOpenError ExitCode = 7 + ExitCodeKernelDeviceOpenError ExitCode = 8 + ExitCodeDownloadTransferError ExitCode = 9 + ExitCodePayloadHashMismatchError ExitCode = 10 + ExitCodePayloadSizeMismatchError ExitCode = 11 + ExitCodeDownloadPayloadVerificationError ExitCode = 12 + ExitCodeDownloadNewPartitionInfoError ExitCode = 13 + ExitCodeDownloadWriteError ExitCode = 14 + ExitCodeNewRootfsVerificationError ExitCode = 15 + ExitCodeNewKernelVerificationError ExitCode = 16 + ExitCodeSignedDeltaPayloadExpectedError ExitCode = 17 + ExitCodeDownloadPayloadPubKeyVerificationError ExitCode = 18 + ExitCodePostinstallBootedFromFirmwareB ExitCode = 19 + ExitCodeDownloadStateInitializationError ExitCode = 20 + ExitCodeDownloadInvalidMetadataMagicString ExitCode = 21 + ExitCodeDownloadSignatureMissingInManifest ExitCode = 22 + ExitCodeDownloadManifestParseError ExitCode = 23 + ExitCodeDownloadMetadataSignatureError ExitCode = 24 + ExitCodeDownloadMetadataSignatureVerificationError ExitCode = 25 + ExitCodeDownloadMetadataSignatureMismatch ExitCode = 26 + ExitCodeDownloadOperationHashVerificationError ExitCode = 27 + ExitCodeDownloadOperationExecutionError ExitCode = 28 + ExitCodeDownloadOperationHashMismatch ExitCode = 29 + ExitCodeOmahaRequestEmptyResponseError ExitCode = 30 + ExitCodeOmahaRequestXMLParseError ExitCode = 31 + ExitCodeDownloadInvalidMetadataSize ExitCode = 32 + ExitCodeDownloadInvalidMetadataSignature ExitCode = 33 + ExitCodeOmahaResponseInvalid ExitCode = 34 + ExitCodeOmahaUpdateIgnoredPerPolicy ExitCode = 35 + ExitCodeOmahaUpdateDeferredPerPolicy ExitCode = 36 + ExitCodeOmahaErrorInHTTPResponse ExitCode = 37 + ExitCodeDownloadOperationHashMissingError ExitCode = 38 + ExitCodeDownloadMetadataSignatureMissingError ExitCode = 39 + ExitCodeOmahaUpdateDeferredForBackoff ExitCode = 40 + ExitCodePostinstallPowerwashError ExitCode = 41 + ExitCodeNewPCRPolicyVerificationError ExitCode = 42 + ExitCodeNewPCRPolicyHTTPError ExitCode = 43 + + // Use the 2xxx range to encode HTTP errors from the Omaha server. + // Sometimes aggregated into ExitCodeOmahaErrorInHTTPResponse + ExitCodeOmahaRequestHTTPResponseBase ExitCode = 2000 // + HTTP response code +) + +func (e ExitCode) String() string { + switch e { + case ExitCodeSuccess: + return "success" + case ExitCodeError: + return "error" + case ExitCodeOmahaRequestError: + return "omaha request error" + case ExitCodeOmahaResponseHandlerError: + return "omaha response handler error" + case ExitCodeFilesystemCopierError: + return "filesystem copier error" + case ExitCodePostinstallRunnerError: + return "postinstall runner error" + case ExitCodeSetBootableFlagError: + return "set bootable flag error" + case ExitCodeInstallDeviceOpenError: + return "install device open error" + case ExitCodeKernelDeviceOpenError: + return "kernel device open error" + case ExitCodeDownloadTransferError: + return "download transfer error" + case ExitCodePayloadHashMismatchError: + return "payload hash mismatch error" + case ExitCodePayloadSizeMismatchError: + return "payload size mismatch error" + case ExitCodeDownloadPayloadVerificationError: + return "download payload verification error" + case ExitCodeDownloadNewPartitionInfoError: + return "download new partition info error" + case ExitCodeDownloadWriteError: + return "download write error" + case ExitCodeNewRootfsVerificationError: + return "new rootfs verification error" + case ExitCodeNewKernelVerificationError: + return "new kernel verification error" + case ExitCodeSignedDeltaPayloadExpectedError: + return "signed delta payload expected error" + case ExitCodeDownloadPayloadPubKeyVerificationError: + return "download payload pubkey verification error" + case ExitCodePostinstallBootedFromFirmwareB: + return "postinstall booted from firmware B" + case ExitCodeDownloadStateInitializationError: + return "download state initialization error" + case ExitCodeDownloadInvalidMetadataMagicString: + return "download invalid metadata magic string" + case ExitCodeDownloadSignatureMissingInManifest: + return "download signature missing in manifest" + case ExitCodeDownloadManifestParseError: + return "download manifest parse error" + case ExitCodeDownloadMetadataSignatureError: + return "download metadata signature error" + case ExitCodeDownloadMetadataSignatureVerificationError: + return "download metadata signature verification error" + case ExitCodeDownloadMetadataSignatureMismatch: + return "download metadata signature mismatch" + case ExitCodeDownloadOperationHashVerificationError: + return "download operation hash verification error" + case ExitCodeDownloadOperationExecutionError: + return "download operation execution error" + case ExitCodeDownloadOperationHashMismatch: + return "download operation hash mismatch" + case ExitCodeOmahaRequestEmptyResponseError: + return "omaha request empty response error" + case ExitCodeOmahaRequestXMLParseError: + return "omaha request XML parse error" + case ExitCodeDownloadInvalidMetadataSize: + return "download invalid metadata size" + case ExitCodeDownloadInvalidMetadataSignature: + return "download invalid metadata signature" + case ExitCodeOmahaResponseInvalid: + return "omaha response invalid" + case ExitCodeOmahaUpdateIgnoredPerPolicy: + return "omaha update ignored per policy" + case ExitCodeOmahaUpdateDeferredPerPolicy: + return "omaha update deferred per policy" + case ExitCodeOmahaErrorInHTTPResponse: + return "omaha error in HTTP response" + case ExitCodeDownloadOperationHashMissingError: + return "download operation hash missing error" + case ExitCodeDownloadMetadataSignatureMissingError: + return "download metadata signature missing error" + case ExitCodeOmahaUpdateDeferredForBackoff: + return "omaha update deferred for backoff" + case ExitCodePostinstallPowerwashError: + return "postinstall powerwash error" + case ExitCodeNewPCRPolicyVerificationError: + return "new PCR policy verification error" + case ExitCodeNewPCRPolicyHTTPError: + return "new PCR policy HTTP error" + default: + if e > ExitCodeOmahaRequestHTTPResponseBase { + return fmt.Sprintf("omaha response HTTP %d error", + e-ExitCodeOmahaRequestHTTPResponseBase) + } + return fmt.Sprintf("error code %d", e) + } +} + +// NewErrorEvent creates an EventRequest for reporting errors. +func NewErrorEvent(e ExitCode) *omaha.EventRequest { + return &omaha.EventRequest{ + Type: omaha.EventTypeUpdateComplete, + Result: omaha.EventResultError, + ErrorCode: int(e), + } +} + +// EventString allows for easily logging events in a readable format. +func EventString(e *omaha.EventRequest) string { + s := fmt.Sprintf("omaha event: %s: %s", e.Type, e.Result) + if e.ErrorCode != 0 { + s = fmt.Sprintf("%s (%d - %s)", s, + e.ErrorCode, ExitCode(e.ErrorCode)) + } + return s +} + +// ErrorEvent is an error type that can generate EventRequests for reporting. +type ErrorEvent interface { + error + ErrorEvent() *omaha.EventRequest +} From 2d48f3a029611d79d664238895ec84a900d2470e Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 8 Jun 2017 14:28:48 -0700 Subject: [PATCH 6/9] client: send error events if update check or ping fails Does not report errors when sending events since that would be cyclic. --- omaha/client/client.go | 20 +++++++++++++++++--- omaha/client/error.go | 36 +++++++++++++++++++++++++++++++++--- omaha/client/http.go | 9 ++++----- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/omaha/client/client.go b/omaha/client/client.go index 8e287e3..61c25cd 100644 --- a/omaha/client/client.go +++ b/omaha/client/client.go @@ -209,11 +209,16 @@ func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { ac.sentPing = true appResp, err := ac.doReq(ac.apiEndpoint, req) - if err != nil { + if err, ok := err.(ErrorEvent); ok { + ac.Event(err.ErrorEvent()) + return nil, err + } else if err != nil { + ac.Event(NewErrorEvent(ExitCodeOmahaRequestError)) return nil, err } if appResp.Ping == nil { + ac.Event(NewErrorEvent(ExitCodeOmahaResponseInvalid)) return nil, fmt.Errorf("omaha: ping status missing from response") } @@ -222,6 +227,7 @@ func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { } if appResp.UpdateCheck == nil { + ac.Event(NewErrorEvent(ExitCodeOmahaResponseInvalid)) return nil, fmt.Errorf("omaha: update check missing from response") } @@ -240,11 +246,16 @@ func (ac *AppClient) Ping() error { ac.sentPing = true appResp, err := ac.doReq(ac.apiEndpoint, req) - if err != nil { + if err, ok := err.(ErrorEvent); ok { + ac.Event(err.ErrorEvent()) + return err + } else if err != nil { + ac.Event(NewErrorEvent(ExitCodeOmahaRequestError)) return err } if appResp.Ping == nil { + ac.Event(NewErrorEvent(ExitCodeOmahaResponseInvalid)) return fmt.Errorf("omaha: ping status missing from response") } @@ -325,7 +336,10 @@ func (ac *AppClient) doReq(url string, req *omaha.Request) (*omaha.AppResponse, appResp := resp.GetApp(appID) if appResp == nil { - return nil, fmt.Errorf("omaha: app %s missing from response", appID) + return nil, &omahaError{ + Err: fmt.Errorf("app %s missing from response", appID), + Code: ExitCodeOmahaResponseInvalid, + } } if appResp.Status != omaha.AppOK { diff --git a/omaha/client/error.go b/omaha/client/error.go index 9e1a80e..cf221a2 100644 --- a/omaha/client/error.go +++ b/omaha/client/error.go @@ -21,11 +21,19 @@ import ( "net" "net/http" "time" + + "github.com/coreos/go-omaha/omaha" ) var ( - bodySizeError = errors.New("http response exceeded 1MB") - bodyEmptyError = errors.New("http response was empty") + bodySizeError = &omahaError{ + Err: errors.New("http response exceeded 1MB"), + Code: ExitCodeOmahaResponseInvalid, + } + bodyEmptyError = &omahaError{ + Err: errors.New("http response was empty"), + Code: ExitCodeOmahaRequestEmptyResponseError, + } // default parameters for expNetBackoff backoffStart = time.Second @@ -60,7 +68,21 @@ func isUnexpectedEOF(err error) bool { return err == io.ErrUnexpectedEOF } -// httpError implements error and net.Error for http responses. +// omahaError implements error and ErrorEvent for omaha requests/responses. +type omahaError struct { + Err error + Code ExitCode +} + +func (oe *omahaError) Error() string { + return "omaha: request failed: " + oe.Err.Error() +} + +func (oe *omahaError) ErrorEvent() *omaha.EventRequest { + return NewErrorEvent(oe.Code) +} + +// httpError implements error, net.Error, and ErrorEvent for http responses. type httpError struct { *http.Response } @@ -69,6 +91,14 @@ func (he *httpError) Error() string { return "http error: " + he.Status } +func (he *httpError) ErrorEvent() *omaha.EventRequest { + code := ExitCodeOmahaRequestError + if he.StatusCode > 0 && he.StatusCode < 1000 { + code = ExitCodeOmahaRequestHTTPResponseBase + ExitCode(he.StatusCode) + } + return NewErrorEvent(code) +} + func (he *httpError) Timeout() bool { switch he.StatusCode { case http.StatusRequestTimeout: // 408 diff --git a/omaha/client/http.go b/omaha/client/http.go index 13e4a7b..9a634b7 100644 --- a/omaha/client/http.go +++ b/omaha/client/http.go @@ -45,7 +45,7 @@ func newHTTPClient() *httpClient { func (hc *httpClient) doPost(url string, reqBody []byte) (*omaha.Response, error) { resp, err := hc.Post(url, "text/xml; charset=utf-8", bytes.NewReader(reqBody)) if err != nil { - return nil, err + return nil, &omahaError{err, ExitCodeOmahaRequestError} } defer resp.Body.Close() @@ -59,6 +59,8 @@ func (hc *httpClient) doPost(url string, reqBody []byte) (*omaha.Response, error err = bodySizeError } else if err == io.EOF { err = bodyEmptyError + } else if err != nil { + err = &omahaError{err, ExitCodeOmahaRequestXMLParseError} } // Prefer reporting HTTP errors over XML parsing errors. @@ -81,9 +83,6 @@ func (hc *httpClient) Omaha(url string, req *omaha.Request) (resp *omaha.Respons resp, err = hc.doPost(url, buf.Bytes()) return err }) - if err != nil { - return nil, fmt.Errorf("omaha: request failed: %v", err) - } - return resp, nil + return resp, err } From d946c1e7b22bdafe4b409e0ca48576415e5fcb2f Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 8 Jun 2017 12:54:51 -0700 Subject: [PATCH 7/9] client: disable checking for ping and event status in responses Turns out CoreUpdate does not currently send and in responses like the protocol says it should. --- omaha/client/client.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/omaha/client/client.go b/omaha/client/client.go index 61c25cd..1187129 100644 --- a/omaha/client/client.go +++ b/omaha/client/client.go @@ -217,14 +217,15 @@ func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { return nil, err } - if appResp.Ping == nil { + // BUG: CoreUpdate does not send ping status in response. + /*if appResp.Ping == nil { ac.Event(NewErrorEvent(ExitCodeOmahaResponseInvalid)) return nil, fmt.Errorf("omaha: ping status missing from response") } if appResp.Ping.Status != "ok" { return nil, fmt.Errorf("omaha: ping status %s", appResp.Ping.Status) - } + }*/ if appResp.UpdateCheck == nil { ac.Event(NewErrorEvent(ExitCodeOmahaResponseInvalid)) @@ -254,14 +255,16 @@ func (ac *AppClient) Ping() error { return err } - if appResp.Ping == nil { + // BUG: CoreUpdate does not send ping status in response. + _ = appResp + /*if appResp.Ping == nil { ac.Event(NewErrorEvent(ExitCodeOmahaResponseInvalid)) return fmt.Errorf("omaha: ping status missing from response") } if appResp.Ping.Status != "ok" { return fmt.Errorf("omaha: ping status %s", appResp.Ping.Status) - } + }*/ return nil } @@ -282,7 +285,9 @@ func (ac *AppClient) Event(event *omaha.EventRequest) <-chan error { return } - if len(appResp.Events) == 0 { + // BUG: CoreUpdate does not send event status in response. + _ = appResp + /*if len(appResp.Events) == 0 { errc <- fmt.Errorf("omaha: event status missing from response") return } @@ -290,7 +295,7 @@ func (ac *AppClient) Event(event *omaha.EventRequest) <-chan error { if appResp.Events[0].Status != "ok" { errc <- fmt.Errorf("omaha: event status %s", appResp.Events[0].Status) return - } + }*/ errc <- nil return From a2f653da3480bea025684ac8b105c418d8feb59b Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Thu, 8 Jun 2017 17:12:59 -0700 Subject: [PATCH 8/9] client: send "Complete" event in update checks Needed in order to match update_engine behavior. --- README.md | 3 ++- omaha/client/client.go | 5 +++++ omaha/client/client_test.go | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 573622a..8899cf9 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,8 @@ These differences include: - No offline activity tracking. The protocol's ping mechanism allows for tracking application usage, reporting the number of days since the last ping and how many of those days saw active usage. CoreUpdate does not use this, instead assuming update clients are always online and checking in once every ~45-50 minutes. - Each check in should include a ping and optionally an update check. + Clients not actively updating should send only a ping, indicating CoreUpdate's "Instance-Hold" state. + Clients requesting an update should send a ping, update check, and an UpdateComplete:SuccessReboot event indicating CoreUpdate's "Complete" state. - Various protocol extensions/abuses. update_engine, likely due to earlier limitations of the protocol and Google's server implementation, uses a number of non-standard fields. diff --git a/omaha/client/client.go b/omaha/client/client.go index 1187129..8bb4572 100644 --- a/omaha/client/client.go +++ b/omaha/client/client.go @@ -206,6 +206,11 @@ func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { app.AddPing() app.AddUpdateCheck() + // Tell CoreUpdate to consider us in its "Complete" state, + // otherwise it interprets ping as "Instance-Hold" which is + // nonsense when we are sending an update check! + app.Events = append(app.Events, EventComplete) + ac.sentPing = true appResp, err := ac.doReq(ac.apiEndpoint, req) diff --git a/omaha/client/client_test.go b/omaha/client/client_test.go index 251f3ff..bab5772 100644 --- a/omaha/client/client_test.go +++ b/omaha/client/client_test.go @@ -100,6 +100,15 @@ func TestClientNoUpdate(t *testing.T) { if len(r.checks) != 1 { t.Fatalf("expected 1 update check, not %d", len(r.checks)) } + + if len(r.events) != 1 { + t.Fatalf("expected 1 event, not %d", len(r.events)) + } + + if r.events[0].Type != omaha.EventTypeUpdateComplete || + r.events[0].Result != omaha.EventResultSuccessReboot { + t.Fatalf("expected %#v, not %#v", EventComplete, r.events[0]) + } } func TestClientWithUpdate(t *testing.T) { @@ -132,6 +141,15 @@ func TestClientWithUpdate(t *testing.T) { if len(r.checks) != 1 { t.Fatalf("expected 1 update check, not %d", len(r.checks)) } + + if len(r.events) != 1 { + t.Fatalf("expected 1 event, not %d", len(r.events)) + } + + if r.events[0].Type != omaha.EventTypeUpdateComplete || + r.events[0].Result != omaha.EventResultSuccessReboot { + t.Fatalf("expected %#v, not %#v", EventComplete, r.events[0]) + } } func TestClientPing(t *testing.T) { From 0152a8b1b029d0bc413ed0a8375a45ded872bbdf Mon Sep 17 00:00:00 2001 From: Michael Marineau Date: Fri, 9 Jun 2017 12:02:58 -0700 Subject: [PATCH 9/9] client: export NewAppRequest and SendAppRequest methods Allow for more flexibility in constructing requests if needed. --- omaha/client/client.go | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/omaha/client/client.go b/omaha/client/client.go index 8bb4572..6639ae6 100644 --- a/omaha/client/client.go +++ b/omaha/client/client.go @@ -201,7 +201,7 @@ func (ac *AppClient) SetOEM(oem string) { } func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { - req := ac.newReq() + req := ac.NewAppRequest() app := req.Apps[0] app.AddPing() app.AddUpdateCheck() @@ -213,12 +213,8 @@ func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { ac.sentPing = true - appResp, err := ac.doReq(ac.apiEndpoint, req) - if err, ok := err.(ErrorEvent); ok { - ac.Event(err.ErrorEvent()) - return nil, err - } else if err != nil { - ac.Event(NewErrorEvent(ExitCodeOmahaRequestError)) + appResp, err := ac.SendAppRequest(req) + if err != nil { return nil, err } @@ -245,18 +241,14 @@ func (ac *AppClient) UpdateCheck() (*omaha.UpdateResponse, error) { } func (ac *AppClient) Ping() error { - req := ac.newReq() + req := ac.NewAppRequest() app := req.Apps[0] app.AddPing() ac.sentPing = true - appResp, err := ac.doReq(ac.apiEndpoint, req) - if err, ok := err.(ErrorEvent); ok { - ac.Event(err.ErrorEvent()) - return err - } else if err != nil { - ac.Event(NewErrorEvent(ExitCodeOmahaRequestError)) + appResp, err := ac.SendAppRequest(req) + if err != nil { return err } @@ -279,7 +271,7 @@ func (ac *AppClient) Ping() error { func (ac *AppClient) Event(event *omaha.EventRequest) <-chan error { errc := make(chan error, 1) url := ac.apiEndpoint - req := ac.newReq() + req := ac.NewAppRequest() app := req.Apps[0] app.Events = append(app.Events, event) @@ -309,7 +301,8 @@ func (ac *AppClient) Event(event *omaha.EventRequest) <-chan error { return errc } -func (ac *AppClient) newReq() *omaha.Request { +// NewAppRequest creates a Request object containing one application. +func (ac *AppClient) NewAppRequest() *omaha.Request { req := omaha.NewRequest() req.Version = ac.clientVersion req.UserID = ac.userID @@ -332,6 +325,21 @@ func (ac *AppClient) newReq() *omaha.Request { return req } +// SendAppRequest sends a Request object and validates the response. +// On failure an error event is automatically sent to the server. +func (ac *AppClient) SendAppRequest(req *omaha.Request) (*omaha.AppResponse, error) { + resp, err := ac.doReq(ac.apiEndpoint, req) + if _, ok := err.(omaha.AppStatus); ok { + // No point to sending an error if we got a well-formed + // non-ok application status in the response. + } else if err, ok := err.(ErrorEvent); ok { + ac.Event(err.ErrorEvent()) + } else if err != nil { + ac.Event(NewErrorEvent(ExitCodeOmahaRequestError)) + } + return resp, err +} + // doReq posts an omaha request. It may be called in its own goroutine so // it should not touch any mutable data in AppClient, but apiClient is ok. func (ac *AppClient) doReq(url string, req *omaha.Request) (*omaha.AppResponse, error) {