-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
chore: provide OSSF security-insight #18940
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mmorel-35 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @mmorel-35. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
b30279d
to
c797b74
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted filessee 21 files with indirect coverage changes @@ Coverage Diff @@
## main #18940 +/- ##
==========================================
+ Coverage 68.73% 68.74% +0.01%
==========================================
Files 420 420
Lines 35561 35561
==========================================
+ Hits 24442 24448 +6
+ Misses 9690 9683 -7
- Partials 1429 1430 +1 Continue to review full report in Codecov by Sentry.
|
cc. @jmhbnz, thoughts on this? /ok-to-test |
Thanks @mmorel-35 for raising the proposal. I like the idea / intent. However I am not a fan of the implementation as it feels like more clutter in the project root directory. It's not clear to me the benefits, is there a specific problem we are trying to solve? I would like to see atttention/tidy-up/improvement to what we already hold under |
c797b74
to
1928ab8
Compare
@jmhbnz , What’s troubling you with the information provided in SECURITY-INSIGHTS.yaml. Have I missed something or provided a wrong information ? |
Signed-off-by: Matthieu MOREL <[email protected]>
1928ab8
to
ca32fe9
Compare
Yes - As mentioned I have no issues with the idea / intent behind the pull request. What I dislike is the implementation (i.e. more clutter in our project top level directory).
No concerns with the content of the file, only the placement of the file within the project directory structure.
It's not clear to me the benefit or improvement to our project security posture as a result of merging this pr. It seems to be optional reporting metadata which I don't think warrants living at project top level directory. Just because a standard exists does not mean we have to adopt it blindly, there are trade-offs we need to consider. We have previously done work in #15778 and #15987 to reduce clutter in our top level directory so any new addition here should have a strong case. Ultimately this is just my current personal view and I defer decision on merging this pr to the project tech leads @ahrtr and @serathius. |
Thanks for your contribution, but please don't assume that others have the same background or did the same investigation/study as you. Please try to provide more context when you raise a PR. It took me roughly 15 ~ 20 minutes to investigate&google the background, the following is what I got, for others reference,
|
Clomonitor requires a SECURITY-INSIGHTS.yml,
See https://clomonitor.io/projects/cncf/etcd#etcd_security
This was done with the help of https://github.com/ossf/security-insights-spec/tree/main/specification