-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
CSHARP-5048: Integrate with silk and get SBOM document for releases #1340
Conversation
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.
Nice work, minor comment.
@@ -1,7 +1,7 @@ | |||
# ${PRODUCT_NAME} SSDLC compliance report | |||
|
|||
This report is available | |||
<a href=https://us-west-2.console.aws.amazon.com/s3/object/csharp-driver-release-assets?region=us-west-2&bucketType=general&prefix=${PRODUCT_NAME}/${PACKAGE_VERSION}/ssdlc_compliance_report.md>here</a>. | |||
<a href="https://us-west-2.console.aws.amazon.com/s3/object/csharp-driver-release-assets?region=us-west-2&bucketType=general&prefix=${PRODUCT_NAME}/${PACKAGE_VERSION}/ssdlc_compliance_report.md">here</a>. |
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.
+1
evergreen/download-augmented-sbom.sh
Outdated
@@ -0,0 +1,16 @@ | |||
#!/usr/bin/env bash | |||
|
|||
cat << EOF > silkbomb.env |
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.
Let's specify the input vars, like here
I know that we have not always been doing this, but we should :)
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 + minor suggestions.
evergreen/evergreen.yml
Outdated
script: | | ||
${PREPARE_SHELL} | ||
./evergreen/download-augmented-sbom.sh | ||
- command: ec2.assume_role |
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.
It looks like we can move assume role into the task, because we do not need to do it twice (for SSDLC report upload and for SBOM upload). @BorisDog ?
evergreen/download-augmented-sbom.sh
Outdated
echo "Downloading augmented sbom from silk" | ||
|
||
docker run --platform="linux/amd64" --rm -v ${PWD}:/pwd \ | ||
--env-file silkbomb.env \ |
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.
We can simplify a little by passing environment variables not through the file, but as docker cli parameters. Like:
-e SILK_CLIENT_ID "${SILK_CLIENT_ID}" \
-e SILK_CLIENT_SECRET "${SILK_CLIENT_SECRET}" \
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
No description provided.