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

Include an explicit list of kubernetes protocol annotations in docs. #4170

Merged
merged 3 commits into from
Nov 15, 2018
Merged

Include an explicit list of kubernetes protocol annotations in docs. #4170

merged 3 commits into from
Nov 15, 2018

Conversation

shanna
Copy link

@shanna shanna commented Nov 7, 2018

What does this PR do?

Includes an explicit list of valid protocols in kubernetes backend documentation.

Motivation

The idea of using port 443 or the special ingress spec port name seemed like something that will trip up other developers on my team. I saw the ingress.kubernetes.io/protocol annotation in the documentation but couldn't find the valid values. Both http2 and h2 as someone suggested on slack both produced errors in the log so I was forced to search the source code.

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

Can the special port 443 and https prefix logic be deprecated and eventually removed now there is an explicit way to override the default protocol in the kubernetes back end?

@shanna shanna requested a review from a team as a code owner November 7, 2018 17:04
@ldez ldez changed the base branch from master to v1.7 November 8, 2018 07:19
@ldez ldez added this to the 1.7 milestone Nov 8, 2018
Copy link
Collaborator

@SantoDE SantoDE left a comment

Choose a reason for hiding this comment

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

LGTM 👼

To your note: Yeah, I think we can deprecate it imho. Let's see what the others have to add on that :)

docs/configuration/backends/kubernetes.md Outdated Show resolved Hide resolved
Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@traefiker traefiker merged commit 18962b6 into traefik:v1.7 Nov 15, 2018
@shanna shanna deleted the kubernetes-protocol-annotation branch November 15, 2018 10:23
@ldez ldez mentioned this pull request Nov 19, 2018
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants