Merge pull request #479 from CircleCI-Public/path-escape-and-version

Send proper user agent, and delete resource-classes by name

api/rest/client.go

@@ -10,6 +10,8 @@ import (
 	"net/url"
 	"strings"
 	"time"
+
+	"github.com/CircleCI-Public/circleci-cli/version"
 )
 
 type Client struct {
@@ -52,7 +54,7 @@ func (c *Client) NewRequest(method string, u *url.URL, payload interface{}) (req
 
 	req.Header.Set("Circle-Token", c.circleToken)
 	req.Header.Set("Accept-Type", "application/json")
-	req.Header.Set("User-Agent", "circleci-cli")
+	req.Header.Set("User-Agent", version.UserAgent())
 	if payload != nil {
 		req.Header.Set("Content-Type", "application/json")
 	}

api/rest/client_test.go

@@ -11,6 +11,8 @@ import (
 
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/assert/cmp"
+
+	"github.com/CircleCI-Public/circleci-cli/version"
 )
 
 func TestClient_DoRequest(t *testing.T) {
@@ -47,7 +49,7 @@ func TestClient_DoRequest(t *testing.T) {
 				"Circle-Token":    {"fake-token"},
 				"Content-Length":  {"20"},
 				"Content-Type":    {"application/json"},
-				"User-Agent":      {"circleci-cli"},
+				"User-Agent":      {version.UserAgent()},
 			}))
 			assert.Check(t, cmp.Equal(fix.Body(), `{"A":"aaa","B":123}`+"\n"))
 		})
@@ -76,7 +78,7 @@ func TestClient_DoRequest(t *testing.T) {
 				"Accept-Encoding": {"gzip"},
 				"Accept-Type":     {"application/json"},
 				"Circle-Token":    {"fake-token"},
-				"User-Agent":      {"circleci-cli"},
+				"User-Agent":      {version.UserAgent()},
 			}))
 			assert.Check(t, cmp.Equal(fix.Body(), ""))
 		})
@@ -108,7 +110,7 @@ func TestClient_DoRequest(t *testing.T) {
 				"Accept-Encoding": {"gzip"},
 				"Accept-Type":     {"application/json"},
 				"Circle-Token":    {"fake-token"},
-				"User-Agent":      {"circleci-cli"},
+				"User-Agent":      {version.UserAgent()},
 			}))
 			assert.Check(t, cmp.Equal(fix.Body(), ""))
 		})

api/runner/runner.go

@@ -1,7 +1,9 @@
 package runner
 
 import (
+	"fmt"
 	"net/url"
+	"strings"
 	"time"
 
 	"github.com/CircleCI-Public/circleci-cli/api/rest"
@@ -38,6 +40,27 @@ func (r *Runner) CreateResourceClass(resourceClass, desc string) (rc *ResourceCl
 	return rc, err
 }
 
+func (r *Runner) GetResourceClassByName(resourceClass string) (rc *ResourceClass, err error) {
+	s := strings.SplitN(resourceClass, "/", 2)
+	if len(s) != 2 {
+		return nil, fmt.Errorf("bad resource class: %q", resourceClass)
+	}
+
+	namespace := s[0]
+	rcs, err := r.GetResourceClassesByNamespace(namespace)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, rc := range rcs {
+		if rc.ResourceClass == resourceClass {
+			return &rc, nil
+		}
+	}
+
+	return nil, fmt.Errorf("resource class %q not found", resourceClass)
+}
+
 func (r *Runner) GetResourceClassesByNamespace(namespace string) ([]ResourceClass, error) {
 	query := url.Values{}
 	query.Add("namespace", namespace)
@@ -54,7 +77,7 @@ func (r *Runner) GetResourceClassesByNamespace(namespace string) ([]ResourceClas
 }
 
 func (r *Runner) DeleteResourceClass(id string) error {
-	req, err := r.rc.NewRequest("DELETE", &url.URL{Path: "runner/resource/" + url.QueryEscape(id)}, nil)
+	req, err := r.rc.NewRequest("DELETE", &url.URL{Path: "runner/resource/" + url.PathEscape(id)}, nil)
 	if err != nil {
 		return err
 	}
@@ -106,7 +129,7 @@ func (r *Runner) GetRunnerTokensByResourceClass(resourceClass string) ([]Token,
 }
 
 func (r *Runner) DeleteToken(id string) error {
-	req, err := r.rc.NewRequest("DELETE", &url.URL{Path: "runner/token/" + url.QueryEscape(id)}, nil)
+	req, err := r.rc.NewRequest("DELETE", &url.URL{Path: "runner/token/" + url.PathEscape(id)}, nil)
 	if err != nil {
 		return err
 	}

api/runner/runner_test.go

@@ -14,6 +14,7 @@ import (
 	"gotest.tools/v3/assert/cmp"
 
 	"github.com/CircleCI-Public/circleci-cli/api/rest"
+	"github.com/CircleCI-Public/circleci-cli/version"
 )
 
 func TestRunner_CreateResourceClass(t *testing.T) {
@@ -48,12 +49,57 @@ func TestRunner_CreateResourceClass(t *testing.T) {
 			"Circle-Token":    {"fake-token"},
 			"Content-Length":  {"86"},
 			"Content-Type":    {"application/json"},
-			"User-Agent":      {"circleci-cli"},
+			"User-Agent":      {version.UserAgent()},
 		}))
 		assert.Check(t, cmp.Equal(fix.Body(), `{"resource_class":"the-namespace/the-resource-class","description":"the-description"}`+"\n"))
 	})
 }
 
