DevFW-CICD/ingress-nginx-helm
16
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Address review comments

Browse source
This commit is contained in:
Nick Sardo 2017-04-26 09:20:21 -07:00
parent 748b5c5cd2
commit f704d81c16
2 changed files with 88 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

151
controllers/gce/loadbalancers/loadbalancers.go
View file

@ -344,10 +344,8 @@ func (l *L7) deleteOldSSLCert() (err error) {
return nil
}
glog.Infof("Cleaning up old SSL Certificate %v, current name %v", l.oldSSLCert.Name, l.sslCert.Name)
if err := l.cloud.DeleteSslCertificate(l.oldSSLCert.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteSslCertificate(l.oldSSLCert.Name)); err != nil {
return err
}
l.oldSSLCert = nil
return nil
@ -362,38 +360,71 @@ func getResourceNameFromLink(link string) string {
return s[len(s)-1]
}
func (l *L7) checkSSLCert() (err error) {
// Handle Pre-Shared cert
preSharedCertName := l.runtimeInfo.TLSName
func (l *L7) usePreSharedCert() (bool, error) {
// Use the named GCE cert when it is specified by the annotation.
if preSharedCertName != "" {
// Ask GCE for the cert, checking for problems and existence.
cert, err := l.cloud.GetSslCertificate(preSharedCertName)
if err != nil {
return err
}
if cert == nil {
return fmt.Errorf("cannot find existing sslCertificate %v for %v", preSharedCertName, l.Name)
}
glog.V(2).Infof("Using existing sslCertificate %v for %v", preSharedCertName, l.Name)
l.sslCert = cert
return nil
preSharedCertName := l.runtimeInfo.TLSName
if preSharedCertName == "" {
return false, nil
}
// Handle secret-created cert
// Ask GCE for the cert, checking for problems and existence.
cert, err := l.cloud.GetSslCertificate(preSharedCertName)
if err != nil {
return true, err
}
if cert == nil {
return true, fmt.Errorf("cannot find existing sslCertificate %v for %v", preSharedCertName, l.Name)
}
glog.V(2).Infof("Using existing sslCertificate %v for %v", preSharedCertName, l.Name)
l.sslCert = cert
return true, nil
}
func (l *L7) populateSSLCert() error {
// Determine what certificate name is being used
var expectedCertName string
if l.sslCert != nil {
expectedCertName = l.sslCert.Name
} else {
// Retrieve the ssl certificate in use by the expected target proxy (if exists)
expectedCertName = getResourceNameFromLink(l.getSslCertInUse())
expectedCertName = getResourceNameFromLink(l.getSslCertLinkInUse())
}
// Retrieve known ssl certificate
var err error
if expectedCertName != "" {
l.sslCert, _ = l.cloud.GetSslCertificate(expectedCertName)
// Retrieve the certificate and ignore error if certificate wasn't found
l.sslCert, err = l.cloud.GetSslCertificate(expectedCertName)
if err != nil {
return utils.IgnoreHTTPNotFound(err)
}
}
return nil
}
func (l *L7) nextCertificateName() string {
// The name of the cert for this lb flip-flops between these 2 on
// every certificate update. We don't append the index at the end so we're
// sure it isn't truncated.
// TODO: Clean this code up into a ring buffer.
primaryCertName := l.namer.Truncate(fmt.Sprintf("%v-%v", sslCertPrefix, l.Name))
secondaryCertName := l.namer.Truncate(fmt.Sprintf("%v-%d-%v", sslCertPrefix, 1, l.Name))
if l.sslCert != nil && l.sslCert.Name == primaryCertName {
return secondaryCertName
}
return primaryCertName
}
func (l *L7) checkSSLCert() error {
// Handle Pre-Shared cert and early return if used
if used, err := l.usePreSharedCert(); used {
return err
}
// Get updated value of certificate for comparison
if err := l.populateSSLCert(); err != nil {
return err
}
// TODO: Currently, GCE only supports a single certificate per static IP
@ -402,32 +433,20 @@ func (l *L7) checkSSLCert() (err error) {
ingCert := l.runtimeInfo.TLS.Cert
ingKey := l.runtimeInfo.TLS.Key
// The name of the cert for this lb flip-flops between these 2 on
// every certificate update. We don't append the index at the end so we're
// sure it isn't truncated.
// TODO: Clean this code up into a ring buffer.
primaryCertName := l.namer.Truncate(fmt.Sprintf("%v-%v", sslCertPrefix, l.Name))
secondaryCertName := l.namer.Truncate(fmt.Sprintf("%v-%d-%v", sslCertPrefix, 1, l.Name))
// PrivateKey is write only, so compare certs alone. We're assuming that
// no one will change just the key. We can remember the key and compare,
// but a bug could end up leaking it, which feels worse.
if l.sslCert == nil || ingCert != l.sslCert.Certificate {
newCertName := primaryCertName
if l.sslCert != nil && (ingCert != l.sslCert.Certificate) {
if l.sslCert.Name == primaryCertName {
newCertName = secondaryCertName
}
}
newCertName := l.nextCertificateName()
// Perform a delete in case a certificate exists with the exact name
// This certificate should be unused since we check the target proxy's certificate prior
// to this point. Although, it's possible an actor pointed a target proxy to this certificate.
if err = l.cloud.DeleteSslCertificate(newCertName); err != nil && !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteSslCertificate(newCertName)); err != nil {
return fmt.Errorf("unable to delete ssl certificate with name %q, expected it to be unused. err: %v", newCertName, err)
}
glog.Infof("Creating new sslCertificates %v for %v", newCertName, l.Name)
glog.V(2).Infof("Creating new sslCertificate %v for %v", newCertName, l.Name)
cert, err := l.cloud.CreateSslCertificate(&compute.SslCertificate{
Name: newCertName,
Certificate: ingCert,
@ -444,7 +463,7 @@ func (l *L7) checkSSLCert() (err error) {
return nil
}
func (l *L7) getSslCertInUse() string {
func (l *L7) getSslCertLinkInUse() string {
proxyName := l.namer.Truncate(fmt.Sprintf("%v-%v", targetHTTPSProxyPrefix, l.Name))
proxy, _ := l.cloud.GetTargetHttpsProxy(proxyName)
if proxy != nil && len(proxy.SslCertificates) > 0 {
@ -497,10 +516,8 @@ func (l *L7) checkForwardingRule(name, proxyLink, ip, portRange string) (fw *com
if fw != nil && (ip != "" && fw.IPAddress != ip || fw.PortRange != portRange) {
glog.Warningf("Recreating forwarding rule %v(%v), so it has %v(%v)",
fw.IPAddress, fw.PortRange, ip, portRange)
if err := l.cloud.DeleteGlobalForwardingRule(name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return nil, err
}
if err = utils.IgnoreHTTPNotFound(l.cloud.DeleteGlobalForwardingRule(name)); err != nil {
return nil, err
}
fw = nil
}
@ -863,65 +880,51 @@ func mapsEqual(a, b *compute.UrlMap) bool {
func (l *L7) Cleanup() error {
if l.fw != nil {
glog.Infof("Deleting global forwarding rule %v", l.fw.Name)
if err := l.cloud.DeleteGlobalForwardingRule(l.fw.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteGlobalForwardingRule(l.fw.Name)); err != nil {
return err
}
l.fw = nil
}
if l.fws != nil {
glog.Infof("Deleting global forwarding rule %v", l.fws.Name)
if err := l.cloud.DeleteGlobalForwardingRule(l.fws.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
l.fws = nil
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteGlobalForwardingRule(l.fws.Name)); err != nil {
return err
}
l.fws = nil
}
if l.ip != nil {
glog.Infof("Deleting static IP %v(%v)", l.ip.Name, l.ip.Address)
if err := l.cloud.DeleteGlobalStaticIP(l.ip.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
l.ip = nil
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteGlobalStaticIP(l.ip.Name)); err != nil {
return err
}
l.ip = nil
}
if l.tps != nil {
glog.Infof("Deleting target https proxy %v", l.tps.Name)
if err := l.cloud.DeleteTargetHttpsProxy(l.tps.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteTargetHttpsProxy(l.tps.Name)); err != nil {
return err
}
l.tps = nil
}
// Delete the SSL cert if it is from a secret, not referencing a pre-created GCE cert.
if l.sslCert != nil && l.runtimeInfo.TLSName == "" {
glog.Infof("Deleting sslcert %v", l.sslCert.Name)
if err := l.cloud.DeleteSslCertificate(l.sslCert.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteSslCertificate(l.sslCert.Name)); err != nil {
return err
}
l.sslCert = nil
}
if l.tp != nil {
glog.Infof("Deleting target http proxy %v", l.tp.Name)
if err := l.cloud.DeleteTargetHttpProxy(l.tp.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteTargetHttpProxy(l.tp.Name)); err != nil {
return err
}
l.tp = nil
}
if l.um != nil {
glog.Infof("Deleting url map %v", l.um.Name)
if err := l.cloud.DeleteUrlMap(l.um.Name); err != nil {
if !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
return err
}
if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteUrlMap(l.um.Name)); err != nil {
return err
}
l.um = nil
}

12
controllers/gce/utils/utils.go
View file

@ -18,6 +18,7 @@ package utils
import (
"fmt"
"net/http"
"regexp"
"strconv"
"strings"
@ -310,7 +311,7 @@ func (g GCEURLMap) PutDefaultBackend(d *compute.BackendService) {
}
}
// FakeNotFoundErr creates a NotFound error with type googleapi.Error
// FakeGoogleAPINotFoundErr creates a NotFound error with type googleapi.Error
func FakeGoogleAPINotFoundErr() *googleapi.Error {
return &googleapi.Error{Code: 404}
}
@ -322,6 +323,15 @@ func IsHTTPErrorCode(err error, code int) bool {
return ok && apiErr.Code == code
}
// IgnoreHTTPNotFound returns the passed err if it's not a GoogleAPI error
// with a NotFound status code.
func IgnoreHTTPNotFound(err error) error {
if err != nil && IsHTTPErrorCode(err, http.StatusNotFound) {
return nil
}
return err
}
// CompareLinks returns true if the 2 self links are equal.
func CompareLinks(l1, l2 string) bool {
// TODO: These can be partial links