Test Cleanup

controllers/networkHttpController.go

@@ -295,13 +295,9 @@ func AlertNetwork(netid string) error {
 
 //Update a network
 func updateNetwork(w http.ResponseWriter, r *http.Request) {
-
 	w.Header().Set("Content-Type", "application/json")
-
 	var params = mux.Vars(r)
-
 	var network models.Network
-
 	network, err := functions.GetParentNetwork(params["networkname"])
 	if err != nil {
 		returnErrorResponse(w, r, formatError(err, "internal"))
@@ -310,10 +306,6 @@ func updateNetwork(w http.ResponseWriter, r *http.Request) {
 
 	var networkChange models.NetworkUpdate
 
-	haschange := false
-	hasrangeupdate := false
-	haslocalrangeupdate := false
-
 	_ = json.NewDecoder(r.Body).Decode(&networkChange)
 	if networkChange.AddressRange == "" {
 		networkChange.AddressRange = network.AddressRange
@@ -330,39 +322,36 @@ func updateNetwork(w http.ResponseWriter, r *http.Request) {
 		returnErrorResponse(w, r, formatError(err, "badrequest"))
 		return
 	}
+	returnednetwork, err := UpdateNetwork(networkChange, network)
+	if err != nil {
+		returnErrorResponse(w, r, formatError(err, "badrequest"))
+		return
+	}
+
+	w.WriteHeader(http.StatusOK)
+	json.NewEncoder(w).Encode(returnednetwork)
+}
 
+func UpdateNetwork(networkChange models.NetworkUpdate, network models.Network) (models.Network, error) {
 	//NOTE: Network.NetID is intentionally NOT editable. It acts as a static ID for the network.
 	//DisplayName can be changed instead, which is what shows on the front end
 	if networkChange.NetID != network.NetID {
-		returnErrorResponse(w, r, formatError(errors.New("NetID is not editable"), "badrequest"))
-		return
+		return models.Network{}, errors.New("NetID is not editable")
 	}
-	//MRK:  I think this code block is redundant.  valdiateNetworkUpdate(networkChange) covers this
-	if networkChange.AddressRange != "" {
 
-		network.AddressRange = networkChange.AddressRange
+	haschange := false
+	hasrangeupdate := false
+	haslocalrangeupdate := false
 
-		var isAddressOK bool = functions.IsIpCIDR(networkChange.AddressRange)
-		if !isAddressOK {
-			err := errors.New("Invalid Range of " + networkChange.AddressRange + " for addresses.")
-			returnErrorResponse(w, r, formatError(err, "internal"))
-			return
-		}
+	if networkChange.AddressRange != "" {
 		haschange = true
 		hasrangeupdate = true
-
+		network.AddressRange = networkChange.AddressRange
 	}
 	if networkChange.LocalRange != "" {
-		network.LocalRange = networkChange.LocalRange
-
-		var isAddressOK bool = functions.IsIpCIDR(networkChange.LocalRange)
-		if !isAddressOK {
-			err := errors.New("Invalid Range of " + networkChange.LocalRange + " for internal addresses.")
-			returnErrorResponse(w, r, formatError(err, "internal"))
-			return
-		}
 		haschange = true
 		haslocalrangeupdate = true
+		network.LocalRange = networkChange.LocalRange
 	}
 	if networkChange.IsLocal != nil {
 		network.IsLocal = networkChange.IsLocal
@@ -405,7 +394,7 @@ func updateNetwork(w http.ResponseWriter, r *http.Request) {
 
 	collection := mongoconn.Client.Database("netmaker").Collection("networks")
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	filter := bson.M{"netid": params["networkname"]}
+	filter := bson.M{"netid": network.NetID}
 
 	if haschange {
 		network.SetNetworkLastModified()
@@ -432,38 +421,32 @@ func updateNetwork(w http.ResponseWriter, r *http.Request) {
 		}},
 	}
 
-	err = collection.FindOneAndUpdate(ctx, filter, update).Decode(&network)
+	err := collection.FindOneAndUpdate(ctx, filter, update).Decode(&network)
 	defer cancel()
 
 	if err != nil {
-		returnErrorResponse(w, r, formatError(err, "internal"))
-		return
+		return models.Network{}, err
 	}
 
 	//Cycles through nodes and gives them new IP's based on the new range
 	//Pretty cool, but also pretty inefficient currently
 	if hasrangeupdate {
-		err = functions.UpdateNetworkNodeAddresses(params["networkname"])
+		err = functions.UpdateNetworkNodeAddresses(network.NetID)
 		if err != nil {
-			returnErrorResponse(w, r, formatError(err, "internal"))
-			return
+			return models.Network{}, err
 		}
 	}
 	if haslocalrangeupdate {
-		err = functions.UpdateNetworkPrivateAddresses(params["networkname"])
+		err = functions.UpdateNetworkPrivateAddresses(network.NetID)
 		if err != nil {
-			returnErrorResponse(w, r, formatError(err, "internal"))
-			return
+			return models.Network{}, err
 		}
 	}
 	returnnetwork, err := functions.GetParentNetwork(network.NetID)
 	if err != nil {
-		returnErrorResponse(w, r, formatError(err, "internal"))
-		return
+		return models.Network{}, err
 	}
-
-	w.WriteHeader(http.StatusOK)
-	json.NewEncoder(w).Encode(returnnetwork)
+	return returnnetwork, nil
 }
 
 //Delete a network
@@ -600,6 +583,7 @@ func createAccessKey(w http.ResponseWriter, r *http.Request) {
 }
 
 func CreateAccessKey(accesskey models.AccessKey, network models.Network) (models.AccessKey, error) {
+	fmt.Println(accesskey)
 	if accesskey.Name == "" {
 		accesskey.Name = functions.GenKeyName()
 	}

controllers/networkHttpController_test.go

@@ -31,6 +31,10 @@ func createNet() {
 		CreateNetwork(network)
 	}
 }
+func getNet() models.Network {
+	network, _ := GetNetwork("skynet")
+	return network
+}
 
 func TestGetNetworks(t *testing.T) {
 	//calls functions.ListNetworks --- nothing to be done
@@ -88,6 +92,24 @@ func TestGetNetwork(t *testing.T) {
 	})
 }
 func TestUpdateNetwork(t *testing.T) {
+	createNet()
+	network := getNet()
+	t.Run("NetID", func(t *testing.T) {
+		var networkupdate models.NetworkUpdate
+		networkupdate.NetID = "wirecat"
+		_, err := UpdateNetwork(networkupdate, network)
+		assert.NotNil(t, err)
+		assert.Equal(t, "NetID is not editable", err.Error())
+	})
+	t.Run("LocalRange", func(t *testing.T) {
+		var networkupdate models.NetworkUpdate
+		//NetID needs to be set as it will be in updateNetwork
+		networkupdate.NetID = "skynet"
+		networkupdate.LocalRange = "192.168.0.1/24"
+		update, err := UpdateNetwork(networkupdate, network)
+		assert.Nil(t, err)
+		t.Log(err, update)
+	})
 }
 
 func TestKeyUpdate(t *testing.T) {

models/network.go

@@ -44,7 +44,7 @@ type NetworkUpdate struct {
 	AddressRange6 string `json:"addressrange6" bson:"addressrange6" validate:"omitempty,cidr"`
 	//can't have min=1 with omitempty
 	DisplayName         string      `json:"displayname,omitempty" bson:"displayname,omitempty" validate:"omitempty,alphanum,min=2,max=20"`
-	NetID               string      `json:"netid" bson:"netid" validate:"alphanum,min=1,max=12"`
+	NetID               string      `json:"netid" bson:"netid" validate:"omitempty,alphanum,min=1,max=12"`
 	NodesLastModified   int64       `json:"nodeslastmodified" bson:"nodeslastmodified"`
 	NetworkLastModified int64       `json:"networklastmodified" bson:"networklastmodified"`
 	DefaultInterface    string      `json:"defaultinterface" bson:"defaultinterface"`

test/api_test.go

@@ -152,15 +152,19 @@ func createKey(t *testing.T) {
 }
 
 func createAccessKey(t *testing.T) (key models.AccessKey) {
+	//delete existing key if
+	_, _ = api(t, "", http.MethodDelete, baseURL+"/api/networks/skynet/keys/skynet", "secretkey")
 	createkey := models.AccessKey{}
 	createkey.Name = "skynet"
 	createkey.Uses = 10
 	response, err := api(t, createkey, http.MethodPost, baseURL+"/api/networks/skynet/keys", "secretkey")
+	t.Log(err, response)
 	assert.Nil(t, err, err)
 	assert.Equal(t, http.StatusOK, response.StatusCode)
 	defer response.Body.Close()
 	err = json.NewDecoder(response.Body).Decode(&key)
 	assert.Nil(t, err, err)
+	t.Log("key is ", key)
 	return key
 }
 

test/node_test.go

@@ -503,8 +503,8 @@ func TestCreateNode(t *testing.T) {
 		assert.Contains(t, "W1R3: Network does not exist! ", message.Message)
 	})
 	t.Run("Valid", func(t *testing.T) {
-		setup(t)
-		deleteNode(t)
+		deleteNetworks(t)
+		createNetwork(t)
 		key := createAccessKey(t)
 
 		var node models.Node