[trafficcontrol] branch master updated: Ensure SSL changes are logged. (#5423)

[ma...@apache.org]

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

mattjackson 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 7d1192f  Ensure SSL changes are logged. (#5423)
7d1192f is described below

commit 7d1192fe21d0440e4c78a8b989333efb09e3448f
Author: Taylor Clayton Frey <ta...@gmail.com>
AuthorDate: Fri Jan 15 08:58:45 2021 -0700

    Ensure SSL changes are logged. (#5423)
    
    * Ensure SSL changes are logged.
    
    There are three endpoints (Add, Generate, Delete) that manipulate SSL certificates
    for a delivery service. (Actually there are more with the Let's Encrypt ACME
    endpoints, but those have a different changelog order of operations). These all
    must log their action in the Changelog for verification and confirmation when
    the actions complete. Generate and Delete succesfully log the changes, however
    Add was apparently not.
    
    In fact, there are two successful cases where the SSL keys could be added and
    the endpoint would return prematurely, preventing the action from being logged
    in the Changelog. The Changelog entry is now performed before the return of
    these two logic flows.
    
    Additionally added a comment to a public package function (Generate) and clarified
    language within the Changelog messages.
    
    * Add changelog.md entry for bugfix
    
    * Move CHANGELOG note to correct section
    
    Co-authored-by: Taylor Frey <ta...@comcast.com>
---
 CHANGELOG.md                                              | 1 +
 traffic_ops/traffic_ops_golang/deliveryservice/keys.go    | 5 ++++-
 traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go | 5 ++++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9df5ee8..b116cf0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,6 +36,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#5311](https://github.com/apache/trafficcontrol/issues/5311) - Better TO log messages when failures calling TM CacheStats
 - [#5364](https://github.com/apache/trafficcontrol/issues/5364) - Cascade server deletes to delete corresponding IP addresses and interfaces
 - [#5390](https://github.com/apache/trafficcontrol/issues/5390) - Improve the way TO deals with delivery service server assignments
+- [#5339](https://github.com/apache/trafficcontrol/issues/5339) - Ensure Changelog entries for SSL key changes
 
 ### Changed
 - Refactored the Traffic Ops Go client internals so that all public methods have a consistent behavior/implementation
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go
index 22c6a8f..d7e8c4e 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go
@@ -117,6 +117,9 @@ func AddSSLKeys(w http.ResponseWriter, r *http.Request) {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("adding SSL keys to delivery service '"+*req.DeliveryService+"': "+err.Error()))
 		return
 	}
+
+	api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: Added/Updated SSL keys", inf.User, inf.Tx.Tx)
+
 	if isUnknownAuth {
 		api.WriteRespAlert(w, r, tc.WarnLevel, "WARNING: SSL keys were successfully added for '"+*req.DeliveryService+"', but the input certificate may be invalid (certificate is signed by an unknown authority)")
 		return
@@ -125,7 +128,7 @@ func AddSSLKeys(w http.ResponseWriter, r *http.Request) {
 		api.WriteRespAlert(w, r, tc.WarnLevel, "WARNING: SSL keys were successfully added for '"+*req.DeliveryService+"', but the input certificate may be invalid (certificate verification produced a different chain)")
 		return
 	}
-	api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: Added SSL keys", inf.User, inf.Tx.Tx)
+
 	api.WriteResp(w, r, "Successfully added ssl keys for "+*req.DeliveryService)
 }
 
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go b/traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go
index 6b99a55..75e1494 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go
@@ -32,6 +32,9 @@ import (
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
 )
 
+// GenerateSSLKeys generates a new private key, certificate signing request and
+// certificate based on the values submitted. It then stores these values in
+// TrafficVault and updates the SSL key version.
 func GenerateSSLKeys(w http.ResponseWriter, r *http.Request) {
 	inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
 	if userErr != nil || sysErr != nil {
@@ -65,7 +68,7 @@ func GenerateSSLKeys(w http.ResponseWriter, r *http.Request) {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("generating SSL keys for delivery service '"+*req.DeliveryService+"': "+err.Error()))
 		return
 	}
-	api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: Added SSL keys", inf.User, inf.Tx.Tx)
+	api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: Generated SSL keys", inf.User, inf.Tx.Tx)
 	api.WriteResp(w, r, "Successfully created ssl keys for "+*req.DeliveryService)
 }