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.

Sign up for GitHub

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

Initial refactors to bootstrap self-contained translator #1101

Merged
merged 2 commits into from
May 27, 2020
Merged

Initial refactors to bootstrap self-contained translator #1101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
879292b
Move function which generated composite URLMap to translator pkg
rramkumar1 May 12, 2020
34f0947
Add function in translator to fetch secrets for Ingress
rramkumar1 May 12, 2020
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
46 changes: 0 additions & 46 deletions hack/run-local-glbc.sh
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion pkg/loadbalancers/loadbalancers_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this code does not have unit tests right now. I plan on adding some in a follow up PR.

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