Generate documentation page for third-party dependencies #2611
Conversation
charith-elastic
added
>docs
|
Jenkins test this please |
|
Jenkins test this please |
| This directory contains the scripts to generate licence notice and dependency information documentation. | ||
|
|
||
| - `generate-notice.sh`: Invoked by `make generate-notice-file` to automatically generate `NOTICE.txt` and `docs/dependencies.asciidoc`. | ||
| - `generate-image-deps.sh`: Manually invoked script to update the container image dependency information in `docs/container-image-dependencies.csv`. |
There was a problem hiding this comment.
Can you clarify what the CSV is intended for? It doesn't look like it's used as an input to anything else. I was also a little surprised when I was testing it out and it needed to sudo
There was a problem hiding this comment.
The CSV is included by docs/dependencies.asciidoc and contains the licence information for the Docker image. The sudo request by Tern is indeed a bit worrying but apparently it's needed to mount the procfs file system:
Tern requires root privileges to run because it needs to mount procfs in order to run commands within a chroot environment and call the Docker CLI. It is enough if you have configured sudo; Tern will ask for your password before running any priviledged commands.
I wish they supported OCI images rather than relying so heavily on Docker daemon and CLI but it's the only reliable tool I have managed to find. The alternative is to manually maintain a list of dependencies in the Docker image
There was a problem hiding this comment.
Got it, thanks for the clarification. I wonder if there's a clearer phrase than "container image dependency" since our direct dependencies are also included in the image (just inside the binary). Maybe changing to "container base image dependency..."?
| "os" | ||
| "path/filepath" | ||
|
|
||
| securejoin "github.com/cyphar/filepath-securejoin" |
There was a problem hiding this comment.
I learned about a new lib, nice
| } | ||
|
|
||
| decoder := json.NewDecoder(f) | ||
| for { |
There was a problem hiding this comment.
This might be me overthinking it, but we mostly parse it this way because we want to end up with a map for mkDepInfo(), correct? If we wanted a slice we could just store overrides.json as a json array and unmarshal it right into a slice (and also get to prettify the JSON file rather than making each item be on one line).
There was a problem hiding this comment.
I chose the JSON lines format because it is well-known and Go supports it out of the box.
The amount of data stored on each line is pretty minimal and I don't expect it to be edited too often. If it starts to become too complex and requires pretty formatting, I think we can consider switching to a more human-friendly file format at that point.
| func MkClassifier(dataPath string) (*licenseclassifier.License, error) { | ||
| absPath, err := filepath.Abs(dataPath) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to determine absolute path of licence data file: %w", err) |
There was a problem hiding this comment.
nit: it might be worth considering using pkg/errors so we get those sweet, sweet stack traces
There was a problem hiding this comment.
I think pkg/errors is now in maintenance mode in favour of the changes to errors introduced by Go 1.13. I am using the error wrapping method introduced by 1.13 here. I prefer error messages with added context like this over stacktraces as they are easier to read and search for in the code.
There was a problem hiding this comment.
I think it's been in maintenance mode for a bit now, but you are correct 1.13 adds the error wrapping functionality that pkg/errors had. There's still no easy support afaik for including stack traces as well though. Like I said it was just a personal style suggestion, definitely not a blocker
|
It's a little odd the base image deps are formatted differently than the module deps, but it matches the ECE doc and I expect this will probably be the least visited page in our entire docs, so it is definitely not worth changing. |
|
|
||
| This script generates licence information for the contents of the ECK container image. As the container base image is rarely ever changed and the tool used ([Tern](https://github.com/vmware/tern)) is slow to run, this script is not invoked automatically by the build process. | ||
|
|
||
| To generate the dependency list (`docs/container-image-dependencies.csv`) for a particular image tag, invoke the script as follows: |
There was a problem hiding this comment.
| To generate the dependency list (`docs/container-image-dependencies.csv`) for a particular image tag, invoke the script as follows: | |
| To generate the dependency list (`docs/container-image-dependencies.csv`) for a particular image tag, invoke the script as follows. Note that this will prompt for root and [is necessary](https://github.com/vmware/tern/blob/master/docs/releases/v0_1_0.md#notes): |
| "github.com/karrick/godirwalk" | ||
| ) | ||
|
|
||
| var errLicenceNotFound = errors.New("failed to detect licence") | ||
| // detectionThreshold is the minimum confidence score required from the licence classifier. | ||
| const detectionThreshold = 0.85 |
There was a problem hiding this comment.
Just curious, was the 0.8 here: https://github.com/google/licenseclassifier/blob/master/classifier.go#L39
too loose?
There was a problem hiding this comment.
It's mainly for self-documentation here so that someone reading the code would be aware of the existence of a confidence score. Also, all of our current dependencies had scores higher than 0.85 so I thought it would be good to have a higher threshold to reduce the potential for a false positive slipping through.
Since Go dependencies have long names, having the URL as a separate column makes the table too wide for the content area -- causing the text to get squished and unreadable. |
Generates a documentation page containing third-party dependency information.
This change refactors the licence-detector to add the following functionality:
Dependency page preview: http://cloud-on-k8s_2611.docs-preview.app.elstc.co/guide/en/cloud-on-k8s/master/k8s-dependencies.html
Fixes elastic/k8s-dev#103