From 2a9b2d4026206094501ccd1bb133c53110e8ec55 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 19 Oct 2021 13:28:41 +0200 Subject: [PATCH 1/9] fix linting errors found by staticcheck --- configuration/config.go | 8 +- database/database.go | 12 +- database/database_test.go | 17 +- database/roles.go | 7 +- doc/api/responses.go | 2 + helper/amqp.go | 5 +- helper/test_utilities.go | 47 +-- helper/utilities.go | 4 +- .../component-configuration/config_methods.go | 7 +- routes/component-configuration/config_test.go | 30 +- routes/dashboard/dashboard_methods.go | 8 +- routes/dashboard/dashboard_test.go | 8 +- routes/file/file_methods.go | 12 + routes/file/file_s3.go | 13 + routes/file/file_test.go | 13 +- .../infrastructure-component/ic_amqpclient.go | 2 +- .../infrastructure-component/ic_apiquery.go | 273 +++++++++--------- .../infrastructure-component/ic_endpoints.go | 2 +- routes/infrastructure-component/ic_test.go | 51 ++-- .../infrastructure-component/ic_validators.go | 5 +- routes/register.go | 4 +- routes/register_test.go | 3 + routes/result/result_methods.go | 6 +- routes/result/result_test.go | 20 +- routes/scenario/scenario_methods.go | 4 + routes/scenario/scenario_middleware.go | 3 +- routes/scenario/scenario_test.go | 12 +- routes/signal/signal_test.go | 38 ++- routes/user/user_methods.go | 6 +- routes/user/user_middleware.go | 2 +- routes/user/user_test.go | 24 +- routes/user/user_validators.go | 8 +- routes/widget/widget_test.go | 8 +- start.go | 2 +- 34 files changed, 376 insertions(+), 290 deletions(-) diff --git a/configuration/config.go b/configuration/config.go index 1c316e9..fef618f 100644 --- a/configuration/config.go +++ b/configuration/config.go @@ -121,25 +121,25 @@ func InitConfig() error { "k8sCluster": *k8sCluster, } - if *dbClear == true { + if *dbClear { static["db.clear"] = "true" } else { static["db.clear"] = "false" } - if *s3NoSSL == true { + if *s3NoSSL { static["s3.nossl"] = "true" } else { static["s3.nossl"] = "false" } - if *s3PathStyle == true { + if *s3PathStyle { static["s3.pathstyle"] = "true" } else { static["s3.pathstyle"] = "false" } - if *authExternal == true { + if *authExternal { static["auth.external.enabled"] = "true" } else { static["auth.external.enabled"] = "false" diff --git a/database/database.go b/database/database.go index 0ed6435..b0da0e5 100644 --- a/database/database.go +++ b/database/database.go @@ -23,12 +23,13 @@ package database import ( "fmt" - "golang.org/x/crypto/bcrypt" "log" "math/rand" "strings" "time" + "golang.org/x/crypto/bcrypt" + "github.com/jinzhu/gorm" _ "github.com/jinzhu/gorm/dialects/postgres" "github.com/zpatrick/go-config" @@ -120,12 +121,15 @@ func MigrateModels() { } // DBAddAdminUser adds a default admin user to the DB -func DBAddAdminUser(cfg *config.Config) (error, string) { +func DBAddAdminUser(cfg *config.Config) (string, error) { DBpool.AutoMigrate(User{}) // Check if admin user exists in DB var users []User err := DBpool.Where("Role = ?", "Admin").Find(&users).Error + if err != nil { + return "", err + } adminPW := "" adminName := "" @@ -157,10 +161,10 @@ func DBAddAdminUser(cfg *config.Config) (error, string) { // add admin user to DB err = DBpool.Create(&user).Error if err != nil { - return err, "" + return "", err } } - return nil, adminPW + return adminPW, nil } func generatePassword(Len int) string { diff --git a/database/database_test.go b/database/database_test.go index 685f318..1af89ab 100644 --- a/database/database_test.go +++ b/database/database_test.go @@ -22,7 +22,6 @@ package database import ( - "fmt" "os" "testing" @@ -68,12 +67,14 @@ func TestInitDB(t *testing.T) { assert.Error(t, err) dbuser, err := configuration.GlobalConfig.String("db.user") + assert.NoError(t, err) static["db.user"] = dbuser ownconfig = config.NewConfig([]config.Provider{defaults, env}) err = InitDB(ownconfig, "true") assert.Error(t, err) dbpass, err := configuration.GlobalConfig.String("db.pass") + assert.NoError(t, err) static["db.pass"] = dbpass ownconfig = config.NewConfig([]config.Provider{defaults, env}) err = InitDB(ownconfig, "true") @@ -118,7 +119,7 @@ func TestUserAssociations(t *testing.T) { assert.NoError(t, DBpool.Model(&userB).Association("Scenarios").Append(&scenarioA).Error) var usr1 User - assert.NoError(t, DBpool.Find(&usr1, "ID = ?", 1).Error, fmt.Sprintf("Find User with ID=1")) + assert.NoError(t, DBpool.Find(&usr1, "ID = ?", 1).Error, "Find User with ID=1") // Get scenarios of usr1 var scenarios []Scenario @@ -196,7 +197,7 @@ func TestScenarioAssociations(t *testing.T) { assert.NoError(t, DBpool.Model(&scenarioA).Association("Results").Append(&resultB).Error) var scenario1 Scenario - assert.NoError(t, DBpool.Find(&scenario1, 1).Error, fmt.Sprintf("Find Scenario with ID=1")) + assert.NoError(t, DBpool.Find(&scenario1, 1).Error, "Find Scenario with ID=1") // Get users of scenario1 var users []User @@ -263,7 +264,7 @@ func TestICAssociations(t *testing.T) { assert.NoError(t, DBpool.Model(&icA).Association("ComponentConfigurations").Append(&configB).Error) var ic1 InfrastructureComponent - assert.NoError(t, DBpool.Find(&ic1, 1).Error, fmt.Sprintf("Find InfrastructureComponent with ID=1")) + assert.NoError(t, DBpool.Find(&ic1, 1).Error, "Find InfrastructureComponent with ID=1") // Get Component Configurations of ic1 var configs []ComponentConfiguration @@ -314,7 +315,7 @@ func TestComponentConfigurationAssociations(t *testing.T) { assert.NoError(t, DBpool.Model(&icA).Association("ComponentConfigurations").Append(&configB).Error) var config1 ComponentConfiguration - assert.NoError(t, DBpool.Find(&config1, 1).Error, fmt.Sprintf("Find ComponentConfiguration with ID=1")) + assert.NoError(t, DBpool.Find(&config1, 1).Error, "Find ComponentConfiguration with ID=1") // Check IC ID if config1.ICID != 1 { @@ -355,7 +356,7 @@ func TestDashboardAssociations(t *testing.T) { assert.NoError(t, DBpool.Model(&dashboardA).Association("Widgets").Append(&widgetB).Error) var dashboard1 Dashboard - assert.NoError(t, DBpool.Find(&dashboard1, 1).Error, fmt.Sprintf("Find Dashboard with ID=1")) + assert.NoError(t, DBpool.Find(&dashboard1, 1).Error, "Find Dashboard with ID=1") //Get widgets of dashboard1 var widgets []Widget @@ -380,7 +381,7 @@ func TestWidgetAssociations(t *testing.T) { assert.NoError(t, DBpool.Create(&widgetB).Error) var widget1 Widget - assert.NoError(t, DBpool.Find(&widget1, 1).Error, fmt.Sprintf("Find Widget with ID=1")) + assert.NoError(t, DBpool.Find(&widget1, 1).Error, "Find Widget with ID=1") } func TestFileAssociations(t *testing.T) { @@ -401,5 +402,5 @@ func TestFileAssociations(t *testing.T) { assert.NoError(t, DBpool.Create(&fileD).Error) var file1 File - assert.NoError(t, DBpool.Find(&file1, 1).Error, fmt.Sprintf("Find File with ID=1")) + assert.NoError(t, DBpool.Find(&file1, 1).Error, "Find File with ID=1") } diff --git a/database/roles.go b/database/roles.go index 7beeac1..3979891 100644 --- a/database/roles.go +++ b/database/roles.go @@ -66,10 +66,11 @@ type RoleActions map[string]ModelActions // Predefined CRUD operations permissions to be used in Roles var crud = Permission{Create: true, Read: true, Update: true, Delete: true} var _ru_ = Permission{Create: false, Read: true, Update: true, Delete: false} -var __u_ = Permission{Create: false, Read: false, Update: true, Delete: false} var _r__ = Permission{Create: false, Read: true, Update: false, Delete: false} var none = Permission{Create: false, Read: false, Update: false, Delete: false} +// var __u_ = Permission{Create: false, Read: false, Update: true, Delete: false} + // Roles is used as a look up variable to determine if a certain user is // allowed to do a certain action on a given model based on his role var Roles = RoleActions{ @@ -135,12 +136,12 @@ func ValidateRole(c *gin.Context, model ModelName, action CRUD) error { // Get user's role from context role, exists := c.Get(UserRoleCtx) if !exists { - return fmt.Errorf("Request does not contain user's role") + return fmt.Errorf("request does not contain user's role") } // Check if the role can execute the action on the model if !Roles[role.(string)][model][action] { - return fmt.Errorf("Action not allowed for role %v", role) + return fmt.Errorf("action not allowed for role %v", role) } return nil diff --git a/doc/api/responses.go b/doc/api/responses.go index cfcb3f3..139c44f 100644 --- a/doc/api/responses.go +++ b/doc/api/responses.go @@ -21,6 +21,8 @@ *********************************************************************************/ package api +//lint:file-ignore U1000 Ignore all unused code, it's generated + import "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" // This file defines the responses to any endpoint in the backend diff --git a/helper/amqp.go b/helper/amqp.go index ef36844..532c86e 100644 --- a/helper/amqp.go +++ b/helper/amqp.go @@ -209,8 +209,11 @@ func SendActionAMQP(action Action, destinationUUID string) error { false, false, msg) - return PublishAMQP(msg) + if err != nil { + return err + } + return PublishAMQP(msg) } func PublishAMQP(msg amqp.Publishing) error { diff --git a/helper/test_utilities.go b/helper/test_utilities.go index ed655db..470e75e 100644 --- a/helper/test_utilities.go +++ b/helper/test_utilities.go @@ -25,13 +25,14 @@ import ( "bytes" "encoding/json" "fmt" + "log" + "net/http" + "net/http/httptest" + "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" "github.com/gin-gonic/gin" "github.com/nsf/jsondiff" "golang.org/x/crypto/bcrypt" - "log" - "net/http" - "net/http/httptest" ) // data type used in testing @@ -73,8 +74,8 @@ type UserRequest struct { } type Credentials struct { - Username string `json:"username,required"` - Password string `json:"password,required"` + Username string `json:"username"` + Password string `json:"password"` } var AdminCredentials = Credentials{ @@ -105,13 +106,13 @@ var GuestCredentials = Credentials{ func GetResponseID(resp *bytes.Buffer) (int, error) { // Transform bytes buffer into byte slice - respBytes := []byte(resp.String()) + respBytes := resp.Bytes() // Map JSON response to a map[string]map[string]interface{} var respRemapped map[string]map[string]interface{} err := json.Unmarshal(respBytes, &respRemapped) if err != nil { - return 0, fmt.Errorf("Unmarshal failed for respRemapped %v", err) + return 0, fmt.Errorf("failed to unmarshal for respRemapped %v", err) } // Get an arbitrary key from tha map. The only key (entry) of @@ -124,11 +125,11 @@ func GetResponseID(resp *bytes.Buffer) (int, error) { // the conversion to integer before returning id, ok := respRemapped[arbitrary_key]["id"].(float64) if !ok { - return 0, fmt.Errorf("Cannot type assert respRemapped") + return 0, fmt.Errorf("cannot type assert respRemapped") } return int(id), nil } - return 0, fmt.Errorf("GetResponse reached exit") + return 0, fmt.Errorf("getResponse reached exit") } // Return the length of an response in case it is an array @@ -139,7 +140,7 @@ func LengthOfResponse(router *gin.Engine, token string, url string, req, err := http.NewRequest(method, url, nil) if err != nil { - return 0, fmt.Errorf("Failed to create new request: %v", err) + return 0, fmt.Errorf("failed to create new request: %v", err) } req.Header.Set("Content-Type", "application/json") req.Header.Add("Authorization", "Bearer "+token) @@ -152,7 +153,7 @@ func LengthOfResponse(router *gin.Engine, token string, url string, } // Convert the response in array of bytes - responseBytes := []byte(w.Body.String()) + responseBytes := w.Body.Bytes() // First we are trying to unmarshal the response into an array of // general type variables ([]interface{}). If this fails we will try @@ -180,7 +181,7 @@ func LengthOfResponse(router *gin.Engine, token string, url string, } // Failed to identify response. - return 0, fmt.Errorf("Length of response cannot be detected") + return 0, fmt.Errorf("length of response cannot be detected") } // Make a request to an endpoint @@ -192,13 +193,13 @@ func TestEndpoint(router *gin.Engine, token string, url string, // Marshal the HTTP request body body, err := json.Marshal(requestBody) if err != nil { - return 0, nil, fmt.Errorf("Failed to marshal request body: %v", err) + return 0, nil, fmt.Errorf("failed to marshal request body: %v", err) } // Create the request req, err := http.NewRequest(method, url, bytes.NewBuffer(body)) if err != nil { - return 0, nil, fmt.Errorf("Failed to create new request: %v", err) + return 0, nil, fmt.Errorf("failed to create new request: %v", err) } req.Header.Set("Content-Type", "application/json") req.Header.Add("Authorization", "Bearer "+token) @@ -251,14 +252,14 @@ func CompareResponse(resp *bytes.Buffer, expected interface{}) error { // Serialize expected response expectedBytes, err := json.Marshal(expected) if err != nil { - return fmt.Errorf("Failed to marshal expected response: %v", err) + return fmt.Errorf("failed to marshal expected response: %v", err) } // Compare opts := jsondiff.DefaultConsoleOptions() diff, text := jsondiff.Compare(resp.Bytes(), expectedBytes, &opts) if diff.String() != "FullMatch" && diff.String() != "SupersetMatch" { log.Println(text) - return fmt.Errorf("Response: Expected \"%v\". Got \"%v\".", + return fmt.Errorf("response: Expected \"%v\". Got \"%v\"", "(FullMatch OR SupersetMatch)", diff.String()) } @@ -273,25 +274,25 @@ func AuthenticateForTest(router *gin.Engine, credentials interface{}) (string, e // Marshal credentials body, err := json.Marshal(credentials) if err != nil { - return "", fmt.Errorf("Failed to marshal credentials: %v", err) + return "", fmt.Errorf("failed to marshal credentials: %v", err) } req, err := http.NewRequest("POST", "/api/v2/authenticate/internal", bytes.NewBuffer(body)) if err != nil { - return "", fmt.Errorf("Failed to create new request: %v", err) + return "", fmt.Errorf("failed to create new request: %v", err) } req.Header.Set("Content-Type", "application/json") router.ServeHTTP(w, req) // Check that return HTTP Code is 200 (OK) if w.Code != http.StatusOK { - return "", fmt.Errorf("HTTP Code: Expected \"%v\". Got \"%v\".", + return "", fmt.Errorf("http code: Expected \"%v\". Got \"%v\"", http.StatusOK, w.Code) } // Get the response var body_data map[string]interface{} - err = json.Unmarshal([]byte(w.Body.String()), &body_data) + err = json.Unmarshal(w.Body.Bytes(), &body_data) if err != nil { return "", err } @@ -299,16 +300,16 @@ func AuthenticateForTest(router *gin.Engine, credentials interface{}) (string, e // Check the response success, ok := body_data["success"].(bool) if !ok { - return "", fmt.Errorf("Type asssertion of response[\"success\"] failed") + return "", fmt.Errorf("type asssertion of response[\"success\"] failed") } if !success { - return "", fmt.Errorf("Authentication failed: %v", body_data["message"]) + return "", fmt.Errorf("authentication failed: %v", body_data["message"]) } // Extract the token token, ok := body_data["token"].(string) if !ok { - return "", fmt.Errorf("Type assertion of response[\"token\"] failed") + return "", fmt.Errorf("type assertion of response[\"token\"] failed") } // Return the token and nil error diff --git a/helper/utilities.go b/helper/utilities.go index 4cb600e..233416c 100644 --- a/helper/utilities.go +++ b/helper/utilities.go @@ -33,14 +33,14 @@ func GetIDOfElement(c *gin.Context, elementName string, source string, providedI if source == "path" { id, err := strconv.Atoi(c.Param(elementName)) if err != nil { - BadRequestError(c, fmt.Sprintf("No or incorrect format of path parameter")) + BadRequestError(c, "No or incorrect format of path parameter") return -1, err } return id, nil } else if source == "query" { id, err := strconv.Atoi(c.Request.URL.Query().Get(elementName)) if err != nil { - BadRequestError(c, fmt.Sprintf("No or incorrect format of query parameter")) + BadRequestError(c, "No or incorrect format of query parameter") return -1, err } return id, nil diff --git a/routes/component-configuration/config_methods.go b/routes/component-configuration/config_methods.go index 6f416a8..ce615d8 100644 --- a/routes/component-configuration/config_methods.go +++ b/routes/component-configuration/config_methods.go @@ -22,9 +22,10 @@ package component_configuration import ( + "log" + "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" "git.rwth-aachen.de/acs/public/villas/web-backend-go/routes/scenario" - "log" ) type ComponentConfiguration struct { @@ -149,7 +150,7 @@ func (m *ComponentConfiguration) delete() error { if err != nil { return err } - for sig, _ := range InputMappingSignals { + for sig := range InputMappingSignals { err = db.Delete(&sig).Error if err != nil { return err @@ -162,7 +163,7 @@ func (m *ComponentConfiguration) delete() error { if err != nil { return err } - for sig, _ := range OutputMappingSignals { + for sig := range OutputMappingSignals { err = db.Delete(&sig).Error if err != nil { return err diff --git a/routes/component-configuration/config_test.go b/routes/component-configuration/config_test.go index f99252e..eadb004 100644 --- a/routes/component-configuration/config_test.go +++ b/routes/component-configuration/config_test.go @@ -85,16 +85,18 @@ func addScenarioAndIC() (scenarioID uint, ICID uint) { // POST $newICA newICA := ICRequest{ - UUID: "7be0322d-354e-431e-84bd-ae4c9633138b", - WebsocketURL: "https://villas.k8s.eonerc.rwth-aachen.de/ws/ws_sig", - Type: "villas-node", - Name: "ACS Demo Signals", - Category: "gateway", - State: "idle", - Location: "k8s", - Description: "A signal generator for testing purposes", - StartParameterSchema: postgres.Jsonb{json.RawMessage(`{"prop1" : "a nice prop"}`)}, - ManagedExternally: newFalse(), + UUID: "7be0322d-354e-431e-84bd-ae4c9633138b", + WebsocketURL: "https://villas.k8s.eonerc.rwth-aachen.de/ws/ws_sig", + Type: "villas-node", + Name: "ACS Demo Signals", + Category: "gateway", + State: "idle", + Location: "k8s", + Description: "A signal generator for testing purposes", + StartParameterSchema: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"prop1" : "a nice prop"}`), + }, + ManagedExternally: newFalse(), } code, resp, err := helper.TestEndpoint(router, token, @@ -118,8 +120,10 @@ func addScenarioAndIC() (scenarioID uint, ICID uint) { // POST $newScenario newScenario := ScenarioRequest{ - Name: "Scenario1", - StartParameters: postgres.Jsonb{json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`)}, + Name: "Scenario1", + StartParameters: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`), + }, } code, resp, err = helper.TestEndpoint(router, token, "/api/v2/scenarios", "POST", helper.KeyModels{"scenario": newScenario}) @@ -131,7 +135,7 @@ func addScenarioAndIC() (scenarioID uint, ICID uint) { newScenarioID, _ := helper.GetResponseID(resp) // add the guest user to the new scenario - _, resp, _ = helper.TestEndpoint(router, token, + _, _, _ = helper.TestEndpoint(router, token, fmt.Sprintf("/api/v2/scenarios/%v/user?username=User_C", newScenarioID), "PUT", nil) return uint(newScenarioID), uint(newICID) diff --git a/routes/dashboard/dashboard_methods.go b/routes/dashboard/dashboard_methods.go index 913b93c..bfde2f0 100644 --- a/routes/dashboard/dashboard_methods.go +++ b/routes/dashboard/dashboard_methods.go @@ -89,6 +89,9 @@ func (d *Dashboard) delete() error { // remove association between Dashboard and Scenario err = db.Model(&sim).Association("Dashboards").Delete(d).Error + if err != nil { + return err + } // get all widgets of the dashboard var widgets []database.Widget @@ -98,8 +101,11 @@ func (d *Dashboard) delete() error { } // Delete widgets - for widget, _ := range widgets { + for widget := range widgets { err = db.Delete(&widget).Error + if err != nil { + return err + } } // Delete dashboard diff --git a/routes/dashboard/dashboard_test.go b/routes/dashboard/dashboard_test.go index 512eac0..a2f1b61 100644 --- a/routes/dashboard/dashboard_test.go +++ b/routes/dashboard/dashboard_test.go @@ -61,8 +61,10 @@ func addScenario(token string) (scenarioID uint) { // POST $newScenario newScenario := ScenarioRequest{ - Name: "Scenario1", - StartParameters: postgres.Jsonb{json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`)}, + Name: "Scenario1", + StartParameters: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`), + }, } _, resp, err := helper.TestEndpoint(router, token, "/api/v2/scenarios", "POST", helper.KeyModels{"scenario": newScenario}) @@ -74,7 +76,7 @@ func addScenario(token string) (scenarioID uint) { newScenarioID, _ := helper.GetResponseID(resp) // add the guest user to the new scenario - _, resp, _ = helper.TestEndpoint(router, token, + _, _, _ = helper.TestEndpoint(router, token, fmt.Sprintf("/api/v2/scenarios/%v/user?username=User_C", newScenarioID), "PUT", nil) return uint(newScenarioID) diff --git a/routes/file/file_methods.go b/routes/file/file_methods.go index 788d123..9f17181 100644 --- a/routes/file/file_methods.go +++ b/routes/file/file_methods.go @@ -99,8 +99,14 @@ func (f *File) Register(fileHeader *multipart.FileHeader, scenarioID uint) error defer fileContent.Close() bucket, err := configuration.GlobalConfig.String("s3.bucket") + if err != nil { + return err + } if bucket == "" { f.FileData, err = ioutil.ReadAll(fileContent) + if err != nil { + return err + } f.Key = "" } else { err := f.putS3(fileContent) @@ -161,8 +167,14 @@ func (f *File) update(fileHeader *multipart.FileHeader) error { defer fileContent.Close() bucket, err := configuration.GlobalConfig.String("s3.bucket") + if err != nil { + return err + } if bucket == "" { f.FileData, err = ioutil.ReadAll(fileContent) + if err != nil { + return err + } f.Key = "" } else { err := f.putS3(fileContent) diff --git a/routes/file/file_s3.go b/routes/file/file_s3.go index 047a279..3a247fd 100644 --- a/routes/file/file_s3.go +++ b/routes/file/file_s3.go @@ -58,9 +58,21 @@ func getS3Session() (*session.Session, string, error) { func createS3Session() (*session.Session, error) { endpoint, err := configuration.GlobalConfig.String("s3.endpoint") + if err != nil { + return nil, err + } region, err := configuration.GlobalConfig.String("s3.region") + if err != nil { + return nil, err + } pathStyle, err := configuration.GlobalConfig.Bool("s3.pathstyle") + if err != nil { + return nil, err + } nossl, err := configuration.GlobalConfig.Bool("s3.nossl") + if err != nil { + return nil, err + } sess, err := session.NewSession( &aws.Config{ @@ -134,6 +146,7 @@ func (f *File) getS3Url() (string, error) { return urlStr, nil } +//lint:ignore U1000 will be used later func (f *File) deleteS3() error { // The session the S3 Uploader will use diff --git a/routes/file/file_test.go b/routes/file/file_test.go index 948ebda..9c66f6e 100644 --- a/routes/file/file_test.go +++ b/routes/file/file_test.go @@ -53,10 +53,10 @@ type ScenarioRequest struct { func addScenario() (scenarioID uint) { // authenticate as admin - token, _ := helper.AuthenticateForTest(router, helper.AdminCredentials) + _, _ = helper.AuthenticateForTest(router, helper.AdminCredentials) // authenticate as normal user - token, _ = helper.AuthenticateForTest(router, helper.UserACredentials) + token, _ := helper.AuthenticateForTest(router, helper.UserACredentials) // POST $newScenario newScenario := ScenarioRequest{ @@ -70,7 +70,7 @@ func addScenario() (scenarioID uint) { newScenarioID, _ := helper.GetResponseID(resp) // add the guest user to the new scenario - _, resp, _ = helper.TestEndpoint(router, token, + _, _, _ = helper.TestEndpoint(router, token, fmt.Sprintf("/api/v2/scenarios/%v/user?username=User_C", newScenarioID), "PUT", nil) return uint(newScenarioID) @@ -129,7 +129,7 @@ func TestAddFile(t *testing.T) { // try to POST without a scenario ID // should return a bad request error code, resp, err = helper.TestEndpoint(router, token, - fmt.Sprintf("/api/v2/files"), "POST", emptyBuf) + "/api/v2/files", "POST", emptyBuf) assert.NoError(t, err) assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) @@ -304,6 +304,7 @@ func TestUpdateFile(t *testing.T) { assert.Equalf(t, 200, w_updated.Code, "Response body: \n%v\n", w_updated.Body) newFileIDUpdated, err := helper.GetResponseID(w_updated.Body) + assert.NoError(t, err) assert.Equal(t, newFileID, newFileIDUpdated) @@ -407,7 +408,7 @@ func TestDeleteFile(t *testing.T) { // try to DELETE non-existing fileID // should return not found code, resp, err = helper.TestEndpoint(router, token, - fmt.Sprintf("/api/v2/files/5"), "DELETE", nil) + "/api/v2/files/5", "DELETE", nil) assert.NoError(t, err) assert.Equalf(t, 404, code, "Response body: \n%v\n", resp) @@ -473,7 +474,7 @@ func TestGetAllFilesOfScenario(t *testing.T) { //try to get all files with missing scenario ID; should return a bad request error code, resp, err = helper.TestEndpoint(router, token, - fmt.Sprintf("/api/v2/files"), "GET", nil) + "/api/v2/files", "GET", nil) assert.NoError(t, err) assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) diff --git a/routes/infrastructure-component/ic_amqpclient.go b/routes/infrastructure-component/ic_amqpclient.go index 3f129ab..ee81693 100644 --- a/routes/infrastructure-component/ic_amqpclient.go +++ b/routes/infrastructure-component/ic_amqpclient.go @@ -104,7 +104,7 @@ func ProcessMessage(message amqp.Delivery) error { ICUUID := payload.Properties.UUID _, err = uuid.Parse(ICUUID) if err != nil { - return fmt.Errorf("AMQP: UUID not valid: %v, message ignored: %v \n", ICUUID, string(message.Body)) + return fmt.Errorf("amqp: UUID not valid: %v, message ignored: %vi", ICUUID, string(message.Body)) } var sToBeUpdated InfrastructureComponent diff --git a/routes/infrastructure-component/ic_apiquery.go b/routes/infrastructure-component/ic_apiquery.go index 36b20fc..32d00c6 100644 --- a/routes/infrastructure-component/ic_apiquery.go +++ b/routes/infrastructure-component/ic_apiquery.go @@ -25,23 +25,21 @@ package infrastructure_component import ( "encoding/json" "fmt" - "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" - "github.com/go-resty/resty/v2" - "github.com/jinzhu/gorm/dialects/postgres" "log" "strconv" "strings" "time" + + "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" + "github.com/go-resty/resty/v2" + "github.com/jinzhu/gorm/dialects/postgres" ) func QueryICAPIs(d time.Duration) { - client := resty.New() - //client.SetDebug(true) - go func() { - for _ = range time.Tick(d) { + for range time.Tick(d) { //log.Println("Querying IC APIs at time:", x) var err error @@ -55,134 +53,143 @@ func QueryICAPIs(d time.Duration) { // iterate over ICs in DB for _, ic := range ics { - - if ic.ManagedExternally { - continue - } - - if ic.APIURL == "" || (!strings.HasPrefix(ic.APIURL, "http://") && !strings.HasPrefix(ic.APIURL, "https://")) { - continue - } - - if ic.Category == "gateway" && ic.Type == "villas-node" { - - log.Println("External API: checking for villas-node gateway", ic.Name) - statusResponse, err := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL + "/status") - if err != nil { - log.Println("Error querying status of", ic.Name, err) - continue - } - var status map[string]interface{} - err = json.Unmarshal(statusResponse.Body(), &status) - if err != nil { - log.Println("Error unmarshalling status of", ic.Name, err) - continue - } - - parts := strings.Split(ic.WebsocketURL, "/") - if len(parts) > 0 && parts[len(parts)-1] != "" { - - configResponse, _ := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL + "/node/" + parts[len(parts)-1]) - statsResponse, _ := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL + "/node/" + parts[len(parts)-1] + "/stats") - - var config map[string]interface{} - err = json.Unmarshal(configResponse.Body(), &config) - if err == nil { - status["config"] = config - } - var stats map[string]interface{} - err = json.Unmarshal(statsResponse.Body(), &stats) - if err == nil { - status["statistics"] = stats - } - } - - var updatedIC UpdateICRequest - statusRaw, _ := json.Marshal(status) - updatedIC.InfrastructureComponent.StatusUpdateRaw = postgres.Jsonb{RawMessage: statusRaw} - updatedIC.InfrastructureComponent.State = fmt.Sprintf("%v", status["state"]) - updatedIC.InfrastructureComponent.UUID = fmt.Sprintf("%v", status["uuid"]) - timeNow, myerr := strconv.ParseFloat(fmt.Sprintf("%v", status["time_now"]), 64) - if myerr != nil { - log.Println("Error parsing time_now to float", myerr.Error()) - continue - } - timeStarted, myerr := strconv.ParseFloat(fmt.Sprintf("%v", status["time_started"]), 64) - if myerr != nil { - log.Println("Error parsing time_started to float", myerr.Error()) - continue - } - uptime := timeNow - timeStarted - updatedIC.InfrastructureComponent.Uptime = uptime - - // validate the update - err = updatedIC.validate() - if err != nil { - log.Println("Error validating updated villas-node gateway", ic.Name, ic.UUID, err.Error()) - continue - } - - // create the update and update IC in DB - var x InfrastructureComponent - err = x.byID(ic.ID) - if err != nil { - log.Println("Error getting villas-node gateway by ID", ic.Name, err) - continue - } - u := updatedIC.updatedIC(x) - err = x.update(u) - if err != nil { - log.Println("Error updating villas-node gateway", ic.Name, ic.UUID, err.Error()) - continue - } - - } else if ic.Category == "manager" && ic.Type == "villas-relay" { - - log.Println("External API: checking for villas-relay manager", ic.Name) - statusResponse, err := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL) - if err != nil { - log.Println("Error querying API of", ic.Name, err) - continue - } - var status map[string]interface{} - err = json.Unmarshal(statusResponse.Body(), &status) - if err != nil { - log.Println("Error unmarshalling status villas-relay manager", ic.Name, err) - continue - } - - var updatedIC UpdateICRequest - statusRaw, _ := json.Marshal(status) - updatedIC.InfrastructureComponent.StatusUpdateRaw = postgres.Jsonb{RawMessage: statusRaw} - updatedIC.InfrastructureComponent.UUID = fmt.Sprintf("%v", status["uuid"]) - - // validate the update - err = updatedIC.validate() - if err != nil { - log.Println("Error validating updated villas-relay manager", ic.Name, ic.UUID, err.Error()) - continue - } - - // create the update and update IC in DB - var x InfrastructureComponent - err = x.byID(ic.ID) - if err != nil { - log.Println("Error getting villas-relay manager by ID", ic.Name, err) - continue - } - u := updatedIC.updatedIC(x) - err = x.update(u) - if err != nil { - log.Println("Error updating villas-relay manager", ic.Name, ic.UUID, err.Error()) - continue - } - - } else if ic.Category == "gateway" && ic.Type == "villas-relay" { - - // TODO add code here once API for VILLASrelay sessions is available - + err := queryIC(&ic) + if err != nil { + fmt.Println(err) } } } }() } + +func queryIC(ic *database.InfrastructureComponent) error { + if ic.ManagedExternally || ic.APIURL == "" || (!strings.HasPrefix(ic.APIURL, "http://") && !strings.HasPrefix(ic.APIURL, "https://")) { + return nil + } + + if ic.Category == "gateway" { + if ic.Type == "villas-node" { + err := queryVillasNodeGateway(ic) + if err != nil { + return err + } + } else if ic.Type == "villas-relay" { + err := queryVillasRelayGateway(ic) + if err != nil { + return err + } + } + } + + return nil +} + +func queryVillasNodeGateway(ic *database.InfrastructureComponent) error { + client := resty.New() + + log.Println("External API: checking for villas-node gateway", ic.Name) + statusResponse, err := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL + "/status") + if err != nil { + return fmt.Errorf("failed to query the status of %s: %w", ic.Name, err) + } + var status map[string]interface{} + err = json.Unmarshal(statusResponse.Body(), &status) + if err != nil { + return fmt.Errorf("failed to unmarshal status of %s: %w", ic.Name, err) + } + + parts := strings.Split(ic.WebsocketURL, "/") + if len(parts) > 0 && parts[len(parts)-1] != "" { + + configResponse, _ := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL + "/node/" + parts[len(parts)-1]) + statsResponse, _ := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL + "/node/" + parts[len(parts)-1] + "/stats") + + var config map[string]interface{} + err = json.Unmarshal(configResponse.Body(), &config) + if err == nil { + status["config"] = config + } + var stats map[string]interface{} + err = json.Unmarshal(statsResponse.Body(), &stats) + if err == nil { + status["statistics"] = stats + } + } + + var updatedIC UpdateICRequest + statusRaw, _ := json.Marshal(status) + updatedIC.InfrastructureComponent.StatusUpdateRaw = postgres.Jsonb{RawMessage: statusRaw} + updatedIC.InfrastructureComponent.State = fmt.Sprintf("%v", status["state"]) + updatedIC.InfrastructureComponent.UUID = fmt.Sprintf("%v", status["uuid"]) + timeNow, err := strconv.ParseFloat(fmt.Sprintf("%v", status["time_now"]), 64) + if err != nil { + return fmt.Errorf("failed to parse time_now to float: %w", err) + } + timeStarted, err := strconv.ParseFloat(fmt.Sprintf("%v", status["time_started"]), 64) + if err != nil { + return fmt.Errorf("failed to parse time_started to float: %w", err) + } + uptime := timeNow - timeStarted + updatedIC.InfrastructureComponent.Uptime = uptime + + // validate the update + err = updatedIC.validate() + if err != nil { + return fmt.Errorf("failed to validate updated villas-node gateway: %s (%s): %w", ic.Name, ic.UUID, err) + } + + // create the update and update IC in DB + var x InfrastructureComponent + err = x.byID(ic.ID) + if err != nil { + return fmt.Errorf("failed to get villas-node gateway by ID %s (%s): %w", ic.Name, ic.UUID, err) + } + u := updatedIC.updatedIC(x) + err = x.update(u) + if err != nil { + return fmt.Errorf("failed to update villas-node gateway %s (%s): %w", ic.Name, ic.UUID, err) + } + + return nil +} + +func queryVillasRelayGateway(ic *database.InfrastructureComponent) error { + client := resty.New() + + log.Println("External API: checking for villas-relay manager", ic.Name) + statusResponse, err := client.R().SetHeader("Accept", "application/json").Get(ic.APIURL) + if err != nil { + return fmt.Errorf("failed querying API of %s (%s): %w", ic.Name, ic.UUID, err) + } + + var status map[string]interface{} + err = json.Unmarshal(statusResponse.Body(), &status) + if err != nil { + return fmt.Errorf("failed to unmarshal status villas-relay manager %s (%s): %w", ic.Name, ic.UUID, err) + } + + var updatedIC UpdateICRequest + statusRaw, _ := json.Marshal(status) + updatedIC.InfrastructureComponent.StatusUpdateRaw = postgres.Jsonb{RawMessage: statusRaw} + updatedIC.InfrastructureComponent.UUID = fmt.Sprintf("%v", status["uuid"]) + + // validate the update + err = updatedIC.validate() + if err != nil { + return fmt.Errorf("failed to validate updated villas-relay manager %s (%s): %w", ic.Name, ic.UUID, err) + } + + // create the update and update IC in DB + var x InfrastructureComponent + err = x.byID(ic.ID) + if err != nil { + return fmt.Errorf("failed to get villas-relay manager by ID %s (%s): %w", ic.Name, ic.UUID, err) + } + u := updatedIC.updatedIC(x) + err = x.update(u) + if err != nil { + return fmt.Errorf("failed to update villas-relay manager %s (%s): %w", ic.Name, ic.UUID, err) + } + + return nil +} diff --git a/routes/infrastructure-component/ic_endpoints.go b/routes/infrastructure-component/ic_endpoints.go index 0f1471c..5c77227 100644 --- a/routes/infrastructure-component/ic_endpoints.go +++ b/routes/infrastructure-component/ic_endpoints.go @@ -102,7 +102,7 @@ func addIC(c *gin.Context) { } // Check if IC to be created is managed externally - if *req.InfrastructureComponent.ManagedExternally == true { + if *req.InfrastructureComponent.ManagedExternally { // if so: refuse creation helper.BadRequestError(c, "create for externally managed IC not possible with this endpoint - use /ic/{ICID}/action endpoint instead to request creation of the component") return diff --git a/routes/infrastructure-component/ic_test.go b/routes/infrastructure-component/ic_test.go index d777d4f..013d897 100644 --- a/routes/infrastructure-component/ic_test.go +++ b/routes/infrastructure-component/ic_test.go @@ -87,8 +87,8 @@ var newIC1 = ICRequest{ State: "idle", Location: "k8s", Description: "A signal generator for testing purposes", - StartParameterSchema: postgres.Jsonb{json.RawMessage(`{"startprop1" : "a nice prop"}`)}, - CreateParameterSchema: postgres.Jsonb{json.RawMessage(`{"createprop1" : "a really nice prop"}`)}, + StartParameterSchema: postgres.Jsonb{RawMessage: json.RawMessage(`{"startprop1" : "a nice prop"}`)}, + CreateParameterSchema: postgres.Jsonb{RawMessage: json.RawMessage(`{"createprop1" : "a really nice prop"}`)}, ManagedExternally: newFalse(), Manager: "7be0322d-354e-431e-84bd-ae4c9633beef", } @@ -103,8 +103,8 @@ var newIC2 = ICRequest{ State: "running", Location: "k8s", Description: "This is a test description", - StartParameterSchema: postgres.Jsonb{json.RawMessage(`{"startprop1" : "a nice prop"}`)}, - CreateParameterSchema: postgres.Jsonb{json.RawMessage(`{"createprop1" : "a really nice prop"}`)}, + StartParameterSchema: postgres.Jsonb{RawMessage: json.RawMessage(`{"startprop1" : "a nice prop"}`)}, + CreateParameterSchema: postgres.Jsonb{RawMessage: json.RawMessage(`{"createprop1" : "a really nice prop"}`)}, ManagedExternally: newTrue(), Manager: "4854af30-325f-44a5-ad59-b67b2597de99", } @@ -141,9 +141,9 @@ func TestMain(m *testing.M) { // connect AMQP client // Make sure that AMQP_HOST, AMQP_USER, AMQP_PASS are set - host, err := configuration.GlobalConfig.String("amqp.host") - usr, err := configuration.GlobalConfig.String("amqp.user") - pass, err := configuration.GlobalConfig.String("amqp.pass") + host, _ := configuration.GlobalConfig.String("amqp.host") + usr, _ := configuration.GlobalConfig.String("amqp.user") + pass, _ := configuration.GlobalConfig.String("amqp.pass") amqpURI := "amqp://" + usr + ":" + pass + "@" + host // AMQP Connection startup is tested here @@ -306,9 +306,8 @@ func TestUpdateICAsAdmin(t *testing.T) { payload, err := json.Marshal(update) assert.NoError(t, err) - var headers map[string]interface{} - headers = make(map[string]interface{}) // empty map - headers["uuid"] = newIC2.Manager // set uuid + var headers map[string]interface{} = make(map[string]interface{}) // empty map + headers["uuid"] = newIC2.Manager // set uuid msg := amqp.Publishing{ DeliveryMode: 2, @@ -426,9 +425,8 @@ func TestDeleteICAsAdmin(t *testing.T) { payload, err := json.Marshal(update) assert.NoError(t, err) - var headers map[string]interface{} - headers = make(map[string]interface{}) // empty map - headers["uuid"] = newIC2.UUID // set uuid + var headers map[string]interface{} = make(map[string]interface{}) // empty map + headers["uuid"] = newIC2.UUID // set uuid msg := amqp.Publishing{ DeliveryMode: 2, @@ -620,7 +618,7 @@ func TestSendActionToIC(t *testing.T) { var params startParams params.UUID = newIC1.UUID - paramsRaw, err := json.Marshal(¶ms) + paramsRaw, _ := json.Marshal(¶ms) action1.Parameters = paramsRaw actions := [1]helper.Action{action1} @@ -654,9 +652,8 @@ func TestCreateUpdateViaAMQPRecv(t *testing.T) { payload, err := json.Marshal(update) assert.NoError(t, err) - var headers map[string]interface{} - headers = make(map[string]interface{}) // empty map - headers["uuid"] = newIC1.Manager // set uuid + var headers map[string]interface{} = make(map[string]interface{}) // empty map + headers["uuid"] = newIC1.Manager // set uuid msg := amqp.Publishing{ DeliveryMode: 2, @@ -697,8 +694,7 @@ func TestCreateUpdateViaAMQPRecv(t *testing.T) { payload, err = json.Marshal(update) assert.NoError(t, err) - var headersA map[string]interface{} - headersA = make(map[string]interface{}) // empty map + var headersA map[string]interface{} = make(map[string]interface{}) // empty map headersA["uuid"] = newIC1.Manager msg = amqp.Publishing{ @@ -777,9 +773,8 @@ func TestDeleteICViaAMQPRecv(t *testing.T) { payload, err := json.Marshal(update) assert.NoError(t, err) - var headers map[string]interface{} - headers = make(map[string]interface{}) // empty map - headers["uuid"] = newIC1.Manager // set uuid + var headers map[string]interface{} = make(map[string]interface{}) // empty map + headers["uuid"] = newIC1.Manager // set uuid msg := amqp.Publishing{ DeliveryMode: 2, @@ -825,11 +820,13 @@ func TestDeleteICViaAMQPRecv(t *testing.T) { // Add component config and associate with IC and scenario newConfig := ConfigRequest{ - Name: "ConfigA", - ScenarioID: uint(newScenarioID), - ICID: 1, - StartParameters: postgres.Jsonb{json.RawMessage(`{"parameter1" : "testValue1B", "parameter2" : "testValue2B", "parameter3" : 55}`)}, - FileIDs: []int64{}, + Name: "ConfigA", + ScenarioID: uint(newScenarioID), + ICID: 1, + StartParameters: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"parameter1" : "testValue1B", "parameter2" : "testValue2B", "parameter3" : 55}`), + }, + FileIDs: []int64{}, } code, resp, err = helper.TestEndpoint(router, token, diff --git a/routes/infrastructure-component/ic_validators.go b/routes/infrastructure-component/ic_validators.go index 5c25ce0..3fff11c 100644 --- a/routes/infrastructure-component/ic_validators.go +++ b/routes/infrastructure-component/ic_validators.go @@ -88,7 +88,7 @@ func (r *AddICRequest) validate() error { return errs } - if *r.InfrastructureComponent.ManagedExternally == true { + if *r.InfrastructureComponent.ManagedExternally { // check if valid manager UUID is provided _, errs = uuid.Parse(r.InfrastructureComponent.Manager) if errs != nil { @@ -119,8 +119,7 @@ func (r *UpdateICRequest) validate() error { func (r *AddICRequest) createIC() (InfrastructureComponent, error) { var s InfrastructureComponent - var err error - err = nil + var err error = nil s.UUID = r.InfrastructureComponent.UUID s.WebsocketURL = r.InfrastructureComponent.WebsocketURL diff --git a/routes/register.go b/routes/register.go index 25cc977..cee7968 100644 --- a/routes/register.go +++ b/routes/register.go @@ -311,7 +311,7 @@ func AddTestData(cfg *config.Config, router *gin.Engine) (*bytes.Buffer, error) defer fh.Close() // io copy - _, err = io.Copy(fileWriter, fh) + _, _ = io.Copy(fileWriter, fh) contentType := bodyWriter.FormDataContentType() bodyWriter.Close() @@ -338,7 +338,7 @@ func AddTestData(cfg *config.Config, router *gin.Engine) (*bytes.Buffer, error) defer fh.Close() // io copy - _, err = io.Copy(fileWriter, fh) + _, _ = io.Copy(fileWriter, fh) contentType := bodyWriter.FormDataContentType() bodyWriter.Close() diff --git a/routes/register_test.go b/routes/register_test.go index 33f8834..8b112bb 100644 --- a/routes/register_test.go +++ b/routes/register_test.go @@ -66,8 +66,11 @@ func TestStartAMQP(t *testing.T) { // connect AMQP client // Make sure that AMQP_HOST, AMQP_USER, AMQP_PASS are set host, err := configuration.GlobalConfig.String("amqp.host") + assert.NoError(t, err) user, err := configuration.GlobalConfig.String("amqp.user") + assert.NoError(t, err) pass, err := configuration.GlobalConfig.String("amqp.pass") + assert.NoError(t, err) amqpURI := "amqp://" + user + ":" + pass + "@" + host // AMQP Connection startup is tested here diff --git a/routes/result/result_methods.go b/routes/result/result_methods.go index cad7c55..c716ddf 100644 --- a/routes/result/result_methods.go +++ b/routes/result/result_methods.go @@ -23,10 +23,11 @@ package result import ( + "log" + "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" "git.rwth-aachen.de/acs/public/villas/web-backend-go/routes/file" "git.rwth-aachen.de/acs/public/villas/web-backend-go/routes/scenario" - "log" ) type Result struct { @@ -92,6 +93,9 @@ func (r *Result) delete() error { // remove association between Result and Scenario err = db.Model(&sco).Association("Results").Delete(r).Error + if err != nil { + return err + } // Delete result files for _, fileid := range r.ResultFileIDs { diff --git a/routes/result/result_test.go b/routes/result/result_test.go index 9d4aeff..ab4475f 100644 --- a/routes/result/result_test.go +++ b/routes/result/result_test.go @@ -70,10 +70,10 @@ var newResult = ResultRequest{ func addScenario() (scenarioID uint) { // authenticate as admin - token, _ := helper.AuthenticateForTest(router, helper.AdminCredentials) + helper.AuthenticateForTest(router, helper.AdminCredentials) // authenticate as normal user - token, _ = helper.AuthenticateForTest(router, helper.UserACredentials) + token, _ := helper.AuthenticateForTest(router, helper.UserACredentials) // POST $newScenario newScenario := ScenarioRequest{ @@ -87,7 +87,7 @@ func addScenario() (scenarioID uint) { newScenarioID, _ := helper.GetResponseID(resp) // add the guest user to the new scenario - _, resp, _ = helper.TestEndpoint(router, token, + _, _, _ = helper.TestEndpoint(router, token, fmt.Sprintf("/api/v2/scenarios/%v/user?username=User_C", newScenarioID), "PUT", nil) return uint(newScenarioID) @@ -135,7 +135,9 @@ func TestGetAllResultsOfScenario(t *testing.T) { // test POST newResult configSnapshot1 := json.RawMessage(`{"configs": [ {"Name" : "conf1", "scenarioID" : 1}, {"Name" : "conf2", "scenarioID" : 1}]}`) - confSnapshots := postgres.Jsonb{configSnapshot1} + confSnapshots := postgres.Jsonb{ + RawMessage: configSnapshot1, + } newResult.ScenarioID = scenarioID newResult.ConfigSnapshots = confSnapshots @@ -174,7 +176,9 @@ func TestAddGetUpdateDeleteResult(t *testing.T) { // by adding a scenario scenarioID := addScenario() configSnapshot1 := json.RawMessage(`{"configs": [ {"Name" : "conf1", "scenarioID" : 1}, {"Name" : "conf2", "scenarioID" : 1}]}`) - confSnapshots := postgres.Jsonb{configSnapshot1} + confSnapshots := postgres.Jsonb{ + RawMessage: configSnapshot1, + } newResult.ScenarioID = scenarioID newResult.ConfigSnapshots = confSnapshots // authenticate as normal userB who has no access to new scenario @@ -347,7 +351,9 @@ func TestAddDeleteResultFile(t *testing.T) { // by adding a scenario scenarioID := addScenario() configSnapshot1 := json.RawMessage(`{"configs": [ {"Name" : "conf1", "scenarioID" : 1}, {"Name" : "conf2", "scenarioID" : 1}]}`) - confSnapshots := postgres.Jsonb{configSnapshot1} + confSnapshots := postgres.Jsonb{ + RawMessage: configSnapshot1, + } newResult.ScenarioID = scenarioID newResult.ConfigSnapshots = confSnapshots @@ -404,6 +410,7 @@ func TestAddDeleteResultFile(t *testing.T) { assert.Equalf(t, 200, w.Code, "Response body: \n%v\n", w.Body) err = helper.CompareResponse(w.Body, helper.KeyModels{"result": newResult}) + assert.NoError(t, err) // extract file ID from response body var respResult ResponseResult @@ -455,6 +462,7 @@ func TestAddDeleteResultFile(t *testing.T) { assert.Equalf(t, 200, w2.Code, "Response body: \n%v\n", w2.Body) err = helper.CompareResponse(w2.Body, helper.KeyModels{"result": newResult}) + assert.NoError(t, err) // extract file ID from response body var respResult3 ResponseResult diff --git a/routes/scenario/scenario_methods.go b/routes/scenario/scenario_methods.go index 49ea335..04fa083 100644 --- a/routes/scenario/scenario_methods.go +++ b/routes/scenario/scenario_methods.go @@ -23,6 +23,7 @@ package scenario import ( "fmt" + "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" "git.rwth-aachen.de/acs/public/villas/web-backend-go/routes/user" "github.com/jinzhu/gorm" @@ -105,6 +106,9 @@ func (s *Scenario) deleteUser(username string) error { // There is only one associated user var remainingUser user.User err = db.Model(s).Related(&remainingUser, "Users").Error + if err != nil { + return err + } if remainingUser.Username == username { // if the remaining user is the one to be deleted return fmt.Errorf("cannot delete last user from scenario without deleting scenario itself, doing nothing") diff --git a/routes/scenario/scenario_middleware.go b/routes/scenario/scenario_middleware.go index d908e03..1e461b4 100644 --- a/routes/scenario/scenario_middleware.go +++ b/routes/scenario/scenario_middleware.go @@ -23,6 +23,7 @@ package scenario import ( "fmt" + "git.rwth-aachen.de/acs/public/villas/web-backend-go/helper" "github.com/gin-gonic/gin" @@ -55,7 +56,7 @@ func CheckPermissions(c *gin.Context, operation database.CRUD, scenarioIDsource return false, so } - if so.checkAccess(userID.(uint), operation) == false { + if !so.checkAccess(userID.(uint), operation) { helper.UnprocessableEntityError(c, "Access denied (user has no access or scenario is locked).") return false, so } diff --git a/routes/scenario/scenario_test.go b/routes/scenario/scenario_test.go index 8580e49..2aaf761 100644 --- a/routes/scenario/scenario_test.go +++ b/routes/scenario/scenario_test.go @@ -53,13 +53,17 @@ type UserRequest struct { } var newScenario1 = ScenarioRequest{ - Name: "Scenario1", - StartParameters: postgres.Jsonb{json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`)}, + Name: "Scenario1", + StartParameters: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`), + }, } var newScenario2 = ScenarioRequest{ - Name: "Scenario2", - StartParameters: postgres.Jsonb{json.RawMessage(`{"parameter1" : "testValue1B", "parameter2" : "testValue2B", "parameter3" : 55}`)}, + Name: "Scenario2", + StartParameters: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"parameter1" : "testValue1B", "parameter2" : "testValue2B", "parameter3" : 55}`), + }, } func TestMain(m *testing.M) { diff --git a/routes/signal/signal_test.go b/routes/signal/signal_test.go index e48b775..68cc323 100644 --- a/routes/signal/signal_test.go +++ b/routes/signal/signal_test.go @@ -95,17 +95,21 @@ func addScenarioAndICAndConfig() (scenarioID uint, ICID uint, configID uint) { // POST $newICA newICA := ICRequest{ - UUID: "7be0322d-354e-431e-84bd-ae4c9633138b", - WebsocketURL: "https://villas.k8s.eonerc.rwth-aachen.de/ws/ws_sig", - Type: "villas-node", - Name: "ACS Demo Signals", - Category: "gateway", - State: "idle", - Location: "k8s", - Description: "A signal generator for testing purposes", - StartParameterSchema: postgres.Jsonb{json.RawMessage(`{"startprop1" : "a nice prop"}`)}, - CreateParameterSchema: postgres.Jsonb{json.RawMessage(`{"createprop1" : "a really nice prop"}`)}, - ManagedExternally: newFalse(), + UUID: "7be0322d-354e-431e-84bd-ae4c9633138b", + WebsocketURL: "https://villas.k8s.eonerc.rwth-aachen.de/ws/ws_sig", + Type: "villas-node", + Name: "ACS Demo Signals", + Category: "gateway", + State: "idle", + Location: "k8s", + Description: "A signal generator for testing purposes", + StartParameterSchema: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"startprop1" : "a nice prop"}`), + }, + CreateParameterSchema: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"createprop1" : "a really nice prop"}`), + }, + ManagedExternally: newFalse(), } _, resp, _ := helper.TestEndpoint(router, token, "/api/v2/ic", "POST", helper.KeyModels{"ic": newICA}) @@ -118,8 +122,10 @@ func addScenarioAndICAndConfig() (scenarioID uint, ICID uint, configID uint) { // POST $newScenario newScenario := ScenarioRequest{ - Name: "Scenario1", - StartParameters: postgres.Jsonb{json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`)}, + Name: "Scenario1", + StartParameters: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`), + }, } _, resp, _ = helper.TestEndpoint(router, token, "/api/v2/scenarios", "POST", helper.KeyModels{"scenario": newScenario}) @@ -141,7 +147,7 @@ func addScenarioAndICAndConfig() (scenarioID uint, ICID uint, configID uint) { newConfigID, _ := helper.GetResponseID(resp) // add the guest user to the new scenario - _, resp, _ = helper.TestEndpoint(router, token, + _, _, _ = helper.TestEndpoint(router, token, fmt.Sprintf("/api/v2/scenarios/%v/user?username=User_C", newScenarioID), "PUT", nil) return uint(newScenarioID), uint(newICID), uint(newConfigID) @@ -189,12 +195,12 @@ func TestAddSignal(t *testing.T) { _, _, configID := addScenarioAndICAndConfig() // authenticate as normal user - token, err := helper.AuthenticateForTest(router, helper.UserACredentials) + _, err := helper.AuthenticateForTest(router, helper.UserACredentials) assert.NoError(t, err) newSignal1.ConfigID = configID // authenticate as normal userB who has no access to new scenario - token, err = helper.AuthenticateForTest(router, helper.UserBCredentials) + token, err := helper.AuthenticateForTest(router, helper.UserBCredentials) assert.NoError(t, err) // try to POST to component config without access diff --git a/routes/user/user_methods.go b/routes/user/user_methods.go index db22eb7..9396f07 100644 --- a/routes/user/user_methods.go +++ b/routes/user/user_methods.go @@ -43,7 +43,7 @@ func NewUser(username, password, mail, role string, active bool) (User, error) { // Check that the username is NOT taken err := newUser.ByUsername(username) if err == nil { - return newUser, fmt.Errorf("Username is already taken") + return newUser, fmt.Errorf("username is already taken") } newUser.Username = username @@ -97,12 +97,12 @@ func (u *User) ByID(id uint) error { func (u *User) setPassword(password string) error { if len(password) == 0 { - return fmt.Errorf("Password cannot be empty") + return fmt.Errorf("password cannot be empty") } newPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcryptCost) if err != nil { - return fmt.Errorf("Failed to generate hash from password") + return fmt.Errorf("failed to generate hash from password") } u.Password = string(newPassword) return nil diff --git a/routes/user/user_middleware.go b/routes/user/user_middleware.go index 8ff4285..82801c2 100644 --- a/routes/user/user_middleware.go +++ b/routes/user/user_middleware.go @@ -62,7 +62,7 @@ func isAuthenticated(c *gin.Context) (bool, error) { func(token *jwt.Token) (interface{}, error) { // Validate alg for signing the jwt if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("Unexpected signing alg: %v", + return nil, fmt.Errorf("unexpected signing alg: %v", token.Header["alg"]) } diff --git a/routes/user/user_test.go b/routes/user/user_test.go index c109cba..62e4590 100644 --- a/routes/user/user_test.go +++ b/routes/user/user_test.go @@ -75,7 +75,7 @@ func TestMain(m *testing.M) { func TestAuthenticate(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // try to authenticate with non JSON body @@ -172,7 +172,7 @@ func TestAuthenticateQueryToken(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -193,7 +193,7 @@ func TestAddGetUser(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -308,7 +308,7 @@ func TestAddGetUser(t *testing.T) { // try to GET user with invalid user ID // should result in bad request code, resp, err = helper.TestEndpoint(router, token, - fmt.Sprintf("/api/v2/users/bla"), "GET", nil) + "/api/v2/users/bla", "GET", nil) assert.NoError(t, err) assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) } @@ -317,7 +317,7 @@ func TestUsersNotAllowedActions(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -376,7 +376,7 @@ func TestGetAllUsers(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -429,7 +429,7 @@ func TestModifyAddedUserAsUser(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -460,7 +460,7 @@ func TestModifyAddedUserAsUser(t *testing.T) { // Try PUT with invalid user ID in path // Should return a bad request - code, resp, err = helper.TestEndpoint(router, token, fmt.Sprintf("/api/v2/users/blabla"), "PUT", + code, resp, err = helper.TestEndpoint(router, token, "/api/v2/users/blabla", "PUT", helper.KeyModels{"user": newUser}) assert.NoError(t, err) assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) @@ -584,7 +584,7 @@ func TestInvalidUserUpdate(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -657,7 +657,7 @@ func TestModifyAddedUserAsAdmin(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -774,7 +774,7 @@ func TestDeleteUser(t *testing.T) { database.DropTables() database.MigrateModels() - err, adminpw := database.DBAddAdminUser(configuration.GlobalConfig) + adminpw, err := database.DBAddAdminUser(configuration.GlobalConfig) assert.NoError(t, err) // authenticate as admin @@ -799,7 +799,7 @@ func TestDeleteUser(t *testing.T) { // try to DELETE with invalid ID // should result in bad request code, resp, err = helper.TestEndpoint(router, token, - fmt.Sprintf("/api/v2/users/bla"), "DELETE", nil) + "/api/v2/users/bla", "DELETE", nil) assert.NoError(t, err) assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) diff --git a/routes/user/user_validators.go b/routes/user/user_validators.go index b7da698..49202ef 100644 --- a/routes/user/user_validators.go +++ b/routes/user/user_validators.go @@ -82,16 +82,16 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, // Only the Admin must be able to update user's role if role != "Admin" && r.User.Role != "" { if r.User.Role != u.Role { - return u, fmt.Errorf("Only Admin can update user's Role") + return u, fmt.Errorf("only Admin can update user's Role") } } else if role == "Admin" && r.User.Role != "" { u.Role = r.User.Role } // Only the Admin must be able to update users Active state - if (r.User.Active == "yes" && u.Active == false) || (r.User.Active == "no" && u.Active == true) { + if (r.User.Active == "yes" && !u.Active) || (r.User.Active == "no" && u.Active) { if role != "Admin" { - return u, fmt.Errorf("Only Admin can update user's Active state") + return u, fmt.Errorf("only Admin can update user's Active state") } else { u.Active = !u.Active } @@ -100,7 +100,7 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, // Update the username making sure it is NOT taken var testUser User if err := testUser.ByUsername(r.User.Username); err == nil { - return u, fmt.Errorf("Username is alreaday taken") + return u, fmt.Errorf("username is alreaday taken") } if r.User.Username != "" { diff --git a/routes/widget/widget_test.go b/routes/widget/widget_test.go index e0a987c..dd9ecd3 100644 --- a/routes/widget/widget_test.go +++ b/routes/widget/widget_test.go @@ -86,8 +86,10 @@ func addScenarioAndDashboard(token string) (scenarioID uint, dashboardID uint) { // POST $newScenario newScenario := ScenarioRequest{ - Name: "Scenario1", - StartParameters: postgres.Jsonb{json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`)}, + Name: "Scenario1", + StartParameters: postgres.Jsonb{ + RawMessage: json.RawMessage(`{"parameter1" : "testValue1A", "parameter2" : "testValue2A", "parameter3" : 42}`), + }, } _, resp, _ := helper.TestEndpoint(router, token, "/api/v2/scenarios", "POST", helper.KeyModels{"scenario": newScenario}) @@ -108,7 +110,7 @@ func addScenarioAndDashboard(token string) (scenarioID uint, dashboardID uint) { newDashboardID, _ := helper.GetResponseID(resp) // add the guest user to the new scenario - _, resp, _ = helper.TestEndpoint(router, token, + _, _, _ = helper.TestEndpoint(router, token, fmt.Sprintf("/api/v2/scenarios/%v/user?username=User_C", newScenarioID), "PUT", nil) return uint(newScenarioID), uint(newDashboardID) diff --git a/start.go b/start.go index 992df45..42f0fdd 100644 --- a/start.go +++ b/start.go @@ -135,7 +135,7 @@ func main() { } // Make sure that at least one admin user exists in DB - err, _ = database.DBAddAdminUser(configuration.GlobalConfig) + _, err = database.DBAddAdminUser(configuration.GlobalConfig) if err != nil { fmt.Println("error: adding admin user failed:", err.Error()) log.Fatal(err) From da0929411bdc07b7493ccaf40d5b7da2a3750307 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 19 Oct 2021 13:57:05 +0200 Subject: [PATCH 2/9] add staticcheck to CI --- .gitlab-ci.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cff1cbb..c776a3e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -10,6 +10,14 @@ stages: # Stage: test ############################################################################## +staticcheck: + stage: test + image: golang:1.16-buster + before_script: + - go install honnef.co/go/tools/cmd/staticcheck@latest + script: + - staticcheck ./... + test: stage: test image: golang:1.16-buster From 515ef2de1567cc666092019dd6db8f28bba80127 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 19 Oct 2021 14:18:23 +0200 Subject: [PATCH 3/9] fix CI tests --- database/database.go | 9 +++------ routes/user/user_test.go | 1 - routes/user/user_validators.go | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/database/database.go b/database/database.go index b0da0e5..db2e074 100644 --- a/database/database.go +++ b/database/database.go @@ -126,17 +126,14 @@ func DBAddAdminUser(cfg *config.Config) (string, error) { // Check if admin user exists in DB var users []User - err := DBpool.Where("Role = ?", "Admin").Find(&users).Error - if err != nil { - return "", err - } + DBpool.Where("Role = ?", "Admin").Find(&users) + adminPW := "" - adminName := "" if len(users) == 0 { fmt.Println("No admin user found in DB, adding default admin user.") - adminName, err = cfg.String("admin.user") + adminName, err := cfg.String("admin.user") if err != nil || adminName == "" { adminName = "admin" } diff --git a/routes/user/user_test.go b/routes/user/user_test.go index 62e4590..8231afb 100644 --- a/routes/user/user_test.go +++ b/routes/user/user_test.go @@ -650,7 +650,6 @@ func TestInvalidUserUpdate(t *testing.T) { helper.KeyModels{"user": modRequest}) assert.NoError(t, err) assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) - } func TestModifyAddedUserAsAdmin(t *testing.T) { diff --git a/routes/user/user_validators.go b/routes/user/user_validators.go index 49202ef..18fe56b 100644 --- a/routes/user/user_validators.go +++ b/routes/user/user_validators.go @@ -100,7 +100,7 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, // Update the username making sure it is NOT taken var testUser User if err := testUser.ByUsername(r.User.Username); err == nil { - return u, fmt.Errorf("username is alreaday taken") + return u, fmt.Errorf("username is already taken") } if r.User.Username != "" { From 7631e2a9b50f5c75ce44dce296f90da56e2f90f0 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 19 Oct 2021 14:40:45 +0200 Subject: [PATCH 4/9] pass boolean clear argument to DBInit() --- database/database.go | 4 ++-- database/database_test.go | 12 ++++++------ routes/component-configuration/config_test.go | 2 +- routes/dashboard/dashboard_test.go | 2 +- routes/file/file_test.go | 2 +- routes/healthz/healthz_test.go | 4 ++-- routes/infrastructure-component/ic_test.go | 2 +- routes/register_test.go | 2 +- routes/result/result_test.go | 2 +- routes/scenario/scenario_test.go | 2 +- routes/signal/signal_test.go | 2 +- routes/user/user_test.go | 2 +- routes/widget/widget_test.go | 2 +- start.go | 2 +- 14 files changed, 21 insertions(+), 21 deletions(-) diff --git a/database/database.go b/database/database.go index db2e074..5d21e54 100644 --- a/database/database.go +++ b/database/database.go @@ -38,7 +38,7 @@ import ( var DBpool *gorm.DB // database used by backend // InitDB Initialize connection to the database -func InitDB(cfg *config.Config, dbClear string) error { +func InitDB(cfg *config.Config, clear bool) error { name, err := cfg.String("db.name") if err != nil { return err @@ -76,7 +76,7 @@ func InitDB(cfg *config.Config, dbClear string) error { DBpool = db // drop tables if parameter set - if dbClear == "true" { + if clear { DropTables() log.Println("Database tables dropped") } diff --git a/database/database_test.go b/database/database_test.go index 1af89ab..6eff3c3 100644 --- a/database/database_test.go +++ b/database/database_test.go @@ -50,41 +50,41 @@ func TestInitDB(t *testing.T) { ownconfig := config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, "true") + err = InitDB(ownconfig, true) assert.Error(t, err) dbname, err := configuration.GlobalConfig.String("db.name") assert.NoError(t, err) static["db.name"] = dbname ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, "true") + err = InitDB(ownconfig, true) assert.Error(t, err) dbhost, err := configuration.GlobalConfig.String("db.host") assert.NoError(t, err) static["db.host"] = dbhost ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, "true") + err = InitDB(ownconfig, true) assert.Error(t, err) dbuser, err := configuration.GlobalConfig.String("db.user") assert.NoError(t, err) static["db.user"] = dbuser ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, "true") + err = InitDB(ownconfig, true) assert.Error(t, err) dbpass, err := configuration.GlobalConfig.String("db.pass") assert.NoError(t, err) static["db.pass"] = dbpass ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, "true") + err = InitDB(ownconfig, true) assert.Error(t, err) dbssl, err := configuration.GlobalConfig.String("db.ssl") assert.NoError(t, err) static["db.ssl"] = dbssl ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, "true") + err = InitDB(ownconfig, true) assert.NoError(t, err) // Verify that you can connect to the database diff --git a/routes/component-configuration/config_test.go b/routes/component-configuration/config_test.go index eadb004..e0de543 100644 --- a/routes/component-configuration/config_test.go +++ b/routes/component-configuration/config_test.go @@ -147,7 +147,7 @@ func TestMain(m *testing.M) { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/dashboard/dashboard_test.go b/routes/dashboard/dashboard_test.go index a2f1b61..dd1d64a 100644 --- a/routes/dashboard/dashboard_test.go +++ b/routes/dashboard/dashboard_test.go @@ -87,7 +87,7 @@ func TestMain(m *testing.M) { if err != nil { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/file/file_test.go b/routes/file/file_test.go index 9c66f6e..57b74f8 100644 --- a/routes/file/file_test.go +++ b/routes/file/file_test.go @@ -81,7 +81,7 @@ func TestMain(m *testing.M) { if err != nil { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/healthz/healthz_test.go b/routes/healthz/healthz_test.go index 6202654..e0956af 100644 --- a/routes/healthz/healthz_test.go +++ b/routes/healthz/healthz_test.go @@ -41,7 +41,7 @@ func TestHealthz(t *testing.T) { assert.NoError(t, err) // connect DB - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) assert.NoError(t, err) defer database.DBpool.Close() @@ -60,7 +60,7 @@ func TestHealthz(t *testing.T) { assert.Equalf(t, 500, code, "Response body: \n%v\n", resp) // reconnect DB - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) assert.NoError(t, err) defer database.DBpool.Close() diff --git a/routes/infrastructure-component/ic_test.go b/routes/infrastructure-component/ic_test.go index 013d897..4790017 100644 --- a/routes/infrastructure-component/ic_test.go +++ b/routes/infrastructure-component/ic_test.go @@ -115,7 +115,7 @@ func TestMain(m *testing.M) { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/register_test.go b/routes/register_test.go index 8b112bb..9654a28 100644 --- a/routes/register_test.go +++ b/routes/register_test.go @@ -43,7 +43,7 @@ func TestMain(m *testing.M) { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/result/result_test.go b/routes/result/result_test.go index ab4475f..268d561 100644 --- a/routes/result/result_test.go +++ b/routes/result/result_test.go @@ -98,7 +98,7 @@ func TestMain(m *testing.M) { if err != nil { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/scenario/scenario_test.go b/routes/scenario/scenario_test.go index 2aaf761..a61c63c 100644 --- a/routes/scenario/scenario_test.go +++ b/routes/scenario/scenario_test.go @@ -72,7 +72,7 @@ func TestMain(m *testing.M) { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/signal/signal_test.go b/routes/signal/signal_test.go index 68cc323..84767b6 100644 --- a/routes/signal/signal_test.go +++ b/routes/signal/signal_test.go @@ -159,7 +159,7 @@ func TestMain(m *testing.M) { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/user/user_test.go b/routes/user/user_test.go index 8231afb..97fdcec 100644 --- a/routes/user/user_test.go +++ b/routes/user/user_test.go @@ -56,7 +56,7 @@ func TestMain(m *testing.M) { if err != nil { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/routes/widget/widget_test.go b/routes/widget/widget_test.go index dd9ecd3..2b55488 100644 --- a/routes/widget/widget_test.go +++ b/routes/widget/widget_test.go @@ -122,7 +122,7 @@ func TestMain(m *testing.M) { panic(m) } - err = database.InitDB(configuration.GlobalConfig, "true") + err = database.InitDB(configuration.GlobalConfig, true) if err != nil { panic(m) } diff --git a/start.go b/start.go index 42f0fdd..b8088d2 100644 --- a/start.go +++ b/start.go @@ -101,7 +101,7 @@ func main() { } // Init database - err = database.InitDB(configuration.GlobalConfig, dbClear) + err = database.InitDB(configuration.GlobalConfig, dbClear == "true") if err != nil { log.Fatalf("Error during initialization of database: %s, aborting.", err) } From 9e45f39e31d94394a2ce02c739437b41459566d9 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 19 Oct 2021 14:45:45 +0200 Subject: [PATCH 5/9] simplify database test --- database/database.go | 4 ++++ database/database_test.go | 49 ++++++++++++--------------------------- 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/database/database.go b/database/database.go index 5d21e54..b8b48b5 100644 --- a/database/database.go +++ b/database/database.go @@ -43,14 +43,17 @@ func InitDB(cfg *config.Config, clear bool) error { if err != nil { return err } + host, err := cfg.String("db.host") if err != nil { return err } + user, err := cfg.String("db.user") if err != nil && !strings.Contains(err.Error(), "Required setting 'db.user' not set") { return err } + pass := "" if user != "" { pass, err = cfg.String("db.pass") @@ -58,6 +61,7 @@ func InitDB(cfg *config.Config, clear bool) error { return err } } + sslmode, err := cfg.String("db.ssl") if err != nil { return err diff --git a/database/database_test.go b/database/database_test.go index 6eff3c3..fd2aedc 100644 --- a/database/database_test.go +++ b/database/database_test.go @@ -48,44 +48,25 @@ func TestInitDB(t *testing.T) { defaults := config.NewStatic(static) env := config.NewEnvironment(mappings) - ownconfig := config.NewConfig([]config.Provider{defaults, env}) + ownConfig := config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, true) - assert.Error(t, err) - dbname, err := configuration.GlobalConfig.String("db.name") - assert.NoError(t, err) - static["db.name"] = dbname - ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, true) + err = InitDB(ownConfig, true) assert.Error(t, err) - dbhost, err := configuration.GlobalConfig.String("db.host") - assert.NoError(t, err) - static["db.host"] = dbhost - ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, true) - assert.Error(t, err) + dbOptions := []string{"db.name", "db.host", "db.user", "db.pass", "db.ssl"} + for _, opt := range dbOptions { + val, err := configuration.GlobalConfig.String(opt) + assert.NoError(t, err) + static[opt] = val + ownConfig = config.NewConfig([]config.Provider{defaults, env}) + err = InitDB(ownConfig, true) - dbuser, err := configuration.GlobalConfig.String("db.user") - assert.NoError(t, err) - static["db.user"] = dbuser - ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, true) - assert.Error(t, err) - - dbpass, err := configuration.GlobalConfig.String("db.pass") - assert.NoError(t, err) - static["db.pass"] = dbpass - ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, true) - assert.Error(t, err) - - dbssl, err := configuration.GlobalConfig.String("db.ssl") - assert.NoError(t, err) - static["db.ssl"] = dbssl - ownconfig = config.NewConfig([]config.Provider{defaults, env}) - err = InitDB(ownconfig, true) - assert.NoError(t, err) + if opt == "db.ssl" { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + } // Verify that you can connect to the database db := GetDB() From 786454a100c61cbe571d9ee0ef688ff7e63f9793 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 19 Oct 2021 15:30:27 +0200 Subject: [PATCH 6/9] use proper error types instead of using strings.Contains() --- database/database.go | 4 +- routes/healthz/healthz_endpoint.go | 5 +-- .../infrastructure-component/ic_amqpclient.go | 3 +- routes/infrastructure-component/ic_errors.go | 32 +++++++++++++++ routes/infrastructure-component/ic_methods.go | 3 +- routes/user/user_endpoints.go | 5 +-- routes/user/user_errors.go | 40 +++++++++++++++++++ routes/user/user_methods.go | 2 +- routes/user/user_test.go | 8 ++-- routes/user/user_validators.go | 12 +++--- 10 files changed, 91 insertions(+), 23 deletions(-) create mode 100644 routes/infrastructure-component/ic_errors.go create mode 100644 routes/user/user_errors.go diff --git a/database/database.go b/database/database.go index b8b48b5..e0bc95f 100644 --- a/database/database.go +++ b/database/database.go @@ -49,8 +49,8 @@ func InitDB(cfg *config.Config, clear bool) error { return err } - user, err := cfg.String("db.user") - if err != nil && !strings.Contains(err.Error(), "Required setting 'db.user' not set") { + user, err := cfg.StringOr("db.user", "") + if err != nil { return err } diff --git a/routes/healthz/healthz_endpoint.go b/routes/healthz/healthz_endpoint.go index eb5f65e..d26c263 100644 --- a/routes/healthz/healthz_endpoint.go +++ b/routes/healthz/healthz_endpoint.go @@ -24,7 +24,6 @@ package healthz import ( "log" "net/http" - "strings" "git.rwth-aachen.de/acs/public/villas/web-backend-go/configuration" "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" @@ -55,8 +54,8 @@ func getHealth(c *gin.Context) { } // check if connection to AMQP broker is alive if backend was started with AMQP client - url, err := configuration.GlobalConfig.String("amqp.host") - if err != nil && strings.Contains(err.Error(), "Required setting 'amqp.host' not set") { + url, err := configuration.GlobalConfig.StringOr("amqp.host", "not-set") + if err != nil && url == "not-set" { c.JSON(http.StatusOK, gin.H{}) return } else if err != nil { diff --git a/routes/infrastructure-component/ic_amqpclient.go b/routes/infrastructure-component/ic_amqpclient.go index ee81693..cc071c9 100644 --- a/routes/infrastructure-component/ic_amqpclient.go +++ b/routes/infrastructure-component/ic_amqpclient.go @@ -25,7 +25,6 @@ import ( "encoding/json" "fmt" "log" - "strings" "git.rwth-aachen.de/acs/public/villas/web-backend-go/helper" "github.com/gin-gonic/gin" @@ -197,7 +196,7 @@ func (s *InfrastructureComponent) updateExternalIC(payload ICUpdate, body []byte if err != nil { // if component could not be deleted there are still configurations using it in the DB // continue with the update to save the new state of the component and get back to the deletion later - if strings.Contains(err.Error(), "postponed") { + if _, ok := err.(*DeletionPostponed); ok { log.Println(err) // print log message } else { return err // return upon DB error diff --git a/routes/infrastructure-component/ic_errors.go b/routes/infrastructure-component/ic_errors.go new file mode 100644 index 0000000..1a05fb8 --- /dev/null +++ b/routes/infrastructure-component/ic_errors.go @@ -0,0 +1,32 @@ +/** User package, methods. +* +* @author Sonja Happ +* @copyright 2014-2019, Institute for Automation of Complex Power Systems, EONERC +* @license GNU General Public License (version 3) +* +* VILLASweb-backend-go +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*********************************************************************************/ +package infrastructure_component + +import "fmt" + +type DeletionPostponed struct { + References int +} + +func (e *DeletionPostponed) Error() string { + return fmt.Sprintf("deletion of IC postponed, %d config(s) associated to it", e.References) +} diff --git a/routes/infrastructure-component/ic_methods.go b/routes/infrastructure-component/ic_methods.go index 7c9c307..35389ab 100644 --- a/routes/infrastructure-component/ic_methods.go +++ b/routes/infrastructure-component/ic_methods.go @@ -22,7 +22,6 @@ package infrastructure_component import ( - "fmt" "git.rwth-aachen.de/acs/public/villas/web-backend-go/database" ) @@ -68,7 +67,7 @@ func (s *InfrastructureComponent) delete() error { noConfigs := db.Model(s).Association("ComponentConfigurations").Count() if noConfigs > 0 { - return fmt.Errorf("deletion of IC postponed, %v config(s) associated to it", noConfigs) + return &DeletionPostponed{References: noConfigs} } // delete InfrastructureComponent from DB (does NOT remain as dangling) diff --git a/routes/user/user_endpoints.go b/routes/user/user_endpoints.go index 3e0b03a..21f6a9c 100644 --- a/routes/user/user_endpoints.go +++ b/routes/user/user_endpoints.go @@ -24,7 +24,6 @@ package user import ( "fmt" "net/http" - "strings" "git.rwth-aachen.de/acs/public/villas/web-backend-go/helper" @@ -188,9 +187,9 @@ func updateUser(c *gin.Context) { // case that the request updates the role of the old user) updatedUser, err := req.updatedUser(callerID, callerRole, oldUser) if err != nil { - if strings.Contains(err.Error(), "Admin") || strings.Contains(err.Error(), "pw not changed") { + if _, ok := err.(*ForbiddenError); ok { helper.ForbiddenError(c, err.Error()) - } else if strings.Contains(err.Error(), "Username") || strings.Contains(err.Error(), "old or admin password") { + } else if _, ok := err.(*UsernameAlreadyTaken); ok { helper.BadRequestError(c, err.Error()) } else { // password encryption failed helper.InternalServerError(c, err.Error()) diff --git a/routes/user/user_errors.go b/routes/user/user_errors.go new file mode 100644 index 0000000..f523a36 --- /dev/null +++ b/routes/user/user_errors.go @@ -0,0 +1,40 @@ +/** User package, methods. +* +* @author Sonja Happ +* @copyright 2014-2019, Institute for Automation of Complex Power Systems, EONERC +* @license GNU General Public License (version 3) +* +* VILLASweb-backend-go +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*********************************************************************************/ +package user + +import "fmt" + +type UsernameAlreadyTaken struct { + Username string +} + +func (e *UsernameAlreadyTaken) Error() string { + return fmt.Sprintf("username is already taken: %s", e.Username) +} + +type ForbiddenError struct { + Reason string +} + +func (e *ForbiddenError) Error() string { + return fmt.Sprintf("permission denied: %s", e.Reason) +} diff --git a/routes/user/user_methods.go b/routes/user/user_methods.go index 9396f07..c0ea4d6 100644 --- a/routes/user/user_methods.go +++ b/routes/user/user_methods.go @@ -43,7 +43,7 @@ func NewUser(username, password, mail, role string, active bool) (User, error) { // Check that the username is NOT taken err := newUser.ByUsername(username) if err == nil { - return newUser, fmt.Errorf("username is already taken") + return newUser, &UsernameAlreadyTaken{Username: username} } newUser.Username = username diff --git a/routes/user/user_test.go b/routes/user/user_test.go index 97fdcec..984f084 100644 --- a/routes/user/user_test.go +++ b/routes/user/user_test.go @@ -54,11 +54,11 @@ type UserRequest struct { func TestMain(m *testing.M) { err := configuration.InitConfig() if err != nil { - panic(m) + panic(err) } err = database.InitDB(configuration.GlobalConfig, true) if err != nil { - panic(m) + panic(err) } defer database.DBpool.Close() @@ -541,7 +541,7 @@ func TestModifyAddedUserAsUser(t *testing.T) { fmt.Sprintf("/api/v2/users/%v", newUserID), "PUT", helper.KeyModels{"user": modRequest}) assert.NoError(t, err) - assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) + assert.Equalf(t, 403, code, "Response body: \n%v\n", resp) // modify newUser's password with wring old password modRequest = UserRequest{ @@ -725,7 +725,7 @@ func TestModifyAddedUserAsAdmin(t *testing.T) { fmt.Sprintf("/api/v2/users/%v", newUserID), "PUT", helper.KeyModels{"user": modRequest}) assert.NoError(t, err) - assert.Equalf(t, 400, code, "Response body: \n%v\n", resp) + assert.Equalf(t, 403, code, "Response body: \n%v\n", resp) // modify newUser's password, requires admin password modRequest = UserRequest{ diff --git a/routes/user/user_validators.go b/routes/user/user_validators.go index 18fe56b..2bf5bf1 100644 --- a/routes/user/user_validators.go +++ b/routes/user/user_validators.go @@ -82,7 +82,7 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, // Only the Admin must be able to update user's role if role != "Admin" && r.User.Role != "" { if r.User.Role != u.Role { - return u, fmt.Errorf("only Admin can update user's Role") + return u, &ForbiddenError{Reason: "only Admin can update user's Role"} } } else if role == "Admin" && r.User.Role != "" { u.Role = r.User.Role @@ -91,7 +91,7 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, // Only the Admin must be able to update users Active state if (r.User.Active == "yes" && !u.Active) || (r.User.Active == "no" && u.Active) { if role != "Admin" { - return u, fmt.Errorf("only Admin can update user's Active state") + return u, &ForbiddenError{Reason: "only Admin can update user's Active state"} } else { u.Active = !u.Active } @@ -100,7 +100,7 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, // Update the username making sure it is NOT taken var testUser User if err := testUser.ByUsername(r.User.Username); err == nil { - return u, fmt.Errorf("username is already taken") + return u, &UsernameAlreadyTaken{Username: r.User.Username} } if r.User.Username != "" { @@ -111,7 +111,7 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, if r.User.Password != "" { if r.User.OldPassword == "" { // admin or old password has to be present for pw change - return u, fmt.Errorf("old or admin password is missing in request") + return u, &ForbiddenError{Reason: "missing old or admin password"} } if role == "Admin" { // admin has to enter admin password @@ -123,14 +123,14 @@ func (r *updateUserRequest) updatedUser(callerID interface{}, role interface{}, err = adminUser.validatePassword(r.User.OldPassword) if err != nil { - return u, fmt.Errorf("admin password not correct, pw not changed") + return u, &ForbiddenError{Reason: "admin password not correct, pw not changed"} } } else { //normal or guest user has to enter old password err := oldUser.validatePassword(r.User.OldPassword) if err != nil { - return u, fmt.Errorf("previous password not correct, pw not changed") + return u, &ForbiddenError{Reason: "previous password not correct, pw not changed"} } } From 3dcb67fdd0fe1ca2dd79420e07b8678944a60c0a Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 19 Oct 2021 15:34:45 +0200 Subject: [PATCH 7/9] update gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1314e31..f88b4e9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ villasweb-backend-go routes/file/testfile.txt +routes/result/testfile.csv routes/file/testfileupdated.txt web-backend-go From 941687692e43d1a40265568c210b62608464061c Mon Sep 17 00:00:00 2001 From: Sonja Happ Date: Tue, 19 Oct 2021 15:54:36 +0200 Subject: [PATCH 8/9] fix error handling of s3.bucket param request --- routes/file/file_methods.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/routes/file/file_methods.go b/routes/file/file_methods.go index 9f17181..eeb1b10 100644 --- a/routes/file/file_methods.go +++ b/routes/file/file_methods.go @@ -167,10 +167,9 @@ func (f *File) update(fileHeader *multipart.FileHeader) error { defer fileContent.Close() bucket, err := configuration.GlobalConfig.String("s3.bucket") - if err != nil { - return err - } - if bucket == "" { + if err != nil || bucket == "" { + // s3 object storage not used, s3.bucket param is empty + // save file to postgres DB f.FileData, err = ioutil.ReadAll(fileContent) if err != nil { return err From 481c7c628431abdcaa2faefdeb2ad5c221890db0 Mon Sep 17 00:00:00 2001 From: Sonja Happ Date: Tue, 19 Oct 2021 15:56:50 +0200 Subject: [PATCH 9/9] fix error handling of s3.bucket param request also in upload method --- routes/file/file_methods.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/routes/file/file_methods.go b/routes/file/file_methods.go index eeb1b10..3e767eb 100644 --- a/routes/file/file_methods.go +++ b/routes/file/file_methods.go @@ -99,10 +99,9 @@ func (f *File) Register(fileHeader *multipart.FileHeader, scenarioID uint) error defer fileContent.Close() bucket, err := configuration.GlobalConfig.String("s3.bucket") - if err != nil { - return err - } - if bucket == "" { + if err != nil || bucket == "" { + // s3 object storage not used, s3.bucket param is empty + // save file to postgres DB f.FileData, err = ioutil.ReadAll(fileContent) if err != nil { return err