-
Notifications
You must be signed in to change notification settings - Fork 534
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
Jaeger Project Security Self-Assessment - Security Pals #1198
Conversation
Signed-off-by: cp-57 <[email protected]>
✅ Deploy Preview for tag-security canceled.
|
Hi! I'm just starting to take a look at this, and the first thing I noticed is that a "backup" file was committed which was most likely intended for local development only. |
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.
Thanks for the PR @cp-57 and team, appreciate the efforts.
I have completed first pass of review and left several comments on section that needs your attention. Please feel free to reach out here or on slack for any questions and clarifications.
Along with addressing the comments, kindly update the PR branch with the latest content in the repo as this branch is out-of-date with the base branch.
|
||
I. OpenTelemetry SDK | ||
|
||
II. Deprecated [Jaeger agent](https://github.com/jaegertracing/jaeger/issues/4739) (NOT REQUIRED) |
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.
Deprecated actors are not necessary. Please keep the ones that are currently relevant.
|
||
## Self-assessment use | ||
|
||
This self-assessment is created by the Jaeger team to perform an internal analysis of the |
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.
Can you please specify at the top which member of the maintainer team acted as an author on this? (Or otherwise rephrase)
|
||
## Project compliance | ||
|
||
* Compliance. List any security standards or sub-sections the project is |
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.
Please remove any template content such as this one from the self-assessment.
* Title not end with a period | ||
* A description of the problem it’s solving or a reference to the corresponding issue | ||
* Summary of what changes were made | ||
* The pull request will then be reviewed and merged by the maintainer |
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.
Please elaborate on the process that happens henceforth.
- Is review and subsequent approval mandated for every PR?
- What does the CI pipeline look like? what are the security checks performed in this pipeline (SAST, SCA, secret scan, container image scan etc.)
* Inbound Users can contact the Jaeger team via email at [email protected], open an issue on GitHub or send a message to the [#jaeger channel on the CNCF Slack](https://cloud-native.slack.com/archives/CGG7NFUJ3). | ||
|
||
#### Outbound | ||
* Outbound the Jaeger team communicates with their users on their [website](https://www.jaegertracing.io/) and the [#jaeger channel on the CNCF Slack](https://cloud-native.slack.com/archives/CGG7NFUJ3). |
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.
Please include if there is a mail list.
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.
No mail list, but we also have :
https://github.com/orgs/jaegertracing/discussions
https://stackoverflow.com/questions/tagged/jaeger
For questions.
### Communication Channels. | ||
|
||
#### Internal | ||
* Jaeger maintainers and contributors have a monthly [zoom meeting](https://calendar.google.com/calendar/u/0/[email protected]) every 3rd thursday at 11am EST. |
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.
Please include if there is a slack channel
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 add #jaeger-maintainers channel is for maintainers.
|
||
**ZipKin** was an earlier open source distributed tracing system which is used to help users monitor and troubleshoot microservice-based architectures. Both Zipkin and Jaeger aim to provide visibility into the flow of requests and responses across various services in a distributed system. While Zipkin has been around longer, Jaeger is known for its scalability to handle tracing in large and complex microservices and displaying those traces on the Web UI. Jaeger also has backward compatibility with Zipkin to help users transition from Zipkin to Jaeger. While Zipkin has been around longer than Jaeger, Jaeger has the benefit of being a part of Cloud Native Computing Foundation(CNCF), supporting containers, kubernetes and OpenTracing. To conclude, the decision between the two comes down to preference, supported languages and whatever is compatible with your existing tech stack. | ||
|
||
### Action Items. |
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.
Please move the action items to another file within same folder, and link it in the appendix.
Thank you very much for the help @ragashreeshekar. The team and I have gone over the document and made the suggested changes. Please let me know if there is anything else that may need another look. |
Thank you for the help @eddie-knight. I have removed the backup file. |
@cp-57 Were you also able to include feedback you received from the Jaeger maintainers? |
Hi Eddie, we had a couple suggestions/edits from the maintainers that we have incorporated. |
### Communication Channels. | ||
|
||
#### Internal | ||
* Jaeger maintainers and contributors have a monthly [zoom meeting](https://calendar.google.com/calendar/u/0/[email protected]) every 3rd thursday at 11am EST. |
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 add #jaeger-maintainers channel is for maintainers.
* Inbound Users can contact the Jaeger team via email at [email protected], open an issue on GitHub or send a message to the [#jaeger channel on the CNCF Slack](https://cloud-native.slack.com/archives/CGG7NFUJ3). | ||
|
||
#### Outbound | ||
* Outbound the Jaeger team communicates with their users on their [website](https://www.jaegertracing.io/) and the [#jaeger channel on the CNCF Slack](https://cloud-native.slack.com/archives/CGG7NFUJ3). |
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.
No mail list, but we also have :
https://github.com/orgs/jaegertracing/discussions
https://stackoverflow.com/questions/tagged/jaeger
For questions.
Thank you @jkowall. I've added the suggested edits. Please let me know if there is anything else that needs some attention. |
f2d01b4
to
835548a
Compare
Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Add Jaeger SPDX-format SBOM Signed-off-by: cp-57 <[email protected]> Delete assessments/projects/jaeger/jaeger-SBOM.spdx Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Create self_assessment_backup Signed-off-by: cp-57 <[email protected]> Update self-assessment.md Signed-off-by: cp-57 <[email protected]> Update assessments/projects/jaeger/self-assessment.md Co-authored-by: Eddie Knight <[email protected]> Signed-off-by: cp-57 <[email protected]> Update assessments/projects/jaeger/self-assessment.md Co-authored-by: Eddie Knight <[email protected]> Signed-off-by: cp-57 <[email protected]> Delete assessments/projects/jaeger/self_assessment_backup Signed-off-by: cp-57 <[email protected]>
Signed-off-by: cp-57 <[email protected]> Fix link (261) Signed-off-by: cp-57 <[email protected]> Update self-assessment.md (maintainer-suggested edits) Signed-off-by: cp-57 <[email protected]> Fixed development pipeline Signed-off-by: cp-57 <[email protected]> Add actions.md link Signed-off-by: cp-57 <[email protected]> Update self-assessment.md (TAG-security suggested changes) Signed-off-by: cp-57 <[email protected]>
Sorry missed this one, but it looks good. We should merge it and then I can either copy it to our repo or you can open a PR. |
Co-authored-by: torinvdb <[email protected]> Signed-off-by: Raga <[email protected]>
Signed-off-by: Raga <[email protected]>
Co-authored-by: torinvdb <[email protected]> Signed-off-by: Raga <[email protected]>
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 separation of such a small list action items is a little odd, but I understand the rationale.
This is ready to merge.
Sorry to ask, do you want us to create the PR or does the author want to do that? Thanks, @JustinCappos . |
Hey Jonah! Justin asked me to take over from here, so I'll connect with you via slack regarding the next steps to get everything harmonized |
## Description of the changes Big thank you to the security pals team on the self-assessment work they did with the help of the NYU students. The discussion happened in cncf/tag-security#1198 ## How was this change tested? No need to test, just updating URL. ## Checklist - [X] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [X] I have signed all commits --------- Signed-off-by: Jonah Kowall <[email protected]>
Jaeger Tracing Project Self-Assessment
Jia Lin Weng
Cristian Panaro
Sameer Gori
Sarah Moughal