diff --git a/changelog/unreleased/fix-select-next-gateway-client.md b/changelog/unreleased/fix-select-next-gateway-client.md index 80730a01151..b08263f1a65 100644 --- a/changelog/unreleased/fix-select-next-gateway-client.md +++ b/changelog/unreleased/fix-select-next-gateway-client.md @@ -2,4 +2,5 @@ Bugfix: Always select next gateway client We now use the gateway selector to always select the next gateway client. This ensures that we can always connect to the gateway during up- and downscaling. -https://github.com/owncloud/ocis/pull/10133 +https://github.com/owncloud/ocis/pull/10141 +https://github.com/owncloud/ocis/pull/10133 \ No newline at end of file diff --git a/services/collaboration/pkg/connector/contentconnector.go b/services/collaboration/pkg/connector/contentconnector.go index 0bdd4e427ac..41d1b0aaa6f 100644 --- a/services/collaboration/pkg/connector/contentconnector.go +++ b/services/collaboration/pkg/connector/contentconnector.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto/tls" + "fmt" "io" "net/http" "strconv" @@ -105,13 +106,10 @@ func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) e sResp, err := gwc.Stat(ctx, &providerv1beta1.StatRequest{ Ref: wopiContext.FileReference, }) - if err != nil { - logger.Error().Err(err).Msg("GetFile: Stat Request failed") + if err := requestFailed(logger, sResp.GetStatus(), err, "GetFile: Stat Request failed"); err != nil { return err } - if sResp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { - return NewConnectorError(500, sResp.GetStatus().GetCode().String()+" "+sResp.GetStatus().GetMessage()) - } + // Initiate download request req := &providerv1beta1.InitiateFileDownloadRequest{ Ref: wopiContext.FileReference, @@ -125,19 +123,10 @@ func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) e return err } resp, err := gwc.InitiateFileDownload(ctx, req) - if err != nil { - logger.Error().Err(err).Msg("GetFile: InitiateFileDownload failed") + if err := requestFailed(logger, resp.GetStatus(), err, "GetFile: InitiateFileDownload failed"); err != nil { return err } - if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { - logger.Error(). - Str("StatusCode", resp.GetStatus().GetCode().String()). - Str("StatusMsg", resp.GetStatus().GetMessage()). - Msg("GetFile: InitiateFileDownload failed with wrong status") - return NewConnectorError(500, resp.GetStatus().GetCode().String()+" "+resp.GetStatus().GetMessage()) - } - // Figure out the download endpoint and download token downloadEndpoint := "" downloadToken := "" @@ -152,6 +141,10 @@ func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) e } } + logger = logger.With(). + Str("Endpoint", downloadEndpoint). + Bool("HasDownloadToken", hasDownloadToken).Logger() + httpClient := http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ @@ -164,21 +157,13 @@ func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) e // public link downloads have the token in the download endpoint httpReq, err := newHttpRequest(ctx, wopiContext, http.MethodGet, downloadEndpoint, downloadToken, bytes.NewReader([]byte(""))) if err != nil { - logger.Error(). - Err(err). - Str("Endpoint", downloadEndpoint). - Bool("HasDownloadToken", hasDownloadToken). - Msg("GetFile: Could not create the request to the endpoint") + logger.Error().Err(err).Msg("GetFile: Could not create the request to the endpoint") return err } httpResp, err := httpClient.Do(httpReq) if err != nil { - logger.Error(). - Err(err). - Str("Endpoint", downloadEndpoint). - Bool("HasDownloadToken", hasDownloadToken). - Msg("GetFile: Get request to the download endpoint failed") + logger.Error().Err(err).Msg("GetFile: Get request to the download endpoint failed") return err } @@ -186,7 +171,6 @@ func (c *ContentConnector) GetFile(ctx context.Context, w http.ResponseWriter) e if httpResp.StatusCode != http.StatusOK { logger.Error(). - Err(err). Int("HttpCode", httpResp.StatusCode). Msg("GetFile: downloading the file failed") return NewConnectorError(500, "GetFile: Downloading the file failed") @@ -248,19 +232,10 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream statRes, err := gwc.Stat(ctx, &providerv1beta1.StatRequest{ Ref: wopiContext.FileReference, }) - if err != nil { - logger.Error().Err(err).Msg("PutFile: stat failed") + if err := requestFailed(logger, statRes.GetStatus(), err, "PutFile: stat failed"); err != nil { return nil, err } - if statRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK && statRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_NOT_FOUND { - logger.Error(). - Str("StatusCode", statRes.GetStatus().GetCode().String()). - Str("StatusMsg", statRes.GetStatus().GetMessage()). - Msg("PutFile: stat failed with unexpected status") - return NewResponse(500), nil - } - mtime := statRes.GetInfo().GetMtime() // If there is a lock and it mismatches, return 409 if statRes.GetInfo().GetLock() != nil && statRes.GetInfo().GetLock().GetLockId() != lockID { @@ -308,19 +283,10 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream } // Initiate the upload request resp, err := gwc.InitiateFileUpload(ctx, req) - if err != nil { - logger.Error().Err(err).Msg("UploadHelper: InitiateFileUpload failed") + if err := requestFailed(logger, resp.GetStatus(), err, "PutFile: InitiateFileUpload failed"); err != nil { return nil, err } - if resp.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { - logger.Error(). - Str("StatusCode", resp.GetStatus().GetCode().String()). - Str("StatusMsg", resp.GetStatus().GetMessage()). - Msg("UploadHelper: InitiateFileUpload failed with wrong status") - return NewResponse(500), nil - } - // if the content length is greater than 0, we need to upload the content to the // target endpoint, otherwise we're done if streamLength > 0 { @@ -338,6 +304,10 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream } } + logger = logger.With(). + Str("Endpoint", uploadEndpoint). + Bool("HasUploadToken", hasUploadToken).Logger() + httpClient := http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ @@ -351,11 +321,7 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream // public link uploads have the token in the upload endpoint httpReq, err := newHttpRequest(ctx, wopiContext, http.MethodPut, uploadEndpoint, uploadToken, stream) if err != nil { - logger.Error(). - Err(err). - Str("Endpoint", uploadEndpoint). - Bool("HasUploadToken", hasUploadToken). - Msg("UploadHelper: Could not create the request to the endpoint") + logger.Error().Err(err).Msg("UploadHelper: Could not create the request to the endpoint") return nil, err } // "stream" is an *http.body and doesn't fill the httpReq.ContentLength automatically @@ -371,22 +337,16 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream httpResp, err := httpClient.Do(httpReq) if err != nil { - logger.Error(). - Err(err). - Str("Endpoint", uploadEndpoint). - Bool("HasUploadToken", hasUploadToken). - Msg("UploadHelper: Put request to the upload endpoint failed") + logger.Error().Err(err).Msg("UploadHelper: Put request to the upload endpoint failed") return nil, err } defer httpResp.Body.Close() if httpResp.StatusCode != http.StatusOK { logger.Error(). - Str("Endpoint", uploadEndpoint). - Bool("HasUploadToken", hasUploadToken). Int("HttpCode", httpResp.StatusCode). Msg("UploadHelper: Put request to the upload endpoint failed with unexpected status") - return NewResponse(500), nil + return nil, NewConnectorError(500, fmt.Sprintf("unexpected status code %d from the upload endpoint", httpResp.StatusCode)) } gwc, err = c.gws.Next() if err != nil { @@ -397,20 +357,31 @@ func (c *ContentConnector) PutFile(ctx context.Context, stream io.Reader, stream statResAfter, err := gwc.Stat(ctx, &providerv1beta1.StatRequest{ Ref: wopiContext.FileReference, }) - if err != nil { - logger.Error().Err(err).Msg("PutFile: stat after upload failed") + if err := requestFailed(logger, statResAfter.GetStatus(), err, "PutFile: stat after upload failed"); err != nil { return nil, err } - if statResAfter.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK { - logger.Error(). - Str("StatusCode", statRes.GetStatus().GetCode().String()). - Str("StatusMsg", statRes.GetStatus().GetMessage()). - Msg("PutFile: stat after upload failed with unexpected status") - return NewResponse(500), nil - } mtime = statResAfter.GetInfo().GetMtime() } logger.Debug().Msg("PutFile: success") return NewResponseWithVersion(mtime), nil } + +func requestFailed(logger zerolog.Logger, s *rpcv1beta1.Status, err error, msg string) error { + if err != nil { + logger.Error().Err(err).Msg(msg) + return err + } + if s == nil { + logger.Error().Msg(msg + ": nil status") + return NewConnectorError(500, msg+": nil status") + } + if s.GetCode() != rpcv1beta1.Code_CODE_OK { + logger.Error(). + Str("StatusCode", s.GetCode().String()). + Str("StatusMsg", s.GetMessage()). + Msg(msg) + return NewConnectorError(500, s.GetCode().String()+" "+s.GetMessage()) + } + return nil +} diff --git a/services/collaboration/pkg/connector/contentconnector_test.go b/services/collaboration/pkg/connector/contentconnector_test.go index 9bdb374e352..dfe99f09a17 100644 --- a/services/collaboration/pkg/connector/contentconnector_test.go +++ b/services/collaboration/pkg/connector/contentconnector_test.go @@ -126,9 +126,8 @@ var _ = Describe("ContentConnector", func() { }, nil) err := cc.GetFile(ctx, sb) - Expect(err).To(HaveOccurred()) - conErr := err.(*connector.ConnectorError) - Expect(conErr.HttpCodeOut).To(Equal(500)) + targetErr := connector.NewConnectorError(500, "CODE_INTERNAL Something failed") + Expect(err).To(Equal(targetErr)) }) It("Missing download endpoint", func() { @@ -264,9 +263,9 @@ var _ = Describe("ContentConnector", func() { }, nil) response, err := cc.PutFile(ctx, reader, reader.Size(), "notARandomLockId") - Expect(err).ToNot(HaveOccurred()) - Expect(response.Status).To(Equal(500)) - Expect(response.Headers).To(BeNil()) + targetErr := connector.NewConnectorError(500, "CODE_INTERNAL Something failed") + Expect(err).To(Equal(targetErr)) + Expect(response).To(BeNil()) }) It("Mismatched lockId", func() { @@ -354,9 +353,9 @@ var _ = Describe("ContentConnector", func() { }, nil) response, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") - Expect(err).ToNot(HaveOccurred()) - Expect(response.Status).To(Equal(500)) - Expect(response.Headers).To(BeNil()) + targetErr := connector.NewConnectorError(500, "CODE_INTERNAL Something failed") + Expect(err).To(Equal(targetErr)) + Expect(response).To(BeNil()) }) It("Empty upload successful", func() { @@ -406,9 +405,9 @@ var _ = Describe("ContentConnector", func() { }, nil) response, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") - Expect(err).ToNot(HaveOccurred()) - Expect(response.Status).To(Equal(500)) - Expect(response.Headers).To(BeNil()) + targetErr := connector.NewConnectorError(500, "url is missing") + Expect(err).To(Equal(targetErr)) + Expect(response).To(BeNil()) }) It("upload request failed", func() { @@ -438,9 +437,9 @@ var _ = Describe("ContentConnector", func() { response, err := cc.PutFile(ctx, reader, reader.Size(), "goodAndValidLock") Expect(srvReqHeader.Get("X-Access-Token")).To(Equal(wopiCtx.AccessToken)) - Expect(err).ToNot(HaveOccurred()) - Expect(response.Status).To(Equal(500)) - Expect(response.Headers).To(BeNil()) + targetErr := connector.NewConnectorError(500, "unexpected status code 404 from the upload endpoint") + Expect(err).To(Equal(targetErr)) + Expect(response).To(BeNil()) }) It("upload request success", func() {