From afce681f775b7fde5015db00103e10cad0f10100 Mon Sep 17 00:00:00 2001 From: Roman Loginov Date: Thu, 12 Dec 2024 09:28:22 +0300 Subject: [PATCH] [#174] Add kludge additional search Advanced search is needed because some software may keep FileName attribute and ignore FilePath attribute during file upload. Signed-off-by: Roman Loginov --- cmd/http-gw/app.go | 9 +++++ cmd/http-gw/settings.go | 3 ++ config/config.env | 5 ++- config/config.yaml | 4 ++ docs/gate-configuration.md | 15 +++++++- internal/handler/browse.go | 1 + internal/handler/handler.go | 52 ++++++++++++++++++------- internal/handler/handler_test.go | 65 ++++++++++++++++++++++++++++++++ internal/logs/logs.go | 1 + 9 files changed, 139 insertions(+), 16 deletions(-) diff --git a/cmd/http-gw/app.go b/cmd/http-gw/app.go index 53c001c..38b6a7d 100644 --- a/cmd/http-gw/app.go +++ b/cmd/http-gw/app.go @@ -110,6 +110,7 @@ type ( corsExposeHeaders []string corsAllowCredentials bool corsMaxAge int + additionalSearch bool } CORS struct { @@ -189,6 +190,7 @@ func (s *appSettings) update(v *viper.Viper, l *zap.Logger) { corsExposeHeaders := v.GetStringSlice(cfgCORSExposeHeaders) corsAllowCredentials := v.GetBool(cfgCORSAllowCredentials) corsMaxAge := fetchCORSMaxAge(v) + additionalSearch := v.GetBool(cfgKludgeAdditionalSearch) s.mu.Lock() defer s.mu.Unlock() @@ -208,6 +210,7 @@ func (s *appSettings) update(v *viper.Viper, l *zap.Logger) { s.corsExposeHeaders = corsExposeHeaders s.corsAllowCredentials = corsAllowCredentials s.corsMaxAge = corsMaxAge + s.additionalSearch = additionalSearch } func (s *loggerSettings) DroppedLogsInc() { @@ -305,6 +308,12 @@ func (s *appSettings) FormContainerZone(ns string) (zone string, isDefault bool) return ns + ".ns", false } +func (s *appSettings) AdditionalSearch() bool { + s.mu.RLock() + defer s.mu.RUnlock() + return s.additionalSearch +} + func (a *app) initResolver() { var err error a.resolver, err = resolver.NewContainerResolver(a.getResolverConfig()) diff --git a/cmd/http-gw/settings.go b/cmd/http-gw/settings.go index 2298124..cb0e596 100644 --- a/cmd/http-gw/settings.go +++ b/cmd/http-gw/settings.go @@ -164,6 +164,9 @@ const ( cfgMultinetFallbackDelay = "multinet.fallback_delay" cfgMultinetSubnets = "multinet.subnets" + // Kludge. + cfgKludgeAdditionalSearch = "kludge.additional_search" + // Command line args. cmdHelp = "help" cmdVersion = "version" diff --git a/config/config.env b/config/config.env index fd51392..241bc77 100644 --- a/config/config.env +++ b/config/config.env @@ -158,4 +158,7 @@ HTTP_GW_WORKER_POOL_SIZE=1000 # Enable index page support HTTP_GW_INDEX_PAGE_ENABLED=false # Index page template path -HTTP_GW_INDEX_PAGE_TEMPLATE_PATH=internal/handler/templates/index.gotmpl \ No newline at end of file +HTTP_GW_INDEX_PAGE_TEMPLATE_PATH=internal/handler/templates/index.gotmpl + +# Enable using additional search by attribute +HTTP_GW_KLUDGE_ADDITIONAL_SEARCH=false diff --git a/config/config.yaml b/config/config.yaml index ef5c529..e293c14 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -172,3 +172,7 @@ multinet: source_ips: - 1.2.3.4 - 1.2.3.5 + +kludge: + # Enable using additional search by attribute + additional_search: false diff --git a/docs/gate-configuration.md b/docs/gate-configuration.md index c6cb617..038e3d3 100644 --- a/docs/gate-configuration.md +++ b/docs/gate-configuration.md @@ -59,7 +59,7 @@ $ cat http.log | `resolve_bucket` | [Bucket name resolving configuration](#resolve_bucket-section) | | `index_page` | [Index page configuration](#index_page-section) | | `multinet` | [Multinet configuration](#multinet-section) | - +| `kludge` | [Kludge configuration](#kludge-section) | # General section @@ -457,3 +457,16 @@ multinet: |--------------|------------|---------------|---------------|----------------------------------------------------------------------| | `mask` | `string` | yes | | Destination subnet. | | `source_ips` | `[]string` | yes | | Array of source IP addresses to use when dialing destination subnet. | + +# `kludge` section + +Workarounds for non-standard use cases. + +```yaml +kludge: + additional_search: true +``` + +| Parameter | Type | SIGHUP reload | Default value | Description | +|----------------------------|--------|---------------|---------------| ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `kludge.additional_search` | `bool` | yes | `false` | Enable using additional search by attribute. If the value of the `FilePath` attribute in the request contains no `/` symbols or single leading `/` symbol and the object was not found, then an attempt is made to search for the object by the attribute `FileName`. | diff --git a/internal/handler/browse.go b/internal/handler/browse.go index b24a569..c54ab76 100644 --- a/internal/handler/browse.go +++ b/internal/handler/browse.go @@ -26,6 +26,7 @@ const ( attrOID = "OID" attrCreated = "Created" attrFileName = "FileName" + attrFilePath = "FilePath" attrSize = "Size" ) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 9ed7f99..11b4329 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -35,6 +35,7 @@ type Config interface { IndexPageTemplate() string BufferMaxSizeForPut() uint64 NamespaceHeader() string + AdditionalSearch() bool } // PrmContainer groups parameters of FrostFS.Container operation. @@ -291,35 +292,58 @@ func (h *Handler) byAttribute(c *fasthttp.RequestCtx, f func(context.Context, re return } - res, err := h.search(ctx, bktInfo.CID, key, val, object.MatchStringEqual) + objID, err := h.findObjectByAttribute(ctx, log, bktInfo.CID, key, val) if err != nil { - log.Error(logs.CouldNotSearchForObjects, zap.Error(err)) - response.Error(c, "could not search for objects: "+err.Error(), fasthttp.StatusBadRequest) + if errors.Is(err, io.EOF) { + response.Error(c, err.Error(), fasthttp.StatusNotFound) + return + } + + response.Error(c, err.Error(), fasthttp.StatusBadRequest) return } + var addrObj oid.Address + addrObj.SetContainer(bktInfo.CID) + addrObj.SetObject(objID) + + f(ctx, *h.newRequest(c, log), addrObj) +} + +func (h *Handler) findObjectByAttribute(ctx context.Context, log *zap.Logger, cnrID cid.ID, attrKey, attrVal string) (oid.ID, error) { + res, err := h.search(ctx, cnrID, attrKey, attrVal, object.MatchStringEqual) + if err != nil { + log.Error(logs.CouldNotSearchForObjects, zap.Error(err)) + return oid.ID{}, fmt.Errorf("could not search for objects: %w", err) + } defer res.Close() buf := make([]oid.ID, 1) n, err := res.Read(buf) if n == 0 { - if errors.Is(err, io.EOF) { + switch { + case errors.Is(err, io.EOF) && h.needSearchByFileName(attrKey, attrVal): + log.Warn(logs.WarnObjectNotFoundByFilePathTrySearchByFileName) + return h.findObjectByAttribute(ctx, log, cnrID, attrFileName, attrVal) + case errors.Is(err, io.EOF): log.Error(logs.ObjectNotFound, zap.Error(err)) - response.Error(c, "object not found", fasthttp.StatusNotFound) - return + return oid.ID{}, fmt.Errorf("object not found: %w", err) + default: + log.Error(logs.ReadObjectListFailed, zap.Error(err)) + return oid.ID{}, fmt.Errorf("read object list failed: %w", err) } - - log.Error(logs.ReadObjectListFailed, zap.Error(err)) - response.Error(c, "read object list failed: "+err.Error(), fasthttp.StatusBadRequest) - return } - var addrObj oid.Address - addrObj.SetContainer(bktInfo.CID) - addrObj.SetObject(buf[0]) + return buf[0], nil +} - f(ctx, *h.newRequest(c, log), addrObj) +func (h *Handler) needSearchByFileName(key, val string) bool { + if key != attrFilePath || !h.config.AdditionalSearch() { + return false + } + + return strings.HasPrefix(val, "/") && strings.Count(val, "/") == 1 || !strings.ContainsRune(val, '/') } // resolveContainer decode container id, if it's not a valid container id diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index 34668a5..4044907 100644 --- a/internal/handler/handler_test.go +++ b/internal/handler/handler_test.go @@ -44,6 +44,7 @@ func (t *treeClientMock) GetSubTree(context.Context, *data.BucketInfo, string, [ } type configMock struct { + additionalSearch bool } func (c *configMock) DefaultTimestamp() bool { @@ -78,6 +79,10 @@ func (c *configMock) NamespaceHeader() string { return "" } +func (c *configMock) AdditionalSearch() bool { + return c.additionalSearch +} + type handlerContext struct { key *keys.PrivateKey owner user.ID @@ -250,6 +255,66 @@ func TestBasic(t *testing.T) { require.Equal(t, content, string(data)) }) } +func TestNeedSearchByFileName(t *testing.T) { + hc, err := prepareHandlerContext() + require.NoError(t, err) + hc.cfg.additionalSearch = true + + for _, tc := range []struct { + name string + attrKey string + attrVal string + additionalSearchDisabled bool + expected bool + }{ + { + name: "need search - not contains slash", + attrKey: attrFilePath, + attrVal: "cat.png", + expected: true, + }, + { + name: "need search - single lead slash", + attrKey: attrFilePath, + attrVal: "/cat.png", + expected: true, + }, + { + name: "don't need search - single slash but not lead", + attrKey: attrFilePath, + attrVal: "cats/cat.png", + expected: false, + }, + { + name: "don't need search - more one slash", + attrKey: attrFilePath, + attrVal: "/cats/cat.png", + expected: false, + }, + { + name: "don't need search - incorrect attribute key", + attrKey: attrFileName, + attrVal: "cat.png", + expected: false, + }, + { + name: "don't need search - additional search disabled", + attrKey: attrFilePath, + attrVal: "cat.png", + additionalSearchDisabled: true, + expected: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.additionalSearchDisabled { + hc.cfg.additionalSearch = false + } + + res := hc.h.needSearchByFileName(tc.attrKey, tc.attrVal) + require.Equal(t, tc.expected, res) + }) + } +} func prepareUploadRequest(ctx context.Context, bucket, content string) (*fasthttp.RequestCtx, error) { r := new(fasthttp.RequestCtx) diff --git a/internal/logs/logs.go b/internal/logs/logs.go index 4dfa21f..7074172 100644 --- a/internal/logs/logs.go +++ b/internal/logs/logs.go @@ -87,4 +87,5 @@ const ( MultinetDialFail = "multinet dial failed" FailedToLoadMultinetConfig = "failed to load multinet config" MultinetConfigWontBeUpdated = "multinet config won't be updated" + WarnObjectNotFoundByFilePathTrySearchByFileName = "object not found by filePath attribute, try search by fileName" )