Skip to content

Commit

Permalink
Merge pull request #757 from KatrinaHoffert/issue-661
Browse files Browse the repository at this point in the history 
GCP forwarding rule/target proxy description field populated
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Jun 4, 2019
2 parents 688894c + 98cb9d9 commit 436e099
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 56 deletions.
15 changes: 10 additions & 5 deletions pkg/loadbalancers/forwarding_rules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,17 @@ func (l *L7) checkForwardingRule(name, proxyLink, ip, portRange string) (fw *com
}
if fw == nil {
klog.V(3).Infof("Creating forwarding rule for proxy %q and ip %v:%v", proxyLink, ip, portRange)
description, err := l.description()
if err != nil {
return nil, err
}
rule := &compute.ForwardingRule{
Name: name,
IPAddress: ip,
Target: proxyLink,
PortRange: portRange,
IPProtocol: "TCP",
Name: name,
IPAddress: ip,
Target: proxyLink,
PortRange: portRange,
IPProtocol: "TCP",
Description: description,
}
if err = l.cloud.CreateGlobalForwardingRule(rule); err != nil {
return nil, err
Expand Down
14 changes: 14 additions & 0 deletions pkg/loadbalancers/l7.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"strings"

"k8s.io/apimachinery/pkg/types"
"k8s.io/klog"

compute "google.golang.org/api/compute/v1"
Expand Down Expand Up @@ -323,3 +324,16 @@ func GCEResourceName(ingAnnotations map[string]string, resourceName string) stri
resourceName, _ = ingAnnotations[fmt.Sprintf("%v/%v", annotations.StatusPrefix, resourceName)]
return resourceName
}

// description gets a description for the ingress GCP resources.
func (l *L7) description() (string, error) {
if l.runtimeInfo.Ingress == nil {
return "", fmt.Errorf("missing Ingress object to construct description for %s", l.Name)
}

namespace := l.runtimeInfo.Ingress.ObjectMeta.Namespace
ingressName := l.runtimeInfo.Ingress.ObjectMeta.Name
namespacedName := types.NamespacedName{Name: ingressName, Namespace: namespace}

return fmt.Sprintf(`{"kubernetes.io/ingress-name": %q}`, namespacedName.String()), nil
}
110 changes: 63 additions & 47 deletions pkg/loadbalancers/loadbalancers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ import (
)

const (
defaultNamespace = "default"
defaultZone = "zone-a"
clusterName = "uid1"
ingressName = "test"
namespace = "namespace1"
defaultZone = "zone-a"
)

var (
Expand All @@ -47,7 +49,8 @@ var (
func newIngress() *extensions.Ingress {
return &extensions.Ingress{
ObjectMeta: metav1.ObjectMeta{
Namespace: defaultNamespace,
Name: ingressName,
Namespace: namespace,
},
}
}
Expand All @@ -66,9 +69,9 @@ func TestCreateHTTPLoadBalancer(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: namer.LoadBalancer(ingressName),
AllowHTTP: true,
UrlMap: gceUrlMap,
Ingress: newIngress(),
Expand All @@ -89,9 +92,9 @@ func TestCreateHTTPSLoadBalancer(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: namer.LoadBalancer(ingressName),
AllowHTTP: false,
TLS: []*TLSCerts{createCert("key", "cert", "name")},
UrlMap: gceUrlMap,
Expand Down Expand Up @@ -125,6 +128,9 @@ func verifyHTTPSForwardingRuleAndProxyLinks(t *testing.T, f *FakeLoadBalancers)
if !utils.EqualResourcePaths(fws.Target, tps.SelfLink) {
t.Fatalf("fws.Target = %q, want %q", fws.Target, tps.SelfLink)
}
if fws.Description == "" {
t.Errorf("fws.Description not set; expected it to be")
}
}

func verifyHTTPForwardingRuleAndProxyLinks(t *testing.T, f *FakeLoadBalancers) {
Expand All @@ -145,6 +151,9 @@ func verifyHTTPForwardingRuleAndProxyLinks(t *testing.T, f *FakeLoadBalancers) {
if !utils.EqualResourcePaths(fws.Target, tps.SelfLink) {
t.Fatalf("fw.Target = %q, want %q", fws.Target, tps.SelfLink)
}
if fws.Description == "" {
t.Errorf("fws.Description not set; expected it to be")
}
}

// Tests that a certificate is created from the provided Key/Cert combo
Expand All @@ -153,8 +162,8 @@ func TestCertUpdate(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))
certName2 := namer.SSLCertName(lbName, GetCertHash("cert2"))

Expand Down Expand Up @@ -193,8 +202,8 @@ func TestMultipleSecretsWithSameCert(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)

lbInfo := &L7RuntimeInfo{
Name: lbName,
Expand Down Expand Up @@ -223,8 +232,8 @@ func TestCertCreationWithCollision(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))
certName2 := namer.SSLCertName(lbName, GetCertHash("cert2"))

Expand Down Expand Up @@ -277,11 +286,11 @@ func TestMultipleCertRetentionAfterRestart(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
namer := utils.NewNamer(clusterName, "fw1")
cert1 := createCert("key", "cert", "name")
cert2 := createCert("key2", "cert2", "name2")
cert3 := createCert("key3", "cert3", "name3")
lbName := namer.LoadBalancer("test")
lbName := namer.LoadBalancer(ingressName)
certName1 := namer.SSLCertName(lbName, cert1.CertHash)
certName2 := namer.SSLCertName(lbName, cert2.CertHash)
certName3 := namer.SSLCertName(lbName, cert3.CertHash)
Expand Down Expand Up @@ -328,8 +337,8 @@ func TestUpgradeToNewCertNames(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
lbInfo := &L7RuntimeInfo{
Name: lbName,
AllowHTTP: false,
Expand All @@ -347,7 +356,11 @@ func TestUpgradeToNewCertNames(t *testing.T) {
sslCert := &compute.SslCertificate{Name: oldCertName, Certificate: "cert"}
f.CreateSslCertificate(sslCert)
tpName := f.TPName(true)
newProxy := &compute.TargetHttpsProxy{Name: tpName, SslCertificates: []string{sslCert.SelfLink}}
newProxy := &compute.TargetHttpsProxy{
Name: tpName,
Description: "fake",
SslCertificates: []string{sslCert.SelfLink},
}
err := f.CreateTargetHTTPSProxy(newProxy)
if err != nil {
t.Fatalf("Failed to create Target proxy %v - %v", newProxy, err)
Expand Down Expand Up @@ -381,8 +394,8 @@ func TestMaxCertsUpload(t *testing.T) {
var tlsCerts []*TLSCerts
expectCerts := make(map[string]string)
expectCertsExtra := make(map[string]string)
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)

for ix := 0; ix < FakeCertQuota; ix++ {
str := strconv.Itoa(ix)
Expand Down Expand Up @@ -445,8 +458,8 @@ func TestIdenticalHostnameCerts(t *testing.T) {
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
var tlsCerts []*TLSCerts
expectCerts := make(map[string]string)
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
contents := ""

for ix := 0; ix < 3; ix++ {
Expand Down Expand Up @@ -481,9 +494,9 @@ func TestIdenticalHostnameCertsPreShared(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: namer.LoadBalancer(ingressName),
AllowHTTP: false,
UrlMap: gceUrlMap,
Ingress: newIngress(),
Expand Down Expand Up @@ -528,8 +541,8 @@ func TestPreSharedToSecretBasedCertUpdate(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))
certName2 := namer.SSLCertName(lbName, GetCertHash("cert2"))

Expand Down Expand Up @@ -666,6 +679,10 @@ func verifyCertAndProxyLink(expectCerts map[string]string, expectCertsProxy map[
certName, expectCertsProxy, tps.SslCertificates)
}
}

if tps.Description == "" {
t.Errorf("tps.Description not set; expected it to be")
}
}

func TestCreateHTTPSLoadBalancerAnnotationCert(t *testing.T) {
Expand All @@ -675,9 +692,9 @@ func TestCreateHTTPSLoadBalancerAnnotationCert(t *testing.T) {
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
tlsName := "external-cert-name"
namer := utils.NewNamer("uid1", "fw1")
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: namer.LoadBalancer(ingressName),
AllowHTTP: false,
TLSName: tlsName,
UrlMap: gceUrlMap,
Expand Down Expand Up @@ -706,9 +723,9 @@ func TestCreateBothLoadBalancers(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: namer.LoadBalancer(ingressName),
AllowHTTP: true,
TLS: []*TLSCerts{{Key: "key", Cert: "cert"}},
UrlMap: gceUrlMap,
Expand Down Expand Up @@ -768,8 +785,8 @@ func TestUrlMapChange(t *testing.T) {
um2.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar1", Backend: utils.ServicePort{NodePort: 30003}}})
um2.DefaultBackend = &utils.ServicePort{NodePort: 30004}

namer := utils.NewNamer("uid1", "fw1")
lbInfo := &L7RuntimeInfo{Name: namer.LoadBalancer("test"), AllowHTTP: true, UrlMap: um1, Ingress: newIngress()}
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{Name: namer.LoadBalancer(ingressName), AllowHTTP: true, UrlMap: um1, Ingress: newIngress()}

f := NewFakeLoadBalancers(lbInfo.Name, namer)
pool := newFakeLoadBalancerPool(f, t, namer)
Expand Down Expand Up @@ -813,8 +830,8 @@ func TestPoolSyncNoChanges(t *testing.T) {
um2.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar1", Backend: utils.ServicePort{NodePort: 30002}}})
um2.DefaultBackend = &utils.ServicePort{NodePort: 30003}

namer := utils.NewNamer("uid1", "fw1")
lbInfo := &L7RuntimeInfo{Name: namer.LoadBalancer("test"), AllowHTTP: true, UrlMap: um1, Ingress: newIngress()}
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{Name: namer.LoadBalancer(ingressName), AllowHTTP: true, UrlMap: um1, Ingress: newIngress()}
f := NewFakeLoadBalancers(lbInfo.Name, namer)
pool := newFakeLoadBalancerPool(f, t, namer)
if _, err := pool.Ensure(lbInfo); err != nil {
Expand Down Expand Up @@ -856,9 +873,9 @@ func TestClusterNameChange(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer("test"),
Name: namer.LoadBalancer(ingressName),
AllowHTTP: true,
TLS: []*TLSCerts{{Key: "key", Cert: "cert"}},
UrlMap: gceUrlMap,
Expand Down Expand Up @@ -928,10 +945,9 @@ func TestList(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := "test"
namer := utils.NewNamer(clusterName, "fw1")
lbInfo := &L7RuntimeInfo{
Name: namer.LoadBalancer(lbName),
Name: namer.LoadBalancer(ingressName),
AllowHTTP: true,
TLS: []*TLSCerts{{Key: "key", Cert: "cert"}},
UrlMap: gceUrlMap,
Expand Down Expand Up @@ -973,8 +989,8 @@ func TestSecretBasedAndPreSharedCerts(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))
certName2 := namer.SSLCertName(lbName, GetCertHash("cert2"))

Expand Down Expand Up @@ -1032,8 +1048,8 @@ func TestMaxSecretBasedAndPreSharedCerts(t *testing.T) {
var tlsCerts []*TLSCerts
expectCerts := make(map[string]string)
expectCertsExtra := make(map[string]string)
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)

for ix := 0; ix < TargetProxyCertLimit; ix++ {
str := strconv.Itoa(ix)
Expand Down Expand Up @@ -1116,8 +1132,8 @@ func TestSecretBasedToPreSharedCertUpdate(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))

lbInfo := &L7RuntimeInfo{
Expand Down Expand Up @@ -1169,8 +1185,8 @@ func TestSecretBasedToPreSharedCertUpdateWithErrors(t *testing.T) {
gceUrlMap := utils.NewGCEURLMap()
gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234}
gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{utils.PathRule{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000}}})
namer := utils.NewNamer("uid1", "fw1")
lbName := namer.LoadBalancer("test")
namer := utils.NewNamer(clusterName, "fw1")
lbName := namer.LoadBalancer(ingressName)
certName1 := namer.SSLCertName(lbName, GetCertHash("cert"))

lbInfo := &L7RuntimeInfo{
Expand Down
18 changes: 14 additions & 4 deletions pkg/loadbalancers/target_proxies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,14 @@ func (l *L7) checkProxy() (err error) {
proxy, _ := l.cloud.GetTargetHTTPProxy(proxyName)
if proxy == nil {
klog.V(3).Infof("Creating new http proxy for urlmap %v", l.um.Name)
description, err := l.description()
if err != nil {
return err
}
newProxy := &compute.TargetHttpProxy{
Name: proxyName,
UrlMap: urlMapLink,
Name: proxyName,
Description: description,
UrlMap: urlMapLink,
}
if err = l.cloud.CreateTargetHTTPProxy(newProxy); err != nil {
return err
Expand Down Expand Up @@ -70,9 +75,14 @@ func (l *L7) checkHttpsProxy() (err error) {
proxy, _ := l.cloud.GetTargetHTTPSProxy(proxyName)
if proxy == nil {
klog.V(3).Infof("Creating new https proxy for urlmap %q", l.um.Name)
description, err := l.description()
if err != nil {
return err
}
newProxy := &compute.TargetHttpsProxy{
Name: proxyName,
UrlMap: urlMapLink,
Name: proxyName,
Description: description,
UrlMap: urlMapLink,
}

for _, c := range l.sslCerts {
Expand Down

0 comments on commit 436e099

Please sign in to comment.