Merge pull request #1126 from rramkumar1/forwarding-rule-translator

Add translation for ForwardingRule

pkg/loadbalancers/forwarding_rules.go

@@ -26,17 +26,13 @@ import (
 	"k8s.io/ingress-gce/pkg/annotations"
 	"k8s.io/ingress-gce/pkg/composite"
 	"k8s.io/ingress-gce/pkg/flags"
+	"k8s.io/ingress-gce/pkg/translator"
 	"k8s.io/ingress-gce/pkg/utils"
 	"k8s.io/ingress-gce/pkg/utils/namer"
 	"k8s.io/klog"
 	"k8s.io/legacy-cloud-providers/gce"
 )
 
-const (
-	httpDefaultPortRange  = "80-80"
-	httpsDefaultPortRange = "443-443"
-)
-
 func (l *L7) checkHttpForwardingRule() (err error) {
 	if l.tp == nil {
 		return fmt.Errorf("cannot create forwarding rule without proxy")
@@ -46,7 +42,7 @@ func (l *L7) checkHttpForwardingRule() (err error) {
 	if err != nil {
 		return err
 	}
-	fw, err := l.checkForwardingRule(name, l.tp.SelfLink, address, httpDefaultPortRange)
+	fw, err := l.checkForwardingRule(namer.HTTPProtocol, name, l.tp.SelfLink, address)
 	if err != nil {
 		return err
 	}
@@ -64,30 +60,40 @@ func (l *L7) checkHttpsForwardingRule() (err error) {
 	if err != nil {
 		return err
 	}
-	fws, err := l.checkForwardingRule(name, l.tps.SelfLink, address, httpsDefaultPortRange)
+	fws, err := l.checkForwardingRule(namer.HTTPSProtocol, name, l.tps.SelfLink, address)
 	if err != nil {
 		return err
 	}
 	l.fws = fws
 	return nil
 }
 
-func (l *L7) checkForwardingRule(name, proxyLink, ip, portRange string) (fw *composite.ForwardingRule, err error) {
+func (l *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink, ip string) (existing *composite.ForwardingRule, err error) {
 	key, err := l.CreateKey(name)
 	if err != nil {
 		return nil, err
 	}
 	version := l.Versions().ForwardingRule
-	fw, _ = composite.GetForwardingRule(l.cloud, key, version)
-	if fw != nil && (ip != "" && fw.IPAddress != ip || fw.PortRange != portRange) {
+	description, err := l.description()
+	if err != nil {
+		return nil, err
+	}
+
+	isL7ILB := flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress)
+	tr := translator.NewTranslator(isL7ILB, l.namer)
+	env := &translator.Env{VIP: ip, Network: l.cloud.NetworkURL(), Subnetwork: l.cloud.SubnetworkURL()}
+	fr := tr.ToCompositeForwardingRule(env, protocol, version, proxyLink, description)
+
+	existing, _ = composite.GetForwardingRule(l.cloud, key, version)
+	if existing != nil && (fr.IPAddress != "" && existing.IPAddress != fr.IPAddress || existing.PortRange != fr.PortRange) {
 		klog.Warningf("Recreating forwarding rule %v(%v), so it has %v(%v)",
-			fw.IPAddress, fw.PortRange, ip, portRange)
+			existing.IPAddress, existing.PortRange, fr.IPAddress, fr.PortRange)
 		if err = utils.IgnoreHTTPNotFound(composite.DeleteForwardingRule(l.cloud, key, version)); err != nil {
 			return nil, err
 		}
-		fw = nil
+		existing = nil
 	}
-	if fw == nil {
+	if existing == nil {
 		// This is a special case where exactly one of http or https forwarding rule
 		// existed before and the existing forwarding rule uses ingress managed static ip address.
 		// In this case, the forwarding rule needs to be created with the same static ip.
@@ -104,55 +110,35 @@ func (l *L7) checkForwardingRule(name, proxyLink, ip, portRange string) (fw *com
 				}
 			}
 		}
-		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 := &composite.ForwardingRule{
-			Name:        name,
-			IPAddress:   ip,
-			Target:      proxyLink,
-			PortRange:   portRange,
-			IPProtocol:  "TCP",
-			Description: description,
-			Version:     version,
-		}
-
-		// Update rule for L7-ILB
-		if flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) {
-			rule.LoadBalancingScheme = "INTERNAL_MANAGED"
-			rule.Network = l.cloud.NetworkURL()
-			rule.Subnetwork = l.cloud.SubnetworkURL()
-		}
+		klog.V(3).Infof("Creating forwarding rule for proxy %q and ip %v:%v", proxyLink, ip, protocol)
 
-		if err = composite.CreateForwardingRule(l.cloud, key, rule); err != nil {
+		if err = composite.CreateForwardingRule(l.cloud, key, fr); err != nil {
 			return nil, err
 		}
 		key, err = l.CreateKey(name)
 		if err != nil {
 			return nil, err
 		}
-		fw, err = composite.GetForwardingRule(l.cloud, key, version)
+		existing, err = composite.GetForwardingRule(l.cloud, key, version)
 		if err != nil {
 			return nil, err
 		}
 	}
 	// TODO: If the port range and protocol don't match, recreate the rule
-	if utils.EqualResourceIDs(fw.Target, proxyLink) {
-		klog.V(4).Infof("Forwarding rule %v already exists", fw.Name)
+	if utils.EqualResourceIDs(existing.Target, proxyLink) {
+		klog.V(4).Infof("Forwarding rule %v already exists", existing.Name)
 	} else {
 		klog.V(3).Infof("Forwarding rule %v has the wrong proxy, setting %v overwriting %v",
-			fw.Name, fw.Target, proxyLink)
-		key, err := l.CreateKey(fw.Name)
+			existing.Name, existing.Target, proxyLink)
+		key, err := l.CreateKey(existing.Name)
 		if err != nil {
 			return nil, err
 		}
-		if err := composite.SetProxyForForwardingRule(l.cloud, key, fw, proxyLink); err != nil {
+		if err := composite.SetProxyForForwardingRule(l.cloud, key, existing, proxyLink); err != nil {
 			return nil, err
 		}
 	}
-	return fw, nil
+	return existing, nil
 }
 
 // getEffectiveIP returns a string with the IP to use in the HTTP and HTTPS

pkg/tls/tls.go

@@ -41,7 +41,7 @@ type TLSCertsFromSecretsLoader struct {
 var _ TlsLoader = &TLSCertsFromSecretsLoader{}
 
 func (t *TLSCertsFromSecretsLoader) Load(ing *v1beta1.Ingress) ([]*loadbalancers.TLSCerts, error) {
-	env, err := translator.NewEnv(ing, t.Client)
+	env, err := translator.NewEnv(ing, t.Client, "", "", "")
 	if err != nil {
 		return nil, fmt.Errorf("error initializing translator env: %v", err)
 	}

pkg/translator/translator.go

@@ -33,19 +33,24 @@ import (
 	"k8s.io/ingress-gce/pkg/utils/namer"
 )
 
-// Env contains all k8s-style configuration needed to perform the translation.
+// Env contains all k8s & GCP configuration needed to perform the translation.
 type Env struct {
 	// Ing is the Ingress we are translating.
 	Ing *v1beta1.Ingress
 	// SecretsMap contains a mapping from Secret name to the actual resource.
 	// It is assumed that the map contains resources from a single namespace.
 	// This is the same namespace as the Ingress namespace.
 	SecretsMap map[string]*api_v1.Secret
+	// VIP is the IP address assigned to the Ingress. This could be a raw IP address in GCP or the
+	// name of an Address resource.
+	VIP        string
+	Network    string
+	Subnetwork string
 }
 
 // NewEnv returns an Env for the given Ingress.
-func NewEnv(ing *v1beta1.Ingress, client kubernetes.Interface) (*Env, error) {
-	ret := &Env{Ing: ing, SecretsMap: make(map[string]*api_v1.Secret)}
+func NewEnv(ing *v1beta1.Ingress, client kubernetes.Interface, vip, net, subnet string) (*Env, error) {
+	ret := &Env{Ing: ing, SecretsMap: make(map[string]*api_v1.Secret), VIP: vip, Network: net, Subnetwork: subnet}
 	secrets, err := client.CoreV1().Secrets(ing.Namespace).List(context.TODO(), meta_v1.ListOptions{})
 	if err != nil {
 		return nil, err
@@ -57,12 +62,17 @@ func NewEnv(ing *v1beta1.Ingress, client kubernetes.Interface) (*Env, error) {
 	return ret, nil
 }
 
-// Translator implements the mapping between an MCI and its corresponding GCE resources.
-type Translator struct{}
+// Translator implements the mapping between an Ingress and its corresponding GCE resources.
+type Translator struct {
+	// IsL7ILB is true if the Ingress will be translated into an L7 ILB (as opposed to an XLB).
+	IsL7ILB bool
+	// FrontendNamer generates names for frontend resources.
+	FrontendNamer namer.IngressFrontendNamer
+}
 
 // NewTranslator returns a new Translator.
-func NewTranslator() *Translator {
-	return &Translator{}
+func NewTranslator(isL7ILB bool, frontendNamer namer.IngressFrontendNamer) *Translator {
+	return &Translator{IsL7ILB: isL7ILB, FrontendNamer: frontendNamer}
 }
 
 // Secrets returns the Secrets from the environment which are specified in the Ingress.
@@ -179,3 +189,36 @@ func getNameForPathMatcher(hostRule string) string {
 	hasher.Write([]byte(hostRule))
 	return fmt.Sprintf("%v%v", hostRulePrefix, hex.EncodeToString(hasher.Sum(nil)))
 }
+
+const (
+	httpDefaultPortRange  = "80-80"
+	httpsDefaultPortRange = "443-443"
+)
+
+// ToCompositeForwardingRule returns a composite.ForwardingRule of type HTTP or HTTPS.
+func (t *Translator) ToCompositeForwardingRule(env *Env, protocol namer.NamerProtocol, version meta.Version, proxyLink, description string) *composite.ForwardingRule {
+	var portRange string
+	if protocol == namer.HTTPProtocol {
+		portRange = httpDefaultPortRange
+	} else {
+		portRange = httpsDefaultPortRange
+	}
+
+	fr := &composite.ForwardingRule{
+		Name:        t.FrontendNamer.ForwardingRule(protocol),
+		IPAddress:   env.VIP,
+		Target:      proxyLink,
+		PortRange:   portRange,
+		IPProtocol:  "TCP",
+		Description: description,
+		Version:     version,
+	}
+
+	if t.IsL7ILB {
+		fr.LoadBalancingScheme = "INTERNAL_MANAGED"
+		fr.Network = env.Network
+		fr.Subnetwork = env.Subnetwork
+	}
+
+	return fr
+}

pkg/translator/translator_test.go

@@ -15,6 +15,7 @@ package translator
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
@@ -29,6 +30,39 @@ import (
 	namer_util "k8s.io/ingress-gce/pkg/utils/namer"
 )
 
+// testNamer implements IngressFrontendNamer
+type testNamer struct {
+	prefix string
+}
+
+func (n *testNamer) ForwardingRule(namer_util.NamerProtocol) string {
+	return fmt.Sprintf("%s-fr", n.prefix)
+}
+
+func (n *testNamer) TargetProxy(namer_util.NamerProtocol) string {
+	return fmt.Sprintf("%s-tp", n.prefix)
+}
+
+func (n *testNamer) UrlMap() string {
+	return fmt.Sprintf("%s-um", n.prefix)
+}
+
+func (n *testNamer) SSLCertName(secretHash string) string {
+	return fmt.Sprintf("%s-cert-%s", n.prefix, secretHash)
+}
+
+func (n *testNamer) IsCertNameForLB(string) bool {
+	panic("Unimplemented")
+}
+
+func (n *testNamer) IsLegacySSLCert(string) bool {
+	panic("Unimplemented")
+}
+
+func (n *testNamer) LoadBalancer() namer_util.LoadBalancerName {
+	panic("Unimplemented")
+}
+
 func TestToComputeURLMap(t *testing.T) {
 	t.Parallel()
 
@@ -242,7 +276,7 @@ func TestSecrets(t *testing.T) {
 				kubeClient.CoreV1().Secrets(tc.ing.Namespace).Create(context.TODO(), v, meta_v1.CreateOptions{})
 			}
 
-			env, err := NewEnv(tc.ing, kubeClient)
+			env, err := NewEnv(tc.ing, kubeClient, "", "", "")
 			if err != nil {
 				t.Fatalf("NewEnv(): %v", err)
 			}
@@ -261,3 +295,91 @@ func TestSecrets(t *testing.T) {
 		})
 	}
 }
+
+func TestToForwardingRule(t *testing.T) {
+	proxyLink := "my-proxy"
+	description := "foo"
+	version := meta.VersionGA
+	network := "my-network"
+	subnetwork := "my-subnetwork"
+	vip := "127.0.0.1"
+
+	cases := []struct {
+		desc     string
+		isL7ILB  bool
+		protocol namer_util.NamerProtocol
+		want     *composite.ForwardingRule
+	}{
+		{
+			desc:     "http-xlb",
+			protocol: namer_util.HTTPProtocol,
+			want: &composite.ForwardingRule{
+				Name:        "foo-fr",
+				IPAddress:   vip,
+				Target:      proxyLink,
+				PortRange:   httpDefaultPortRange,
+				IPProtocol:  "TCP",
+				Description: description,
+				Version:     version,
+			},
+		},
+		{
+			desc:     "https-xlb",
+			protocol: namer_util.HTTPSProtocol,
+			want: &composite.ForwardingRule{
+				Name:        "foo-fr",
+				IPAddress:   vip,
+				Target:      proxyLink,
+				PortRange:   httpsDefaultPortRange,
+				IPProtocol:  "TCP",
+				Description: description,
+				Version:     version,
+			},
+		},
+		{
+			desc:     "http-ilb",
+			isL7ILB:  true,
+			protocol: namer_util.HTTPProtocol,
+			want: &composite.ForwardingRule{
+				Name:                "foo-fr",
+				IPAddress:           vip,
+				Target:              proxyLink,
+				PortRange:           httpDefaultPortRange,
+				IPProtocol:          "TCP",
+				Description:         description,
+				Version:             version,
+				LoadBalancingScheme: "INTERNAL_MANAGED",
+				Network:             network,
+				Subnetwork:          subnetwork,
+			},
+		},
+		{
+			desc:     "https-ilb",
+			isL7ILB:  true,
+			protocol: namer_util.HTTPSProtocol,
+			want: &composite.ForwardingRule{
+				Name:                "foo-fr",
+				IPAddress:           vip,
+				Target:              proxyLink,
+				PortRange:           httpsDefaultPortRange,
+				IPProtocol:          "TCP",
+				Description:         description,
+				Version:             version,
+				LoadBalancingScheme: "INTERNAL_MANAGED",
+				Network:             network,
+				Subnetwork:          subnetwork,
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			tr := NewTranslator(tc.isL7ILB, &testNamer{"foo"})
+			env := &Env{VIP: vip, Network: network, Subnetwork: subnetwork}
+			got := tr.ToCompositeForwardingRule(env, tc.protocol, version, proxyLink, description)
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Fatalf("Got diff for ForwardingRule (-want +got):\n%s", diff)
+			}
+		})
+	}
+}