From 915e318795888d297fd6752a8c2d5074c1dad1cf Mon Sep 17 00:00:00 2001 From: Terrence Asselin Date: Wed, 15 Jan 2025 16:25:27 -0600 Subject: [PATCH] HPCC-32847 Code review comment changes * Vary the permission feature required to access logs to match the one used for the cluster deployment- containerized or bare metal. * The WsLogAccess permission is used when pulling all containerized log files, including eclcc, because the remote log accessor is required to pull them all. * Because users are used to seeing errors in log file stubs, instead of actual log entries, continue that behavior here. When access to log files is denied, insert a message into each (otherwise empty) log file included in the ZAP. * Some code paths don't currently replace files with error messages, so to maintain current behavior and user expectations don't insert error messages into any file other than a log file. Signed-off-by: Terrence Asselin --- .../ws_workunits/ws_workunitsHelpers.cpp | 73 ++++++++++++------- .../ws_workunits/ws_workunitsHelpers.hpp | 14 +--- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/esp/services/ws_workunits/ws_workunitsHelpers.cpp b/esp/services/ws_workunits/ws_workunitsHelpers.cpp index 30f812f10bb..1192ac70634 100644 --- a/esp/services/ws_workunits/ws_workunitsHelpers.cpp +++ b/esp/services/ws_workunits/ws_workunitsHelpers.cpp @@ -45,8 +45,11 @@ namespace ws_workunits { -// Same feature access as in ws_logaccess.ecm -#define WS_LOGACCESS "WsLogAccess" +#ifdef _CONTAINERIZED +static const char* LOG_ACCESS_FEATURE = "WsLogAccess"; +#else +static const char* LOG_ACCESS_FEATURE = "ClusterTopologyAccess"; +#endif const char * const timerFilterText = "measure[time],source[global],depth[1,]"; // Does not include hthor subgraph timings const char* zipFolder = "tempzipfiles" PATHSEPSTR; @@ -3994,13 +3997,12 @@ void CWsWuFileHelper::cleanFolder(IFile* folder, bool removeFolder) } #ifndef _CONTAINERIZED -void CWsWuFileHelper::createProcessLogfile(IConstWorkUnit* cwu, WsWuInfo& winfo, const char* process, const char* path) +void CWsWuFileHelper::createProcessLogfile(IConstWorkUnit* cwu, WsWuInfo& winfo, const char* process, const char* path, bool hasLogsAccess) { BoolHash uniqueProcesses; Owned procs = cwu->getProcesses(process, NULL); ForEach (*procs) { - StringBuffer logSpec; IPropertyTree& proc = procs->query(); const char* processName = proc.queryName(); if (isEmpty(processName)) @@ -4024,11 +4026,17 @@ void CWsWuFileHelper::createProcessLogfile(IConstWorkUnit* cwu, WsWuInfo& winfo, pid.appendf("%d", proc.getPropInt("@pid")); fileName.append("_eclagent.log"); + if (!hasLogsAccess) + throw makeStringExceptionV(ECLWATCH_ACCESS_TO_FILE_DENIED, "Access to log files is denied. You are missing permission %s:READ - check with your system admin.", LOG_ACCESS_FEATURE); + winfo.getWorkunitEclAgentLog(processName, nullptr, pid.str(), mb, fileName.str()); } else if (strieq(process, "Thor")) { fileName.append("_thormaster.log"); + if (!hasLogsAccess) + throw makeStringExceptionV(ECLWATCH_ACCESS_TO_FILE_DENIED, "Access to log files is denied. You are missing permission %s:READ - check with your system admin.", LOG_ACCESS_FEATURE); + winfo.getWorkunitThorMasterLog(processName, nullptr, mb, fileName.str()); } } @@ -4036,14 +4044,14 @@ void CWsWuFileHelper::createProcessLogfile(IConstWorkUnit* cwu, WsWuInfo& winfo, { StringBuffer s; e->errorMessage(s); - IERRLOG("Error accessing Process Log file %s: %s", logSpec.str(), s.str()); + IERRLOG("Error accessing Process Log file %s: %s", processName, s.str()); writeToFile(fileName.str(), s.length(), s.str()); e->Release(); } } } -void CWsWuFileHelper::createThorSlaveLogfile(IConstWorkUnit* cwu, WsWuInfo& winfo, const char* path) +void CWsWuFileHelper::createThorSlaveLogfile(IConstWorkUnit* cwu, WsWuInfo& winfo, const char* path, bool hasLogsAccess) { if (cwu->getWuidVersion() == 0) return; @@ -4058,6 +4066,18 @@ void CWsWuFileHelper::createThorSlaveLogfile(IConstWorkUnit* cwu, WsWuInfo& winf return; } + // Since there is not already a handler inserting exceptions into the thor slave log, + // make a simple case here to insert a message about permission failure and write it to + // a file that could be recognized as a thor slave log. + if (!hasLogsAccess) + { + StringBuffer msg; + msg.appendf("Access to log files is denied. You are missing permission %s:READ - check with your system admin.", LOG_ACCESS_FEATURE); + VStringBuffer fileName("%s%cthorslave.log", path, PATHSEPCHAR); + writeToFile(fileName.str(), msg.length(), msg.str()); + return; + } + Owned threadFactory = new CGetThorSlaveLogToFileThreadFactory(); Owned threadPool = createThreadPool("WsWuFileHelper GetThorSlaveLogToFile Thread Pool", threadFactory, false, nullptr, thorSlaveLogThreadPoolSize, INFINITE); @@ -4152,11 +4172,6 @@ void CWsWuFileHelper::createZAPInfoFile(CWsWuZAPInfoReq &request, IConstWorkUnit writeZAPWUInfoToIOStream(outFile, "Problem: ", request.problemDesc); writeZAPWUInfoToIOStream(outFile, "What Changed: ", request.whatChanged); writeZAPWUInfoToIOStream(outFile, "Timing: ", request.whereSlow); - if (request.logsExcludedDueToNoAccess()) - { - writeZAPWUInfoToIOStream(outFile, "Logs: ", "Excluded due to no access, need WsLogAccess:Read"); - LOG(MCuserWarning, "ZAP report for workunit %s excludes logs due to access restrictions. Need WsLogAccess:READ", request.wuid.str()); - } } void CWsWuFileHelper::writeZAPWUInfoToIOStream(IFileIOStream* outFile, const char* name, SCMStringBuffer& value) @@ -4288,6 +4303,8 @@ void CWsWuFileHelper::readWULogToFile(const char *logFileName, WsWuInfo &winfo, { try { + if (!zapLogFilterOptions.hasLogsAccess) + throw makeStringExceptionV(ECLWATCH_ACCESS_TO_FILE_DENIED, "Access to log files is denied. You are missing permission %s:READ - check with your system admin.", LOG_ACCESS_FEATURE); winfo.readWorkunitComponentLogs(logFileName, zapLogFilterOptions); } catch(IException* e) @@ -4524,7 +4541,7 @@ int CWsWuFileHelper::zipAFolder(const char* folder, bool gzip, const char* zipFi void CWsWuFileHelper::createWUZAPFile(IEspContext& context, IConstWorkUnit* cwu, CWsWuZAPInfoReq& request, const char* tempDirName, StringBuffer& zapFileName, StringBuffer& zipFileNameWithFullPath, unsigned _thorSlaveLogThreadPoolSize) { - request.hasLogsAccess = context.validateFeatureAccess(WS_LOGACCESS, SecAccess_Read, false); + request.hasLogsAccess = context.validateFeatureAccess(LOG_ACCESS_FEATURE, SecAccess_Read, false); setZAPReportName(request.zapFileName, cwu->queryWuid(), zapFileName); zipFileNameWithFullPath.set(tempDirName).append(PATHSEPCHAR).append("zapreport.zip"); @@ -4540,17 +4557,14 @@ void CWsWuFileHelper::createWUZAPFile(IEspContext& context, IConstWorkUnit* cwu, StringArray localFiles; createZAPWUQueryAssociatedFiles(cwu, tempDirName, localFiles, request.hasLogsAccess); - if (request.hasLogsAccess) - { #ifndef _CONTAINERIZED - createProcessLogfile(cwu, winfo, "EclAgent", tempDirName); - createProcessLogfile(cwu, winfo, "Thor", tempDirName); + createProcessLogfile(cwu, winfo, "EclAgent", tempDirName, request.hasLogsAccess); + createProcessLogfile(cwu, winfo, "Thor", tempDirName, request.hasLogsAccess); if (request.includeThorSlaveLog.isEmpty() || strieq(request.includeThorSlaveLog.str(), "on")) - createThorSlaveLogfile(cwu, winfo, tempDirName); + createThorSlaveLogfile(cwu, winfo, tempDirName, request.hasLogsAccess); #else readWULogToFiles(cwu, winfo, tempDirName, request); #endif - } //Write out to ZIP file zipAllZAPFiles(tempDirName, localFiles, request.password, zipFileNameWithFullPath); @@ -4573,12 +4587,24 @@ void CWsWuFileHelper::createZAPWUQueryAssociatedFiles(IConstWorkUnit* cwu, const cur.getName(name); cur.getIp(ip); - if (!hasLogsAccess && endsWith(name.str(), ".eclcc.log")) - continue; - RemoteFilename rfn; SocketEndpoint ep(ip.str()); rfn.setPath(ep, name.str()); + StringBuffer fileName(name.str()); + getFileNameOnly(fileName, false); + + VStringBuffer outFileName("%s%c%s", tempDirName, PATHSEPCHAR, fileName.str()); + + // Since users are only accustomed to seeing error messages replacing log file contents, + // special case an access denied error message for the one log file processed here. + if (!hasLogsAccess && endsWith(name.str(), ".eclcc.log")) + { + StringBuffer msg; + msg.appendf("Access to log files is denied. You are missing permission %s:READ - check with your system admin.", LOG_ACCESS_FEATURE); + writeToFile(outFileName.str(), msg.length(), msg.str()); + continue; + } + if (rfn.isLocal()) { localFiles.append(name.str()); @@ -4592,11 +4618,6 @@ void CWsWuFileHelper::createZAPWUQueryAssociatedFiles(IConstWorkUnit* cwu, const continue; } - StringBuffer fileName(name.str()); - getFileNameOnly(fileName, false); - - VStringBuffer outFileName("%s%c%s", tempDirName, PATHSEPCHAR, fileName.str()); - OwnedIFile outFile = createIFile(outFileName); if (!outFile) { diff --git a/esp/services/ws_workunits/ws_workunitsHelpers.hpp b/esp/services/ws_workunits/ws_workunitsHelpers.hpp index 15009d727f2..7dde648c6c1 100644 --- a/esp/services/ws_workunits/ws_workunitsHelpers.hpp +++ b/esp/services/ws_workunits/ws_workunitsHelpers.hpp @@ -383,16 +383,6 @@ struct CWsWuZAPInfoReq logFilter.populateLogFilter(wuid.str(), httpRequest); } - // True when logs _would have been included_ if the user had access - bool logsExcludedDueToNoAccess() - { -#ifdef _CONTAINERIZED - return (includeRelatedLogs || includePerComponentLogs) && !hasLogsAccess; -#else - // Bare metal historically always includes Thor and EclAgent logs - return !hasLogsAccess; -#endif - } }; class WsWuInfo @@ -908,8 +898,8 @@ class CWsWuFileHelper void createZAPWUQueryAssociatedFiles(IConstWorkUnit *cwu, const char *pathToCreate, StringArray &localFiles, bool hasLogsAccess); void createZAPWUGraphProgressFile(const char *wuid, const char *pathNameStr); #ifndef _CONTAINERIZED - void createProcessLogfile(IConstWorkUnit *cwu, WsWuInfo &winfo, const char *process, const char *path); - void createThorSlaveLogfile(IConstWorkUnit *cwu, WsWuInfo &winfo, const char *path); + void createProcessLogfile(IConstWorkUnit *cwu, WsWuInfo &winfo, const char *process, const char *path, bool hasLogsAccess); + void createThorSlaveLogfile(IConstWorkUnit *cwu, WsWuInfo &winfo, const char *path, bool hasLogsAccess); #endif LogAccessLogFormat getComponentLogFormatFromLogName(const char *log); void readWULogToFiles(IConstWorkUnit *cwu, WsWuInfo &winfo, const char *path, CWsWuZAPInfoReq &zapLogFilterOptions);