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

Kubernetes docs: clarify steps around use/creation of TLS assets. #1358

Conversation

OwenTuz
Copy link

@OwenTuz OwenTuz commented Nov 22, 2018

Related to issue #1262 .

While not strictly related to TLS, I'd like to make one or more follow-up PRs to fix issues around old references to TPRs, missing instructions on creating the serviceAccount and roles for RBAC etc. But this is a first pass, hopefully it makes reviewing easier.

@OwenTuz
Copy link
Author

OwenTuz commented Nov 22, 2018

Got my issue numbers wrong, embarassing

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for your contribution 🎈 😃

@OwenTuz
Copy link
Author

OwenTuz commented Nov 23, 2018

Thanks for the fast review! I've updated (sorry) with some of the follow-ups I mentioned above

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

No worries, thank you for improving this further! 😃

Documentation/kubernetes.md Outdated Show resolved Hide resolved
@@ -262,7 +262,7 @@ connectors:
# freeIPA server's CA
rootCA: ca.crt
userSearch:
# Would translate to the query "(&(objectClass=person)(uid=<username>))".
# Would translate to the query "(&(objectClass=posixAccount)(uid=<username>))".
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 good catch! 😄

@srenatus
Copy link
Contributor

Looks good to me. However, I have no idea if it's correct. Please bear with me, let's get someone with sufficient knowledge of the domain to review this. (If you know someone, feel free to pull them in, it doesn't have to be someone with "commit bit". I'll merge when this has been reviewed, then.)

@OwenTuz
Copy link
Author

OwenTuz commented Nov 23, 2018

That sounds good to me, I'm not sure who to call on as this is all based on my own testing but I'd definitely appreciate a second set of eyes.

@OwenTuz
Copy link
Author

OwenTuz commented Nov 26, 2018

@whereisaaron @rendhalver would this be something one of you could weigh in on at all?

@rendhalver
Copy link

Nice work!
Note to self: We should align the chart with these docs.

@srenatus srenatus merged commit 007e4da into dexidp:master Nov 26, 2018
@OwenTuz OwenTuz deleted the issue-1132-initial-kubernetes-documentation-improvements branch November 26, 2018 13:10
@whereisaaron
Copy link

Good update

mmrath pushed a commit to mmrath/dex that referenced this pull request Sep 2, 2019
…etes-documentation-improvements

Kubernetes docs: clarify steps around use/creation of TLS assets.
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.

4 participants