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.

Sign up for GitHub

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

Jump to bottom

Draft for review of security for plugin developers #4701

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Draft for review of security for plugin developers #4701

wants to merge 18 commits into from

Conversation

StackScribe
Copy link
Contributor

@StackScribe StackScribe commented Nov 8, 2021 •
edited

This started as the beginnings of a page that summarizes security issues for plugin developers and maintainers. The concept is that this can serve as a checklist and also that other docs (like the Plugin Devloper Tutorial that is under construction) can link to this page to provide current information so we do not have to maintain these links in multiple places.

The general concept is good but this belongs in /doc/developer rather than /doc/book. This PR moves the current content of /doc/developer/security/index.adoc into a separate security-architecture file/page and starts the summary/checklist material (with links) in index.adoc. See discussion below.

This makes this independent of #4612.

@daniel-beck @Wadeck @MarkEWaite @dheerajodha

@StackScribe StackScribe requested a review from a team as a code owner November 8, 2021 01:33
@probot-autolabeler probot-autolabeler bot added the documentation Jenkins documentation, including user and developer docs, solution pages, etc. label Nov 8, 2021
@daniel-beck
Copy link
Contributor

Developer docs are entirely separate (doc/book vs. doc/developer) so probably better to restructure https://www.jenkins.io/doc/developer/security/ to make sense; moving that top-level content to a page "Authentication and Authorization" or so, and then you have a reasonable index page to work with. That's been on my todo list for a while, I've just gotten by directly linking to child pages as needed.

kwhetstone
kwhetstone reviewed Nov 9, 2021
View reviewed changes
Copy link
Contributor

@kwhetstone kwhetstone left a comment

Choose a reason for hiding this comment

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

I like that it's shorter with links out to appropriate pages so developers can go to the pages they most care about.

content/doc/book/security/plugin-developer.adoc Outdated Show resolved Hide resolved
@StackScribe
Copy link
Contributor Author

@daniel-beck I would be happy to work on https://www.jenkins.io/doc/developer/security/ with you. I think @MarkEWaite and @dheerajodha are going to fix the doc/developer structure so it works like doc/book. So do I understand you to say that you want the current copy of that developer/security section broken up into smaller files with an index.adoc file that is this shorter "checklist" of topics with links to other docs? That would make sense. And then the doc/book/security/index.adoc file could just mention the doc/developer/security section with a link.

Should some of doc/book/security/controller-isolation/required-role-check/ be moved to the doc/developer/security section as well? Or maybe the developer part is already provided in http://localhost:4242/doc/developer/security/remoting-callables/ ? Let's discuss when you have time and then I can implement.

@StackScribe
Copy link
Contributor Author

I reread the content with Daniel's remarks in my head and I think I understand what he wants so I did it:

  • Moved the existing contents of /doc/developer/security/index.adoc into a new security-architecture.adoc file
  • Incorporated the content that I had created for /doc/book/security/plugin-developer.adoc into the /doc/developer/security/index.adoc file
  • Deleted the /doc/book/security/plugin-developer.adoc file
    I believe this conforms to the structure that Daniel recommends and it gives us the summary list with links in /doc/developer/security/index.adoc, which really makes more sense.

@daniel-beck
Copy link
Contributor

Should some of doc/book/security/controller-isolation/required-role-check/ be moved to the doc/developer/security section as well? Or maybe the developer part is already provided in http://localhost:4242/doc/developer/security/remoting-callables/ ?

Yes, that should be the separation already; if there's developer content on the admin docs, it should be removed; but all content should be admin suitable.

are going to fix the doc/developer structure so it works like doc/book

Other than the index page, it already does, kinda? Curious to see how that would look though.

And then the doc/book/security/index.adoc file could just mention the doc/developer/security section with a link.

Maybe? Different audiences, so unsure how much of that we need. There are references in the role check doc because it's fairly likely people end up there when things go wrong, so pointing to instructions how to fix (even if implemented by someone else) makes sense IMO.

@StackScribe
Copy link
Contributor Author

The navigation is different for /dev/developer and /dev/book. Specifically:

  • Contents of left frame does not collapse/expand by chapter
  • No breadcrumbs accross the top
  • Rendered pages do not display subsections in the upper right corner
  • Ergo, it's not possible to link to a subsection of a page from another doc
    None of these are real show-stoppers but I think that fixing them will not be terribly disruptive.

@daniel-beck
Copy link
Contributor

daniel-beck commented Nov 14, 2021 •
edited

Ergo, it's not possible to link to a subsection of a page from another doc

Most pages on jenkins.io have an 'anchor' link next to each header, it's to the right and appears on hover (unlike GitHub's which is to the left).

Screenshot

Click it and you jump to the anchor. Explicit anchor definitions from Asciidoc also exist, see e.g. https://github.com/jenkins-infra/jenkins.io/blame/d178e145d26822666ff0040c6a451dbdbbcdffdf/content/_data/upgrades/2-303-3.adoc#L3-L4 to get https://www.jenkins.io/doc/upgrade-guide/2.303/#SECURITY-2458

@kwhetstone
Copy link
Contributor

Meg, anything else we need to check for this?

@MarkEWaite MarkEWaite added the stalled Pull requests that are not progressing label Apr 20, 2023
@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Apr 21, 2023
@github-actions
Copy link
Contributor

Please take a moment and address the merge conflicts of your pull request. Thanks!

StackScribe and others added 4 commits May 9, 2024 20:07
@StackScribe @lemeurherve
delete duplicate item about disk storage
318aeaf 
Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
@StackScribe @lemeurherve
capitalization
82cdd09 
Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
@StackScribe @lemeurherve
Duplicated text
67a4e53 
Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
@StackScribe
wordsmithing CLI section
1875608
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwhetstone kwhetstone kwhetstone left review comments

@lemeurherve lemeurherve lemeurherve left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
documentation Jenkins documentation, including user and developer docs, solution pages, etc. stalled Pull requests that are not progressing unresolved-merge-conflict There is a merge conflict with the target branch. work-in-progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@StackScribe @daniel-beck @kwhetstone @lemeurherve @MarkEWaite