Add datapolicy tags to pkg/kubelet/ #96004

serathius · 2020-10-29T17:43:58Z

/kind feature

What this PR does / why we need it:
This PR adds "datapolicy" tags to golang structures as described in Defend against logging secrets via static analysis. Those tags will be used by for ensuring this data will not be written to logs by Kubernetes system components.

List of datapolicy tags available:

security-key - for TLS certificate keys. Keywords: key, cert, pem
token - for HTTP authorization tokens. Keywords: token, secret, header, auth
password - anything passwordlike. Keywords: password

Special notes for your reviewer:

Due to size of Kubernetes codebase first iteration of tagging was done based on greping for particular keyword. Please ensure that tagged fields do contain type of sensitive data that matches their tag. Feel free to suggest any additional places that you think should be tagged.

Does this PR introduce a user-facing change?:
No

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/issues/1933

/cc @PurelyApplied
/sig instrumentation security
/priority important-soon
/milestone v1.20

k8s-ci-robot · 2020-10-29T17:44:00Z

@serathius: GitHub didn't allow me to request PR reviews from the following users: PurelyApplied.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/kind feature

What this PR does / why we need it:
This PR adds "datapolicy" tags to golang structures as described in Kubernetes system components logs sanitization KEP. Those tags will be used by for ensuring this data will not be written to logs written by Kubernetes system components.

List of datapolicy tags available:

security-key - for TLS certificate keys. Keywords: key, cert, pem

token - for HTTP authorization tokens. Keywords: token, secret, header, auth

password - anything passwordlike. Keywords: password

Special notes for your reviewer:

Due to size of Kubernetes codebase first iteration of tagging was done based on greping for particular keyword. Please ensure that tagged fields do contain type of sensitive data that matches their tag. Feel free to suggest any additional places that you think should be tagged.

Does this PR introduce a user-facing change?:
No
NONE
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/1753-logs-sanitization
/cc @PurelyApplied
/sig instrumentation security
/priority important-soon
/milestone v1.20

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/test-infra repository.

serathius · 2020-10-29T17:47:44Z

/assign @yujuhong

serathius · 2020-10-29T18:53:06Z

/retest

serathius · 2020-11-02T12:37:21Z

ping @yujuhong

serathius · 2020-11-10T14:04:37Z

/milestone clear

ehashman

/lgtm

As per #95997 I'm assuming the StaticPodURLHeader headers could potentially contain auth info.

dashpole · 2021-01-13T17:17:56Z

/triage accepted

ehashman · 2022-03-03T00:42:00Z

/remove-lifecycle rotten
/reopen

k8s-ci-robot · 2022-03-03T00:42:18Z

@ehashman: Reopened this PR.

In response to this:

/remove-lifecycle rotten
/reopen

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/test-infra repository.

ehashman · 2022-03-03T00:48:29Z

/assign @liggitt

see #107121 (review)

liggitt · 2022-03-03T14:06:12Z

pkg/kubelet/apis/config/types.go

