[GitHub] [apisix-ingress-controller] lingsamuel commented on a change in pull request #685: feat: support cert-manager

[GitBox <gi...@apache.org>]

lingsamuel commented on a change in pull request #685:
URL: https://github.com/apache/apisix-ingress-controller/pull/685#discussion_r710728647



##########
File path: pkg/ingress/secret.go
##########
@@ -143,54 +142,58 @@ func (c *secretController) sync(ctx context.Context, ev *types.Event) error {
 		}
 		tls, err := c.controller.apisixTlsLister.ApisixTlses(tlsNamespace).Get(tlsName)
 		if err != nil {
-			log.Warnw("secret related ApisixTls resource not found, skip",
+			log.Debugw("secret related ApisixTls resource not found, skip",
 				zap.String("ApisixTls", tlsMetaKey),
 			)
 			return true
 		}
+
+		// We don't expect a secret to be used as both SSL and mTLS in ApisixTls
 		if tls.Spec.Secret.Namespace == sec.Namespace && tls.Spec.Secret.Name == sec.Name {
-			cert, ok := sec.Data["cert"]
-			if !ok {
-				log.Warnw("secret required by ApisixTls invalid",
-					zap.String("ApisixTls", tlsMetaKey),
-					zap.Error(translation.ErrEmptyCert),
-				)
-				return true
-			}
-			pkey, ok := sec.Data["key"]
-			if !ok {
-				log.Warnw("secret required by ApisixTls invalid",
+			cert, pkey, err := c.controller.translator.ExtractKeyPair(sec, true)
+			if err != nil {
+				log.Errorw("secret required by ApisixTls invalid",
 					zap.String("ApisixTls", tlsMetaKey),
-					zap.Error(translation.ErrEmptyPrivKey),
+					zap.Error(err),
 				)
+				go func(tls *configv1.ApisixTls) {
+					c.controller.recorderEventS(tls, corev1.EventTypeWarning, _resourceSyncAborted,
+						fmt.Sprintf("sync from secret %s changes failed, error: %s", key, err.Error()))
+					c.controller.recordStatus(tls, _resourceSyncAborted, err, metav1.ConditionFalse)
+				}(tls)
 				return true
 			}
 			// sync ssl
 			ssl.Cert = string(cert)
 			ssl.Key = string(pkey)
 		} else if tls.Spec.Client != nil &&
 			tls.Spec.Client.CASecret.Namespace == sec.Namespace && tls.Spec.Client.CASecret.Name == sec.Name {
-			ca, ok := sec.Data["cert"]
-			if !ok {
-				log.Warnw("secret required by ApisixTls invalid",
-					zap.String("resource", tlsMetaKey),
-					zap.Error(translation.ErrEmptyCert),
+			ca, _, err := c.controller.translator.ExtractKeyPair(sec, false)
+			if err != nil {
+				log.Errorw("ca secret required by ApisixTls invalid",
+					zap.String("ApisixTls", tlsMetaKey),
+					zap.Error(err),
 				)
+				go func(tls *configv1.ApisixTls) {
+					c.controller.recorderEventS(tls, corev1.EventTypeWarning, _resourceSyncAborted,
+						fmt.Sprintf("sync from ca secret %s changes failed, error: %s", key, err.Error()))
+					c.controller.recordStatus(tls, _resourceSyncAborted, err, metav1.ConditionFalse)
+				}(tls)
 				return true
 			}
 			ssl.Client = &apisixv1.MutualTLSClientConfig{
 				CA: string(ca),
 			}
 		} else {
-			log.Warnw("stale secret cache, ApisixTls doesn't requires target secret",
+			log.Infow("stale secret cache, ApisixTls doesn't requires target secret",
 				zap.String("ApisixTls", tlsMetaKey),
 				zap.String("secret", key),
 			)
 			return true
 		}
 		// Use another goroutine to send requests, to avoid
 		// long time lock occupying.
-		go func(ssl *apisixv1.Ssl) {
+		go func(ssl *apisixv1.Ssl, tls *configv1.ApisixTls) {

Review comment:
       How long is reasonable?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org