From 77ec51996ecf4a01bf173bdda432eec74db1f330 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Mon, 9 May 2022 17:33:17 -0600 Subject: [PATCH 1/7] prevent modifying and/or deleting reserved statuses --- CHANGELOG.md | 1 + lib/go-tc/enum.go | 5 + ...50916074300_add_reserved_statuses.down.sql | 18 ++ ...2050916074300_add_reserved_statuses.up.sql | 22 ++ traffic_ops/testing/api/v2/statuses_test.go | 117 +++++++---- traffic_ops/testing/api/v3/statuses_test.go | 153 +++++++++----- traffic_ops/testing/api/v4/statuses_test.go | 188 +++++++++++------- .../traffic_ops_golang/status/statuses.go | 34 +++- 8 files changed, 369 insertions(+), 169 deletions(-) create mode 100644 traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql create mode 100644 traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index a6c302f728..638c5cee9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Added layered profile feature to 4.0 for `GET` /deliveryservices/{id}/servers/ and /deliveryservices/{id}/servers/eligible. ### Fixed +- [#6291](https://github.com/apache/trafficcontrol/issues/6291) Prevent Traffic Ops from modifying and/or deleting reserved statuses. - Update traffic\_portal dependencies to mitigate `npm audit` issues. - Fixed a cdn-in-a-box build issue when using `RHEL_VERSION=7` - `dequeueing` server updates should not require checking for cdn locks. diff --git a/lib/go-tc/enum.go b/lib/go-tc/enum.go index 014a624485..5b4464ecd5 100644 --- a/lib/go-tc/enum.go +++ b/lib/go-tc/enum.go @@ -374,6 +374,11 @@ const ( // by Traffic Monitor. The vast majority of cache servers should have this // Status. CacheStatusReported = CacheStatus("REPORTED") + // TODO: fix this description + // CacheStatusPreProd represents a cache server which is polled for health + // by Traffic Monitor. The vast majority of cache servers should have this + // Status. + CacheStatusPreProd = CacheStatus("PRE_PROD") // CacheStatusInvalid represents an unrecognized Status value. Note that // this is not actually "invalid", because Statuses may have any unique // name, not just those captured as CacheStatus values in this package. diff --git a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql new file mode 100644 index 0000000000..8786aacb86 --- /dev/null +++ b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql @@ -0,0 +1,18 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you 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. + */ + +DELETE FROM public.status WHERE name IN ('ONLINE', 'OFFLINE', 'REPORTED', 'ADMIN_DOWN', 'PRE_PROD'); diff --git a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql new file mode 100644 index 0000000000..d2315d7936 --- /dev/null +++ b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you 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. + */ + +INSERT INTO public.status ("name", description) VALUES ('ONLINE', 'Server is online.') ON CONFLICT ("name") DO NOTHING; +INSERT INTO public.status ("name", description) VALUES ('OFFLINE', 'Server is Offline. Not active in any configuration.') ON CONFLICT ("name") DO NOTHING; +INSERT INTO public.status ("name", description) VALUES ('REPORTED', 'Server is online and reported in the health protocol.') ON CONFLICT ("name") DO NOTHING; +INSERT INTO public.status ("name", description) VALUES ('ADMIN_DOWN', 'Sever is administrative down and does not receive traffic.') ON CONFLICT ("name") DO NOTHING; +INSERT INTO public.status ("name", description) VALUES ('PRE_PROD', 'Pre Production. Not active in any configuration.') ON CONFLICT ("name") DO NOTHING; diff --git a/traffic_ops/testing/api/v2/statuses_test.go b/traffic_ops/testing/api/v2/statuses_test.go index 9a15714723..9dda1ba63d 100644 --- a/traffic_ops/testing/api/v2/statuses_test.go +++ b/traffic_ops/testing/api/v2/statuses_test.go @@ -21,6 +21,14 @@ import ( tc "github.com/apache/trafficcontrol/lib/go-tc" ) +var statusNameMap = map[string]bool{ + "ADMIN_DOWN": true, + "ONLINE": true, + "OFFLINE": true, + "REPORTED": true, + "PRE_PROD": true, +} + func TestStatuses(t *testing.T) { WithObjs(t, []TCObj{Parameters, Statuses}, func() { UpdateTestStatuses(t) @@ -42,35 +50,44 @@ func CreateTestStatuses(t *testing.T) { func UpdateTestStatuses(t *testing.T) { - firstStatus := testData.Statuses[0] - if firstStatus.Name == nil { - t.Fatal("cannot update test statuses: first test data status must have a name") - } - - // Retrieve the Status by name so we can get the id for the Update - resp, _, err := TOSession.GetStatusByName(*firstStatus.Name) - if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", firstStatus.Name, err) - } - remoteStatus := resp[0] - expectedStatusDesc := "new description" - remoteStatus.Description = expectedStatusDesc - var alert tc.Alerts - alert, _, err = TOSession.UpdateStatusByID(remoteStatus.ID, remoteStatus) - if err != nil { - t.Errorf("cannot UPDATE Status by id: %v - %v", err, alert) - } - - // Retrieve the Status to check Status name got updated - resp, _, err = TOSession.GetStatusByID(remoteStatus.ID) - if err != nil { - t.Errorf("cannot GET Status by ID: %v - %v", firstStatus.Description, err) + if len(testData.Statuses) < 1 { + t.Fatal("Need at least one Status to test updating a Status") } - respStatus := resp[0] - if respStatus.Description != expectedStatusDesc { - t.Errorf("results do not match actual: %s, expected: %s", respStatus.Name, expectedStatusDesc) + for _, status := range testData.Statuses { + if status.Name == nil { + t.Fatal("cannot update test statuses: test data status must have a name") + } + // Retrieve the Status by name so we can get the id for the Update + resp, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) + } + remoteStatus := resp[0] + expectedStatusDesc := "new description" + remoteStatus.Description = expectedStatusDesc + var alert tc.Alerts + alert, _, err = TOSession.UpdateStatusByID(remoteStatus.ID, remoteStatus) + + if _, ok := statusNameMap[*status.Name]; ok { + if err == nil { + t.Errorf("expected an error about while updating a reserved status, but got nothing") + } + } else { + if err != nil { + t.Errorf("cannot UPDATE Status by id: %v - %v", err, alert) + } + + // Retrieve the Status to check Status name got updated + resp, _, err = TOSession.GetStatusByID(remoteStatus.ID) + if err != nil { + t.Errorf("cannot GET Status by ID: %v - %v", status.Description, err) + } + respStatus := resp[0] + if respStatus.Description != expectedStatusDesc { + t.Errorf("results do not match actual: %s, expected: %s", respStatus.Name, expectedStatusDesc) + } + } } - } func GetTestStatuses(t *testing.T) { @@ -106,17 +123,43 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if err != nil { - t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) + if _, ok := statusNameMap[*status.Name]; !ok { + if err != nil { + t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) + } + + // Retrieve the Status to see if it got deleted + types, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("error deleting Status name: %s", err.Error()) + } + if len(types) > 0 { + t.Errorf("expected Status name: %s to be deleted", *status.Name) + } + } else { + if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + } } + } + ForceDeleteStatuses(t) +} - // Retrieve the Status to see if it got deleted - types, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("error deleting Status name: %s", err.Error()) - } - if len(types) > 0 { - t.Errorf("expected Status name: %s to be deleted", *status.Name) - } +// ForceDeleteStatuses forcibly deletes the statuses from the db. +func ForceDeleteStatuses(t *testing.T) { + + // NOTE: Special circumstances! This should *NOT* be done without a really good reason! + // Connects directly to the DB to remove users rather than going thru the client. + // This is required here because the DeleteUser action does not really delete users, but disables them. + db, err := OpenConnection() + if err != nil { + t.Error("cannot open db") + } + defer db.Close() + + q := `DELETE FROM status` + err = execSQL(db, q) + if err != nil { + t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) } } diff --git a/traffic_ops/testing/api/v3/statuses_test.go b/traffic_ops/testing/api/v3/statuses_test.go index db095c6c83..2bd15e85d1 100644 --- a/traffic_ops/testing/api/v3/statuses_test.go +++ b/traffic_ops/testing/api/v3/statuses_test.go @@ -25,6 +25,14 @@ import ( tc "github.com/apache/trafficcontrol/lib/go-tc" ) +var statusNameMap = map[string]bool{ + "ADMIN_DOWN": true, + "ONLINE": true, + "OFFLINE": true, + "REPORTED": true, + "PRE_PROD": true, +} + func TestStatuses(t *testing.T) { WithObjs(t, []TCObj{Parameters, Statuses}, func() { GetTestStatusesIMS(t) @@ -47,27 +55,31 @@ func TestStatuses(t *testing.T) { } func UpdateTestStatusesWithHeaders(t *testing.T, header http.Header) { - if len(testData.Statuses) > 0 { - firstStatus := testData.Statuses[0] - if firstStatus.Name == nil { - t.Fatal("cannot update test statuses: first test data status must have a name") - } + if len(testData.Statuses) < 1 { + t.Fatal("Need at least one Status to test updating a status with an HTTP header") + } - // Retrieve the Status by name so we can get the id for the Update - resp, _, err := TOSession.GetStatusByNameWithHdr(*firstStatus.Name, header) - if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", firstStatus.Name, err) + for _, status := range testData.Statuses { + if status.Name == nil { + t.Fatal("cannot update test statuses: test data status must have a name") } - if len(resp) > 0 { - remoteStatus := resp[0] - expectedStatusDesc := "new description" - remoteStatus.Description = expectedStatusDesc - _, reqInf, err := TOSession.UpdateStatusByIDWithHdr(remoteStatus.ID, remoteStatus, header) - if err == nil { - t.Errorf("Expected error about precondition failed, but got none") + if _, ok := statusNameMap[*status.Name]; !ok { + // Retrieve the Status by name so we can get the id for the Update + resp, _, err := TOSession.GetStatusByNameWithHdr(*status.Name, header) + if err != nil { + t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) } - if reqInf.StatusCode != http.StatusPreconditionFailed { - t.Errorf("Expected status code 412, got %v", reqInf.StatusCode) + if len(resp) > 0 { + remoteStatus := resp[0] + expectedStatusDesc := "new description" + remoteStatus.Description = expectedStatusDesc + _, reqInf, err := TOSession.UpdateStatusByIDWithHdr(remoteStatus.ID, remoteStatus, header) + if err == nil { + t.Errorf("Expected error about precondition failed, but got none") + } + if reqInf.StatusCode != http.StatusPreconditionFailed { + t.Errorf("Expected status code 412, got %v", reqInf.StatusCode) + } } } } @@ -156,36 +168,41 @@ func SortTestStatuses(t *testing.T) { } func UpdateTestStatuses(t *testing.T) { + for _, status := range testData.Statuses { + if status.Name == nil { + t.Fatal("cannot update test statuses: test data status must have a name") + } + // Retrieve the Status by name so we can get the id for the Update + resp, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) + } + remoteStatus := resp[0] + expectedStatusDesc := "new description" + remoteStatus.Description = expectedStatusDesc + var alert tc.Alerts + alert, _, err = TOSession.UpdateStatusByID(remoteStatus.ID, remoteStatus) - firstStatus := testData.Statuses[0] - if firstStatus.Name == nil { - t.Fatal("cannot update test statuses: first test data status must have a name") - } - - // Retrieve the Status by name so we can get the id for the Update - resp, _, err := TOSession.GetStatusByName(*firstStatus.Name) - if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", firstStatus.Name, err) - } - remoteStatus := resp[0] - expectedStatusDesc := "new description" - remoteStatus.Description = expectedStatusDesc - var alert tc.Alerts - alert, _, err = TOSession.UpdateStatusByID(remoteStatus.ID, remoteStatus) - if err != nil { - t.Errorf("cannot UPDATE Status by id: %v - %v", err, alert) - } + if _, ok := statusNameMap[*status.Name]; ok { + if err == nil { + t.Errorf("expected an error about while updating a reserved status, but got nothing") + } + } else { + if err != nil { + t.Errorf("cannot UPDATE Status by id: %v - %v", err, alert) + } - // Retrieve the Status to check Status name got updated - resp, _, err = TOSession.GetStatusByID(remoteStatus.ID) - if err != nil { - t.Errorf("cannot GET Status by ID: %v - %v", firstStatus.Description, err) - } - respStatus := resp[0] - if respStatus.Description != expectedStatusDesc { - t.Errorf("results do not match actual: %s, expected: %s", respStatus.Name, expectedStatusDesc) + // Retrieve the Status to check Status name got updated + resp, _, err = TOSession.GetStatusByID(remoteStatus.ID) + if err != nil { + t.Errorf("cannot GET Status by ID: %v - %v", status.Description, err) + } + respStatus := resp[0] + if respStatus.Description != expectedStatusDesc { + t.Errorf("results do not match actual: %s, expected: %s", respStatus.Name, expectedStatusDesc) + } + } } - } func GetTestStatuses(t *testing.T) { @@ -216,17 +233,43 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if err != nil { - t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) - } + if _, ok := statusNameMap[*status.Name]; !ok { + if err != nil { + t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) + } - // Retrieve the Status to see if it got deleted - types, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("error deleting Status name: %s", err.Error()) - } - if len(types) > 0 { - t.Errorf("expected Status name: %s to be deleted", *status.Name) + // Retrieve the Status to see if it got deleted + types, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("error deleting Status name: %s", err.Error()) + } + if len(types) > 0 { + t.Errorf("expected Status name: %s to be deleted", *status.Name) + } + } else { + if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + } } } + ForceDeleteStatuses(t) +} + +// ForceDeleteStatuses forcibly deletes the statuses from the db. +func ForceDeleteStatuses(t *testing.T) { + + // NOTE: Special circumstances! This should *NOT* be done without a really good reason! + // Connects directly to the DB to remove users rather than going thru the client. + // This is required here because the DeleteUser action does not really delete users, but disables them. + db, err := OpenConnection() + if err != nil { + t.Error("cannot open db") + } + defer db.Close() + + q := `DELETE FROM status` + err = execSQL(db, q) + if err != nil { + t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) + } } diff --git a/traffic_ops/testing/api/v4/statuses_test.go b/traffic_ops/testing/api/v4/statuses_test.go index 106d5ca112..e9dd5fd52c 100644 --- a/traffic_ops/testing/api/v4/statuses_test.go +++ b/traffic_ops/testing/api/v4/statuses_test.go @@ -26,6 +26,14 @@ import ( client "github.com/apache/trafficcontrol/traffic_ops/v4-client" ) +var statusNameMap = map[string]bool{ + "ADMIN_DOWN": true, + "ONLINE": true, + "OFFLINE": true, + "REPORTED": true, + "PRE_PROD": true, +} + func TestStatuses(t *testing.T) { WithObjs(t, []TCObj{Parameters, Statuses}, func() { GetTestStatusesIMS(t) @@ -52,31 +60,33 @@ func UpdateTestStatusesWithHeaders(t *testing.T, header http.Header) { t.Fatal("Need at least one Status to test updating a status with an HTTP header") } - firstStatus := testData.Statuses[0] - if firstStatus.Name == nil { - t.Fatal("cannot update test statuses: first test data status must have a name") - } - - // Retrieve the Status by name so we can get the id for the Update - opts := client.NewRequestOptions() - opts.Header = header - opts.QueryParameters.Set("name", *firstStatus.Name) - resp, _, err := TOSession.GetStatuses(opts) - if err != nil { - t.Errorf("cannot get Status by name '%s': %v - alerts %+v", *firstStatus.Name, err, resp.Alerts) - } - if len(resp.Response) > 0 { - remoteStatus := resp.Response[0] - expectedStatusDesc := "new description" - remoteStatus.Description = expectedStatusDesc - - opts.QueryParameters.Del("name") - _, reqInf, err := TOSession.UpdateStatus(remoteStatus.ID, remoteStatus, opts) - if err == nil { - t.Errorf("Expected error about precondition failed, but got none") + for _, status := range testData.Statuses { + if status.Name == nil { + t.Fatal("cannot update test statuses: test data status must have a name") } - if reqInf.StatusCode != http.StatusPreconditionFailed { - t.Errorf("Expected status code 412, got %d", reqInf.StatusCode) + if _, ok := statusNameMap[*status.Name]; !ok { + // Retrieve the Status by name so we can get the id for the Update + opts := client.NewRequestOptions() + opts.Header = header + opts.QueryParameters.Set("name", *status.Name) + resp, _, err := TOSession.GetStatuses(opts) + if err != nil { + t.Errorf("cannot get Status by name '%s': %v - alerts %+v", *status.Name, err, resp.Alerts) + } + if len(resp.Response) > 0 { + remoteStatus := resp.Response[0] + expectedStatusDesc := "new description" + remoteStatus.Description = expectedStatusDesc + + opts.QueryParameters.Del("name") + _, reqInf, err := TOSession.UpdateStatus(remoteStatus.ID, remoteStatus, opts) + if err == nil { + t.Errorf("Expected error about precondition failed, but got none") + } + if reqInf.StatusCode != http.StatusPreconditionFailed { + t.Errorf("Expected status code 412, got %d", reqInf.StatusCode) + } + } } } } @@ -171,45 +181,50 @@ func UpdateTestStatuses(t *testing.T) { if len(testData.Statuses) < 1 { t.Fatal("Need at least one Status to test updating a Status") } - firstStatus := testData.Statuses[0] - if firstStatus.Name == nil { - t.Fatal("cannot update test statuses: first test data status must have a name") - } - - // Retrieve the Status by name so we can get the id for the Update - opts := client.NewRequestOptions() - opts.QueryParameters.Set("name", *firstStatus.Name) - resp, _, err := TOSession.GetStatuses(opts) - if err != nil { - t.Errorf("cannot get Status by name '%s': %v - alerts: %+v", *firstStatus.Name, err, resp.Alerts) - } - if len(resp.Response) != 1 { - t.Fatalf("Expected exactly one Status to exist with name '%s', found: %d", *firstStatus.Name, len(resp.Response)) - } - remoteStatus := resp.Response[0] - expectedStatusDesc := "new description" - remoteStatus.Description = expectedStatusDesc - - alert, _, err := TOSession.UpdateStatus(remoteStatus.ID, remoteStatus, client.RequestOptions{}) - if err != nil { - t.Errorf("cannot update Status: %v - alerts: %+v", err, alert.Alerts) - } - - // Retrieve the Status to check Status name got updated - opts.QueryParameters.Del("name") - opts.QueryParameters.Set("id", strconv.Itoa(remoteStatus.ID)) - resp, _, err = TOSession.GetStatuses(opts) - if err != nil { - t.Errorf("cannot get Status '%s' by ID %d: %v - alerts: %+v", *firstStatus.Name, remoteStatus.ID, err, resp.Alerts) - } - if len(resp.Response) != 1 { - t.Fatalf("Expected exactly one Status to exist with ID %d, found: %d", remoteStatus.ID, len(resp.Response)) - } - respStatus := resp.Response[0] - if respStatus.Description != expectedStatusDesc { - t.Errorf("results do not match actual: %s, expected: %s", respStatus.Name, expectedStatusDesc) + for _, status := range testData.Statuses { + if status.Name == nil { + t.Fatal("cannot update test statuses: test data status must have a name") + } + // Retrieve the Status by name so we can get the id for the Update + opts := client.NewRequestOptions() + opts.QueryParameters.Set("name", *status.Name) + resp, _, err := TOSession.GetStatuses(opts) + if err != nil { + t.Errorf("cannot get Status by name '%s': %v - alerts: %+v", *status.Name, err, resp.Alerts) + } + if len(resp.Response) != 1 { + t.Fatalf("Expected exactly one Status to exist with name '%s', found: %d", *status.Name, len(resp.Response)) + } + remoteStatus := resp.Response[0] + expectedStatusDesc := "new description" + remoteStatus.Description = expectedStatusDesc + alert, _, err := TOSession.UpdateStatus(remoteStatus.ID, remoteStatus, client.RequestOptions{}) + + if _, ok := statusNameMap[*status.Name]; ok { + if err == nil { + t.Errorf("expected an error about while updating a reserved status, but got nothing") + } + } else { + if err != nil { + t.Errorf("cannot update Status: %v - alerts: %+v", err, alert.Alerts) + } + + // Retrieve the Status to check Status name got updated + opts.QueryParameters.Del("name") + opts.QueryParameters.Set("id", strconv.Itoa(remoteStatus.ID)) + resp, _, err = TOSession.GetStatuses(opts) + if err != nil { + t.Errorf("cannot get Status '%s' by ID %d: %v - alerts: %+v", *status.Name, remoteStatus.ID, err, resp.Alerts) + } + if len(resp.Response) != 1 { + t.Fatalf("Expected exactly one Status to exist with ID %d, found: %d", remoteStatus.ID, len(resp.Response)) + } + respStatus := resp.Response[0] + if respStatus.Description != expectedStatusDesc { + t.Errorf("results do not match actual: %s, expected: %s", respStatus.Name, expectedStatusDesc) + } + } } - } func GetTestStatuses(t *testing.T) { @@ -232,8 +247,6 @@ func DeleteTestStatuses(t *testing.T) { if status.Name == nil { t.Fatal("cannot get test statuses: test data statuses must have names") } - - // Retrieve the Status by name so we can get the id for the Update opts.QueryParameters.Set("name", *status.Name) resp, _, err := TOSession.GetStatuses(opts) if err != nil { @@ -242,17 +255,44 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp.Response[0] delResp, _, err := TOSession.DeleteStatus(respStatus.ID, client.RequestOptions{}) - if err != nil { - t.Errorf("cannot delete Status: %v - alerts: %+v", err, delResp.Alerts) + if _, ok := statusNameMap[*status.Name]; !ok { + // Retrieve the Status by name so we can get the id for the Update + if err != nil { + t.Errorf("cannot delete Status: %v - alerts: %+v", err, delResp.Alerts) + } + + // Retrieve the Status to see if it got deleted + resp, _, err = TOSession.GetStatuses(opts) + if err != nil { + t.Errorf("Unexpected error getting Statuses filtered by name after deletion: %v - alerts: %+v", err, resp.Alerts) + } + if len(resp.Response) > 0 { + t.Errorf("expected Status '%s' to be deleted, but it was found in Traffic Ops", *status.Name) + } + } else { + if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + } } + } + ForceDeleteStatuses(t) +} - // Retrieve the Status to see if it got deleted - resp, _, err = TOSession.GetStatuses(opts) - if err != nil { - t.Errorf("Unexpected error getting Statuses filtered by name after deletion: %v - alerts: %+v", err, resp.Alerts) - } - if len(resp.Response) > 0 { - t.Errorf("expected Status '%s' to be deleted, but it was found in Traffic Ops", *status.Name) - } +// ForceDeleteStatuses forcibly deletes the statuses from the db. +func ForceDeleteStatuses(t *testing.T) { + + // NOTE: Special circumstances! This should *NOT* be done without a really good reason! + // Connects directly to the DB to remove users rather than going thru the client. + // This is required here because the DeleteUser action does not really delete users, but disables them. + db, err := OpenConnection() + if err != nil { + t.Error("cannot open db") + } + defer db.Close() + + q := `DELETE FROM status` + err = execSQL(db, q) + if err != nil { + t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) } } diff --git a/traffic_ops/traffic_ops_golang/status/statuses.go b/traffic_ops/traffic_ops_golang/status/statuses.go index f5f75d5940..394f9daec3 100644 --- a/traffic_ops/traffic_ops_golang/status/statuses.go +++ b/traffic_ops/traffic_ops_golang/status/statuses.go @@ -40,6 +40,14 @@ type TOStatus struct { tc.StatusNullable } +var statusNameMap = map[tc.CacheStatus]bool{ + tc.CacheStatusAdminDown: true, + tc.CacheStatusOnline: true, + tc.CacheStatusOffline: true, + tc.CacheStatusReported: true, + tc.CacheStatusPreProd: true, +} + func (v *TOStatus) GetLastUpdated() (*time.Time, bool, error) { return api.GetLastUpdated(v.APIInfo().Tx, *v.ID, "status") } @@ -119,9 +127,29 @@ func (st *TOStatus) Read(h http.Header, useIMS bool) ([]interface{}, error, erro return readVals, nil, nil, errCode, maxTime } -func (st *TOStatus) Update(h http.Header) (error, error, int) { return api.GenericUpdate(h, st) } -func (st *TOStatus) Create() (error, error, int) { return api.GenericCreate(st) } -func (st *TOStatus) Delete() (error, error, int) { return api.GenericDelete(st) } +func (st *TOStatus) Update(h http.Header) (error, error, int) { + var statusName string + err := st.APIInfo().Tx.QueryRow(`SELECT name from status WHERE id = $1`, *st.ID).Scan(&statusName) + if err != nil { + return nil, fmt.Errorf("error querying status name from ID: %v", err), http.StatusInternalServerError + } + if _, ok := statusNameMap[tc.CacheStatus(statusName)]; ok { + return fmt.Errorf("cannot modify %s status", statusName), nil, http.StatusForbidden + } + return api.GenericUpdate(h, st) +} +func (st *TOStatus) Create() (error, error, int) { return api.GenericCreate(st) } +func (st *TOStatus) Delete() (error, error, int) { + var statusName string + err := st.APIInfo().Tx.QueryRow(`SELECT name from status WHERE id = $1`, *st.ID).Scan(&statusName) + if err != nil { + return nil, fmt.Errorf("error querying status name from ID: %v", err), http.StatusInternalServerError + } + if _, ok := statusNameMap[tc.CacheStatus(statusName)]; ok { + return fmt.Errorf("cannot delete %s status", statusName), nil, http.StatusForbidden + } + return api.GenericDelete(st) +} func selectQuery() string { return ` From 93c6afdca92b4926305db78c382bf2381fbfad37 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Mon, 9 May 2022 17:50:02 -0600 Subject: [PATCH 2/7] fix tests --- .../testing/ort-tests/tcdata/statuses.go | 54 +++++++++++++++---- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/cache-config/testing/ort-tests/tcdata/statuses.go b/cache-config/testing/ort-tests/tcdata/statuses.go index 81d82b288b..a99a8a4a41 100644 --- a/cache-config/testing/ort-tests/tcdata/statuses.go +++ b/cache-config/testing/ort-tests/tcdata/statuses.go @@ -19,6 +19,14 @@ import ( "testing" ) +var statusNameMap = map[string]bool{ + "ADMIN_DOWN": true, + "ONLINE": true, + "OFFLINE": true, + "REPORTED": true, + "PRE_PROD": true, +} + func (r *TCData) CreateTestStatuses(t *testing.T) { for _, status := range r.TestData.Statuses { @@ -46,17 +54,43 @@ func (r *TCData) DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if err != nil { - t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) - } + if _, ok := statusNameMap[*status.Name]; !ok { + if err != nil { + t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) + } - // Retrieve the Status to see if it got deleted - types, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("error deleting Status name: %v", err) - } - if len(types) > 0 { - t.Errorf("expected Status name: %s to be deleted", *status.Name) + // Retrieve the Status to see if it got deleted + types, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("error deleting Status name: %v", err) + } + if len(types) > 0 { + t.Errorf("expected Status name: %s to be deleted", *status.Name) + } + } else { + if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + } } } + r.ForceDeleteStatuses(t) +} + +// ForceDeleteStatuses forcibly deletes the statuses from the db. +func (r *TCData) ForceDeleteStatuses(t *testing.T) { + + // NOTE: Special circumstances! This should *NOT* be done without a really good reason! + // Connects directly to the DB to remove users rather than going thru the client. + // This is required here because the DeleteUser action does not really delete users, but disables them. + db, err := r.OpenConnection() + if err != nil { + t.Error("cannot open db") + } + defer db.Close() + + q := `DELETE FROM status` + err = execSQL(db, q) + if err != nil { + t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) + } } From aeb9714b7ea42924e11d30bf154fdb553e5a4fbc Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Mon, 9 May 2022 23:16:55 -0600 Subject: [PATCH 3/7] cleanup --- cache-config/testing/ort-tests/tcdata/statuses.go | 4 ++-- lib/go-tc/enum.go | 6 ++---- .../2022050916074300_add_reserved_statuses.down.sql | 1 + .../2022050916074300_add_reserved_statuses.up.sql | 1 + traffic_ops/testing/api/v2/statuses_test.go | 4 ++-- traffic_ops/testing/api/v3/statuses_test.go | 4 ++-- traffic_ops/testing/api/v4/statuses_test.go | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cache-config/testing/ort-tests/tcdata/statuses.go b/cache-config/testing/ort-tests/tcdata/statuses.go index a99a8a4a41..2b49cc96b3 100644 --- a/cache-config/testing/ort-tests/tcdata/statuses.go +++ b/cache-config/testing/ort-tests/tcdata/statuses.go @@ -80,8 +80,8 @@ func (r *TCData) DeleteTestStatuses(t *testing.T) { func (r *TCData) ForceDeleteStatuses(t *testing.T) { // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove users rather than going thru the client. - // This is required here because the DeleteUser action does not really delete users, but disables them. + // Connects directly to the DB to remove statuses rather than going thru the client. + // This is required to delte the special statuses. db, err := r.OpenConnection() if err != nil { t.Error("cannot open db") diff --git a/lib/go-tc/enum.go b/lib/go-tc/enum.go index 5b4464ecd5..8d100fa683 100644 --- a/lib/go-tc/enum.go +++ b/lib/go-tc/enum.go @@ -374,10 +374,8 @@ const ( // by Traffic Monitor. The vast majority of cache servers should have this // Status. CacheStatusReported = CacheStatus("REPORTED") - // TODO: fix this description - // CacheStatusPreProd represents a cache server which is polled for health - // by Traffic Monitor. The vast majority of cache servers should have this - // Status. + // CacheStatusPreProd represents a cache server that is not deployed to "production", + // but is ready for it. CacheStatusPreProd = CacheStatus("PRE_PROD") // CacheStatusInvalid represents an unrecognized Status value. Note that // this is not actually "invalid", because Statuses may have any unique diff --git a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql index 8786aacb86..6f91f69a9c 100644 --- a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql +++ b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql @@ -16,3 +16,4 @@ */ DELETE FROM public.status WHERE name IN ('ONLINE', 'OFFLINE', 'REPORTED', 'ADMIN_DOWN', 'PRE_PROD'); + diff --git a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql index d2315d7936..ddd9a128d3 100644 --- a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql +++ b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.up.sql @@ -20,3 +20,4 @@ INSERT INTO public.status ("name", description) VALUES ('OFFLINE', 'Server is Of INSERT INTO public.status ("name", description) VALUES ('REPORTED', 'Server is online and reported in the health protocol.') ON CONFLICT ("name") DO NOTHING; INSERT INTO public.status ("name", description) VALUES ('ADMIN_DOWN', 'Sever is administrative down and does not receive traffic.') ON CONFLICT ("name") DO NOTHING; INSERT INTO public.status ("name", description) VALUES ('PRE_PROD', 'Pre Production. Not active in any configuration.') ON CONFLICT ("name") DO NOTHING; + diff --git a/traffic_ops/testing/api/v2/statuses_test.go b/traffic_ops/testing/api/v2/statuses_test.go index 9dda1ba63d..80d3f661c3 100644 --- a/traffic_ops/testing/api/v2/statuses_test.go +++ b/traffic_ops/testing/api/v2/statuses_test.go @@ -149,8 +149,8 @@ func DeleteTestStatuses(t *testing.T) { func ForceDeleteStatuses(t *testing.T) { // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove users rather than going thru the client. - // This is required here because the DeleteUser action does not really delete users, but disables them. + // Connects directly to the DB to remove statuses rather than going thru the client. + // This is required to delte the special statuses. db, err := OpenConnection() if err != nil { t.Error("cannot open db") diff --git a/traffic_ops/testing/api/v3/statuses_test.go b/traffic_ops/testing/api/v3/statuses_test.go index 2bd15e85d1..1b1e23ae16 100644 --- a/traffic_ops/testing/api/v3/statuses_test.go +++ b/traffic_ops/testing/api/v3/statuses_test.go @@ -259,8 +259,8 @@ func DeleteTestStatuses(t *testing.T) { func ForceDeleteStatuses(t *testing.T) { // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove users rather than going thru the client. - // This is required here because the DeleteUser action does not really delete users, but disables them. + // Connects directly to the DB to remove statuses rather than going thru the client. + // This is required to delte the special statuses. db, err := OpenConnection() if err != nil { t.Error("cannot open db") diff --git a/traffic_ops/testing/api/v4/statuses_test.go b/traffic_ops/testing/api/v4/statuses_test.go index e9dd5fd52c..fc234948ba 100644 --- a/traffic_ops/testing/api/v4/statuses_test.go +++ b/traffic_ops/testing/api/v4/statuses_test.go @@ -282,8 +282,8 @@ func DeleteTestStatuses(t *testing.T) { func ForceDeleteStatuses(t *testing.T) { // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove users rather than going thru the client. - // This is required here because the DeleteUser action does not really delete users, but disables them. + // Connects directly to the DB to remove statuses rather than going thru the client. + // This is required to delte the special statuses. db, err := OpenConnection() if err != nil { t.Error("cannot open db") From cccb114d9e46884100334cad88572ea95ecedb91 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Tue, 10 May 2022 15:36:29 -0600 Subject: [PATCH 4/7] code review fixes --- .../testing/ort-tests/tcdata/statuses.go | 62 ++++++---------- lib/go-tc/statuses.go | 16 ++++ ...50916074300_add_reserved_statuses.down.sql | 2 - traffic_ops/testing/api/v2/statuses_test.go | 69 +++++++----------- traffic_ops/testing/api/v3/statuses_test.go | 73 +++++++------------ traffic_ops/testing/api/v4/statuses_test.go | 62 ++++++---------- .../traffic_ops_golang/status/statuses.go | 8 +- 7 files changed, 118 insertions(+), 174 deletions(-) diff --git a/cache-config/testing/ort-tests/tcdata/statuses.go b/cache-config/testing/ort-tests/tcdata/statuses.go index 2b49cc96b3..6beabd50da 100644 --- a/cache-config/testing/ort-tests/tcdata/statuses.go +++ b/cache-config/testing/ort-tests/tcdata/statuses.go @@ -17,33 +17,39 @@ package tcdata import ( "testing" -) -var statusNameMap = map[string]bool{ - "ADMIN_DOWN": true, - "ONLINE": true, - "OFFLINE": true, - "REPORTED": true, - "PRE_PROD": true, -} + "github.com/apache/trafficcontrol/lib/go-tc" +) func (r *TCData) CreateTestStatuses(t *testing.T) { + response, _, err := TOSession.GetStatusesWithHdr(nil) + if err != nil { + t.Errorf("could not get statuses: %v", err) + } + statusNameMap := make(map[string]bool, 0) + for _, r := range response { + statusNameMap[r.Name] = true + } + for _, status := range r.TestData.Statuses { - resp, _, err := TOSession.CreateStatusNullable(status) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not CREATE types: %v", err) + if status.Name != nil { + if _, ok := statusNameMap[*status.Name]; !ok { + resp, _, err := TOSession.CreateStatusNullable(status) + t.Log("Response: ", resp) + if err != nil { + t.Errorf("could not CREATE status: %v", err) + } + } } } - } func (r *TCData) DeleteTestStatuses(t *testing.T) { for _, status := range r.TestData.Statuses { if status.Name == nil { - t.Fatal("cannot get ftest statuses: test data statuses must have names") + t.Fatal("cannot get test statuses: test data statuses must have names") } // Retrieve the Status by name so we can get the id for the Update @@ -54,7 +60,7 @@ func (r *TCData) DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if _, ok := statusNameMap[*status.Name]; !ok { + if tc.IsReservedStatus(*status.Name) { if err != nil { t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) } @@ -67,30 +73,8 @@ func (r *TCData) DeleteTestStatuses(t *testing.T) { if len(types) > 0 { t.Errorf("expected Status name: %s to be deleted", *status.Name) } - } else { - if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") - } + } else if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") } } - r.ForceDeleteStatuses(t) -} - -// ForceDeleteStatuses forcibly deletes the statuses from the db. -func (r *TCData) ForceDeleteStatuses(t *testing.T) { - - // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove statuses rather than going thru the client. - // This is required to delte the special statuses. - db, err := r.OpenConnection() - if err != nil { - t.Error("cannot open db") - } - defer db.Close() - - q := `DELETE FROM status` - err = execSQL(db, q) - if err != nil { - t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) - } } diff --git a/lib/go-tc/statuses.go b/lib/go-tc/statuses.go index eafaf1a4e5..0faae6c537 100644 --- a/lib/go-tc/statuses.go +++ b/lib/go-tc/statuses.go @@ -70,3 +70,19 @@ type StatusNullable struct { LastUpdated *TimeNoMod `json:"lastUpdated" db:"last_updated"` Name *string `json:"name" db:"name"` } + +func IsReservedStatus(status string) bool { + switch CacheStatus(status) { + case CacheStatusOffline: + fallthrough + case CacheStatusReported: + fallthrough + case CacheStatusOnline: + fallthrough + case CacheStatusPreProd: + fallthrough + case CacheStatusAdminDown: + return true + } + return false +} diff --git a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql index 6f91f69a9c..6b6a6e9174 100644 --- a/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql +++ b/traffic_ops/app/db/migrations/2022050916074300_add_reserved_statuses.down.sql @@ -15,5 +15,3 @@ * the License. */ -DELETE FROM public.status WHERE name IN ('ONLINE', 'OFFLINE', 'REPORTED', 'ADMIN_DOWN', 'PRE_PROD'); - diff --git a/traffic_ops/testing/api/v2/statuses_test.go b/traffic_ops/testing/api/v2/statuses_test.go index 80d3f661c3..67fcf08ad5 100644 --- a/traffic_ops/testing/api/v2/statuses_test.go +++ b/traffic_ops/testing/api/v2/statuses_test.go @@ -21,14 +21,6 @@ import ( tc "github.com/apache/trafficcontrol/lib/go-tc" ) -var statusNameMap = map[string]bool{ - "ADMIN_DOWN": true, - "ONLINE": true, - "OFFLINE": true, - "REPORTED": true, - "PRE_PROD": true, -} - func TestStatuses(t *testing.T) { WithObjs(t, []TCObj{Parameters, Statuses}, func() { UpdateTestStatuses(t) @@ -37,12 +29,23 @@ func TestStatuses(t *testing.T) { } func CreateTestStatuses(t *testing.T) { - + response, _, err := TOSession.GetStatuses() + if err != nil { + t.Errorf("could not get statuses: %v", err) + } + statusNameMap := make(map[string]bool, 0) + for _, r := range response { + statusNameMap[r.Name] = true + } for _, status := range testData.Statuses { - resp, _, err := TOSession.CreateStatusNullable(status) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not CREATE types: %v", err) + if status.Name != nil { + if _, ok := statusNameMap[*status.Name]; !ok { + resp, _, err := TOSession.CreateStatusNullable(status) + t.Log("Response: ", resp) + if err != nil { + t.Errorf("could not CREATE status: %v", err) + } + } } } @@ -60,7 +63,7 @@ func UpdateTestStatuses(t *testing.T) { // Retrieve the Status by name so we can get the id for the Update resp, _, err := TOSession.GetStatusByName(*status.Name) if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) + t.Errorf("cannot GET Status by name: %s - %v", *status.Name, err) } remoteStatus := resp[0] expectedStatusDesc := "new description" @@ -68,19 +71,19 @@ func UpdateTestStatuses(t *testing.T) { var alert tc.Alerts alert, _, err = TOSession.UpdateStatusByID(remoteStatus.ID, remoteStatus) - if _, ok := statusNameMap[*status.Name]; ok { + if tc.IsReservedStatus(*status.Name) { if err == nil { t.Errorf("expected an error about while updating a reserved status, but got nothing") } } else { if err != nil { - t.Errorf("cannot UPDATE Status by id: %v - %v", err, alert) + t.Errorf("cannot UPDATE Status by id: %d, err: %v - %v", remoteStatus.ID, err, alert) } // Retrieve the Status to check Status name got updated resp, _, err = TOSession.GetStatusByID(remoteStatus.ID) if err != nil { - t.Errorf("cannot GET Status by ID: %v - %v", status.Description, err) + t.Errorf("cannot GET Status by ID: %d - %v", remoteStatus.ID, err) } respStatus := resp[0] if respStatus.Description != expectedStatusDesc { @@ -114,7 +117,7 @@ func DeleteTestStatuses(t *testing.T) { // Retrieve the Status by name so we can get the id for the Update resp, _, err := TOSession.GetStatusByName(*status.Name) if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) + t.Errorf("cannot GET Status by name: %s - %v", *status.Name, err) } if len(resp) != 1 { t.Errorf("Expected exactly one Status to exist with name '%s', found: %d", *status.Name, len(resp)) @@ -123,7 +126,7 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if _, ok := statusNameMap[*status.Name]; !ok { + if !tc.IsReservedStatus(*status.Name) { if err != nil { t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) } @@ -131,35 +134,13 @@ func DeleteTestStatuses(t *testing.T) { // Retrieve the Status to see if it got deleted types, _, err := TOSession.GetStatusByName(*status.Name) if err != nil { - t.Errorf("error deleting Status name: %s", err.Error()) + t.Errorf("error deleting status name: %s, err: %v", *status.Name, err) } if len(types) > 0 { t.Errorf("expected Status name: %s to be deleted", *status.Name) } - } else { - if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") - } + } else if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") } } - ForceDeleteStatuses(t) -} - -// ForceDeleteStatuses forcibly deletes the statuses from the db. -func ForceDeleteStatuses(t *testing.T) { - - // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove statuses rather than going thru the client. - // This is required to delte the special statuses. - db, err := OpenConnection() - if err != nil { - t.Error("cannot open db") - } - defer db.Close() - - q := `DELETE FROM status` - err = execSQL(db, q) - if err != nil { - t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) - } } diff --git a/traffic_ops/testing/api/v3/statuses_test.go b/traffic_ops/testing/api/v3/statuses_test.go index 1b1e23ae16..da4ad0f070 100644 --- a/traffic_ops/testing/api/v3/statuses_test.go +++ b/traffic_ops/testing/api/v3/statuses_test.go @@ -25,14 +25,6 @@ import ( tc "github.com/apache/trafficcontrol/lib/go-tc" ) -var statusNameMap = map[string]bool{ - "ADMIN_DOWN": true, - "ONLINE": true, - "OFFLINE": true, - "REPORTED": true, - "PRE_PROD": true, -} - func TestStatuses(t *testing.T) { WithObjs(t, []TCObj{Parameters, Statuses}, func() { GetTestStatusesIMS(t) @@ -63,11 +55,11 @@ func UpdateTestStatusesWithHeaders(t *testing.T, header http.Header) { if status.Name == nil { t.Fatal("cannot update test statuses: test data status must have a name") } - if _, ok := statusNameMap[*status.Name]; !ok { + if !tc.IsReservedStatus(*status.Name) { // Retrieve the Status by name so we can get the id for the Update resp, _, err := TOSession.GetStatusByNameWithHdr(*status.Name, header) if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) + t.Errorf("cannot GET Status by name: %s - %v", *status.Name, err) } if len(resp) > 0 { remoteStatus := resp[0] @@ -137,15 +129,26 @@ func GetTestStatusesIMS(t *testing.T) { } func CreateTestStatuses(t *testing.T) { + response, _, err := TOSession.GetStatusesWithHdr(nil) + if err != nil { + t.Errorf("could not get statuses: %v", err) + } + statusNameMap := make(map[string]bool, 0) + for _, r := range response { + statusNameMap[r.Name] = true + } for _, status := range testData.Statuses { - resp, _, err := TOSession.CreateStatusNullable(status) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not CREATE types: %v", err) + if status.Name != nil { + if _, ok := statusNameMap[*status.Name]; !ok { + resp, _, err := TOSession.CreateStatusNullable(status) + t.Log("Response: ", resp) + if err != nil { + t.Errorf("could not CREATE status: %v", err) + } + } } } - } func SortTestStatuses(t *testing.T) { @@ -175,7 +178,7 @@ func UpdateTestStatuses(t *testing.T) { // Retrieve the Status by name so we can get the id for the Update resp, _, err := TOSession.GetStatusByName(*status.Name) if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) + t.Errorf("cannot GET Status by name: %s - %v", *status.Name, err) } remoteStatus := resp[0] expectedStatusDesc := "new description" @@ -183,19 +186,19 @@ func UpdateTestStatuses(t *testing.T) { var alert tc.Alerts alert, _, err = TOSession.UpdateStatusByID(remoteStatus.ID, remoteStatus) - if _, ok := statusNameMap[*status.Name]; ok { + if tc.IsReservedStatus(*status.Name) { if err == nil { t.Errorf("expected an error about while updating a reserved status, but got nothing") } } else { if err != nil { - t.Errorf("cannot UPDATE Status by id: %v - %v", err, alert) + t.Errorf("cannot UPDATE Status by id: %d, err: %v - %v", remoteStatus.ID, err, alert) } // Retrieve the Status to check Status name got updated resp, _, err = TOSession.GetStatusByID(remoteStatus.ID) if err != nil { - t.Errorf("cannot GET Status by ID: %v - %v", status.Description, err) + t.Errorf("cannot GET Status by ID: %d - %v", remoteStatus.ID, err) } respStatus := resp[0] if respStatus.Description != expectedStatusDesc { @@ -228,12 +231,12 @@ func DeleteTestStatuses(t *testing.T) { // Retrieve the Status by name so we can get the id for the Update resp, _, err := TOSession.GetStatusByName(*status.Name) if err != nil { - t.Errorf("cannot GET Status by name: %v - %v", status.Name, err) + t.Errorf("cannot GET Status by name: %s - %v", *status.Name, err) } respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if _, ok := statusNameMap[*status.Name]; !ok { + if !tc.IsReservedStatus(*status.Name) { if err != nil { t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) } @@ -241,35 +244,13 @@ func DeleteTestStatuses(t *testing.T) { // Retrieve the Status to see if it got deleted types, _, err := TOSession.GetStatusByName(*status.Name) if err != nil { - t.Errorf("error deleting Status name: %s", err.Error()) + t.Errorf("error deleting status name: %s, err: %v", *status.Name, err) } if len(types) > 0 { t.Errorf("expected Status name: %s to be deleted", *status.Name) } - } else { - if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") - } + } else if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") } } - ForceDeleteStatuses(t) -} - -// ForceDeleteStatuses forcibly deletes the statuses from the db. -func ForceDeleteStatuses(t *testing.T) { - - // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove statuses rather than going thru the client. - // This is required to delte the special statuses. - db, err := OpenConnection() - if err != nil { - t.Error("cannot open db") - } - defer db.Close() - - q := `DELETE FROM status` - err = execSQL(db, q) - if err != nil { - t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) - } } diff --git a/traffic_ops/testing/api/v4/statuses_test.go b/traffic_ops/testing/api/v4/statuses_test.go index fc234948ba..592979a4f3 100644 --- a/traffic_ops/testing/api/v4/statuses_test.go +++ b/traffic_ops/testing/api/v4/statuses_test.go @@ -23,17 +23,10 @@ import ( "time" "github.com/apache/trafficcontrol/lib/go-rfc" + "github.com/apache/trafficcontrol/lib/go-tc" client "github.com/apache/trafficcontrol/traffic_ops/v4-client" ) -var statusNameMap = map[string]bool{ - "ADMIN_DOWN": true, - "ONLINE": true, - "OFFLINE": true, - "REPORTED": true, - "PRE_PROD": true, -} - func TestStatuses(t *testing.T) { WithObjs(t, []TCObj{Parameters, Statuses}, func() { GetTestStatusesIMS(t) @@ -64,7 +57,7 @@ func UpdateTestStatusesWithHeaders(t *testing.T, header http.Header) { if status.Name == nil { t.Fatal("cannot update test statuses: test data status must have a name") } - if _, ok := statusNameMap[*status.Name]; !ok { + if !tc.IsReservedStatus(*status.Name) { // Retrieve the Status by name so we can get the id for the Update opts := client.NewRequestOptions() opts.Header = header @@ -152,13 +145,26 @@ func GetTestStatusesIMS(t *testing.T) { } func CreateTestStatuses(t *testing.T) { + response, _, err := TOSession.GetStatuses(client.NewRequestOptions()) + if err != nil { + t.Errorf("could not get statuses: %v", err) + } + statusNameMap := make(map[string]bool, 0) + for _, r := range response.Response { + statusNameMap[r.Name] = true + } + for _, status := range testData.Statuses { - resp, _, err := TOSession.CreateStatus(status, client.RequestOptions{}) - if err != nil { - t.Errorf("could not create Status: %v - alerts: %+v", err, resp.Alerts) + if status.Name != nil { + if _, ok := statusNameMap[*status.Name]; !ok { + resp, _, err := TOSession.CreateStatus(status, client.NewRequestOptions()) + t.Log("Response: ", resp) + if err != nil { + t.Errorf("could not create Status: %v - alerts: %+v", err, resp.Alerts) + } + } } } - } func SortTestStatuses(t *testing.T) { @@ -200,7 +206,7 @@ func UpdateTestStatuses(t *testing.T) { remoteStatus.Description = expectedStatusDesc alert, _, err := TOSession.UpdateStatus(remoteStatus.ID, remoteStatus, client.RequestOptions{}) - if _, ok := statusNameMap[*status.Name]; ok { + if tc.IsReservedStatus(*status.Name) { if err == nil { t.Errorf("expected an error about while updating a reserved status, but got nothing") } @@ -255,7 +261,7 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp.Response[0] delResp, _, err := TOSession.DeleteStatus(respStatus.ID, client.RequestOptions{}) - if _, ok := statusNameMap[*status.Name]; !ok { + if !tc.IsReservedStatus(*status.Name) { // Retrieve the Status by name so we can get the id for the Update if err != nil { t.Errorf("cannot delete Status: %v - alerts: %+v", err, delResp.Alerts) @@ -269,30 +275,8 @@ func DeleteTestStatuses(t *testing.T) { if len(resp.Response) > 0 { t.Errorf("expected Status '%s' to be deleted, but it was found in Traffic Ops", *status.Name) } - } else { - if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") - } + } else if err == nil { + t.Errorf("expected an error while trying to delete a reserved status, but got nothing") } } - ForceDeleteStatuses(t) -} - -// ForceDeleteStatuses forcibly deletes the statuses from the db. -func ForceDeleteStatuses(t *testing.T) { - - // NOTE: Special circumstances! This should *NOT* be done without a really good reason! - // Connects directly to the DB to remove statuses rather than going thru the client. - // This is required to delte the special statuses. - db, err := OpenConnection() - if err != nil { - t.Error("cannot open db") - } - defer db.Close() - - q := `DELETE FROM status` - err = execSQL(db, q) - if err != nil { - t.Errorf("cannot execute SQL: %s; SQL is %s", err.Error(), q) - } } diff --git a/traffic_ops/traffic_ops_golang/status/statuses.go b/traffic_ops/traffic_ops_golang/status/statuses.go index 394f9daec3..751c10b660 100644 --- a/traffic_ops/traffic_ops_golang/status/statuses.go +++ b/traffic_ops/traffic_ops_golang/status/statuses.go @@ -131,9 +131,9 @@ func (st *TOStatus) Update(h http.Header) (error, error, int) { var statusName string err := st.APIInfo().Tx.QueryRow(`SELECT name from status WHERE id = $1`, *st.ID).Scan(&statusName) if err != nil { - return nil, fmt.Errorf("error querying status name from ID: %v", err), http.StatusInternalServerError + return nil, fmt.Errorf("error querying status name from ID: %w", err), http.StatusInternalServerError } - if _, ok := statusNameMap[tc.CacheStatus(statusName)]; ok { + if tc.IsReservedStatus(statusName) { return fmt.Errorf("cannot modify %s status", statusName), nil, http.StatusForbidden } return api.GenericUpdate(h, st) @@ -143,9 +143,9 @@ func (st *TOStatus) Delete() (error, error, int) { var statusName string err := st.APIInfo().Tx.QueryRow(`SELECT name from status WHERE id = $1`, *st.ID).Scan(&statusName) if err != nil { - return nil, fmt.Errorf("error querying status name from ID: %v", err), http.StatusInternalServerError + return nil, fmt.Errorf("error querying status name from ID: %w", err), http.StatusInternalServerError } - if _, ok := statusNameMap[tc.CacheStatus(statusName)]; ok { + if tc.IsReservedStatus(statusName) { return fmt.Errorf("cannot delete %s status", statusName), nil, http.StatusForbidden } return api.GenericDelete(st) From 019946402384d84d141a67710f5bc99e284ff536 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Tue, 10 May 2022 16:57:29 -0600 Subject: [PATCH 5/7] fix test --- cache-config/testing/ort-tests/tcdata/statuses.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cache-config/testing/ort-tests/tcdata/statuses.go b/cache-config/testing/ort-tests/tcdata/statuses.go index 6beabd50da..ce9311a67e 100644 --- a/cache-config/testing/ort-tests/tcdata/statuses.go +++ b/cache-config/testing/ort-tests/tcdata/statuses.go @@ -60,7 +60,7 @@ func (r *TCData) DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if tc.IsReservedStatus(*status.Name) { + if !tc.IsReservedStatus(*status.Name) { if err != nil { t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) } From 0da842ff6ded5a0672ae380b0a0cf72d825c75dd Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 12 May 2022 15:24:50 -0600 Subject: [PATCH 6/7] code review round 2 --- .../testing/ort-tests/t3c-apply-diff_test.go | 2 +- .../ort-tests/t3c-apply-unset-update_test.go | 2 +- .../t3c-apply-wait-for-parents_test.go | 6 +-- .../ort-tests/t3c-create-empty-file_test.go | 2 +- .../ort-tests/t3c-dns-local-bind_test.go | 2 +- .../testing/ort-tests/t3c-fail-log_test.go | 2 +- .../testing/ort-tests/t3c-git_test.go | 2 +- .../testing/ort-tests/t3c-ims_test.go | 2 +- .../testing/ort-tests/t3c-jobs_test.go | 2 +- .../testing/ort-tests/t3c-lockfile_test.go | 2 +- .../ort-tests/t3c-no-outgoing-ip-flag_test.go | 2 +- .../ort-tests/t3c-os-hostname-short_test.go | 2 +- .../testing/ort-tests/t3c-reload_test.go | 2 +- .../testing/ort-tests/t3c_mode_test.go | 2 +- .../ort-tests/t3c_update_to_flags_test.go | 2 +- .../testing/ort-tests/tc-fixtures.json | 29 ----------- .../testing/ort-tests/tcdata/statuses.go | 48 ++++++------------- cache-config/testing/ort-tests/tcdata/todb.go | 2 +- .../testing/ort-tests/tcdata/withobjs.go | 2 - .../testing/ort-tests/to_requester_test.go | 2 +- .../testing/ort-tests/to_updater_test.go | 2 +- lib/go-tc/statuses.go | 2 + .../tests/health-client-startup_test.go | 2 +- traffic_ops/testing/api/v2/statuses_test.go | 45 ++++++----------- traffic_ops/testing/api/v2/tc-fixtures.json | 20 -------- traffic_ops/testing/api/v2/todb_test.go | 2 +- traffic_ops/testing/api/v3/statuses_test.go | 47 ++++++------------ traffic_ops/testing/api/v3/tc-fixtures.json | 20 -------- traffic_ops/testing/api/v3/todb_test.go | 2 +- traffic_ops/testing/api/v4/statuses_test.go | 46 ++++++------------ traffic_ops/testing/api/v4/tc-fixtures.json | 20 -------- traffic_ops/testing/api/v4/todb_test.go | 2 +- .../traffic_ops_golang/status/statuses.go | 8 ---- 33 files changed, 83 insertions(+), 252 deletions(-) diff --git a/cache-config/testing/ort-tests/t3c-apply-diff_test.go b/cache-config/testing/ort-tests/t3c-apply-diff_test.go index b35e3da372..f7613bc68e 100644 --- a/cache-config/testing/ort-tests/t3c-apply-diff_test.go +++ b/cache-config/testing/ort-tests/t3c-apply-diff_test.go @@ -28,7 +28,7 @@ import ( func TestApplyDiff(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-apply-unset-update_test.go b/cache-config/testing/ort-tests/t3c-apply-unset-update_test.go index 0d3ae688f0..55b95c36f4 100644 --- a/cache-config/testing/ort-tests/t3c-apply-unset-update_test.go +++ b/cache-config/testing/ort-tests/t3c-apply-unset-update_test.go @@ -73,7 +73,7 @@ func verifyUpdateStatusIsTrue() error { func TestT3cUnsetsUpdateFlag(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-apply-wait-for-parents_test.go b/cache-config/testing/ort-tests/t3c-apply-wait-for-parents_test.go index 94df96e26a..4b86042d4d 100644 --- a/cache-config/testing/ort-tests/t3c-apply-wait-for-parents_test.go +++ b/cache-config/testing/ort-tests/t3c-apply-wait-for-parents_test.go @@ -36,7 +36,7 @@ const childCacheHostName = DefaultCacheHostName func TestWaitForParentsTrue(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { @@ -147,7 +147,7 @@ func TestWaitForParentsTrue(t *testing.T) { func TestWaitForParentsDefaultReval(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { @@ -216,7 +216,7 @@ func TestWaitForParentsDefaultReval(t *testing.T) { func TestWaitForParentsFalse(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-create-empty-file_test.go b/cache-config/testing/ort-tests/t3c-create-empty-file_test.go index 9389597c79..48781faf4b 100644 --- a/cache-config/testing/ort-tests/t3c-create-empty-file_test.go +++ b/cache-config/testing/ort-tests/t3c-create-empty-file_test.go @@ -31,7 +31,7 @@ func TestT3cCreateEmptyFile(t *testing.T) { // t3c must create semantically blank files. Failing to do so will cause other config files that reference them to fail. tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-dns-local-bind_test.go b/cache-config/testing/ort-tests/t3c-dns-local-bind_test.go index 247fc53391..be158d885e 100644 --- a/cache-config/testing/ort-tests/t3c-dns-local-bind_test.go +++ b/cache-config/testing/ort-tests/t3c-dns-local-bind_test.go @@ -29,7 +29,7 @@ import ( func TestT3CDNSLocalBind(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-fail-log_test.go b/cache-config/testing/ort-tests/t3c-fail-log_test.go index 98e310412e..47a6a3014c 100644 --- a/cache-config/testing/ort-tests/t3c-fail-log_test.go +++ b/cache-config/testing/ort-tests/t3c-fail-log_test.go @@ -24,7 +24,7 @@ import ( func TestT3cApplyFailMsg(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-git_test.go b/cache-config/testing/ort-tests/t3c-git_test.go index 2250595d83..7f58773dc5 100644 --- a/cache-config/testing/ort-tests/t3c-git_test.go +++ b/cache-config/testing/ort-tests/t3c-git_test.go @@ -31,7 +31,7 @@ import ( func TestT3cGit(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-ims_test.go b/cache-config/testing/ort-tests/t3c-ims_test.go index 7772a539ab..50e27c119a 100644 --- a/cache-config/testing/ort-tests/t3c-ims_test.go +++ b/cache-config/testing/ort-tests/t3c-ims_test.go @@ -34,7 +34,7 @@ import ( func TestIMS(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-jobs_test.go b/cache-config/testing/ort-tests/t3c-jobs_test.go index e3fff5905b..980ee67ce3 100644 --- a/cache-config/testing/ort-tests/t3c-jobs_test.go +++ b/cache-config/testing/ort-tests/t3c-jobs_test.go @@ -29,7 +29,7 @@ import ( func TestT3CJobs(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-lockfile_test.go b/cache-config/testing/ort-tests/t3c-lockfile_test.go index 677d5b2900..6f06a8ff2b 100644 --- a/cache-config/testing/ort-tests/t3c-lockfile_test.go +++ b/cache-config/testing/ort-tests/t3c-lockfile_test.go @@ -27,7 +27,7 @@ import ( func TestLockfile(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-no-outgoing-ip-flag_test.go b/cache-config/testing/ort-tests/t3c-no-outgoing-ip-flag_test.go index 2c74bb92c6..8e14dc8068 100644 --- a/cache-config/testing/ort-tests/t3c-no-outgoing-ip-flag_test.go +++ b/cache-config/testing/ort-tests/t3c-no-outgoing-ip-flag_test.go @@ -55,7 +55,7 @@ func testNoOutgoingIPAfterUpdate(t *testing.T, noOutgoingIP *bool) { func TestT3CNoOutgoingIP(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-os-hostname-short_test.go b/cache-config/testing/ort-tests/t3c-os-hostname-short_test.go index 49d42136f8..5eaace94dc 100644 --- a/cache-config/testing/ort-tests/t3c-os-hostname-short_test.go +++ b/cache-config/testing/ort-tests/t3c-os-hostname-short_test.go @@ -25,7 +25,7 @@ import ( func TestT3cApplyOSHostnameShort(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c-reload_test.go b/cache-config/testing/ort-tests/t3c-reload_test.go index a9705b66c7..5dd390d5a5 100644 --- a/cache-config/testing/ort-tests/t3c-reload_test.go +++ b/cache-config/testing/ort-tests/t3c-reload_test.go @@ -28,7 +28,7 @@ import ( func TestT3cReload(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices, tcdata.InvalidationJobs}, func() { diff --git a/cache-config/testing/ort-tests/t3c_mode_test.go b/cache-config/testing/ort-tests/t3c_mode_test.go index 12cb8d667c..702e14e8e9 100644 --- a/cache-config/testing/ort-tests/t3c_mode_test.go +++ b/cache-config/testing/ort-tests/t3c_mode_test.go @@ -106,7 +106,7 @@ func checkDiff(fName, atsUid, atsGid string, t *testing.T) { func TestT3cBadassAndSyncDs(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/t3c_update_to_flags_test.go b/cache-config/testing/ort-tests/t3c_update_to_flags_test.go index 35ae74f980..39c9dec0ce 100644 --- a/cache-config/testing/ort-tests/t3c_update_to_flags_test.go +++ b/cache-config/testing/ort-tests/t3c_update_to_flags_test.go @@ -25,7 +25,7 @@ import ( func TestT3cTOUpdates(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices, tcdata.InvalidationJobs}, func() { diff --git a/cache-config/testing/ort-tests/tc-fixtures.json b/cache-config/testing/ort-tests/tc-fixtures.json index 4f7098aeb0..54a3677132 100644 --- a/cache-config/testing/ort-tests/tc-fixtures.json +++ b/cache-config/testing/ort-tests/tc-fixtures.json @@ -4535,35 +4535,6 @@ "type": "AAAA_RECORD" } ], - "statuses": [ - { - "description": "Edge: Puts server in CCR config file in this state, but CCR will never route traffic to it. Mid: Server will not be included in parent.config files for its edge caches", - "name": "OFFLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will always route traffic to it. Mid: Server will be included in parent.config files for its edges", - "name": "ONLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will adhere to the health protocol. Mid: N/A for now", - "name": "REPORTED" - }, - { - "description": "Temporary down. Edge: XMPP client will send status OFFLINE to CCR, otherwise similar to REPORTED. Mid: Server will not be included in parent.config files for its edge caches", - "name": "ADMIN_DOWN" - }, - { - "description": "Edge: 12M will not include caches in this state in CCR config files. Mid: N/A for now", - "name": "CCR_IGNORE" - }, - { - "description": "Pre Production. Not active in any configuration.", - "name": "PRE_PROD" - }, - { - "name": "TEST_NULL_DESCRIPTION" - } - ], "tenants": [ { "active": true, diff --git a/cache-config/testing/ort-tests/tcdata/statuses.go b/cache-config/testing/ort-tests/tcdata/statuses.go index ce9311a67e..bb1206df15 100644 --- a/cache-config/testing/ort-tests/tcdata/statuses.go +++ b/cache-config/testing/ort-tests/tcdata/statuses.go @@ -17,30 +17,14 @@ package tcdata import ( "testing" - - "github.com/apache/trafficcontrol/lib/go-tc" ) func (r *TCData) CreateTestStatuses(t *testing.T) { - - response, _, err := TOSession.GetStatusesWithHdr(nil) - if err != nil { - t.Errorf("could not get statuses: %v", err) - } - statusNameMap := make(map[string]bool, 0) - for _, r := range response { - statusNameMap[r.Name] = true - } - for _, status := range r.TestData.Statuses { - if status.Name != nil { - if _, ok := statusNameMap[*status.Name]; !ok { - resp, _, err := TOSession.CreateStatusNullable(status) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not CREATE status: %v", err) - } - } + resp, _, err := TOSession.CreateStatusNullable(status) + t.Log("Response: ", resp) + if err != nil { + t.Errorf("could not CREATE status: %v", err) } } } @@ -60,21 +44,17 @@ func (r *TCData) DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if !tc.IsReservedStatus(*status.Name) { - if err != nil { - t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) - } + if err != nil { + t.Errorf("cannot DELETE Status by ID: %v - %v", err, delResp) + } - // Retrieve the Status to see if it got deleted - types, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("error deleting Status name: %v", err) - } - if len(types) > 0 { - t.Errorf("expected Status name: %s to be deleted", *status.Name) - } - } else if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + // Retrieve the Status to see if it got deleted + statuses, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("error getting Status name: %v", err) + } + if len(statuses) > 0 { + t.Errorf("expected Status name: %s to be deleted", *status.Name) } } } diff --git a/cache-config/testing/ort-tests/tcdata/todb.go b/cache-config/testing/ort-tests/tcdata/todb.go index d1df501774..0bbb27b76c 100644 --- a/cache-config/testing/ort-tests/tcdata/todb.go +++ b/cache-config/testing/ort-tests/tcdata/todb.go @@ -279,7 +279,7 @@ func (r *TCData) Teardown(db *sql.DB) error { DELETE FROM cachegroup; DELETE FROM coordinate; DELETE FROM type; - DELETE FROM status; + DELETE FROM status s WHERE s.name NOT IN ('OFFLINE', 'ONLINE', 'PRE_PROD', 'ADMIN_DOWN', 'REPORTED'); DELETE FROM snapshot; DELETE FROM cdn; DELETE FROM service_category; diff --git a/cache-config/testing/ort-tests/tcdata/withobjs.go b/cache-config/testing/ort-tests/tcdata/withobjs.go index b3bf3337d1..d2460bea3e 100644 --- a/cache-config/testing/ort-tests/tcdata/withobjs.go +++ b/cache-config/testing/ort-tests/tcdata/withobjs.go @@ -49,7 +49,6 @@ const ( ServerServerCapabilities Servers ServiceCategories - Statuses StaticDNSEntries SteeringTargets Tenants @@ -99,7 +98,6 @@ func (r *TCData) WithObjs(t *testing.T, objs []TCObj, f func()) { ServerServerCapabilities: {r.CreateTestServerServerCapabilities, r.DeleteTestServerServerCapabilities}, Servers: {r.CreateTestServers, r.DeleteTestServers}, ServiceCategories: {r.CreateTestServiceCategories, r.DeleteTestServiceCategories}, - Statuses: {r.CreateTestStatuses, r.DeleteTestStatuses}, StaticDNSEntries: {r.CreateTestStaticDNSEntries, r.DeleteTestStaticDNSEntries}, SteeringTargets: {r.SetupSteeringTargets, r.DeleteTestSteeringTargets}, Tenants: {r.CreateTestTenants, r.DeleteTestTenants}, diff --git a/cache-config/testing/ort-tests/to_requester_test.go b/cache-config/testing/ort-tests/to_requester_test.go index cbc1e521f1..982ba47923 100644 --- a/cache-config/testing/ort-tests/to_requester_test.go +++ b/cache-config/testing/ort-tests/to_requester_test.go @@ -35,7 +35,7 @@ type Package struct { func TestTORequester(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices}, func() { diff --git a/cache-config/testing/ort-tests/to_updater_test.go b/cache-config/testing/ort-tests/to_updater_test.go index 5384818e86..efc97cab4e 100644 --- a/cache-config/testing/ort-tests/to_updater_test.go +++ b/cache-config/testing/ort-tests/to_updater_test.go @@ -31,7 +31,7 @@ import ( func TestTOUpdater(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, tcdata.DeliveryServices, tcdata.InvalidationJobs}, func() { diff --git a/lib/go-tc/statuses.go b/lib/go-tc/statuses.go index 0faae6c537..74c847b4a3 100644 --- a/lib/go-tc/statuses.go +++ b/lib/go-tc/statuses.go @@ -71,6 +71,8 @@ type StatusNullable struct { Name *string `json:"name" db:"name"` } +// IsReservedStatus returns true if the passed in status name is reserved, and false if it isn't. +// Currently, the reserved statuses are OFFLINE, ONLINE, REPORTED, PRE_PROD and ADMIN_DOWN. func IsReservedStatus(status string) bool { switch CacheStatus(status) { case CacheStatusOffline: diff --git a/tc-health-client/testing/tests/health-client-startup_test.go b/tc-health-client/testing/tests/health-client-startup_test.go index 60c198bbe8..6d03fa45f3 100644 --- a/tc-health-client/testing/tests/health-client-startup_test.go +++ b/tc-health-client/testing/tests/health-client-startup_test.go @@ -39,7 +39,7 @@ func startHealthClient() { func TestHealthClientStartup(t *testing.T) { tcd.WithObjs(t, []tcdata.TCObj{ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters, - tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses, + tcdata.Profiles, tcdata.ProfileParameters, tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations, tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies, }, func() { diff --git a/traffic_ops/testing/api/v2/statuses_test.go b/traffic_ops/testing/api/v2/statuses_test.go index 67fcf08ad5..9a11866f20 100644 --- a/traffic_ops/testing/api/v2/statuses_test.go +++ b/traffic_ops/testing/api/v2/statuses_test.go @@ -29,26 +29,13 @@ func TestStatuses(t *testing.T) { } func CreateTestStatuses(t *testing.T) { - response, _, err := TOSession.GetStatuses() - if err != nil { - t.Errorf("could not get statuses: %v", err) - } - statusNameMap := make(map[string]bool, 0) - for _, r := range response { - statusNameMap[r.Name] = true - } for _, status := range testData.Statuses { - if status.Name != nil { - if _, ok := statusNameMap[*status.Name]; !ok { - resp, _, err := TOSession.CreateStatusNullable(status) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not CREATE status: %v", err) - } - } + resp, _, err := TOSession.CreateStatusNullable(status) + t.Log("Response: ", resp) + if err != nil { + t.Errorf("could not CREATE status: %v", err) } } - } func UpdateTestStatuses(t *testing.T) { @@ -126,21 +113,17 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if !tc.IsReservedStatus(*status.Name) { - if err != nil { - t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) - } + if err != nil { + t.Errorf("cannot DELETE Status by ID: %v - %v", err, delResp) + } - // Retrieve the Status to see if it got deleted - types, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("error deleting status name: %s, err: %v", *status.Name, err) - } - if len(types) > 0 { - t.Errorf("expected Status name: %s to be deleted", *status.Name) - } - } else if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + // Retrieve the Status to see if it got deleted + statuses, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("error getting status by name: %s, err: %v", *status.Name, err) + } + if len(statuses) > 0 { + t.Errorf("expected Status name: %s to be deleted", *status.Name) } } } diff --git a/traffic_ops/testing/api/v2/tc-fixtures.json b/traffic_ops/testing/api/v2/tc-fixtures.json index cdb6a0cad1..77467ff78c 100644 --- a/traffic_ops/testing/api/v2/tc-fixtures.json +++ b/traffic_ops/testing/api/v2/tc-fixtures.json @@ -2224,30 +2224,10 @@ } ], "statuses": [ - { - "description": "Edge: Puts server in CCR config file in this state, but CCR will never route traffic to it. Mid: Server will not be included in parent.config files for its edge caches", - "name": "OFFLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will always route traffic to it. Mid: Server will be included in parent.config files for its edges", - "name": "ONLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will adhere to the health protocol. Mid: N/A for now", - "name": "REPORTED" - }, - { - "description": "Temporary down. Edge: XMPP client will send status OFFLINE to CCR, otherwise similar to REPORTED. Mid: Server will not be included in parent.config files for its edge caches", - "name": "ADMIN_DOWN" - }, { "description": "Edge: 12M will not include caches in this state in CCR config files. Mid: N/A for now", "name": "CCR_IGNORE" }, - { - "description": "Pre Production. Not active in any configuration.", - "name": "PRE_PROD" - }, { "name": "TEST_NULL_DESCRIPTION" } diff --git a/traffic_ops/testing/api/v2/todb_test.go b/traffic_ops/testing/api/v2/todb_test.go index d8ea4870b0..887bbcdb78 100644 --- a/traffic_ops/testing/api/v2/todb_test.go +++ b/traffic_ops/testing/api/v2/todb_test.go @@ -283,7 +283,7 @@ func Teardown(db *sql.DB) error { DELETE FROM cachegroup; DELETE FROM coordinate; DELETE FROM type; - DELETE FROM status; + DELETE FROM status s WHERE s.name NOT IN ('OFFLINE', 'ONLINE', 'PRE_PROD', 'ADMIN_DOWN', 'REPORTED'); DELETE FROM snapshot; DELETE FROM cdn; DELETE FROM service_category; diff --git a/traffic_ops/testing/api/v3/statuses_test.go b/traffic_ops/testing/api/v3/statuses_test.go index da4ad0f070..b5ed2b1c7d 100644 --- a/traffic_ops/testing/api/v3/statuses_test.go +++ b/traffic_ops/testing/api/v3/statuses_test.go @@ -129,24 +129,11 @@ func GetTestStatusesIMS(t *testing.T) { } func CreateTestStatuses(t *testing.T) { - response, _, err := TOSession.GetStatusesWithHdr(nil) - if err != nil { - t.Errorf("could not get statuses: %v", err) - } - statusNameMap := make(map[string]bool, 0) - for _, r := range response { - statusNameMap[r.Name] = true - } - for _, status := range testData.Statuses { - if status.Name != nil { - if _, ok := statusNameMap[*status.Name]; !ok { - resp, _, err := TOSession.CreateStatusNullable(status) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not CREATE status: %v", err) - } - } + resp, _, err := TOSession.CreateStatusNullable(status) + t.Log("Response: ", resp) + if err != nil { + t.Errorf("could not CREATE status: %v", err) } } } @@ -225,7 +212,7 @@ func DeleteTestStatuses(t *testing.T) { for _, status := range testData.Statuses { if status.Name == nil { - t.Fatal("cannot get ftest statuses: test data statuses must have names") + t.Fatal("cannot get test statuses: test data statuses must have names") } // Retrieve the Status by name so we can get the id for the Update @@ -236,21 +223,17 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp[0] delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if !tc.IsReservedStatus(*status.Name) { - if err != nil { - t.Errorf("cannot DELETE Status by name: %v - %v", err, delResp) - } + if err != nil { + t.Errorf("cannot DELETE Status by ID: %v - %v", err, delResp) + } - // Retrieve the Status to see if it got deleted - types, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("error deleting status name: %s, err: %v", *status.Name, err) - } - if len(types) > 0 { - t.Errorf("expected Status name: %s to be deleted", *status.Name) - } - } else if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + // Retrieve the Status to see if it got deleted + statuses, _, err := TOSession.GetStatusByName(*status.Name) + if err != nil { + t.Errorf("error getting status by name: %s, err: %v", *status.Name, err) + } + if len(statuses) > 0 { + t.Errorf("expected Status name: %s to be deleted", *status.Name) } } } diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json b/traffic_ops/testing/api/v3/tc-fixtures.json index d9b962a512..39671def48 100644 --- a/traffic_ops/testing/api/v3/tc-fixtures.json +++ b/traffic_ops/testing/api/v3/tc-fixtures.json @@ -4785,30 +4785,10 @@ } ], "statuses": [ - { - "description": "Edge: Puts server in CCR config file in this state, but CCR will never route traffic to it. Mid: Server will not be included in parent.config files for its edge caches", - "name": "OFFLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will always route traffic to it. Mid: Server will be included in parent.config files for its edges", - "name": "ONLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will adhere to the health protocol. Mid: N/A for now", - "name": "REPORTED" - }, - { - "description": "Temporary down. Edge: XMPP client will send status OFFLINE to CCR, otherwise similar to REPORTED. Mid: Server will not be included in parent.config files for its edge caches", - "name": "ADMIN_DOWN" - }, { "description": "Edge: 12M will not include caches in this state in CCR config files. Mid: N/A for now", "name": "CCR_IGNORE" }, - { - "description": "Pre Production. Not active in any configuration.", - "name": "PRE_PROD" - }, { "name": "TEST_NULL_DESCRIPTION" } diff --git a/traffic_ops/testing/api/v3/todb_test.go b/traffic_ops/testing/api/v3/todb_test.go index 26a7539cc4..f42ae00ad4 100644 --- a/traffic_ops/testing/api/v3/todb_test.go +++ b/traffic_ops/testing/api/v3/todb_test.go @@ -285,7 +285,7 @@ func Teardown(db *sql.DB) error { DELETE FROM cachegroup; DELETE FROM coordinate; DELETE FROM type; - DELETE FROM status; + DELETE FROM status s WHERE s.name NOT IN ('OFFLINE', 'ONLINE', 'PRE_PROD', 'ADMIN_DOWN', 'REPORTED'); DELETE FROM snapshot; DELETE FROM cdn; DELETE FROM service_category; diff --git a/traffic_ops/testing/api/v4/statuses_test.go b/traffic_ops/testing/api/v4/statuses_test.go index 592979a4f3..90958e94e5 100644 --- a/traffic_ops/testing/api/v4/statuses_test.go +++ b/traffic_ops/testing/api/v4/statuses_test.go @@ -145,24 +145,10 @@ func GetTestStatusesIMS(t *testing.T) { } func CreateTestStatuses(t *testing.T) { - response, _, err := TOSession.GetStatuses(client.NewRequestOptions()) - if err != nil { - t.Errorf("could not get statuses: %v", err) - } - statusNameMap := make(map[string]bool, 0) - for _, r := range response.Response { - statusNameMap[r.Name] = true - } - for _, status := range testData.Statuses { - if status.Name != nil { - if _, ok := statusNameMap[*status.Name]; !ok { - resp, _, err := TOSession.CreateStatus(status, client.NewRequestOptions()) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not create Status: %v - alerts: %+v", err, resp.Alerts) - } - } + resp, _, err := TOSession.CreateStatus(status, client.RequestOptions{}) + if err != nil { + t.Errorf("could not create Status: %v - alerts: %+v", err, resp.Alerts) } } } @@ -253,6 +239,7 @@ func DeleteTestStatuses(t *testing.T) { if status.Name == nil { t.Fatal("cannot get test statuses: test data statuses must have names") } + // Retrieve the Status by name so we can get the id for the Update opts.QueryParameters.Set("name", *status.Name) resp, _, err := TOSession.GetStatuses(opts) if err != nil { @@ -261,22 +248,17 @@ func DeleteTestStatuses(t *testing.T) { respStatus := resp.Response[0] delResp, _, err := TOSession.DeleteStatus(respStatus.ID, client.RequestOptions{}) - if !tc.IsReservedStatus(*status.Name) { - // Retrieve the Status by name so we can get the id for the Update - if err != nil { - t.Errorf("cannot delete Status: %v - alerts: %+v", err, delResp.Alerts) - } + if err != nil { + t.Errorf("cannot delete Status: %v - alerts: %+v", err, delResp.Alerts) + } - // Retrieve the Status to see if it got deleted - resp, _, err = TOSession.GetStatuses(opts) - if err != nil { - t.Errorf("Unexpected error getting Statuses filtered by name after deletion: %v - alerts: %+v", err, resp.Alerts) - } - if len(resp.Response) > 0 { - t.Errorf("expected Status '%s' to be deleted, but it was found in Traffic Ops", *status.Name) - } - } else if err == nil { - t.Errorf("expected an error while trying to delete a reserved status, but got nothing") + // Retrieve the Status to see if it got deleted + resp, _, err = TOSession.GetStatuses(opts) + if err != nil { + t.Errorf("Unexpected error getting Statuses filtered by name after deletion: %v - alerts: %+v", err, resp.Alerts) + } + if len(resp.Response) > 0 { + t.Errorf("expected Status '%s' to be deleted, but it was found in Traffic Ops", *status.Name) } } } diff --git a/traffic_ops/testing/api/v4/tc-fixtures.json b/traffic_ops/testing/api/v4/tc-fixtures.json index 632153f725..2956a60cf1 100644 --- a/traffic_ops/testing/api/v4/tc-fixtures.json +++ b/traffic_ops/testing/api/v4/tc-fixtures.json @@ -5437,30 +5437,10 @@ } ], "statuses": [ - { - "description": "Edge: Puts server in CCR config file in this state, but CCR will never route traffic to it. Mid: Server will not be included in parent.config files for its edge caches", - "name": "OFFLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will always route traffic to it. Mid: Server will be included in parent.config files for its edges", - "name": "ONLINE" - }, - { - "description": "Edge: Puts server in CCR config file in this state, and CCR will adhere to the health protocol. Mid: N/A for now", - "name": "REPORTED" - }, - { - "description": "Temporary down. Edge: XMPP client will send status OFFLINE to CCR, otherwise similar to REPORTED. Mid: Server will not be included in parent.config files for its edge caches", - "name": "ADMIN_DOWN" - }, { "description": "Edge: 12M will not include caches in this state in CCR config files. Mid: N/A for now", "name": "CCR_IGNORE" }, - { - "description": "Pre Production. Not active in any configuration.", - "name": "PRE_PROD" - }, { "name": "TEST_NULL_DESCRIPTION" } diff --git a/traffic_ops/testing/api/v4/todb_test.go b/traffic_ops/testing/api/v4/todb_test.go index 697f35df36..7f9104a8a3 100644 --- a/traffic_ops/testing/api/v4/todb_test.go +++ b/traffic_ops/testing/api/v4/todb_test.go @@ -417,7 +417,7 @@ func Teardown(db *sql.DB) error { DELETE FROM cachegroup; DELETE FROM coordinate; DELETE FROM type; - DELETE FROM status; + DELETE FROM status s WHERE s.name NOT IN ('OFFLINE', 'ONLINE', 'PRE_PROD', 'ADMIN_DOWN', 'REPORTED'); DELETE FROM snapshot; DELETE FROM cdn; DELETE FROM service_category; diff --git a/traffic_ops/traffic_ops_golang/status/statuses.go b/traffic_ops/traffic_ops_golang/status/statuses.go index 751c10b660..875edce434 100644 --- a/traffic_ops/traffic_ops_golang/status/statuses.go +++ b/traffic_ops/traffic_ops_golang/status/statuses.go @@ -40,14 +40,6 @@ type TOStatus struct { tc.StatusNullable } -var statusNameMap = map[tc.CacheStatus]bool{ - tc.CacheStatusAdminDown: true, - tc.CacheStatusOnline: true, - tc.CacheStatusOffline: true, - tc.CacheStatusReported: true, - tc.CacheStatusPreProd: true, -} - func (v *TOStatus) GetLastUpdated() (*time.Time, bool, error) { return api.GetLastUpdated(v.APIInfo().Tx, *v.ID, "status") } From ff572eea5a67b9311b74a68d637aed2684a109a1 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 12 May 2022 15:56:00 -0600 Subject: [PATCH 7/7] remove unused file --- .../testing/ort-tests/tcdata/statuses.go | 60 ------------------- 1 file changed, 60 deletions(-) delete mode 100644 cache-config/testing/ort-tests/tcdata/statuses.go diff --git a/cache-config/testing/ort-tests/tcdata/statuses.go b/cache-config/testing/ort-tests/tcdata/statuses.go deleted file mode 100644 index bb1206df15..0000000000 --- a/cache-config/testing/ort-tests/tcdata/statuses.go +++ /dev/null @@ -1,60 +0,0 @@ -package tcdata - -/* - - 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 ( - "testing" -) - -func (r *TCData) CreateTestStatuses(t *testing.T) { - for _, status := range r.TestData.Statuses { - resp, _, err := TOSession.CreateStatusNullable(status) - t.Log("Response: ", resp) - if err != nil { - t.Errorf("could not CREATE status: %v", err) - } - } -} - -func (r *TCData) DeleteTestStatuses(t *testing.T) { - - for _, status := range r.TestData.Statuses { - if status.Name == nil { - t.Fatal("cannot get test statuses: test data statuses must have names") - } - - // Retrieve the Status by name so we can get the id for the Update - resp, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("cannot GET Status by name: %s - %v", *status.Name, err) - } - respStatus := resp[0] - - delResp, _, err := TOSession.DeleteStatusByID(respStatus.ID) - if err != nil { - t.Errorf("cannot DELETE Status by ID: %v - %v", err, delResp) - } - - // Retrieve the Status to see if it got deleted - statuses, _, err := TOSession.GetStatusByName(*status.Name) - if err != nil { - t.Errorf("error getting Status name: %v", err) - } - if len(statuses) > 0 { - t.Errorf("expected Status name: %s to be deleted", *status.Name) - } - } -}