@@ -92,7 +92,7 @@ type KubeletConfiguration struct {
 	// staticPodURL is the URL for accessing static pods to run
 	StaticPodURL string


the URL can contain sensitive info as well as query parameters, right?

liggitt · 2022-03-03T14:08:11Z

The description links to a KEP marked as deprecated. Is there an updated link or doc describing why we're adding these tags (for static analysis only, I assume)

liggitt · 2022-03-03T14:11:35Z

/approve
from API perspective, adding the tag is fine
/hold for additional tag on the URL field and an updated PR description to a non-deprecated doc/reference

k8s-ci-robot · 2022-03-03T14:12:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ehashman, liggitt, mrunalp, serathius

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:

~~pkg/kubelet/OWNERS~~ [liggitt,mrunalp]
~~pkg/kubelet/apis/config/OWNERS~~ [liggitt]

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

ehashman · 2022-03-03T16:45:00Z

The description links to a KEP marked as deprecated. Is there an updated link or doc describing why we're adding these tags (for static analysis only, I assume)

While the linked KEP is deprecated, these are still used by the stable KEP 1933. Let me see if someone can edit that into the docs release note.

liggitt · 2022-03-03T19:26:47Z

I updated the PR description to link to that KEP instead, thanks

ehashman · 2022-03-03T21:59:35Z

/hold cancel

k8s-triage-robot · 2022-03-04T02:48:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2022-03-04T07:18:52Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

serathius · 2022-03-04T09:34:33Z

The description links to a KEP marked as deprecated. Is there an updated link or doc describing why we're adding these tags (for static analysis only, I assume)

Done

k8s-triage-robot · 2022-03-04T11:38:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2022-03-04T15:58:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2022-03-04T20:20:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

Add datapolicy tags to pkg/kubelet/

035469b

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Oct 29, 2020

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. labels Oct 29, 2020

k8s-ci-robot added this to the v1.20 milestone Oct 29, 2020

k8s-ci-robot assigned yujuhong Oct 29, 2020

serathius mentioned this pull request Oct 29, 2020

Kubernetes system components logs sanitization kubernetes/enhancements#1753

Closed

k8s-ci-robot removed this from the v1.20 milestone Nov 10, 2020

mlevesquedion mentioned this pull request Dec 1, 2020

Avoid logging headers from kubelet configuration #97000

Merged

ehashman added this to Needs Reviewer in SIG Node PR Triage Jan 6, 2021

ehashman approved these changes Jan 6, 2021

View reviewed changes

ehashman moved this from Needs Reviewer to Needs Approver in SIG Node PR Triage Jan 6, 2021

k8s-ci-robot assigned ehashman Jan 6, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 6, 2021

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 13, 2021

ehashman mentioned this pull request Jan 13, 2021

Request: Add ehashman as sig-node-reviewer #98036

Merged

13 tasks

serathius closed this May 29, 2021

k8s-ci-robot reopened this Mar 3, 2022

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Mar 3, 2022

mrunalp approved these changes Mar 3, 2022

View reviewed changes

k8s-ci-robot assigned liggitt Mar 3, 2022

liggitt added the api-review Categorizes an issue or PR as actively needing an API review. label Mar 3, 2022

liggitt added this to In progress in API Reviews Mar 3, 2022

liggitt reviewed Mar 3, 2022

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2022

liggitt moved this from In progress to API review completed, 1.24 in API Reviews Mar 3, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2022

k8s-ci-robot merged commit 5d6ef39 into kubernetes:master Mar 4, 2022

k8s-ci-robot added this to the v1.24 milestone Mar 4, 2022

SataQiu mentioned this pull request Aug 17, 2022

kubelet: add datapolicy for v1beta1.KubeletConfiguration #111889

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add datapolicy tags to pkg/kubelet/ #96004

Add datapolicy tags to pkg/kubelet/ #96004

serathius commented Oct 29, 2020 •

edited

k8s-ci-robot commented Oct 29, 2020

serathius commented Oct 29, 2020

serathius commented Oct 29, 2020

serathius commented Nov 2, 2020

serathius commented Nov 10, 2020

ehashman left a comment

dashpole commented Jan 13, 2021

ehashman commented Mar 3, 2022

k8s-ci-robot commented Mar 3, 2022

ehashman commented Mar 3, 2022

liggitt Mar 3, 2022

liggitt commented Mar 3, 2022

liggitt commented Mar 3, 2022 •

edited

k8s-ci-robot commented Mar 3, 2022

ehashman commented Mar 3, 2022

liggitt commented Mar 3, 2022

ehashman commented Mar 3, 2022

k8s-triage-robot commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

serathius commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

		@@ -92,7 +92,7 @@ type KubeletConfiguration struct {
		// staticPodURL is the URL for accessing static pods to run
		StaticPodURL string

Add datapolicy tags to pkg/kubelet/ #96004

Add datapolicy tags to pkg/kubelet/ #96004

Conversation

serathius commented Oct 29, 2020 • edited

k8s-ci-robot commented Oct 29, 2020

serathius commented Oct 29, 2020

serathius commented Oct 29, 2020

serathius commented Nov 2, 2020

serathius commented Nov 10, 2020

ehashman left a comment

Choose a reason for hiding this comment

dashpole commented Jan 13, 2021

ehashman commented Mar 3, 2022

k8s-ci-robot commented Mar 3, 2022

ehashman commented Mar 3, 2022

liggitt Mar 3, 2022

Choose a reason for hiding this comment

liggitt commented Mar 3, 2022

liggitt commented Mar 3, 2022 • edited

k8s-ci-robot commented Mar 3, 2022

ehashman commented Mar 3, 2022

liggitt commented Mar 3, 2022

ehashman commented Mar 3, 2022

k8s-triage-robot commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

serathius commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

k8s-triage-robot commented Mar 4, 2022

serathius commented Oct 29, 2020 •

edited

liggitt commented Mar 3, 2022 •

edited