Merge pull request #1807 from sawsa307/l7-ilb-https-shared-vip

Add Ingress Support for L7-ILB HTTP+HTTPS on same LB

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

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
 		}

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")
 	}
 

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.