From a60ac7ca170e527f3481e7d0efbd62beb196960b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 3 Jan 2022 01:28:52 -0800 Subject: [PATCH] fix: audit log to support object names in multipleObjectNames() handler (#14017) --- Makefile | 2 +- cmd/api-datatypes.go | 9 ++++- cmd/bucket-handlers.go | 17 +++++++-- cmd/bucket-handlers_test.go | 17 ++++++++- cmd/bucket-quota.go | 6 ++- cmd/bucket-replication-utils.go | 6 ++- cmd/data-scanner.go | 6 ++- cmd/erasure-object_test.go | 6 ++- cmd/object-handlers.go | 13 +++++-- cmd/server_test.go | 4 +- cmd/utils.go | 16 ++++++++ docs/debugging/xl-meta/main.go | 3 +- internal/logger/audit.go | 7 ++++ internal/logger/logger.go | 32 +++++++--------- internal/logger/message/audit/entry.go | 25 +++++++----- internal/logger/message/log/entry.go | 14 +++++-- internal/logger/reqinfo.go | 46 +++++++++++++---------- internal/logger/target/console/console.go | 19 +++++++--- 18 files changed, 171 insertions(+), 77 deletions(-) diff --git a/Makefile b/Makefile index 3dc5e1025..2d41c522a 100644 --- a/Makefile +++ b/Makefile @@ -37,7 +37,7 @@ lint: ## runs golangci-lint suite of linters @echo "Running $@ check" @${GOPATH}/bin/golangci-lint cache clean @${GOPATH}/bin/golangci-lint run --build-tags kqueue --timeout=10m --config ./.golangci.yml - @${GOPATH}/bin/gofumpt -s -l . + @${GOPATH}/bin/gofumpt -l . check: test test: verifiers build ## builds minio, runs linters, tests diff --git a/cmd/api-datatypes.go b/cmd/api-datatypes.go index 3d4350e7a..b86b15a20 100644 --- a/cmd/api-datatypes.go +++ b/cmd/api-datatypes.go @@ -48,10 +48,15 @@ func (t DeleteMarkerMTime) MarshalXML(e *xml.Encoder, startElement xml.StartElem return e.EncodeElement(t.Time.Format(time.RFC3339), startElement) } -// ObjectToDelete carries key name for the object to delete. -type ObjectToDelete struct { +// ObjectV object version key/versionId +type ObjectV struct { ObjectName string `xml:"Key"` VersionID string `xml:"VersionId"` +} + +// ObjectToDelete carries key name for the object to delete. +type ObjectToDelete struct { + ObjectV // Replication status of DeleteMarker DeleteMarkerReplicationStatus string `xml:"DeleteMarkerReplicationStatus"` // Status of versioned delete (of object or DeleteMarker) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 5283a99bf..dffa3bc81 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -422,11 +422,16 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, return } + objects := make([]ObjectV, len(deleteObjectsReq.Objects)) // Convert object name delete objects if it has `/` in the beginning. for i := range deleteObjectsReq.Objects { deleteObjectsReq.Objects[i].ObjectName = trimLeadingSlash(deleteObjectsReq.Objects[i].ObjectName) + objects[i] = deleteObjectsReq.Objects[i].ObjectV } + // Make sure to update context to print ObjectNames for multi objects. + ctx = updateReqContext(ctx, objects...) + // Call checkRequestAuthType to populate ReqInfo.AccessKey before GetBucketInfo() // Ignore errors here to preserve the S3 error behavior of GetBucketInfo() checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, "") @@ -527,8 +532,10 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, if replicateDeletes { dsc = checkReplicateDelete(ctx, bucket, ObjectToDelete{ - ObjectName: object.ObjectName, - VersionID: object.VersionID, + ObjectV: ObjectV{ + ObjectName: object.ObjectName, + VersionID: object.VersionID, + }, }, goi, opts, gerr) if dsc.ReplicateAny() { if object.VersionID != "" { @@ -581,8 +588,10 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, // created during DeleteMarker creation when client didn't // specify a versionID. objToDel := ObjectToDelete{ - ObjectName: dObjects[i].ObjectName, - VersionID: dObjects[i].VersionID, + ObjectV: ObjectV{ + ObjectName: dObjects[i].ObjectName, + VersionID: dObjects[i].VersionID, + }, VersionPurgeStatus: dObjects[i].VersionPurgeStatus(), VersionPurgeStatuses: dObjects[i].ReplicationState.VersionPurgeStatusInternal, DeleteMarkerReplicationStatus: dObjects[i].ReplicationState.ReplicationStatusInternal, diff --git a/cmd/bucket-handlers_test.go b/cmd/bucket-handlers_test.go index f0e85d883..2103896f1 100644 --- a/cmd/bucket-handlers_test.go +++ b/cmd/bucket-handlers_test.go @@ -698,7 +698,9 @@ func testAPIDeleteMultipleObjectsHandler(obj ObjectLayer, instanceType, bucketNa getObjectToDeleteList := func(objectNames []string) (objectList []ObjectToDelete) { for _, objectName := range objectNames { objectList = append(objectList, ObjectToDelete{ - ObjectName: objectName, + ObjectV: ObjectV{ + ObjectName: objectName, + }, }) } @@ -717,10 +719,21 @@ func testAPIDeleteMultipleObjectsHandler(obj ObjectLayer, instanceType, bucketNa return deleteErrorList } + objects := []ObjectToDelete{} + objects = append(objects, ObjectToDelete{ + ObjectV: ObjectV{ + ObjectName: "private/object", + }, + }) + objects = append(objects, ObjectToDelete{ + ObjectV: ObjectV{ + ObjectName: "public/object", + }, + }) requestList := []DeleteObjectsRequest{ {Quiet: false, Objects: getObjectToDeleteList(objectNames[:5])}, {Quiet: true, Objects: getObjectToDeleteList(objectNames[5:])}, - {Quiet: false, Objects: []ObjectToDelete{{ObjectName: "private/object"}, {ObjectName: "public/object"}}}, + {Quiet: false, Objects: objects}, } // generate multi objects delete response. diff --git a/cmd/bucket-quota.go b/cmd/bucket-quota.go index 99317a036..67780ad3c 100644 --- a/cmd/bucket-quota.go +++ b/cmd/bucket-quota.go @@ -193,8 +193,10 @@ func enforceFIFOQuotaBucket(ctx context.Context, objectAPI ObjectLayer, bucket s numKeys := len(scorer.fileObjInfos()) for i, obj := range scorer.fileObjInfos() { objects = append(objects, ObjectToDelete{ - ObjectName: obj.Name, - VersionID: obj.VersionID, + ObjectV: ObjectV{ + ObjectName: obj.Name, + VersionID: obj.VersionID, + }, }) if len(objects) < maxDeleteList && (i < numKeys-1) { // skip deletion until maxDeleteList or end of slice diff --git a/cmd/bucket-replication-utils.go b/cmd/bucket-replication-utils.go index b3f71ca91..0c7de5a8a 100644 --- a/cmd/bucket-replication-utils.go +++ b/cmd/bucket-replication-utils.go @@ -502,8 +502,10 @@ func getHealReplicateObjectInfo(objInfo ObjectInfo, rcfg replicationConfig) Repl var tgtStatuses map[string]replication.StatusType if oi.DeleteMarker || !oi.VersionPurgeStatus.Empty() { dsc = checkReplicateDelete(GlobalContext, oi.Bucket, ObjectToDelete{ - ObjectName: oi.Name, - VersionID: oi.VersionID, + ObjectV: ObjectV{ + ObjectName: oi.Name, + VersionID: oi.VersionID, + }, }, oi, ObjectOptions{}, nil) } else { dsc = mustReplicate(GlobalContext, oi.Bucket, oi.Name, getMustReplicateOptions(ObjectInfo{ diff --git a/cmd/data-scanner.go b/cmd/data-scanner.go index 4d5c7ea1e..b71fb6741 100644 --- a/cmd/data-scanner.go +++ b/cmd/data-scanner.go @@ -1022,8 +1022,10 @@ func (i *scannerItem) applyNewerNoncurrentVersionLimit(ctx context.Context, _ Ob } toDel = append(toDel, ObjectToDelete{ - ObjectName: fi.Name, - VersionID: fi.VersionID, + ObjectV: ObjectV{ + ObjectName: fi.Name, + VersionID: fi.VersionID, + }, }) } diff --git a/cmd/erasure-object_test.go b/cmd/erasure-object_test.go index a55bc1b0e..3d83ef8ab 100644 --- a/cmd/erasure-object_test.go +++ b/cmd/erasure-object_test.go @@ -177,7 +177,11 @@ func TestErasureDeleteObjectsErasureSet(t *testing.T) { toObjectNames := func(testCases []testCaseType) []ObjectToDelete { names := make([]ObjectToDelete, len(testCases)) for i := range testCases { - names[i] = ObjectToDelete{ObjectName: testCases[i].object} + names[i] = ObjectToDelete{ + ObjectV: ObjectV{ + ObjectName: testCases[i].object, + }, + } } return names } diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 06d1401cf..e2b6a62c7 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -3387,7 +3387,12 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http. os.SetTransitionState(goi.TransitionedObject) } - dsc := checkReplicateDelete(ctx, bucket, ObjectToDelete{ObjectName: object, VersionID: opts.VersionID}, goi, opts, gerr) + dsc := checkReplicateDelete(ctx, bucket, ObjectToDelete{ + ObjectV: ObjectV{ + ObjectName: object, + VersionID: opts.VersionID, + }, + }, goi, opts, gerr) if dsc.ReplicateAny() { opts.SetDeleteReplicationState(dsc, opts.VersionID) } @@ -3414,8 +3419,10 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http. } if vID != "" { apiErr = enforceRetentionBypassForDelete(ctx, r, bucket, ObjectToDelete{ - ObjectName: object, - VersionID: vID, + ObjectV: ObjectV{ + ObjectName: object, + VersionID: vID, + }, }, goi, gerr) if apiErr != ErrNone && apiErr != ErrNoSuchKey { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(apiErr), r.URL) diff --git a/cmd/server_test.go b/cmd/server_test.go index 8bdc48055..9cf0cb115 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -574,7 +574,9 @@ func (s *TestSuiteCommon) TestDeleteMultipleObjects(c *check) { c.Assert(response.StatusCode, http.StatusOK) // Append all objects. delObjReq.Objects = append(delObjReq.Objects, ObjectToDelete{ - ObjectName: objName, + ObjectV: ObjectV{ + ObjectName: objName, + }, }) } // Marshal delete request. diff --git a/cmd/utils.go b/cmd/utils.go index 07fa6cc18..e3d72e5b5 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -752,6 +752,21 @@ func likelyUnescapeGeneric(p string, escapeFn func(string) (string, error)) stri return ep } +func updateReqContext(ctx context.Context, objects ...ObjectV) context.Context { + req := logger.GetReqInfo(ctx) + if req != nil { + req.Objects = make([]logger.ObjectVersion, 0, len(objects)) + for _, ov := range objects { + req.Objects = append(req.Objects, logger.ObjectVersion{ + ObjectName: ov.ObjectName, + VersionID: ov.VersionID, + }) + } + return logger.SetReqInfo(ctx, req) + } + return ctx +} + // Returns context with ReqInfo details set in the context. func newContext(r *http.Request, w http.ResponseWriter, api string) context.Context { vars := mux.Vars(r) @@ -770,6 +785,7 @@ func newContext(r *http.Request, w http.ResponseWriter, api string) context.Cont API: api, BucketName: bucket, ObjectName: object, + VersionID: strings.TrimSpace(r.Form.Get(xhttp.VersionID)), } return logger.SetReqInfo(r.Context(), reqInfo) } diff --git a/docs/debugging/xl-meta/main.go b/docs/debugging/xl-meta/main.go index ba2d65fdb..d72e0c997 100644 --- a/docs/debugging/xl-meta/main.go +++ b/docs/debugging/xl-meta/main.go @@ -131,7 +131,7 @@ FLAGS: Header json.RawMessage Metadata json.RawMessage } - var versions = make([]version, nVers) + versions := make([]version, nVers) err = decodeVersions(v, nVers, func(idx int, hdr, meta []byte) error { var header xlMetaV2VersionHeaderV2 if _, err := header.UnmarshalMsg(hdr); err != nil { @@ -462,7 +462,6 @@ func (x xlMetaInlineData) files(fn func(name string, data []byte)) error { fn(string(key), val) } return nil - } const ( diff --git a/internal/logger/audit.go b/internal/logger/audit.go index 3260cf7f0..36c44f68e 100644 --- a/internal/logger/audit.go +++ b/internal/logger/audit.go @@ -208,6 +208,13 @@ func AuditLog(ctx context.Context, w http.ResponseWriter, r *http.Request, reqCl entry.API.Name = reqInfo.API entry.API.Bucket = reqInfo.BucketName entry.API.Object = reqInfo.ObjectName + entry.API.Objects = make([]audit.ObjectVersion, 0, len(reqInfo.Objects)) + for _, ov := range reqInfo.Objects { + entry.API.Objects = append(entry.API.Objects, audit.ObjectVersion{ + ObjectName: ov.ObjectName, + VersionID: ov.VersionID, + }) + } entry.API.Status = http.StatusText(statusCode) entry.API.StatusCode = statusCode entry.API.InputBytes = r.ContentLength diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 39c6d1714..89f685851 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -62,23 +62,6 @@ var matchingFuncNames = [...]string{ "http.HandlerFunc.ServeHTTP", "cmd.serverMain", "cmd.StartGateway", - "cmd.(*webAPIHandlers).ListBuckets", - "cmd.(*webAPIHandlers).MakeBucket", - "cmd.(*webAPIHandlers).DeleteBucket", - "cmd.(*webAPIHandlers).ListObjects", - "cmd.(*webAPIHandlers).RemoveObject", - "cmd.(*webAPIHandlers).Login", - "cmd.(*webAPIHandlers).SetAuth", - "cmd.(*webAPIHandlers).CreateURLToken", - "cmd.(*webAPIHandlers).Upload", - "cmd.(*webAPIHandlers).Download", - "cmd.(*webAPIHandlers).DownloadZip", - "cmd.(*webAPIHandlers).GetBucketPolicy", - "cmd.(*webAPIHandlers).ListAllBucketPolicies", - "cmd.(*webAPIHandlers).SetBucketPolicy", - "cmd.(*webAPIHandlers).PresignedGet", - "cmd.(*webAPIHandlers).ServerInfo", - "cmd.(*webAPIHandlers).StorageInfo", // add more here .. } @@ -338,6 +321,15 @@ func logIf(ctx context.Context, err error, errKind ...interface{}) { if req.DeploymentID == "" { req.DeploymentID = globalDeploymentID } + + objects := make([]log.ObjectVersion, 0, len(req.Objects)) + for _, ov := range req.Objects { + objects = append(objects, log.ObjectVersion{ + ObjectName: ov.ObjectName, + VersionID: ov.VersionID, + }) + } + entry := log.Entry{ DeploymentID: req.DeploymentID, Level: ErrorLvl.String(), @@ -350,8 +342,10 @@ func logIf(ctx context.Context, err error, errKind ...interface{}) { API: &log.API{ Name: API, Args: &log.Args{ - Bucket: req.BucketName, - Object: req.ObjectName, + Bucket: req.BucketName, + Object: req.ObjectName, + VersionID: req.VersionID, + Objects: objects, }, }, Trace: &log.Trace{ diff --git a/internal/logger/message/audit/entry.go b/internal/logger/message/audit/entry.go index fcc52c394..54ac3bcff 100644 --- a/internal/logger/message/audit/entry.go +++ b/internal/logger/message/audit/entry.go @@ -29,6 +29,12 @@ import ( // Version - represents the current version of audit log structure. const Version = "1" +// ObjectVersion object version key/versionId +type ObjectVersion struct { + ObjectName string `json:"objectName"` + VersionID string `json:"VersionId,omitempty"` +} + // Entry - audit entry logs. type Entry struct { Version string `json:"version"` @@ -36,15 +42,16 @@ type Entry struct { Time time.Time `json:"time"` Trigger string `json:"trigger"` API struct { - Name string `json:"name,omitempty"` - Bucket string `json:"bucket,omitempty"` - Object string `json:"object,omitempty"` - Status string `json:"status,omitempty"` - StatusCode int `json:"statusCode,omitempty"` - InputBytes int64 `json:"rx"` - OutputBytes int64 `json:"tx"` - TimeToFirstByte string `json:"timeToFirstByte,omitempty"` - TimeToResponse string `json:"timeToResponse,omitempty"` + Name string `json:"name,omitempty"` + Bucket string `json:"bucket,omitempty"` + Object string `json:"object,omitempty"` + Objects []ObjectVersion `json:"objects,omitempty"` + Status string `json:"status,omitempty"` + StatusCode int `json:"statusCode,omitempty"` + InputBytes int64 `json:"rx"` + OutputBytes int64 `json:"tx"` + TimeToFirstByte string `json:"timeToFirstByte,omitempty"` + TimeToResponse string `json:"timeToResponse,omitempty"` } `json:"api"` RemoteHost string `json:"remotehost,omitempty"` RequestID string `json:"requestID,omitempty"` diff --git a/internal/logger/message/log/entry.go b/internal/logger/message/log/entry.go index cb872e7b2..a8cd95262 100644 --- a/internal/logger/message/log/entry.go +++ b/internal/logger/message/log/entry.go @@ -22,11 +22,19 @@ import ( "time" ) +// ObjectVersion object version key/versionId +type ObjectVersion struct { + ObjectName string `json:"objectName"` + VersionID string `json:"VersionId,omitempty"` +} + // Args - defines the arguments for the API. type Args struct { - Bucket string `json:"bucket,omitempty"` - Object string `json:"object,omitempty"` - Metadata map[string]string `json:"metadata,omitempty"` + Bucket string `json:"bucket,omitempty"` + Object string `json:"object,omitempty"` + VersionID string `json:"versionId,omitempty"` + Objects []ObjectVersion `json:"objects,omitempty"` + Metadata map[string]string `json:"metadata,omitempty"` } // Trace - defines the trace. diff --git a/internal/logger/reqinfo.go b/internal/logger/reqinfo.go index 56e8c3ea8..c2b62ae26 100644 --- a/internal/logger/reqinfo.go +++ b/internal/logger/reqinfo.go @@ -34,32 +34,40 @@ type KeyVal struct { Val interface{} } +// ObjectVersion object version key/versionId +type ObjectVersion struct { + ObjectName string + VersionID string `json:"VersionId,omitempty"` +} + // ReqInfo stores the request info. type ReqInfo struct { - RemoteHost string // Client Host/IP - Host string // Node Host/IP - UserAgent string // User Agent - DeploymentID string // x-minio-deployment-id - RequestID string // x-amz-request-id - API string // API name - GetObject PutObject NewMultipartUpload etc. - BucketName string // Bucket name - ObjectName string // Object name - AccessKey string // Access Key - tags []KeyVal // Any additional info not accommodated by above fields + RemoteHost string // Client Host/IP + Host string // Node Host/IP + UserAgent string // User Agent + DeploymentID string // x-minio-deployment-id + RequestID string // x-amz-request-id + API string // API name - GetObject PutObject NewMultipartUpload etc. + BucketName string `json:",omitempty"` // Bucket name + ObjectName string `json:",omitempty"` // Object name + VersionID string `json:",omitempty"` // corresponding versionID for the object + Objects []ObjectVersion `json:",omitempty"` // Only set during MultiObject delete handler. + AccessKey string // Access Key + tags []KeyVal // Any additional info not accommodated by above fields sync.RWMutex } // NewReqInfo : func NewReqInfo(remoteHost, userAgent, deploymentID, requestID, api, bucket, object string) *ReqInfo { - req := ReqInfo{} - req.RemoteHost = remoteHost - req.UserAgent = userAgent - req.API = api - req.DeploymentID = deploymentID - req.RequestID = requestID - req.BucketName = bucket - req.ObjectName = object - return &req + return &ReqInfo{ + RemoteHost: remoteHost, + UserAgent: userAgent, + API: api, + DeploymentID: deploymentID, + RequestID: requestID, + BucketName: bucket, + ObjectName: object, + } } // AppendTags - appends key/val to ReqInfo.tags diff --git a/internal/logger/target/console/console.go b/internal/logger/target/console/console.go index 9aec6de34..69dd55ffe 100644 --- a/internal/logger/target/console/console.go +++ b/internal/logger/target/console/console.go @@ -20,6 +20,7 @@ package console import ( "encoding/json" "fmt" + "strconv" "strings" "github.com/minio/minio/internal/color" @@ -83,11 +84,19 @@ func (c *Target) Send(e interface{}, logKind string) error { var apiString string if entry.API != nil { apiString = "API: " + entry.API.Name + "(" - if entry.API.Args != nil && entry.API.Args.Bucket != "" { - apiString = apiString + "bucket=" + entry.API.Args.Bucket - } - if entry.API.Args != nil && entry.API.Args.Object != "" { - apiString = apiString + ", object=" + entry.API.Args.Object + if entry.API.Args != nil { + if entry.API.Args.Bucket != "" { + apiString = apiString + "bucket=" + entry.API.Args.Bucket + } + if entry.API.Args.Object != "" { + apiString = apiString + ", object=" + entry.API.Args.Object + } + if entry.API.Args.VersionID != "" { + apiString = apiString + ", versionId=" + entry.API.Args.VersionID + } + if len(entry.API.Args.Objects) > 0 { + apiString = apiString + ", multiObject=true, numberOfObjects=" + strconv.Itoa(len(entry.API.Args.Objects)) + } } apiString += ")" } else {