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.

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

GCP forwarding rule/target proxy description field populated #757

Merged
merged 1 commit into from
Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions pkg/loadbalancers/forwarding_rules.go
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
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 @@ -308,3 +309,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
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"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason why the namespace was changed from "default" to "namespace1"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it solely so that the tests were obvious in using the specified namespace. I was wary of the possibility of "default" being assumed somewhere (since, well, it's the default when no namespace is specified) and wanted the tests to be able to differentiate from that possibility.

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
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