Skip to content

Commit

Permalink
WIP: tcp ingress
Browse files Browse the repository at this point in the history
  • Loading branch information
marun committed Aug 8, 2016
1 parent 495eaeb commit 13d20e0
Show file tree
Hide file tree
Showing 14 changed files with 1,639 additions and 14 deletions.
5 changes: 5 additions & 0 deletions pkg/cmd/server/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,11 @@ type MasterNetworkConfig struct {
// CIDR will be rejected. Rejections will be applied first, then the IP checked against one of the allowed CIDRs. You
// should ensure this range does not overlap with your nodes, pods, or service CIDRs for security reasons.
ExternalIPNetworkCIDRs []string
// IngressIPNetworkCIDR controls the range to assign ingress ips from for services of type LoadBalancer on bare
// metal. If empty, ingress ips will not be assigned. It may contain a single CIDR that will be allocated from.
// For security reasons, you should ensure that this range does not overlap with the CIDRs reserved for external ips,
// nodes, pods, or services.
IngressIPNetworkCIDR string
}

type ImageConfig struct {
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/server/api/v1/swagger_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ var map_MasterNetworkConfig = map[string]string{
"hostSubnetLength": "HostSubnetLength is the number of bits to allocate to each host's subnet e.g. 8 would mean a /24 network on the host",
"serviceNetworkCIDR": "ServiceNetwork is the CIDR string to specify the service networks",
"externalIPNetworkCIDRs": "ExternalIPNetworkCIDRs controls what values are acceptable for the service external IP field. If empty, no externalIP may be set. It may contain a list of CIDRs which are checked for access. If a CIDR is prefixed with !, IPs in that CIDR will be rejected. Rejections will be applied first, then the IP checked against one of the allowed CIDRs. You should ensure this range does not overlap with your nodes, pods, or service CIDRs for security reasons.",
"ingressIPNetworkCIDR": "IngressIPNetworkCIDR controls the range to assign ingress ips from for services of type LoadBalancer on bare metal. If empty, ingress ips will not be assigned. It may contain a single CIDR that will be allocated from. For security reasons, you should ensure that this range does not overlap with the CIDRs reserved for external ips, nodes, pods, or services.",
}

func (MasterNetworkConfig) SwaggerDoc() map[string]string {
Expand Down
5 changes: 5 additions & 0 deletions pkg/cmd/server/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,11 @@ type MasterNetworkConfig struct {
// CIDR will be rejected. Rejections will be applied first, then the IP checked against one of the allowed CIDRs. You
// should ensure this range does not overlap with your nodes, pods, or service CIDRs for security reasons.
ExternalIPNetworkCIDRs []string `json:"externalIPNetworkCIDRs"`
// IngressIPNetworkCIDR controls the range to assign ingress ips from for services of type LoadBalancer on bare
// metal. If empty, ingress ips will not be assigned. It may contain a single CIDR that will be allocated from.
// For security reasons, you should ensure that this range does not overlap with the CIDRs reserved for external ips,
// nodes, pods, or services.
IngressIPNetworkCIDR string `json:"ingressIPNetworkCIDR"`
}

// ImageConfig holds the necessary configuration options for building image names for system components
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/server/api/v1/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ networkConfig:
clusterNetworkCIDR: ""
externalIPNetworkCIDRs: null
hostSubnetLength: 0
ingressIPNetworkCIDR: ""
networkPluginName: ""
serviceNetworkCIDR: ""
oauthConfig:
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/server/api/validation/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ func ValidateMasterConfig(config *api.MasterConfig, fldPath *field.Path) Validat
}
}
}
if len(config.NetworkConfig.IngressIPNetworkCIDR) > 0 {
cidr := config.NetworkConfig.IngressIPNetworkCIDR
if _, _, err := net.ParseCIDR(cidr); err != nil {
validationResults.AddErrors(field.Invalid(fldPath.Child("networkConfig", "ingressIPNetworkCIDR").Index(0), cidr, "must be a valid CIDR notation IP range (e.g. 172.30.0.0/16)"))
}
}

validationResults.AddErrors(ValidateKubeConfig(config.MasterClients.OpenShiftLoopbackKubeConfig, fldPath.Child("masterClients", "openShiftLoopbackKubeConfig"))...)

