Third round of comments

Author: MadalinaPatrichi

pkg/oci/load_balancer.go

@@ -203,14 +203,14 @@ func (cp *CloudProvider) readSSLSecret(ns, name string) (*certificateData, error
 	}
 	var ok bool
 	var cacert, cert, key, pass []byte
-	cacert, _ = secret.Data[SSLCAFileName]
+	cacert = secret.Data[SSLCAFileName]
 	if cert, ok = secret.Data[SSLCertificateFileName]; !ok {
 		return nil, errors.Errorf("%s not found in secret %s/%s", SSLCertificateFileName, ns, name)
 	}
 	if key, ok = secret.Data[SSLPrivateKeyFileName]; !ok {
 		return nil, errors.Errorf("%s not found in secret %s/%s", SSLPrivateKeyFileName, ns, name)
 	}
-	pass, _ = secret.Data[SSLPassphrase]
+	pass = secret.Data[SSLPassphrase]
 	return &certificateData{CACert: cacert, PublicCert: cert, PrivateKey: key, Passphrase: pass}, nil
 }
 
@@ -323,8 +323,8 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
 		if err != nil {
 			return nil, err
 		}
-		secretListenerString, _ := service.Annotations[ServiceAnnotationLoadBalancerTLSSecret]
-		secretBackendSetString, _ := service.Annotations[ServiceAnnotationLoadBalancerBackendSetSecret]
+		secretListenerString := service.Annotations[ServiceAnnotationLoadBalancerTLSSecret]
+		secretBackendSetString := service.Annotations[ServiceAnnotationLoadBalancerBackendSetSecret]
 		sslConfig = NewSSLConfig(secretListenerString, secretBackendSetString, ports, cp)
 	}
 	subnets := []string{cp.config.LoadBalancer.Subnet1, cp.config.LoadBalancer.Subnet2}

pkg/oci/load_balancer_spec.go

