Add information about securing access to Cilium pods and provide a single page security reference

[zacharysarah]

UPDATE 10 Feb 23: The description is wholly rewritten based on reviewer feedback and some fundamental changes in scope and approach.

Fixes: #23655

This PR makes several additions, changes, and updates to security content and organization:

• Renames security/tutorial-toc.rst to security/index.rst

  Currently https://docs.cilium.io/en/latest/security/ returns 404, which isn't a great user experience. Because security/tutorial-toc serves as a one-page reference to security content, it makes the most sense for it to serve as a security/ landing.

  Note: This page would benefit from having more narrative content to guide users, but that's a separate PR.

• On the renamed security/index, changes the page heading from "Network Policy Security Tutorials" to "Securing networks with Cilium"

  "Tutorials" isn't an accurate description for procedural content.

• adds a new page about securing Cilium pods by restricting access via K8s RBAC

  security/restrict-pod-access.rst

• links the page about restricting pod access to the renamed /security/index landing

• updates the site landing page by adding a link to security/index

  This makes security content easier to find from the site landing.

• makes minor improvements to information architecture for security content

  This includes cleaning up unused ref: anchors and adding/renaming other ref: anchors to support new content

[zacharysarah]

@qmonnet 👋🏻 This PR is very much in minimum viable state. Things I'd love feedback on:

• Do we need to say anything more on the one-page reference page itself?

What's missing that readers need?

• Does the order of security practices make sense?

I copied the existing TOC tree order, but I'm open to better organization--and maybe mirroring page organization in the site TOC as a tiny step towards a better information architecture.

Thanks in advance for your review! 🙇🏻

[zacharysarah]

Moving back to WIP to populate the file on restricting access to pods

[qmonnet]

We usually do that by converting to draft (so we can filter on draft:false for listing the PRs that currently need reviews, for example) 🙂

[zacharysarah]

@qmonnet 👋🏻 This PR is ready for review again. I updated the PR description, PTAL.

We usually do that by converting to draft

Next time! ✅ 😅

[tixxdz]

Please check with the rest of the team before. Thank you!

[zacharysarah]

@qmonnet

Note: The release-note block in the PR description should usually be a single, relatively short line summarising the change. It's what ends up in release descriptions, such as this one.

When I look at the release note descriptions and the associated upstream PRs, I don't see release-note blocks in any of the PRs. For example, in #23134 release notes appear to be generated from the commit message.

Should I be using the ```release-note syntax at all?

[qmonnet]

When I look at the release note descriptions and the associated upstream PRs, I don't see release-note blocks in any of the PRs. For example, in #23134 release notes appear to be generated from the commit message.

If the release-note block is present, the message it contains is picked up for the release notes. Otherwise, it falls back to the title of the PR (which may or may not be the same as the commit title, it's usually different when PRs contain several commits).

Should I be using the ```release-note syntax at all?

You can, in particular if the title of the PR does not work well as a description for the release notes. If the title of your PR makes for a good description, then you don't have to :)

We have this documented at items 11 and 12 of this list.

[qmonnet]

I acknowledge that the commits to date are difficult to evaluate individually, and will do my best to clean them up accordingly.

Thank you!

For context, this is my first PR to the repo. The commits reflect several retroactive changes based on new discoveries about the information architecture and CI requirements.

Given just how much discovery, trial-by-error, and undocumented CI behavior went into the learn-as-you-go nature of this PR, there was no way to plan out a logical or aesthetically pleasing sequence of commits in advance.

I do understand that some new edits, or CI-related adjustments, come up as necessary after the main changes have been committed or even after the PR has been created.

From a technical point of view, this is not incompatible with producing a sequence of stand-alone, logical commits, given that Git makes it possible to rework and rebase commits. Each of these changes should be integrated to the relevant commits or moved to dedicated commits, depending on the case.

Now, I do realise this is not a trivial task to rework the commits, and that users do not always possess the knowledge to do it.

