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

Specify Warning header usage #393

Merged
merged 2 commits into from
Mar 19, 2023
Merged

Specify Warning header usage #393

merged 2 commits into from
Mar 19, 2023

Conversation

imjasonh
Copy link
Member

Fixes #390

This is heavily inspired by Kubernetes' support for warnings returned by the API server and surfaced by clients like kubectl: https://kubernetes.io/blog/2020/09/03/warnings/

tl;dr: Registries MAY return informational warning messages in the Warning header, as described in https://www.rfc-editor.org/rfc/rfc7234#section-5.5, with the added restriction that the warn-code must be 299 and the warn-agent must be unset (-). Clients SHOULD surface those messages to the user in an unobtrusive and helpful way -- how they do that is up to them.

Blatantly cribbing further helpful advice from #390 (comment):

From https://kubernetes.io/blog/2020/09/03/warnings/#admission-webhooks

If you are implementing a webhook that returns a warning message, here are some tips:

  • Don't include a "Warning:" prefix in the message (that is added by clients on output)
  • Use warning messages to describe problems the client making the API request should correct or be aware of
  • Be brief; limit warnings to 120 characters if possible

and from #390 (comment)

One more side note: a key provision of the warnings RFC is that for warnings returned with code 299: "A system receiving this warning MUST NOT take any automated action." Kubernetes leans heavily on this to assume it is always safe to return a warning or stop returning a warning without backwards compatibility implications. You would think it goes without saying, but warning header contents are not APIs.

cc @BenTheElder @liggitt

@jonjohnsonjr @jdolitsky @sudo-bmitch @sajayantony

spec.md Outdated Show resolved Hide resolved
spec.md Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
spec.md Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
Signed-off-by: Jason Hall <jason@chainguard.dev>
spec.md Outdated Show resolved Hide resolved
Signed-off-by: Jason Hall <jason@chainguard.dev>
Copy link

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: server-sent warnings
7 participants