cmd: Fix deployment notes (#114)

Fixes the previously broken deployment note commands.

With the new deployment api and the introduction of the "deployment resource" concept, the api calls to /api/v1/deployments/{deployment_id}/notes no longer work in the same manner.

Until this api endpoint is updated to work using the deployment resource concept, the only deployment IDs it interacts with are the elasticsearch deployments related to the ES IDs (I tested this thoroughly using cURL directly as well as ecctl). So this means we cannot write notes on kibana, apm or appsearch for the time being. I left comments above all related code. In the UI all notes show up on the deployment's main page as well as the ES page.

The UI is using a different method, other than calls to this api endpoint, to leave comments on the deployments. This means that comments made through the UI will not show up on deployment note list.

Author: karencfv

cmd/deployment/note/command.go

@@ -38,16 +38,19 @@ var Command = &cobra.Command{
 }
 
 var deploymentNoteCreateCmd = &cobra.Command{
-	Use:     "create <deployment id> --message <message content> --type [elasticsearch|kibana|apm]",
+	Use:     "create <deployment id> --comment <comment content>",
 	Aliases: []string{"add"},
 	Short:   "Adds a note to a deployment",
 	PreRunE: cmdutil.MinimumNArgsAndUUID(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
+		comment, _ := cmd.Flags().GetString("comment")
 		return note.Add(note.AddParams{
-			API:     ecctl.Get().API,
-			ID:      args[0],
-			Type:    cmd.Flag("type").Value.String(),
-			Message: cmd.Flag("message").Value.String(),
+			Params: note.Params{
+				Params: deployment.Params{
+					API: ecctl.Get().API,
+					ID:  args[0],
+				}},
+			Message: comment,
 			UserID:  ecctl.Get().Config.User,
 		})
 	},
@@ -58,7 +61,7 @@ var deploymentNoteListCmd = &cobra.Command{
 	Short:   "Lists the deployment notes",
 	PreRunE: cmdutil.MinimumNArgsAndUUID(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		res, err := note.List(note.ListParams{
+		res, err := note.List(note.Params{
 			Params: deployment.Params{
 				API: ecctl.Get().API,
 				ID:  args[0],
@@ -73,16 +76,18 @@ var deploymentNoteListCmd = &cobra.Command{
 }
 
 var deploymentNoteUpdateCmd = &cobra.Command{
-	Use:     "update <deployment id> --id <note id> --message <message content>",
+	Use:     "update <deployment id> --id <note id> --comment <comment content>",
 	Short:   "Updates the deployment notes",
 	PreRunE: cmdutil.MinimumNArgsAndUUID(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
+		comment, _ := cmd.Flags().GetString("comment")
+		noteID, _ := cmd.Flags().GetString("id")
 		return util.ReturnErrOnly(
 			note.Update(note.UpdateParams{
-				Message: cmd.Flag("message").Value.String(),
+				Message: comment,
 				UserID:  ecctl.Get().Config.User,
+				NoteID:  noteID,
 				Params: note.Params{
-					NoteID: cmd.Flag("id").Value.String(),
 					Params: deployment.Params{
 						API: ecctl.Get().API,
 						ID:  args[0],
@@ -98,9 +103,10 @@ var deploymentNoteShowCmd = &cobra.Command{
 	Short:   "Shows a deployment note",
 	PreRunE: cmdutil.MinimumNArgsAndUUID(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
+		noteID, _ := cmd.Flags().GetString("id")
 		res, err := note.Get(note.GetParams{
+			NoteID: noteID,
 			Params: note.Params{
-				NoteID: cmd.Flag("id").Value.String(),
 				Params: deployment.Params{
 					API: ecctl.Get().API,
 					ID:  args[0],
@@ -123,7 +129,12 @@ func init() {
 		deploymentNoteShowCmd,
 	)
 
-	deploymentNoteCreateCmd.Flags().String("type", "elasticsearch", "Type of deployment to comment, valid options are: [elasticsearch, kibana, apm]")
+	deploymentNoteCreateCmd.Flags().String("comment", "", "Content of your deployment note")
+	deploymentNoteCreateCmd.MarkFlagRequired("comment")
+	deploymentNoteUpdateCmd.Flags().String("comment", "", "Content of your deployment note")
+	deploymentNoteUpdateCmd.MarkFlagRequired("comment")
 	deploymentNoteUpdateCmd.Flags().String("id", "", "Note ID")
+	deploymentNoteUpdateCmd.MarkFlagRequired("id")
 	deploymentNoteShowCmd.Flags().String("id", "", "Note ID")
+	deploymentNoteShowCmd.MarkFlagRequired("id")
 }

docs/ecctl_deployment_note_create.md

@@ -7,14 +7,14 @@ Adds a note to a deployment
 Adds a note to a deployment
 
 ```
-ecctl deployment note create <deployment id> --message <message content> --type [elasticsearch|kibana|apm] [flags]
+ecctl deployment note create <deployment id> --comment <comment content> [flags]
 ```
 
 ### Options
 
 ```
-  -h, --help          help for create
-      --type string   Type of deployment to comment, valid options are: [elasticsearch, kibana, apm] (default "elasticsearch")
+      --comment string   Content of your deployment note
+  -h, --help             help for create
 ```
 
 ### Options inherited from parent commands

docs/ecctl_deployment_note_update.md

@@ -7,14 +7,15 @@ Updates the deployment notes
 Updates the deployment notes
 
 ```
-ecctl deployment note update <deployment id> --id <note id> --message <message content> [flags]
+ecctl deployment note update <deployment id> --id <note id> --comment <comment content> [flags]
 ```
 
 ### Options
 
 ```
-  -h, --help        help for update
-      --id string   Note ID
+      --comment string   Content of your deployment note
+  -h, --help             help for update
+      --id string        Note ID
 ```
 
 ### Options inherited from parent commands

pkg/deployment/elasticsearch/reallocate.go

@@ -29,6 +29,7 @@ import (
 	"github.com/elastic/cloud-sdk-go/pkg/plan"
 	multierror "github.com/hashicorp/go-multierror"
 
+	"github.com/elastic/ecctl/pkg/deployment"
 	"github.com/elastic/ecctl/pkg/deployment/note"
 	"github.com/elastic/ecctl/pkg/ecctl"
 	"github.com/elastic/ecctl/pkg/util"
@@ -88,9 +89,11 @@ func Reallocate(params ReallocateParams) error {
 	// note to the deployment is not critical and is only a nice to have.
 	//nolint
 	note.Add(note.AddParams{
-		API:         params.API,
-		ID:          params.ClusterID,
-		Type:        "elasticsearch",
+		Params: note.Params{
+		Params: deployment.Params{
+			API:         params.API,
+			ID:          params.ClusterID,
+		}},
 		Message:     fmt.Sprintf(reallocateMessage, "elasticsearch", strings.Join(params.Instances, " ")),
 		UserID:      params.User,
 		Commentator: ecctl.GetOperationInstance(),

pkg/deployment/get.go

@@ -173,3 +173,14 @@ func GetKibana(params GetParams) (*models.KibanaResourceInfo, error) {
 	}
 	return res.Payload, nil
 }
+
+// GetElasticsearchID returns the deployment's elasticsearch resource ID
+func GetElasticsearchID(params GetParams) (string, error) {
+	res, err := Get(params)
+	if err != nil {
+		return "", err
+	}
+
+	esID := *res.Resources.Elasticsearch[0].ID
+	return esID, nil
+}

pkg/deployment/get_test.go

@@ -503,3 +503,77 @@ func TestGetKibana(t *testing.T) {
 		})
 	}
 }
+
+func TestGetElasticsearchID(t *testing.T) {
+	const getResponse = `{
+  "healthy": true,
+  "id": "e3dac8bf3dc64c528c295a94d0f19a77",
+  "resources": {
+    "elasticsearch": [{
+      "id": "418017cd1c7f402cbb7a981b2004ceeb",
+      "ref_id": "main-elasticsearch",
+      "region": "ece-region"
+    }]
+  }
+}`
+
+	const getErrorResponse = `{
+  "errors": null
+}`
+
+	type args struct {
+		params GetParams
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    string
+		wantErr bool
+		err     error
+	}{
+		{
+			name: "Get fails due to API failure",
+			args: args{
+				params: GetParams{
+					DeploymentID: "e3dac8bf3dc64c528c295a94d0f19a77",
+					API: api.NewMock(mock.Response{Response: http.Response{
+						Body:       mock.NewStringBody(""),
+						StatusCode: 500,
+					}}),
+				},
+			},
+			wantErr: true,
+			err:     errors.New(getErrorResponse),
+		},
+		{
+			name: "Get succeeds",
+			args: args{
+				params: GetParams{
+					DeploymentID: "e3dac8bf3dc64c528c295a94d0f19a77",
+					API: api.NewMock(mock.Response{Response: http.Response{
+						Body:       mock.NewStringBody(getResponse),
+						StatusCode: 200,
+					}}),
+				},
+			},
+			want: "418017cd1c7f402cbb7a981b2004ceeb",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetElasticsearchID(tt.args.params)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getElasticsearchID() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if tt.wantErr && tt.err != nil && err.Error() != tt.err.Error() {
+				t.Errorf("getElasticsearchID() actual error = '%v', want error '%v'", err, tt.err)
+			}
+
+			if !tt.wantErr && !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("getElasticsearchID() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/deployment/note/list.go

@@ -0,0 +1,51 @@
+// Licensed to Elasticsearch B.V. under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Elasticsearch B.V. licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package note
+
+import (
+	"github.com/elastic/cloud-sdk-go/pkg/api"
+	"github.com/elastic/cloud-sdk-go/pkg/client/deployments"
+	"github.com/elastic/cloud-sdk-go/pkg/models"
+)
+
+// ListParams is used by List
+type ListParams struct {
+	Params
+}
+
+// List lists all of the notes for the deployment
+func List(params Params) (*models.Notes, error) {
+	if err := params.Validate(); err != nil {
+		return nil, err
+	}
+
+	if err := params.fillDefaults(); err != nil {
+		return nil, err
+	}
+
+	res, err := params.API.V1API.Deployments.GetDeploymentNotes(
+		deployments.NewGetDeploymentNotesParams().
+			WithDeploymentID(params.ID),
+		params.AuthWriter,
+	)
+	if err != nil {
+		return nil, api.UnwrapError(err)
+	}
+
+	return res.Payload, nil
+}