From c07353a6a642c067093fea9b031c4bf044d489dd Mon Sep 17 00:00:00 2001
From: Spencer Hance <shance@google.com>
Date: Fri, 27 Sep 2019 19:45:38 -0700
Subject: [PATCH] Check for invalid L7-ILB HTTPS configuration

Create a WillNotConfigureFrontend event warning the user if they set
up an internal https ingress without the allow-http annotation
set to false, since that functionality is currently not supported.
---
 pkg/loadbalancers/l7.go                 |  9 ++++++++-
 pkg/loadbalancers/loadbalancers_test.go | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/pkg/loadbalancers/l7.go b/pkg/loadbalancers/l7.go
index f20b9da405..d17c306c3a 100644
--- a/pkg/loadbalancers/l7.go
+++ b/pkg/loadbalancers/l7.go
@@ -19,6 +19,7 @@ package loadbalancers
 import (
 	"encoding/json"
 	"fmt"
+	"k8s.io/ingress-gce/pkg/flags"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
@@ -154,6 +155,13 @@ func (l *L7) edgeHop() error {
 	// Keeps track if we will "try" to setup frontend resources based on user configuration.
 	// If user configuration dictates we do not, then we emit an event.
 	willConfigureFrontend := false
+	sslConfigured := l.runtimeInfo.TLS != nil || l.runtimeInfo.TLSName != ""
+
+	// Check for invalid L7-ILB HTTPS config before attempting sync
+	if flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) && sslConfigured && l.runtimeInfo.AllowHTTP {
+		l.recorder.Eventf(l.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).")
+		return fmt.Errorf("error invalid internal ingress https config")
+	}
 
 	if err := l.ensureComputeURLMap(); err != nil {
 		return err
@@ -165,7 +173,6 @@ func (l *L7) edgeHop() error {
 		}
 	}
 	// Defer promoting an ephemeral to a static IP until it's really needed.
-	sslConfigured := l.runtimeInfo.TLS != nil || l.runtimeInfo.TLSName != ""
 	if l.runtimeInfo.AllowHTTP && sslConfigured {
 		klog.V(3).Infof("checking static ip for %v", l)
 		if err := l.checkStaticIP(); err != nil {
diff --git a/pkg/loadbalancers/loadbalancers_test.go b/pkg/loadbalancers/loadbalancers_test.go
index d7f097fa32..9c530303c8 100644
--- a/pkg/loadbalancers/loadbalancers_test.go
+++ b/pkg/loadbalancers/loadbalancers_test.go
@@ -207,6 +207,26 @@ func TestCreateHTTPSILBLoadBalancer(t *testing.T) {
 	verifyHTTPSForwardingRuleAndProxyLinks(t, j, l7)
 }
 
+// Test case with HTTPS ILB Load balancer and AllowHttp set to true (not currently supported)
+// Ensure should throw an error
+func TestCreateHTTPSILBLoadBalancerAllowHTTP(t *testing.T) {
+	j := newTestJig(t)
+
+	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:       []*TLSCerts{createCert("key", "cert", "name")},
+		UrlMap:    gceUrlMap,
+		Ingress:   newILBIngress(),
+	}
+
+	if _, err := j.pool.Ensure(lbInfo); err == nil {
+		t.Fatalf("j.pool.Ensure(%v) = nil, want err", lbInfo)
+	}
+}
+
 func TestCreateHTTPSLoadBalancer(t *testing.T) {
 	// This should NOT create the forwarding rule and target proxy
 	// associated with the HTTP branch of this loadbalancer.