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

Docs: Egress router DNS proxy mode #4854

Merged
merged 1 commit into from
Jul 28, 2017

Conversation

pravisankar
Copy link

@pravisankar
Copy link
Author

@openshift/networking @openshift/team-documentation PTAL

@vikram-redhat
Copy link
Contributor

@bfallonf - PTAL.

@pravisankar - is this for 3.7?

@pravisankar
Copy link
Author

pravisankar commented Jul 27, 2017 via email

@bfallonf
Copy link

Thanks @pravisankar . This is good, so I'll merge and tag for the 3.7 release. I'll attach it to the docs card can do a followup later if needed.

@bfallonf
Copy link

[rev_history]
|xref:../admin_guide/managing_networking.adoc#admin-guide-manage-networking[Managing Networking]
|Added the xref:../admin_guide/managing_networking.adoc#admin-guide-deploying-an-egress-router-dns-proxy-pod[Deploying an Egress Router DNS Proxy Pod] section.
%

@bfallonf bfallonf merged commit ded21db into openshift:master Jul 28, 2017
@bfallonf
Copy link

privileged: true
envFrom: <2>
- configMapRef:
name: egress-router-env
Copy link
Contributor

Choose a reason for hiding this comment

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

Blah, just got back from PTO and noticed this. It's not safe to use envFrom here, because that might allow project admins to subvert the functioning of the (privileged) origin-egress-router pod by setting variables like IFS or PATH in the environment that egress-router.sh will run in.

Given that the PR implementing this feature hasn't landed yet anyway, maybe this PR should just be reverted?

Copy link
Author

Choose a reason for hiding this comment

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

You mean project admin sets undesirable environment variables in configMap and cluster admin redeploys the egress router without noticing this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Copy link

Choose a reason for hiding this comment

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

Thanks for pointing this out, @danwinship . @pravisankar submitted a fix in another PR, so I'll merge that.

@ghyde
Copy link
Contributor

ghyde commented Sep 5, 2017

Can we revert this PR, merging it back after the feature has landed? From a support perspective, it's frustrating for customers when they follow the documentation and try to implement a feature that doesn't exist yet.

@pravisankar
Copy link
Author

@bfallonf
To support this feature, we need at least HAproxy 1.6+ but unfortunately RHEL carries older version of haproxy. We are trying to package the needed version, trello card: https://trello.com/c/W3TQb4FF but this is unlikely to land in 3.7 release. So the egress DNS proxy mode feature has been pushed to 3.8 release (updated in https://trello.com/c/407uoUFz).
As @ghyde pointed out, we should revert this commit.

@bfallonf
Copy link

bfallonf commented Sep 6, 2017

Sure thing. I've created a revert PR in #5187 . I've changed the labels to 3.8 for this and the follow up PR. @ghyde @pravisankar Please let me know if there's anything more needed here.

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