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

Creating application security checklist #46326

Merged

Conversation

AnshumanTripathi
Copy link
Contributor

@AnshumanTripathi AnshumanTripathi commented May 12, 2024

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. language/en Issues or PRs related to English language labels May 12, 2024
@k8s-ci-robot k8s-ci-robot added sig/docs Categorizes an issue or PR as relevant to SIG Docs. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 12, 2024
Copy link

netlify bot commented May 12, 2024

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 86c48ee
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/66f9c132aeb1760008bcb862
😎 Deploy Preview https://deploy-preview-46326--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@AnshumanTripathi AnshumanTripathi force-pushed the anshuman/app_sec_checklist branch 2 times, most recently from 252c1b0 to 1dae29a Compare May 14, 2024 06:11
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/blog Issues or PRs related to the Kubernetes Blog subproject area/localization General issues or PRs related to localization area/release-eng Issues or PRs related to the Release Engineering subproject area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes language/bn Issues or PRs related to Bengali language language/de Issues or PRs related to German language language/es Issues or PRs related to Spanish language language/fr Issues or PRs related to French language language/hi Issues or PRs related to Hindi language language/id Issues or PRs related to Indonesian language language/it Issues or PRs related to Italian language language/ja Issues or PRs related to Japanese language language/ko Issues or PRs related to Korean language language/pl Issues or PRs related to Polish language language/pt Issues or PRs related to Portuguese language and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 10, 2024
@AnshumanTripathi AnshumanTripathi force-pushed the anshuman/app_sec_checklist branch from 78f4f72 to a93d643 Compare July 22, 2024 07:35
@AnshumanTripathi
Copy link
Contributor Author

@NissesSenap I have updated the checklist structure based on your recommendation.

Can you expand on

rbac, if you are using rbac in your http api, you are most likely doing something wrong. I would emphasize that really hard in the document.

I'm not sure what you mean here.

@AnshumanTripathi AnshumanTripathi force-pushed the anshuman/app_sec_checklist branch from a93d643 to c1c6f4a Compare July 22, 2024 07:41
@NissesSenap
Copy link

@NissesSenap I have updated the checklist structure based on your recommendation.

Can you expand on

rbac, if you are using rbac in your http api, you are most likely doing something wrong. I would emphasize that really hard in the document.

I'm not sure what you mean here.

That is great @AnshumanTripathi , I will try to take a deeper look at the update as soon as possible. My point is that the majority of all apps ruining in Kubernetes don't need access to its API. So I would emphasize that custom rbac rules for apps just aren't needed, in the majority of applications. Still good to talk about it, just like you have, but point to that there are limited use cases for it.

@AnshumanTripathi
Copy link
Contributor Author

@NissesSenap I have updated the checklist structure based on your recommendation.
Can you expand on

rbac, if you are using rbac in your http api, you are most likely doing something wrong. I would emphasize that really hard in the document.

I'm not sure what you mean here.

That is great @AnshumanTripathi , I will try to take a deeper look at the update as soon as possible. My point is that the majority of all apps ruining in Kubernetes don't need access to its API. So I would emphasize that custom rbac rules for apps just aren't needed, in the majority of applications. Still good to talk about it, just like you have, but point to that there are limited use cases for it.

Makes sense. I am not covering applications using Kubernetes client as a part of this checklist. I think having a doc about secure practices for applications that use the Kubernetes API (and operator) can be very helpful. I will create an issue for this :)

@AnshumanTripathi
Copy link
Contributor Author

AnshumanTripathi commented Jul 24, 2024

@NissesSenap created kubernetes/sig-security#121 for hardening guide for using Kubernetes API
Cross posted issue on k website - #47405

@tengqm
Copy link
Contributor

tengqm commented Sep 22, 2024

I'd suggest we place this under content/en/docs/reference/security/, a new directory for consolidating security related contents. This page is not suitable for the concepts section.

@AnshumanTripathi
Copy link
Contributor Author

I'd suggest we place this under content/en/docs/reference/security/, a new directory for consolidating security related contents. This page is not suitable for the concepts section.

Actually we can move this checklist and the existing checklist to content/en/docs/reference/issues-security maybe? I dont think I should do that as a part of this change though.

@tengqm
Copy link
Contributor

tengqm commented Sep 22, 2024

I'd suggest we place this under content/en/docs/reference/security/, a new directory for consolidating security related contents. This page is not suitable for the concepts section.

Actually we can move this checklist and the existing checklist to content/en/docs/reference/issues-security maybe? I dont think I should do that as a part of this change though.

The issues-security directory is positioned to be a folder for disclosing the vulnerabilities of kubernetes release. What we are trying to compile is a collection of guidelines, best practices for users.

I don't think we want to kick this in and then follow up with another PR to move it.
We may want to get the page into a suitable folder in the first place.
The migration of other relevant pages could be left as follow-ups, for sure.

@AnshumanTripathi
Copy link
Contributor Author

I'd suggest we place this under content/en/docs/reference/security/, a new directory for consolidating security related contents. This page is not suitable for the concepts section.

Actually we can move this checklist and the existing checklist to content/en/docs/reference/issues-security maybe? I dont think I should do that as a part of this change though.

The issues-security directory is positioned to be a folder for disclosing the vulnerabilities of kubernetes release. What we are trying to compile is a collection of guidelines, best practices for users.

I don't think we want to kick this in and then follow up with another PR to move it. We may want to get the page into a suitable folder in the first place. The migration of other relevant pages could be left as follow-ups, for sure.

Makes sense. I'll try to make that change

@drackpack drackpack mentioned this pull request Sep 22, 2024
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks! We could really use this new page.

Here's some advice about how to get this ready to merge.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 29, 2024
@AnshumanTripathi AnshumanTripathi force-pushed the anshuman/app_sec_checklist branch 2 times, most recently from 06322b3 to 18bfd54 Compare September 29, 2024 21:04
Signed-off-by: Anshuman Tripathi <[email protected]>

Update based on feedback

Signed-off-by: Anshuman Tripathi <[email protected]>

Update based on feedback

Signed-off-by: Anshuman Tripathi <[email protected]>

Update checklist reading guide

Signed-off-by: Anshuman Tripathi <[email protected]>

Update checklist structure based on feedback

Signed-off-by: Anshuman Tripathi <[email protected]>

Apply suggestions from code review

Adding the suggestions from the last review.

Co-authored-by: Tim Bannister <[email protected]>

Fix style

Signed-off-by: Anshuman Tripathi <[email protected]>
@AnshumanTripathi AnshumanTripathi force-pushed the anshuman/app_sec_checklist branch from 18bfd54 to 86c48ee Compare September 29, 2024 21:05
@tengqm
Copy link
Contributor

tengqm commented Sep 30, 2024

This page, in its current format, with its current contents, doesn't belong to the concepts section. Please consider place it under the references section.

@reylejano
Copy link
Member

I think this proposed "application security checklist" aligns with the live "security checklist" page which is aimed towards operators/cluster admins.
Since the "security checklist" page is currently under concepts, I'm ok to merge the "application security checklist" since under concepts

If we decide references is a better place then a PR can be made to move both of the "checklists" and possibly more like the
Authentication Mechanism Hardening Guide

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: reylejano

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 25, 2024
@savitharaghunathan
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 25, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 36479c14b720d1addb2eb45e29c2988783bab833

@k8s-ci-robot k8s-ci-robot merged commit 739b4e4 into kubernetes:main Oct 25, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/security Categorizes an issue or PR as relevant to SIG Security. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants