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.

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

docs: update air-gapped docs #7160

Merged
merged 10 commits into from
Aug 9, 2024
Merged

docs: update air-gapped docs #7160

merged 10 commits into from
Aug 9, 2024

Conversation

itaysk
Copy link
Contributor

@itaysk itaysk commented Jul 14, 2024

discussed here #7029 (comment)

I did my best to restructure the docs in a way that makes sense, I have a few open comments:

  1. not sure how authentication works with --checks-bundle-repository @simar7
  2. do we have a "manually populate cache" option for checks? @simar7
  3. inconsistency in naming between --checks-bundle-repository and --skip-check-update, not sure we want to fix this at this point.
  4. --offline-scan isn't clearly explained, I presume this is about maven central? then the name is misleading (should be java specific). again, not sure we want to fix now but wanted to bring to attention. Also I thought this can be expanded to disable all the external dependencies (all the DBs) in one flag.

Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

--offline-scan isn't clearly explained, I presume this is about maven central? then the name is misleading (should be java specific). again, not sure we want to fix now but wanted to bring to attention. Also I thought this can be expanded to disable all the external dependencies (all the DBs) in one flag.

--offline-scan should affect all scanning, not limited to JAR scanning (To complicate things, JAR scanning was switched to Java DB, so this flag is not used for anything now.). When we implement internet access in the future, it should be disabled by --offline-scan. Also, I think misconfiguration scanning should respect this flag (downloading Terraform modules, etc.).

docs/docs/advanced/air-gap.md Outdated Show resolved Hide resolved
docs/docs/advanced/air-gap.md Outdated Show resolved Hide resolved
docs/docs/advanced/air-gap.md Outdated Show resolved Hide resolved
@nikpivkin
Copy link
Contributor

  1. The checks bundle is distributed as an OCI artifacts and retrieved from the Artifact registry. Authentication methods are described here.

@itaysk
Copy link
Contributor Author

itaysk commented Jul 15, 2024

The checks bundle is distributed as an OCI artifacts and retrieved from the Artifact registry. Authentication methods are described here.

@nikpivkin , @knqyf263 just said that the vuln DBs support only docker login, they don't support user/pass. is it the same for checks bundle as well? or checks bundle does support user/pass unlike vuln scanner?

@nikpivkin
Copy link
Contributor

@itaysk supports login and password

Copy link
Member

@simar7 simar7 left a comment

Choose a reason for hiding this comment

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

overall lgtm, left a couple of comments.

@itaysk
Copy link
Contributor Author

itaysk commented Jul 16, 2024

@knqyf263 wdyt about removing the manual cache manipulation guide? it seems very internal and i think using the flag is better

@knqyf263
Copy link
Collaborator

@itaysk We discussed it in #7029 and agreed not to add flags.

@knqyf263
Copy link
Collaborator

knqyf263 commented Jul 16, 2024

Or do you mean we should force users to upload DB to their OCI registry? In any case, users need to manually download the DB and copy it to the environment running Trivy, so copying it under the cache directory is not a big problem.

@itaysk
Copy link
Contributor Author

itaysk commented Jul 16, 2024

I meant the latter. not saying it's a big problem, just seems redundant and makeshift to me. ok i'll keep it, but then I want to confirm what is the procedure for misconfig checks (@simar7 @nikpivkin )?

@itaysk
Copy link
Contributor Author

itaysk commented Jul 16, 2024

Added another node about loading config from directory using --config-check flag. Also found many outdated references to this flag which I fixed in the same commit. FTR, also adding this to the list of inconsistencies (checks vs check) @simar7 can you please review

@simar7
Copy link
Member

simar7 commented Jul 17, 2024

I meant the latter. not saying it's a big problem, just seems redundant and makeshift to me. ok i'll keep it, but then I want to confirm what is the procedure for misconfig checks (@simar7 @nikpivkin )?

The process would be the same as trivy-db, the users have to navigate Trivy's cache to place the checks in the right place in case of an air gap solution.

Maybe we should document this cache structure in more detail as a whole as it is a common place for all scanners?

@itaysk
Copy link
Contributor Author

itaysk commented Aug 7, 2024

@knqyf263 I've addressed vex hub in air gapped env, let me know if it's ok 2327c5f

Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

LGTM. Left some comments.

docs/docs/advanced/air-gap.md Outdated Show resolved Hide resolved
docs/docs/advanced/air-gap.md Outdated Show resolved Hide resolved
docs/docs/advanced/air-gap.md Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have an idea about re-structuring this page, but we can do that in another PR.

@knqyf263 knqyf263 added this pull request to the merge queue Aug 9, 2024
Merged via the queue into aquasecurity:main with commit 08cc14b Aug 9, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants