-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
refactor: remove parallel walk #5180
Conversation
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
@santhosh1729 I think we'll merge this PR now, and you can give us feedback anytime. |
Signed-off-by: knqyf263 <knqyf263@gmail.com>
This PR is stale because it has been labeled with inactivity. |
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
@DmitriyLewen Could you quickly take a look? Aqua confirmed it, and it should be fine, but I want to double-check it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
Signed-off-by: knqyf263 <knqyf263@gmail.com>
skipFiles []string | ||
skipDirs []string | ||
var defaultSkipDirs = []string{ | ||
"**/.git", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a question. Why you just ignore the .git folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git dir shouldn't have files Trivy wants to find, and it is time-consuming to walk the dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git dir shouldn't have files Trivy wants to find, and it is time-consuming to walk the dir.
thank u for answering. But I want to say something. just like what github official does.
They allows save PAT tokens or request OAuth tokens in this actions. And backup this So some careless people do like
FROM node:16
RUN mkdir /app
COPY . /app
RUN yarn install
EXPOSE 80
CMD ["node", "/app/app.js"]
And Python/PHP/Javascript programmers always do like this. : )
So at least whitelist the .git/config
.git/modules/[any]/config
and blacklist something like .git/objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FROM node:16
RUN mkdir /app
COPY . /app
RUN yarn install
EXPOSE 80
CMD ["node", "/app/app.js"]
What's wrong in this Dockerfile? PAT will be deleted in the post-job step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FROM node:16 RUN mkdir /app COPY . /app RUN yarn install EXPOSE 80 CMD ["node", "/app/app.js"]
What's wrong in this Dockerfile? PAT will be deleted in the post-job step.
Yes post job. But docker image building is before it :)
It's
graph TD;
START ---> github.token.start(register a Temp github.token) ---> checkout ---> checkout-1(PAT sets so use PAT, do submodules or sth, save it in .git/config) ---> docker-image-building ---> docker-image-uploading(docker image tag login push) ---> post-docker-login(post docker login if login action) ---> post-checkout(post checkout action, clean up if not set persisence, delete local config) ---> End(Action Ends and revoke the Temp github.token)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. It's interesting. Did you ensure PAT can be exposed to a container image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. It's interesting. Did you ensure PAT can be exposed to a container image?
pretty sure. Play with cross repos ref submodules they must do like that. I can create an environment. : )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created an issue. Thanks!
#6699
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created an issue. Thanks! #6699
nice work :)
Description
Since filepath.WalkDir is fast enough now, this PR removes
github.com/saracen/walker
. Also, it defines theWalker
interface so that other tools can override the behavior.Related issues
Checklist