@@ -173,35 +173,27 @@ func (s *LBSpec) Certificates() (map[string]loadbalancer.CertificateDetails, err
 	if s.SSLConfig == nil {
 		return certs, nil
 	}
-	//Read listener Kubernetes Secret
-	sslSecret, err := s.SSLConfig.readSSLSecret(s.service.Namespace, s.SSLConfig.ListenerSSLSecretName)
-	if err != nil {
-		return nil, errors.Wrap(err, "reading SSL Listener Secret")
+	secrets := make([]string, 0, 2)
+	if s.SSLConfig.ListenerSSLSecretName != "" {
+		secrets = append(secrets, s.SSLConfig.ListenerSSLSecretName)
 	}
-	if len(sslSecret.PublicCert) == 0 || len(sslSecret.PrivateKey) == 0 {
-		return certs, nil
+	if s.SSLConfig.BackendSetSSLSecretName != "" {
+		secrets = append(secrets, s.SSLConfig.BackendSetSSLSecretName)
 	}
 
-	certs[s.SSLConfig.ListenerSSLSecretName] = loadbalancer.CertificateDetails{
-		CertificateName:   &s.SSLConfig.ListenerSSLSecretName,
-		PublicCertificate: common.String(string(sslSecret.PublicCert)),
-		PrivateKey:        common.String(string(sslSecret.PrivateKey)),
-	}
-	// Read backendSet Kubernetes Secret
-	sslSecret, err = s.SSLConfig.readSSLSecret(s.service.Namespace, s.SSLConfig.BackendSetSSLSecretName)
-	if err != nil {
-		return nil, errors.Wrap(err, "reading SSL BackendSet Secret")
-	}
-	if len(sslSecret.PublicCert) == 0 || len(sslSecret.PrivateKey) == 0 {
-		return certs, nil
-	}
+	for _, name := range secrets {
+		cert, err := s.SSLConfig.readSSLSecret(s.service.Namespace, name)
+		if err != nil {
+			return nil, errors.Wrap(err, "reading SSL BackendSet Secret")
+		}
 
-	certs[s.SSLConfig.BackendSetSSLSecretName] = loadbalancer.CertificateDetails{
-		CertificateName:   &s.SSLConfig.BackendSetSSLSecretName,
-		CaCertificate:     common.String(string(sslSecret.CACert)),
-		PublicCertificate: common.String(string(sslSecret.PublicCert)),
-		PrivateKey:        common.String(string(sslSecret.PrivateKey)),
-		Passphrase:        common.String(string(sslSecret.Passphrase)),
+		certs[name] = loadbalancer.CertificateDetails{
+			CertificateName:   &name,
+			CaCertificate:     common.String(string(cert.CACert)),
+			PublicCertificate: common.String(string(cert.PublicCert)),
+			PrivateKey:        common.String(string(cert.PrivateKey)),
+			Passphrase:        common.String(string(cert.Passphrase)),
+		}
 	}
 	return certs, nil
 }
@@ -277,23 +269,14 @@ func getBackendSets(svc *v1.Service, nodes []*v1.Node, sslCfg *SSLConfig) map[st
 		name := getBackendSetName(string(servicePort.Protocol), int(servicePort.Port))
 		port := int(servicePort.Port)
 		var secretName string
-		if sslCfg != nil {
+		if sslCfg != nil && len(sslCfg.BackendSetSSLSecretName) !=0 {
 			secretName = sslCfg.BackendSetSSLSecretName
 		}
-		sslConfig := getSSLConfiguration(sslCfg, secretName, port)
-		if sslConfig != nil {
-			backendSets[name] = loadbalancer.BackendSetDetails{
-				Policy:           common.String(DefaultLoadBalancerPolicy),
-				Backends:         getBackends(nodes, servicePort.NodePort),
-				HealthChecker:    getHealthChecker(sslCfg, port, svc),
-				SslConfiguration: sslConfig,
-			}
-		} else {
-			backendSets[name] = loadbalancer.BackendSetDetails{
-				Policy:        common.String(DefaultLoadBalancerPolicy),
-				Backends:      getBackends(nodes, servicePort.NodePort),
-				HealthChecker: getHealthChecker(sslCfg, port, svc),
-			}
+		backendSets[name] = loadbalancer.BackendSetDetails{
+			Policy:           common.String(DefaultLoadBalancerPolicy),
+			Backends:         getBackends(nodes, servicePort.NodePort),
+			HealthChecker:    getHealthChecker(sslCfg, port, svc),
+			SslConfiguration: getSSLConfiguration(sslCfg, secretName, port),
 		}
 	}
 	return backendSets
@@ -322,7 +305,7 @@ func getHealthChecker(cfg *SSLConfig, port int, svc *v1.Service) *loadbalancer.H
 }
 
 func getSSLConfiguration(cfg *SSLConfig, name string, port int) *loadbalancer.SslConfigurationDetails {
-	if cfg == nil || !cfg.Ports.Has(port) {
+	if cfg == nil || !cfg.Ports.Has(port) || len(name) ==0 {
 		return nil
 	}
 	return &loadbalancer.SslConfigurationDetails{
@@ -365,7 +348,7 @@ func getListeners(svc *v1.Service, sslCfg *SSLConfig) (map[string]loadbalancer.L
 		}
 		port := int(servicePort.Port)
 		var secretName string
-		if sslCfg != nil {
+		if sslCfg != nil && len(sslCfg.ListenerSSLSecretName) !=0 {
 			secretName = sslCfg.ListenerSSLSecretName
 		}
 		sslConfiguration := getSSLConfiguration(sslCfg, secretName, port)

test/e2e/framework/networking_util.go

@@ -104,8 +104,7 @@ func TestReachableHTTPWithContentTimeout(secure bool, ip string, port int, reque
 func TestReachableHTTPWithContentTimeoutWithRetriableErrorCodes(secure bool, ip string, port int, request string, expect string, content *bytes.Buffer, retriableErrCodes []int, timeout time.Duration) (bool, error) {
 
 	ipPort := net.JoinHostPort(ip, strconv.Itoa(port))
-	var url string
-	url = fmt.Sprintf("http://%s%s", ipPort, request)
+	url := fmt.Sprintf("http://%s%s", ipPort, request)
 	if secure {
 		url = fmt.Sprintf("https://%s%s", ipPort, request)
 	}