+func TestRunner_GetResourceClassByName(t *testing.T) {
+	fix := fixture{}
+	runner, cleanup := fix.Run(
+		http.StatusOK,
+		`
+{
+	"items": [
+		{"id": "7101f2a4-1617-4ef9-8fd4-f72de73896bd", "resource_class": "the-namespace/the-resource-class-1", "description": "the-description-1"},
+		{"id": "b2713ad1-13b9-44f6-9b0d-1bf5f38571db", "resource_class": "the-namespace/the-resource-class-2", "description": "the-one-we-want"},
+		{"id": "aa8cdb84-bc8e-4e42-a04a-8e719b586069", "resource_class": "the-namespace/the-resource-class-3", "description": "the-description-3"}
+	]
+}`,
+	)
+	defer cleanup()
+
+	t.Run("Check resource-class list results", func(t *testing.T) {
+		rc, err := runner.GetResourceClassByName("the-namespace/the-resource-class-2")
+		assert.NilError(t, err)
+		assert.Check(t, cmp.DeepEqual(rc, &ResourceClass{
+			ID:            "b2713ad1-13b9-44f6-9b0d-1bf5f38571db",
+			ResourceClass: "the-namespace/the-resource-class-2",
+			Description:   "the-one-we-want",
+		}))
+	})
+
+	t.Run("Check request", func(t *testing.T) {
+		assert.Check(t, cmp.Equal(fix.URL(), url.URL{Path: "/api/v2/runner/resource", RawQuery: "namespace=the-namespace"}))
+		assert.Check(t, cmp.Equal(fix.method, "GET"))
+		assert.Check(t, cmp.DeepEqual(fix.Header(), http.Header{
+			"Accept-Encoding": {"gzip"},
+			"Accept-Type":     {"application/json"},
+			"Circle-Token":    {"fake-token"},
+			"User-Agent":      {version.UserAgent()},
+		}))
+		assert.Check(t, cmp.Equal(fix.Body(), ``))
+	})
+}
+
+func TestRunner_GetResourceClassByName_BadResourceClass(t *testing.T) {
+	r := Runner{}
+	rc, err := r.GetResourceClassByName("there-is-no-slash")
+	assert.Check(t, cmp.Nil(rc))
+	assert.ErrorContains(t, err, "bad resource class")
+}
+
 func TestRunner_GetResourceClassesByNamespace(t *testing.T) {
 	fix := fixture{}
 	runner, cleanup := fix.Run(
@@ -92,7 +138,7 @@ func TestRunner_GetResourceClassesByNamespace(t *testing.T) {
 			"Accept-Encoding": {"gzip"},
 			"Accept-Type":     {"application/json"},
 			"Circle-Token":    {"fake-token"},
-			"User-Agent":      {"circleci-cli"},
+			"User-Agent":      {version.UserAgent()},
 		}))
 		assert.Check(t, cmp.Equal(fix.Body(), ``))
 	})
@@ -115,12 +161,27 @@ func TestRunner_DeleteResourceClass(t *testing.T) {
 			"Accept-Encoding": {"gzip"},
 			"Accept-Type":     {"application/json"},
 			"Circle-Token":    {"fake-token"},
-			"User-Agent":      {"circleci-cli"},
+			"User-Agent":      {version.UserAgent()},
 		}))
 		assert.Check(t, cmp.Equal(fix.Body(), ``))
 	})
 }
 