I can only imagine that other new contributors will have a similar experience.

It really depends on contributors. Some are familiar enough with moving around things between commits with Git and have no particular issue with this process, others are struggling more indeed.

I would ask reviewers to consider whether, especially for new contributors with smaller PRs, aesthetically pleasing commit sequences are necessary enough to impose a gatekeeping requirement for merge. I would suggest instead that we give new contributors feedback about commits that guides them to better process next time. I'd like us to aim to invite contributors in rather than keep them out unless they demonstrate 100% compliance to a process that nominally eases reviews but isn't strictly necessary for merge. (Reverts and other history-altering commits are an obvious exception to easing compliance.)

This is a debate that goes beyond this Pull Request, and that, perhaps, would be better addressed on another channel (Slack or Community Meeting?).

We are all in for inviting contributors rather than keeping them out. Whether we manage to do so, is another story. In this particular example, I understand that getting some pushback from the reviewers, and being asked to rework the commits so they look “aesthetically pleasing”, can feel frustrating.

However, I would argue that this is not a matter of aestheticism, and that this is not (only) about facilitating reviews. I find that proper commits for code changes, and to a lesser extend (but still) for documentation, are essential for understanding the changes when looking later at the Git history (with git log or git blame). They help understand the motivation for the change, if the commits are stand-alone then it helps understanding the full extent of the change without looking for neighbour commits, etc. We maintain three branches of Cilium and are regularly backporting some fixes, including to documentation, and again, it's much, much easier to do when things are split in a clean way. At last, although this arguably applies seldom to documentation, we sometimes run git bisect on the repository to identify what commit introduced an error, and it is essential in that case that the build succeeds on every commit, or bisecting becomes much more complex and involved.

Now, as a reviewer, I wouldn't expect the same level of “cleanness” for the PR from a new/occasional person willing to submit a fix in passing, as from regular contributors. But because of the points I mentioned above, it's hard for me to wave aside all expectations on PR “cleanness” (I lack a better term).

[Please keep in mind that the above is only representative of my own opinion.]

Happy to discuss this further, here or another channel, if you have concerns with any of what I mentioned. And again, happy to help if any technical obstacle arises along the path.

Instead we could have, for example, one commit to rename existing pages, then one to reorganise files and add links on the toplevel index, and, at last, one to add the new page for pod access restriction. Let me know if I can help!

I appreciate the offer for help. I will do my best to revise commits with the content you requested. I will also point out that complex commit revisions require non-trivial amounts of effort in order to provide nominal ease to reviewers. Review ease is a good and worthy goal. It's worth upholding clean and discrete commits as a repo standard. At the same time, I want to make sure that for new contributors, we're asking for things that invite people in rather than turn them away.

I hear you, and I value this feedback. I wrote my arguments above, but don't have an ideal answer to improve things concretely. I believe that this is mostly up to the reviewers to use their best judgement and figure out the balance between welcoming new contributors, and making sure that Git history, backports, bisects, remain in a sane condition. Some of the other reviewers assigned to this PR may have opinions on the matter, maybe they'll chime in.

[qmonnet]

Some changes should still be moved between commits (some fuel for the debate...), please see below.

All other items I reported have been addressed, thank you!

[zacharysarah]

@qmonnet

You can, in particular if the title of the PR does not work well as a description for the release notes. If the title of your PR makes for a good description, then you don't have to :)

The title seems pretty good to me, so I removed the release-note block from the description. 😄

[zacharysarah]

Netlify preview looks good, links work: https://deploy-preview-23599--docs-cilium-io.netlify.app/

[qmonnet]

Everything looks in order from my side this time, thanks a lot! (I'd maybe recommend squashing the last two commits.) Preview looks good indeed. Thank you for addressing all the feedback!

The title seems pretty good to me, so I removed the release-note block from the description. 😄

Works for me :)