-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Kubectl command headers in requests: KEP 859 #98952
Kubectl command headers in requests: KEP 859 #98952
Conversation
/milestone v1.21 |
d4f8987
to
e232e6f
Compare
bfd94b4
to
ba140bd
Compare
/priority important-soon |
/triage accepted |
/retest |
/cc @brianpursley |
staging/src/k8s.io/cli-runtime/pkg/genericclioptions/command_headers.go
Outdated
Show resolved
Hide resolved
LGTM. I left one nit comment. Only thing that gave me pause was the special-case handling of proxy in cmd.go. It sort of feels like this detail should be kept inside proxy.go. I'm not familiar enough with why proxy is incompatible with CommandHeaderRoundTripper to know how it could be handled any better than this. All that being said, I think it looks good. |
staging/src/k8s.io/cli-runtime/pkg/genericclioptions/command_headers.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/cli-runtime/pkg/genericclioptions/command_headers.go
Outdated
Show resolved
Hide resolved
42ebf94
to
fa52f0d
Compare
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
fa52f0d
to
02ef178
Compare
/retest |
02ef178
to
211fc12
Compare
/retest |
2 similar comments
/retest |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brianpursley, seans3, soltysh 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 |
BTW, headers starting |
FYI this was discussed on the original KEP PR: kubernetes/enhancements#855 (comment) |
X-Kubectl-Command
to each http request to document which kubectl command spawned the request.-v=7
, showing headers being added to request for a command with subcommands:-v=7
, showing multiple http requests from a single kubectl command (notice the same session id in each request):kubectl get
command):/kind feature
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
SIG CLI KEP 859 Kubectl Command Headers