Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: add option to disable versioning in EOS upon PUT request #4864

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1 +1 @@
* @labkode @glpatcern @diocas @jessejeens
* @labkode @glpatcern @diocas @jessegeens
4 changes: 4 additions & 0 deletions changelog/unreleased/disable-versioning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Enhancement: Add HTTP header to disable versioning on EOS

jessegeens marked this conversation as resolved.
Show resolved Hide resolved
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
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -95,6 +95,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
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
3 changes: 3 additions & 0 deletions internal/http/services/appprovider/appprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"encoding/json"
"net/http"
"path"
"strconv"

apppb "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1"
appregistry "github.com/cs3org/go-cs3apis/cs3/app/registry/v1beta1"
Expand All @@ -32,6 +33,7 @@ import (
storagepb "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/cs3org/reva/internal/http/services/datagateway"
"github.com/cs3org/reva/internal/http/services/owncloud/ocdav"
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/httpclient"
"github.com/cs3org/reva/pkg/rgrpc/status"
Expand Down Expand Up @@ -242,6 +244,7 @@ func (s *svc) handleNew(w http.ResponseWriter, r *http.Request) {
}

httpReq.Header.Set(datagateway.TokenTransportHeader, token)
httpReq.Header.Set(ocdav.HeaderContentLength, strconv.Itoa(0))

tr := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: s.conf.Insecure}}
httpRes, err := httpclient.New(httpclient.RoundTripper(tr)).Do(httpReq)
Expand Down
1 change: 1 addition & 0 deletions internal/http/services/datagateway/datagateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ func (s *svc) doPut(w http.ResponseWriter, r *http.Request) {

copyHeader(w.Header(), httpRes.Header)
if httpRes.StatusCode != http.StatusOK {
log.Warn().Int("StatusCode", httpRes.StatusCode).Msg("Non-OK Status Code when sending request to internal data server")
// swallow the body and set content-length to 0 to prevent reverse proxies from trying to read from it
w.Header().Set("Content-Length", "0")
w.WriteHeader(httpRes.StatusCode)
Expand Down
16 changes: 16 additions & 0 deletions internal/http/services/owncloud/ocdav/put.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,22 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ
httpReq.Header.Set(HeaderLockHolder, lockholder)
}

// We need to pass Content-Length to the storage backend (e.g. EOS).
// However, the Go HTTP Client library may arbitrarily modify or drop
// the Content-Length header, for example when it compresses the data
// See: https://pkg.go.dev/net/http#Request
// Therefore, we use another header to pass it through the internal
// data server
httpReq.Header.Set(HeaderUploadLength, strconv.FormatInt(length, 10))

// 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")
Expand Down
94 changes: 94 additions & 0 deletions internal/http/services/owncloud/ocdav/put_test.go
Original file line number Diff line number Diff line change
@@ -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. ")
}
}
1 change: 1 addition & 0 deletions internal/http/services/owncloud/ocdav/webdav.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const (
HeaderTransferAuth = "TransferHeaderAuthorization"
HeaderLockID = "X-Lock-Id"
HeaderLockHolder = "X-Lock-Holder"
HeaderDisableVersioning = "X-Disable-Versioning"
)

// WebDavHandler implements a dav endpoint.
Expand Down
16 changes: 12 additions & 4 deletions pkg/eosclient/eosbinary/eosbinary.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,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, length int64, app string, disableVersioning bool) error {
fd, err := os.CreateTemp(c.opt.CacheDirectory, "eoswrite-")
if err != nil {
return err
Expand All @@ -735,19 +735,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
Expand Down
2 changes: 1 addition & 1 deletion pkg/eosclient/eosclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, length int64, 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
Expand Down
8 changes: 3 additions & 5 deletions pkg/eosclient/eosgrpc/eosgrpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1364,11 +1364,9 @@ 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, length int64, 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
length = -1

u, err := getUser(ctx)
if err != nil {
Expand Down Expand Up @@ -1397,10 +1395,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.
Expand Down
28 changes: 16 additions & 12 deletions pkg/eosclient/eosgrpc/eoshttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,16 +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 {
log.Error().Str("func", "PUTFile").Str("url", finalurl).Str("err", err.Error()).Msg("can't create request")
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()

req, err := http.NewRequestWithContext(ctx, http.MethodPut, finalurl, nil)
if err != nil {
log.Error().Str("func", "PUTFile").Str("url", finalurl).Str("err", err.Error()).Msg("can't create request")
Expand Down Expand Up @@ -409,15 +420,8 @@ func (c *EOSHTTPClient) PUTFile(ctx context.Context, remoteuser string, auth eos
}
if length >= 0 {
log.Debug().Str("func", "PUTFile").Int64("Content-Length", length).Msg("setting header")
req.Header.Set("Content-Length", strconv.FormatInt(length, 10))
}
if err != nil {
log.Error().Str("func", "PUTFile").Str("url", loc.String()).Str("err", err.Error()).Msg("can't create redirected request")
return err
}
if length >= 0 {
log.Debug().Str("func", "PUTFile").Int64("Content-Length", length).Msg("setting header")
req.Header.Set("Content-Length", strconv.FormatInt(length, 10))
req.ContentLength = length
req.Header.Set("Content-Length", fmt.Sprintf("%d", length))
}

log.Debug().Str("func", "PUTFile").Str("location", loc.String()).Msg("redirection")
Expand Down
59 changes: 59 additions & 0 deletions pkg/eosclient/eosgrpc/eoshttp_test.go
Original file line number Diff line number Diff line change
@@ -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. ")
}
}
Loading
Loading