Expand Down
40 changes: 40 additions & 0 deletions pkg/cmd/server/bootstrappolicy/infra_sa_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ const (

InfraEndpointControllerServiceAccountName = "endpoint-controller"
EndpointControllerRoleName = "system:endpoint-controller"

InfraServiceIngressIPControllerServiceAccountName = "service-ingress-ip-controller"
ServiceIngressIPControllerRoleName = "system:service-ingress-ip-controller"
)

type InfraServiceAccounts struct {
Expand Down Expand Up @@ -766,4 +769,41 @@ func init() {
panic(err)
}

err = InfraSAs.addServiceAccount(
InfraServiceIngressIPControllerServiceAccountName,
authorizationapi.ClusterRole{
ObjectMeta: kapi.ObjectMeta{
Name: ServiceIngressIPControllerRoleName,
},
Rules: []authorizationapi.PolicyRule{
// Listing and watching services
{
APIGroups: []string{kapi.GroupName},
Verbs: sets.NewString("list", "watch"),
Resources: sets.NewString("services"),
},
// IngressIPController.persistSpec changes the spec of the service
{
APIGroups: []string{kapi.GroupName},
Verbs: sets.NewString("update"),
Resources: sets.NewString("services"),
},
// IngressIPController.persistStatus changes the status of the service
{
APIGroups: []string{kapi.GroupName},
Verbs: sets.NewString("update"),
Resources: sets.NewString("services/status"),
},
// IngressIPController.recorder
{
Verbs: sets.NewString("create", "update", "patch"),
Resources: sets.NewString("events"),
},
},
},
)
if err != nil {
panic(err)
}

}
4 changes: 3 additions & 1 deletion pkg/cmd/server/origin/master_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,9 @@ func newAdmissionChain(pluginNames []string, admissionConfigFilename string, plu
// should have been caught with validation
return nil, err
}
plugins = append(plugins, serviceadmit.NewExternalIPRanger(reject, admit))
// TODO need to disallow if a cloud provider is configured
allowIngressIP := len(options.NetworkConfig.IngressIPNetworkCIDR) > 0
plugins = append(plugins, serviceadmit.NewExternalIPRanger(reject, admit, allowIngressIP))

case serviceadmit.RestrictedEndpointsPluginName:
// we need to set some customer parameters, so create by hand
Expand Down
19 changes: 19 additions & 0 deletions pkg/cmd/server/origin/run_components.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/openshift/origin/pkg/security/mcs"
"github.com/openshift/origin/pkg/security/uid"
"github.com/openshift/origin/pkg/security/uidallocator"
"github.com/openshift/origin/pkg/service/controller/ingressip"
servingcertcontroller "github.com/openshift/origin/pkg/service/controller/servingcert"

configapi "github.com/openshift/origin/pkg/cmd/server/api"
Expand All @@ -61,6 +62,8 @@ const (

// from CMServer MinResyncPeriod
defaultReplenishmentSyncPeriod time.Duration = 12 * time.Hour

defaultIngressIPSyncPeriod time.Duration = 10 * time.Minute
)

// RunProjectAuthorizationCache starts the project authorization cache
Expand Down Expand Up @@ -516,3 +519,19 @@ func (c *MasterConfig) RunClusterQuotaReconciliationController() {
c.ClusterQuotaMappingController.GetClusterQuotaMapper().AddListener(controller)
go controller.Run(5, utilwait.NeverStop)
}

// RunIngressIPController starts the ingress ip controller if IngressIPNetworkCIDR is configured.
func (c *MasterConfig) RunIngressIPController(client *kclient.Client) {
// TODO need to disallow if a cloud provider is configured
if len(c.Options.NetworkConfig.IngressIPNetworkCIDR) == 0 {
return
}

_, ipNet, err := net.ParseCIDR(c.Options.NetworkConfig.IngressIPNetworkCIDR)
if err != nil {
// should have been caught with validation
glog.Fatalf("Unable to start ingress ip controller: %v", err)
}
ingressIPController := ingressip.NewIngressIPController(client, ipNet, defaultIngressIPSyncPeriod)
go ingressIPController.Run(utilwait.NeverStop)
}
6 changes: 6 additions & 0 deletions pkg/cmd/server/start/start_master.go
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,12 @@ func startControllers(oc *origin.MasterConfig, kc *kubernetes.MasterConfig) erro
}
oc.RunServiceServingCertController(serviceServingCertClient)

