stabilize client API and improve endpoint URL generation

This commit stabilizes the client API for a `v1.0.0`. Therefore, this commit renames the event log functions: - `TraceAuditLog()` => `AuditLog()` - `TraceErrorLog()` => `ErrorLog()` Further, this commit improves the endpoint URL generation by the client. Before, an endpoint string like `https://play.min.io:7373/` (whitespace) would have caused an endpoint URL with two `/`. Now, all whitespaces are removed from the base endpoint, parameters are URL-escaped and the endpoint URL is built properly.
minio · Aug 18, 2020 · 945afb7 · 945afb7
1 parent 558854a
commit 945afb7
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 30 deletions.
diff --git a/client.go b/client.go
@@ -10,11 +10,12 @@ import (
 	"encoding"
 	"encoding/base64"
 	"encoding/json"
-	"fmt"
 	"io"
 	"net"
 	"net/http"
 	"net/url"
+	"path"
+	"strings"
 	"time"
 )
 
@@ -191,7 +192,7 @@ func (d *DEK) UnmarshalBinary(data []byte) error {
 // KES server.
 func (c *Client) Version() (string, error) {
 	client := retry(c.HTTPClient)
-	resp, err := client.Get(fmt.Sprintf("%s/version", c.Endpoint))
+	resp, err := client.Get(endpoint(c.Endpoint, "/version"))
 	if err != nil {
 		return "", err
 	}
@@ -216,9 +217,9 @@ func (c *Client) Version() (string, error) {
 // The key will be generated by the server. The client
 // application does not have the cryptographic key at
 // any point in time.
-func (c *Client) CreateKey(key string) error {
+func (c *Client) CreateKey(name string) error {
 	client := retry(c.HTTPClient)
-	resp, err := client.Post(fmt.Sprintf("%s/v1/key/create/%s", c.Endpoint, key), "application/json", nil)
+	resp, err := client.Post(endpoint(c.Endpoint, "/v1/key/create", url.PathEscape(name)), "application/json", nil)
 	if err != nil {
 		return err
 	}
@@ -245,7 +246,7 @@ func (c *Client) ImportKey(name string, key []byte) error {
 	}
 
 	client := retry(c.HTTPClient)
-	url := fmt.Sprintf("%s/v1/key/import/%s", c.Endpoint, name)
+	url := endpoint(c.Endpoint, "/v1/key/import", url.PathEscape(name))
 	resp, err := client.Post(url, "application/json", bytes.NewReader(body))
 	if err != nil {
 		return err
@@ -259,8 +260,8 @@ func (c *Client) ImportKey(name string, key []byte) error {
 // DeleteKey deletes the given key. Once a key has been deleted
 // all data, that has been encrypted with it, cannot be decrypted
 // anymore.
-func (c *Client) DeleteKey(key string) error {
-	url := fmt.Sprintf("%s/v1/key/delete/%s", c.Endpoint, key)
+func (c *Client) DeleteKey(name string) error {
+	url := endpoint(c.Endpoint, "/v1/key/delete", url.PathEscape(name))
 	req, err := http.NewRequest(http.MethodDelete, url, retryBody(nil))
 	if err != nil {
 		return err
@@ -298,7 +299,7 @@ func (c *Client) DeleteKey(key string) error {
 //
 // If an application does not wish to specify a context
 // value it can set it to nil.
-func (c *Client) GenerateKey(key string, context []byte) (DEK, error) {
+func (c *Client) GenerateKey(name string, context []byte) (DEK, error) {
 	type Request struct {
 		Context []byte `json:"context,omitempty"` // A context is optional
 	}
@@ -310,7 +311,7 @@ func (c *Client) GenerateKey(key string, context []byte) (DEK, error) {
 	}
 
 	client := retry(c.HTTPClient)
-	url := fmt.Sprintf("%s/v1/key/generate/%s", c.Endpoint, key)
+	url := endpoint(c.Endpoint, "/v1/key/generate", url.PathEscape(name))
 	resp, err := client.Post(url, "application/json", bytes.NewReader(body))
 	if err != nil {
 		return DEK{}, err
@@ -340,7 +341,7 @@ func (c *Client) GenerateKey(key string, context []byte) (DEK, error) {
 // encrypted. Therefore, the same context value must be provided
 // for decryption. Clients should remember or be able to
 // re-generate the context value.
-func (c *Client) Encrypt(key string, plaintext, context []byte) ([]byte, error) {
+func (c *Client) Encrypt(name string, plaintext, context []byte) ([]byte, error) {
 	type Request struct {
 		Plaintext []byte `json:"plaintext"`
 		Context   []byte `json:"context,omitempty"` // A context is optional
@@ -354,7 +355,7 @@ func (c *Client) Encrypt(key string, plaintext, context []byte) ([]byte, error)
 	}
 
 	client := retry(c.HTTPClient)
-	url := fmt.Sprintf("%s/v1/key/encrypt/%s", c.Endpoint, key)
+	url := endpoint(c.Endpoint, "/v1/key/encrypt", url.PathEscape(name))
 	resp, err := client.Post(url, "application/json", bytes.NewReader(body))
 	if err != nil {
 		return nil, err
@@ -381,7 +382,7 @@ func (c *Client) Encrypt(key string, plaintext, context []byte) ([]byte, error)
 // The context value must match the context used when
 // the ciphertext was produced. If no context was used
 // the context value should be set to nil.
-func (c *Client) Decrypt(key string, ciphertext, context []byte) ([]byte, error) {
+func (c *Client) Decrypt(name string, ciphertext, context []byte) ([]byte, error) {
 	type Request struct {
 		Ciphertext []byte `json:"ciphertext"`
 		Context    []byte `json:"context,omitempty"` // A context is optional
@@ -395,7 +396,7 @@ func (c *Client) Decrypt(key string, ciphertext, context []byte) ([]byte, error)
 	}
 
 	client := retry(c.HTTPClient)
-	url := fmt.Sprintf("%s/v1/key/decrypt/%s", c.Endpoint, key)
+	url := endpoint(c.Endpoint, "/v1/key/decrypt", url.PathEscape(name))
 	resp, err := client.Post(url, "application/json", bytes.NewReader(body))
 	if err != nil {
 		return nil, err
@@ -433,7 +434,7 @@ func (c *Client) SetPolicy(name string, policy *Policy) error {
 		return err
 	}
 	client := retry(c.HTTPClient)
-	url := fmt.Sprintf("%s/v1/policy/write/%s", c.Endpoint, name)
+	url := endpoint(c.Endpoint, "/v1/policy/write", url.PathEscape(name))
 	resp, err := client.Post(url, "application/json", bytes.NewReader(content))
 	if err != nil {
 		return err
@@ -448,7 +449,7 @@ func (c *Client) SetPolicy(name string, policy *Policy) error {
 // policy exists then GetPolicy returns ErrPolicyNotFound.
 func (c *Client) GetPolicy(name string) (*Policy, error) {
 	client := retry(c.HTTPClient)
-	resp, err := client.Get(fmt.Sprintf("%s/v1/policy/read/%s", c.Endpoint, name))
+	resp, err := client.Get(endpoint(c.Endpoint, "/v1/policy/read", url.PathEscape(name)))
 	if err != nil {
 		return nil, err
 	}
@@ -479,7 +480,7 @@ func (c *Client) ListPolicies(pattern string) ([]string, error) {
 		pattern = "*" // => default to: list "all" policies
 	}
 	client := retry(c.HTTPClient)
-	resp, err := client.Get(fmt.Sprintf("%s/v1/policy/list/%s", c.Endpoint, url.PathEscape(pattern)))
+	resp, err := client.Get(endpoint(c.Endpoint, "/v1/policy/list", c.Endpoint, url.PathEscape(pattern)))
 	if err != nil {
 		return nil, err
 	}
@@ -508,7 +509,7 @@ func (c *Client) ListPolicies(pattern string) ([]string, error) {
 // The later will remove the policy as well as all identities
 // assigned to it.
 func (c *Client) DeletePolicy(name string) error {
-	url := fmt.Sprintf("%s/v1/policy/delete/%s", c.Endpoint, name)
+	url := endpoint(c.Endpoint, "/v1/policy/delete", url.PathEscape(name))
 	req, err := http.NewRequest(http.MethodDelete, url, retryBody(nil))
 	if err != nil {
 		return err
@@ -526,7 +527,7 @@ func (c *Client) DeletePolicy(name string) error {
 
 func (c *Client) AssignIdentity(policy string, id Identity) error {
 	client := retry(c.HTTPClient)
-	url := fmt.Sprintf("%s/v1/identity/assign/%s/%s", c.Endpoint, policy, id.String())
+	url := endpoint(c.Endpoint, "/v1/identity/assign", url.PathEscape(policy), url.PathEscape(id.String()))
 	resp, err := client.Post(url, "application/json", nil)
 	if err != nil {
 		return err
@@ -539,7 +540,7 @@ func (c *Client) AssignIdentity(policy string, id Identity) error {
 
 func (c *Client) ListIdentities(pattern string) (map[Identity]string, error) {
 	client := retry(c.HTTPClient)
-	resp, err := client.Get(fmt.Sprintf("%s/v1/identity/list/%s", c.Endpoint, url.PathEscape(pattern)))
+	resp, err := client.Get(endpoint(c.Endpoint, "/v1/identity/list", url.PathEscape(pattern)))
 	if err != nil {
 		return nil, err
 	}
@@ -556,7 +557,7 @@ func (c *Client) ListIdentities(pattern string) (map[Identity]string, error) {
 }
 
 func (c *Client) ForgetIdentity(id Identity) error {
-	url := fmt.Sprintf("%s/v1/identity/forget/%s", c.Endpoint, id.String())
+	url := endpoint(c.Endpoint, "/v1/identity/forget", url.PathEscape(id.String()))
 	req, err := http.NewRequest(http.MethodDelete, url, retryBody(nil))
 	if err != nil {
 		return err
@@ -572,15 +573,16 @@ func (c *Client) ForgetIdentity(id Identity) error {
 	return nil
 }
 
-// TraceAuditLog subscribes to the KES server audit
-// log and returns a stream of audit events on success.
+// AuditLog returns a stream of audit events produced by the
+// KES server. The stream does not contain any events that
+// happend in the past.
 //
 // It returns ErrNotAllowed if the client does not
 // have sufficient permissions to subscribe to the
 // audit log.
-func (c *Client) TraceAuditLog() (*AuditStream, error) {
+func (c *Client) AuditLog() (*AuditStream, error) {
 	client := retry(c.HTTPClient)
-	resp, err := client.Get(fmt.Sprintf("%s/v1/log/audit/trace", c.Endpoint))
+	resp, err := client.Get(endpoint(c.Endpoint, "/v1/log/audit/trace"))
 	if err != nil {
 		return nil, err
 	}
@@ -590,15 +592,16 @@ func (c *Client) TraceAuditLog() (*AuditStream, error) {
 	return NewAuditStream(resp.Body), nil
 }
 
-// TraceErrorLog subscribes to the KES server error
-// log and returns a stream of error events on success.
+// ErrorLog returns a stream of error events produced by the
+// KES server. The stream does not contain any events that
+// happend in the past.
 //
 // It returns ErrNotAllowed if the client does not
 // have sufficient permissions to subscribe to the
 // error log.
-func (c *Client) TraceErrorLog() (*ErrorStream, error) {
+func (c *Client) ErrorLog() (*ErrorStream, error) {
 	client := retry(c.HTTPClient)
-	resp, err := client.Get(fmt.Sprintf("%s/v1/log/error/trace", c.Endpoint))
+	resp, err := client.Get(endpoint(c.Endpoint, "/v1/log/error/trace"))
 	if err != nil {
 		return nil, err
 	}
@@ -607,3 +610,25 @@ func (c *Client) TraceErrorLog() (*ErrorStream, error) {
 	}
 	return NewErrorStream(resp.Body), nil
 }
+
+// endpoint returns an endpoint URL starting with the
+// given endpoint followed by the path elements.
+//
+// For example:
+//   • endpoint("https://127.0.0.1:7373", "version")                => "https://127.0.0.1:7373/version"
+//   • endpoint("https://127.0.0.1:7373/", "/key/create", "my-key") => "https://127.0.0.1:7373/key/create/my-key"
+//
+// Any leading or trailing whitespaces are removed from
+// the endpoint before it is concatenated with the path
+// elements.
+//
+// The path elements will not be URL-escaped.
+func endpoint(endpoint string, elems ...string) string {
+	endpoint = strings.TrimSpace(endpoint)
+	endpoint = strings.TrimSuffix(endpoint, "/")
+
+	if len(elems) > 0 && !strings.HasPrefix(elems[0], "/") {
+		endpoint += "/"
+	}
+	return endpoint + path.Join(elems...)
+}
diff --git a/client_test.go b/client_test.go
@@ -0,0 +1,46 @@
+// Copyright 2020 - MinIO, Inc. All rights reserved.
+// Use of this source code is governed by the AGPLv3
+// license that can be found in the LICENSE file.
+
+package kes
+
+import "testing"
+
+var endpointTests = []struct {
+	Endpoint string
+	Elements []string
+	URL      string
+}{
+	{Endpoint: "https://127.0.0.1:7373", Elements: nil, URL: "https://127.0.0.1:7373"},
+	{Endpoint: "https://127.0.0.1:7373/", Elements: nil, URL: "https://127.0.0.1:7373"},
+	{Endpoint: " https://127.0.0.1:7373/ ", Elements: nil, URL: "https://127.0.0.1:7373"},
+
+	{
+		Endpoint: "https://play.min.io:7373",
+		Elements: []string{"/version"},
+		URL:      "https://play.min.io:7373/version",
+	},
+	{
+		Endpoint: "https://play.min.io:7373",
+		Elements: []string{"version"},
+		URL:      "https://play.min.io:7373/version",
+	},
+	{
+		Endpoint: "https://127.0.0.1:7373",
+		Elements: []string{"/key/create/my-key"},
+		URL:      "https://127.0.0.1:7373/key/create/my-key",
+	},
+	{
+		Endpoint: "https://127.0.0.1:7373",
+		Elements: []string{"/key", "/create", "my-key"},
+		URL:      "https://127.0.0.1:7373/key/create/my-key",
+	},
+}
+
+func TestEndpoint(t *testing.T) {
+	for i, test := range endpointTests {
+		if url := endpoint(test.Endpoint, test.Elements...); url != test.URL {
+			t.Fatalf("Test %d: endpoint url mismatch: got '%s' - want '%s'", i, url, test.URL)
+		}
+	}
+}
diff --git a/cmd/kes/log.go b/cmd/kes/log.go
@@ -96,7 +96,7 @@ func logTrace(args []string) error {
 
 	switch strings.ToLower(typeFlag) {
 	case "audit":
-		stream, err := client.TraceAuditLog()
+		stream, err := client.AuditLog()
 		if err != nil {
 			return err
 		}
@@ -111,7 +111,7 @@ func logTrace(args []string) error {
 		}
 		return traceAuditLogWithUI(stream)
 	case "error":
-		stream, err := client.TraceErrorLog()
+		stream, err := client.ErrorLog()
 		if err != nil {
 			return err
 		}