From a8010013ae30ad822566ae496c64244958d9ffbf Mon Sep 17 00:00:00 2001 From: Jesse Geens Date: Fri, 4 Oct 2024 09:07:21 +0200 Subject: [PATCH] Implemented X-Disable-Versioning header to disable versioning on EOS This commit introduces a new header, `X-Disable-Versioning`, that is available on PUT requests. This header will disable versioning for this file save if the storage backend is EOS. Additionally, this commit introduces two unit tests related to this functionality: - TestDisableVersioningLeadsToCorrectQueryParams: test whether disabling versioning leads to correct query parameteers for the EOS HTTP / GRPC client - TestDisableVersioningHeaderPassedAlong: test whether a header passed to the initial endpoint is propagated to the actual upload endpoint Finally, this commit also fixes a bug that was already present, where the app was not passed along in the case of token-based authz --- changelog/unreleased/disable-versioning.md | 4 + go.mod | 3 +- go.sum | 6 ++ internal/http/services/owncloud/ocdav/put.go | 8 ++ .../http/services/owncloud/ocdav/put_test.go | 94 +++++++++++++++++++ .../http/services/owncloud/ocdav/webdav.go | 1 + pkg/eosclient/eosbinary/eosbinary.go | 16 +++- pkg/eosclient/eosclient.go | 2 +- pkg/eosclient/eosgrpc/eosgrpc.go | 6 +- pkg/eosclient/eosgrpc/eoshttp.go | 19 +++- pkg/eosclient/eosgrpc/eoshttp_test.go | 59 ++++++++++++ pkg/rhttp/datatx/manager/simple/simple.go | 4 + pkg/storage/utils/eosfs/upload.go | 9 +- 13 files changed, 219 insertions(+), 12 deletions(-) create mode 100644 changelog/unreleased/disable-versioning.md create mode 100644 internal/http/services/owncloud/ocdav/put_test.go create mode 100644 pkg/eosclient/eosgrpc/eoshttp_test.go diff --git a/changelog/unreleased/disable-versioning.md b/changelog/unreleased/disable-versioning.md new file mode 100644 index 0000000000..b5f5496ef5 --- /dev/null +++ b/changelog/unreleased/disable-versioning.md @@ -0,0 +1,4 @@ +Enhancement: Add HTTP header to disable versioning on EOS + +This enhancement introduces a new header, `X-Disable-Versioning`, on PUT requests. EOS will not version this file save whenever this header is set with a truthy value. +See also: https://github.com/cs3org/reva/pull/4864 \ No newline at end of file diff --git a/go.mod b/go.mod index 43860145f6..5ceaef2561 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/coreos/go-oidc/v3 v3.9.0 github.com/creasty/defaults v1.7.0 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e - github.com/cs3org/go-cs3apis v0.0.0-20240802083356-d617314e1795 + github.com/cs3org/go-cs3apis v0.0.0-20240927085705-d50e291cbf4c github.com/dgraph-io/ristretto v0.1.1 github.com/dolthub/go-mysql-server v0.14.0 github.com/gdexlab/go-render v1.0.1 @@ -94,6 +94,7 @@ require ( github.com/hashicorp/golang-lru v1.0.2 // indirect github.com/huandu/xstrings v1.4.0 // indirect github.com/imdario/mergo v0.3.16 // indirect + github.com/jarcoal/httpmock v1.3.1 // indirect github.com/klauspost/compress v1.17.7 // indirect github.com/leodido/go-urn v1.4.0 // indirect github.com/lestrrat-go/strftime v1.0.4 // indirect diff --git a/go.sum b/go.sum index 6927c6794d..5cd08da11b 100644 --- a/go.sum +++ b/go.sum @@ -895,6 +895,10 @@ github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJff github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4= github.com/cs3org/go-cs3apis v0.0.0-20240802083356-d617314e1795 h1:8WkweBxMQ1W6IhcK0X3eWY+aQCjEktGwVt/4KLrtOZ8= github.com/cs3org/go-cs3apis v0.0.0-20240802083356-d617314e1795/go.mod h1:yyP8PRo0EZou3nSH7H4qjlzQwaydPeIRNgX50npQHpE= +github.com/cs3org/go-cs3apis v0.0.0-20240906084627-d1b1d7653d75 h1:Gcs8Y6T5/rLsUIq8vRymNbxDUpzHvqhVHT0i/LkrjAo= +github.com/cs3org/go-cs3apis v0.0.0-20240906084627-d1b1d7653d75/go.mod h1:yyP8PRo0EZou3nSH7H4qjlzQwaydPeIRNgX50npQHpE= +github.com/cs3org/go-cs3apis v0.0.0-20240927085705-d50e291cbf4c h1:91oR7NL5bBvwHj00a/1aTePzOBheIjeNGqBWzG6try0= +github.com/cs3org/go-cs3apis v0.0.0-20240927085705-d50e291cbf4c/go.mod h1:DedpcqXl193qF/08Y04IO0PpxyyMu8+GrkD6kWK2MEQ= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -1202,6 +1206,8 @@ github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4= github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/influxdata/influxdb1-client v0.0.0-20191209144304-8bf82d3c094d/go.mod h1:qj24IKcXYK6Iy9ceXlo3Tc+vtHo9lIhSX5JddghvEPo= +github.com/jarcoal/httpmock v1.3.1 h1:iUx3whfZWVf3jT01hQTO/Eo5sAYtB2/rqaUuOtpInww= +github.com/jarcoal/httpmock v1.3.1/go.mod h1:3yb8rc4BI7TCBhFY8ng0gjuLKJNquuDNiPaZjnENuYg= github.com/jedib0t/go-pretty v4.3.0+incompatible h1:CGs8AVhEKg/n9YbUenWmNStRW2PHJzaeDodcfvRAbIo= github.com/jedib0t/go-pretty v4.3.0+incompatible/go.mod h1:XemHduiw8R651AF9Pt4FwCTKeG3oo7hrHJAoznj9nag= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index e9a000b1bf..755b3a6e92 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -278,6 +278,14 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ httpReq.Header.Set(HeaderLockHolder, lockholder) } + // Propagate X-Disable-Versioning header + // Used to disable versioning for applications that do not expect this behaviour + // See reva#4855 for more info + disableVersioning, err := strconv.ParseBool(r.Header.Get(HeaderDisableVersioning)) + if err == nil && disableVersioning { + httpReq.Header.Set(HeaderDisableVersioning, strconv.FormatBool(true)) + } + httpRes, err := s.client.Do(httpReq) if err != nil { log.Error().Err(err).Msg("error doing PUT request to data service") diff --git a/internal/http/services/owncloud/ocdav/put_test.go b/internal/http/services/owncloud/ocdav/put_test.go new file mode 100644 index 0000000000..0fed79db25 --- /dev/null +++ b/internal/http/services/owncloud/ocdav/put_test.go @@ -0,0 +1,94 @@ +// Copyright 2018-2024 CERN +// +// 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. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package ocdav + +import ( + "context" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + mockgateway "github.com/cs3org/go-cs3apis/mocks/github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + "github.com/cs3org/reva/pkg/httpclient" + "github.com/cs3org/reva/pkg/rgrpc/todo/pool" + "github.com/rs/zerolog" + "github.com/stretchr/testify/mock" +) + +// Test that when calls come in to the PUT endpoint with a X-Disable-Versioning header, +// this header is propagated to the actual upload endpoint +func TestDisableVersioningHeaderPassedAlong(t *testing.T) { + + gatewayAPIEndpoint := "my-api-endpoint" + incomingPath := "http://my-reva.com/myfile.txt" + input := "Hello world!" + + // create HTTP request + request := httptest.NewRequest(http.MethodPut, incomingPath, strings.NewReader(input)) + request.Header.Add(HeaderContentLength, strconv.Itoa(len(input))) + request.Header.Add(HeaderDisableVersioning, "true") + + // Create fake HTTP server for upload endpoint + // Here we will check whether the header was correctly set + calls := 0 + w := httptest.NewRecorder() + mockServerUpload := httptest.NewServer( + http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + if header := r.Header.Get(HeaderDisableVersioning); header == "" { + t.Errorf("expected header %s but header was not set", HeaderDisableVersioning) + } + calls++ + }, + ), + ) + endpointPath := mockServerUpload.URL + + // Set up mocked GatewayAPIClient + gatewayClient := mockgateway.NewMockGatewayAPIClient(t) + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}}, nil) + gatewayClient.On("InitiateFileUpload", mock.Anything, mock.Anything).Return(&gateway.InitiateFileUploadResponse{ + Status: &rpc.Status{Code: rpc.Code_CODE_OK}, + Protocols: []*gateway.FileUploadProtocol{ + {Protocol: "simple", UploadEndpoint: endpointPath, Token: "my-secret-token"}, + }}, nil) + pool.RegisterGatewayServiceClient(gatewayClient, gatewayAPIEndpoint) + + // Set up OCDAV Service + service := svc{ + c: &Config{ + GatewaySvc: gatewayAPIEndpoint, + }, + client: httpclient.New(), + } + ref := provider.Reference{} + + // Do the actual call + service.handlePut(context.Background(), w, request, &ref, zerolog.Logger{}) + + // If no connection was made to the upload endpoint, something is also wrong + if calls == 0 { + t.Errorf("Upload endpoint was not called. ") + } +} diff --git a/internal/http/services/owncloud/ocdav/webdav.go b/internal/http/services/owncloud/ocdav/webdav.go index 06a1480156..c52f5e263a 100644 --- a/internal/http/services/owncloud/ocdav/webdav.go +++ b/internal/http/services/owncloud/ocdav/webdav.go @@ -78,6 +78,7 @@ const ( HeaderTransferAuth = "TransferHeaderAuthorization" HeaderLockID = "X-Lock-Id" HeaderLockHolder = "X-Lock-Holder" + HeaderDisableVersioning = "X-Disable-Versioning" ) // WebDavHandler implements a dav endpoint. diff --git a/pkg/eosclient/eosbinary/eosbinary.go b/pkg/eosclient/eosbinary/eosbinary.go index 10189b7daf..016e7eab5e 100644 --- a/pkg/eosclient/eosbinary/eosbinary.go +++ b/pkg/eosclient/eosbinary/eosbinary.go @@ -723,7 +723,7 @@ func (c *Client) Read(ctx context.Context, auth eosclient.Authorization, path st } // Write writes a stream to the mgm. -func (c *Client) Write(ctx context.Context, auth eosclient.Authorization, path string, stream io.ReadCloser, app string) error { +func (c *Client) Write(ctx context.Context, auth eosclient.Authorization, path string, stream io.ReadCloser, app string, disableVersioning bool) error { fd, err := os.CreateTemp(c.opt.CacheDirectory, "eoswrite-") if err != nil { return err @@ -736,19 +736,27 @@ func (c *Client) Write(ctx context.Context, auth eosclient.Authorization, path s if err != nil { return err } - return c.writeFile(ctx, auth, path, fd.Name(), app) + return c.writeFile(ctx, auth, path, fd.Name(), app, disableVersioning) } // WriteFile writes an existing file to the mgm. -func (c *Client) writeFile(ctx context.Context, auth eosclient.Authorization, path, source, app string) error { +func (c *Client) writeFile(ctx context.Context, auth eosclient.Authorization, path, source, app string, disableVersioning bool) error { xrdPath := fmt.Sprintf("%s//%s", c.opt.URL, path) args := []string{"--nopbar", "--silent", "-f", source, xrdPath} + options := fmt.Sprintf("-ODeos.app=%s", app) + if disableVersioning { + options += "&eos.versioning=0" + } + if auth.Token != "" { args[4] += "?authz=" + auth.Token } else if auth.Role.UID != "" && auth.Role.GID != "" { - args = append(args, fmt.Sprintf("-ODeos.ruid=%s&eos.rgid=%s&eos.app=%s", auth.Role.UID, auth.Role.GID, app)) + options += fmt.Sprintf("&eos.ruid=%s&eos.rgid=%s", auth.Role.UID, auth.Role.GID) + } else { + return errors.New("No authentication provided") } + args = append(args, options) _, _, err := c.executeXRDCopy(ctx, args) return err diff --git a/pkg/eosclient/eosclient.go b/pkg/eosclient/eosclient.go index bbbf31a428..b9a7b42929 100644 --- a/pkg/eosclient/eosclient.go +++ b/pkg/eosclient/eosclient.go @@ -51,7 +51,7 @@ type EOSClient interface { Rename(ctx context.Context, auth Authorization, oldPath, newPath string) error List(ctx context.Context, auth Authorization, path string) ([]*FileInfo, error) Read(ctx context.Context, auth Authorization, path string) (io.ReadCloser, error) - Write(ctx context.Context, auth Authorization, path string, stream io.ReadCloser, app string) error + Write(ctx context.Context, auth Authorization, path string, stream io.ReadCloser, app string, disableVersioning bool) error ListDeletedEntries(ctx context.Context, auth Authorization, maxentries int, from, to time.Time) ([]*DeletedEntry, error) RestoreDeletedEntry(ctx context.Context, auth Authorization, key string) error PurgeDeletedEntries(ctx context.Context, auth Authorization) error diff --git a/pkg/eosclient/eosgrpc/eosgrpc.go b/pkg/eosclient/eosgrpc/eosgrpc.go index 8a86c1b5cd..fc8b609536 100644 --- a/pkg/eosclient/eosgrpc/eosgrpc.go +++ b/pkg/eosclient/eosgrpc/eosgrpc.go @@ -1321,7 +1321,7 @@ func (c *Client) Read(ctx context.Context, auth eosclient.Authorization, path st // Write writes a file to the mgm // Somehow the same considerations as Read apply. -func (c *Client) Write(ctx context.Context, auth eosclient.Authorization, path string, stream io.ReadCloser, app string) error { +func (c *Client) Write(ctx context.Context, auth eosclient.Authorization, path string, stream io.ReadCloser, app string, disableVersioning bool) error { log := appctx.GetLogger(ctx) log.Info().Str("func", "Write").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Msg("") var length int64 @@ -1354,10 +1354,10 @@ func (c *Client) Write(ctx context.Context, auth eosclient.Authorization, path s defer wfd.Close() defer os.RemoveAll(fd.Name()) - return c.httpcl.PUTFile(ctx, u.Username, auth, path, wfd, length, app) + return c.httpcl.PUTFile(ctx, u.Username, auth, path, wfd, length, app, disableVersioning) } - return c.httpcl.PUTFile(ctx, u.Username, auth, path, stream, length, app) + return c.httpcl.PUTFile(ctx, u.Username, auth, path, stream, length, app, disableVersioning) } // ListDeletedEntries returns a list of the deleted entries. diff --git a/pkg/eosclient/eosgrpc/eoshttp.go b/pkg/eosclient/eosgrpc/eoshttp.go index e447afea13..8010259983 100644 --- a/pkg/eosclient/eosgrpc/eoshttp.go +++ b/pkg/eosclient/eosgrpc/eoshttp.go @@ -352,12 +352,27 @@ func (c *EOSHTTPClient) GETFile(ctx context.Context, remoteuser string, auth eos } // PUTFile does an entire PUT to upload a full file, taking the data from a stream. -func (c *EOSHTTPClient) PUTFile(ctx context.Context, remoteuser string, auth eosclient.Authorization, urlpath string, stream io.ReadCloser, length int64, app string) error { +func (c *EOSHTTPClient) PUTFile(ctx context.Context, remoteuser string, auth eosclient.Authorization, urlpath string, stream io.ReadCloser, length int64, app string, disableVersioning bool) error { log := appctx.GetLogger(ctx) log.Info().Str("func", "PUTFile").Str("remoteuser", remoteuser).Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", urlpath).Int64("length", length).Str("app", app).Msg("") // Now send the req and see what happens - finalurl, err := c.buildFullURL(urlpath, auth) + tempUrl, err := c.buildFullURL(urlpath, auth) + if err != nil { + return err + } + base, err := url.Parse(tempUrl) + if err != nil { + return errtypes.PermissionDenied("Could not parse url " + urlpath) + } + queryValues := base.Query() + + if disableVersioning { + queryValues.Add("eos.versioning", strconv.Itoa(0)) + } + base.RawQuery = queryValues.Encode() + finalurl := base.String() + if err != nil { log.Error().Str("func", "PUTFile").Str("url", finalurl).Str("err", err.Error()).Msg("can't create request") return err diff --git a/pkg/eosclient/eosgrpc/eoshttp_test.go b/pkg/eosclient/eosgrpc/eoshttp_test.go new file mode 100644 index 0000000000..1b72f63698 --- /dev/null +++ b/pkg/eosclient/eosgrpc/eoshttp_test.go @@ -0,0 +1,59 @@ +package eosgrpc + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/cs3org/reva/pkg/eosclient" +) + +// Test that, when the PUTFile method is called with disableVersioning +// set to true, the url for the EOS endpoint contains the right query param +func TestDisableVersioningLeadsToCorrectQueryParams(t *testing.T) { + + stream := io.NopCloser(strings.NewReader("Hello world!")) + length := int64(12) + app := "my-app" + urlpath := "/my-file.txt?queryparam=1" + token := "my-secret-token" + + // Create fake HTTP server that acts as the EOS endpoint + calls := 0 + mockServerUpload := httptest.NewServer( + http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + calls++ + queryValues := r.URL.Query() + if queryValues.Get("eos.versioning") == "" { + t.Errorf("Query parameter eos.versioning not set") + } + if q := queryValues.Get("eos.versioning"); q != strconv.Itoa(0) { + t.Errorf("Query parameter eos.versioning set to wrong value; got %s, expected 0", q) + } + }, + ), + ) + + // Create EOS HTTP Client + // TODO: right now, expects files to be on the FS + client, err := NewEOSHTTPClient(&HTTPOptions{ + BaseURL: mockServerUpload.URL, + }) + if err != nil { + t.Errorf("Failed to construct client: %s", err.Error()) + } + + // Test actual PUTFile call + client.PUTFile(context.Background(), "remote-user", eosclient.Authorization{ + Token: token}, urlpath, stream, length, app, true) + + // If no connection was made to the EOS endpoint, something is wrong + if calls == 0 { + t.Errorf("EOS endpoint was not called. ") + } +} diff --git a/pkg/rhttp/datatx/manager/simple/simple.go b/pkg/rhttp/datatx/manager/simple/simple.go index a776ba3f7b..1a03acaf4e 100644 --- a/pkg/rhttp/datatx/manager/simple/simple.go +++ b/pkg/rhttp/datatx/manager/simple/simple.go @@ -84,6 +84,10 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { metadata["lockholder"] = lockholder } + if disableVersioning := r.Header.Get(ocdav.HeaderDisableVersioning); disableVersioning != "" { + metadata["disableVersioning"] = disableVersioning + } + err := fs.Upload(ctx, ref, r.Body, metadata) switch v := err.(type) { case nil: diff --git a/pkg/storage/utils/eosfs/upload.go b/pkg/storage/utils/eosfs/upload.go index fb46c2699d..833d63591f 100644 --- a/pkg/storage/utils/eosfs/upload.go +++ b/pkg/storage/utils/eosfs/upload.go @@ -23,6 +23,7 @@ import ( "io" "os" "path" + "strconv" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/errtypes" @@ -86,7 +87,13 @@ func (fs *eosfs) Upload(ctx context.Context, ref *provider.Reference, r io.ReadC // if we have a lock context, the app for EOS must match the lock holder app = fs.EncodeAppName(app) } - return fs.c.Write(ctx, auth, fn, r, app) + + disableVersioning, err := strconv.ParseBool(metadata["disableVersioning"]) + if err != nil { + disableVersioning = false + } + + return fs.c.Write(ctx, auth, fn, r, app, disableVersioning) } func (fs *eosfs) InitiateUpload(ctx context.Context, ref *provider.Reference, uploadLength int64, metadata map[string]string) (map[string]string, error) {