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.

Sign up for GitHub

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

Jump to bottom

Route security management by end user #4590

Merged
merged 1 commit into from
Jun 20, 2017
Merged

Route security management by end user #4590

merged 1 commit into from
Jun 20, 2017

Conversation

pecameron
Copy link

Openshift 3.6

Add a new route annotation "haproxy.router.openshift.io/ip_whitelist"
that specifies a space separated list of white listed source IP
addresses and/or CIDRs. Requests from IP addresses that are not in
the whitelist are dropped.

origin PR 14536
openshift/origin#14536

Trello: TbZPhHKE Route security management by end user
https://trello.com/c/TbZPhHKE/

Bug: 1426562
https://bugzilla.redhat.com/show_bug.cgi?id=1426562

@pecameron
Copy link
Author

@knobunc @bfallonf PTAL

knobunc
knobunc requested changes Jun 16, 2017
View reviewed changes
architecture/core_concepts/routes.adoc Outdated
@@ -807,6 +807,7 @@ For all the items outlined in this section, you can set annotations on the
|`*haproxy.router.openshift.io/rate-limit-connections.rate-tcp*`| Limits the rate at which an IP address can make TCP connections. |
|`*haproxy.router.openshift.io/timeout*` | Sets a server-side timeout for the route. xref:time-units[(TimeUnits)] | `ROUTER_DEFAULT_SERVER_TIMEOUT`
|`*router.openshift.io/haproxy.health.check.interval*`| Sets the interval for the back-end health checks. xref:time-units[(TimeUnits)] | `ROUTER_BACKEND_CHECK_INTERVAL`
|`*haproxy.router.openshift.io/p_whitelist*` | xref:whitelist[Whitelist] |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need a little description here before the xref...

Copy link
Author

Choose a reason for hiding this comment

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

OK

architecture/core_concepts/routes.adoc Outdated
== Route Specific IP Whitelists
You can restrict access to a route to a select set of IP addresses by using the
*haproxy.router.openshift.io/ip_whitelist* annotation on the route. The whitelist is a space
separated list of IP addresses and/or CIDRs for the approved users. Requests from
Copy link
Contributor

Choose a reason for hiding this comment

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

users isn't quite right... how about source addresses, or client addresses or something.

Copy link
Author

Choose a reason for hiding this comment

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

Changed.

architecture/core_concepts/routes.adoc Outdated
Some examples:

When editing a route add the following annotation to define the desired
source ip's. Alternatively, oc annotate route <name> may be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrap the "code" in backticks please: oc annotate route <name>

Copy link
Author

Choose a reason for hiding this comment

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

OK

architecture/core_concepts/routes.adoc Outdated
[[whitelist]]
== Route Specific IP Whitelists
You can restrict access to a route to a select set of IP addresses by using the
*haproxy.router.openshift.io/ip_whitelist* annotation on the route. The whitelist is a space
Copy link
Contributor

Choose a reason for hiding this comment

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

Why asterisks not backticks? I'd have thought we wanted to typeset them in fixed width.

Copy link
Author

Choose a reason for hiding this comment

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

Got me... I changed them.

architecture/core_concepts/routes.adoc Outdated
When editing a route add the following annotation to define the desired
source ip's. Alternatively, oc annotate route <name> may be used.

Allow only one ip:
Copy link
Contributor

Choose a reason for hiding this comment

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

IP address

Copy link
Author

Choose a reason for hiding this comment

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

OK. Cleaning up the rest in the description as well.

architecture/core_concepts/routes.adoc Outdated
haproxy.router.openshift.io/ip_whitelist: 192.168.1.10
----

Several ip's:
Copy link
Contributor

Choose a reason for hiding this comment

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

IP addresses

Copy link
Author

Choose a reason for hiding this comment

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

OK

architecture/core_concepts/routes.adoc Outdated
haproxy.router.openshift.io/ip_whitelist: 192.168.1.10 192.168.1.11 192.168.1.12
----

Ip ranges:
Copy link
Contributor

