From 2c4e3ad46bc743773480092f9624d1446387959d Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Tue, 15 Oct 2024 08:15:39 -0700 Subject: [PATCH] Report errors and better file names for mc support inspect (#5062) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check the returned stream for errors and show them. Don't upload/keep pointless data Use the request alias to create a better file name... Example: ``` λ mc support inspect play/testbucket/testdat/** mc: Unable to download file data. GetRawData: No files matched the given pattern. ``` Previously the error would just be ignored. ``` λ mc support inspect --airgap play/testbucket/testdata/** File data successfully downloaded as inspect-play_testbucket_testdata.enc ``` Previously all files would be called `inspect-data.enc`, which would just be difficult to distinguish. --- cmd/support-inspect.go | 80 ++++++++++++++++++++++++++++++++++-------- cmd/utils.go | 18 ++++++++++ 2 files changed, 83 insertions(+), 15 deletions(-) diff --git a/cmd/support-inspect.go b/cmd/support-inspect.go index 51cb0025aa..8ea83caf70 100644 --- a/cmd/support-inspect.go +++ b/cmd/support-inspect.go @@ -22,6 +22,7 @@ import ( "encoding/base64" "encoding/binary" "encoding/hex" + "errors" "fmt" "hash/crc32" "io" @@ -29,19 +30,20 @@ import ( "path/filepath" "runtime" "strings" + "sync" "time" "github.com/fatih/color" "github.com/minio/cli" json "github.com/minio/colorjson" "github.com/minio/madmin-go/v3" + "github.com/minio/madmin-go/v3/estream" "github.com/minio/mc/pkg/probe" "github.com/minio/pkg/v3/console" ) const ( - defaultPublicKey = "MIIBCgKCAQEAs/128UFS9A8YSJY1XqYKt06dLVQQCGDee69T+0Tip/1jGAB4z0/3QMpH0MiS8Wjs4BRWV51qvkfAHzwwdU7y6jxU05ctb/H/WzRj3FYdhhHKdzear9TLJftlTs+xwj2XaADjbLXCV1jGLS889A7f7z5DgABlVZMQd9BjVAR8ED3xRJ2/ZCNuQVJ+A8r7TYPGMY3wWvhhPgPk3Lx4WDZxDiDNlFs4GQSaESSsiVTb9vyGe/94CsCTM6Cw9QG6ifHKCa/rFszPYdKCabAfHcS3eTr0GM+TThSsxO7KfuscbmLJkfQev1srfL2Ii2RbnysqIJVWKEwdW05ID8ryPkuTuwIDAQAB" - inspectOutputFilename = "inspect-data.enc" + defaultPublicKey = "MIIBCgKCAQEAs/128UFS9A8YSJY1XqYKt06dLVQQCGDee69T+0Tip/1jGAB4z0/3QMpH0MiS8Wjs4BRWV51qvkfAHzwwdU7y6jxU05ctb/H/WzRj3FYdhhHKdzear9TLJftlTs+xwj2XaADjbLXCV1jGLS889A7f7z5DgABlVZMQd9BjVAR8ED3xRJ2/ZCNuQVJ+A8r7TYPGMY3wWvhhPgPk3Lx4WDZxDiDNlFs4GQSaESSsiVTb9vyGe/94CsCTM6Cw9QG6ifHKCa/rFszPYdKCabAfHcS3eTr0GM+TThSsxO7KfuscbmLJkfQev1srfL2Ii2RbnysqIJVWKEwdW05ID8ryPkuTuwIDAQAB" ) var supportInspectFlags = append(subnetCommonFlags, @@ -186,13 +188,62 @@ func mainSupportInspect(ctx *cli.Context) error { // Download the inspect data in a temporary file first tmpFile, e := os.CreateTemp("", "mc-inspect-") fatalIf(probe.NewError(e), "Unable to download file data.") - _, e = io.Copy(tmpFile, r) + copied := false + + // Validate stream, report errors on stream. + if len(key) == 0 { + pr, pw := io.Pipe() + copied = true + var aErr error + var wg sync.WaitGroup + wg.Add(1) + // Validate stream... + go func() { + defer wg.Done() + er, err := estream.NewReader(pr) + if err != nil { + // Ignore if header is non-parsable... + io.Copy(io.Discard, pr) + return + } + // We don't care about decrypting at this point. + er.SkipEncrypted(true) + for { + var st *estream.Stream + st, aErr = er.NextStream() + if errors.Is(aErr, io.EOF) { + aErr = nil + pr.CloseWithError(nil) + return + } + if aErr != nil { + pr.CloseWithError(aErr) + return + } + aErr = st.Skip() + if aErr != nil { + fmt.Println("Skip:", aErr) + pr.CloseWithError(aErr) + return + } + } + }() + _, e = io.Copy(io.MultiWriter(tmpFile, pw), r) + pw.CloseWithError(e) + wg.Wait() + if e == nil && aErr != nil { + e = aErr + } + } + if !copied { + _, e = io.Copy(tmpFile, r) + } fatalIf(probe.NewError(e), "Unable to download file data.") r.Close() tmpFile.Close() - + wantFileName := "inspect-" + conservativeFileName(strings.Join(splits, "_")) + ".enc" if globalAirgapped { - saveInspectDataFile(key, tmpFile) + saveInspectDataFile(wantFileName, key, tmpFile) return nil } @@ -203,14 +254,14 @@ func mainSupportInspect(ctx *cli.Context) error { _, e = (&SubnetFileUploader{ alias: alias, FilePath: tmpFileName, - filename: inspectOutputFilename, + filename: wantFileName, ReqURL: reqURL, Headers: headers, DeleteAfterUpload: true, }).UploadFileToSubnet() if e != nil { console.Errorln("Unable to upload inspect data to SUBNET portal: " + e.Error()) - saveInspectDataFile(key, tmpFile) + saveInspectDataFile(wantFileName, key, tmpFile) return nil } @@ -218,34 +269,33 @@ func mainSupportInspect(ctx *cli.Context) error { return nil } -func saveInspectDataFile(key []byte, tmpFile *os.File) { +func saveInspectDataFile(dstFileName string, key []byte, tmpFile *os.File) { var keyHex string - downloadPath := inspectOutputFilename // Choose a name and move the inspect data to its final destination if key != nil { // Create an id that is also crc. var id [4]byte binary.LittleEndian.PutUint32(id[:], crc32.ChecksumIEEE(key[:])) // We use 4 bytes of the 32 bytes to identify they file. - downloadPath = fmt.Sprintf("inspect-data.%s.enc", hex.EncodeToString(id[:])) + dstFileName = fmt.Sprintf("inspect-data.%s.enc", hex.EncodeToString(id[:])) keyHex = hex.EncodeToString(id[:]) + hex.EncodeToString(key[:]) } - fi, e := os.Stat(downloadPath) + fi, e := os.Stat(dstFileName) if e == nil && !fi.IsDir() { - e = moveFile(downloadPath, downloadPath+"."+time.Now().Format(dateTimeFormatFilename)) - fatalIf(probe.NewError(e), "Unable to create a backup of "+downloadPath) + e = moveFile(dstFileName, dstFileName+"."+time.Now().Format(dateTimeFormatFilename)) + fatalIf(probe.NewError(e), "Unable to create a backup of "+dstFileName) } else { if !os.IsNotExist(e) { fatal(probe.NewError(e), "Unable to download file data") } } - fatalIf(probe.NewError(moveFile(tmpFile.Name(), downloadPath)), "Unable to rename downloaded data, file exists at %s", tmpFile.Name()) + fatalIf(probe.NewError(moveFile(tmpFile.Name(), dstFileName)), "Unable to rename downloaded data, file exists at %s", tmpFile.Name()) printMsg(inspectMessage{ - File: downloadPath, + File: dstFileName, Key: keyHex, }) } diff --git a/cmd/utils.go b/cmd/utils.go index 2e1d6a9b37..6d14ab6768 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -366,3 +366,21 @@ func getPrometheusToken(hostConfig *aliasConfigV10) (string, error) { } return token, nil } + +// conservativeFileName returns a conservative file name +func conservativeFileName(s string) string { + return strings.Trim(strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z': + return r + case r >= 'A' && r <= 'Z': + return r + case r >= '0' && r <= '9': + return r + case strings.ContainsAny(string(r), "+-_%()[]!@"): + return r + default: + return '_' + } + }, s), "_") +}