Fix status code and alerts when querying delivery service with no SSL…

… keys (#7640)

CHANGELOG.md

@@ -65,6 +65,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7621](https://github.com/apache/trafficcontrol/pull/7621) *Traffic Ops* Use ID token for OAuth authentication, not Access Token
 
 ### Fixed
+- [#4393](https://github.com/apache/trafficcontrol/issues/4393) *Traffic Ops* Fixed the error code and alert structure when TO is queried for a delivery service with no ssl keys.
 - [#7623] (https://github.com/apache/trafficcontrol/pull/7623) *Traffic Ops* Removed TryIfModifiedSinceQuery from servicecategories.go and reused from ims.go
 - [#7608](https://github.com/apache/trafficcontrol/pull/7608) *Traffic Monitor* Use stats_over_http(plugin.system_stats.timestamp_ms) timestamp field to calculate bandwidth for TM's caches.
 - [#6318](https://github.com/apache/trafficcontrol/issues/6318) *Docs* Included docs for POST, PUT, DELETE (v3,v4,v5) for statuses and statuses{id}

traffic_ops/testing/api/v5/deliveryservices_keys_test.go

@@ -14,6 +14,7 @@ package v5
 
 import (
 	"encoding/json"
+	"net/http"
 	"strconv"
 	"strings"
 	"testing"
@@ -41,6 +42,7 @@ func TestDeliveryServicesKeys(t *testing.T) {
 		t.Run("Delete URI Signing keys for a Delivery Service", DeleteTestDeliveryServicesURISigningKeys)
 		t.Run("Delete old CDN SSL keys", DeleteCDNOldSSLKeys)
 		t.Run("Create and retrieve SSL keys for a Delivery Service", DeliveryServiceSSLKeys)
+		t.Run("Retrieve SSL keys for a Delivery Service without keys", DeliveryServiceNoSSLKeys)
 	})
 }
 
@@ -234,6 +236,28 @@ func DeleteCDNOldSSLKeys(t *testing.T) {
 	assert.RequireEqual(t, len(newCdnKeys), 1, "Expected 1 ssl keys for CDN %v, got %d instead", cdn.Name, len(newCdnKeys))
 }
 
+func DeliveryServiceNoSSLKeys(t *testing.T) {
+	cdn := createBlankCDN("testDSWithNoSSLKeysCDN", t)
+
+	opts := client.NewRequestOptions()
+	opts.QueryParameters.Set("name", "HTTP")
+	types, _, err := TOSession.GetTypes(opts)
+	assert.RequireNoError(t, err, "Unable to get Types: %v - alerts: %+v", err, types.Alerts)
+	assert.RequireGreaterOrEqual(t, len(types.Response), 1, "Expected at least one type")
+
+	customDS := getCustomDS(cdn.ID, types.Response[0].ID, "dsNoSSLKeys", "routingName", "https://test.com", "dsNoSSLKeys")
+	customDS.Protocol = util.Ptr(0)
+	resp, _, err := TOSession.CreateDeliveryService(customDS, client.RequestOptions{})
+	assert.RequireNoError(t, err, "Unexpected error creating a Delivery Service: %v - alerts: %+v", err, resp.Alerts)
+
+	ds := resp.Response
+	assert.RequireNotNil(t, ds.XMLID, "Traffic Ops returned a representation for a Delivery Service with null or undefined XMLID")
+
+	_, reqInf, err := TOSession.GetDeliveryServiceSSLKeys(ds.XMLID, client.NewRequestOptions())
+	assert.RequireNotNil(t, err, "expected error getting ssl keys, but got nothing")
+	assert.RequireEqual(t, reqInf.StatusCode, http.StatusNotFound)
+}
+
 func DeliveryServiceSSLKeys(t *testing.T) {
 	cdn := createBlankCDN("sslkeytransfer", t)
 

traffic_ops/traffic_ops_golang/deliveryservice/keys.go

@@ -187,12 +187,27 @@ func GetSSLKeysByXMLID(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	var userError error
+	sc := http.StatusInternalServerError
+	logAlert := true
 	keyObjV4, err := getSslKeys(inf, r.Context())
 	if err != nil {
-		userErr := api.LogErr(r, http.StatusInternalServerError, nil, err)
-		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
-		api.WriteAlerts(w, r, http.StatusInternalServerError, alerts)
-		return
+		if err == sql.ErrNoRows {
+			if inf.Version.GreaterThanOrEqualTo(&api.Version{Major: 5, Minor: 0}) {
+				sc = http.StatusNotFound
+				userError = api.LogErr(r, sc, errors.New("no ssl keys for XML ID "+xmlID), nil)
+			} else {
+				// For versions lesser than 5.0, don't log an alert if the error is ErrNoRows. This is for backward compatibility reasons.
+				logAlert = false
+			}
+		} else {
+			userError = api.LogErr(r, sc, nil, err)
+		}
+		if logAlert {
+			alerts.AddNewAlert(tc.ErrorLevel, userError.Error())
+			api.WriteAlerts(w, r, sc, alerts)
+			return
+		}
 	}
 
 	var keyObj interface{}
@@ -216,6 +231,9 @@ func getSslKeys(inf *api.APIInfo, ctx context.Context) (tc.DeliveryServiceSSLKey
 
 	keyObjFromTv, ok, err := inf.Vault.GetDeliveryServiceSSLKeys(xmlID, version, inf.Tx.Tx, ctx)
 	if err != nil {
+		if err == sql.ErrNoRows {
+			return tc.DeliveryServiceSSLKeysV4{}, err
+		}
 		return tc.DeliveryServiceSSLKeysV4{}, errors.New("getting ssl keys: " + err.Error())
 	}
 	keyObj := tc.DeliveryServiceSSLKeysV4{}

traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go

@@ -122,7 +122,9 @@ func GeneratePlaceholderSelfSignedCert(ds tc.DeliveryServiceV5, inf *api.APIInfo
 	tv := inf.Vault
 	_, ok, err := tv.GetDeliveryServiceSSLKeys(ds.XMLID, "", tx, context)
 	if err != nil {
-		return fmt.Errorf("getting latest ssl keys for XMLID '%s': %w", ds.XMLID, err), http.StatusInternalServerError
+		if err != sql.ErrNoRows {
+			return fmt.Errorf("getting latest ssl keys for XMLID '%s': %w", ds.XMLID, err), http.StatusInternalServerError
+		}
 	}
 	if ok {
 		return nil, http.StatusOK

traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go

@@ -138,7 +138,7 @@ func (p *Postgres) GetDeliveryServiceSSLKeys(xmlID string, version string, tx *s
 	err = tvTx.QueryRow(query, xmlID, version).Scan(&encryptedSslKeys)
 	if err != nil {
 		if err == sql.ErrNoRows {
-			return tc.DeliveryServiceSSLKeysV15{}, false, nil
+			return tc.DeliveryServiceSSLKeysV15{}, false, err
 		}
 		e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err())
 		return tc.DeliveryServiceSSLKeysV15{}, false, e