ocket8888 commented on code in PR #6763: URL: https://github.com/apache/trafficcontrol/pull/6763#discussion_r878279303
########## traffic_ops/testing/api/v4/cdn_dnsseckeys_test.go: ########## @@ -0,0 +1,156 @@ +package v4 + +/* + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +import ( + "net/http" + "reflect" + "strconv" + "strings" + "testing" + + tc "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-util" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" + client "github.com/apache/trafficcontrol/traffic_ops/v4-client" +) + +func TestCDNsDNSSEC(t *testing.T) { + if !includeSystemTests { + t.Skip() + } + WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServerCapabilities, ServiceCategories, DeliveryServices}, func() { + t.Run("GENERATE DNSSEC KEYS", func(t *testing.T) { GenerateDNSSECKeys(t) }) + t.Run("REFRESH DNSSEC KEYS", func(t *testing.T) { RefreshDNSSECKeys(t) }) // NOTE: testing refresh last (while no keys exist) because it's asynchronous and might affect other tests + }) +} + +func RefreshDNSSECKeys(t *testing.T) { + resp, reqInf, err := TOSession.RefreshDNSSECKeys(client.RequestOptions{}) + assert.NoError(t, err, "Unable to refresh DNSSEC keys: %v - alerts: %+v", err, resp.Alerts) + assert.Equal(t, reqInf.StatusCode, http.StatusAccepted, "Refreshing DNSSEC keys - Expected: status code %d, Actual: %d", http.StatusAccepted, reqInf.StatusCode) + + loc := reqInf.RespHeaders.Get("Location") + if loc == "" { + t.Fatalf("Refreshing DNSSEC keys - Expected: non-empty 'Location' response header, Actual: empty") + } + asyncID, err := strconv.Atoi(strings.Split(loc, "/")[4]) + assert.RequireNoError(t, err, "Parsing async_status ID from 'Location' response header - Expected: no error, Actual: %v", err) + + status, _, err := TOSession.GetAsyncStatus(asyncID, client.RequestOptions{}) + assert.NoError(t, err, "Getting async status id %d - Expected: no error, Actual: %v", asyncID, err) + assert.NotNil(t, status.Response.Message, "Getting async status for DNSSEC refresh job - Expected: non-nil message, Actual: nil") +} + +func GenerateDNSSECKeys(t *testing.T) { + assert.GreaterOrEqual(t, len(testData.CDNs), 1, "Need at least one CDN to test updating CDNs") + firstCDN := testData.CDNs[0] Review Comment: since this doesn't use the return value of `assert.GreaterOrEqual` it doesn't protect the following line against a panic ########## traffic_ops/testing/api/v3/cdns_test.go: ########## @@ -16,192 +16,187 @@ package v3 */ import ( + "encoding/json" "net/http" + "net/url" "sort" "testing" "time" "github.com/apache/trafficcontrol/lib/go-rfc" tc "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/utils" + "github.com/apache/trafficcontrol/traffic_ops/toclientlib" ) func TestCDNs(t *testing.T) { WithObjs(t, []TCObj{CDNs, Parameters}, func() { - GetTestCDNsIMS(t) - currentTime := time.Now().UTC().Add(-5 * time.Second) - time := currentTime.Format(time.RFC1123) - var header http.Header - header = make(map[string][]string) - header.Set(rfc.IfModifiedSince, time) - header.Set(rfc.IfUnmodifiedSince, time) - SortTestCDNs(t) - UpdateTestCDNs(t) - UpdateTestCDNsWithHeaders(t, header) - header = make(map[string][]string) - etag := rfc.ETag(currentTime) - header.Set(rfc.IfMatch, etag) - UpdateTestCDNsWithHeaders(t, header) - GetTestCDNs(t) - GetTestCDNsIMSAfterChange(t, header) + currentTime := time.Now().UTC().Add(-15 * time.Second) + currentTimeRFC := currentTime.Format(time.RFC1123) + tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123) + + methodTests := utils.V3TestCase{ + "GET": { + "NOT MODIFIED when NO CHANGES made": { + ClientSession: TOSession, RequestHeaders: http.Header{rfc.IfModifiedSince: {tomorrow}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)), + }, + "OK when VALID request": { + ClientSession: TOSession, Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), + utils.ResponseLengthGreaterOrEqual(1), validateCDNSort()), + }, + "OK when VALID NAME parameter": { + ClientSession: TOSession, RequestParams: url.Values{"name": {"cdn1"}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1), + validateCDNFields(map[string]interface{}{"Name": "cdn1"})), + }, + }, + "PUT": { + "OK when VALID request": { + EndpointId: GetCDNID(t, "cdn1"), ClientSession: TOSession, + RequestBody: map[string]interface{}{ + "dnssecEnabled": false, + "domainName": "newDomain", + "name": "cdn1", + }, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), + }, + "PRECONDITION FAILED when updating with IF-UNMODIFIED-SINCE Headers": { + EndpointId: GetCDNID(t, "cdn1"), ClientSession: TOSession, + RequestHeaders: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}}, + RequestBody: map[string]interface{}{ + "dnssecEnabled": false, + "domainName": "newDomain", + "name": "cdn1", + }, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)), + }, + "PRECONDITION FAILED when updating with IFMATCH ETAG Header": { + EndpointId: GetCDNID(t, "cdn1"), ClientSession: TOSession, + RequestHeaders: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}}, + RequestBody: map[string]interface{}{ + "dnssecEnabled": false, + "domainName": "newDomain", + "name": "cdn1", + }, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)), + }, + }, + "GET AFTER CHANGES": { + "OK when CHANGES made": { + ClientSession: TOSession, + RequestHeaders: http.Header{rfc.IfModifiedSince: {currentTimeRFC}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), + }, + }, + } + for method, testCases := range methodTests { + t.Run(method, func(t *testing.T) { + for name, testCase := range testCases { + cdn := tc.CDN{} + + if testCase.RequestBody != nil { + dat, err := json.Marshal(testCase.RequestBody) + assert.NoError(t, err, "Error occurred when marshalling request body: %v", err) + err = json.Unmarshal(dat, &cdn) + assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err) + } + + switch method { + case "GET", "GET AFTER CHANGES": + t.Run(name, func(t *testing.T) { + if name == "OK when VALID NAME parameter" { + resp, reqInf, err := testCase.ClientSession.GetCDNByNameWithHdr(testCase.RequestParams["name"][0], testCase.RequestHeaders) + for _, check := range testCase.Expectations { + check(t, reqInf, resp, tc.Alerts{}, err) + } + } else { + resp, reqInf, err := testCase.ClientSession.GetCDNsWithHdr(testCase.RequestHeaders) + for _, check := range testCase.Expectations { + check(t, reqInf, resp, tc.Alerts{}, err) + } + } + }) + case "PUT": + t.Run(name, func(t *testing.T) { + alerts, reqInf, err := testCase.ClientSession.UpdateCDNByIDWithHdr(testCase.EndpointId(), cdn, testCase.RequestHeaders) + for _, check := range testCase.Expectations { + check(t, reqInf, nil, alerts, err) + } + }) + case "DELETE": + t.Run(name, func(t *testing.T) { + alerts, reqInf, err := testCase.ClientSession.DeleteCDNByID(testCase.EndpointId()) + for _, check := range testCase.Expectations { + check(t, reqInf, nil, alerts, err) + } + }) + } + } + }) + } }) } -func UpdateTestCDNsWithHeaders(t *testing.T, header http.Header) { - firstCDN := testData.CDNs[0] - // Retrieve the CDN by name so we can get the id for the Update - resp, _, err := TOSession.GetCDNByNameWithHdr(firstCDN.Name, header) - if err != nil { - t.Errorf("cannot GET CDN by name: '%s', %v", firstCDN.Name, err) - } - if len(resp) > 0 { - remoteCDN := resp[0] - remoteCDN.DomainName = "domain2" - _, reqInf, err := TOSession.UpdateCDNByIDWithHdr(remoteCDN.ID, remoteCDN, header) - if err == nil { - t.Errorf("Expected error about Precondition Failed, got none") - } - if reqInf.StatusCode != http.StatusPreconditionFailed { - t.Errorf("Expected status code 412, got %v", reqInf.StatusCode) +func validateCDNFields(expectedResp map[string]interface{}) utils.CkReqFunc { + return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) { + cdnResp := resp.([]tc.CDN) + for field, expected := range expectedResp { + for _, cdn := range cdnResp { + switch field { + case "Name": + assert.Equal(t, expected, cdn.Name, "Expected Name to be %v, but got %v", expected, cdn.Name) + case "DomainName": + assert.Equal(t, expected, cdn.DomainName, "Expected DomainName to be %v, but got %v", expected, cdn.DomainName) + case "DNSSECEnabled": + assert.Equal(t, expected, cdn.DNSSECEnabled, "Expected DNSSECEnabled to be %v, but got %v", expected, cdn.DNSSECEnabled) + default: + t.Errorf("Expected field: %v, does not exist in response", field) + } + } } } } -func GetTestCDNsIMSAfterChange(t *testing.T, header http.Header) { - for _, cdn := range testData.CDNs { - _, reqInf, err := TOSession.GetCDNByNameWithHdr(cdn.Name, header) - if err != nil { - t.Fatalf("Expected no error, but got %v", err.Error()) - } - if reqInf.StatusCode != http.StatusOK { - t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode) - } - } - currentTime := time.Now().UTC() - currentTime = currentTime.Add(1 * time.Second) - timeStr := currentTime.Format(time.RFC1123) - header.Set(rfc.IfModifiedSince, timeStr) - for _, cdn := range testData.CDNs { - _, reqInf, err := TOSession.GetCDNByNameWithHdr(cdn.Name, header) - if err != nil { - t.Fatalf("Expected no error, but got %v", err.Error()) - } - if reqInf.StatusCode != http.StatusNotModified { - t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode) +func validateCDNSort() utils.CkReqFunc { + return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) { + var cdnNames []string + cdnResp := resp.([]tc.CDN) + for _, cdn := range cdnResp { + cdnNames = append(cdnNames, cdn.Name) } + assert.Equal(t, true, sort.StringsAreSorted(cdnNames), "List is not sorted by their names: %v", cdnNames) } } -func GetTestCDNsIMS(t *testing.T) { - var header http.Header - header = make(map[string][]string) - for _, cdn := range testData.CDNs { - futureTime := time.Now().AddDate(0, 0, 1) - time := futureTime.Format(time.RFC1123) - header.Set(rfc.IfModifiedSince, time) - _, reqInf, err := TOSession.GetCDNByNameWithHdr(cdn.Name, header) - if err != nil { - t.Fatalf("Expected no error, but got %v", err.Error()) - } - if reqInf.StatusCode != http.StatusNotModified { - t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode) - } +func GetCDNID(t *testing.T, cdnName string) func() int { + return func() int { + cdnsResp, _, err := TOSession.GetCDNByNameWithHdr(cdnName, http.Header{}) + assert.NoError(t, err, "Get CDNs Request failed with error:", err) + assert.Equal(t, 1, len(cdnsResp), "Expected response object length 1, but got %d", len(cdnsResp)) + assert.NotNil(t, cdnsResp[0].ID, "Expected id to not be nil") Review Comment: These aren't using the return value, so I think they need to be <code>Require<var>Assertion</var></code> to prevent segfaulting. ########## traffic_ops/testing/api/v4/cdn_dnsseckeys_test.go: ########## @@ -0,0 +1,156 @@ +package v4 + +/* + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +import ( + "net/http" + "reflect" + "strconv" + "strings" + "testing" + + tc "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-util" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" + client "github.com/apache/trafficcontrol/traffic_ops/v4-client" +) + +func TestCDNsDNSSEC(t *testing.T) { + if !includeSystemTests { + t.Skip() + } + WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServerCapabilities, ServiceCategories, DeliveryServices}, func() { + t.Run("GENERATE DNSSEC KEYS", func(t *testing.T) { GenerateDNSSECKeys(t) }) + t.Run("REFRESH DNSSEC KEYS", func(t *testing.T) { RefreshDNSSECKeys(t) }) // NOTE: testing refresh last (while no keys exist) because it's asynchronous and might affect other tests + }) +} + +func RefreshDNSSECKeys(t *testing.T) { + resp, reqInf, err := TOSession.RefreshDNSSECKeys(client.RequestOptions{}) + assert.NoError(t, err, "Unable to refresh DNSSEC keys: %v - alerts: %+v", err, resp.Alerts) + assert.Equal(t, reqInf.StatusCode, http.StatusAccepted, "Refreshing DNSSEC keys - Expected: status code %d, Actual: %d", http.StatusAccepted, reqInf.StatusCode) + + loc := reqInf.RespHeaders.Get("Location") + if loc == "" { + t.Fatalf("Refreshing DNSSEC keys - Expected: non-empty 'Location' response header, Actual: empty") + } + asyncID, err := strconv.Atoi(strings.Split(loc, "/")[4]) + assert.RequireNoError(t, err, "Parsing async_status ID from 'Location' response header - Expected: no error, Actual: %v", err) + + status, _, err := TOSession.GetAsyncStatus(asyncID, client.RequestOptions{}) + assert.NoError(t, err, "Getting async status id %d - Expected: no error, Actual: %v", asyncID, err) + assert.NotNil(t, status.Response.Message, "Getting async status for DNSSEC refresh job - Expected: non-nil message, Actual: nil") +} + +func GenerateDNSSECKeys(t *testing.T) { + assert.GreaterOrEqual(t, len(testData.CDNs), 1, "Need at least one CDN to test updating CDNs") + firstCDN := testData.CDNs[0] + opts := client.NewRequestOptions() + opts.QueryParameters.Set("name", firstCDN.Name) + cdns, _, err := TOSession.GetCDNs(opts) + assert.NoError(t, err, "Unexpected error getting CDNs filtered by name '%s': %v - alerts: %+v", firstCDN.Name, err, cdns.Alerts) + assert.Equal(t, len(cdns.Response), 1, "Expected exactly one CDN named '%s' to exist, found: %d", firstCDN.Name, len(cdns.Response)) + + cdn := cdns.Response[0] Review Comment: Since the return value of `assert.Equal` is ignored here, it doesn't protect line 67 from a memory access violation. ########## traffic_ops/testing/api/v4/cdn_dnsseckeys_test.go: ########## @@ -0,0 +1,156 @@ +package v4 + +/* + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +import ( + "net/http" + "reflect" + "strconv" + "strings" + "testing" + + tc "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-util" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" + client "github.com/apache/trafficcontrol/traffic_ops/v4-client" +) + +func TestCDNsDNSSEC(t *testing.T) { + if !includeSystemTests { + t.Skip() + } + WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServerCapabilities, ServiceCategories, DeliveryServices}, func() { + t.Run("GENERATE DNSSEC KEYS", func(t *testing.T) { GenerateDNSSECKeys(t) }) + t.Run("REFRESH DNSSEC KEYS", func(t *testing.T) { RefreshDNSSECKeys(t) }) // NOTE: testing refresh last (while no keys exist) because it's asynchronous and might affect other tests + }) +} + +func RefreshDNSSECKeys(t *testing.T) { + resp, reqInf, err := TOSession.RefreshDNSSECKeys(client.RequestOptions{}) + assert.NoError(t, err, "Unable to refresh DNSSEC keys: %v - alerts: %+v", err, resp.Alerts) + assert.Equal(t, reqInf.StatusCode, http.StatusAccepted, "Refreshing DNSSEC keys - Expected: status code %d, Actual: %d", http.StatusAccepted, reqInf.StatusCode) + + loc := reqInf.RespHeaders.Get("Location") + if loc == "" { + t.Fatalf("Refreshing DNSSEC keys - Expected: non-empty 'Location' response header, Actual: empty") + } + asyncID, err := strconv.Atoi(strings.Split(loc, "/")[4]) Review Comment: This should check if the string actually can be separated into 5 parts by a delimiter of `/` before doing this indexing, to avoid a segmentation fault. ########## traffic_ops/testing/api/v4/cdn_dnsseckeys_test.go: ########## @@ -0,0 +1,156 @@ +package v4 + +/* + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +import ( + "net/http" + "reflect" + "strconv" + "strings" + "testing" + + tc "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-util" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" + client "github.com/apache/trafficcontrol/traffic_ops/v4-client" +) + +func TestCDNsDNSSEC(t *testing.T) { + if !includeSystemTests { + t.Skip() + } + WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServerCapabilities, ServiceCategories, DeliveryServices}, func() { + t.Run("GENERATE DNSSEC KEYS", func(t *testing.T) { GenerateDNSSECKeys(t) }) + t.Run("REFRESH DNSSEC KEYS", func(t *testing.T) { RefreshDNSSECKeys(t) }) // NOTE: testing refresh last (while no keys exist) because it's asynchronous and might affect other tests + }) +} + +func RefreshDNSSECKeys(t *testing.T) { + resp, reqInf, err := TOSession.RefreshDNSSECKeys(client.RequestOptions{}) + assert.NoError(t, err, "Unable to refresh DNSSEC keys: %v - alerts: %+v", err, resp.Alerts) + assert.Equal(t, reqInf.StatusCode, http.StatusAccepted, "Refreshing DNSSEC keys - Expected: status code %d, Actual: %d", http.StatusAccepted, reqInf.StatusCode) + + loc := reqInf.RespHeaders.Get("Location") + if loc == "" { + t.Fatalf("Refreshing DNSSEC keys - Expected: non-empty 'Location' response header, Actual: empty") + } + asyncID, err := strconv.Atoi(strings.Split(loc, "/")[4]) + assert.RequireNoError(t, err, "Parsing async_status ID from 'Location' response header - Expected: no error, Actual: %v", err) + + status, _, err := TOSession.GetAsyncStatus(asyncID, client.RequestOptions{}) + assert.NoError(t, err, "Getting async status id %d - Expected: no error, Actual: %v", asyncID, err) + assert.NotNil(t, status.Response.Message, "Getting async status for DNSSEC refresh job - Expected: non-nil message, Actual: nil") +} + +func GenerateDNSSECKeys(t *testing.T) { + assert.GreaterOrEqual(t, len(testData.CDNs), 1, "Need at least one CDN to test updating CDNs") + firstCDN := testData.CDNs[0] + opts := client.NewRequestOptions() + opts.QueryParameters.Set("name", firstCDN.Name) + cdns, _, err := TOSession.GetCDNs(opts) + assert.NoError(t, err, "Unexpected error getting CDNs filtered by name '%s': %v - alerts: %+v", firstCDN.Name, err, cdns.Alerts) + assert.Equal(t, len(cdns.Response), 1, "Expected exactly one CDN named '%s' to exist, found: %d", firstCDN.Name, len(cdns.Response)) + + cdn := cdns.Response[0] + + ttl := util.JSONIntStr(60) + req := tc.CDNDNSSECGenerateReq{ + Key: util.StrPtr(firstCDN.Name), + TTL: &ttl, + KSKExpirationDays: &ttl, + ZSKExpirationDays: &ttl, + } + resp, _, err := TOSession.GenerateCDNDNSSECKeys(req, client.RequestOptions{}) + assert.RequireNoError(t, err, "Unexpected error generating CDN DNSSEC keys: %v - alerts: %+v", err, resp.Alerts) + + res, _, err := TOSession.GetCDNDNSSECKeys(firstCDN.Name, client.RequestOptions{}) + assert.RequireNoError(t, err, "Unexpected error getting CDN DNSSEC keys: %v - alerts: %+v", err, res.Alerts) + + if _, ok := res.Response[firstCDN.Name]; !ok { + t.Errorf("getting CDN DNSSEC keys - expected: key %s, actual: missing", firstCDN.Name) + } + originalKeys := res.Response + + resp, _, err = TOSession.GenerateCDNDNSSECKeys(req, client.RequestOptions{}) + assert.RequireNoError(t, err, "Unexpected error generating CDN DNSSEC keys: %v - alerts: %+v", err, resp.Alerts) + + res, _, err = TOSession.GetCDNDNSSECKeys(firstCDN.Name, client.RequestOptions{}) + assert.RequireNoError(t, err, "Unexpected error getting CDN DNSSEC keys: %v - alerts: %+v", err, res.Alerts) + + newKeys := res.Response + + if reflect.DeepEqual(originalKeys, newKeys) { + t.Errorf("Generating CDN DNSSEC keys - expected: original keys to differ from new keys, actual: they are the same") + } + + kskReq := tc.CDNGenerateKSKReq{ + ExpirationDays: util.Uint64Ptr(30), + } + originalKSK := newKeys + resp, _, err = TOSession.GenerateCDNDNSSECKSK(firstCDN.Name, kskReq, client.RequestOptions{}) + assert.NoError(t, err, "Unexpected error generating DNSSEC KSK: %v - alerts: %+v", err, resp.Alerts) + + res, _, err = TOSession.GetCDNDNSSECKeys(firstCDN.Name, client.RequestOptions{}) + assert.NoError(t, err, "Unexpected error getting CDN DNSSEC keys: %v - alerts: %+v", err, res.Alerts) + + if _, ok := res.Response[firstCDN.Name]; !ok { + t.Fatalf("getting CDN DNSSEC keys - expected: key %s, actual: missing", firstCDN.Name) + } + newKSK := res.Response + if reflect.DeepEqual(originalKSK[firstCDN.Name].KSK, newKSK[firstCDN.Name].KSK) { + t.Error("Generating CDN DNSSEC KSK - Expected: KSK to be different, Actual: KSK is the same") + } + if !reflect.DeepEqual(originalKSK[firstCDN.Name].ZSK, newKSK[firstCDN.Name].ZSK) { + t.Error("Generating CDN DNSSEC KSK - Expected: ZSK to be equal, Actual: ZSK is different") + } + + // ensure that when DNSSEC is enabled on a CDN, creating a new DS will generate DNSSEC keys for that DS: + if !cdn.DNSSECEnabled { + cdn.DNSSECEnabled = true + resp, _, err := TOSession.UpdateCDN(cdn.ID, cdn, client.RequestOptions{}) + assert.NoError(t, err, "Unexpected error updating CDN: %v - alerts: %+v", err, resp.Alerts) + + defer func() { + cdn.DNSSECEnabled = false + resp, _, err := TOSession.UpdateCDN(cdn.ID, cdn, client.RequestOptions{}) + assert.NoError(t, err, "Unexpected error updating CDN: %v - alerts: %+v", err, resp.Alerts) + }() + } + + opts.QueryParameters.Set("name", "HTTP") + types, _, err := TOSession.GetTypes(opts) + assert.RequireNoError(t, err, "Unexpected error getting Types filtered by name 'HTTP': %v - alerts: %+v", err, types.Alerts) + assert.RequireEqual(t, len(types.Response), 1, "Expected exactly one Type to exist with name 'HTTP', found: %d", len(types.Response)) + + dsXMLID := "testdnssecgen" + customDS := getCustomDS(cdn.ID, types.Response[0].ID, dsXMLID, "cdn", "https://testdnssecgen.example.com";, dsXMLID) + ds, _, err := TOSession.CreateDeliveryService(customDS, client.RequestOptions{}) + assert.NoError(t, err, "Unexpected error creating Delivery Service: %v - alerts: %+v", err, ds.Alerts) + assert.Equal(t, len(ds.Response), 1, "Expected creating a Delivery Service to create exactly one Delivery Service, Traffic Ops returned: %d", len(ds.Response)) + assert.NotNil(t, ds.Response[0].ID, nil, "Traffic Ops returned a representation for a created Delivery Service with null or undefined ID") Review Comment: This should use `RequireEqual` and `RequireNotNil` on lines 142 and 143, respectively ########## traffic_ops/testing/api/v3/cdns_test.go: ########## @@ -16,192 +16,187 @@ package v3 */ import ( + "encoding/json" "net/http" + "net/url" "sort" "testing" "time" "github.com/apache/trafficcontrol/lib/go-rfc" tc "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/utils" + "github.com/apache/trafficcontrol/traffic_ops/toclientlib" ) func TestCDNs(t *testing.T) { WithObjs(t, []TCObj{CDNs, Parameters}, func() { - GetTestCDNsIMS(t) - currentTime := time.Now().UTC().Add(-5 * time.Second) - time := currentTime.Format(time.RFC1123) - var header http.Header - header = make(map[string][]string) - header.Set(rfc.IfModifiedSince, time) - header.Set(rfc.IfUnmodifiedSince, time) - SortTestCDNs(t) - UpdateTestCDNs(t) - UpdateTestCDNsWithHeaders(t, header) - header = make(map[string][]string) - etag := rfc.ETag(currentTime) - header.Set(rfc.IfMatch, etag) - UpdateTestCDNsWithHeaders(t, header) - GetTestCDNs(t) - GetTestCDNsIMSAfterChange(t, header) + currentTime := time.Now().UTC().Add(-15 * time.Second) + currentTimeRFC := currentTime.Format(time.RFC1123) + tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123) + + methodTests := utils.V3TestCase{ + "GET": { + "NOT MODIFIED when NO CHANGES made": { + ClientSession: TOSession, RequestHeaders: http.Header{rfc.IfModifiedSince: {tomorrow}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)), + }, + "OK when VALID request": { + ClientSession: TOSession, Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), + utils.ResponseLengthGreaterOrEqual(1), validateCDNSort()), + }, + "OK when VALID NAME parameter": { + ClientSession: TOSession, RequestParams: url.Values{"name": {"cdn1"}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1), + validateCDNFields(map[string]interface{}{"Name": "cdn1"})), + }, + }, + "PUT": { + "OK when VALID request": { + EndpointId: GetCDNID(t, "cdn1"), ClientSession: TOSession, + RequestBody: map[string]interface{}{ + "dnssecEnabled": false, + "domainName": "newDomain", + "name": "cdn1", + }, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), + }, Review Comment: The original `PUT` test checked that the update was actually performed by comparing the `domainName` property of the response to a subsequent `GET` request to that provided in the `PUT` request, this appears to have dropped that. ########## traffic_ops/testing/api/v4/cdns_test.go: ########## @@ -16,705 +16,309 @@ package v4 */ import ( + "encoding/json" "net/http" "net/url" - "reflect" "sort" "strconv" - "strings" "testing" "time" "github.com/apache/trafficcontrol/lib/go-rfc" tc "github.com/apache/trafficcontrol/lib/go-tc" - "github.com/apache/trafficcontrol/lib/go-util" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" + "github.com/apache/trafficcontrol/traffic_ops/testing/api/utils" + "github.com/apache/trafficcontrol/traffic_ops/toclientlib" client "github.com/apache/trafficcontrol/traffic_ops/v4-client" ) func TestCDNs(t *testing.T) { WithObjs(t, []TCObj{CDNs, Parameters}, func() { - GetTestCDNsIMS(t) - currentTime := time.Now().UTC().Add(-5 * time.Second) - time := currentTime.Format(time.RFC1123) - var header http.Header - header = make(map[string][]string) - header.Set(rfc.IfModifiedSince, time) - header.Set(rfc.IfUnmodifiedSince, time) - SortTestCDNs(t) - UpdateTestCDNs(t) - UpdateTestCDNsWithHeaders(t, header) - header = make(map[string][]string) - etag := rfc.ETag(currentTime) - header.Set(rfc.IfMatch, etag) - UpdateTestCDNsWithHeaders(t, header) - GetTestCDNs(t) - t.Run("get CDNs filtered by Domain Name", GetTestCDNsbyDomainName) - GetTestCDNsbyDnssec(t) - GetTestCDNsIMSAfterChange(t, header) - CreateTestCDNEmptyName(t) - CreateTestCDNEmptyDomainName(t) - GetTestPaginationSupportCdns(t) - SortTestCdnDesc(t) - CreateTestCDNsAlreadyExist(t) - DeleteTestCDNsInvalidId(t) - UpdateDeleteCDNWithLocks(t) - }) -} - -func UpdateDeleteCDNWithLocks(t *testing.T) { - // Create a new user with operations level privileges - user1 := tc.UserV4{ - Username: "lock_user1", - RegistrationSent: new(time.Time), - LocalPassword: util.StrPtr("test_pa$$word"), - ConfirmLocalPassword: util.StrPtr("test_pa$$word"), - Role: "operations", - } - user1.Email = util.StrPtr("[email protected]") - user1.TenantID = 1 - user1.FullName = util.StrPtr("firstName LastName") - _, _, err := TOSession.CreateUser(user1, client.RequestOptions{}) - if err != nil { - t.Fatalf("could not create test user with username: %s", user1.Username) - } - defer ForceDeleteTestUsersByUsernames(t, []string{"lock_user1"}) - - // Establish a session with the newly created non admin level user - userSession, _, err := client.LoginWithAgent(Config.TrafficOps.URL, user1.Username, *user1.LocalPassword, true, "to-api-v4-client-tests", false, toReqTimeout) - if err != nil { - t.Fatalf("could not login with user lock_user1: %v", err) - } - - cdn := createBlankCDN("locksCDN", t) - - // Create a lock for this user - _, _, err = userSession.CreateCDNLock(tc.CDNLock{ - CDN: cdn.Name, - Message: util.StrPtr("test lock"), - Soft: util.BoolPtr(false), - }, client.RequestOptions{}) - if err != nil { - t.Fatalf("couldn't create cdn lock: %v", err) - } - - opts := client.NewRequestOptions() - opts.QueryParameters.Set("name", cdn.Name) - cdns, _, err := userSession.GetCDNs(opts) - if err != nil { - t.Fatalf("couldn't get cdn: %v", err) - } - if len(cdns.Response) != 1 { - t.Fatal("couldn't get exactly one cdn in the response, quitting") - } - cdnID := cdns.Response[0].ID - // Try to update a CDN that another user has a hard lock on -> this should fail - cdns.Response[0].DomainName = "changed_domain_name" - _, reqInf, err := TOSession.UpdateCDN(cdnID, cdns.Response[0], client.RequestOptions{}) - if err == nil { - t.Error("expected an error while updating a CDN for which a hard lock is held by another user, but got nothing") - } - if reqInf.StatusCode != http.StatusForbidden { - t.Errorf("expected a 403 forbidden status while updating a CDN for which a hard lock is held by another user, but got %d", reqInf.StatusCode) - } - - // Try to update a CDN that the same user has a hard lock on -> this should succeed - _, reqInf, err = userSession.UpdateCDN(cdnID, cdns.Response[0], client.RequestOptions{}) - if err != nil { - t.Errorf("expected no error while updating a CDN for which a hard lock is held by the same user, but got %v", err) - } - - // Try to delete a CDN that another user has a hard lock on -> this should fail - _, reqInf, err = TOSession.DeleteCDN(cdnID, client.RequestOptions{}) - if err == nil { - t.Error("expected an error while deleting a CDN for which a hard lock is held by another user, but got nothing") - } - if reqInf.StatusCode != http.StatusForbidden { - t.Errorf("expected a 403 forbidden status while deleting a CDN for which a hard lock is held by another user, but got %d", reqInf.StatusCode) - } - - // Try to delete a CDN that the same user has a hard lock on -> this should succeed - // This should also delete the lock associated with this CDN - _, reqInf, err = userSession.DeleteCDN(cdnID, client.RequestOptions{}) - if err != nil { - t.Errorf("expected no error while deleting a CDN for which a hard lock is held by the same user, but got %v", err) - } - - locks, _, _ := userSession.GetCDNLocks(client.RequestOptions{}) - if len(locks.Response) != 0 { - t.Errorf("expected deletion of CDN to delete it's associated lock, and no locks in the response, but got %d locks instead", len(locks.Response)) - } -} - -func TestCDNsDNSSEC(t *testing.T) { - WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServerCapabilities, ServiceCategories, DeliveryServices}, func() { - if includeSystemTests { - GenerateDNSSECKeys(t) - RefreshDNSSECKeys(t) // NOTE: testing refresh last (while no keys exist) because it's asynchronous and might affect other tests + currentTime := time.Now().UTC().Add(-15 * time.Second) + currentTimeRFC := currentTime.Format(time.RFC1123) + tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123) + + methodTests := utils.V4TestCase{ + "GET": { + "NOT MODIFIED when NO CHANGES made": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {tomorrow}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)), + }, + "OK when VALID request": { + ClientSession: TOSession, Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), + utils.ResponseLengthGreaterOrEqual(1), validateCDNSort()), + }, + "OK when VALID NAME parameter": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"name": {"cdn1"}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1), + validateCDNFields(map[string]interface{}{"Name": "cdn1"})), + }, + "OK when VALID DOMAINNAME parameter": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"domainName": {"test.cdn2.net"}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1), + validateCDNFields(map[string]interface{}{"DomainName": "test.cdn2.net"})), + }, + "OK when VALID DNSSECENABLED parameter": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"dnssecEnabled": {"false"}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), + validateCDNFields(map[string]interface{}{"DNSSECEnabled": false})), + }, + "VALID when SORTORDER param is DESC": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"sortOrder": {"desc"}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCDNDescSort()), + }, + "FIRST RESULT when LIMIT=1": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"id"}, "limit": {"1"}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCDNPagination("limit")), + }, + "SECOND RESULT when LIMIT=1 OFFSET=1": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"id"}, "limit": {"1"}, "offset": {"1"}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCDNPagination("offset")), + }, + "SECOND RESULT when LIMIT=1 PAGE=2": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"id"}, "limit": {"1"}, "page": {"2"}}}, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCDNPagination("page")), + }, + "BAD REQUEST when INVALID LIMIT parameter": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"-2"}}}, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), + }, + "BAD REQUEST when INVALID OFFSET parameter": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "offset": {"0"}}}, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), + }, + "BAD REQUEST when INVALID PAGE parameter": { + ClientSession: TOSession, RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "page": {"0"}}}, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), + }, + }, + "POST": { + "BAD REQUEST when CDN ALREADY EXISTS": { + ClientSession: TOSession, RequestBody: map[string]interface{}{ + "name": "cdn3", + "dnssecEnabled": false, + "domainName": "test.cdn3.net", + }, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), + }, + "BAD REQUEST when EMPTY NAME": { + ClientSession: TOSession, RequestBody: map[string]interface{}{ + "name": "", + "dnssecEnabled": false, + "domainName": "test.noname.net", + }, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), + }, + "BAD REQUEST when EMPTY DOMAIN NAME": { + ClientSession: TOSession, RequestBody: map[string]interface{}{ + "name": "nodomain", + "dnssecEnabled": false, + "domainName": "", + }, + Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), + }, + }, + "PUT": { + "OK when VALID request": { + EndpointId: GetCDNID(t, "cdn1"), ClientSession: TOSession, + RequestBody: map[string]interface{}{ + "dnssecEnabled": false, + "domainName": "newDomain", + "name": "cdn1", + }, + Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), + }, Review Comment: Same as above -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
