Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Support multiple pre-shared-certs. #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
7 changes: 3 additions & 4 deletions app/kubemci/pkg/gcp/loadbalancer/loadbalancersyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,14 @@ func (l *Syncer) CreateLoadBalancer(ing *v1beta1.Ingress, forceUpdate, validate
if (ingAnnotations.UseNamedTLS() != "") || len(ing.Spec.TLS) > 0 {
// Note that we expect to load the cert from any cluster,
// so users are required to create the secret in all clusters.
certLink, cErr := l.scs.EnsureSSLCert(l.lbName, ing, client, forceUpdate)
certLinks, cErr := l.scs.EnsureSSLCerts(l.lbName, ing, client, forceUpdate)
if cErr != nil {
// Aggregate errors and return all at the end.
cErr = fmt.Errorf("Error ensuring SSL certs: %s", cErr)
fmt.Println(cErr)
err = multierror.Append(err, cErr)
}

tpLink, tpErr := l.tps.EnsureHTTPSTargetProxy(l.lbName, umLink, certLink, forceUpdate)
tpLink, tpErr := l.tps.EnsureHTTPSTargetProxy(l.lbName, umLink, certLinks, forceUpdate)
if tpErr != nil {
// Aggregate errors and return all at the end.
tpErr = fmt.Errorf("Error ensuring HTTPS target proxy: %s", tpErr)
Expand Down Expand Up @@ -246,7 +245,7 @@ func (l *Syncer) DeleteLoadBalancer(ing *v1beta1.Ingress, forceDelete bool) erro
// Aggregate errors and return all at the end.
err = multierror.Append(err, tpErr)
}
if scErr := l.scs.DeleteSSLCert(); scErr != nil {
if scErr := l.scs.DeleteSSLCerts(); scErr != nil {
// Aggregate errors and return all at the end.
err = multierror.Append(err, scErr)
}
Expand Down
6 changes: 3 additions & 3 deletions app/kubemci/pkg/gcp/sslcert/fake_sslcertsyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,17 @@ var _ SyncerInterface = &fakeSSLCertSyncer{}

// EnsureSSLCert ensures that the required ssl certs exist for the given load balancer.
// See interface for more details.
func (f *fakeSSLCertSyncer) EnsureSSLCert(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface, forceUpdate bool) (string, error) {
func (f *fakeSSLCertSyncer) EnsureSSLCerts(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface, forceUpdate bool) ([]string, error) {
f.EnsuredSSLCerts = append(f.EnsuredSSLCerts, fakeSSLCert{
LBName: lbName,
Ingress: ing,
})
return FakeSSLCertSelfLink, nil
return []string{FakeSSLCertSelfLink}, nil
}

// DeleteSSLCert deletes the ssl certs that EnsureSSLCert would have created.
// See interface for more details.
func (f *fakeSSLCertSyncer) DeleteSSLCert() error {
func (f *fakeSSLCertSyncer) DeleteSSLCerts() error {
f.EnsuredSSLCerts = nil
return nil
}
10 changes: 5 additions & 5 deletions app/kubemci/pkg/gcp/sslcert/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (

// SyncerInterface is an interface to manage GCP ssl certs.
type SyncerInterface interface {
// EnsureSSLCert ensures that the required ssl cert exists for the given ingress.
// Will only change an existing SSL cert if forceUpdate=True.
// EnsureSSLCerts ensures that the required ssl cert exists for the given ingress.
// Will only change existing SSL certs if forceUpdate=True.
// Returns the self link for the ensured ssl cert.
EnsureSSLCert(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface, forceUpdate bool) (string, error)
// DeleteSSLCert deletes the ssl cert that EnsureSSLCert would have created.
DeleteSSLCert() error
EnsureSSLCerts(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface, forceUpdate bool) ([]string, error)
// DeleteSSLCerts deletes the ssl certs that EnsureSSLCerts would have created.
DeleteSSLCerts() error
}
36 changes: 23 additions & 13 deletions app/kubemci/pkg/gcp/sslcert/sslcertsyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"net/http"
"reflect"
"strings"

compute "google.golang.org/api/compute/v1"

Expand Down Expand Up @@ -53,27 +54,36 @@ var _ SyncerInterface = &Syncer{}

// EnsureSSLCert ensures that the required ssl certs exist for the given ingress.
// See the interface for more details.
func (s *Syncer) EnsureSSLCert(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface, forceUpdate bool) (string, error) {
func (s *Syncer) EnsureSSLCerts(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface, forceUpdate bool) ([]string, error) {
fmt.Println("Ensuring ssl cert")
annotations := annotations.FromIngress(ing)
if annotations.UseNamedTLS() != "" {
return s.ensurePreSharedSSLCert(lbName, ing, forceUpdate)
return s.ensurePreSharedSSLCerts(lbName, ing, forceUpdate)
}
return s.ensureSecretSSLCert(lbName, ing, client, forceUpdate)
// TODO: Support multiple secret SSL certs.
cert, err := s.ensureSecretSSLCert(lbName, ing, client, forceUpdate)
if err != nil {
return nil, err
}
return []string{cert}, err
}

func (s *Syncer) ensurePreSharedSSLCert(lbName string, ing *v1beta1.Ingress, forceUpdate bool) (string, error) {
func (s *Syncer) ensurePreSharedSSLCerts(lbName string, ing *v1beta1.Ingress, forceUpdate bool) ([]string, error) {
ingAnnotations := annotations.FromIngress(ing)
certName := ingAnnotations.UseNamedTLS()
if certName == "" {
return "", fmt.Errorf("unexpected empty value for %s annotation", annotations.PreSharedCertKey)
certNames := ingAnnotations.UseNamedTLS()
if certNames == "" {
return nil, fmt.Errorf("unexpected empty value for %s annotation", annotations.PreSharedCertKey)
}
// Fetch the certificate and return its self link.
cert, err := s.scp.GetSslCertificate(certName)
if err != nil {
return "", err
var certs []string
for _, certName := range strings.Split(certNames, ",") {
// Fetch the certificate and return its self link.
cert, err := s.scp.GetSslCertificate(certName)
if err != nil {
return nil, err
}
certs = append(certs, cert.SelfLink)
}
return cert.SelfLink, nil
return certs, nil
}

func (s *Syncer) ensureSecretSSLCert(lbName string, ing *v1beta1.Ingress, client kubeclient.Interface, forceUpdate bool) (string, error) {
Expand Down Expand Up @@ -110,7 +120,7 @@ func (s *Syncer) ensureSecretSSLCert(lbName string, ing *v1beta1.Ingress, client

// DeleteSSLCert deletes the ssl certs that EnsureSSLCert would have created.
// See the interface for more details.
func (s *Syncer) DeleteSSLCert() error {
func (s *Syncer) DeleteSSLCerts() error {
name := s.namer.SSLCertName()
fmt.Println("Deleting SSL cert", name)
err := s.scp.DeleteSslCertificate(name)
Expand Down
61 changes: 35 additions & 26 deletions app/kubemci/pkg/gcp/sslcert/sslcertsyncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
"github.com/golang/glog"
)

func TestEnsureSSLCertForSecret(t *testing.T) {
func TestEnsureSSLCertsForSecret(t *testing.T) {
lbName := "lb-name"
scp := ingresslb.NewFakeLoadBalancers("" /* name */, nil /* namer */)
namer := utilsnamer.NewNamer("mci1", lbName)
Expand Down Expand Up @@ -79,19 +79,19 @@ func TestEnsureSSLCertForSecret(t *testing.T) {
}

ing, client := setupIng(true /* withSecret */)
if _, err := scs.EnsureSSLCert(lbName, ing, client, false /*forceUpdate*/); err != nil {
if _, err := scs.EnsureSSLCerts(lbName, ing, client, false /*forceUpdate*/); err != nil {
for _, c := range testCases {
glog.Infof("\nTest case: %s", c.desc)
ensuredCertName, err := scs.EnsureSSLCert(c.lBName, ing, client, c.forceUpdate)
ensuredCertNames, err := scs.EnsureSSLCerts(c.lBName, ing, client, c.forceUpdate)
if (err != nil) != c.ensureErr {
t.Errorf("Ensuring SSL cert... expected err? %v. actual: %v", c.ensureErr, err)
}
if c.ensureErr {
t.Logf("Skipping validation.")
continue
}
if ensuredCertName != certName {
t.Errorf("unexpected cert name, expected: %s, actual: %s", certName, ensuredCertName)
if ensuredCertNames[0] != certName {
t.Errorf("unexpected cert name, expected: %s, actual: %s", certName, ensuredCertNames[0])
}
// Verify that GET does not return NotFound.
cert, err := scp.GetSslCertificate(certName)
Expand All @@ -105,44 +105,53 @@ func TestEnsureSSLCertForSecret(t *testing.T) {
}
}

func TestEnsureSSLCertForPreShared(t *testing.T) {
func TestEnsureSSLCertsForPreShared(t *testing.T) {
lbName := "lb-name"
certName := "testCert"
certNames := []string{"testCert1", "testCert2"}
scp := ingresslb.NewFakeLoadBalancers("" /* name */, nil /* namer */)
namer := utilsnamer.NewNamer("mci1", lbName)
scs := NewSSLCertSyncer(namer, scp)
// GET should return NotFound.
if _, err := scp.GetSslCertificate(certName); err == nil {
t.Fatalf("expected NotFound error, got nil")
for _, certName := range certNames {
if _, err := scp.GetSslCertificate(certName); err == nil {
t.Fatalf("expected NotFound error, got nil")
}
}
ing, client := setupIng(false /* withSecret */)

// EnsureSSLCert should give an error when both the secret and pre shared annotation are missing.
_, err := scs.EnsureSSLCert(lbName, ing, client, false /*forceUpdate*/)
// EnsureSSLCerts should give an error when both the secret and pre shared annotation are missing.
_, err := scs.EnsureSSLCerts(lbName, ing, client, false /*forceUpdate*/)
if err == nil {
t.Errorf("unexpected nil error, expected error since both secret and pre shared cert annotation are missing")
}
// Adding the annotation without a cert should still return an error.
ing.ObjectMeta.Annotations = map[string]string{
annotations.PreSharedCertKey: certName,
annotations.PreSharedCertKey: strings.Join(certNames, ","),
}
_, err = scs.EnsureSSLCert(lbName, ing, client, false /*forceUpdate*/)
_, err = scs.EnsureSSLCerts(lbName, ing, client, false /*forceUpdate*/)
if err == nil {
t.Errorf("unexpected nil error, expected error since the pre shared cert is missing")
}

// EnsureSSLCert should return success if the cert exists as expected.
if _, err := scp.CreateSslCertificate(&compute.SslCertificate{
Name: certName,
}); err != nil {
t.Errorf("unexpected error in creating ssl cert: %s", err)
// EnsureSSLCerts should return success if the cert exists as expected.
for _, certName := range certNames {
if _, err := scp.CreateSslCertificate(&compute.SslCertificate{
Name: certName,
}); err != nil {
t.Errorf("unexpected error in creating ssl cert: %s", err)
}
}
ensuredCertName, err := scs.EnsureSSLCert(lbName, ing, client, false /*forceUpdate*/)
ensuredCertNames, err := scs.EnsureSSLCerts(lbName, ing, client, false /*forceUpdate*/)
if err != nil {
t.Errorf("unexpected error in ensuring ssl cert: %s", err)
}
if ensuredCertName != certName {
t.Errorf("unexpected ensured cert name, expected: %s, actual: %s", certName, ensuredCertName)
if len(ensuredCertNames) != len(certNames) {
t.Errorf("unexpected number of ensured cert names, expected: %d, actual: %d", len(certNames), len(ensuredCertNames))
}
for i, certName := range certNames {
if ensuredCertNames[i] != certName {
t.Errorf("unexpected ensured cert name, expected: %s, actual: %s", certName, ensuredCertNames[i])
}
}
}

Expand Down Expand Up @@ -185,26 +194,26 @@ func setupIngAndClientForSecret(ing *v1beta1.Ingress, client *fake.Clientset, se
})
}

func TestDeleteSSLCert(t *testing.T) {
func TestDeleteSSLCerts(t *testing.T) {
lbName := "lb-name"
// Should create the ssl cert as expected.
scp := ingresslb.NewFakeLoadBalancers("" /* name */, nil /* namer */)
namer := utilsnamer.NewNamer("mci1", lbName)
certName := namer.SSLCertName()
scs := NewSSLCertSyncer(namer, scp)
// Calling DeleteSSLCert when cert does not exist should not return an error.
if err := scs.DeleteSSLCert(); err != nil {
// Calling DeleteSSLCerts when cert does not exist should not return an error.
if err := scs.DeleteSSLCerts(); err != nil {
t.Fatalf("unexpected err while deleting SSL cert that does not exist: %s", err)
}
ing, client := setupIng(true /* withSecret */)
if _, err := scs.EnsureSSLCert(lbName, ing, client, false /*forceUpdate*/); err != nil {
if _, err := scs.EnsureSSLCerts(lbName, ing, client, false /*forceUpdate*/); err != nil {
t.Fatalf("expected no error in ensuring ssl cert, actual: %v", err)
}
if _, err := scp.GetSslCertificate(certName); err != nil {
t.Fatalf("expected nil error, actual: %v", err)
}
// Verify that GET fails after DELETE
if err := scs.DeleteSSLCert(); err != nil {
if err := scs.DeleteSSLCerts(); err != nil {
t.Fatalf("unexpected err while deleting SSL cert: %s", err)
}
if _, err := scp.GetSslCertificate(certName); err == nil {
Expand Down
10 changes: 5 additions & 5 deletions app/kubemci/pkg/gcp/targetproxy/fake_targetproxysyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type fakeTargetProxy struct {
LBName string
UmLink string
// Link to the SSL certificate. This is present only for HTTPS target proxies.
CertLink string
CertLinks []string
}

// FakeTargetProxySyncer is a fake implementation of SyncerInterface to be used in tests.
Expand Down Expand Up @@ -52,11 +52,11 @@ func (f *FakeTargetProxySyncer) EnsureHTTPTargetProxy(lbName, umLink string, for

// EnsureHTTPSTargetProxy ensures that a https target proxy exists for the given load balancer.
// See interface comments for details.
func (f *FakeTargetProxySyncer) EnsureHTTPSTargetProxy(lbName, umLink, certLink string, forceUpdate bool) (string, error) {
func (f *FakeTargetProxySyncer) EnsureHTTPSTargetProxy(lbName, umLink string, certLinks []string, forceUpdate bool) (string, error) {
f.EnsuredTargetProxies = append(f.EnsuredTargetProxies, fakeTargetProxy{
LBName: lbName,
UmLink: umLink,
CertLink: certLink,
LBName: lbName,
UmLink: umLink,
CertLinks: certLinks,
})
return FakeTargetProxySelfLink, nil
}
Expand Down
2 changes: 1 addition & 1 deletion app/kubemci/pkg/gcp/targetproxy/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type SyncerInterface interface {
// overwrite an existing and different http target proxy if forceUpdate
// is true.
// Returns the self link for the ensured proxy.
EnsureHTTPSTargetProxy(lbName, urlMapLink, certLink string, forceUpdate bool) (string, error)
EnsureHTTPSTargetProxy(lbName, urlMapLink string, certLinks []string, forceUpdate bool) (string, error)
// DeleteTargetProxies deletes the target proxies that EnsureHTTPTargetProxy
// and EnsureHTTPSTargetProxy would have created.
DeleteTargetProxies() error
Expand Down
16 changes: 8 additions & 8 deletions app/kubemci/pkg/gcp/targetproxy/targetproxysyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ func (s *Syncer) EnsureHTTPTargetProxy(lbName, umLink string, forceUpdate bool)

// EnsureHTTPSTargetProxy ensures that the required https target proxy exists for the given url map.
// Does nothing if it exists already, else creates a new one.
func (s *Syncer) EnsureHTTPSTargetProxy(lbName, umLink, certLink string, forceUpdate bool) (string, error) {
func (s *Syncer) EnsureHTTPSTargetProxy(lbName, umLink string, certLinks []string, forceUpdate bool) (string, error) {
fmt.Println("Ensuring http target proxy.")
var err error
tpLink, proxyErr := s.ensureHTTPSProxy(lbName, umLink, certLink, forceUpdate)
tpLink, proxyErr := s.ensureHTTPSProxy(lbName, umLink, certLinks, forceUpdate)
if proxyErr != nil {
proxyErr = fmt.Errorf("Error in ensuring https target proxy: %s", proxyErr)
err = multierror.Append(err, proxyErr)
Expand Down Expand Up @@ -147,8 +147,8 @@ func (s *Syncer) updateHTTPTargetProxy(desiredHTTPProxy *compute.TargetHttpProxy
name := desiredHTTPProxy.Name
fmt.Println("Updating existing target http proxy", name, "to match the desired state")
// There is no UpdateTargetHTTPProxy method.
// Apart from name, UrlMap is the only field that can be different. We update that field directly.
// TODO(nikhiljindal): Handle description field differences:
// We can only update the UrlMap via SetUrlMapForTargetHttpProxy.
// TODO(nikhiljindal): Handle sslcert and description field differences:
// https://github.com/GoogleCloudPlatform/k8s-multicluster-ingress/issues/94.
urlMap := &compute.UrlMap{SelfLink: desiredHTTPProxy.UrlMap}
glog.Infof("Setting URL Map to:%+v", urlMap)
Expand Down Expand Up @@ -211,9 +211,9 @@ func (s *Syncer) desiredHTTPTargetProxy(lbName, umLink string) *compute.TargetHt
// ensureHTTPSProxy ensures that the required target proxy exists for the given port.
// Does nothing if it exists already, else creates a new one.
// Returns the self link for the ensured https proxy.
func (s *Syncer) ensureHTTPSProxy(lbName, umLink, certLink string, forceUpdate bool) (string, error) {
func (s *Syncer) ensureHTTPSProxy(lbName, umLink string, certLinks []string, forceUpdate bool) (string, error) {
fmt.Println("Ensuring target https proxy")
desiredHTTPSProxy := s.desiredHTTPSTargetProxy(lbName, umLink, certLink)
desiredHTTPSProxy := s.desiredHTTPSTargetProxy(lbName, umLink, certLinks)
name := desiredHTTPSProxy.Name
// Check if target proxy already exists.
existingHTTPSProxy, err := s.tpp.GetTargetHttpsProxy(name)
Expand Down Expand Up @@ -292,12 +292,12 @@ func targetHTTPSProxyMatches(desiredHTTPSProxy, existingHTTPSProxy compute.Targe
}
return equal
}
func (s *Syncer) desiredHTTPSTargetProxy(lbName, umLink, certLink string) *compute.TargetHttpsProxy {
func (s *Syncer) desiredHTTPSTargetProxy(lbName, umLink string, certLinks []string) *compute.TargetHttpsProxy {
// Compute the desired target https proxy.
return &compute.TargetHttpsProxy{
Name: s.namer.TargetHTTPSProxyName(),
Description: fmt.Sprintf("Target https proxy for kubernetes multicluster loadbalancer %s", lbName),
UrlMap: umLink,
SslCertificates: []string{certLink},
SslCertificates: certLinks,
}
}
Loading