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.

Sign up for GitHub

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

HPCC-32847 Require WsLogAccess to include logs in ZAP #19389

Open
wants to merge 1 commit into
base: candidate-9.8.x
Choose a base branch
from
Open

HPCC-32847 Require WsLogAccess to include logs in ZAP #19389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 33 additions & 20 deletions esp/services/ws_workunits/ws_workunitsHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@

namespace ws_workunits {

// Same feature access as in ws_logaccess.ecm
#define WS_LOGACCESS "WsLogAccess"

const char * const timerFilterText = "measure[time],source[global],depth[1,]"; // Does not include hthor subgraph timings
const char* zipFolder = "tempzipfiles" PATHSEPSTR;

Expand Down Expand Up @@ -4096,8 +4099,7 @@ void CWsWuFileHelper::createThorSlaveLogfile(IConstWorkUnit* cwu, WsWuInfo& winf
}
#endif

void CWsWuFileHelper::createZAPInfoFile(const char* url, const char* esp, const char* thor, const char* problemDesc,
const char* whatChanged, const char* timing, IConstWorkUnit* cwu, const char* tempDirName)
void CWsWuFileHelper::createZAPInfoFile(CWsWuZAPInfoReq &request, IConstWorkUnit* cwu, const char* tempDirName)
{
VStringBuffer fileName("%s%c%s.txt", tempDirName, PATHSEPCHAR, cwu->queryWuid());
Owned<IFileIOStream> outFile = createBufferedIOStreamFromFile(fileName.str(), IFOcreate);
Expand All @@ -4109,11 +4111,11 @@ void CWsWuFileHelper::createZAPInfoFile(const char* url, const char* esp, const
sb.append("User: ").append(cwu->queryUser()).append("\r\n");
sb.append("Build Version:").append(getBuildVersion()).append("\r\n");
sb.append("Cluster: ").append(cwu->queryClusterName()).append("\r\n");
sb.append("ESP: ").append(esp).append("\r\n");
if (!isEmptyString(url))
sb.append("URL: ").append(url).append("\r\n");
if (!isEmptyString(thor))
sb.append("Thor: ").append(thor).append("\r\n");
sb.append("ESP: ").append(request.esp).append("\r\n");
if (!isEmptyString(request.url))
sb.append("URL: ").append(request.url).append("\r\n");
if (!isEmptyString(request.thor))
sb.append("Thor: ").append(request.thor).append("\r\n");
outFile->write(sb.length(), sb.str());

//Exceptions/Warnings/Info
Expand Down Expand Up @@ -4147,9 +4149,14 @@ void CWsWuFileHelper::createZAPInfoFile(const char* url, const char* esp, const
}

//User provided Information
writeZAPWUInfoToIOStream(outFile, "Problem: ", problemDesc);
writeZAPWUInfoToIOStream(outFile, "What Changed: ", whatChanged);
writeZAPWUInfoToIOStream(outFile, "Timing: ", timing);
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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor, but potentially very helpful would be to add "user lacks 'WsLogAccess:Read' feature access, please contact admin" or something to that affect.

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)
Expand Down Expand Up @@ -4517,36 +4524,39 @@ 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);
setZAPReportName(request.zapFileName, cwu->queryWuid(), zapFileName);

zipFileNameWithFullPath.set(tempDirName).append(PATHSEPCHAR).append("zapreport.zip");
thorSlaveLogThreadPoolSize = _thorSlaveLogThreadPoolSize;

//create WU ZAP files
createZAPInfoFile(request.url.str(), request.esp.str(), request.thor.str(), request.problemDesc.str(), request.whatChanged.str(),
request.whereSlow.str(), cwu, tempDirName);
//create WU ZAP files
createZAPInfoFile(request, cwu, tempDirName);
createZAPECLQueryArchiveFiles(cwu, tempDirName);

WsWuInfo winfo(context, cwu);
createZAPWUXMLFile(winfo, tempDirName);
createZAPWUGraphProgressFile(request.wuid.str(), tempDirName);

StringArray localFiles;
createZAPWUQueryAssociatedFiles(cwu, tempDirName, localFiles);
createZAPWUQueryAssociatedFiles(cwu, tempDirName, localFiles, request.hasLogsAccess);
if (request.hasLogsAccess)
{
#ifndef _CONTAINERIZED
createProcessLogfile(cwu, winfo, "EclAgent", tempDirName);
createProcessLogfile(cwu, winfo, "Thor", tempDirName);
if (request.includeThorSlaveLog.isEmpty() || strieq(request.includeThorSlaveLog.str(), "on"))
createThorSlaveLogfile(cwu, winfo, tempDirName);
createProcessLogfile(cwu, winfo, "EclAgent", tempDirName);
createProcessLogfile(cwu, winfo, "Thor", tempDirName);
if (request.includeThorSlaveLog.isEmpty() || strieq(request.includeThorSlaveLog.str(), "on"))
createThorSlaveLogfile(cwu, winfo, tempDirName);
#else
readWULogToFiles(cwu, winfo, tempDirName, request);
readWULogToFiles(cwu, winfo, tempDirName, request);
#endif
}

//Write out to ZIP file
zipAllZAPFiles(tempDirName, localFiles, request.password, zipFileNameWithFullPath);
}

