Merge pull request ystia#581 from ystia/bugfix/ystiaGH-574-locations-…

…config-apply

Apply locations config file even when no locations are configured

CHANGELOG.md

@@ -33,6 +33,7 @@ func GetWorkflow(ctx context.Context, deploymentID, workflowName string) (*tosca
 * Deployment stuck and cannot be resumed in certain circumstances ([GH-563](https://github.com/ystia/yorc/issues/563))
 * Yorc bootstrap on 4.0.0-M7 doesn't work unless an alternative download URL is provided for Yorc ([GH-561](https://github.com/ystia/yorc/issues/561))
 * Location properties stored in Vault are no longer resolvable ([GH-565](https://github.com/ystia/yorc/issues/565))
+* If no locations configured when first starting a Yorc server, the command "locations apply config_locations_file_path" won't work ([GH-574](https://github.com/ystia/yorc/issues/5674))
 * An error during deployment purge may let the deployment in a wrong state ([GH-572](https://github.com/ystia/yorc/issues/572))
 * Can have current deployment and undeployment on the same application on specific conditions ([GH-567](https://github.com/ystia/yorc/issues/567))
 * API calls to deploy and update a deployment will now prevent other API calls that may modify a deployment to run at the same time

commands/locations/locations_apply.go

@@ -76,14 +76,23 @@ func readConfigFile(client httputil.HTTPClient, path string) (*rest.LocationsCol
 	return locationsApplied, nil
 }
 
-func applyLocationsConfig(client httputil.HTTPClient, args []string, autoApprove bool) error {
-	colorize := !noColor
+// Get locations definitions from a file proposed by the client
+func readLocationsConfig(client httputil.HTTPClient, args []string) (*rest.LocationsCollection, error) {
 	if len(args) != 1 {
-		return errors.Errorf("Expecting a path to a file (got %d parameters)", len(args))
+		return nil, errors.Errorf("Expecting a path to a file (got %d parameters)", len(args))
+	}
+	locationsApplied, err := readConfigFile(client, args[0])
+	if err != nil {
+		return nil, err
 	}
+	return locationsApplied, nil
+}
+
+func applyLocationsConfig(client httputil.HTTPClient, args []string, autoApprove bool) error {
+	colorize := !noColor
 
 	// Get locations definitions proposed by the client
-	locationsApplied, err := readConfigFile(client, args[0])
+	locationsApplied, err := readLocationsConfig(client, args)
 	if err != nil {
 		return err
 	}
@@ -100,52 +109,51 @@ func applyLocationsConfig(client httputil.HTTPClient, args []string, autoApprove
 	deleteLocationsMap := make(map[string]rest.LocationConfiguration)
 
 	// Get existent locations configuration
-	locsConfig, err := getLocationsConfig(client)
-	for _, locConfig := range locsConfig.Locations {
-		if newLocConfig, ok := newLocationsMap[locConfig.Name]; ok {
-			// newLocConfig corresponds to an already defined location
-			// Check if there is any change before registering the need to update
-			if locConfig.Type != newLocConfig.Type ||
-				!reflect.DeepEqual(locConfig.Properties, newLocConfig.Properties) {
-				// Add newLocConfig to the map for locations to update
-				updateLocationsMap[locConfig.Name] = newLocConfig
-			}
-			// Delete newLocConfig from the map of locations to create
-			delete(newLocationsMap, locConfig.Name)
-		} else {
-			// locConfig is not in the new locations specifications, delete it from consul
-			deleteLocationsMap[locConfig.Name] = locConfig
-		}
+	locsConfig, err := getLocationsConfig(client, false)
+	if err != nil {
+		return err
+	}
+	// Use an array for existent locations configuration
+	// to avoid nesting a for that contains if/the/else, in an if
+	var existentLocsConfig []rest.LocationConfiguration
+	if locsConfig != nil {
+		existentLocsConfig = locsConfig.Locations
+	} else {
+		existentLocsConfig = make([]rest.LocationConfiguration, 0)
+	}
+
+	// update newLocationsMap, updateLocationsMap and deleteLocationsMap
+	// based on already existent locations configurations
+	for _, existLocConfig := range existentLocsConfig {
+		amendLocationsMap(newLocationsMap, updateLocationsMap, deleteLocationsMap, existLocConfig)
 	}
 
 	// Present locations to be created
-	presentLocationsMap(&newLocationsMap, colorize, locationCreation)
+	presentLocationsMap(newLocationsMap, colorize, locationCreation)
 
 	// Present locations to be updated
-	presentLocationsMap(&updateLocationsMap, colorize, locationUpdate)
+	presentLocationsMap(updateLocationsMap, colorize, locationUpdate)
 
 	// Present locations to be deleted
-	presentLocationsMap(&deleteLocationsMap, colorize, locationDeletion)
+	presentLocationsMap(deleteLocationsMap, colorize, locationDeletion)
 
-	if (len(newLocationsMap) + len(updateLocationsMap) + len(deleteLocationsMap)) > 0 {
-		// Changes to do. Let's see what user decides
-		if !approveToApply(autoApprove) {
-			return nil
-		}
+	// If changes to do, ask user if he approve these changes
+	if ((len(newLocationsMap) + len(updateLocationsMap) + len(deleteLocationsMap)) > 0) && !approveToApply(autoApprove) {
+		return nil
 	}
 
 	// Proceed to apply changes
-	err = doApply(client, &newLocationsMap, &updateLocationsMap, &deleteLocationsMap)
+	err = doApply(client, newLocationsMap, updateLocationsMap, deleteLocationsMap)
 	if err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func doApply(client httputil.HTTPClient, createMap, updateMap, deleteMap *map[string]rest.LocationConfiguration) error {
+func doApply(client httputil.HTTPClient, createMap, updateMap, deleteMap map[string]rest.LocationConfiguration) error {
 	// Proceed to the create
-	for _, newLocation := range *createMap {
+	for _, newLocation := range createMap {
 		locConfig := rest.LocationConfiguration{
 			Name:       newLocation.Name,
 			Type:       newLocation.Type,
@@ -162,7 +170,7 @@ func doApply(client httputil.HTTPClient, createMap, updateMap, deleteMap *map[st
 	}
 
 	// Proceed to update
-	for _, updateLocation := range *updateMap {
+	for _, updateLocation := range updateMap {
 		locConfig := rest.LocationConfiguration{
 			Name:       updateLocation.Name,
 			Type:       updateLocation.Type,
@@ -179,7 +187,7 @@ func doApply(client httputil.HTTPClient, createMap, updateMap, deleteMap *map[st
 	}
 
 	// Proceed to delete
-	for locNameToDelete := range *deleteMap {
+	for locNameToDelete := range deleteMap {
 		err := deleteLocationConfig(client, locNameToDelete)
 		if err != nil {
 			return err
@@ -192,17 +200,37 @@ func doApply(client httputil.HTTPClient, createMap, updateMap, deleteMap *map[st
 	return nil
 }
 
-func presentLocationsMap(locationsMap *map[string]rest.LocationConfiguration, colorize bool, op int) {
-	if len(*locationsMap) > 0 {
+func amendLocationsMap(newLocationsMap, updateLocationsMap, deleteLocationsMap map[string]rest.LocationConfiguration, existLocConfig rest.LocationConfiguration) {
+	locName := existLocConfig.Name
+	newLocConfig, ok := newLocationsMap[locName]
+	checkUpdate := false
+	if ok {
+		// newLocConfig corresponds to an already defined location
+		// Delete newLocConfig from the map of locations to create
+		delete(newLocationsMap, locName)
+		checkUpdate = true
+	} else {
+		// locConfig is not in the new locations specifications, delete it from consul
+		deleteLocationsMap[locName] = existLocConfig
+	}
+	// Check if there is any change before registering the need to update
+	if checkUpdate && (existLocConfig.Type != newLocConfig.Type || !reflect.DeepEqual(existLocConfig.Properties, newLocConfig.Properties)) {
+		// Add newLocConfig to the map for locations to update
+		updateLocationsMap[locName] = newLocConfig
+	}
+}
+
+func presentLocationsMap(locationsMap map[string]rest.LocationConfiguration, colorize bool, op int) {
+	if len(locationsMap) > 0 {
 		locationsTable := tabutil.NewTable()
 		locationsTable.AddHeaders("Name", "Type", "Properties")
-		for _, locConfig := range *locationsMap {
+		for _, locConfig := range locationsMap {
 			addRow(locationsTable, colorize, op, locConfig)
 		}
 		fmt.Printf("\n- Locations to %s:", opNames[op])
 		fmt.Println("")
 		fmt.Println(locationsTable.Render())
-		fmt.Printf("Number of locations to %s : %v ", opNames[op], len(*locationsMap))
+		fmt.Printf("Number of locations to %s : %v ", opNames[op], len(locationsMap))
 		fmt.Println("")
 	}
 }

commands/locations/locations_apply_on_empty_test.go

@@ -0,0 +1,67 @@
+// Copyright 2019 Bull S.A.S. Atos Technologies - Bull, Rue Jean Jaures, B.P.68, 78340, Les Clayes-sous-Bois, France.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package locations
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// Mock client implementation for the apply command
+
+type httpMockClientApplyOnEmpty struct {
+}
+
+func (c *httpMockClientApplyOnEmpty) NewRequest(method, path string, body io.Reader) (*http.Request, error) {
+	return http.NewRequest(method, path, body)
+}
+
+func (c *httpMockClientApplyOnEmpty) Do(req *http.Request) (*http.Response, error) {
+
+	if req.Method == "POST" || req.Method == "PUT" {
+		res := httptest.ResponseRecorder{Code: 201}
+		return res.Result(), nil
+	}
+
+	w := httptest.NewRecorder()
+
+	return w.Result(), nil
+}
+
+func (c *httpMockClientApplyOnEmpty) Get(path string) (*http.Response, error) {
+	return &http.Response{}, nil
+}
+
+func (c *httpMockClientApplyOnEmpty) Head(path string) (*http.Response, error) {
+	return &http.Response{}, nil
+}
+
+func (c *httpMockClientApplyOnEmpty) Post(path string, contentType string, body io.Reader) (*http.Response, error) {
+	return &http.Response{}, nil
+}
+
+func (c *httpMockClientApplyOnEmpty) PostForm(path string, data url.Values) (*http.Response, error) {
+	return &http.Response{}, nil
+}
+
+func TestLocationApply(t *testing.T) {
+	err := applyLocationsConfig(&httpMockClientApplyOnEmpty{}, []string{"./testdata/locations.json"}, true)
+	require.NoError(t, err, "Failed to apply locations config on empty list")
+}

commands/locations/locations_apply_test.go

@@ -112,7 +112,7 @@ func TestLocationApplyWithDirPath(t *testing.T) {
 	require.Error(t, err, "Expecting a path to a file")
 }
 
-func TestLocationApply(t *testing.T) {
+func TestLocationApplyOk(t *testing.T) {
 	err := applyLocationsConfig(&httpMockClientApply{}, []string{"./testdata/locations.json"}, true)
 	require.NoError(t, err, "Failed to apply locations config")
 }

commands/locations/locations_list.go

@@ -46,7 +46,7 @@ var listCmd = &cobra.Command{
 
 func listLocations(client httputil.HTTPClient, args []string) error {
 	// Get locations definitions
-	locsConfig, err := getLocationsConfig(client)
+	locsConfig, err := getLocationsConfig(client, true)
 	if err != nil {
 		httputil.ErrExit(err)
 	}
@@ -72,8 +72,12 @@ func listLocations(client httputil.HTTPClient, args []string) error {
 	return nil
 }
 
-// getLocationsConfig makes a GET request to locations API and returns the existent location definitions
-func getLocationsConfig(client httputil.HTTPClient) (*rest.LocationsCollection, error) {
+// getLocationsConfig uses the HTTP client to make a request to locations API.
+// It returns a collection of existent location definitions. If no location definition exist,
+// the treatement depends on retOnError value :
+// - true : the command returns and status code displayed
+// - false: the function returns a nil value to the caller
+func getLocationsConfig(client httputil.HTTPClient, retOnError bool) (*rest.LocationsCollection, error) {
 	request, err := client.NewRequest("GET", "/locations", nil)
 	if err != nil {
 		return nil, err
@@ -84,13 +88,20 @@ func getLocationsConfig(client httputil.HTTPClient) (*rest.LocationsCollection,
 		return nil, err
 	}
 	defer response.Body.Close()
-	httputil.HandleHTTPStatusCode(response, "", "locations", http.StatusOK)
-	var locRefs rest.LocationCollection
-
 	body, err := ioutil.ReadAll(response.Body)
 	if err != nil {
 		return nil, err
 	}
+	if retOnError {
+		httputil.HandleHTTPStatusCode(response, "", "locations", http.StatusOK)
+	} else {
+		// check for body content and renturn nil result if no values found
+		if len(body) == 0 {
+			return nil, nil
+		}
+	}
+
+	var locRefs rest.LocationCollection
 	err = json.Unmarshal(body, &locRefs)
 	if err != nil {
 		return nil, err

doc/cli.rst

@@ -339,6 +339,13 @@ Flags:
   * ``--data`` or ``-d`` :  Specify a JSON format for location definition to add.
 
 
+Example of location definition "testname" using ``--data`` flag:
+
+.. code-block:: bash
+
+     yorc locations add --data '{"name": "testname", "type": "t", "properties" : { "p1" : "v1", "p2" : "v2" }}'
+
+
 Update a location
 ~~~~~~~~~~~~~~~~~
 
@@ -351,6 +358,14 @@ Update a given location's definition.
 Flags:
   * ``--data`` or ``-d`` :  Specify a JSON format for the location definition to update.
 
+
+Example of "testname" location update using ``--data`` flag:
+
+.. code-block:: bash
+
+     yorc locations add --data '{"name": "testname", "type": "other", "properties" : { "p1" : "v111" }}'
+
+
 Delete a location
 ~~~~~~~~~~~~~~~~~