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

Proposal for adding headers with the command name to kubectl requests #855

Merged
merged 4 commits into from
Mar 14, 2019
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 219 additions & 0 deletions keps/sig-cli/kubectl-commands-in-headers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
---
title: Kubectl Commands In Headers
authors:
- "@pwittrock"
owning-sig: sig-cli
participating-sigs:
- sig-api-machinery
reviewers:
- "@lavalamp"
- "@kow3ns"
approvers:
- "@soltysh"
- "@seans3"
editor: TBD
creation-date: 2019-02-22
last-updated: 2019-02-22
status: implementable
see-also:
replaces:
superseded-by:
---

# kubectl-commands-in-headers


## Table of Contents

A table of contents is helpful for quickly jumping to sections of a KEP and for highlighting any additional information provided beyond the standard KEP template.
[Tools for generating][] a table of contents from markdown are available.

- [Table of Contents](#table-of-contents)
- [Release Signoff Checklist](#release-signoff-checklist)
- [Summary](#summary)
- [Motivation](#motivation)
- [Goals](#goals)
- [Non-Goals](#non-goals)
- [Proposal](#proposal)
- [Risks and Mitigations](#risks-and-mitigations)
- [Design Details](#design-details)
- [Test Plan](#test-plan)
- [Implementation History](#implementation-history)

[Tools for generating]: https://github.com/ekalinin/github-markdown-toc

## Release Signoff Checklist

- [ ] kubernetes/enhancements issue in release milestone, which links to KEP (this should be a link to the KEP location in kubernetes/enhancements, not the initial KEP PR)
- [x] KEP approvers have set the KEP status to `implementable`
- [x] Design details are appropriately documented
- [ ] Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
- [x] Graduation criteria is in place
- [ ] "Implementation History" section is up-to-date for milestone
- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
- [ ] Supporting documentation e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes

[kubernetes.io]: https://kubernetes.io/
[kubernetes/enhancements]: https://github.com/kubernetes/enhancements/issues
[kubernetes/kubernetes]: https://github.com/kubernetes/kubernetes
[kubernetes/website]: https://github.com/kubernetes/website

## Summary

Requests sent to the apiserver from kubectl include http request headers with context about the kubectl command that
created the request. This information could be used by cluster administrators for debugging or
to gather telemetry about how users are interacting with the cluster.


## Motivation

Kubectl generates requests sent to the apiserver for commands such as `apply`, `delete`, `edit`, `run`, however
the context of the command for the requests is lost and unavailable to cluster administrators. Context would be
useful to cluster admins both for debugging the cause of requests as well as providing telemetry about how users
are interacting with the cluster.

### Goals

- Allow cluster administrators to identify how requests in the logs were generated from
Copy link
Contributor

Choose a reason for hiding this comment

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

That’s it? No other goals?

Copy link
Member Author

Choose a reason for hiding this comment

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

What did you have in mind?

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried to add some color here. LMK if it makes sense to you.

kubectl commands.

### Non-Goals

NA
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should explicitly indicate that this information is not provided to be used by the apiserver to determine behavior of a request in any way. I remember there was an occasion a few years back where someone tried to make the server reply differently based on which user-agent came through.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made this an anti-goal, which is a bit stronger.


## Proposal

Include in requests made from kubectl to the apiserver:

- the kubectl subcommand
- which flags were specified (but not the values)
Copy link
Contributor

Choose a reason for hiding this comment

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

will there be an opt-in for certain cases? I'd like to see the kubectl patch --type value to know which kinds of patches are actually being frequently used.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it might be useful to have a mechanism to indicate which flag values are sensitive or not. -o for example is complex because some values can be sensitive (-o column-name=...) and some aren't (-o yaml). We could improve the flag registration mechanisms that we use in kubectl to have this sort of semantic maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

SGTM. We could start with a whitelisted set of flags we will include an enumerated value for (never arbitrary values supplied by the user). We can expand the list over time. That was the intention for -f and -o values.

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM. We could start with a whitelisted set of flags we will include an enumerated value for (never arbitrary values supplied by the user). We can expand the list over time. That was the intention for -f and -o values.

Yeah, I think that would work well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

- enum values for stdin and stdout
- a generated session id

### Kubectl-Command Header

The `Kubectl-Command` Header contains the kubectl sub command. It contains
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren’t we supposed to put X- in front of custom headers,

Copy link
Member Author

@pwittrock pwittrock Feb 28, 2019

Choose a reason for hiding this comment

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

My understanding is that it is deprecated: link

Copy link
Member

Choose a reason for hiding this comment

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

I think the deprecation is primarily for headers that may be standardized in an RFC at some point in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good to know. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

My reading of that RFC is that X- prefixes are deprecated pretty much for all cases - hence the title Deprecating the "X-" Prefix and Similar Constructs in Application Protocols

If it's not too late to fix this before the beta, it'd be great to sort that out.

the path to the subcommand (e.g. `kubectl apply`) to disambiguate sub commands
that might have the same name and different paths.

Examples:

- `Kubectl-Command: kubectl apply`
- `Kubectl-Command: kubectl create secret tls`
- `Kubectl-Command: kubectl delete`
- `Kubectl-Command: kubectl get`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd this report just Kubectl-Command: get for example, we already know it's kubectl command (from header Kubectl-command), why repeat yourself.

Copy link
Member

Choose a reason for hiding this comment

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

And I don't think we want to know how people name their kubectl command on their filesystem.

Copy link
Member Author

Choose a reason for hiding this comment

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

SGTM


### Kubectl-Flags Header

The `Kubectl-Flags` Header contains a list of the kubectl flags that were provided to the sub
command. It does *not* contain the flag values, only the names of the flags. It
does not normalize into short or long form. If a flag is provided multiple times
it will appear multiple times in the list. Flags are sorted alpha-numerically and
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you want to sort them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will make it easier for humans to read.

separated by a ','.

Examples:

- `Kubectl-Flags: --filename,--recursive`
- `Kubectl-Flags: -f,-f,-f,-R`
- `Kubectl-Flags: --dry-run,-o`

### Kubectl-Input Header

The `Kubectl-Input` Header contains the types of input used. Because the flag values are not
provided, this can be used to determine if the command input was from STDIN, Local Files or
Remote Files. Format is a list of the types of input provided.

Examples:

- `Kubectl-Input: stdin`
- `Kubectl-Input: file`
- `Kubectl-Input: http`
- `Kubectl-Input: file,stdin`
- `Kubectl-Input: file,file,http`

### Kubectl-Output Header

The `Kubectl-Output` Header contains the type of output used. Because the flag values are
not provided, this can be used to determine if the output is yaml,json,go-template,wide.
Note: it does *not* contain non-enumeration values, such as the custom-columns values for
for custom-columns output.

Examples:

- `Kubectl-Output: yaml`
Copy link
Contributor

Choose a reason for hiding this comment

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

This will overlap with Kubectl-flags or you're planning to exclude output-related flags?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or this will let us differentiate between default output and requested output, like you're describing in your example?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. I could actually move these into flags as enums.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

- `Kubectl-Output: json`
- `Kubectl-Output: wide`
- `Kubectl-Output: default`
- `Kubectl-Output: jsonpath`
- `Kubectl-Output: custom-columns`
- `Kubectl-Output: custom-columns-file`

### Kubectl-Session Header

The `Kubectl-Session` Header contains a Session ID that can be used to identify that multiple
requests were made from the same kubectl command invocation. The Session Header is generated
once for each kubectl process.

- `Kubectl-Session: 67b540bf-d219-4868-abd8-b08c77fefeca`
Copy link
Contributor

Choose a reason for hiding this comment

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

Very good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was @apelisse 's idea :)


### Example

```sh
$ kubectl apply -f - -o yaml
```

```
Kubectl-Command: kubectl apply
Kubectl-Flags: -f
Kubectl-Input: stdin
Kubectl-Output: yaml
Kubectl-Session: 67b540bf-d219-4868-abd8-b08c77fefeca
```


```sh
$ kubectl apply -f ./local/file -o=custom-columns=NAME:.metadata.name
```

```
Kubectl-Command: kubectl apply
Kubectl-Flags: -f;-o
Kubectl-Input: file
Kubectl-Output: custom-columns
Kubectl-Session: 0087f200-3079-458e-ae9a-b35305fb7432
```

### Risks and Mitigations

Unintentionally including sensitive information in the request headers - such as local directory paths
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't pass flag values, so that should not be a concern.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

or cluster names.

Mitigations: only include the following non-sensative information:
Copy link
Contributor

Choose a reason for hiding this comment

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

sensitive

Copy link
Member

Choose a reason for hiding this comment

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

hm?

Copy link
Member Author

Choose a reason for hiding this comment

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

spelling. thx


- The sub command itself
- Which flags were specified, but not their values. e.g. `-f`, but not the argument
- What type of input was specified (e.g. stdin, local files, remote files)
- What type of output was specified (e.g. yaml, json, wide, default, name, etc)

## Design Details

### Test Plan

- Verify the Header is sent for commands and has the right value
- Verify the Header is sent for flags and has the right value

### Graduation Criteria

NA

### Upgrade / Downgrade Strategy

NA

### Version Skew Strategy

NA

## Implementation History