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

Publish the 2022 Cilium security audits #26213

Merged
merged 1 commit into from Jun 20, 2023

Conversation

zacharysarah
Copy link
Contributor

@zacharysarah zacharysarah commented Jun 14, 2023

Fixes: #23791

Because Sphinx is the way that it is, I had to add some logic to force newlines between file downloads. Also because reST gonna reST, I had to leave whitespace before each newline in order to mollify the linter.

Screenshot 2023-06-14 at 1 57 07 AM

The result is pretty OK.

Screenshot 2023-06-14 at 1 56 30 AM

@zacharysarah zacharysarah added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Jun 14, 2023
@zacharysarah zacharysarah requested a review from a team as a code owner June 14, 2023 09:00
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Maybe a naive question, but why not just use a regular list?

The directives fail to render when they're bulleted. Bullets aren't enough to force newlines.

Screenshot 2023-06-14 at 3 08 11 AM

Screenshot 2023-06-14 at 3 08 20 AM

Also, would it make sense to have a page dedicated to audits?

I thought about this. I'm open to it, but like I think you're saying, nothing feels quite right. 🤔

UPDATE: Maybe a page banner limited to pages in the security/ path?

@qmonnet
Copy link
Member

qmonnet commented Jun 14, 2023

Note: You can do “quote reply” by clicking on the ellipsis menu of a comment to cite the previous message, which should be easier to follow than editing my own messages :)

Maybe a naive question, but why not just use a regular list?

The directives fail to render when they're bulleted. Bullets aren't enough to force newlines.

You need a blank line between your paragraph and the list items, otherwise it's all part of the same paragraph:

    The 2022 security audits for Cilium are available:

    - :download:`Cilium Security Audit 2022 <audits/CiliumSecurityAudit2022.pdf>`
    - :download:`Cilium Fuzzing Audit 2022 <audits/CiliumFuzzingAudit2022.pdf>`

Also, would it make sense to have a page dedicated to audits?

I thought about this. I'm open to it, but like I think you're saying, nothing feels quite right. thinking

UPDATE: Maybe a page banner limited to pages in the security/ path?

Could work, although I'm not sure it deserves this much attention (and not sure if worth spending time on how to set up the banner for some pages only - although I think Sphinx allows it rather easily). Your call

@zacharysarah
Copy link
Contributor Author

zacharysarah commented Jun 16, 2023

Note: You can do “quote reply” by clicking on the ellipsis menu of a comment to cite the previous message, which should be easier to follow than editing my own messages :)

Is there any notification advantage? What's the benefit to quoting that specific way? It's not a hardship for me to select and copypaste as opposed to selecting and making GH copypaste. :-)

You need a blank line between your paragraph and the list items, otherwise it's all part of the same paragraph:

So it is. I'll add a commit for code review.

Your call

Let's do it this minimal way and observe how much traffic we get.

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

New preview looks good, thanks!

image

@qmonnet
Copy link
Member

qmonnet commented Jun 16, 2023

Note: You can do “quote reply” by clicking on the ellipsis menu of a comment to cite the previous message, which should be easier to follow than editing my own messages :)

Is there any notification advantage? What's the benefit to quoting that specific way? It's not a hardship for me to select and copypaste as opposed to selecting and making GH copypaste. :-)

Oh sure that works, too. No notification advantage. My point was simply that any of those is better than answering inside someone else's post, because the latter makes it harder to follow the conversation.

You need a blank line between your paragraph and the list items, otherwise it's all part of the same paragraph:

So it is. I'll add a commit for code review.

Thanks! Would you mind squashing these three commits together, please?

Your call

Let's do it this minimal way and observe how much traffic we get.

Sounds good to me :)

@zacharysarah
Copy link
Contributor Author

zacharysarah commented Jun 16, 2023

My point was simply that any of those is better than answering inside someone else's post, because the latter makes it harder to follow the conversation.

I still don't follow. Are you saying you would prefer quoting an entire paragraph instead of only an excerpt?

Thanks! Would you mind squashing these three commits together, please?

Done!

@qmonnet
Copy link
Member

qmonnet commented Jun 19, 2023

My point was simply that any of those is better than answering inside someone else's post, because the latter makes it harder to follow the conversation.

I still don't follow. Are you saying you would prefer quoting an entire paragraph instead of only an excerpt?

No. I mean that your did your first reply on this thread not by quoting, but by editing my message directly:

image

and this makes it difficult to follow the conversation. It looks like I wrote everything in that comment box, when in fact it contains both my comment and your replies. Makes it hard for other readers to follow the discussion (or for me, for that matter 😅).

Thanks! Would you mind squashing these three commits together, please?

Done!

Thanks! @lizrice would you mind validating, please?

@lizrice
Copy link
Member

lizrice commented Jun 19, 2023

Looks fine to me, thank you

@qmonnet
Copy link
Member

qmonnet commented Jun 19, 2023

Doc change only and passing the relevant workflows, this is ready to merge.

@qmonnet qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 19, 2023
@zacharysarah
Copy link
Contributor Author

I mean that your did your first reply on this thread not by quoting, but by editing my message directly:

Oh shoot, I'm so sorry about that, @qmonnet! I didn't even catch that I'd done it, so thank you for pointing it out. 😳

@qmonnet
Copy link
Member

qmonnet commented Jun 20, 2023

Oh shoot, I'm so sorry about that, @qmonnet! I didn't even catch that I'd done it, so thank you for pointing it out. 😳

I thought it was intentional at first, but figured that after all it had to be an accident 😄 All good, and apologies for the confusing discussion - let's move on! 🚀

Signed-off-by: ZSC <sarah.corleissen@isovalent.com>
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Weird, I totally thought we already published these :-)

Thanks for adding this!

@joestringer joestringer merged commit c4f8b59 into cilium:main Jun 20, 2023
39 checks passed
@zacharysarah zacharysarah deleted the fix-23791 branch June 21, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: link to security audits from docs
4 participants