-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Add documentation for k8s RBAC configuration #1404
Conversation
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
3972ca9
to
c8b9e21
Compare
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.
Thanks @aolwas :)
LGTM
docs/user-guide/kubernetes.md
Outdated
If your cluster is configured with RBAC, you need to authorize Træfɪk to use | ||
kubernetes API using ClusterRole, ServiceAccount and ClusterRoleBinding resources: | ||
|
||
```yaml |
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.
If the example is identical to the inline definition, could we just point to the example instead of repeating it?
(This will avoid getting out of sync.)
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.
by inline definition, you mean the official k8s documentation ?
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.
No I mean the part starting in line 67. It seems to be identical to the example file you also included in this PR.
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.
its a pain, but I feel there is some value in having things both inline (so the documentation can be read) and as a file so the example is runnable...
The ideal thing would be to have something inline the contents of a file into the rendered documentation, I don't know if the tool we are using supports that though?
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.
Unfortunately, it does not seem to be supported.
Personally, I don't mind the flow break. After all, that's what hyperlinking is all about. :-)
That said, I'm not overly enthusiastic on this point. A bit of duplication is acceptable for me here.
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.
@timoreimann You're right, the code in the documentation is embedded as is in the example file. I asked myself if I should provide only the example file but it seems to me more confortable for the reader to have directly the code in the doc.
Tell me what you think is the best.
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.
I'm okay with keeping the inline spec, especially since you and @errm are in favor of it. Happy to let myself get overruled by the majority. :-)
As soon as you address the other point, we can merge.
docs/user-guide/kubernetes.md
Outdated
Kubernetes introduces [Role Based Access Control (RBAC)](https://kubernetes.io/docs/admin/authorization/) in 1.6+ to allow fine-grained control | ||
of Kubernetes resources and api. | ||
|
||
If your cluster is configured with RBAC, you need to authorize Træfɪk to use |
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.
Aren't we using the normal spelling for Traefik these days instead of the special ae combo? @emilevauge?
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.
Indeed, @aolwas could you replace Træfɪk
by Træfik
please ?
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.
I used this spelling because I found it elsewhere in the document. Do you want me to also fix the spelling in the whole document ?
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.
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, thanks.
01ba647
to
fc3cc9a
Compare
Tests green, merging! |
Refs #1379. |
This PR add documentation and manifest example to use Traefik with k8s 1.6+ and RBAC enabled