+func TestRunner_DeleteResourceClass_PathEscaping(t *testing.T) {
+	fix := fixture{}
+	runner, cleanup := fix.Run(http.StatusOK, ``)
+	defer cleanup()
+
+	t.Run("Check resource-class is deleted", func(t *testing.T) {
+		err := runner.DeleteResourceClass("escape~,/;?~noescape~$&+:=@")
+		assert.NilError(t, err)
+	})
+
+	t.Run("Check request", func(t *testing.T) {
+		assert.Check(t, cmp.Equal(fix.URL(), url.URL{Path: "/api/v2/runner/resource/escape~%2C%2F%3B%3F~noescape~$&+:=@"}))
+	})
+}
+
 func TestRunner_CreateToken(t *testing.T) {
 	fix := fixture{}
 	runner, cleanup := fix.Run(
@@ -155,7 +216,7 @@ func TestRunner_CreateToken(t *testing.T) {
 			"Circle-Token":    {"fake-token"},
 			"Content-Length":  {"80"},
 			"Content-Type":    {"application/json"},
-			"User-Agent":      {"circleci-cli"},
+			"User-Agent":      {version.UserAgent()},
 		}))
 		assert.Check(t, cmp.Equal(fix.Body(), `{"resource_class":"the-namespace/the-resource-class","nickname":"the-nickname"}`+"\n"))
 	})
@@ -223,7 +284,7 @@ func TestRunner_GetRunnerTokensByResourceClass(t *testing.T) {
 			"Accept-Encoding": {"gzip"},
 			"Accept-Type":     {"application/json"},
 			"Circle-Token":    {"fake-token"},
-			"User-Agent":      {"circleci-cli"},
+			"User-Agent":      {version.UserAgent()},
 		}))
 		assert.Check(t, cmp.Equal(fix.Body(), ""))
 	})
@@ -246,12 +307,27 @@ func TestRunner_DeleteToken(t *testing.T) {
 			"Accept-Encoding": {"gzip"},
 			"Accept-Type":     {"application/json"},
 			"Circle-Token":    {"fake-token"},
-			"User-Agent":      {"circleci-cli"},
+			"User-Agent":      {version.UserAgent()},
 		}))
 		assert.Check(t, cmp.Equal(fix.Body(), ``))
 	})
 }
 
+func TestRunner_DeleteToken_PathEscaping(t *testing.T) {
+	fix := fixture{}
+	runner, cleanup := fix.Run(http.StatusOK, ``)
+	defer cleanup()
+
+	t.Run("Check token is deleted", func(t *testing.T) {
+		err := runner.DeleteToken("escape~,/;?~noescape~$&+:=@")
+		assert.NilError(t, err)
+	})
+
+	t.Run("Check request", func(t *testing.T) {
+		assert.Check(t, cmp.Equal(fix.URL(), url.URL{Path: "/api/v2/runner/token/escape~%2C%2F%3B%3F~noescape~$&+:=@"}))
+	})
+}
+
 type fixture struct {
 	mu     sync.Mutex
 	url    url.URL

cmd/runner/resource_class.go

@@ -39,7 +39,11 @@ func newResourceClassCommand(r *runner.Runner, preRunE validator) *cobra.Command
 		Args:    cobra.ExactArgs(1),
 		PreRunE: preRunE,
 		RunE: func(_ *cobra.Command, args []string) error {
-			return r.DeleteResourceClass(args[0])
+			rc, err := r.GetResourceClassByName(args[0])
+			if err != nil {
+				return err
+			}
+			return r.DeleteResourceClass(rc.ID)
 		},
 	})
 
@@ -70,10 +74,10 @@ func newResourceClassCommand(r *runner.Runner, preRunE validator) *cobra.Command
 
 func newResourceClassTable() *tablewriter.Table {
 	table := tablewriter.NewWriter(os.Stdout)
-	table.SetHeader([]string{"ID", "Resource Class", "Description"})
+	table.SetHeader([]string{"Resource Class", "Description"})
 	return table
 }
 
 func appendResourceClass(table *tablewriter.Table, rc runner.ResourceClass) {
-	table.Append([]string{rc.ID, rc.ResourceClass, rc.Description})
+	table.Append([]string{rc.ResourceClass, rc.Description})
 }