From 5c743f594bf76f735babaafe1ac2167c844f0a3d Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 06:56:01 +0200 Subject: [PATCH 01/10] CF CredHub KeyStore --- internal/keystore/credhub/credhub.go | 329 +++++++++++++++ internal/keystore/credhub/credhub_test.go | 412 +++++++++++++++++++ internal/keystore/credhub/http_client.go | 76 ++++ internal/keystore/credhub/value_converter.go | 31 ++ kesconf/config.go | 38 ++ kesconf/credhub_test.go | 40 ++ kesconf/file.go | 16 +- kesconf/testdata/credhub.yml | 19 + 8 files changed, 959 insertions(+), 2 deletions(-) create mode 100644 internal/keystore/credhub/credhub.go create mode 100644 internal/keystore/credhub/credhub_test.go create mode 100644 internal/keystore/credhub/http_client.go create mode 100644 internal/keystore/credhub/value_converter.go create mode 100644 kesconf/credhub_test.go create mode 100644 kesconf/testdata/credhub.yml diff --git a/internal/keystore/credhub/credhub.go b/internal/keystore/credhub/credhub.go new file mode 100644 index 00000000..7c406cb1 --- /dev/null +++ b/internal/keystore/credhub/credhub.go @@ -0,0 +1,329 @@ +// Copyright 2023 - 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 credhub + +import ( + "bytes" + "context" + "crypto/tls" + "crypto/x509" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "github.com/golang/groupcache/singleflight" + "github.com/google/uuid" + "github.com/minio/kes" + "github.com/minio/kes/internal/keystore" + kesdk "github.com/minio/kms-go/kes" + "net/http" + "os" + "strings" + "time" +) + +const ( + contentType = "Content-Type" + applicationJson = "application/json" +) + +type Config struct { + BaseUrl string // The base URL endpoint of the CredHub service. + EnableMutualTls bool // If set to true, enables mutual TLS. + ClientCertFilePath string // Path to the client's certificate file used for mutual TLS authentication. + ClientKeyFilePath string // Path to the client's private key file used for mutual TLS authentication. + ServerInsecureSkipVerify bool // If set to true, server's certificate will not be verified against the provided CA certificate. + ServerCaCertFilePath string // Path to the CA certificate file for verifying the CredHub server's certificate. + Namespace string // A namespace within CredHub where credentials are stored. + ForceBase64ValuesEncoding bool // If set to true, forces encoding of all the values as base64 before storage. +} + +type Certs struct { + ServerCaCert *x509.Certificate + ClientKeyPair tls.Certificate +} + +func (c *Config) Validate() (*Certs, error) { + certs := &Certs{} + if c.BaseUrl == "" { + return certs, errors.New("credhub config: `BaseUrl` can't be empty") + } + if c.Namespace == "" { + return certs, errors.New("credhub config: `Namespace` can't be empty") + } + if !c.ServerInsecureSkipVerify { + if c.ServerCaCertFilePath == "" { + return certs, errors.New("credhub config: `ServerCaCertFilePath` can't be empty when `ServerInsecureSkipVerify` is false") + } + _, sCertDerBytes, err := c.validatePemFile(c.ServerCaCertFilePath, "ServerCaCertFilePath") + if err != nil { + return nil, err + } + certs.ServerCaCert, err = x509.ParseCertificate(sCertDerBytes) + if err != nil { + return nil, errors.New(fmt.Sprintf("credhub config: error parsing the certificate '%s': %v", "ServerCaCertFilePath", err)) + } + } + if c.EnableMutualTls { + if c.ClientCertFilePath == "" || c.ClientKeyFilePath == "" { + return certs, errors.New("credhub config: `ClientCertFilePath` and `ClientKeyFilePath` can't be empty when `EnableMutualTls` is true") + } + cCertPemBytes, cCertDerBytes, err := c.validatePemFile(c.ClientCertFilePath, "ClientCertFilePath") + if err != nil { + return certs, err + } + _, err = x509.ParseCertificate(cCertDerBytes) + if err != nil { + return nil, errors.New(fmt.Sprintf("credhub config: error parsing the certificate '%s': %v", "ClientCertFilePath", err)) + } + cKeyPemBytes, _, err := c.validatePemFile(c.ClientKeyFilePath, "ClientKeyFilePath") + if err != nil { + return certs, err + } + certs.ClientKeyPair, err = tls.X509KeyPair(cCertPemBytes, cKeyPemBytes) + if err != nil { + return certs, err + } + } + return certs, nil +} + +func (c *Config) validatePemFile(path, name string) (pemBytes, derBytes []byte, err error) { + pemBytes, err = os.ReadFile(path) + if err != nil { + return pemBytes, nil, errors.New(fmt.Sprintf("credhub config: failed to load PEM file '%s'='%s': %v", name, path, err)) + } + derBlock, _ := pem.Decode(pemBytes) + if derBlock == nil { + return pemBytes, nil, errors.New(fmt.Sprintf("credhub config: failed to decode the '%s'='%s' from PEM format, no PEM data found", name, path)) + } + return pemBytes, derBlock.Bytes, nil +} + +type Store struct { + LastError error + config *Config + client HttpClient + sfGroup singleflight.Group +} + +func NewStore(_ context.Context, config *Config) (*Store, error) { + client, err := NewHttpMTlsClient(config) + if err != nil { + return nil, err + } else { + return &Store{config: config, client: client}, nil + } +} + +// Status returns the current state of the KeyStore. +// +// CredHub "Get Server Status": +// - https://docs.cloudfoundry.org/api/credhub/version/main/#_get_server_status +// - `credhub curl -X=GET -p /health` +func (s *Store) Status(ctx context.Context) (kes.KeyStoreState, error) { + uri := "/health" + startTime := time.Now() + resp := s.client.DoRequest(ctx, http.MethodGet, uri, nil) + defer resp.CloseResource() + if resp.err != nil { + return kes.KeyStoreState{Latency: 0}, resp.err + } + state := kes.KeyStoreState{ + Latency: time.Since(startTime), + } + + if resp.IsStatusCode2xx() { + var responseData struct { + Status string `json:"status"` + } + if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { + return state, fmt.Errorf("failed to parse response: %v", err) + } else { + if responseData.Status == "UP" { + return state, nil + } else { + return state, fmt.Errorf("CredHub is not UP, status: %s", responseData.Status) + } + } + } else { + return state, fmt.Errorf("the CredHub (%s) is not healthy, status: %s", uri, resp.Status) + } +} + +// Create creates a new entry with the given name if and only +// if no such entry exists. +// Otherwise, Create returns kes.ErrKeyExists. +// +// CredHub: there is no method to do it, implemented workaround with limitations +func (s *Store) Create(ctx context.Context, name string, value []byte) error { + return s.create(ctx, name, value, uuid.New().String()) +} + +func (s *Store) create(ctx context.Context, name string, value []byte, operationID string) error { + _, err := s.sfGroup.Do(s.config.Namespace+"/"+name, func() (interface{}, error) { + _, err := s.Get(ctx, name) + if err == nil { + return nil, fmt.Errorf("key '%s' already exists: %w", name, kesdk.ErrKeyExists) + } else if errors.Is(err, kesdk.ErrKeyNotFound) { + return nil, s.put(ctx, name, value, operationID) + } else { + return nil, err + } + }) + return err +} + +// CredHub "Set a Value Credential": +// - https://docs.cloudfoundry.org/api/credhub/version/main/#_set_a_value_credential +// - `credhub curl -X=PUT -p "/api/v1/data" -d='{"name":"/test-namespace/key-1","type":"value","value":"1"}` +func (s *Store) put(ctx context.Context, name string, value []byte, operationID string) error { + uri := "/api/v1/data" + valueStr := BytesToJsonString(value, s.config.ForceBase64ValuesEncoding) + data := map[string]interface{}{ + "name": s.config.Namespace + "/" + name, + "type": "value", + "value": valueStr, + "metadata": map[string]string{ + "operation_id": operationID, + }, + } + payload, err := json.Marshal(data) + if err != nil { + return err + } + resp := s.client.DoRequest(ctx, http.MethodPut, uri, bytes.NewBuffer(payload)) + defer resp.CloseResource() + if resp.err != nil { + return resp.err + } + + if resp.IsStatusCode2xx() { + var responseData struct { + Value string `json:"value"` + Metadata struct { + OperationId string `json:"operation_id"` + } `json:"metadata"` + } + if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { + return fmt.Errorf("can't decode response of put entry (status: %s)", resp.Status) + } + if responseData.Value != valueStr { + return fmt.Errorf("key '%s' was inserted but overwritten by other process (the returned value is different from the the one sent): %w", name, kesdk.ErrKeyExists) + } + if responseData.Metadata.OperationId != operationID { + return fmt.Errorf("key '%s' was inserted but overwritten by other process (operation ID %s != %s): %w", name, responseData.Metadata.OperationId, operationID, kesdk.ErrKeyExists) + } + return nil + + } else { + return fmt.Errorf("failed to put entry (status: %s)", resp.Status) + } +} + +// Delete removes the entry. It may return either no error or +// kes.ErrKeyNotFound if no such entry exists. +// +// CredHub "Delete a Credential": +// - https://docs.cloudfoundry.org/api/credhub/version/main/#_delete_a_credential +// - `credhub curl -X=DELETE -p "/api/v1/data?name=/test-namespace/key-2"` +func (s *Store) Delete(ctx context.Context, name string) error { + uri := fmt.Sprintf("/api/v1/data?name=%s/%s", s.config.Namespace, name) + resp := s.client.DoRequest(ctx, http.MethodDelete, uri, nil) + defer resp.CloseResource() + if resp.err != nil { + return resp.err + } + + if resp.StatusCode == http.StatusNotFound { + return kesdk.ErrKeyNotFound + } else if !resp.IsStatusCode2xx() { + return fmt.Errorf("failed to delete entry: %s", resp.Status) + } + return nil +} + +// Get returns the value for the given name. It returns +// kes.ErrKeyNotFound if no such entry exits. +// +// CredHub "Get a Credential by Name": +// - https://docs.cloudfoundry.org/api/credhub/version/main/#_get_a_credential_by_name +// - `credhub curl -X=GET -p "/api/v1/data?name=/test-namespace/key-4¤t=true"` +func (s *Store) Get(ctx context.Context, name string) ([]byte, error) { + uri := fmt.Sprintf("/api/v1/data?current=true&name=%s/%s", s.config.Namespace, name) + resp := s.client.DoRequest(ctx, http.MethodGet, uri, nil) + defer resp.CloseResource() + if resp.err != nil { + return nil, resp.err + } + + if resp.StatusCode == http.StatusNotFound { + return nil, kesdk.ErrKeyNotFound + } else if !resp.IsStatusCode2xx() { + return nil, fmt.Errorf("failed to get entry (status: %s)", resp.Status) + } + var responseData struct { + Data []struct { + Value string `json:"value"` + } `json:"data"` + } + if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { + return nil, err + } + + if len(responseData.Data) == 0 { + return nil, kesdk.ErrKeyNotFound + } + if len(responseData.Data) > 1 { + return nil, fmt.Errorf("received multiple entries (%d) for the same key", len(responseData.Data)) + } + return JsonStringToBytes(responseData.Data[0].Value) +} + +// List returns the first n key names, that start with the given +// prefix, and the next prefix from which the listing should +// continue. +// +// It returns all keys with the prefix if n < 0 and less than n +// names if n is greater than the number of keys with the prefix. +// +// An empty prefix matches any key name. At the end of the listing +// or when there are no (more) keys starting with the prefix, the +// returned prefix is empty. +// +// CredHub "Find a Credential by Name-Like": +// - https://docs.cloudfoundry.org/api/credhub/version/main/#_find_a_credential_by_name_like +// - `credhub curl -X=GET -p "/api/v1/data?path=/test-namespace/"` +func (s *Store) List(ctx context.Context, prefix string, n int) ([]string, string, error) { + pathPrefix := s.config.Namespace + "/" + uri := fmt.Sprintf("/api/v1/data?name-like=%s%s", pathPrefix, prefix) + resp := s.client.DoRequest(ctx, http.MethodGet, uri, nil) + defer resp.CloseResource() + if resp.err != nil { + return nil, "", resp.err + } + + if !resp.IsStatusCode2xx() { + return nil, "", fmt.Errorf("failed to list entries (status: %s)", resp.Status) + } + var responseData struct { + Credentials []struct { + Name string `json:"name"` + } `json:"credentials"` + } + if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { + return nil, "", err + } + + var names []string + for _, credential := range responseData.Credentials { + names = append(names, strings.TrimPrefix(credential.Name, pathPrefix)) + } + resNames, resPrefix, err := keystore.List(names, prefix, n) + return resNames, resPrefix, err +} + +// Close terminate or release resources that were opened or acquired. +func (s *Store) Close() error { return nil } diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go new file mode 100644 index 00000000..82abb21a --- /dev/null +++ b/internal/keystore/credhub/credhub_test.go @@ -0,0 +1,412 @@ +package credhub + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "github.com/minio/kes/internal/api" + "github.com/minio/kms-go/kes" + "io" + "net/http" + "reflect" + "testing" +) + +/** CredHub Rest API contract tests. +The following is checked: +- correctness of requests: method, url, body +- correctness of responses: status, body parsing +*/ + +const testNamespace = "/test-namespace" + +// `curl -v --cert ./client.cert --key ./client.key --cacert ./server-ca.cert https://localhost:8844/api/v1/data?path=/` +func TestStore_MTls(t *testing.T) { + t.Run("get status request contract", func(t *testing.T) { + t.Skip("skipping due to this being an integration test that requires specific configuration for a CredHub instance") + client, err := NewHttpMTlsClient(&Config{ + BaseUrl: "https://localhost:8844", + Namespace: testNamespace, + EnableMutualTls: true, + ClientCertFilePath: "../../../client.cert", + ClientKeyFilePath: "../../../client.key", + ServerInsecureSkipVerify: false, + ServerCaCertFilePath: "../../../server-ca.cert", + }) + assertNoError(t, err) + resp := client.DoRequest(context.Background(), "GET", "/api/v1/data?path=/", nil) + assertNoError(t, resp.err) + fmt.Println(resp.Status) + }) + +} + +// `credhub curl -X=GET -p /health` +func TestStore_Status(t *testing.T) { + fakeClient, store := NewFakeStore() + + t.Run("get status request contract", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = `{"status" : "UP"}` + _, err := store.Status(context.Background()) + assertNoError(t, err) + assertRequest(t, fakeClient, "GET", "/health") + }) + + t.Run("returns error for status 200 and not 'UP'", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = `{"status" : "DOWN"}` + _, err := store.Status(context.Background()) + assertError(t, err) + }) + + t.Run("returns error for non-200 status", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 500 + _, err := store.Status(context.Background()) + assertError(t, err) + }) +} + +// `credhub curl -X=PUT -p "/api/v1/data" -d='{"name":"/test-namespace/key-1","type":"value","value":"1"}` +func TestStore_put(t *testing.T) { + fakeClient, store := NewFakeStore() + + t.Run("PUT string value without encoding request contract", func(t *testing.T) { + fakeClient.respStatusCodes["PUT"] = 200 + const key = "key" + const value = "string-value" + const operationID = "test" + fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, value, operationID) + store.config.ForceBase64ValuesEncoding = false + err := store.put(context.Background(), key, []byte(value), operationID) + assertNoError(t, err) + assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, value, operationID)) + }) + + t.Run("PUT string value with forced Base64 encoding request contract", func(t *testing.T) { + fakeClient.respStatusCodes["PUT"] = 200 + const key = "key" + const value = "string-value" + const encodedValue = "Base64:c3RyaW5nLXZhbHVl" + const operationID = "test" + store.config.ForceBase64ValuesEncoding = true + fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID) + err := store.put(context.Background(), key, []byte(value), operationID) + assertNoError(t, err) + assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID)) + }) + t.Run("PUT bytes value with not valid UTF-8 bytes", func(t *testing.T) { + fakeClient.respStatusCodes["PUT"] = 200 + const key = "key" + value := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 80, 114, 122, 255, 121, 107, 108, 255} + const encodedValue = "Base64:AAECAwQFBgcICQpQcnr/eWts/w==" + const operationID = "test" + store.config.ForceBase64ValuesEncoding = false + fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID) + err := store.put(context.Background(), key, value, operationID) + assertNoError(t, err) + assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID)) + }) + t.Run("PUT string value starts with 'Base64:' request contract", func(t *testing.T) { + fakeClient.respStatusCodes["PUT"] = 200 + const key = "key" + const value = "Base64:string-value" + const encodedValue = "Base64:QmFzZTY0OnN0cmluZy12YWx1ZQ==" + const operationID = "test" + store.config.ForceBase64ValuesEncoding = false + fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID) + err := store.put(context.Background(), key, []byte(value), operationID) + assertNoError(t, err) + assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID)) + }) +} + +func TestStore_Create(t *testing.T) { + fakeClient, store := NewFakeStore() + existsRespBody := `{"data":[{"value":"something"}]}` + + t.Run("create element that exists", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = existsRespBody + const key = "key" + const value = "string-value" + err := store.Create(context.Background(), key, []byte(value)) + assertErrorIs(t, err, kes.ErrKeyExists) + assertApiErrorStatus(t, err, http.StatusBadRequest) + }) + t.Run("create element that doesn't exist", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 404 + fakeClient.respStatusCodes["PUT"] = 200 + const key = "key" + const value = "string-value" + const operationID = "test" + fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, value, operationID) + err := store.create(context.Background(), key, []byte(value), operationID) + assertNoError(t, err) + }) + t.Run("create element unknown error", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = existsRespBody + fakeClient.respStatusCodes["PUT"] = 500 + const key = "key" + const value = "string-value" + err := store.Create(context.Background(), key, []byte(value)) + assertError(t, err) + }) +} + +// `credhub curl -X=GET -p "/api/v1/data?name=/test-namespace/key-4¤t=true"` +func TestStore_Get(t *testing.T) { + fakeClient, store := NewFakeStore() + + t.Run("GET string value without encoding request contract", func(t *testing.T) { + const key = "key" + const value = "string-value" + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = fmt.Sprintf(` + { + "data" : [ { + "type" : "value", + "version_created_at" : "2019-02-01T20:37:52Z", + "id" : "2e094eda-719c-43cb-a0f5-04face0a79be", + "name" : "%s/%s", + "metadata" : { + "description" : "example metadata" + }, + "value" : "%s" + } ] + } + `, testNamespace, key, value) + b, err := store.Get(context.Background(), key) + assertNoError(t, err) + assertRequest(t, fakeClient, "GET", fmt.Sprintf("/api/v1/data?current=true&name=%s/%s", testNamespace, key)) + assertEqualComparable(t, value, string(b)) + }) + t.Run("GET bytes value with Base64 encoding request contract", func(t *testing.T) { + const key = "key" + value := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 80, 114, 122, 255, 121, 107, 108, 255} + encodedValue := BytesToJsonString(value, true) + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = fmt.Sprintf(` + { + "data" : [ { + "type" : "value", + "version_created_at" : "2019-02-01T20:37:52Z", + "id" : "2e094eda-719c-43cb-a0f5-04face0a79be", + "name" : "%s/%s", + "metadata" : { + "description" : "example metadata" + }, + "value" : "%s" + } ] + } + `, testNamespace, key, encodedValue) + b, err := store.Get(context.Background(), key) + assertNoError(t, err) + assertRequest(t, fakeClient, "GET", fmt.Sprintf("/api/v1/data?current=true&name=%s/%s", testNamespace, key)) + assertEqualBytes(t, value, b) + }) + + t.Run("GET element that doesn't exist", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 404 + const name = "element-name" + _, err := store.Get(context.Background(), name) + assertErrorIs(t, err, kes.ErrKeyNotFound) + assertApiErrorStatus(t, err, http.StatusNotFound) + }) + +} + +// `credhub curl -X=DELETE -p "/api/v1/data?name=/test-namespace/element-name"` +func TestStore_Delete(t *testing.T) { + fakeClient, store := NewFakeStore() + + t.Run("DELETE element request contract", func(t *testing.T) { + fakeClient.respStatusCodes["DELETE"] = 200 + const name = "element-name" + err := store.Delete(context.Background(), name) + assertNoError(t, err) + assertRequest(t, fakeClient, "DELETE", fmt.Sprintf("/api/v1/data?name=%s/%s", testNamespace, name)) + }) + + t.Run("DELETE element that doesn't exist", func(t *testing.T) { + fakeClient.respStatusCodes["DELETE"] = 404 + const name = "element-name" + err := store.Delete(context.Background(), name) + assertErrorIs(t, err, kes.ErrKeyNotFound) + assertApiErrorStatus(t, err, http.StatusNotFound) + }) +} + +// `credhub curl -X=GET -p "/api/v1/data?name-like=/test-namespace/prefix"` +func TestStore_List(t *testing.T) { + fakeClient, store := NewFakeStore() + + t.Run("list keys request contract", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = `{"credentials":[]}` + _, _, err := store.List(context.Background(), "", 1) + assertNoError(t, err) + assertRequest(t, fakeClient, "GET", fmt.Sprintf("/api/v1/data?name-like=%s", testNamespace+"/")) + }) + + t.Run("returns empty list", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = `{"credentials":[]}` + list, prefix, err := store.List(context.Background(), "prefix", 1) + assertNoError(t, err) + assertEqualComparable(t, 0, len(list)) + assertEqualComparable(t, "", prefix) + }) + + t.Run("returns list of two elements", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = `{"credentials":[ + {"name":"/test-namespace/prefix-key-2"}, + {"name":"/test-namespace/prefix-key-1"}, + {"name":"/test-namespace/other-key"} + ]}` + list, prefix, err := store.List(context.Background(), "prefix", 2) + assertNoError(t, err) + assertEqualComparable(t, 2, len(list)) + assertEqualComparable(t, "prefix-key-1", list[0]) + assertEqualComparable(t, "prefix-key-2", list[1]) + assertEqualComparable(t, "", prefix) + }) + + t.Run("returns limited list with continuation", func(t *testing.T) { + fakeClient.respStatusCodes["GET"] = 200 + fakeClient.respBody = `{"credentials":[ + {"name":"/test-namespace/prefix-key-3"}, + {"name":"/test-namespace/prefix-key-1"}, + {"name":"/test-namespace/other-key"}, + {"name":"/test-namespace/prefix-key-2"} + ]}` + list, prefix, err := store.List(context.Background(), "prefix", 2) + assertNoError(t, err) + assertEqualComparable(t, 2, len(list)) + assertEqualComparable(t, "prefix-key-1", list[0]) + assertEqualComparable(t, "prefix-key-2", list[1]) + assertEqualComparable(t, "prefix-key-3", prefix) + }) +} + +//=== tools: + +func NewFakeStore() (*FakeHttpClient, *Store) { + fakeClient := &FakeHttpClient{respStatusCodes: map[string]int{}} + store := &Store{ + config: &Config{Namespace: testNamespace}, + client: fakeClient, + } + return fakeClient, store +} + +type FakeHttpClient struct { + reqMethod string + reqUri string + reqBody string + respStatusCodes map[string]int + respStatus string + respBody string + error error +} + +type FakeReadCloser struct { + io.Reader +} + +func (m *FakeReadCloser) Close() error { + return nil +} + +func (c *FakeHttpClient) DoRequest(_ context.Context, method, url string, body io.Reader) HttpResponse { + c.reqMethod = method + c.reqUri = url + c.reqBody = "" + if body != nil { + bodyBytes, err := io.ReadAll(body) + if err == nil { + c.reqBody = string(bodyBytes) + } + } + mockBody := &FakeReadCloser{ + Reader: bytes.NewBufferString(c.respBody), + } + return HttpResponse{StatusCode: c.respStatusCodes[method], Status: c.respStatus, Body: mockBody, err: c.error} +} + +func assertError(t *testing.T, err error) { + if err == nil { + t.Fatal("expected an error, got nil") + } +} + +func assertErrorIs(t *testing.T, err, target error) { + if err == nil || target == nil { + t.Fatal("error can't be null") + } + if !errors.Is(err, target) { + t.Fatal(fmt.Sprintf("error '%v' isn't '%v'", err, target)) + } +} + +func assertApiErrorStatus(t *testing.T, err error, status int) { + if err == nil { + t.Fatal("error can't be null") + } + apiErr, isIt := api.IsError(err) + if !isIt { + t.Fatal(fmt.Sprintf("error '%+v' isn't api error '%+v'", err, apiErr)) + } + if apiErr.Status() != status { + t.Fatal(fmt.Sprintf("expect error status '%d', got '%d'", status, apiErr.Status())) + } +} + +func assertNoError(t *testing.T, err error) { + if err != nil { + t.Fatalf("expected no error, got %v", err) + } +} + +func assertEqualComparable(t *testing.T, expected, got any) { + if expected != got { + t.Fatalf("expected '%v' got '%v'", expected, got) + } +} +func assertEqualBytes(t *testing.T, expected, got []byte) { + if !bytes.Equal(expected, got) { + t.Fatalf("expected '%v' got '%v'", expected, got) + } +} + +func assertRequest(t *testing.T, fc *FakeHttpClient, method, uri string) { + if fc.reqMethod != method { + t.Fatalf("expected requested method '%s' but got '%s'", method, fc.reqMethod) + } + if fc.reqUri != uri { + t.Fatalf("expected requested uri '%s' but got '%s'", uri, fc.reqUri) + } +} +func assertRequestWithJsonBody(t *testing.T, fc *FakeHttpClient, method, uri string, jsonBody string) { + assertRequest(t, fc, method, uri) + + var gotJson, expectedJson interface{} + err1 := json.Unmarshal([]byte(fc.reqBody), &gotJson) + err2 := json.Unmarshal([]byte(jsonBody), &expectedJson) + + if err1 != nil || err2 != nil { + t.Fatalf("jsons deserialization errors: %v, %v", err1, err2) + } + + if !reflect.DeepEqual(gotJson, expectedJson) { + t.Fatalf("expected requested body '%s' but got '%s'", jsonBody, fc.reqBody) + } +} diff --git a/internal/keystore/credhub/http_client.go b/internal/keystore/credhub/http_client.go new file mode 100644 index 00000000..803ab76c --- /dev/null +++ b/internal/keystore/credhub/http_client.go @@ -0,0 +1,76 @@ +package credhub + +import ( + "context" + "crypto/tls" + "crypto/x509" + "io" + "net/http" +) + +type HttpResponse struct { + StatusCode int + Status string + Body io.ReadCloser + err error +} + +func NewHttpResponseError(err error) HttpResponse { + return HttpResponse{StatusCode: -1, Status: "", Body: nil, err: err} +} +func (c *HttpResponse) IsStatusCode2xx() bool { + return c.StatusCode >= http.StatusOK && c.StatusCode < http.StatusMultipleChoices +} + +func (c *HttpResponse) CloseResource() { + if c.Body != nil { + _ = c.Body.Close() + } +} + +type HttpClient interface { + DoRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse +} + +type HttpMTlsClient struct { + baseUrl string + httpClient *http.Client +} + +func NewHttpMTlsClient(config *Config) (HttpClient, error) { + certs, err := config.Validate() + if err != nil { + return nil, err + } + tlsConfig := &tls.Config{ + InsecureSkipVerify: config.ServerInsecureSkipVerify, + } + if !config.ServerInsecureSkipVerify { + // Setup mutual TLS - server + caCertPool := x509.NewCertPool() + caCertPool.AddCert(certs.ServerCaCert) + tlsConfig.RootCAs = caCertPool + } + if config.EnableMutualTls { + // Setup mutual TLS - client + tlsConfig.Certificates = []tls.Certificate{certs.ClientKeyPair} + } + transport := &http.Transport{TLSClientConfig: tlsConfig} + httpClient := &http.Client{Transport: transport} + return &HttpMTlsClient{baseUrl: config.BaseUrl, httpClient: httpClient}, nil +} + +func (s *HttpMTlsClient) DoRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse { + url := s.baseUrl + uri + req, err := http.NewRequestWithContext(ctx, method, url, body) + if err != nil { + return NewHttpResponseError(err) + } + req.Header.Set(contentType, applicationJson) + resp, err := s.httpClient.Do(req) + if err != nil { + return NewHttpResponseError(err) + } else { + return HttpResponse{StatusCode: resp.StatusCode, Status: resp.Status, Body: resp.Body, err: nil} + } +} diff --git a/internal/keystore/credhub/value_converter.go b/internal/keystore/credhub/value_converter.go new file mode 100644 index 00000000..2145e1b9 --- /dev/null +++ b/internal/keystore/credhub/value_converter.go @@ -0,0 +1,31 @@ +// Copyright 2023 - 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 credhub + +import ( + "encoding/base64" + "strings" + "unicode/utf8" +) + +const Base64Prefix = "Base64:" + +func BytesToJsonString(bytes []byte, forceBase64 bool) (value string) { + if utf8.Valid(bytes) && !forceBase64 { + strBytes := string(bytes) + if !strings.HasPrefix(strBytes, Base64Prefix) { + return string(bytes) + } + } + return Base64Prefix + base64.StdEncoding.EncodeToString(bytes) +} + +func JsonStringToBytes(value string) (bytes []byte, err error) { + if strings.HasPrefix(value, Base64Prefix) { + return base64.StdEncoding.DecodeString(strings.TrimPrefix(value, Base64Prefix)) + } else { + return []byte(value), nil + } +} diff --git a/kesconf/config.go b/kesconf/config.go index d9940f43..8b9d45fa 100644 --- a/kesconf/config.go +++ b/kesconf/config.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/minio/kes/internal/keystore/credhub" "github.com/minio/kms-go/kes" "gopkg.in/yaml.v3" ) @@ -194,6 +195,7 @@ type ymlFile struct { } `yaml:"managed_identity"` } `yaml:"keyvault"` } `yaml:"azure"` + Entrust *struct { KeyControl *struct { Endpoint env[string] `yaml:"endpoint"` @@ -208,6 +210,17 @@ type ymlFile struct { } `yaml:"tls"` } `yaml:"keycontrol"` } `yaml:"entrust"` + + CredHub *struct { + BaseUrl env[string] `yaml:"base_url"` + EnableMutualTls env[bool] `yaml:"enable_mutual_tls"` + ClientCertFilePath env[string] `yaml:"client_cert_file_path"` + ClientKeyFilePath env[string] `yaml:"client_key_file_path"` + ServerCaCertFilePath env[string] `yaml:"server_ca_cert_file_path"` + ServerInsecureSkipVerify env[bool] `yaml:"server_insecure_skip_verify"` + Namespace env[string] `yaml:"namespace"` + ForceBase64ValuesEncoding env[bool] `yaml:"force_base64_values_encoding"` + } `yaml:"credhub"` } `yaml:"keystore"` } @@ -627,6 +640,8 @@ func ymlToKeyStore(y *ymlFile) (KeyStore, error) { } keystore = s } + + // Entrust if y.KeyStore.Entrust != nil && y.KeyStore.Entrust.KeyControl != nil { if keystore != nil { return nil, errors.New("kesconf: invalid keystore config: more than once keystore specified") @@ -656,6 +671,29 @@ func ymlToKeyStore(y *ymlFile) (KeyStore, error) { } } + // CF CredHub + if y.KeyStore.CredHub != nil { + if keystore != nil { + return nil, errors.New("kesconf: invalid CredHub config: more than once keystore specified") + } + config := credhub.Config{ + BaseUrl: y.KeyStore.CredHub.BaseUrl.Value, + EnableMutualTls: y.KeyStore.CredHub.EnableMutualTls.Value, + ClientCertFilePath: y.KeyStore.CredHub.ClientCertFilePath.Value, + ClientKeyFilePath: y.KeyStore.CredHub.ClientKeyFilePath.Value, + ServerInsecureSkipVerify: y.KeyStore.CredHub.ServerInsecureSkipVerify.Value, + ServerCaCertFilePath: y.KeyStore.CredHub.ServerCaCertFilePath.Value, + Namespace: y.KeyStore.CredHub.Namespace.Value, + ForceBase64ValuesEncoding: y.KeyStore.CredHub.ForceBase64ValuesEncoding.Value, + } + _, err := config.Validate() + + if err != nil { + return nil, err + } + keystore = &CredHubKeyStore{Config: &config} + } + if keystore == nil { return nil, errors.New("kesconf: no keystore specified") } diff --git a/kesconf/credhub_test.go b/kesconf/credhub_test.go new file mode 100644 index 00000000..94ae3d25 --- /dev/null +++ b/kesconf/credhub_test.go @@ -0,0 +1,40 @@ +// Copyright 2023 - 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 kesconf_test + +import ( + "flag" + "testing" + + "github.com/minio/kes/kesconf" +) + +var credhubConfigFile = flag.String("credhub.config", "", "Path to a KES config file with CredHub config") + +func TestCredHub(t *testing.T) { + if *credhubConfigFile == "" { + t.Skip("CredHub tests disabled. Use -credhub.config= to enable them") + } + + config, err := kesconf.ReadFile(*credhubConfigFile) + if err != nil { + t.Fatal(err) + } + if _, ok := config.KeyStore.(*kesconf.CredHubKeyStore); !ok { + t.Fatalf("Invalid Keystore: want %T - got %T", config.KeyStore, &kesconf.CredHubKeyStore{}) + } + + ctx, cancel := testingContext(t) + defer cancel() + + store, err := config.KeyStore.Connect(ctx) + if err != nil { + t.Fatal(err) + } + + t.Run("Create", func(t *testing.T) { testCreate(ctx, store, t, RandString(ranStringLength)) }) + t.Run("Get", func(t *testing.T) { testGet(ctx, store, t, RandString(ranStringLength)) }) + t.Run("Status", func(t *testing.T) { testStatus(ctx, store, t) }) +} diff --git a/kesconf/file.go b/kesconf/file.go index c60218d6..7663dcb1 100644 --- a/kesconf/file.go +++ b/kesconf/file.go @@ -22,6 +22,7 @@ import ( "github.com/minio/kes/internal/https" "github.com/minio/kes/internal/keystore/aws" "github.com/minio/kes/internal/keystore/azure" + "github.com/minio/kes/internal/keystore/credhub" "github.com/minio/kes/internal/keystore/entrust" "github.com/minio/kes/internal/keystore/fortanix" "github.com/minio/kes/internal/keystore/fs" @@ -29,7 +30,7 @@ import ( "github.com/minio/kes/internal/keystore/gemalto" "github.com/minio/kes/internal/keystore/vault" kesdk "github.com/minio/kms-go/kes" - yaml "gopkg.in/yaml.v3" + "gopkg.in/yaml.v3" ) // ReadFile opens the given file and reads the KES configuration @@ -39,7 +40,8 @@ func ReadFile(filename string) (*File, error) { if err != nil { return nil, err } - defer f.Close() // make sure to close file in case of panic + // make sure to close file in case of panic + defer func(f *os.File) { _ = f.Close() }(f) file, err := ReadFrom(f) if cErr := f.Close(); err == nil { @@ -828,3 +830,13 @@ func (s *EntrustKeyControlKeyStore) Connect(ctx context.Context) (kes.KeyStore, }, }) } + +// CredHubKeyStore is a structure containing the configuration for CredHub. +type CredHubKeyStore struct { + Config *credhub.Config +} + +// Connect returns a kv.Store that stores key-value pairs on CredHub. +func (s *CredHubKeyStore) Connect(ctx context.Context) (kes.KeyStore, error) { + return credhub.NewStore(ctx, s.Config) +} diff --git a/kesconf/testdata/credhub.yml b/kesconf/testdata/credhub.yml new file mode 100644 index 00000000..3b15e752 --- /dev/null +++ b/kesconf/testdata/credhub.yml @@ -0,0 +1,19 @@ +version: v1 + +admin: + identity: disabled + +tls: + key: ./server.key + cert: ./server.cert + +keystore: + credhub: + base_url: https://localhost:8844 + enable_mutual_tls: true + client_cert_file_path: ./client.cert + client_key_file_path: ./client.key + server_insecure_skip_verify: false + server_ca_cert_file_path: ./server-ca.cert + namespace: /test-namespace + force_base64_values_encoding: false \ No newline at end of file From 863814ab34054f4912c0b9dfab85979ec83662f2 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 09:09:55 +0200 Subject: [PATCH 02/10] Lint related changes --- internal/keystore/credhub/credhub.go | 65 ++++++++++---------- internal/keystore/credhub/credhub_test.go | 10 +-- internal/keystore/credhub/http_client.go | 24 ++++---- internal/keystore/credhub/value_converter.go | 7 +-- 4 files changed, 53 insertions(+), 53 deletions(-) diff --git a/internal/keystore/credhub/credhub.go b/internal/keystore/credhub/credhub.go index 7c406cb1..c8ca23ee 100644 --- a/internal/keystore/credhub/credhub.go +++ b/internal/keystore/credhub/credhub.go @@ -126,8 +126,8 @@ func NewStore(_ context.Context, config *Config) (*Store, error) { func (s *Store) Status(ctx context.Context) (kes.KeyStoreState, error) { uri := "/health" startTime := time.Now() - resp := s.client.DoRequest(ctx, http.MethodGet, uri, nil) - defer resp.CloseResource() + resp := s.client.doRequest(ctx, http.MethodGet, uri, nil) + defer resp.closeResource() if resp.err != nil { return kes.KeyStoreState{Latency: 0}, resp.err } @@ -135,11 +135,11 @@ func (s *Store) Status(ctx context.Context) (kes.KeyStoreState, error) { Latency: time.Since(startTime), } - if resp.IsStatusCode2xx() { + if resp.isStatusCode2xx() { var responseData struct { Status string `json:"status"` } - if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { + if err := json.NewDecoder(resp.body).Decode(&responseData); err != nil { return state, fmt.Errorf("failed to parse response: %v", err) } else { if responseData.Status == "UP" { @@ -149,7 +149,7 @@ func (s *Store) Status(ctx context.Context) (kes.KeyStoreState, error) { } } } else { - return state, fmt.Errorf("the CredHub (%s) is not healthy, status: %s", uri, resp.Status) + return state, fmt.Errorf("the CredHub (%s) is not healthy, status: %s", uri, resp.status) } } @@ -165,11 +165,12 @@ func (s *Store) Create(ctx context.Context, name string, value []byte) error { func (s *Store) create(ctx context.Context, name string, value []byte, operationID string) error { _, err := s.sfGroup.Do(s.config.Namespace+"/"+name, func() (interface{}, error) { _, err := s.Get(ctx, name) - if err == nil { + switch { + case err == nil: return nil, fmt.Errorf("key '%s' already exists: %w", name, kesdk.ErrKeyExists) - } else if errors.Is(err, kesdk.ErrKeyNotFound) { + case errors.Is(err, kesdk.ErrKeyNotFound): return nil, s.put(ctx, name, value, operationID) - } else { + default: return nil, err } }) @@ -181,7 +182,7 @@ func (s *Store) create(ctx context.Context, name string, value []byte, operation // - `credhub curl -X=PUT -p "/api/v1/data" -d='{"name":"/test-namespace/key-1","type":"value","value":"1"}` func (s *Store) put(ctx context.Context, name string, value []byte, operationID string) error { uri := "/api/v1/data" - valueStr := BytesToJsonString(value, s.config.ForceBase64ValuesEncoding) + valueStr := bytesToJsonString(value, s.config.ForceBase64ValuesEncoding) data := map[string]interface{}{ "name": s.config.Namespace + "/" + name, "type": "value", @@ -194,21 +195,21 @@ func (s *Store) put(ctx context.Context, name string, value []byte, operationID if err != nil { return err } - resp := s.client.DoRequest(ctx, http.MethodPut, uri, bytes.NewBuffer(payload)) - defer resp.CloseResource() + resp := s.client.doRequest(ctx, http.MethodPut, uri, bytes.NewBuffer(payload)) + defer resp.closeResource() if resp.err != nil { return resp.err } - if resp.IsStatusCode2xx() { + if resp.isStatusCode2xx() { var responseData struct { Value string `json:"value"` Metadata struct { OperationId string `json:"operation_id"` } `json:"metadata"` } - if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { - return fmt.Errorf("can't decode response of put entry (status: %s)", resp.Status) + if err := json.NewDecoder(resp.body).Decode(&responseData); err != nil { + return fmt.Errorf("can't decode response of put entry (status: %s)", resp.status) } if responseData.Value != valueStr { return fmt.Errorf("key '%s' was inserted but overwritten by other process (the returned value is different from the the one sent): %w", name, kesdk.ErrKeyExists) @@ -219,7 +220,7 @@ func (s *Store) put(ctx context.Context, name string, value []byte, operationID return nil } else { - return fmt.Errorf("failed to put entry (status: %s)", resp.Status) + return fmt.Errorf("failed to put entry (status: %s)", resp.status) } } @@ -231,16 +232,16 @@ func (s *Store) put(ctx context.Context, name string, value []byte, operationID // - `credhub curl -X=DELETE -p "/api/v1/data?name=/test-namespace/key-2"` func (s *Store) Delete(ctx context.Context, name string) error { uri := fmt.Sprintf("/api/v1/data?name=%s/%s", s.config.Namespace, name) - resp := s.client.DoRequest(ctx, http.MethodDelete, uri, nil) - defer resp.CloseResource() + resp := s.client.doRequest(ctx, http.MethodDelete, uri, nil) + defer resp.closeResource() if resp.err != nil { return resp.err } - if resp.StatusCode == http.StatusNotFound { + if resp.statusCode == http.StatusNotFound { return kesdk.ErrKeyNotFound - } else if !resp.IsStatusCode2xx() { - return fmt.Errorf("failed to delete entry: %s", resp.Status) + } else if !resp.isStatusCode2xx() { + return fmt.Errorf("failed to delete entry: %s", resp.status) } return nil } @@ -253,23 +254,23 @@ func (s *Store) Delete(ctx context.Context, name string) error { // - `credhub curl -X=GET -p "/api/v1/data?name=/test-namespace/key-4¤t=true"` func (s *Store) Get(ctx context.Context, name string) ([]byte, error) { uri := fmt.Sprintf("/api/v1/data?current=true&name=%s/%s", s.config.Namespace, name) - resp := s.client.DoRequest(ctx, http.MethodGet, uri, nil) - defer resp.CloseResource() + resp := s.client.doRequest(ctx, http.MethodGet, uri, nil) + defer resp.closeResource() if resp.err != nil { return nil, resp.err } - if resp.StatusCode == http.StatusNotFound { + if resp.statusCode == http.StatusNotFound { return nil, kesdk.ErrKeyNotFound - } else if !resp.IsStatusCode2xx() { - return nil, fmt.Errorf("failed to get entry (status: %s)", resp.Status) + } else if !resp.isStatusCode2xx() { + return nil, fmt.Errorf("failed to get entry (status: %s)", resp.status) } var responseData struct { Data []struct { Value string `json:"value"` } `json:"data"` } - if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { + if err := json.NewDecoder(resp.body).Decode(&responseData); err != nil { return nil, err } @@ -279,7 +280,7 @@ func (s *Store) Get(ctx context.Context, name string) ([]byte, error) { if len(responseData.Data) > 1 { return nil, fmt.Errorf("received multiple entries (%d) for the same key", len(responseData.Data)) } - return JsonStringToBytes(responseData.Data[0].Value) + return jsonStringToBytes(responseData.Data[0].Value) } // List returns the first n key names, that start with the given @@ -299,21 +300,21 @@ func (s *Store) Get(ctx context.Context, name string) ([]byte, error) { func (s *Store) List(ctx context.Context, prefix string, n int) ([]string, string, error) { pathPrefix := s.config.Namespace + "/" uri := fmt.Sprintf("/api/v1/data?name-like=%s%s", pathPrefix, prefix) - resp := s.client.DoRequest(ctx, http.MethodGet, uri, nil) - defer resp.CloseResource() + resp := s.client.doRequest(ctx, http.MethodGet, uri, nil) + defer resp.closeResource() if resp.err != nil { return nil, "", resp.err } - if !resp.IsStatusCode2xx() { - return nil, "", fmt.Errorf("failed to list entries (status: %s)", resp.Status) + if !resp.isStatusCode2xx() { + return nil, "", fmt.Errorf("failed to list entries (status: %s)", resp.status) } var responseData struct { Credentials []struct { Name string `json:"name"` } `json:"credentials"` } - if err := json.NewDecoder(resp.Body).Decode(&responseData); err != nil { + if err := json.NewDecoder(resp.body).Decode(&responseData); err != nil { return nil, "", err } diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go index 82abb21a..ea9d7062 100644 --- a/internal/keystore/credhub/credhub_test.go +++ b/internal/keystore/credhub/credhub_test.go @@ -36,9 +36,9 @@ func TestStore_MTls(t *testing.T) { ServerCaCertFilePath: "../../../server-ca.cert", }) assertNoError(t, err) - resp := client.DoRequest(context.Background(), "GET", "/api/v1/data?path=/", nil) + resp := client.doRequest(context.Background(), "GET", "/api/v1/data?path=/", nil) assertNoError(t, resp.err) - fmt.Println(resp.Status) + fmt.Println(resp.status) }) } @@ -191,7 +191,7 @@ func TestStore_Get(t *testing.T) { t.Run("GET bytes value with Base64 encoding request contract", func(t *testing.T) { const key = "key" value := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 80, 114, 122, 255, 121, 107, 108, 255} - encodedValue := BytesToJsonString(value, true) + encodedValue := bytesToJsonString(value, true) fakeClient.respStatusCodes["GET"] = 200 fakeClient.respBody = fmt.Sprintf(` { @@ -326,7 +326,7 @@ func (m *FakeReadCloser) Close() error { return nil } -func (c *FakeHttpClient) DoRequest(_ context.Context, method, url string, body io.Reader) HttpResponse { +func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body io.Reader) HttpResponse { c.reqMethod = method c.reqUri = url c.reqBody = "" @@ -339,7 +339,7 @@ func (c *FakeHttpClient) DoRequest(_ context.Context, method, url string, body i mockBody := &FakeReadCloser{ Reader: bytes.NewBufferString(c.respBody), } - return HttpResponse{StatusCode: c.respStatusCodes[method], Status: c.respStatus, Body: mockBody, err: c.error} + return HttpResponse{statusCode: c.respStatusCodes[method], status: c.respStatus, body: mockBody, err: c.error} } func assertError(t *testing.T, err error) { diff --git a/internal/keystore/credhub/http_client.go b/internal/keystore/credhub/http_client.go index 803ab76c..00ffb860 100644 --- a/internal/keystore/credhub/http_client.go +++ b/internal/keystore/credhub/http_client.go @@ -9,27 +9,27 @@ import ( ) type HttpResponse struct { - StatusCode int - Status string - Body io.ReadCloser + statusCode int + status string + body io.ReadCloser err error } func NewHttpResponseError(err error) HttpResponse { - return HttpResponse{StatusCode: -1, Status: "", Body: nil, err: err} + return HttpResponse{statusCode: -1, status: "", body: nil, err: err} } -func (c *HttpResponse) IsStatusCode2xx() bool { - return c.StatusCode >= http.StatusOK && c.StatusCode < http.StatusMultipleChoices +func (c *HttpResponse) isStatusCode2xx() bool { + return c.statusCode >= http.StatusOK && c.statusCode < http.StatusMultipleChoices } -func (c *HttpResponse) CloseResource() { - if c.Body != nil { - _ = c.Body.Close() +func (c *HttpResponse) closeResource() { + if c.body != nil { + _ = c.body.Close() } } type HttpClient interface { - DoRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse + doRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse } type HttpMTlsClient struct { @@ -60,7 +60,7 @@ func NewHttpMTlsClient(config *Config) (HttpClient, error) { return &HttpMTlsClient{baseUrl: config.BaseUrl, httpClient: httpClient}, nil } -func (s *HttpMTlsClient) DoRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse { +func (s *HttpMTlsClient) doRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse { url := s.baseUrl + uri req, err := http.NewRequestWithContext(ctx, method, url, body) if err != nil { @@ -71,6 +71,6 @@ func (s *HttpMTlsClient) DoRequest(ctx context.Context, method, uri string, body if err != nil { return NewHttpResponseError(err) } else { - return HttpResponse{StatusCode: resp.StatusCode, Status: resp.Status, Body: resp.Body, err: nil} + return HttpResponse{statusCode: resp.StatusCode, status: resp.Status, body: resp.Body, err: nil} } } diff --git a/internal/keystore/credhub/value_converter.go b/internal/keystore/credhub/value_converter.go index 2145e1b9..28029478 100644 --- a/internal/keystore/credhub/value_converter.go +++ b/internal/keystore/credhub/value_converter.go @@ -12,7 +12,7 @@ import ( const Base64Prefix = "Base64:" -func BytesToJsonString(bytes []byte, forceBase64 bool) (value string) { +func bytesToJsonString(bytes []byte, forceBase64 bool) (value string) { if utf8.Valid(bytes) && !forceBase64 { strBytes := string(bytes) if !strings.HasPrefix(strBytes, Base64Prefix) { @@ -22,10 +22,9 @@ func BytesToJsonString(bytes []byte, forceBase64 bool) (value string) { return Base64Prefix + base64.StdEncoding.EncodeToString(bytes) } -func JsonStringToBytes(value string) (bytes []byte, err error) { +func jsonStringToBytes(value string) (bytes []byte, err error) { if strings.HasPrefix(value, Base64Prefix) { return base64.StdEncoding.DecodeString(strings.TrimPrefix(value, Base64Prefix)) - } else { - return []byte(value), nil } + return []byte(value), nil } From b2886adafb917339ba4ae7c6fa6bd849f1a79748 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 09:18:45 +0200 Subject: [PATCH 03/10] Lint related changes --- internal/keystore/credhub/credhub.go | 2 +- internal/keystore/credhub/credhub_test.go | 4 ++-- internal/keystore/credhub/http_client.go | 29 +++++++++++------------ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/internal/keystore/credhub/credhub.go b/internal/keystore/credhub/credhub.go index c8ca23ee..a263bfab 100644 --- a/internal/keystore/credhub/credhub.go +++ b/internal/keystore/credhub/credhub.go @@ -105,7 +105,7 @@ func (c *Config) validatePemFile(path, name string) (pemBytes, derBytes []byte, type Store struct { LastError error config *Config - client HttpClient + client HTTPClient sfGroup singleflight.Group } diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go index ea9d7062..3bf571a1 100644 --- a/internal/keystore/credhub/credhub_test.go +++ b/internal/keystore/credhub/credhub_test.go @@ -326,7 +326,7 @@ func (m *FakeReadCloser) Close() error { return nil } -func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body io.Reader) HttpResponse { +func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body io.Reader) HTTPResponse { c.reqMethod = method c.reqUri = url c.reqBody = "" @@ -339,7 +339,7 @@ func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body i mockBody := &FakeReadCloser{ Reader: bytes.NewBufferString(c.respBody), } - return HttpResponse{statusCode: c.respStatusCodes[method], status: c.respStatus, body: mockBody, err: c.error} + return HTTPResponse{statusCode: c.respStatusCodes[method], status: c.respStatus, body: mockBody, err: c.error} } func assertError(t *testing.T, err error) { diff --git a/internal/keystore/credhub/http_client.go b/internal/keystore/credhub/http_client.go index 00ffb860..c7782b50 100644 --- a/internal/keystore/credhub/http_client.go +++ b/internal/keystore/credhub/http_client.go @@ -8,36 +8,36 @@ import ( "net/http" ) -type HttpResponse struct { +type HTTPResponse struct { statusCode int status string body io.ReadCloser err error } -func NewHttpResponseError(err error) HttpResponse { - return HttpResponse{statusCode: -1, status: "", body: nil, err: err} +func NewHTTPResponseError(err error) HTTPResponse { + return HTTPResponse{statusCode: -1, status: "", body: nil, err: err} } -func (c *HttpResponse) isStatusCode2xx() bool { +func (c *HTTPResponse) isStatusCode2xx() bool { return c.statusCode >= http.StatusOK && c.statusCode < http.StatusMultipleChoices } -func (c *HttpResponse) closeResource() { +func (c *HTTPResponse) closeResource() { if c.body != nil { _ = c.body.Close() } } -type HttpClient interface { - doRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse +type HTTPClient interface { + doRequest(ctx context.Context, method, uri string, body io.Reader) HTTPResponse } -type HttpMTlsClient struct { +type HTTPMTlsClient struct { baseUrl string httpClient *http.Client } -func NewHttpMTlsClient(config *Config) (HttpClient, error) { +func NewHttpMTlsClient(config *Config) (HTTPClient, error) { certs, err := config.Validate() if err != nil { return nil, err @@ -57,20 +57,19 @@ func NewHttpMTlsClient(config *Config) (HttpClient, error) { } transport := &http.Transport{TLSClientConfig: tlsConfig} httpClient := &http.Client{Transport: transport} - return &HttpMTlsClient{baseUrl: config.BaseUrl, httpClient: httpClient}, nil + return &HTTPMTlsClient{baseUrl: config.BaseUrl, httpClient: httpClient}, nil } -func (s *HttpMTlsClient) doRequest(ctx context.Context, method, uri string, body io.Reader) HttpResponse { +func (s *HTTPMTlsClient) doRequest(ctx context.Context, method, uri string, body io.Reader) HTTPResponse { url := s.baseUrl + uri req, err := http.NewRequestWithContext(ctx, method, url, body) if err != nil { - return NewHttpResponseError(err) + return NewHTTPResponseError(err) } req.Header.Set(contentType, applicationJson) resp, err := s.httpClient.Do(req) if err != nil { - return NewHttpResponseError(err) - } else { - return HttpResponse{statusCode: resp.StatusCode, status: resp.Status, body: resp.Body, err: nil} + return NewHTTPResponseError(err) } + return HTTPResponse{statusCode: resp.StatusCode, status: resp.Status, body: resp.Body, err: nil} } From db0956d90dc0d4752ca47308c6a3bf2f26ac3986 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 09:22:41 +0200 Subject: [PATCH 04/10] Lint related changes --- internal/keystore/credhub/http_client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/keystore/credhub/http_client.go b/internal/keystore/credhub/http_client.go index c7782b50..62b2013a 100644 --- a/internal/keystore/credhub/http_client.go +++ b/internal/keystore/credhub/http_client.go @@ -33,7 +33,7 @@ type HTTPClient interface { } type HTTPMTlsClient struct { - baseUrl string + baseURL string httpClient *http.Client } @@ -57,11 +57,11 @@ func NewHttpMTlsClient(config *Config) (HTTPClient, error) { } transport := &http.Transport{TLSClientConfig: tlsConfig} httpClient := &http.Client{Transport: transport} - return &HTTPMTlsClient{baseUrl: config.BaseUrl, httpClient: httpClient}, nil + return &HTTPMTlsClient{baseURL: config.BaseUrl, httpClient: httpClient}, nil } func (s *HTTPMTlsClient) doRequest(ctx context.Context, method, uri string, body io.Reader) HTTPResponse { - url := s.baseUrl + uri + url := s.baseURL + uri req, err := http.NewRequestWithContext(ctx, method, url, body) if err != nil { return NewHTTPResponseError(err) From aba8964c5ecababb199c4ba65834f49d5ff42405 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 09:35:42 +0200 Subject: [PATCH 05/10] Lint related changes --- internal/keystore/credhub/credhub.go | 28 +++++++++------------ internal/keystore/credhub/credhub_test.go | 12 ++++----- internal/keystore/credhub/http_client.go | 30 +++++++++++------------ kesconf/config.go | 8 +++--- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/internal/keystore/credhub/credhub.go b/internal/keystore/credhub/credhub.go index a263bfab..e109ecb4 100644 --- a/internal/keystore/credhub/credhub.go +++ b/internal/keystore/credhub/credhub.go @@ -30,8 +30,8 @@ const ( ) type Config struct { - BaseUrl string // The base URL endpoint of the CredHub service. - EnableMutualTls bool // If set to true, enables mutual TLS. + BaseURL string // The base URL endpoint of the CredHub service. + EnableMutualTLS bool // If set to true, enables mutual TLS. ClientCertFilePath string // Path to the client's certificate file used for mutual TLS authentication. ClientKeyFilePath string // Path to the client's private key file used for mutual TLS authentication. ServerInsecureSkipVerify bool // If set to true, server's certificate will not be verified against the provided CA certificate. @@ -47,8 +47,8 @@ type Certs struct { func (c *Config) Validate() (*Certs, error) { certs := &Certs{} - if c.BaseUrl == "" { - return certs, errors.New("credhub config: `BaseUrl` can't be empty") + if c.BaseURL == "" { + return certs, errors.New("credhub config: `BaseURL` can't be empty") } if c.Namespace == "" { return certs, errors.New("credhub config: `Namespace` can't be empty") @@ -66,9 +66,9 @@ func (c *Config) Validate() (*Certs, error) { return nil, errors.New(fmt.Sprintf("credhub config: error parsing the certificate '%s': %v", "ServerCaCertFilePath", err)) } } - if c.EnableMutualTls { + if c.EnableMutualTLS { if c.ClientCertFilePath == "" || c.ClientKeyFilePath == "" { - return certs, errors.New("credhub config: `ClientCertFilePath` and `ClientKeyFilePath` can't be empty when `EnableMutualTls` is true") + return certs, errors.New("credhub config: `ClientCertFilePath` and `ClientKeyFilePath` can't be empty when `EnableMutualTLS` is true") } cCertPemBytes, cCertDerBytes, err := c.validatePemFile(c.ClientCertFilePath, "ClientCertFilePath") if err != nil { @@ -105,17 +105,16 @@ func (c *Config) validatePemFile(path, name string) (pemBytes, derBytes []byte, type Store struct { LastError error config *Config - client HTTPClient + client httpClient sfGroup singleflight.Group } func NewStore(_ context.Context, config *Config) (*Store, error) { - client, err := NewHttpMTlsClient(config) + client, err := newHTTPMTLSClient(config) if err != nil { return nil, err - } else { - return &Store{config: config, client: client}, nil } + return &Store{config: config, client: client}, nil } // Status returns the current state of the KeyStore. @@ -144,13 +143,11 @@ func (s *Store) Status(ctx context.Context) (kes.KeyStoreState, error) { } else { if responseData.Status == "UP" { return state, nil - } else { - return state, fmt.Errorf("CredHub is not UP, status: %s", responseData.Status) } + return state, fmt.Errorf("CredHub is not UP, status: %s", responseData.Status) } - } else { - return state, fmt.Errorf("the CredHub (%s) is not healthy, status: %s", uri, resp.status) } + return state, fmt.Errorf("the CredHub (%s) is not healthy, status: %s", uri, resp.status) } // Create creates a new entry with the given name if and only @@ -219,9 +216,8 @@ func (s *Store) put(ctx context.Context, name string, value []byte, operationID } return nil - } else { - return fmt.Errorf("failed to put entry (status: %s)", resp.status) } + return fmt.Errorf("failed to put entry (status: %s)", resp.status) } // Delete removes the entry. It may return either no error or diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go index 3bf571a1..d9472663 100644 --- a/internal/keystore/credhub/credhub_test.go +++ b/internal/keystore/credhub/credhub_test.go @@ -26,10 +26,10 @@ const testNamespace = "/test-namespace" func TestStore_MTls(t *testing.T) { t.Run("get status request contract", func(t *testing.T) { t.Skip("skipping due to this being an integration test that requires specific configuration for a CredHub instance") - client, err := NewHttpMTlsClient(&Config{ - BaseUrl: "https://localhost:8844", + client, err := newHTTPMTLSClient(&Config{ + BaseURL: "https://localhost:8844", Namespace: testNamespace, - EnableMutualTls: true, + EnableMutualTLS: true, ClientCertFilePath: "../../../client.cert", ClientKeyFilePath: "../../../client.key", ServerInsecureSkipVerify: false, @@ -297,7 +297,7 @@ func TestStore_List(t *testing.T) { }) } -//=== tools: +// === tools: func NewFakeStore() (*FakeHttpClient, *Store) { fakeClient := &FakeHttpClient{respStatusCodes: map[string]int{}} @@ -326,7 +326,7 @@ func (m *FakeReadCloser) Close() error { return nil } -func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body io.Reader) HTTPResponse { +func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body io.Reader) httpResponse { c.reqMethod = method c.reqUri = url c.reqBody = "" @@ -339,7 +339,7 @@ func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body i mockBody := &FakeReadCloser{ Reader: bytes.NewBufferString(c.respBody), } - return HTTPResponse{statusCode: c.respStatusCodes[method], status: c.respStatus, body: mockBody, err: c.error} + return httpResponse{statusCode: c.respStatusCodes[method], status: c.respStatus, body: mockBody, err: c.error} } func assertError(t *testing.T, err error) { diff --git a/internal/keystore/credhub/http_client.go b/internal/keystore/credhub/http_client.go index 62b2013a..d44d4d94 100644 --- a/internal/keystore/credhub/http_client.go +++ b/internal/keystore/credhub/http_client.go @@ -8,36 +8,36 @@ import ( "net/http" ) -type HTTPResponse struct { +type httpResponse struct { statusCode int status string body io.ReadCloser err error } -func NewHTTPResponseError(err error) HTTPResponse { - return HTTPResponse{statusCode: -1, status: "", body: nil, err: err} +func newHTTPResponseError(err error) httpResponse { + return httpResponse{statusCode: -1, status: "", body: nil, err: err} } -func (c *HTTPResponse) isStatusCode2xx() bool { +func (c *httpResponse) isStatusCode2xx() bool { return c.statusCode >= http.StatusOK && c.statusCode < http.StatusMultipleChoices } -func (c *HTTPResponse) closeResource() { +func (c *httpResponse) closeResource() { if c.body != nil { _ = c.body.Close() } } -type HTTPClient interface { - doRequest(ctx context.Context, method, uri string, body io.Reader) HTTPResponse +type httpClient interface { + doRequest(ctx context.Context, method, uri string, body io.Reader) httpResponse } -type HTTPMTlsClient struct { +type httpMTLSClient struct { baseURL string httpClient *http.Client } -func NewHttpMTlsClient(config *Config) (HTTPClient, error) { +func newHTTPMTLSClient(config *Config) (httpClient, error) { certs, err := config.Validate() if err != nil { return nil, err @@ -51,25 +51,25 @@ func NewHttpMTlsClient(config *Config) (HTTPClient, error) { caCertPool.AddCert(certs.ServerCaCert) tlsConfig.RootCAs = caCertPool } - if config.EnableMutualTls { + if config.EnableMutualTLS { // Setup mutual TLS - client tlsConfig.Certificates = []tls.Certificate{certs.ClientKeyPair} } transport := &http.Transport{TLSClientConfig: tlsConfig} httpClient := &http.Client{Transport: transport} - return &HTTPMTlsClient{baseURL: config.BaseUrl, httpClient: httpClient}, nil + return &httpMTLSClient{baseURL: config.BaseURL, httpClient: httpClient}, nil } -func (s *HTTPMTlsClient) doRequest(ctx context.Context, method, uri string, body io.Reader) HTTPResponse { +func (s *httpMTLSClient) doRequest(ctx context.Context, method, uri string, body io.Reader) httpResponse { url := s.baseURL + uri req, err := http.NewRequestWithContext(ctx, method, url, body) if err != nil { - return NewHTTPResponseError(err) + return newHTTPResponseError(err) } req.Header.Set(contentType, applicationJson) resp, err := s.httpClient.Do(req) if err != nil { - return NewHTTPResponseError(err) + return newHTTPResponseError(err) } - return HTTPResponse{statusCode: resp.StatusCode, status: resp.Status, body: resp.Body, err: nil} + return httpResponse{statusCode: resp.StatusCode, status: resp.Status, body: resp.Body, err: nil} } diff --git a/kesconf/config.go b/kesconf/config.go index 8b9d45fa..7692749e 100644 --- a/kesconf/config.go +++ b/kesconf/config.go @@ -212,8 +212,8 @@ type ymlFile struct { } `yaml:"entrust"` CredHub *struct { - BaseUrl env[string] `yaml:"base_url"` - EnableMutualTls env[bool] `yaml:"enable_mutual_tls"` + BaseURL env[string] `yaml:"base_url"` + EnableMutualTLS env[bool] `yaml:"enable_mutual_tls"` ClientCertFilePath env[string] `yaml:"client_cert_file_path"` ClientKeyFilePath env[string] `yaml:"client_key_file_path"` ServerCaCertFilePath env[string] `yaml:"server_ca_cert_file_path"` @@ -677,8 +677,8 @@ func ymlToKeyStore(y *ymlFile) (KeyStore, error) { return nil, errors.New("kesconf: invalid CredHub config: more than once keystore specified") } config := credhub.Config{ - BaseUrl: y.KeyStore.CredHub.BaseUrl.Value, - EnableMutualTls: y.KeyStore.CredHub.EnableMutualTls.Value, + BaseURL: y.KeyStore.CredHub.BaseURL.Value, + EnableMutualTLS: y.KeyStore.CredHub.EnableMutualTLS.Value, ClientCertFilePath: y.KeyStore.CredHub.ClientCertFilePath.Value, ClientKeyFilePath: y.KeyStore.CredHub.ClientKeyFilePath.Value, ServerInsecureSkipVerify: y.KeyStore.CredHub.ServerInsecureSkipVerify.Value, From 1e20962698a84d2d3ab855b98edec5c48f3d3fa9 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 09:47:11 +0200 Subject: [PATCH 06/10] Lint related changes --- internal/keystore/credhub/credhub.go | 12 ++++---- internal/keystore/credhub/credhub_test.go | 36 +++++++++++------------ internal/keystore/credhub/http_client.go | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/internal/keystore/credhub/credhub.go b/internal/keystore/credhub/credhub.go index e109ecb4..8c9b3a37 100644 --- a/internal/keystore/credhub/credhub.go +++ b/internal/keystore/credhub/credhub.go @@ -26,7 +26,7 @@ import ( const ( contentType = "Content-Type" - applicationJson = "application/json" + applicationJSON = "application/json" ) type Config struct { @@ -63,7 +63,7 @@ func (c *Config) Validate() (*Certs, error) { } certs.ServerCaCert, err = x509.ParseCertificate(sCertDerBytes) if err != nil { - return nil, errors.New(fmt.Sprintf("credhub config: error parsing the certificate '%s': %v", "ServerCaCertFilePath", err)) + return nil, fmt.Errorf("credhub config: error parsing the certificate '%s': %v", "ServerCaCertFilePath", err) } } if c.EnableMutualTLS { @@ -76,7 +76,7 @@ func (c *Config) Validate() (*Certs, error) { } _, err = x509.ParseCertificate(cCertDerBytes) if err != nil { - return nil, errors.New(fmt.Sprintf("credhub config: error parsing the certificate '%s': %v", "ClientCertFilePath", err)) + return nil, fmt.Errorf("credhub config: error parsing the certificate '%s': %v", "ClientCertFilePath", err) } cKeyPemBytes, _, err := c.validatePemFile(c.ClientKeyFilePath, "ClientKeyFilePath") if err != nil { @@ -202,7 +202,7 @@ func (s *Store) put(ctx context.Context, name string, value []byte, operationID var responseData struct { Value string `json:"value"` Metadata struct { - OperationId string `json:"operation_id"` + OperationID string `json:"operation_id"` } `json:"metadata"` } if err := json.NewDecoder(resp.body).Decode(&responseData); err != nil { @@ -211,8 +211,8 @@ func (s *Store) put(ctx context.Context, name string, value []byte, operationID if responseData.Value != valueStr { return fmt.Errorf("key '%s' was inserted but overwritten by other process (the returned value is different from the the one sent): %w", name, kesdk.ErrKeyExists) } - if responseData.Metadata.OperationId != operationID { - return fmt.Errorf("key '%s' was inserted but overwritten by other process (operation ID %s != %s): %w", name, responseData.Metadata.OperationId, operationID, kesdk.ErrKeyExists) + if responseData.Metadata.OperationID != operationID { + return fmt.Errorf("key '%s' was inserted but overwritten by other process (operation ID %s != %s): %w", name, responseData.Metadata.OperationID, operationID, kesdk.ErrKeyExists) } return nil diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go index d9472663..b47548e7 100644 --- a/internal/keystore/credhub/credhub_test.go +++ b/internal/keystore/credhub/credhub_test.go @@ -23,7 +23,7 @@ The following is checked: const testNamespace = "/test-namespace" // `curl -v --cert ./client.cert --key ./client.key --cacert ./server-ca.cert https://localhost:8844/api/v1/data?path=/` -func TestStore_MTls(t *testing.T) { +func TestStore_MTLS(t *testing.T) { t.Run("get status request contract", func(t *testing.T) { t.Skip("skipping due to this being an integration test that requires specific configuration for a CredHub instance") client, err := newHTTPMTLSClient(&Config{ @@ -82,7 +82,7 @@ func TestStore_put(t *testing.T) { store.config.ForceBase64ValuesEncoding = false err := store.put(context.Background(), key, []byte(value), operationID) assertNoError(t, err) - assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + assertRequestWithJSONBody(t, fakeClient, "PUT", "/api/v1/data", fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, value, operationID)) }) @@ -96,7 +96,7 @@ func TestStore_put(t *testing.T) { fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID) err := store.put(context.Background(), key, []byte(value), operationID) assertNoError(t, err) - assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + assertRequestWithJSONBody(t, fakeClient, "PUT", "/api/v1/data", fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID)) }) t.Run("PUT bytes value with not valid UTF-8 bytes", func(t *testing.T) { @@ -109,7 +109,7 @@ func TestStore_put(t *testing.T) { fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID) err := store.put(context.Background(), key, value, operationID) assertNoError(t, err) - assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + assertRequestWithJSONBody(t, fakeClient, "PUT", "/api/v1/data", fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID)) }) t.Run("PUT string value starts with 'Base64:' request contract", func(t *testing.T) { @@ -122,7 +122,7 @@ func TestStore_put(t *testing.T) { fakeClient.respBody = fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID) err := store.put(context.Background(), key, []byte(value), operationID) assertNoError(t, err) - assertRequestWithJsonBody(t, fakeClient, "PUT", "/api/v1/data", + assertRequestWithJSONBody(t, fakeClient, "PUT", "/api/v1/data", fmt.Sprintf(`{"name":"%s/%s","type":"value","value":"%s","metadata":{"operation_id":"%s"}}`, testNamespace, key, encodedValue, operationID)) }) } @@ -138,7 +138,7 @@ func TestStore_Create(t *testing.T) { const value = "string-value" err := store.Create(context.Background(), key, []byte(value)) assertErrorIs(t, err, kes.ErrKeyExists) - assertApiErrorStatus(t, err, http.StatusBadRequest) + assertAPIErrorStatus(t, err, http.StatusBadRequest) }) t.Run("create element that doesn't exist", func(t *testing.T) { fakeClient.respStatusCodes["GET"] = 404 @@ -218,7 +218,7 @@ func TestStore_Get(t *testing.T) { const name = "element-name" _, err := store.Get(context.Background(), name) assertErrorIs(t, err, kes.ErrKeyNotFound) - assertApiErrorStatus(t, err, http.StatusNotFound) + assertAPIErrorStatus(t, err, http.StatusNotFound) }) } @@ -240,7 +240,7 @@ func TestStore_Delete(t *testing.T) { const name = "element-name" err := store.Delete(context.Background(), name) assertErrorIs(t, err, kes.ErrKeyNotFound) - assertApiErrorStatus(t, err, http.StatusNotFound) + assertAPIErrorStatus(t, err, http.StatusNotFound) }) } @@ -299,8 +299,8 @@ func TestStore_List(t *testing.T) { // === tools: -func NewFakeStore() (*FakeHttpClient, *Store) { - fakeClient := &FakeHttpClient{respStatusCodes: map[string]int{}} +func NewFakeStore() (*FakeHTTPClient, *Store) { + fakeClient := &FakeHTTPClient{respStatusCodes: map[string]int{}} store := &Store{ config: &Config{Namespace: testNamespace}, client: fakeClient, @@ -308,7 +308,7 @@ func NewFakeStore() (*FakeHttpClient, *Store) { return fakeClient, store } -type FakeHttpClient struct { +type FakeHTTPClient struct { reqMethod string reqUri string reqBody string @@ -326,7 +326,7 @@ func (m *FakeReadCloser) Close() error { return nil } -func (c *FakeHttpClient) doRequest(_ context.Context, method, url string, body io.Reader) httpResponse { +func (c *FakeHTTPClient) doRequest(_ context.Context, method, url string, body io.Reader) httpResponse { c.reqMethod = method c.reqUri = url c.reqBody = "" @@ -357,7 +357,7 @@ func assertErrorIs(t *testing.T, err, target error) { } } -func assertApiErrorStatus(t *testing.T, err error, status int) { +func assertAPIErrorStatus(t *testing.T, err error, status int) { if err == nil { t.Fatal("error can't be null") } @@ -387,7 +387,7 @@ func assertEqualBytes(t *testing.T, expected, got []byte) { } } -func assertRequest(t *testing.T, fc *FakeHttpClient, method, uri string) { +func assertRequest(t *testing.T, fc *FakeHTTPClient, method, uri string) { if fc.reqMethod != method { t.Fatalf("expected requested method '%s' but got '%s'", method, fc.reqMethod) } @@ -395,18 +395,18 @@ func assertRequest(t *testing.T, fc *FakeHttpClient, method, uri string) { t.Fatalf("expected requested uri '%s' but got '%s'", uri, fc.reqUri) } } -func assertRequestWithJsonBody(t *testing.T, fc *FakeHttpClient, method, uri string, jsonBody string) { +func assertRequestWithJSONBody(t *testing.T, fc *FakeHTTPClient, method, uri string, jsonBody string) { assertRequest(t, fc, method, uri) - var gotJson, expectedJson interface{} - err1 := json.Unmarshal([]byte(fc.reqBody), &gotJson) + var gotJSON, expectedJson interface{} + err1 := json.Unmarshal([]byte(fc.reqBody), &gotJSON) err2 := json.Unmarshal([]byte(jsonBody), &expectedJson) if err1 != nil || err2 != nil { t.Fatalf("jsons deserialization errors: %v, %v", err1, err2) } - if !reflect.DeepEqual(gotJson, expectedJson) { + if !reflect.DeepEqual(gotJSON, expectedJson) { t.Fatalf("expected requested body '%s' but got '%s'", jsonBody, fc.reqBody) } } diff --git a/internal/keystore/credhub/http_client.go b/internal/keystore/credhub/http_client.go index d44d4d94..3e450db2 100644 --- a/internal/keystore/credhub/http_client.go +++ b/internal/keystore/credhub/http_client.go @@ -66,7 +66,7 @@ func (s *httpMTLSClient) doRequest(ctx context.Context, method, uri string, body if err != nil { return newHTTPResponseError(err) } - req.Header.Set(contentType, applicationJson) + req.Header.Set(contentType, applicationJSON) resp, err := s.httpClient.Do(req) if err != nil { return newHTTPResponseError(err) From 65224a6f66c5083f9590b2371628005e53616366 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 10:26:44 +0200 Subject: [PATCH 07/10] Lint related changes --- internal/keystore/credhub/credhub.go | 23 +++++++++++++------- internal/keystore/credhub/credhub_test.go | 2 +- internal/keystore/credhub/value_converter.go | 12 +++++----- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/internal/keystore/credhub/credhub.go b/internal/keystore/credhub/credhub.go index 8c9b3a37..12b9b3c8 100644 --- a/internal/keystore/credhub/credhub.go +++ b/internal/keystore/credhub/credhub.go @@ -29,6 +29,7 @@ const ( applicationJSON = "application/json" ) +// Config holds the configuration settings for connecting to a CredHub service. type Config struct { BaseURL string // The base URL endpoint of the CredHub service. EnableMutualTLS bool // If set to true, enables mutual TLS. @@ -40,11 +41,14 @@ type Config struct { ForceBase64ValuesEncoding bool // If set to true, forces encoding of all the values as base64 before storage. } +// Certs contains the certificates needed for mutual TLS authentication. type Certs struct { ServerCaCert *x509.Certificate ClientKeyPair tls.Certificate } +// Validate checks the configuration for correctness and loads the necessary certificates for mutual TLS authentication. +// It returns a Certs object containing the server CA certificate and client key pair, or an error if validation fails. func (c *Config) Validate() (*Certs, error) { certs := &Certs{} if c.BaseURL == "" { @@ -93,15 +97,16 @@ func (c *Config) Validate() (*Certs, error) { func (c *Config) validatePemFile(path, name string) (pemBytes, derBytes []byte, err error) { pemBytes, err = os.ReadFile(path) if err != nil { - return pemBytes, nil, errors.New(fmt.Sprintf("credhub config: failed to load PEM file '%s'='%s': %v", name, path, err)) + return pemBytes, nil, fmt.Errorf("credhub config: failed to load PEM file '%s'='%s': %v", name, path, err) } derBlock, _ := pem.Decode(pemBytes) if derBlock == nil { - return pemBytes, nil, errors.New(fmt.Sprintf("credhub config: failed to decode the '%s'='%s' from PEM format, no PEM data found", name, path)) + return pemBytes, nil, fmt.Errorf("credhub config: failed to decode the '%s'='%s' from PEM format, no PEM data found", name, path) } return pemBytes, derBlock.Bytes, nil } +// Store represents a layer that interacts with a CredHub service using HTTP protocol. type Store struct { LastError error config *Config @@ -109,6 +114,8 @@ type Store struct { sfGroup singleflight.Group } +// NewStore creates a new instance of Store, initializing it with the provided configuration. +// It returns an error if the HTTP client initialization fails. func NewStore(_ context.Context, config *Config) (*Store, error) { client, err := newHTTPMTLSClient(config) if err != nil { @@ -140,12 +147,12 @@ func (s *Store) Status(ctx context.Context) (kes.KeyStoreState, error) { } if err := json.NewDecoder(resp.body).Decode(&responseData); err != nil { return state, fmt.Errorf("failed to parse response: %v", err) - } else { - if responseData.Status == "UP" { - return state, nil - } - return state, fmt.Errorf("CredHub is not UP, status: %s", responseData.Status) } + if responseData.Status == "UP" { + return state, nil + } + return state, fmt.Errorf("CredHub is not UP, status: %s", responseData.Status) + } return state, fmt.Errorf("the CredHub (%s) is not healthy, status: %s", uri, resp.status) } @@ -179,7 +186,7 @@ func (s *Store) create(ctx context.Context, name string, value []byte, operation // - `credhub curl -X=PUT -p "/api/v1/data" -d='{"name":"/test-namespace/key-1","type":"value","value":"1"}` func (s *Store) put(ctx context.Context, name string, value []byte, operationID string) error { uri := "/api/v1/data" - valueStr := bytesToJsonString(value, s.config.ForceBase64ValuesEncoding) + valueStr := bytesToJSONString(value, s.config.ForceBase64ValuesEncoding) data := map[string]interface{}{ "name": s.config.Namespace + "/" + name, "type": "value", diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go index b47548e7..72292f67 100644 --- a/internal/keystore/credhub/credhub_test.go +++ b/internal/keystore/credhub/credhub_test.go @@ -191,7 +191,7 @@ func TestStore_Get(t *testing.T) { t.Run("GET bytes value with Base64 encoding request contract", func(t *testing.T) { const key = "key" value := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 80, 114, 122, 255, 121, 107, 108, 255} - encodedValue := bytesToJsonString(value, true) + encodedValue := bytesToJSONString(value, true) fakeClient.respStatusCodes["GET"] = 200 fakeClient.respBody = fmt.Sprintf(` { diff --git a/internal/keystore/credhub/value_converter.go b/internal/keystore/credhub/value_converter.go index 28029478..82dc5fb4 100644 --- a/internal/keystore/credhub/value_converter.go +++ b/internal/keystore/credhub/value_converter.go @@ -10,21 +10,21 @@ import ( "unicode/utf8" ) -const Base64Prefix = "Base64:" +const base64Prefix = "Base64:" -func bytesToJsonString(bytes []byte, forceBase64 bool) (value string) { +func bytesToJSONString(bytes []byte, forceBase64 bool) (value string) { if utf8.Valid(bytes) && !forceBase64 { strBytes := string(bytes) - if !strings.HasPrefix(strBytes, Base64Prefix) { + if !strings.HasPrefix(strBytes, base64Prefix) { return string(bytes) } } - return Base64Prefix + base64.StdEncoding.EncodeToString(bytes) + return base64Prefix + base64.StdEncoding.EncodeToString(bytes) } func jsonStringToBytes(value string) (bytes []byte, err error) { - if strings.HasPrefix(value, Base64Prefix) { - return base64.StdEncoding.DecodeString(strings.TrimPrefix(value, Base64Prefix)) + if strings.HasPrefix(value, base64Prefix) { + return base64.StdEncoding.DecodeString(strings.TrimPrefix(value, base64Prefix)) } return []byte(value), nil } From 4ab70db1fbb5222972f09750f0fcec3992275243 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 10:31:22 +0200 Subject: [PATCH 08/10] Lint related changes --- internal/keystore/credhub/credhub_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go index 72292f67..c84c8fdd 100644 --- a/internal/keystore/credhub/credhub_test.go +++ b/internal/keystore/credhub/credhub_test.go @@ -310,7 +310,7 @@ func NewFakeStore() (*FakeHTTPClient, *Store) { type FakeHTTPClient struct { reqMethod string - reqUri string + reqURI string reqBody string respStatusCodes map[string]int respStatus string @@ -328,7 +328,7 @@ func (m *FakeReadCloser) Close() error { func (c *FakeHTTPClient) doRequest(_ context.Context, method, url string, body io.Reader) httpResponse { c.reqMethod = method - c.reqUri = url + c.reqURI = url c.reqBody = "" if body != nil { bodyBytes, err := io.ReadAll(body) @@ -391,22 +391,22 @@ func assertRequest(t *testing.T, fc *FakeHTTPClient, method, uri string) { if fc.reqMethod != method { t.Fatalf("expected requested method '%s' but got '%s'", method, fc.reqMethod) } - if fc.reqUri != uri { - t.Fatalf("expected requested uri '%s' but got '%s'", uri, fc.reqUri) + if fc.reqURI != uri { + t.Fatalf("expected requested uri '%s' but got '%s'", uri, fc.reqURI) } } func assertRequestWithJSONBody(t *testing.T, fc *FakeHTTPClient, method, uri string, jsonBody string) { assertRequest(t, fc, method, uri) - var gotJSON, expectedJson interface{} + var gotJSON, expectedJSON interface{} err1 := json.Unmarshal([]byte(fc.reqBody), &gotJSON) - err2 := json.Unmarshal([]byte(jsonBody), &expectedJson) + err2 := json.Unmarshal([]byte(jsonBody), &expectedJSON) if err1 != nil || err2 != nil { t.Fatalf("jsons deserialization errors: %v, %v", err1, err2) } - if !reflect.DeepEqual(gotJSON, expectedJson) { + if !reflect.DeepEqual(gotJSON, expectedJSON) { t.Fatalf("expected requested body '%s' but got '%s'", jsonBody, fc.reqBody) } } From 1932d1542a14f7c1de4c66b4f068500fef9c8e66 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 10:38:37 +0200 Subject: [PATCH 09/10] Lint related changes --- internal/keystore/credhub/credhub.go | 9 +++++---- internal/keystore/credhub/credhub_test.go | 9 +++++---- internal/keystore/credhub/http_client.go | 1 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/internal/keystore/credhub/credhub.go b/internal/keystore/credhub/credhub.go index 12b9b3c8..5e96e837 100644 --- a/internal/keystore/credhub/credhub.go +++ b/internal/keystore/credhub/credhub.go @@ -13,15 +13,16 @@ import ( "encoding/pem" "errors" "fmt" + "net/http" + "os" + "strings" + "time" + "github.com/golang/groupcache/singleflight" "github.com/google/uuid" "github.com/minio/kes" "github.com/minio/kes/internal/keystore" kesdk "github.com/minio/kms-go/kes" - "net/http" - "os" - "strings" - "time" ) const ( diff --git a/internal/keystore/credhub/credhub_test.go b/internal/keystore/credhub/credhub_test.go index c84c8fdd..8ef0f55f 100644 --- a/internal/keystore/credhub/credhub_test.go +++ b/internal/keystore/credhub/credhub_test.go @@ -6,12 +6,13 @@ import ( "encoding/json" "errors" "fmt" - "github.com/minio/kes/internal/api" - "github.com/minio/kms-go/kes" "io" "net/http" "reflect" "testing" + + "github.com/minio/kes/internal/api" + "github.com/minio/kms-go/kes" ) /** CredHub Rest API contract tests. @@ -40,7 +41,6 @@ func TestStore_MTLS(t *testing.T) { assertNoError(t, resp.err) fmt.Println(resp.status) }) - } // `credhub curl -X=GET -p /health` @@ -220,7 +220,6 @@ func TestStore_Get(t *testing.T) { assertErrorIs(t, err, kes.ErrKeyNotFound) assertAPIErrorStatus(t, err, http.StatusNotFound) }) - } // `credhub curl -X=DELETE -p "/api/v1/data?name=/test-namespace/element-name"` @@ -381,6 +380,7 @@ func assertEqualComparable(t *testing.T, expected, got any) { t.Fatalf("expected '%v' got '%v'", expected, got) } } + func assertEqualBytes(t *testing.T, expected, got []byte) { if !bytes.Equal(expected, got) { t.Fatalf("expected '%v' got '%v'", expected, got) @@ -395,6 +395,7 @@ func assertRequest(t *testing.T, fc *FakeHTTPClient, method, uri string) { t.Fatalf("expected requested uri '%s' but got '%s'", uri, fc.reqURI) } } + func assertRequestWithJSONBody(t *testing.T, fc *FakeHTTPClient, method, uri string, jsonBody string) { assertRequest(t, fc, method, uri) diff --git a/internal/keystore/credhub/http_client.go b/internal/keystore/credhub/http_client.go index 3e450db2..18ba4dbd 100644 --- a/internal/keystore/credhub/http_client.go +++ b/internal/keystore/credhub/http_client.go @@ -18,6 +18,7 @@ type httpResponse struct { func newHTTPResponseError(err error) httpResponse { return httpResponse{statusCode: -1, status: "", body: nil, err: err} } + func (c *httpResponse) isStatusCode2xx() bool { return c.statusCode >= http.StatusOK && c.statusCode < http.StatusMultipleChoices } From d8af71e5259acc28f6172ad2f616d0ab44922252 Mon Sep 17 00:00:00 2001 From: kinjelom Date: Mon, 26 Aug 2024 11:02:34 +0200 Subject: [PATCH 10/10] Lint related changes --- kesconf/config.go | 1 - 1 file changed, 1 deletion(-) diff --git a/kesconf/config.go b/kesconf/config.go index 7692749e..2495acdf 100644 --- a/kesconf/config.go +++ b/kesconf/config.go @@ -687,7 +687,6 @@ func ymlToKeyStore(y *ymlFile) (KeyStore, error) { ForceBase64ValuesEncoding: y.KeyStore.CredHub.ForceBase64ValuesEncoding.Value, } _, err := config.Validate() - if err != nil { return nil, err }