void CWsWuFileHelper::createZAPWUQueryAssociatedFiles(IConstWorkUnit* cwu, const char* tempDirName, StringArray& localFiles)
void CWsWuFileHelper::createZAPWUQueryAssociatedFiles(IConstWorkUnit* cwu, const char* tempDirName, StringArray& localFiles, bool hasLogsAccess)
{
Owned<IConstWUQuery> query = cwu->getQuery();
if (!query)
Expand All @@ -4563,6 +4573,9 @@ void CWsWuFileHelper::createZAPWUQueryAssociatedFiles(IConstWorkUnit* cwu, const
cur.getName(name);
cur.getIp(ip);

if (!hasLogsAccess && endsWith(name.str(), ".eclcc.log"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow what's special about *.eclcc.log files, this might need a code comment

continue;

RemoteFilename rfn;
SocketEndpoint ep(ip.str());
rfn.setPath(ep, name.str());
Expand Down
17 changes: 14 additions & 3 deletions esp/services/ws_workunits/ws_workunitsHelpers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ struct CWsWuZAPInfoReq
bool sendEmail, attachZAPReportToEmail;
bool includeRelatedLogs = true, includePerComponentLogs = false;
unsigned maxAttachmentSize, port;
bool hasLogsAccess = false;

WUComponentLogOptions logFilter;

Expand All @@ -381,6 +382,17 @@ struct CWsWuZAPInfoReq
{
logFilter.populateLogFilter(wuid.str(), httpRequest);
}

// True when logs _would have been included_ if the user had access
bool logsExcludedDueToNoAccess()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor, but for some reason the current method name misleads me into thinking the logs have already been excluded, but I think the method is determining if the logs should be excluded, does it make sense to rename to excludeLogsDueToNoAccess()?

{
#ifdef _CONTAINERIZED
return (includeRelatedLogs || includePerComponentLogs) && !hasLogsAccess;
#else
// Bare metal historically always includes Thor and EclAgent logs
return !hasLogsAccess;
#endif
}
};

class WsWuInfo
Expand Down Expand Up @@ -890,11 +902,10 @@ class CWsWuFileHelper
IFile *createWorkingFolder(IEspContext &context, const char *wuid, const char *namePrefix,
StringBuffer &namePrefixStr, StringBuffer &folderName);

void createZAPInfoFile(const char *url, const char *espIP, const char *thorIP, const char *problemDesc,
const char *whatChanged, const char *timing, IConstWorkUnit *cwu, const char *pathNameStr);
void createZAPInfoFile(CWsWuZAPInfoReq &request, IConstWorkUnit *cwu, const char *pathNameStr);
void createZAPWUXMLFile(WsWuInfo &winfo, const char *pathNameStr);
void createZAPECLQueryArchiveFiles(IConstWorkUnit *cwu, const char *pathNameStr);
void createZAPWUQueryAssociatedFiles(IConstWorkUnit *cwu, const char *pathToCreate, StringArray &localFiles);
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);
Expand Down
Loading