From 3ee943d434ec4c421caf4ceae50e77cdb74efbc2 Mon Sep 17 00:00:00 2001 From: Prashanth Balasubramanian Date: Wed, 9 Mar 2016 19:29:23 -0800 Subject: [PATCH] Allow a user specified global static ip via annotation. --- controllers/gce/controller/controller.go | 41 ++------ controllers/gce/controller/controller_test.go | 49 ++++++++++ controllers/gce/controller/tls.go | 98 +++++++++++++++++++ controllers/gce/controller/utils.go | 13 ++- controllers/gce/loadbalancers/fakes.go | 12 +++ .../gce/loadbalancers/loadbalancers.go | 63 ++++++++++-- 6 files changed, 235 insertions(+), 41 deletions(-) create mode 100644 controllers/gce/controller/tls.go diff --git a/controllers/gce/controller/controller.go b/controllers/gce/controller/controller.go index ddb97c9f4..34319f9ab 100644 --- a/controllers/gce/controller/controller.go +++ b/controllers/gce/controller/controller.go @@ -67,6 +67,8 @@ type LoadBalancerController struct { // allowing concurrent stoppers leads to stack traces. stopLock sync.Mutex shutdown bool + // tlsLoader loads secrets from the Kubernetes apiserver for Ingresses. + tlsLoader tlsLoader } // NewLoadBalancerController creates a controller for gce loadbalancers. @@ -155,6 +157,7 @@ func NewLoadBalancerController(kubeClient *client.Client, clusterManager *Cluste &api.Node{}, 0, nodeHandlers) lbc.tr = &GCETranslator{&lbc} + lbc.tlsLoader = &apiServerTLSLoader{client: lbc.client} glog.V(3).Infof("Created new loadbalancer controller") return &lbc, nil @@ -353,47 +356,21 @@ func (lbc *LoadBalancerController) ListRuntimeInfo() (lbs []*loadbalancers.L7Run glog.Warningf("Cannot get key for Ingress %v/%v: %v", ing.Namespace, ing.Name, err) continue } - tls, err := lbc.loadSecrets(ing) + tls, err := lbc.tlsLoader.load(ing) if err != nil { glog.Warningf("Cannot get certs for Ingress %v/%v: %v", ing.Namespace, ing.Name, err) } + annotations := ingAnnotations(ing.ObjectMeta.Annotations) lbs = append(lbs, &loadbalancers.L7RuntimeInfo{ - Name: k, - TLS: tls, - AllowHTTP: ingAnnotations(ing.ObjectMeta.Annotations).allowHTTP(), + Name: k, + TLS: tls, + AllowHTTP: annotations.allowHTTP(), + StaticIPName: annotations.staticIPName(), }) } return lbs, nil } -func (lbc *LoadBalancerController) loadSecrets(ing *extensions.Ingress) (*loadbalancers.TLSCerts, error) { - if len(ing.Spec.TLS) == 0 { - return nil, nil - } - // GCE L7s currently only support a single cert. - if len(ing.Spec.TLS) > 1 { - glog.Warningf("Ignoring %d certs and taking the first for ingress %v/%v", - len(ing.Spec.TLS)-1, ing.Namespace, ing.Name) - } - secretName := ing.Spec.TLS[0].SecretName - // TODO: Replace this for a secret watcher. - glog.V(3).Infof("Retrieving secret for ing %v with name %v", ing.Name, secretName) - secret, err := lbc.client.Secrets(ing.Namespace).Get(secretName) - if err != nil { - return nil, err - } - cert, ok := secret.Data[api.TLSCertKey] - if !ok { - return nil, fmt.Errorf("Secret %v has no private key", secretName) - } - key, ok := secret.Data[api.TLSPrivateKeyKey] - if !ok { - return nil, fmt.Errorf("Secret %v has no cert", secretName) - } - // TODO: Validate certificate with hostnames in ingress? - return &loadbalancers.TLSCerts{Key: string(key), Cert: string(cert)}, nil -} - // syncNodes manages the syncing of kubernetes nodes to gce instance groups. // The instancegroups are referenced by loadbalancer backends. func (lbc *LoadBalancerController) syncNodes(key string) { diff --git a/controllers/gce/controller/controller_test.go b/controllers/gce/controller/controller_test.go index 9240e9694..112658950 100644 --- a/controllers/gce/controller/controller_test.go +++ b/controllers/gce/controller/controller_test.go @@ -363,6 +363,55 @@ func TestLbNoService(t *testing.T) { cm.fakeLbs.CheckURLMap(t, l7, expectedMap) } +func TestLbChangeStaticIP(t *testing.T) { + cm := NewFakeClusterManager(DefaultClusterUID) + lbc := newLoadBalancerController(t, cm, "") + inputMap := map[string]utils.FakeIngressRuleValueMap{ + "foo.example.com": { + "/foo1": "foo1svc", + }, + } + ing := newIngress(inputMap) + ing.Spec.Backend.ServiceName = "foo1svc" + cert := extensions.IngressTLS{SecretName: "foo"} + ing.Spec.TLS = []extensions.IngressTLS{cert} + + // Add some certs so we get 2 forwarding rules, the changed static IP + // should be assigned to both the HTTP and HTTPS forwarding rules. + lbc.tlsLoader = &fakeTLSSecretLoader{ + fakeCerts: map[string]*loadbalancers.TLSCerts{ + cert.SecretName: {Key: "foo", Cert: "bar"}, + }, + } + + pm := newPortManager(1, 65536) + addIngress(lbc, ing, pm) + ingStoreKey := getKey(ing, t) + + // First sync creates forwarding rules and allocates an IP. + lbc.sync(ingStoreKey) + + // First allocate a static ip, then specify a userip in annotations. + // The forwarding rules should contain the user ip. + // The static ip should get cleaned up on lb tear down. + oldIP := ing.Status.LoadBalancer.Ingress[0].IP + oldRules := cm.fakeLbs.GetForwardingRulesWithIPs([]string{oldIP}) + if len(oldRules) != 2 || oldRules[0].IPAddress != oldRules[1].IPAddress { + t.Fatalf("Expected 2 forwarding rules with the same IP.") + } + + ing.Annotations = map[string]string{staticIPNameKey: "testip"} + cm.fakeLbs.ReserveGlobalStaticIP("testip", "1.2.3.4") + + // Second sync reassigns 1.2.3.4 to existing forwarding rule (by recreating it) + lbc.sync(ingStoreKey) + + newRules := cm.fakeLbs.GetForwardingRulesWithIPs([]string{"1.2.3.4"}) + if len(newRules) != 2 || newRules[0].IPAddress != newRules[1].IPAddress || newRules[1].IPAddress != "1.2.3.4" { + t.Fatalf("Found unexpected forwaring rules after changing static IP annotation.") + } +} + type testIP struct { start int } diff --git a/controllers/gce/controller/tls.go b/controllers/gce/controller/tls.go new file mode 100644 index 000000000..7caa29459 --- /dev/null +++ b/controllers/gce/controller/tls.go @@ -0,0 +1,98 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "fmt" + + "k8s.io/contrib/ingress/controllers/gce/loadbalancers" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/extensions" + client "k8s.io/kubernetes/pkg/client/unversioned" + + "github.com/golang/glog" +) + +// secretLoaders returns a type containing all the secrets of an Ingress. +type tlsLoader interface { + load(ing *extensions.Ingress) (*loadbalancers.TLSCerts, error) + validate(certs *loadbalancers.TLSCerts) error +} + +// TODO: Add better cert validation. +type noOPValidator struct{} + +func (n *noOPValidator) validate(certs *loadbalancers.TLSCerts) error { + return nil +} + +// apiServerTLSLoader loads TLS certs from the apiserver. +type apiServerTLSLoader struct { + noOPValidator + client *client.Client +} + +func (t *apiServerTLSLoader) load(ing *extensions.Ingress) (*loadbalancers.TLSCerts, error) { + if len(ing.Spec.TLS) == 0 { + return nil, nil + } + // GCE L7s currently only support a single cert. + if len(ing.Spec.TLS) > 1 { + glog.Warningf("Ignoring %d certs and taking the first for ingress %v/%v", + len(ing.Spec.TLS)-1, ing.Namespace, ing.Name) + } + secretName := ing.Spec.TLS[0].SecretName + // TODO: Replace this for a secret watcher. + glog.V(3).Infof("Retrieving secret for ing %v with name %v", ing.Name, secretName) + secret, err := t.client.Secrets(ing.Namespace).Get(secretName) + if err != nil { + return nil, err + } + cert, ok := secret.Data[api.TLSCertKey] + if !ok { + return nil, fmt.Errorf("Secret %v has no private key", secretName) + } + key, ok := secret.Data[api.TLSPrivateKeyKey] + if !ok { + return nil, fmt.Errorf("Secret %v has no cert", secretName) + } + certs := &loadbalancers.TLSCerts{Key: string(key), Cert: string(cert)} + if err := t.validate(certs); err != nil { + return nil, err + } + return certs, nil +} + +// TODO: Add support for file loading so we can support HTTPS default backends. + +// fakeTLSSecretLoader fakes out TLS loading. +type fakeTLSSecretLoader struct { + noOPValidator + fakeCerts map[string]*loadbalancers.TLSCerts +} + +func (f *fakeTLSSecretLoader) load(ing *extensions.Ingress) (*loadbalancers.TLSCerts, error) { + if len(ing.Spec.TLS) == 0 { + return nil, nil + } + for name, cert := range f.fakeCerts { + if ing.Spec.TLS[0].SecretName == name { + return cert, nil + } + } + return nil, fmt.Errorf("Couldn't find secret for ingress %v", ing.Name) +} diff --git a/controllers/gce/controller/utils.go b/controllers/gce/controller/utils.go index e0d2805c0..ec8d019ea 100644 --- a/controllers/gce/controller/utils.go +++ b/controllers/gce/controller/utils.go @@ -34,7 +34,10 @@ import ( "github.com/golang/glog" ) -const allowHTTPKey = "kubernetes.io/ingress.allowHTTP" +const ( + allowHTTPKey = "kubernetes.io/ingress.allow-http" + staticIPNameKey = "kubernetes.io/ingress.global-static-ip-name" +) // ingAnnotations represents Ingress annotations. type ingAnnotations map[string]string @@ -52,6 +55,14 @@ func (ing ingAnnotations) allowHTTP() bool { return v } +func (ing ingAnnotations) staticIPName() string { + val, ok := ing[staticIPNameKey] + if !ok { + return "" + } + return val +} + // errorNodePortNotFound is an implementation of error. type errorNodePortNotFound struct { backend extensions.IngressBackend diff --git a/controllers/gce/loadbalancers/fakes.go b/controllers/gce/loadbalancers/fakes.go index f1b145c96..b5d147e9f 100644 --- a/controllers/gce/loadbalancers/fakes.go +++ b/controllers/gce/loadbalancers/fakes.go @@ -22,6 +22,7 @@ import ( compute "google.golang.org/api/compute/v1" "k8s.io/contrib/ingress/controllers/gce/utils" + "k8s.io/kubernetes/pkg/util/sets" ) var testIPManager = testIP{} @@ -148,6 +149,17 @@ func (f *FakeLoadBalancers) DeleteGlobalForwardingRule(name string) error { return nil } +// GetForwardingRulesWithIPs returns all forwarding rules that match the given ips. +func (f *FakeLoadBalancers) GetForwardingRulesWithIPs(ip []string) (fwRules []*compute.ForwardingRule) { + ipSet := sets.NewString(ip...) + for i := range f.Fw { + if ipSet.Has(f.Fw[i].IPAddress) { + fwRules = append(fwRules, f.Fw[i]) + } + } + return fwRules +} + // UrlMaps fakes // GetUrlMap fakes getting url maps from the cloud. diff --git a/controllers/gce/loadbalancers/loadbalancers.go b/controllers/gce/loadbalancers/loadbalancers.go index cf2740358..3862a56c1 100644 --- a/controllers/gce/loadbalancers/loadbalancers.go +++ b/controllers/gce/loadbalancers/loadbalancers.go @@ -22,6 +22,7 @@ import ( "encoding/json" "fmt" "net/http" + "reflect" "strings" compute "google.golang.org/api/compute/v1" @@ -130,6 +131,11 @@ func (l *L7s) Add(ri *L7RuntimeInfo) (err error) { if err != nil { return err } + } else { + if !reflect.DeepEqual(lb.runtimeInfo, ri) { + glog.Infof("LB %v runtime info changed, old %+v new %+v", lb.Name, lb.runtimeInfo, ri) + lb.runtimeInfo = ri + } } // Add the lb to the pool, in case we create an UrlMap but run out // of quota in creating the ForwardingRule we still need to cleanup @@ -242,6 +248,9 @@ type L7RuntimeInfo struct { // AllowHTTP will not setup :80, if TLS is nil and AllowHTTP is set, // no loadbalancer is created. AllowHTTP bool + // The name of a Global Static IP. If specified, the IP associated with + // this name is used in the Forwarding Rules for this loadbalancer. + StaticIPName string } // L7 represents a single L7 loadbalancer. @@ -413,15 +422,50 @@ func (l *L7) checkForwardingRule(name, proxyLink, ip, portRange string) (fw *com return fw, nil } +// getEffectiveIP returns a string with the IP to use in the HTTP and HTTPS +// forwarding rules, and a boolean indicating if this is an IP the controller +// should manage or not. +func (l *L7) getEffectiveIP() (string, bool) { + + // A note on IP management: + // User specifies a different IP on startup: + // - We create a forwarding rule with the given IP. + // - If this ip doesn't exist in GCE, we create another one in the hope + // that they will rectify it later on. + // - In the happy case, no static ip is created or deleted by this controller. + // Controller allocates a staticIP/ephemeralIP, but user changes it: + // - We still delete the old static IP, but only when we tear down the + // Ingress in Cleanup(). Till then the static IP stays around, but + // the forwarding rules get deleted/created with the new IP. + // - There will be a period of downtime as we flip IPs. + // User specifies the same static IP to 2 Ingresses: + // - GCE will throw a 400, and the controller will keep trying to use + // the IP in the hope that the user manually resolves the conflict + // or deletes/modifies the Ingress. + // TODO: Handle the last case better. + + if l.runtimeInfo.StaticIPName != "" { + // Existing static IPs allocated to forwarding rules will get orphaned + // till the Ingress is torn down. + if ip, err := l.cloud.GetGlobalStaticIP(l.runtimeInfo.StaticIPName); err != nil || ip == nil { + glog.Warningf("The given static IP name %v doesn't translate to an existing global static IP, ignoring it and allocating a new IP: %v", + l.runtimeInfo.StaticIPName, err) + } else { + return ip.Address, false + } + } + if l.ip != nil { + return l.ip.Address, true + } + return "", true +} + func (l *L7) checkHttpForwardingRule() (err error) { if l.tp == nil { return fmt.Errorf("Cannot create forwarding rule without proxy.") } - var address string - if l.ip != nil { - address = l.ip.Address - } name := l.namer.Truncate(fmt.Sprintf("%v-%v", forwardingRulePrefix, l.Name)) + address, _ := l.getEffectiveIP() fw, err := l.checkForwardingRule(name, l.tp.SelfLink, address, httpDefaultPortRange) if err != nil { return err @@ -435,11 +479,8 @@ func (l *L7) checkHttpsForwardingRule() (err error) { glog.V(3).Infof("No https target proxy for %v, not created https forwarding rule", l.Name) return nil } - var address string - if l.ip != nil { - address = l.ip.Address - } name := l.namer.Truncate(fmt.Sprintf("%v-%v", httpsForwardingRulePrefix, l.Name)) + address, _ := l.getEffectiveIP() fws, err := l.checkForwardingRule(name, l.tps.SelfLink, address, httpsDefaultPortRange) if err != nil { return err @@ -448,10 +489,16 @@ func (l *L7) checkHttpsForwardingRule() (err error) { return nil } +// checkStaticIP reserves a static IP allocated to the Forwarding Rule. func (l *L7) checkStaticIP() (err error) { if l.fw == nil || l.fw.IPAddress == "" { return fmt.Errorf("Will not create static IP without a forwarding rule.") } + // Don't manage staticIPs if the user has specified an IP. + if address, manageStaticIP := l.getEffectiveIP(); !manageStaticIP { + glog.V(3).Infof("Not managing user specified static IP %v", address) + return nil + } staticIPName := l.namer.Truncate(fmt.Sprintf("%v-%v", forwardingRulePrefix, l.Name)) ip, _ := l.cloud.GetGlobalStaticIP(staticIPName) if ip == nil {