_, _, ingressIPClient, err := oc.GetServiceAccountClients(bootstrappolicy.InfraServiceIngressIPControllerServiceAccountName)
if err != nil {
glog.Fatalf("Could not get client: %v", err)
}
oc.RunIngressIPController(ingressIPClient)

glog.Infof("Started Origin Controllers")

return nil
Expand Down
40 changes: 31 additions & 9 deletions pkg/service/admission/externalip_admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ const ExternalIPPluginName = "ExternalIPRanger"

func init() {
kadmission.RegisterPlugin("ExternalIPRanger", func(client clientset.Interface, config io.Reader) (kadmission.Interface, error) {
return NewExternalIPRanger(nil, nil), nil
return NewExternalIPRanger(nil, nil, false), nil
})
}

type externalIPRanger struct {
*kadmission.Handler
reject []*net.IPNet
admit []*net.IPNet
reject []*net.IPNet
admit []*net.IPNet
allowIngressIP bool
}

var _ kadmission.Interface = &externalIPRanger{}
Expand Down Expand Up @@ -51,11 +52,12 @@ func ParseRejectAdmitCIDRRules(rules []string) (reject, admit []*net.IPNet, err
}

// NewConstraint creates a new SCC constraint admission plugin.
func NewExternalIPRanger(reject, admit []*net.IPNet) *externalIPRanger {
func NewExternalIPRanger(reject, admit []*net.IPNet, allowIngressIP bool) *externalIPRanger {
return &externalIPRanger{
Handler: kadmission.NewHandler(kadmission.Create, kadmission.Update),
reject: reject,
admit: admit,
Handler: kadmission.NewHandler(kadmission.Create, kadmission.Update),
reject: reject,
admit: admit,
allowIngressIP: allowIngressIP,
}
}

Expand Down Expand Up @@ -84,11 +86,30 @@ func (r *externalIPRanger) Admit(a kadmission.Attributes) error {
return nil
}

// Determine if an ingress ip address should be allowed as an
// external ip by checking the loadbalancer status of the previous
// object state. Only updates need to be validated against the
// ingress ip since the loadbalancer status cannot be set on
// create.
ingressIP := ""
retrieveIngressIP := a.GetOperation() == kadmission.Update &&
r.allowIngressIP && svc.Spec.Type == kapi.ServiceTypeLoadBalancer
if retrieveIngressIP {
old, ok := a.GetOldObject().(*kapi.Service)
ipPresent := ok && old != nil && len(old.Status.LoadBalancer.Ingress) > 0
if ipPresent {
ingressIP = old.Status.LoadBalancer.Ingress[0].IP
}
}

var errs field.ErrorList
switch {
// administrator disabled externalIPs
case len(svc.Spec.ExternalIPs) > 0 && len(r.admit) == 0:
errs = append(errs, field.Forbidden(field.NewPath("spec", "externalIPs"), "externalIPs have been disabled"))
onlyIngressIP := len(svc.Spec.ExternalIPs) == 1 && svc.Spec.ExternalIPs[0] == ingressIP
if !onlyIngressIP {
errs = append(errs, field.Forbidden(field.NewPath("spec", "externalIPs"), "externalIPs have been disabled"))
}
// administrator has limited the range
case len(svc.Spec.ExternalIPs) > 0 && len(r.admit) > 0:
for i, s := range svc.Spec.ExternalIPs {
Expand All @@ -97,7 +118,8 @@ func (r *externalIPRanger) Admit(a kadmission.Attributes) error {
errs = append(errs, field.Forbidden(field.NewPath("spec", "externalIPs").Index(i), "externalIPs must be a valid address"))
continue
}
if NetworkSlice(r.reject).Contains(ip) || !NetworkSlice(r.admit).Contains(ip) {
notIngressIP := s != ingressIP
if (NetworkSlice(r.reject).Contains(ip) || !NetworkSlice(r.admit).Contains(ip)) && notIngressIP {
errs = append(errs, field.Forbidden(field.NewPath("spec", "externalIPs").Index(i), "externalIP is not allowed"))
continue
}
Expand Down
63 changes: 59 additions & 4 deletions pkg/service/admission/externalip_admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func TestAdmission(t *testing.T) {
svc := &kapi.Service{
ObjectMeta: kapi.ObjectMeta{Name: "test"},
}
var oldSvc *kapi.Service

_, ipv4, err := net.ParseCIDR("172.0.0.0/16")
if err != nil {
Expand Down Expand Up @@ -43,6 +44,8 @@ func TestAdmission(t *testing.T) {
externalIPs []string
admit bool
errFn func(err error) bool
loadBalancer bool
ingressIP string
}{
{
admit: true,
Expand Down Expand Up @@ -128,7 +131,6 @@ func TestAdmission(t *testing.T) {
op: admission.Update,
testName: "IP in range on update",
},

// other checks
{
admit: false,
Expand Down Expand Up @@ -156,12 +158,65 @@ func TestAdmission(t *testing.T) {
op: admission.Create,
testName: "rejections can cover the entire range",
},
// Ingress IP checks
{
admit: true,
externalIPs: []string{"1.2.3.4"},
op: admission.Update,
testName: "Ingress ip allowed when external ips are disabled",
loadBalancer: true,
ingressIP: "1.2.3.4",
},
{
admit: true,
admits: []*net.IPNet{ipv4},
externalIPs: []string{"1.2.3.4", "172.0.0.1"},
op: admission.Update,
testName: "Ingress ip allowed when external ips are enabled",
loadBalancer: true,
ingressIP: "1.2.3.4",
},
{
admit: false,
admits: []*net.IPNet{ipv4},
externalIPs: []string{"1.2.3.4", "172.0.0.1"},
op: admission.Update,
testName: "Ingress ip not allowed for non-lb service",
loadBalancer: false,
ingressIP: "1.2.3.4",
},
}
for _, test := range tests {
svc.Spec.ExternalIPs = test.externalIPs
handler := NewExternalIPRanger(test.rejects, test.admits)
allowIngressIP := len(test.ingressIP) > 0 || test.loadBalancer
handler := NewExternalIPRanger(test.rejects, test.admits, allowIngressIP)

if test.loadBalancer {
svc.Spec.Type = kapi.ServiceTypeLoadBalancer
} else {
svc.Spec.Type = kapi.ServiceTypeClusterIP
}

if len(test.ingressIP) > 0 {
// Provide an ingress ip via the previous object state
oldSvc = &kapi.Service{
ObjectMeta: kapi.ObjectMeta{Name: "test"},
Status: kapi.ServiceStatus{
LoadBalancer: kapi.LoadBalancerStatus{
Ingress: []kapi.LoadBalancerIngress{
{
IP: test.ingressIP,
},
},
},
},
}

} else {
oldSvc = nil
}

err := handler.Admit(admission.NewAttributesRecord(svc, nil, kapi.Kind("Service").WithVersion("version"), "namespace", svc.ObjectMeta.Name, kapi.Resource("services").WithVersion("version"), "", test.op, nil))
err := handler.Admit(admission.NewAttributesRecord(svc, oldSvc, kapi.Kind("Service").WithVersion("version"), "namespace", svc.ObjectMeta.Name, kapi.Resource("services").WithVersion("version"), "", test.op, nil))
if test.admit && err != nil {
t.Errorf("%s: expected no error but got: %s", test.testName, err)
} else if !test.admit && err == nil {
Expand All @@ -180,7 +235,7 @@ func TestHandles(t *testing.T) {
admission.Connect: false,
admission.Delete: false,
} {
ranger := NewExternalIPRanger(nil, nil)
ranger := NewExternalIPRanger(nil, nil, false)
if e, a := shouldHandle, ranger.Handles(op); e != a {
t.Errorf("%v: shouldHandle=%t, handles=%t", op, e, a)
}
Expand Down
Loading

0 comments on commit 13d20e0

Please sign in to comment.