Choose a reason for hiding this comment

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

IP CIDR network

Copy link
Author

Choose a reason for hiding this comment

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

OK

architecture/core_concepts/routes.adoc Outdated
haproxy.router.openshift.io/ip_whitelist: 192.168.1.0/24
----

Ip's and ranges:
Copy link
Contributor

Choose a reason for hiding this comment

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

Mixed IP addresses and networks

Copy link
Author

Choose a reason for hiding this comment

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

OK

ahardin-rh
ahardin-rh reviewed Jun 16, 2017
View reviewed changes
architecture/core_concepts/routes.adoc Outdated
separated list of IP addresses and/or CIDRs for the approved users. Requests from
IP addresses that are not in the whitelist are dropped.

When the annotation is present for a route, an acl is set up in the route's haproxy backend with the whitelist.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/acl/ACL
What is an ACL?

Copy link
Author

Choose a reason for hiding this comment

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

Deleted 841. It is a level of detail that won't be interesting to the reader. The discussion is found in the haproxy 1.5 documentation. FYI haproxy includes addess control lists (acl). A list can be created and later used in the backend for different purposes. This PR uses the acl as a source of white list source IP addresses.

ahardin-rh
ahardin-rh reviewed Jun 16, 2017
View reviewed changes
architecture/core_concepts/routes.adoc Outdated
Some examples:

When editing a route add the following annotation to define the desired
source ip's. Alternatively, oc annotate route <name> may be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/ip's/IPs

Copy link
Author

Choose a reason for hiding this comment

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

Changed.

@ahardin-rh ahardin-rh added this to the Future Release milestone Jun 16, 2017
@ahardin-rh ahardin-rh self-assigned this Jun 16, 2017
@pecameron
Route security management by end user
4e2929b 
Openshift 3.6

Add a new route annotation "haproxy.router.openshift.io/ip_whitelist"
that specifies a space separated list of white listed source IP
addresses and/or CIDRs. Requests from IP addresses that are not in
the whitelist are dropped.

origin PR 14536
openshift/origin#14536

Trello: TbZPhHKE Route security management by end user
https://trello.com/c/TbZPhHKE/

Bug: 1426562
https://bugzilla.redhat.com/show_bug.cgi?id=1426562
@pecameron
Copy link
Author

@knobunc @ahardin-rh PTAL

@ahardin-rh
Copy link
Contributor

LGTM

@ahardin-rh ahardin-rh merged commit 8c9b16d into openshift:master Jun 20, 2017
@ahardin-rh ahardin-rh mentioned this pull request Jun 20, 2017
Merged
@ahardin-rh
Copy link
Contributor

[rev_history]
|xref:../architecture/core_concepts/routes.adoc#architecture-core-concepts-routes[Core Concepts -> Routes]
|Added a new xref:../architecture/core_concepts/routes.adoc#whitelist[Route-specific IP Whitelists] section.
%

@vikram-redhat vikram-redhat modified the milestones: Future Release, Staging Jul 7, 2017
@pecameron pecameron deleted the bz1426562 branch July 27, 2017 13:26
@vikram-redhat vikram-redhat modified the milestones: Future Release, Staging Aug 9, 2017
@vikram-redhat vikram-redhat modified the milestones: Future Release, Staging, OCP 3.6 GA Aug 9, 2017
@vikram-redhat vikram-redhat modified the milestones: OCP 3.6 GA, Staging, TEMP Sep 7, 2017
@vikram-redhat vikram-redhat modified the milestones: OCP 3.6 GA, Staging, TEMP Sep 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahardin-rh ahardin-rh ahardin-rh left review comments

@knobunc knobunc knobunc requested changes

Assignees

@ahardin-rh ahardin-rh

Labels
branch/dedicated branch/enterprise-3.6 branch/online
Projects
None yet
Milestone
OCP 3.6 GA
Development

Successfully merging this pull request may close these issues.
4 participants
@pecameron @ahardin-rh @knobunc @vikram-redhat