From 87dfcf373956fd648ad666830218f76533b7a307 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 22 Jun 2023 18:13:00 -0600 Subject: [PATCH 1/7] Fix status code and alert structure for sslkeys endpoint, when no ssl keys are present --- CHANGELOG.md | 1 + .../traffic_ops_golang/deliveryservice/keys.go | 16 +++++++++++++--- .../trafficvault/backends/postgres/postgres.go | 2 +- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e578712b8..f0e0ac9a7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7425](https://github.com/apache/trafficcontrol/pull/7425) *Traffic Control Cache Config (t3c)* Fixed issue with layered profile iteration being done in the wrong order. - [#6385](https://github.com/apache/trafficcontrol/issues/6385) *Traffic Ops* Reserved consistentHashQueryParameters cause internal server error - [#7471](https://github.com/apache/trafficcontrol/pull/7471) *Traffic Control Cache Config (t3c)* Fixed issue with MSO non topo origins from multiple cache groups. +- [#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. ### Removed - [#7271](https://github.com/apache/trafficcontrol/pull/7271) Remove components in `infrastructre/docker/`, not in use as cdn-in-a-box performs the same functionality. diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go index ef02ebb683..d441f8ed97 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go @@ -187,11 +187,18 @@ func GetSSLKeysByXMLID(w http.ResponseWriter, r *http.Request) { return } + var userError error + sc := http.StatusInternalServerError 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) + if err == sql.ErrNoRows { + sc = http.StatusNotFound + userError = api.LogErr(r, sc, errors.New("no ssl keys for XML ID "+xmlID), nil) + } else { + userError = api.LogErr(r, sc, nil, err) + } + alerts.AddNewAlert(tc.ErrorLevel, userError.Error()) + api.WriteAlerts(w, r, sc, alerts) return } @@ -216,6 +223,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{} diff --git a/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go b/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go index b1bdc1b394..1499882d30 100644 --- a/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go +++ b/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 From 1de7005f3458ccc31dbbb4b883dd5549641ac887 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 22 Jun 2023 18:44:59 -0600 Subject: [PATCH 2/7] fix tests --- traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go | 4 +++- .../trafficvault/backends/postgres/postgres.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go b/traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go index f3a133c066..38402e9cf0 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/sslkeys.go +++ b/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 diff --git a/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go b/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go index 1499882d30..263ca70431 100644 --- a/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go +++ b/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, err + return tc.DeliveryServiceSSLKeysV15{}, true, err } e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err()) return tc.DeliveryServiceSSLKeysV15{}, false, e From bdfe42907b0375be93d1faeb0cc9e50860a1ba35 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 22 Jun 2023 23:02:00 -0600 Subject: [PATCH 3/7] fix v4 test --- .../deliveryservice/keys.go | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go index d441f8ed97..65dce07d05 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go @@ -189,17 +189,24 @@ func GetSSLKeysByXMLID(w http.ResponseWriter, r *http.Request) { var userError error sc := http.StatusInternalServerError + logAlert := true keyObjV4, err := getSslKeys(inf, r.Context()) if err != nil { + userError = api.LogErr(r, sc, nil, err) if err == sql.ErrNoRows { - sc = http.StatusNotFound - userError = api.LogErr(r, sc, errors.New("no ssl keys for XML ID "+xmlID), nil) - } else { - userError = api.LogErr(r, sc, nil, err) + 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 + } + } + if logAlert { + alerts.AddNewAlert(tc.ErrorLevel, userError.Error()) + api.WriteAlerts(w, r, sc, alerts) + return } - alerts.AddNewAlert(tc.ErrorLevel, userError.Error()) - api.WriteAlerts(w, r, sc, alerts) - return } var keyObj interface{} From 5e8c691d0c738d3483b07a48a90c37356308c0be Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 23 Jun 2023 11:34:30 -0600 Subject: [PATCH 4/7] fix v4 test --- traffic_ops/traffic_ops_golang/deliveryservice/keys.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go index 65dce07d05..e6aeb63b35 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go @@ -213,7 +213,9 @@ func GetSSLKeysByXMLID(w http.ResponseWriter, r *http.Request) { if inf.Version.Major < 4 { keyObj = keyObjV4.DeliveryServiceSSLKeysV15 } else { - keyObj = keyObjV4 + if inf.Version.LessThan(&api.Version{Major: 5, Minor: 0}) { + keyObj = keyObjV4 + } } if len(alerts.Alerts) == 0 { From 63411d5640ec6f000d5e36a45bdfec1ce6d310c3 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 23 Jun 2023 11:56:55 -0600 Subject: [PATCH 5/7] adding debug --- traffic_ops/testing/api/v5/deliveryservices_keys_test.go | 2 ++ traffic_ops/traffic_ops_golang/deliveryservice/keys.go | 1 + .../trafficvault/backends/postgres/postgres.go | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/traffic_ops/testing/api/v5/deliveryservices_keys_test.go b/traffic_ops/testing/api/v5/deliveryservices_keys_test.go index 54b0775d5b..1cc3bc96a6 100644 --- a/traffic_ops/testing/api/v5/deliveryservices_keys_test.go +++ b/traffic_ops/testing/api/v5/deliveryservices_keys_test.go @@ -14,6 +14,7 @@ package v5 import ( "encoding/json" + "fmt" "strconv" "strings" "testing" @@ -366,6 +367,7 @@ func DeliveryServiceSSLKeys(t *testing.T) { func VerifySSLKeysOnDsCreationTest(t *testing.T) { for _, ds := range testData.DeliveryServices { + fmt.Println("DS Protocol is ", *ds.Protocol) if !(*ds.Protocol == tc.DSProtocolHTTPS || *ds.Protocol == tc.DSProtocolHTTPAndHTTPS || *ds.Protocol == tc.DSProtocolHTTPToHTTPS) { continue } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go index e6aeb63b35..1499fc6dfe 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go @@ -203,6 +203,7 @@ func GetSSLKeysByXMLID(w http.ResponseWriter, r *http.Request) { } } if logAlert { + fmt.Println("SSLkeys error is ", userError) alerts.AddNewAlert(tc.ErrorLevel, userError.Error()) api.WriteAlerts(w, r, sc, alerts) return diff --git a/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go b/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go index 263ca70431..1499882d30 100644 --- a/traffic_ops/traffic_ops_golang/trafficvault/backends/postgres/postgres.go +++ b/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{}, true, err + return tc.DeliveryServiceSSLKeysV15{}, false, err } e := checkErrWithContext("Traffic Vault PostgreSQL: executing SELECT SSL Keys query", err, ctx.Err()) return tc.DeliveryServiceSSLKeysV15{}, false, e From 204ca4955daf3ca7141633132eab79e60359d92e Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 23 Jun 2023 12:44:52 -0600 Subject: [PATCH 6/7] debug --- traffic_ops/testing/api/v5/deliveryservices_keys_test.go | 2 +- traffic_ops/traffic_ops_golang/deliveryservice/keys.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/traffic_ops/testing/api/v5/deliveryservices_keys_test.go b/traffic_ops/testing/api/v5/deliveryservices_keys_test.go index 1cc3bc96a6..ecc6568438 100644 --- a/traffic_ops/testing/api/v5/deliveryservices_keys_test.go +++ b/traffic_ops/testing/api/v5/deliveryservices_keys_test.go @@ -382,7 +382,7 @@ func VerifySSLKeysOnDsCreationTest(t *testing.T) { break } } - + fmt.Println("DS Protocol here is ", *ds.Protocol) if err != nil || dsSSLKey == nil { t.Fatalf("unable to get DS %s SSL key: %v", ds.XMLID, err) } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go index 1499fc6dfe..e6aeb63b35 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go @@ -203,7 +203,6 @@ func GetSSLKeysByXMLID(w http.ResponseWriter, r *http.Request) { } } if logAlert { - fmt.Println("SSLkeys error is ", userError) alerts.AddNewAlert(tc.ErrorLevel, userError.Error()) api.WriteAlerts(w, r, sc, alerts) return From 59d406de7627405d24cd2700cabf1f8c0d862a00 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 23 Jun 2023 14:54:59 -0600 Subject: [PATCH 7/7] fix v5 test --- traffic_ops/testing/api/v5/deliveryservices_keys_test.go | 3 --- traffic_ops/traffic_ops_golang/deliveryservice/keys.go | 4 +--- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/traffic_ops/testing/api/v5/deliveryservices_keys_test.go b/traffic_ops/testing/api/v5/deliveryservices_keys_test.go index ecc6568438..47b1d9cc50 100644 --- a/traffic_ops/testing/api/v5/deliveryservices_keys_test.go +++ b/traffic_ops/testing/api/v5/deliveryservices_keys_test.go @@ -14,7 +14,6 @@ package v5 import ( "encoding/json" - "fmt" "strconv" "strings" "testing" @@ -367,7 +366,6 @@ func DeliveryServiceSSLKeys(t *testing.T) { func VerifySSLKeysOnDsCreationTest(t *testing.T) { for _, ds := range testData.DeliveryServices { - fmt.Println("DS Protocol is ", *ds.Protocol) if !(*ds.Protocol == tc.DSProtocolHTTPS || *ds.Protocol == tc.DSProtocolHTTPAndHTTPS || *ds.Protocol == tc.DSProtocolHTTPToHTTPS) { continue } @@ -382,7 +380,6 @@ func VerifySSLKeysOnDsCreationTest(t *testing.T) { break } } - fmt.Println("DS Protocol here is ", *ds.Protocol) if err != nil || dsSSLKey == nil { t.Fatalf("unable to get DS %s SSL key: %v", ds.XMLID, err) } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go index e6aeb63b35..65dce07d05 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go @@ -213,9 +213,7 @@ func GetSSLKeysByXMLID(w http.ResponseWriter, r *http.Request) { if inf.Version.Major < 4 { keyObj = keyObjV4.DeliveryServiceSSLKeysV15 } else { - if inf.Version.LessThan(&api.Version{Major: 5, Minor: 0}) { - keyObj = keyObjV4 - } + keyObj = keyObjV4 } if len(alerts.Alerts) == 0 {