Skip to content

Commit 7e7d9f0

Browse files
MadalinaPatrichiMadalinaPatrichi
committed
Refactor SSL Certificates
1 parent 89656d0 commit 7e7d9f0

File tree

5 files changed

+115
-122
lines changed

5 files changed

+115
-122
lines changed

‎pkg/oci/client/load_balancer.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type LoadBalancerInterface interface {
3535
DeleteLoadBalancer(ctx context.Context, id string) (string, error)
3636

3737
GetCertificateByName(ctx context.Context, lbID, name string) (*loadbalancer.Certificate, error)
38-
CreateCertificate(ctx context.Context, lbID, certificate, key string) (string, error)
38+
CreateCertificate(ctx context.Context, lbID string, cert loadbalancer.CertificateDetails) (string, error)
3939

4040
CreateBackendSet(ctx context.Context, lbID, name string, details loadbalancer.BackendSetDetails) (string, error)
4141
UpdateBackendSet(ctx context.Context, lbID, name string, details loadbalancer.BackendSetDetails) (string, error)
@@ -150,18 +150,19 @@ func (c *client) GetCertificateByName(ctx context.Context, lbID, name string) (*
150150
return nil, errors.WithStack(errNotFound)
151151
}
152152

153-
func (c *client) CreateCertificate(ctx context.Context, lbID, certificate, key string) (string, error) {
153+
func (c *client) CreateCertificate(ctx context.Context, lbID string, cert loadbalancer.CertificateDetails) (string, error) {
154154
if !c.rateLimiter.Writer.TryAccept() {
155155
return "", RateLimitError(true, "CreateCertificate")
156156
}
157157

158-
// TODO(apryde): We currently don't have a mechanism for supplying
159-
// CreateCertificateDetails.CaCertificate.
160158
resp, err := c.loadbalancer.CreateCertificate(ctx, loadbalancer.CreateCertificateRequest{
161159
LoadBalancerId: &lbID,
162160
CreateCertificateDetails: loadbalancer.CreateCertificateDetails{
163-
PublicCertificate: &certificate,
164-
PrivateKey: &key,
161+
CertificateName: cert.CertificateName,
162+
CaCertificate: cert.CaCertificate,
163+
PublicCertificate: cert.PublicCertificate,
164+
PrivateKey: cert.PrivateKey,
165+
Passphrase: cert.Passphrase,
165166
},
166167
})
167168
incRequestCounter(err, createVerb, certificateResource)

‎pkg/oci/load_balancer.go

+31-67
Original file line numberDiff line numberDiff line change
@@ -196,78 +196,49 @@ func getSubnetsForNodes(ctx context.Context, nodes []*v1.Node, client client.Int
196196

197197
// readSSLSecret returns the certificate and private key from a Kubernetes TLS
198198
// private key Secret.
199-
func (cp *CloudProvider) readSSLSecret(secretType string, svc *v1.Service) (*certificateData, error) {
200-
secretString, ok := svc.Annotations[secretType]
201-
if !ok && secretType == ServiceAnnotationLoadBalancerTLSSecret {
202-
return nil, errors.Errorf("no %q annotation found", secretType)
203-
}
204-
if !ok {
205-
return &certificateData{CACert: "", PublicCert: "", PrivateKey: "",
206-
Passphrase: ""}, nil
207-
}
208-
209-
ns, name := parseSecretString(secretString)
210-
if ns == "" {
211-
ns = svc.Namespace
212-
}
199+
func (cp *CloudProvider) readSSLSecret(ns, name string) (*certificateData, error) {
213200
secret, err := cp.kubeclient.CoreV1().Secrets(ns).Get(name, metav1.GetOptions{})
214201
if err != nil {
215202
return nil, err
216203
}
217-
204+
var ok bool
218205
var cacert, cert, key, pass []byte
219-
var cacertstr, passstr string
220-
if cacert, ok = secret.Data[SSLCAFileName]; !ok {
221-
cacertstr = ""
222-
} else {
223-
cacertstr = string(cacert)
224-
}
206+
cacert, _ = secret.Data[SSLCAFileName]
225207
if cert, ok = secret.Data[SSLCertificateFileName]; !ok {
226208
return nil, errors.Errorf("%s not found in secret %s/%s", SSLCertificateFileName, ns, name)
227209
}
228210
if key, ok = secret.Data[SSLPrivateKeyFileName]; !ok {
229211
return nil, errors.Errorf("%s not found in secret %s/%s", SSLPrivateKeyFileName, ns, name)
230212
}
231-
if pass, ok = secret.Data[SSLPassphrase]; !ok {
232-
passstr = ""
233-
} else {
234-
passstr = string(pass)
235-
}
236-
return &certificateData{CACert: cacertstr, PublicCert: string(cert), PrivateKey: string(key),
237-
Passphrase: passstr}, nil
213+
pass, _ = secret.Data[SSLPassphrase]
214+
return &certificateData{CACert: cacert, PublicCert: cert, PrivateKey: key, Passphrase: pass}, nil
238215
}
239216

240217
// ensureSSLCertificate creates a OCI SSL certificate to the given load
241218
// balancer, if it doesn't already exist.
242-
func (cp *CloudProvider) ensureSSLCertificate(ctx context.Context, lb *loadbalancer.LoadBalancer, sslConfig *SSLConfig, svc *v1.Service) error {
243-
name := sslConfig.Name
244-
logger := cp.logger.With("loadBalancerID", *lb.Id, "certificateName", name)
245-
_, err := cp.client.LoadBalancer().GetCertificateByName(ctx, *lb.Id, name)
246-
if err == nil {
247-
logger.Debug("Certificate already exists on load balancer. Nothing to do.")
248-
return nil
249-
}
250-
if !client.IsNotFound(err) {
251-
return err
252-
}
253-
254-
// Although we iterate here only one certificate is supported at the moment.
255-
certs := make(map[string]loadbalancer.CertificateDetails)
256-
err = buildCertificates(sslConfig, svc, certs)
219+
func (cp *CloudProvider) ensureSSLCertificates(ctx context.Context, lb *loadbalancer.LoadBalancer, spec *LBSpec) error {
220+
logger := cp.logger.With("loadBalancerID", *lb.Id)
221+
// Get all required certificates
222+
certs, err := spec.Certificates()
257223
if err != nil {
258224
return err
259225
}
226+
227+
var ok bool
260228
for _, cert := range certs {
261-
wrID, err := cp.client.LoadBalancer().CreateCertificate(ctx, *lb.Id, *cert.PublicCertificate, *cert.PrivateKey)
262-
if err != nil {
263-
return err
264-
}
265-
_, err = cp.client.LoadBalancer().AwaitWorkRequest(ctx, wrID)
266-
if err != nil {
267-
return err
268-
}
229+
if _, ok = lb.Certificates[*cert.CertificateName]; !ok {
230+
logger = cp.logger.With("certificateName", *cert.CertificateName)
231+
wrID, err := cp.client.LoadBalancer().CreateCertificate(ctx, *lb.Id, cert)
232+
if err != nil {
233+
return err
234+
}
235+
_, err = cp.client.LoadBalancer().AwaitWorkRequest(ctx, wrID)
236+
if err != nil {
237+
return err
238+
}
269239

270-
logger.Info("Certificate created")
240+
logger.Info("Certificate created")
241+
}
271242
}
272243
return nil
273244
}
@@ -289,12 +260,7 @@ func (cp *CloudProvider) createLoadBalancer(ctx context.Context, spec *LBSpec) (
289260
}
290261

291262
// Then we create the load balancer and wait for it to be online.
292-
certs := make(map[string]loadbalancer.CertificateDetails)
293-
err = buildCertificates(spec.ListenerSSLConfig, spec.service, certs)
294-
if err != nil {
295-
return nil, errors.Wrap(err, "get certificates")
296-
}
297-
err = buildCertificates(spec.BackendSetSSLConfig, spec.service, certs)
263+
certs, err := spec.Certificates()
298264
if err != nil {
299265
return nil, errors.Wrap(err, "get certificates")
300266
}
@@ -351,17 +317,18 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
351317
}
352318
exists := !client.IsNotFound(err)
353319

354-
var sslListener, sslBackendSet *SSLConfig
320+
var sslConfig *SSLConfig
355321
if requiresCertificate(service) {
356322
ports, err := getSSLEnabledPorts(service)
357323
if err != nil {
358324
return nil, err
359325
}
360-
sslListener = NewSSLConfig(lbName, ServiceAnnotationLoadBalancerTLSSecret, ports, cp)
361-
sslBackendSet = NewSSLConfig(lbName, ServiceAnnotationLoadBalancerBackendSetSecret, ports, cp)
326+
secretListenerString, _ := service.Annotations[ServiceAnnotationLoadBalancerTLSSecret]
327+
secretBackendSetString, _ := service.Annotations[ServiceAnnotationLoadBalancerBackendSetSecret]
328+
sslConfig = NewSSLConfig(secretListenerString, secretBackendSetString, ports, cp)
362329
}
363330
subnets := []string{cp.config.LoadBalancer.Subnet1, cp.config.LoadBalancer.Subnet2}
364-
spec, err := NewLBSpec(service, nodes, subnets, sslListener, sslBackendSet, cp.securityListManagerFactory)
331+
spec, err := NewLBSpec(service, nodes, subnets, sslConfig, cp.securityListManagerFactory)
365332
if err != nil {
366333
logger.With(zap.Error(err)).Error("Failed to derive LBSpec")
367334
return nil, err
@@ -380,11 +347,8 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
380347

381348
// If the load balancer needs an SSL cert ensure it is present.
382349
if requiresCertificate(service) {
383-
if err := cp.ensureSSLCertificate(ctx, lb, spec.ListenerSSLConfig, spec.service); err != nil {
384-
return nil, errors.Wrap(err, "ensuring ssl certificate for listeners")
385-
}
386-
if err := cp.ensureSSLCertificate(ctx, lb, spec.BackendSetSSLConfig, spec.service); err != nil {
387-
return nil, errors.Wrap(err, "ensuring ssl certificate for backend sets")
350+
if err := cp.ensureSSLCertificates(ctx, lb, spec); err != nil {
351+
return nil, errors.Wrap(err, "ensuring ssl certificates")
388352
}
389353
}
390354

‎pkg/oci/load_balancer_spec.go

+65-44
Original file line numberDiff line numberDiff line change
@@ -33,29 +33,28 @@ import (
3333
// certificateData is a structure containing the data about a K8S secret required
3434
// to store SSL information required for BackendSets and Listeners
3535
type certificateData struct {
36-
CACert string
37-
PublicCert string
38-
PrivateKey string
39-
Passphrase string
36+
Name string
37+
CACert []byte
38+
PublicCert []byte
39+
PrivateKey []byte
40+
Passphrase []byte
4041
}
4142

4243
type sslSecretReader interface {
43-
readSSLSecret(secretType string, svc *v1.Service) (sslSecret *certificateData, err error)
44+
readSSLSecret(ns, name string) (sslSecret *certificateData, err error)
4445
}
4546

4647
type noopSSLSecretReader struct{}
4748

48-
func (ssr noopSSLSecretReader) readSSLSecret(secretType string, svc *v1.Service) (sslSecret *certificateData, err error) {
49-
return &certificateData{}, nil
49+
func (ssr noopSSLSecretReader) readSSLSecret(ns, name string) (sslSecret *certificateData, err error) {
50+
return nil, nil
5051
}
5152

5253
// SSLConfig is a description of a SSL certificate.
5354
type SSLConfig struct {
54-
Name string
55-
Type string
56-
Ports sets.Int
57-
ListenerSSLSecret *certificateData
58-
BackendSetSSLSecret *certificateData
55+
Ports sets.Int
56+
ListenerSSLSecretName string
57+
BackendSetSSLSecretName string
5958

6059
sslSecretReader
6160
}
@@ -66,15 +65,15 @@ func requiresCertificate(svc *v1.Service) bool {
6665
}
6766

6867
// NewSSLConfig constructs a new SSLConfig.
69-
func NewSSLConfig(name string, sslType string, ports []int, ssr sslSecretReader) *SSLConfig {
68+
func NewSSLConfig(listenerSecretName, backendSetSecretName string, ports []int, ssr sslSecretReader) *SSLConfig {
7069
if ssr == nil {
7170
ssr = noopSSLSecretReader{}
7271
}
7372
return &SSLConfig{
74-
Name: name,
75-
Type: sslType,
76-
Ports: sets.NewInt(ports...),
77-
sslSecretReader: ssr,
73+
Ports: sets.NewInt(ports...),
74+
ListenerSSLSecretName: listenerSecretName,
75+
BackendSetSSLSecretName: backendSetSecretName,
76+
sslSecretReader: ssr,
7877
}
7978
}
8079

@@ -90,16 +89,15 @@ type LBSpec struct {
9089

9190
Ports map[string]portSpec
9291
SourceCIDRs []string
93-
ListenerSSLConfig *SSLConfig
94-
BackendSetSSLConfig *SSLConfig
92+
SSLConfig *SSLConfig
9593
securityListManager securityListManager
9694

9795
service *v1.Service
9896
nodes []*v1.Node
9997
}
10098

10199
// NewLBSpec creates a LB Spec from a Kubernetes service and a slice of nodes.
102-
func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, listenerSSLConfig *SSLConfig, backendSetSSLConfig *SSLConfig, secListFactory securityListManagerFactory) (*LBSpec, error) {
100+
func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, sslConfig *SSLConfig, secListFactory securityListManagerFactory) (*LBSpec, error) {
103101
if len(defaultSubnets) != 2 {
104102
return nil, errors.New("default subnets incorrectly configured")
105103
}
@@ -145,7 +143,7 @@ func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, liste
145143
}
146144
}
147145

148-
listeners, err := getListeners(svc, listenerSSLConfig)
146+
listeners, err := getListeners(svc, sslConfig)
149147
if err != nil {
150148
return nil, err
151149
}
@@ -156,12 +154,11 @@ func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, liste
156154
Internal: internal,
157155
Subnets: subnets,
158156
Listeners: listeners,
159-
BackendSets: getBackendSets(svc, nodes, backendSetSSLConfig),
157+
BackendSets: getBackendSets(svc, nodes, sslConfig),
160158

161-
Ports: getPorts(svc),
162-
ListenerSSLConfig: listenerSSLConfig,
163-
BackendSetSSLConfig: backendSetSSLConfig,
164-
SourceCIDRs: sourceCIDRs,
159+
Ports: getPorts(svc),
160+
SSLConfig: sslConfig,
161+
SourceCIDRs: sourceCIDRs,
165162

166163
service: svc,
167164
nodes: nodes,
@@ -171,26 +168,42 @@ func NewLBSpec(svc *v1.Service, nodes []*v1.Node, defaultSubnets []string, liste
171168
}
172169

173170
// Certificates builds a map of required SSL certificates.
174-
func buildCertificates(sslConfig *SSLConfig, svc *v1.Service, certs map[string]loadbalancer.CertificateDetails) error {
175-
if sslConfig == nil {
176-
return nil
171+
func (s *LBSpec) Certificates() (map[string]loadbalancer.CertificateDetails, error) {
172+
certs := make(map[string]loadbalancer.CertificateDetails)
173+
if s.SSLConfig == nil {
174+
return certs, nil
177175
}
178-
sslSecret, err := sslConfig.readSSLSecret(sslConfig.Type, svc)
176+
//Read listener Kubernetes Secret
177+
sslSecret, err := s.SSLConfig.readSSLSecret(s.service.Namespace, s.SSLConfig.ListenerSSLSecretName)
179178
if err != nil {
180-
return errors.Wrap(err, "reading SSL Secret")
179+
return nil, errors.Wrap(err, "reading SSL Listener Secret")
181180
}
182-
if sslSecret.PublicCert == "" || sslSecret.PrivateKey == "" {
183-
return nil
181+
if len(sslSecret.PublicCert) == 0 || len(sslSecret.PrivateKey) == 0 {
182+
return certs, nil
184183
}
185184

186-
certs[sslConfig.Name] = loadbalancer.CertificateDetails{
187-
CertificateName: &sslConfig.Name,
188-
PublicCertificate: &sslSecret.PublicCert,
189-
CaCertificate: &sslSecret.CACert,
190-
PrivateKey: &sslSecret.PrivateKey,
191-
Passphrase: &sslSecret.Passphrase,
185+
certs[s.SSLConfig.ListenerSSLSecretName] = loadbalancer.CertificateDetails{
186+
CertificateName: &s.SSLConfig.ListenerSSLSecretName,
187+
PublicCertificate: common.String(string(sslSecret.PublicCert)),
188+
PrivateKey: common.String(string(sslSecret.PrivateKey)),
192189
}
193-
return nil
190+
// Read backendSet Kubernetes Secret
191+
sslSecret, err = s.SSLConfig.readSSLSecret(s.service.Namespace, s.SSLConfig.BackendSetSSLSecretName)
192+
if err != nil {
193+
return nil, errors.Wrap(err, "reading SSL BackendSet Secret")
194+
}
195+
if len(sslSecret.PublicCert) == 0 || len(sslSecret.PrivateKey) == 0 {
196+
return certs, nil
197+
}
198+
199+
certs[s.SSLConfig.BackendSetSSLSecretName] = loadbalancer.CertificateDetails{
200+
CertificateName: &s.SSLConfig.BackendSetSSLSecretName,
201+
CaCertificate: common.String(string(sslSecret.CACert)),
202+
PublicCertificate: common.String(string(sslSecret.PublicCert)),
203+
PrivateKey: common.String(string(sslSecret.PrivateKey)),
204+
Passphrase: common.String(string(sslSecret.Passphrase)),
205+
}
206+
return certs, nil
194207
}
195208

196209
// TODO(apryde): aggregate errors using an error list.
@@ -263,7 +276,11 @@ func getBackendSets(svc *v1.Service, nodes []*v1.Node, sslCfg *SSLConfig) map[st
263276
for _, servicePort := range svc.Spec.Ports {
264277
name := getBackendSetName(string(servicePort.Protocol), int(servicePort.Port))
265278
port := int(servicePort.Port)
266-
sslConfig := getSSLConfiguration(sslCfg, port)
279+
var secretName string
280+
if sslCfg != nil {
281+
secretName = sslCfg.BackendSetSSLSecretName
282+
}
283+
sslConfig := getSSLConfiguration(sslCfg, secretName, port)
267284
if sslConfig != nil {
268285
backendSets[name] = loadbalancer.BackendSetDetails{
269286
Policy: common.String(DefaultLoadBalancerPolicy),
@@ -304,12 +321,12 @@ func getHealthChecker(cfg *SSLConfig, port int, svc *v1.Service) *loadbalancer.H
304321
}
305322
}
306323

307-
func getSSLConfiguration(cfg *SSLConfig, port int) *loadbalancer.SslConfigurationDetails {
324+
func getSSLConfiguration(cfg *SSLConfig, name string, port int) *loadbalancer.SslConfigurationDetails {
308325
if cfg == nil || !cfg.Ports.Has(port) {
309326
return nil
310327
}
311328
return &loadbalancer.SslConfigurationDetails{
312-
CertificateName: &cfg.Name,
329+
CertificateName: &name,
313330
VerifyDepth: common.Int(0),
314331
VerifyPeerCertificate: common.Bool(false),
315332
}
@@ -347,7 +364,11 @@ func getListeners(svc *v1.Service, sslCfg *SSLConfig) (map[string]loadbalancer.L
347364
}
348365
}
349366
port := int(servicePort.Port)
350-
sslConfiguration := getSSLConfiguration(sslCfg, port)
367+
var secretName string
368+
if sslCfg != nil {
369+
secretName = sslCfg.ListenerSSLSecretName
370+
}
371+
sslConfiguration := getSSLConfiguration(sslCfg, secretName, port)
351372
name := getListenerName(protocol, port, sslConfiguration)
352373

353374
listener := loadbalancer.ListenerDetails{

0 commit comments

Comments
 (0)