Skip to content

Commit

Permalink
Merge pull request #1101 from rramkumar1/translator-refactor
Browse files Browse the repository at this point in the history 
Initial refactors to bootstrap self-contained translator
  • Loading branch information
@rramkumar1
rramkumar1 authored May 27, 2020
2 parents a294488 + 34f0947 commit 85f7e0f
Show file tree
Hide file tree
Showing 7 changed files with 465 additions and 236 deletions.
46 changes: 0 additions & 46 deletions hack/run-local-glbc.sh
View file

This file was deleted.

3 changes: 2 additions & 1 deletion pkg/loadbalancers/loadbalancers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/instances"
"k8s.io/ingress-gce/pkg/loadbalancers/features"
"k8s.io/ingress-gce/pkg/translator"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/common"
namer_util "k8s.io/ingress-gce/pkg/utils/namer"
Expand Down Expand Up @@ -1185,7 +1186,7 @@ func verifyURLMap(t *testing.T, j *testJig, feNamer namer_util.IngressFrontendNa
if err != nil || um == nil {
t.Errorf("j.fakeGCE.GetUrlMap(%q) = %v, %v; want _, nil", name, um, err)
}
wantComputeURLMap := toCompositeURLMap(wantGCEURLMap, feNamer, key)
wantComputeURLMap := translator.ToCompositeURLMap(wantGCEURLMap, feNamer, key)
if !mapsEqual(wantComputeURLMap, um) {
t.Errorf("mapsEqual() = false, got\n%+v\n want\n%+v", um, wantComputeURLMap)
}
Expand Down
103 changes: 2 additions & 101 deletions pkg/loadbalancers/url_maps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,15 @@ limitations under the License.
package loadbalancers

import (
"crypto/md5"
"encoding/hex"
"fmt"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/translator"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/namer"
"k8s.io/klog"
)

const (
// The gce api uses the name of a path rule to match a host rule.
hostRulePrefix = "host"
)

// ensureComputeURLMap retrieves the current URLMap and overwrites it if incorrect. If the resource
// does not exist, the map is created.
func (l *L7) ensureComputeURLMap() error {
Expand All @@ -49,7 +40,7 @@ func (l *L7) ensureComputeURLMap() error {
if err != nil {
return err
}
expectedMap := toCompositeURLMap(l.runtimeInfo.UrlMap, l.namer, key)
expectedMap := translator.ToCompositeURLMap(l.runtimeInfo.UrlMap, l.namer, key)
key.Name = expectedMap.Name

expectedMap.Version = l.Versions().UrlMap
Expand Down Expand Up @@ -176,93 +167,3 @@ func mapsEqual(a, b *composite.UrlMap) bool {
}
return true
}

// toCompositeURLMap translates the given hostname: endpoint->port mapping into a gce url map.
//
// HostRule: Conceptually contains all PathRules for a given host.
// PathMatcher: Associates a path rule with a host rule. Mostly an optimization.
// PathRule: Maps a single path regex to a backend.
//
// The GCE url map allows multiple hosts to share url->backend mappings without duplication, eg:
// Host: foo(PathMatcher1), bar(PathMatcher1,2)
// PathMatcher1:
// /a -> b1
// /b -> b2
// PathMatcher2:
// /c -> b1
// This leads to a lot of complexity in the common case, where all we want is a mapping of
// host->{/path: backend}.
//
// Consider some alternatives:
// 1. Using a single backend per PathMatcher:
// Host: foo(PathMatcher1,3) bar(PathMatcher1,2,3)
// PathMatcher1:
// /a -> b1
// PathMatcher2:
// /c -> b1
// PathMatcher3:
// /b -> b2
// 2. Using a single host per PathMatcher:
// Host: foo(PathMatcher1)
// PathMatcher1:
// /a -> b1
// /b -> b2
// Host: bar(PathMatcher2)
// PathMatcher2:
// /a -> b1
// /b -> b2
// /c -> b1
// In the context of kubernetes services, 2 makes more sense, because we
// rarely want to lookup backends (service:nodeport). When a service is
// deleted, we need to find all host PathMatchers that have the backend
// and remove the mapping. When a new path is added to a host (happens
// more frequently than service deletion) we just need to lookup the 1
// pathmatcher of the host.
func toCompositeURLMap(g *utils.GCEURLMap, namer namer.IngressFrontendNamer, key *meta.Key) *composite.UrlMap {
defaultBackendName := g.DefaultBackend.BackendName()
key.Name = defaultBackendName
resourceID := cloud.ResourceID{ProjectID: "", Resource: "backendServices", Key: key}
m := &composite.UrlMap{
Name: namer.UrlMap(),
DefaultService: resourceID.ResourcePath(),
}

for _, hostRule := range g.HostRules {
// Create a host rule
// Create a path matcher
// Add all given endpoint:backends to pathRules in path matcher
pmName := getNameForPathMatcher(hostRule.Hostname)
m.HostRules = append(m.HostRules, &composite.HostRule{
Hosts: []string{hostRule.Hostname},
PathMatcher: pmName,
})

pathMatcher := &composite.PathMatcher{
Name: pmName,
DefaultService: m.DefaultService,
PathRules: []*composite.PathRule{},
}

// GCE ensures that matched rule with longest prefix wins.
for _, rule := range hostRule.Paths {
beName := rule.Backend.BackendName()
key.Name = beName
resourceID := cloud.ResourceID{ProjectID: "", Resource: "backendServices", Key: key}
beLink := resourceID.ResourcePath()
pathMatcher.PathRules = append(pathMatcher.PathRules, &composite.PathRule{
Paths: []string{rule.Path},
Service: beLink,
})
}
m.PathMatchers = append(m.PathMatchers, pathMatcher)
}
return m
}

// getNameForPathMatcher returns a name for a pathMatcher based on the given host rule.
// The host rule can be a regex, the path matcher name used to associate the 2 cannot.
func getNameForPathMatcher(hostRule string) string {
hasher := md5.New()
hasher.Write([]byte(hostRule))
return fmt.Sprintf("%v%v", hostRulePrefix, hex.EncodeToString(hasher.Sum(nil)))
}
50 changes: 1 addition & 49 deletions pkg/loadbalancers/url_maps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,8 @@ package loadbalancers
import (
"testing"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/utils"
namer_util "k8s.io/ingress-gce/pkg/utils/namer"
)

func TestComputeURLMapEquals(t *testing.T) {
Expand All @@ -33,7 +30,7 @@ func TestComputeURLMapEquals(t *testing.T) {
// Test equality.
same := testCompositeURLMap()
if !mapsEqual(m, same) {
t.Errorf("mapsEqual(%+v, %+v) = true, want false", m, same)
t.Errorf("mapsEqual(%+v, %+v) = false, want true", m, same)
}

// Test different default backend.
Expand All @@ -44,51 +41,6 @@ func TestComputeURLMapEquals(t *testing.T) {
}
}

func TestToComputeURLMap(t *testing.T) {
t.Parallel()

wantComputeMap := testCompositeURLMap()
namer := namer_util.NewNamer("uid1", "fw1")
gceURLMap := &utils.GCEURLMap{
DefaultBackend: &utils.ServicePort{NodePort: 30000, BackendNamer: namer},
HostRules: []utils.HostRule{
{
Hostname: "abc.com",
Paths: []utils.PathRule{
{
Path: "/web",
Backend: utils.ServicePort{NodePort: 32000, BackendNamer: namer},
},
{
Path: "/other",
Backend: utils.ServicePort{NodePort: 32500, BackendNamer: namer},
},
},
},
{
Hostname: "foo.bar.com",
Paths: []utils.PathRule{
{
Path: "/",
Backend: utils.ServicePort{NodePort: 33000, BackendNamer: namer},
},
{
Path: "/*",
Backend: utils.ServicePort{NodePort: 33500, BackendNamer: namer},
},
},
},
},
}

namerFactory := namer_util.NewFrontendNamerFactory(namer, "")
feNamer := namerFactory.NamerForLoadBalancer("ns/lb-name")
gotComputeURLMap := toCompositeURLMap(gceURLMap, feNamer, meta.GlobalKey("ns-lb-name"))
if !mapsEqual(gotComputeURLMap, wantComputeMap) {
t.Errorf("toComputeURLMap() = \n%+v\n want\n%+v", gotComputeURLMap, wantComputeMap)
}
}

func testCompositeURLMap() *composite.UrlMap {
return &composite.UrlMap{
Name: "k8s-um-lb-name",
Expand Down
55 changes: 16 additions & 39 deletions pkg/tls/tls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,70 +17,48 @@ limitations under the License.
package tls

import (
"context"
"fmt"

"k8s.io/klog"

api_v1 "k8s.io/api/core/v1"
"k8s.io/api/networking/v1beta1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/ingress-gce/pkg/loadbalancers"
"k8s.io/ingress-gce/pkg/translator"
)

// TlsLoader is the interface for loading the relevant TLSCerts for a given ingress.
type TlsLoader interface {
// Load loads the relevant TLSCerts based on ing.Spec.TLS
Load(ing *v1beta1.Ingress) ([]*loadbalancers.TLSCerts, error)
// Validate validates the given TLSCerts and returns an error if they are invalid.
Validate(certs *loadbalancers.TLSCerts) error
}

// TODO: Add better cert validation.
type noOPValidator struct{}

func (n *noOPValidator) Validate(certs *loadbalancers.TLSCerts) error {
return nil
}

// TLSCertsFromSecretsLoader loads TLS certs from kubernetes secrets.
type TLSCertsFromSecretsLoader struct {
noOPValidator
Client kubernetes.Interface
}

// Ensure that TLSCertsFromSecretsLoader implements TlsLoader interface.
var _ TlsLoader = &TLSCertsFromSecretsLoader{}

func (t *TLSCertsFromSecretsLoader) Load(ing *v1beta1.Ingress) ([]*loadbalancers.TLSCerts, error) {
if len(ing.Spec.TLS) == 0 {
return nil, nil
env, err := translator.NewEnv(ing, t.Client)
if err != nil {
return nil, fmt.Errorf("error initializing translator env: %v", err)
}

var certs []*loadbalancers.TLSCerts

for _, tlsSecret := range ing.Spec.TLS {
if tlsSecret.SecretName == "" {
continue
}
// TODO: Replace this for a secret watcher.
klog.V(3).Infof("Retrieving secret for ing %v with name %v", ing.Name, tlsSecret.SecretName)
secret, err := t.Client.CoreV1().Secrets(ing.Namespace).Get(context.TODO(), tlsSecret.SecretName, meta_v1.GetOptions{})
if err != nil {
return nil, err
}
cert, ok := secret.Data[api_v1.TLSCertKey]
if !ok {
return nil, fmt.Errorf("secret %v has no 'tls.crt'", tlsSecret.SecretName)
}
key, ok := secret.Data[api_v1.TLSPrivateKeyKey]
if !ok {
return nil, fmt.Errorf("secret %v has no 'tls.key'", tlsSecret.SecretName)
}
newCert := &loadbalancers.TLSCerts{Key: string(key), Cert: string(cert), Name: tlsSecret.SecretName,
CertHash: loadbalancers.GetCertHash(string(cert))}
if err := t.Validate(newCert); err != nil {
return nil, err
secrets, err := translator.Secrets(env)
if err != nil {
return nil, fmt.Errorf("error getting secrets for Ingress: %v", err)
}
for _, secret := range secrets {
cert := string(secret.Data[api_v1.TLSCertKey])
newCert := &loadbalancers.TLSCerts{
Key: string(secret.Data[api_v1.TLSPrivateKeyKey]),
Cert: cert,
Name: secret.Name,
CertHash: loadbalancers.GetCertHash(cert),
}
certs = append(certs, newCert)
}
Expand All @@ -91,7 +69,6 @@ func (t *TLSCertsFromSecretsLoader) Load(ing *v1beta1.Ingress) ([]*loadbalancers

// FakeTLSSecretLoader fakes out TLS loading.
type FakeTLSSecretLoader struct {
noOPValidator
FakeCerts map[string]*loadbalancers.TLSCerts
}

Expand Down
Loading

0 comments on commit 85f7e0f

Please sign in to comment.