You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by oc...@apache.org on 2021/04/23 06:34:07 UTC

[trafficcontrol] branch master updated: Acme async (#5738)

This is an automated email from the ASF dual-hosted git repository.

ocket8888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new ee41354  Acme async (#5738)
ee41354 is described below

commit ee413544850bd549baa9a7ae990872be5c61a85e
Author: mattjackson220 <33...@users.noreply.github.com>
AuthorDate: Fri Apr 23 00:33:48 2021 -0600

    Acme async (#5738)
    
    * Added async status to ACME generation
    
    * updated changelog
    
    * updated to require authtype in acme request and to recover from panic
    
    * fixed tabs/spaces per comment
    
    * fixed recovery and auth type
    
    * fixed errors
    
    * updated per comments
---
 CHANGELOG.md                                       |   1 +
 ...liveryservices_sslkeys_generate_letsencrypt.rst |   2 +-
 ...liveryservices_sslkeys_generate_letsencrypt.rst |   2 +-
 ...liveryservices_sslkeys_generate_letsencrypt.rst |   2 +-
 .../v4/deliveryservices_sslkeys_generate_acme.rst  |   4 +-
 ...liveryservices_sslkeys_generate_letsencrypt.rst |   2 +-
 lib/go-tc/deliveryservice_ssl_keys.go              |  16 ++-
 traffic_ops/traffic_ops_golang/api/async_status.go |   3 +
 .../traffic_ops_golang/deliveryservice/acme.go     | 147 ++++++++++++++++-----
 .../deliveryservice/autorenewcerts.go              |  24 ++--
 .../common/api/DeliveryServiceSslKeysService.js    |  88 ++++++------
 ...FormGenerateDeliveryServiceSslKeysController.js |   2 +-
 12 files changed, 197 insertions(+), 96 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 95ce6db..a0c2a40 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - ORT config generation: Added a rule to ip_allow such that PURGE requests are allowed over localhost
 - Added integration to use ACME to generate new SSL certificates.
 - Add a Federation to the Ansible Dataset Loader
+- Added asynchronous status to ACME certificate generation.
 
 ### Fixed
 - [#5690](https://github.com/apache/trafficcontrol/issues/5690) - Fixed github action for added/modified db migration file.
diff --git a/docs/source/api/v1/deliveryservices_sslkeys_generate_letsencrypt.rst b/docs/source/api/v1/deliveryservices_sslkeys_generate_letsencrypt.rst
index c73c40c..bdf923f 100644
--- a/docs/source/api/v1/deliveryservices_sslkeys_generate_letsencrypt.rst
+++ b/docs/source/api/v1/deliveryservices_sslkeys_generate_letsencrypt.rst
@@ -62,7 +62,7 @@ Response Structure
 
 	{ "alerts": [{
 		"level": "success",
-		"text": "Beginning async call to Let's Encrypt for ds-01. This may take a few minutes."
+		"text": "Beginning async ACME call for demo1 using Lets Encrypt. This may take a few minutes. Status updates can be found here: /api/4.0/async_status/1"
 	}]}
 
 .. [#needOne] Either the ``key`` or the ``deliveryservice`` field must be provided. If both are provided, then they must match.
diff --git a/docs/source/api/v2/deliveryservices_sslkeys_generate_letsencrypt.rst b/docs/source/api/v2/deliveryservices_sslkeys_generate_letsencrypt.rst
index 42dc4b9..54c5288 100644
--- a/docs/source/api/v2/deliveryservices_sslkeys_generate_letsencrypt.rst
+++ b/docs/source/api/v2/deliveryservices_sslkeys_generate_letsencrypt.rst
@@ -60,7 +60,7 @@ Response Structure
 
 	{ "alerts": [{
 		"level": "success",
-		"text": "Beginning async call to Let's Encrypt for ds-01. This may take a few minutes."
+		"text": "Beginning async ACME call for demo1 using Lets Encrypt. This may take a few minutes. Status updates can be found here: /api/4.0/async_status/1"
 	}]}
 
 .. [#needOne] Either the ``key`` or the ``deliveryservice`` field must be provided. If both are provided, then they must match.
diff --git a/docs/source/api/v3/deliveryservices_sslkeys_generate_letsencrypt.rst b/docs/source/api/v3/deliveryservices_sslkeys_generate_letsencrypt.rst
index 9f35f8d..c1bd2ed 100644
--- a/docs/source/api/v3/deliveryservices_sslkeys_generate_letsencrypt.rst
+++ b/docs/source/api/v3/deliveryservices_sslkeys_generate_letsencrypt.rst
@@ -60,7 +60,7 @@ Response Structure
 
 	{ "alerts": [{
 		"level": "success",
-		"text": "Beginning async call to Let's Encrypt for ds-01. This may take a few minutes."
+		"text": "Beginning async ACME call for demo1 using Lets Encrypt. This may take a few minutes. Status updates can be found here: /api/4.0/async_status/1"
 	}]}
 
 .. [#needOne] Either the ``key`` or the ``deliveryservice`` field must be provided. If both are provided, then they must match.
diff --git a/docs/source/api/v4/deliveryservices_sslkeys_generate_acme.rst b/docs/source/api/v4/deliveryservices_sslkeys_generate_acme.rst
index fac9a04..1df09ae 100644
--- a/docs/source/api/v4/deliveryservices_sslkeys_generate_acme.rst
+++ b/docs/source/api/v4/deliveryservices_sslkeys_generate_acme.rst
@@ -48,7 +48,7 @@ Request Structure
 	Content-Type: application/json
 
 	{
-		"authType": "Let's Encrypt",
+		"authType": "Lets Encrypt",
 		"key": "ds-01",
 		"deliveryservice": "ds-01",
 		"version": "3",
@@ -64,7 +64,7 @@ Response Structure
 
 	{ "alerts": [{
 		"level": "success",
-		"text": "Beginning async ACME call for ds-01 using Let's Encrypt. This may take a few minutes."
+		"text": "Beginning async ACME call for demo1 using Lets Encrypt. This may take a few minutes. Status updates can be found here: /api/4.0/async_status/1"
 	}]}
 
 .. [#needOne] Either the ``key`` or the ``deliveryservice`` field must be provided. If both are provided, then they must match.
diff --git a/docs/source/api/v4/deliveryservices_sslkeys_generate_letsencrypt.rst b/docs/source/api/v4/deliveryservices_sslkeys_generate_letsencrypt.rst
index dbcba6c..d1f1294 100644
--- a/docs/source/api/v4/deliveryservices_sslkeys_generate_letsencrypt.rst
+++ b/docs/source/api/v4/deliveryservices_sslkeys_generate_letsencrypt.rst
@@ -65,7 +65,7 @@ Response Structure
 		"text": "This endpoint is deprecated, please use /deliveryservices/sslkeys/generate/acme instead."
 	},{
 		"level": "success",
-		"text": "Beginning async call to Let's Encrypt for ds-01. This may take a few minutes."
+		"text": "Beginning async ACME call for demo1 using Lets Encrypt. This may take a few minutes. Status updates can be found here: /api/4.0/async_status/1"
 	}]}
 
 .. [#needOne] Either the ``key`` or the ``deliveryservice`` field must be provided. If both are provided, then they must match.
diff --git a/lib/go-tc/deliveryservice_ssl_keys.go b/lib/go-tc/deliveryservice_ssl_keys.go
index be91837..b759eda 100644
--- a/lib/go-tc/deliveryservice_ssl_keys.go
+++ b/lib/go-tc/deliveryservice_ssl_keys.go
@@ -186,19 +186,31 @@ func (r *DeliveryServiceGenSSLKeysReq) Validate(tx *sql.Tx) error {
 	return nil
 }
 
-type DeliveryServiceLetsEncryptSSLKeysReq struct {
+type DeliveryServiceAcmeSSLKeysReq struct {
 	DeliveryServiceSSLKeysReq
 }
 
-func (r *DeliveryServiceLetsEncryptSSLKeysReq) Validate(tx *sql.Tx) error {
+func (r *DeliveryServiceAcmeSSLKeysReq) Validate(tx *sql.Tx) error {
 	r.Sanitize()
 	errs := r.validateSharedRequiredRequestFields()
 	if len(errs) > 0 {
 		return errors.New("missing fields: " + strings.Join(errs, "; "))
 	}
+	errs = r.validateAcmeSpecificFields()
+	if len(errs) > 0 {
+		return errors.New("missing fields: " + strings.Join(errs, "; "))
+	}
 	return nil
 }
 
+func (r *DeliveryServiceAcmeSSLKeysReq) validateAcmeSpecificFields() []string {
+	errs := []string{}
+	if checkNilOrEmpty(r.AuthType) {
+		errs = append(errs, "authType required")
+	}
+	return errs
+}
+
 func checkNilOrEmpty(s *string) bool {
 	return s == nil || *s == ""
 }
diff --git a/traffic_ops/traffic_ops_golang/api/async_status.go b/traffic_ops/traffic_ops_golang/api/async_status.go
index eb55879..7a15df2 100644
--- a/traffic_ops/traffic_ops_golang/api/async_status.go
+++ b/traffic_ops/traffic_ops_golang/api/async_status.go
@@ -117,6 +117,9 @@ func InsertAsyncStatus(tx *sql.Tx, message string) (int, int, error, error) {
 
 // UpdateAsyncStatus updates the status table for an asynchronous job.
 func UpdateAsyncStatus(db *sqlx.DB, newStatus string, newMessage string, asyncStatusId int, finished bool) error {
+	if asyncStatusId == 0 {
+		return nil
+	}
 	tx, err := db.Begin()
 	if err != nil {
 		return err
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/acme.go b/traffic_ops/traffic_ops_golang/deliveryservice/acme.go
index 4fee166..ce63298 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/acme.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/acme.go
@@ -29,6 +29,7 @@ import (
 	"database/sql"
 	"encoding/pem"
 	"errors"
+	"fmt"
 	"net/http"
 	"strconv"
 	"strings"
@@ -104,12 +105,12 @@ func (d *DNSProviderTrafficRouter) Present(domain, token, keyAuth string) error
 	tx.Commit()
 	if err != nil {
 		log.Errorf("Inserting dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
-		return errors.New("Inserting dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
+		return fmt.Errorf("Inserting dns txt record for fqdn '"+fqdn+"' record '"+value+"': %v", err)
 	} else {
 		rows, err := response.RowsAffected()
 		if err != nil {
 			log.Errorf("Determining rows affected dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
-			return errors.New("Determining rows affected dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
+			return fmt.Errorf("Determining rows affected dns txt record for fqdn '"+fqdn+"' record '"+value+"': %v", err)
 		}
 		if rows == 0 {
 			log.Errorf("Zero rows affected when inserting dns txt record for fqdn '" + fqdn + "' record '" + value)
@@ -130,12 +131,12 @@ func (d *DNSProviderTrafficRouter) CleanUp(domain, token, keyAuth string) error
 	tx.Commit()
 	if err != nil {
 		log.Errorf("Deleting dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
-		return errors.New("Deleting dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
+		return fmt.Errorf("Deleting dns txt record for fqdn '"+fqdn+"' record '"+value+"': %v", err)
 	} else {
 		rows, err := response.RowsAffected()
 		if err != nil {
 			log.Errorf("Determining rows affected when deleting dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
-			return errors.New("Determining rows affected when deleting dns txt record for fqdn '" + fqdn + "' record '" + value + "': " + err.Error())
+			return fmt.Errorf("Determining rows affected when deleting dns txt record for fqdn '"+fqdn+"' record '"+value+"': %v", err)
 		}
 		if rows == 0 {
 			log.Errorf("Zero rows affected when deleting dns txt record for fqdn '" + fqdn + "' record '" + value)
@@ -160,9 +161,9 @@ func GenerateAcmeCertificates(w http.ResponseWriter, r *http.Request) {
 	}
 	ctx, _ := context.WithTimeout(r.Context(), AcmeTimeout)
 
-	req := tc.DeliveryServiceLetsEncryptSSLKeysReq{}
+	req := tc.DeliveryServiceAcmeSSLKeysReq{}
 	if err := api.Parse(r.Body, nil, &req); err != nil {
-		api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("parsing request: "+err.Error()), nil)
+		api.HandleErr(w, r, nil, http.StatusBadRequest, fmt.Errorf("parsing request: %v", err), nil)
 		return
 	}
 	if *req.DeliveryService == "" {
@@ -171,7 +172,7 @@ func GenerateAcmeCertificates(w http.ResponseWriter, r *http.Request) {
 
 	dsID, cdnName, ok, err := dbhelpers.GetDSIDAndCDNFromName(inf.Tx.Tx, *req.DeliveryService)
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("deliveryservice.GenerateLetsEncryptCertificates: getting DS ID from name "+err.Error()))
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("deliveryservice.GenerateLetsEncryptCertificates: getting DS ID from name: %v", err))
 		return
 	} else if !ok {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("no DS with name "+*req.DeliveryService), nil)
@@ -186,7 +187,7 @@ func GenerateAcmeCertificates(w http.ResponseWriter, r *http.Request) {
 
 	_, ok, err = dbhelpers.GetCDNIDFromName(inf.Tx.Tx, tc.CDNName(*req.CDN))
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking CDN existence: "+err.Error()))
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("checking CDN existence: %v", err))
 		return
 	} else if !ok {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("cdn not found with name "+*req.CDN), nil)
@@ -198,9 +199,21 @@ func GenerateAcmeCertificates(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	go GetAcmeCertificates(inf.Config, req, ctx, inf.User, inf.Vault)
+	asyncStatusId, errCode, userErr, sysErr := api.InsertAsyncStatus(inf.Tx.Tx, "ACME async job has started.")
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+	}
+
+	go GetAcmeCertificates(inf.Config, req, ctx, inf.User, asyncStatusId, inf.Vault)
+
+	var alerts tc.Alerts
+	alerts.AddAlert(tc.Alert{
+		Text:  "Beginning async ACME call for " + *req.DeliveryService + " using " + *req.AuthType + ". This may take a few minutes. Status updates can be found here: " + api.CurrentAsyncEndpoint + strconv.Itoa(asyncStatusId),
+		Level: tc.SuccessLevel.String(),
+	})
 
-	api.WriteRespAlert(w, r, tc.SuccessLevel, "Beginning async ACME call for "+*req.DeliveryService+" using "+*req.AuthType+". This may take a few minutes.")
+	w.Header().Add("Location", api.CurrentAsyncEndpoint+strconv.Itoa(asyncStatusId))
+	api.WriteAlerts(w, r, http.StatusAccepted, alerts)
 }
 
 // GenerateLetsEncryptCertificates gets and saves new certificates from Let's Encrypt.
@@ -219,9 +232,14 @@ func GenerateLetsEncryptCertificates(w http.ResponseWriter, r *http.Request) {
 
 	ctx, _ := context.WithTimeout(r.Context(), AcmeTimeout)
 
-	req := tc.DeliveryServiceLetsEncryptSSLKeysReq{}
+	req := tc.DeliveryServiceAcmeSSLKeysReq{}
+	if req.AuthType == nil {
+		req.AuthType = new(string)
+		*req.AuthType = tc.LetsEncryptAuthType
+	}
+
 	if err := api.Parse(r.Body, nil, &req); err != nil {
-		api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("parsing request: "+err.Error()), nil)
+		api.HandleErr(w, r, nil, http.StatusBadRequest, fmt.Errorf("parsing request: %v", err), nil)
 		return
 	}
 	if *req.DeliveryService == "" {
@@ -230,7 +248,7 @@ func GenerateLetsEncryptCertificates(w http.ResponseWriter, r *http.Request) {
 
 	dsID, cdnName, ok, err := dbhelpers.GetDSIDAndCDNFromName(inf.Tx.Tx, *req.DeliveryService)
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("deliveryservice.GenerateLetsEncryptCertificates: getting DS ID from name "+err.Error()))
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("deliveryservice.GenerateLetsEncryptCertificates: getting DS ID from name: %v", err))
 		return
 	} else if !ok {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("no DS with name "+*req.DeliveryService), nil)
@@ -245,7 +263,7 @@ func GenerateLetsEncryptCertificates(w http.ResponseWriter, r *http.Request) {
 
 	_, ok, err = dbhelpers.GetCDNIDFromName(inf.Tx.Tx, tc.CDNName(*req.CDN))
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking CDN existence: "+err.Error()))
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("checking CDN existence: %v", err))
 		return
 	} else if !ok {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("cdn not found with name "+*req.CDN), nil)
@@ -257,36 +275,65 @@ func GenerateLetsEncryptCertificates(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	go GetAcmeCertificates(inf.Config, req, ctx, inf.User, inf.Vault)
+	asyncStatusId, errCode, userErr, sysErr := api.InsertAsyncStatus(inf.Tx.Tx, "ACME async job has started.")
+	if userErr != nil || sysErr != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+	}
 
-	var alerts tc.Alerts
+	go GetAcmeCertificates(inf.Config, req, ctx, inf.User, asyncStatusId, inf.Vault)
 
+	var alerts tc.Alerts
 	alerts.AddAlerts(api.CreateDeprecationAlerts(util.StrPtr(API_ACME_GENERATE_LE)))
-
 	alerts.AddAlert(tc.Alert{
-		Text:  "Beginning async call to Let's Encrypt for " + *req.DeliveryService + ". This may take a few minutes.",
+		Text:  "Beginning async call to Let's Encrypt for " + *req.DeliveryService + ". This may take a few minutes. Status updates can be found here: " + api.CurrentAsyncEndpoint + strconv.Itoa(asyncStatusId),
 		Level: tc.SuccessLevel.String(),
 	})
 
-	api.WriteAlerts(w, r, http.StatusOK, alerts)
+	w.Header().Add("Location", api.CurrentAsyncEndpoint+strconv.Itoa(asyncStatusId))
+	api.WriteAlerts(w, r, http.StatusAccepted, alerts)
 }
 
 // GetAcmeCertificates gets or creates an ACME account based on the provider, then gets new certificates for the delivery service requested and saves them to Vault.
-func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSSLKeysReq, ctx context.Context, currentUser *auth.CurrentUser, tv trafficvault.TrafficVault) error {
+func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceAcmeSSLKeysReq, ctx context.Context, currentUser *auth.CurrentUser, asyncStatusId int, tv trafficvault.TrafficVault) error {
+	defer func() {
+		if err := recover(); err != nil {
+			db, dbErr := api.GetDB(ctx)
+			if dbErr != nil {
+				log.Errorf(*req.DeliveryService+": Error getting db for recover async update: %s", dbErr.Error())
+				log.Errorf("panic: (err: %v) stacktrace:\n%s\n", err, util.Stacktrace())
+				return
+			}
+
+			if asyncErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asyncErr != nil {
+				log.Errorf("updating async status for id %v: %v", asyncStatusId, asyncErr)
+			}
+			log.Errorf("panic: (err: %v) stacktrace:\n%s\n", err, util.Stacktrace())
+			return
+		}
+	}()
 
 	db, err := api.GetDB(ctx)
 	if err != nil {
 		log.Errorf(*req.DeliveryService+": Error getting db: %s", err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return err
 	}
 	tx, err := db.Begin()
 	if err != nil {
 		log.Errorf(*req.DeliveryService+": Error getting tx: %s", err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return err
 	}
 	userTx, err := db.Begin()
 	if err != nil {
 		log.Errorf(*req.DeliveryService+": Error getting userTx: %s", err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return err
 	}
 	defer userTx.Commit()
@@ -294,6 +341,9 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	logTx, err := db.Begin()
 	if err != nil {
 		log.Errorf(*req.DeliveryService+": Error getting logTx: %s", err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return err
 	}
 	defer logTx.Commit()
@@ -306,10 +356,16 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	if err != nil {
 		log.Errorf("deliveryservice.GenerateSSLKeys: getting DS ID from name " + err.Error())
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
-		return errors.New("deliveryservice.GenerateSSLKeys: getting DS ID from name " + err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
+		return fmt.Errorf("deliveryservice.GenerateSSLKeys: getting DS ID from name: %v", err)
 	} else if !ok {
 		log.Errorf("no DS with name " + *req.DeliveryService)
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return errors.New("no DS with name " + *req.DeliveryService)
 	}
 	tx.Commit()
@@ -317,6 +373,9 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	if cfg == nil {
 		log.Errorf("acme: config was nil for provider %s", provider)
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return errors.New("acme: config was nil")
 	}
 
@@ -336,8 +395,11 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	} else {
 		acmeAccount := GetAcmeAccountConfig(cfg, provider)
 		if acmeAccount == nil {
-			log.Errorf("acme: no account information found for %s" + provider)
+			log.Errorf("acme: no account information found for %s", provider)
 			api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
+			if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+				log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+			}
 			return errors.New("No acme account information in cdn.conf for " + provider)
 		}
 		account = acmeAccount
@@ -345,15 +407,21 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 
 	client, err := GetAcmeClient(account, userTx, db)
 	if err != nil {
-		log.Errorf("acme: getting acme client for provider %s", provider)
+		log.Errorf("acme: getting acme client for provider %s: %v", provider, err)
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
-		return errors.New("getting acme client: " + err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
+		return fmt.Errorf("getting acme client: %v", err)
 	}
 
 	priv, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {
 		log.Errorf(deliveryService + ": Error generating private key: " + err.Error())
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return err
 	}
 	request := certificate.ObtainRequest{
@@ -366,6 +434,9 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	if err != nil {
 		log.Errorf(deliveryService+": Error obtaining acme certificate from %s: %s", provider, err.Error())
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return err
 	}
 
@@ -383,6 +454,9 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	if err != nil {
 		log.Errorf(deliveryService + ": Error converting private key to PEM: " + err.Error())
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
 		return err
 	}
 
@@ -398,23 +472,34 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	if err := tv.PutDeliveryServiceSSLKeys(dsSSLKeys, tx); err != nil {
 		log.Errorf("Error putting ACME certificate in Traffic Vault: %s", err.Error())
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
-		return errors.New(deliveryService + ": putting keys in Traffic Vault: " + err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
+		return fmt.Errorf(deliveryService+": putting keys in Traffic Vault: %v", err)
 	}
 
 	tx2, err := db.Begin()
 	if err != nil {
 		log.Errorf("starting sql transaction for delivery service " + *req.DeliveryService + ": " + err.Error())
-		return errors.New("starting sql transaction for delivery service " + *req.DeliveryService + ": " + err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
+		return fmt.Errorf("starting sql transaction for delivery service "+*req.DeliveryService+": %v", err)
 	}
 
 	if err := updateSSLKeyVersion(*req.DeliveryService, req.Version.ToInt64(), tx2); err != nil {
 		log.Errorf("updating SSL key version for delivery service '" + *req.DeliveryService + "': " + err.Error())
-		return errors.New("updating SSL key version for delivery service '" + *req.DeliveryService + "': " + err.Error())
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+		}
+		return fmt.Errorf("updating SSL key version for delivery service '"+*req.DeliveryService+"': %v", err)
 	}
 	tx2.Commit()
 
 	api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: Added SSL keys with "+provider, currentUser, logTx)
-
+	if asycErr := api.UpdateAsyncStatus(db, api.AsyncSucceeded, "ACME renewal complete.", asyncStatusId, true); asycErr != nil {
+		log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
+	}
 	return nil
 }
 
@@ -542,7 +627,7 @@ func GetAcmeClient(acmeAccount *config.ConfigAcmeAccount, userTx *sql.Tx, db *sq
 		err = storeAcmeAccountInfo(userTx, myUser.Email, string(userKeyPem), myUser.Registration.URI, acmeAccount.AcmeProvider)
 		if err != nil {
 			log.Errorf("storing user account info: " + err.Error())
-			return nil, errors.New("storing user account info: " + err.Error())
+			return nil, fmt.Errorf("storing user account info: %v", err)
 		}
 	}
 
@@ -559,7 +644,7 @@ func ConvertPrivateKeyToKeyPem(userPrivateKey *rsa.PrivateKey) ([]byte, error) {
 	userKeyBuf := bytes.Buffer{}
 	if err := pem.Encode(&userKeyBuf, &pem.Block{Type: "RSA PRIVATE KEY", Bytes: userKeyDer}); err != nil {
 		log.Errorf("pem-encoding private key: " + err.Error())
-		return nil, errors.New("pem-encoding private key: " + err.Error())
+		return nil, fmt.Errorf("pem-encoding private key: %v", err)
 	}
 	return userKeyBuf.Bytes(), nil
 }
@@ -579,7 +664,7 @@ func getStoredAcmeAccountInfo(tx *sql.Tx, email string, provider string) (*AcmeI
 		if err == sql.ErrNoRows {
 			return nil, nil
 		}
-		return nil, errors.New("getting ACME account record: " + err.Error())
+		return nil, fmt.Errorf("getting ACME account record: %v", err)
 	}
 
 	decodedKeyBlock, _ := pem.Decode([]byte(acmeInfo.Key))
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/autorenewcerts.go b/traffic_ops/traffic_ops_golang/deliveryservice/autorenewcerts.go
index 6374b59..4b5318e 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/autorenewcerts.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/autorenewcerts.go
@@ -129,16 +129,16 @@ func RunAutorenewal(existingCerts []ExistingCerts, cfg *config.Config, ctx conte
 	db, err := api.GetDB(ctx)
 	if err != nil {
 		log.Errorf("Error getting db: %s", err.Error())
-		if err = api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); err != nil {
-			log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
 		}
 		return
 	}
 	tx, err := db.Begin()
 	if err != nil {
 		log.Errorf("Error getting tx: %s", err.Error())
-		if err = api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); err != nil {
-			log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
 		}
 		return
 	}
@@ -146,8 +146,8 @@ func RunAutorenewal(existingCerts []ExistingCerts, cfg *config.Config, ctx conte
 	logTx, err := db.Begin()
 	if err != nil {
 		log.Errorf("Error getting logTx: %s", err.Error())
-		if err = api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); err != nil {
-			log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
 		}
 		return
 	}
@@ -220,7 +220,7 @@ func RunAutorenewal(existingCerts []ExistingCerts, cfg *config.Config, ctx conte
 		dsExpInfo.AuthType = keyObj.AuthType
 
 		if keyObj.AuthType == tc.LetsEncryptAuthType || (keyObj.AuthType == tc.SelfSignedCertAuthType && cfg.ConfigLetsEncrypt.ConvertSelfSigned) {
-			req := tc.DeliveryServiceLetsEncryptSSLKeysReq{
+			req := tc.DeliveryServiceAcmeSSLKeysReq{
 				DeliveryServiceSSLKeysReq: tc.DeliveryServiceSSLKeysReq{
 					HostName:        &keyObj.Hostname,
 					DeliveryService: &keyObj.DeliveryService,
@@ -230,7 +230,7 @@ func RunAutorenewal(existingCerts []ExistingCerts, cfg *config.Config, ctx conte
 				},
 			}
 
-			if err := GetAcmeCertificates(cfg, req, ctx, currentUser, tv); err != nil {
+			if err := GetAcmeCertificates(cfg, req, ctx, currentUser, 0, tv); err != nil {
 				dsExpInfo.Error = err
 				errorCount++
 			} else {
@@ -263,8 +263,8 @@ func RunAutorenewal(existingCerts []ExistingCerts, cfg *config.Config, ctx conte
 
 		}
 
-		if err = api.UpdateAsyncStatus(db, api.AsyncPending, "ACME renewal in progress. "+strconv.Itoa(renewedCount)+" certs renewed, "+strconv.Itoa(errorCount)+" errors.", asyncStatusId, false); err != nil {
-			log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+		if asycErr := api.UpdateAsyncStatus(db, api.AsyncPending, "ACME renewal in progress. "+strconv.Itoa(renewedCount)+" certs renewed, "+strconv.Itoa(errorCount)+" errors.", asyncStatusId, false); asycErr != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
 		}
 
 	}
@@ -274,8 +274,8 @@ func RunAutorenewal(existingCerts []ExistingCerts, cfg *config.Config, ctx conte
 	if errorCount > 0 && renewedCount == 0 {
 		asyncStatus = api.AsyncFailed
 	}
-	if err = api.UpdateAsyncStatus(db, asyncStatus, "ACME renewal complete. "+strconv.Itoa(renewedCount)+" certs renewed, "+strconv.Itoa(errorCount)+" errors.", asyncStatusId, true); err != nil {
-		log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+	if asycErr := api.UpdateAsyncStatus(db, asyncStatus, "ACME renewal complete. "+strconv.Itoa(renewedCount)+" certs renewed, "+strconv.Itoa(errorCount)+" errors.", asyncStatusId, true); asycErr != nil {
+		log.Errorf("updating async status for id %v: %v", asyncStatusId, asycErr)
 	}
 
 	if cfg.SMTP.Enabled && cfg.ConfigAcmeRenewal.SummaryEmail != "" {
diff --git a/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js b/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
index 6c1ea81..2870f5d 100644
--- a/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
+++ b/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
@@ -21,33 +21,33 @@ var DeliveryServiceSslKeysService = function($http, locationUtils, messageModel,
     this.successMessage = 'SSL Keys generated and updated for ';
     this.acmeSuccessMessage = 'ACME call has been made successfully. This may take a few minutes. Please watch for a notification in the Change Log. Delivery Service = ';
 
-	this.generateSslKeys = function(deliveryService, sslKeys, generateSslKeyForm) {
-		 return this.generateSslKeysBase(deliveryService, sslKeys, generateSslKeyForm, 'deliveryservices/sslkeys/generate', this.successMessage);
-	};
-
-    this.generateSslKeysWithLetsEncrypt = function(deliveryService, sslKeys, generateSslKeyForm) {
-        return this.generateSslKeysBase(deliveryService, sslKeys, generateSslKeyForm, 'deliveryservices/sslkeys/generate/acme', 'Lets Encrypt: ' + this.acmeSuccessMessage);
+    this.generateSslKeys = function(deliveryService, sslKeys, generateSslKeyForm) {
+        return this.generateSslKeysBase(deliveryService, sslKeys, generateSslKeyForm, 'deliveryservices/sslkeys/generate', this.successMessage);
     };
 
-	this.generateSslKeysWithAcme = function(deliveryService, sslKeys, generateSslKeyForm, provider) {
-		return this.generateSslKeysBase(deliveryService, sslKeys, generateSslKeyForm, 'deliveryservices/sslkeys/generate/acme', provider + ": " + this.acmeSuccessMessage);
-	};
+    this.generateSslKeysWithAcme = function(deliveryService, sslKeys, generateSslKeyForm) {
+        return this.generateSslKeysBase(deliveryService, sslKeys, generateSslKeyForm, 'deliveryservices/sslkeys/generate/acme', null);
+    };
 
-	this.generateSslKeysBase = function(deliveryService, sslKeys, generateSslKeyForm, endpoint, message) {
+    this.generateSslKeysBase = function(deliveryService, sslKeys, generateSslKeyForm, endpoint, message) {
         if (sslKeys.hasOwnProperty('version')){
             generateSslKeyForm.version = parseInt(sslKeys.version, 10) + 1;
         } else {
             generateSslKeyForm.version = 1;
         }
 
-		generateSslKeyForm.cdn = deliveryService.cdnName;
-		generateSslKeyForm.deliveryservice = deliveryService.xmlId;
-		generateSslKeyForm.key = deliveryService.xmlId;
-		generateSslKeyForm.authType = sslKeys.authType;
+        generateSslKeyForm.cdn = deliveryService.cdnName;
+        generateSslKeyForm.deliveryservice = deliveryService.xmlId;
+        generateSslKeyForm.key = deliveryService.xmlId;
+        generateSslKeyForm.authType = sslKeys.authType;
 
         return $http.post(ENV.api['root'] + endpoint, generateSslKeyForm).then(
             function(result) {
-            	messageModel.setMessages([{level: 'success', text: message + deliveryService.xmlId}], true);
+                if (message === null) {
+                    messageModel.setMessages(result.data.alerts, true);
+                } else {
+                    messageModel.setMessages([{level: 'success', text: message + deliveryService.xmlId}], true);
+                }
                 return result.data.response;
             },
             function(err) {
@@ -57,24 +57,24 @@ var DeliveryServiceSslKeysService = function($http, locationUtils, messageModel,
                 throw err;
             }
         );
-	};
+    };
 
-	this.renewCert = function(deliveryService) {
-		return $http.post(ENV.api['root'] + "deliveryservices/xmlId/" + deliveryService.xmlId + "/sslkeys/renew").then(
-			function(result) {
-				messageModel.setMessages(result.data.alerts, false);
-				return result.data.response;
-			},
-			function(err) {
-				if (err.data && err.data.alerts) {
-					messageModel.setMessages(err.data.alerts, false);
-				}
-				throw err;
-			}
-		);
-	};
+    this.renewCert = function(deliveryService) {
+        return $http.post(ENV.api['root'] + "deliveryservices/xmlId/" + deliveryService.xmlId + "/sslkeys/renew").then(
+            function(result) {
+                messageModel.setMessages(result.data.alerts, false);
+                return result.data.response;
+            },
+            function(err) {
+                if (err.data && err.data.alerts) {
+                    messageModel.setMessages(err.data.alerts, false);
+                }
+                throw err;
+            }
+        );
+    };
 
-	this.addSslKeys = function(sslKeys, deliveryService) {
+    this.addSslKeys = function(sslKeys, deliveryService) {
 
         sslKeys.key = deliveryService.xmlId;
         if (sslKeys.hasOwnProperty('version')){
@@ -98,9 +98,9 @@ var DeliveryServiceSslKeysService = function($http, locationUtils, messageModel,
                 throw err;
             }
         );
-	};
+    };
 
-	this.getSslKeys = function(deliveryService) {
+    this.getSslKeys = function(deliveryService) {
         return $http.get(ENV.api['root'] + "deliveryservices/xmlId/" + deliveryService.xmlId + "/sslkeys", {params: {decode: "true"}}).then(
             function(result) {
                 return result.data.response;
@@ -112,18 +112,18 @@ var DeliveryServiceSslKeysService = function($http, locationUtils, messageModel,
                 throw err;
             }
         );
-	};
+    };
 
-	this.getAcmeProviders = function() {
-		return $http.get(ENV.api['root'] + 'acme_accounts/providers').then(
-			function (result) {
-				return result.data.response;
-			},
-			function (err) {
-				throw err;
-			}
-		);
-	};
+    this.getAcmeProviders = function() {
+        return $http.get(ENV.api['root'] + 'acme_accounts/providers').then(
+            function (result) {
+                return result.data.response;
+            },
+            function (err) {
+                throw err;
+            }
+        );
+    };
 };
 
 DeliveryServiceSslKeysService.$inject = ['$http', 'locationUtils', 'messageModel', 'ENV'];
diff --git a/traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/generate/FormGenerateDeliveryServiceSslKeysController.js b/traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/generate/FormGenerateDeliveryServiceSslKeysController.js
index ceee39d..d8f3e0c 100644
--- a/traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/generate/FormGenerateDeliveryServiceSslKeysController.js
+++ b/traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/generate/FormGenerateDeliveryServiceSslKeysController.js
@@ -357,7 +357,7 @@ var FormGenerateDeliveryServiceSslKeysController = function(deliveryService, ssl
         });
         modalInstance.result.then(function() {
             sslKeys.authType = $scope.acmeProvider;
-            deliveryServiceSslKeysService.generateSslKeysWithAcme(deliveryService, sslKeys, sslRequest, $scope.acmeProvider).then(
+            deliveryServiceSslKeysService.generateSslKeysWithAcme(deliveryService, sslKeys, sslRequest).then(
                 function() {
                     locationUtils.navigateToPath('/delivery-services/' + deliveryService.id + '/ssl-keys');
                 });