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

Add translation for ForwardingRule #1126

Merged
merged 1 commit into from
Jun 17, 2020
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
70 changes: 28 additions & 42 deletions pkg/loadbalancers/forwarding_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
}
Expand All @@ -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.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/tls/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
57 changes: 50 additions & 7 deletions pkg/translator/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
124 changes: 123 additions & 1 deletion pkg/translator/translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package translator

import (
"context"
"fmt"
"testing"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
Expand All @@ -29,6 +30,39 @@ import (
namer_util "k8s.io/ingress-gce/pkg/utils/namer"
)

// testNamer implements IngressFrontendNamer
type testNamer struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we already have namer mocks?

Copy link
Contributor

Choose a reason for hiding this comment

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

synced up, we will look into this in a follow up

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

Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
})
}
}