From 17123663792b67c3568aac7a29466bfcaaaab24c Mon Sep 17 00:00:00 2001 From: David Cheung Date: Mon, 12 Sep 2022 21:07:32 +0000 Subject: [PATCH] Add Ingress Support for L7-ILB HTTP+HTTPS on same LB Previously this was not allowed by the L7-ILB, but now it is. Sharing the same VIP can be used only when specifying a static VIP. --- cmd/e2e-test/ilb_test.go | 150 +++++++++++++++++++++++- pkg/e2e/fixtures.go | 1 + pkg/loadbalancers/l7.go | 4 +- pkg/loadbalancers/loadbalancers_test.go | 34 +++++- 4 files changed, 185 insertions(+), 4 deletions(-) diff --git a/cmd/e2e-test/ilb_test.go b/cmd/e2e-test/ilb_test.go index fbea80eb19..d192b463d6 100644 --- a/cmd/e2e-test/ilb_test.go +++ b/cmd/e2e-test/ilb_test.go @@ -221,7 +221,6 @@ func TestILBStaticIP(t *testing.T) { }) } -// TODO(shance): Remove the SetAllowHttp() calls once L7-ILB supports sharing VIPs func TestILBHttps(t *testing.T) { t.Parallel() @@ -368,6 +367,155 @@ func TestILBHttps(t *testing.T) { } } +// TestILBSharedVIP test the support of HTTPS and HTTP on the same LB when static IP is provided +func TestILBSharedVIP(t *testing.T) { + t.Parallel() + + // These names are useful when reading the debug logs + ingressPrefix := "ing2-" + serviceName := "svc-2" + + port80 := v1.ServiceBackendPort{Number: 80} + + for _, tc := range []struct { + desc string + ingBuilder *fuzz.IngressBuilder + hosts []string + certType e2e.CertType + + numForwardingRules int + numBackendServices int + }{ + { + desc: "https ILB one path, pre-shared cert", + ingBuilder: fuzz.NewIngressBuilder("", ingressPrefix+"1", ""). + AddPath("test.com", "/", serviceName, port80). + ConfigureForILB(), + numForwardingRules: 2, + numBackendServices: 2, + certType: e2e.GCPCert, + hosts: []string{"test.com"}, + }, + { + desc: "https ILB one path, tls", + ingBuilder: fuzz.NewIngressBuilder("", ingressPrefix+"2", ""). + AddPath("test.com", "/", serviceName, port80). + ConfigureForILB(), + numForwardingRules: 2, + numBackendServices: 2, + certType: e2e.K8sCert, + hosts: []string{"test.com"}, + }, + { + desc: "https ILB multiple paths, pre-shared cert", + ingBuilder: fuzz.NewIngressBuilder("", ingressPrefix+"3", ""). + AddPath("test.com", "/foo", serviceName, port80). + AddPath("baz.com", "/bar", serviceName, port80). + ConfigureForILB(), + numForwardingRules: 2, + numBackendServices: 2, + certType: e2e.GCPCert, + hosts: []string{"test.com", "baz.com"}, + }, + { + desc: "https ILB multiple paths, tls", + ingBuilder: fuzz.NewIngressBuilder("", ingressPrefix+"4", ""). + AddPath("test.com", "/foo", serviceName, port80). + AddPath("baz.com", "/bar", serviceName, port80). + ConfigureForILB(), + numForwardingRules: 2, + numBackendServices: 2, + certType: e2e.K8sCert, + hosts: []string{"test.com", "baz.com"}, + }, + } { + tc := tc // Capture tc as we are running this in parallel. + Framework.RunWithSandbox(tc.desc, t, func(t *testing.T, s *e2e.Sandbox) { + t.Parallel() + + addrName := fmt.Sprintf("test-addr-%s", s.Namespace) + if err := e2e.NewGCPAddress(s, addrName, Framework.Region); err != nil { + t.Fatalf("e2e.NewGCPAddress(..., %s) = %v, want nil", addrName, err) + } + defer e2e.DeleteGCPAddress(s, addrName, Framework.Region) + + crud := adapter.IngressCRUD{C: Framework.Clientset} + if Framework.CreateILBSubnet { + if err := e2e.CreateILBSubnet(s); err != nil && err != e2e.ErrSubnetExists { + t.Fatalf("e2e.CreateILBSubnet(%+v) = %v", s, err) + } + } + + for i, h := range tc.hosts { + name := fmt.Sprintf("cert%d--%s", i, s.Namespace) + cert, err := e2e.NewCert(name, h, tc.certType, true) + if err != nil { + t.Fatalf("Error initializing cert: %v", err) + } + if err := cert.Create(s); err != nil { + t.Fatalf("Error creating cert %s: %v", cert.Name, err) + } + defer cert.Delete(s) + + if tc.certType == e2e.K8sCert { + tc.ingBuilder.AddTLS([]string{}, cert.Name) + } else { + tc.ingBuilder.AddPresharedCerts([]string{cert.Name}) + } + } + ing := tc.ingBuilder.AddStaticIP(addrName, true).Build() + ing.Namespace = s.Namespace // namespace depends on sandbox + + t.Logf("Ingress = %s", ing.String()) + + _, err := e2e.CreateEchoService(s, serviceName, negAnnotation) + if err != nil { + t.Fatalf("error creating echo service: %v", err) + } + t.Logf("Echo service created (%s/%s)", s.Namespace, serviceName) + + if _, err := crud.Create(ing); err != nil { + t.Fatalf("error creating Ingress spec: %v", err) + } + t.Logf("Ingress created (%s/%s)", s.Namespace, ing.Name) + + ing, err = e2e.WaitForIngress(s, ing, nil, nil) + if err != nil { + t.Fatalf("error waiting for Ingress to stabilize: %v", err) + } + t.Logf("GCLB resources createdd (%s/%s)", s.Namespace, ing.Name) + + // Perform whitebox testing. + if len(ing.Status.LoadBalancer.Ingress) < 1 { + t.Fatalf("Ingress does not have an IP: %+v", ing.Status) + } + + vip := ing.Status.LoadBalancer.Ingress[0].IP + t.Logf("Ingress %s/%s VIP = %s", s.Namespace, ing.Name, vip) + if !e2e.IsRfc1918Addr(vip) { + t.Fatalf("got %v, want RFC1918 address, ing: %v", vip, ing) + } + + params := &fuzz.GCLBForVIPParams{VIP: vip, Region: Framework.Region, Network: Framework.Network, Validators: fuzz.FeatureValidators(features.All)} + gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, params) + if err != nil { + t.Fatalf("Error getting GCP resources for LB with IP = %q: %v", vip, err) + } + + if err = e2e.CheckGCLB(gclb, tc.numForwardingRules, tc.numBackendServices); err != nil { + t.Error(err) + } + + deleteOptions := &fuzz.GCLBDeleteOptions{ + SkipDefaultBackend: false, + } + if err := e2e.WaitForIngressDeletion(context.Background(), gclb, s, ing, deleteOptions); err != nil { + t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ing.Name, err) + } + }) + } +} + // Test Updating ILB and transitioning between ILB/ELB func TestILBUpdate(t *testing.T) { t.Parallel() diff --git a/pkg/e2e/fixtures.go b/pkg/e2e/fixtures.go index 9c7f9b17b7..9630420ff4 100644 --- a/pkg/e2e/fixtures.go +++ b/pkg/e2e/fixtures.go @@ -317,6 +317,7 @@ func NewGCPAddress(s *Sandbox, name string, region string) error { klog.V(2).Infof("Global static IP %s created", name) } else { addr.AddressType = "INTERNAL" + addr.Purpose = "SHARED_LOADBALANCER_VIP" if err := s.f.Cloud.Addresses().Insert(context.Background(), meta.RegionalKey(addr.Name, region), addr); err != nil { return err } diff --git a/pkg/loadbalancers/l7.go b/pkg/loadbalancers/l7.go index 797d484930..513f4ae7d0 100644 --- a/pkg/loadbalancers/l7.go +++ b/pkg/loadbalancers/l7.go @@ -147,8 +147,8 @@ func (l7 *L7) edgeHop() error { } // Check for invalid L7-ILB HTTPS config before attempting sync - if utils.IsGCEL7ILBIngress(l7.runtimeInfo.Ingress) && sslConfigured && l7.runtimeInfo.AllowHTTP { - l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeWarning, "WillNotConfigureFrontend", "gce-internal Ingress class does not currently support both HTTP and HTTPS served on the same IP (kubernetes.io/ingress.allow-http must be false when using HTTPS).") + if utils.IsGCEL7ILBIngress(l7.runtimeInfo.Ingress) && sslConfigured && l7.runtimeInfo.AllowHTTP && l7.runtimeInfo.StaticIPName == "" { + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeWarning, "WillNotConfigureFrontend", "gce-internal Ingress class must be configured with a static-ip to use both HTTP and HTTPS served on the same IP. Please configure a static-ip with Purpose=SHARED_LOADBALANCER_VIP and attach it to the ingress with the kubernetes.io/ingress.regional-static-ip-name annotation.") return fmt.Errorf("error invalid internal ingress https config") } diff --git a/pkg/loadbalancers/loadbalancers_test.go b/pkg/loadbalancers/loadbalancers_test.go index 624ce6c221..02b82a903c 100644 --- a/pkg/loadbalancers/loadbalancers_test.go +++ b/pkg/loadbalancers/loadbalancers_test.go @@ -263,7 +263,7 @@ func TestCreateHTTPSILBLoadBalancer(t *testing.T) { verifyHTTPSForwardingRuleAndProxyLinks(t, j, l7) } -// Test case with HTTPS ILB Load balancer and AllowHttp set to true (not currently supported) +// Test case with HTTPS ILB Load balancer and AllowHttp set to true // Ensure should throw an error func TestCreateHTTPSILBLoadBalancerAllowHTTP(t *testing.T) { j := newTestJig(t) @@ -283,6 +283,38 @@ func TestCreateHTTPSILBLoadBalancerAllowHTTP(t *testing.T) { } } +// Test case with HTTPS ILB Load balancer and AllowHttp set to true and static IP specified +// Ensure no error thrown +func TestCreateHTTPSILBLoadBalancerAllowHTTPSharedVIP(t *testing.T) { + j := newTestJig(t) + + ipName := "test-ilb-static-ip" + ip := "10.1.2.3" + key, err := composite.CreateKey(j.fakeGCE, ipName, features.L7ILBScope()) + if err != nil { + t.Fatal(err) + } + err = composite.CreateAddress(j.fakeGCE, key, &composite.Address{Name: ipName, Version: meta.VersionGA, Address: ip}) + if err != nil { + t.Fatal(err) + } + + gceUrlMap := utils.NewGCEURLMap() + gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234, BackendNamer: j.namer} + gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000, BackendNamer: j.namer}}}) + lbInfo := &L7RuntimeInfo{ + AllowHTTP: true, + TLS: []*translator.TLSCerts{createCert("key", "cert", "name")}, + UrlMap: gceUrlMap, + Ingress: newILBIngress(), + StaticIPName: ipName, + } + + if _, err := j.pool.Ensure(lbInfo); err != nil { + t.Fatalf("j.pool.Ensure(%v) = %v, want nil", lbInfo, err) + } +} + func TestCreateHTTPSLoadBalancer(t *testing.T) { // This should NOT create the forwarding rule and target proxy // associated with the HTTP branch of this loadbalancer.