feat: return error response bodies in config svc (#271)

* feat: return error response bodies in config svc
* chore: linter ignore long comments

.golangci.yml

@@ -46,6 +46,10 @@ linters:
 
 issues:
   max-same-issues: 0
+  exclude-rules:
+  - linters:
+    - lll
+    source: "^\t*// "
 
 linters-settings:
   gomodguard:
@@ -59,4 +63,4 @@ linters-settings:
           - sigs.k8s.io/yaml
       - gopkg.in/yaml.v3:
           recommendations:
-          - sigs.k8s.io/yaml
+          - sigs.k8s.io/yaml

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Table of Contents
 
+- [v0.37.0](#v0370)
 - [v0.36.0](#v0360)
 - [v0.35.0](#v0350)
 - [v0.34.1](#v0341)
@@ -45,6 +46,23 @@
 - [0.2.0](#020)
 - [0.1.0](#010)
 
+## [v0.37.0]
+
+> Release date: 2023/02/03
+
+- **Breaking change:** the `ConfigService` is now directly embedded in the
+  `kong.Client`. Configurations are collections of entities, not entities
+  themselves, so they do not fit with other go-kong services.
+- **Breaking change:** `ReloadDeclarativeRawConfig()` (formerly part of
+  `ConfigService`, now part of `kong.Client`) now has the response signature
+  `([]byte, error)` instead of `error`. The byte slice is the config response
+  body. The error is unchanged.
+- **Breaking change:** `ReloadDeclarativeRawConfig()` now requires a
+  `flattenErrors` boolean argument. When `true`, requests will include
+  `flatten_errors=1` in the query string, to activate the functionality added
+  in https://github.com/Kong/kong/pull/10161.
+  [#273](https://github.com/Kong/go-kong/pull/273)
+
 ## [v0.36.0]
 
 > Release date: 2023/01/24
@@ -666,6 +684,7 @@ authentication credentials in Kong.
   releases of Kong since every release of Kong is introducing breaking changes
   to the Admin API.
 
+[v0.37.0]: https://github.com/Kong/go-kong/compare/v0.36.0...v0.37.0
 [v0.36.0]: https://github.com/Kong/go-kong/compare/v0.35.0...v0.36.0
 [v0.35.0]: https://github.com/Kong/go-kong/compare/v0.34.1...v0.35.0
 [v0.34.1]: https://github.com/Kong/go-kong/compare/v0.34.0...v0.34.1

kong/client.go

@@ -39,7 +39,6 @@ type Client struct {
 	workspace               string       // Do not access directly. Use Workspace()/SetWorkspace().
 	workspaceLock           sync.RWMutex // Synchronizes access to workspace.
 	common                  service
-	Configs                 AbstractConfigService
 	ConsumerGroupConsumers  AbstractConsumerGroupConsumerService
 	ConsumerGroups          AbstractConsumerGroupService
 	Consumers               AbstractConsumerService
@@ -132,7 +131,6 @@ func NewClient(baseURL *string, client *http.Client) (*Client, error) {
 	kong.baseRootURL = url.String()
 
 	kong.common.client = kong
-	kong.Configs = (*ConfigService)(&kong.common)
 	kong.ConsumerGroupConsumers = (*ConsumerGroupConsumerService)(&kong.common)
 	kong.ConsumerGroups = (*ConsumerGroupService)(&kong.common)
 	kong.Consumers = (*ConsumerService)(&kong.common)
@@ -238,6 +236,7 @@ func (c *Client) DoRAW(ctx context.Context, req *http.Request) (*http.Response,
 func (c *Client) Do(ctx context.Context, req *http.Request,
 	v interface{},
 ) (*Response, error) {
+	// TODO https://github.com/Kong/go-kong/issues/273 clear this lint ignore
 	resp, err := c.DoRAW(ctx, req) //nolint:bodyclose
 	if err != nil {
 		return nil, err
@@ -377,3 +376,53 @@ func (c *Client) RootJSON(ctx context.Context) ([]byte, error) {
 func (c *Client) BaseRootURL() string {
 	return c.baseRootURL
 }
+
+// ReloadDeclarativeRawConfig sends out the specified config to configured Admin
+// API endpoint using the provided reader which should contain the JSON
+// serialized body that adheres to the configuration format specified at:
+// https://docs.konghq.com/gateway/latest/production/deployment-topologies/db-less-and-declarative-config/#declarative-configuration-format
+// It returns the response body and an error, if it encounters any.
+func (c *Client) ReloadDeclarativeRawConfig(
+	ctx context.Context,
+	config io.Reader,
+	checkHash bool,
+	flattenErrors bool,
+) ([]byte, error) {
+	type sendConfigParams struct {
+		CheckHash     int `url:"check_hash,omitempty"`
+		FlattenErrors int `url:"flatten_errors,omitempty"`
+	}
+	var checkHashI int
+	if checkHash {
+		checkHashI = 1
+	}
+	var flattenErrorsI int
+	if flattenErrors {
+		flattenErrorsI = 1
+	}
+	req, err := c.NewRequest(
+		"POST",
+		"/config",
+		sendConfigParams{CheckHash: checkHashI, FlattenErrors: flattenErrorsI},
+		config,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("creating new HTTP request for /config: %w", err)
+	}
+
+	resp, err := c.DoRAW(ctx, req)
+	if err != nil {
+		return nil, fmt.Errorf("failed posting new config to /config: %w", err)
+	}
+	defer resp.Body.Close()
+
+	b, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("could not read /config %d status response body: %w", resp.StatusCode, err)
+	}
+	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
+		return b, fmt.Errorf("failed posting new config to /config: got status code %d and body %s", resp.StatusCode, b)
+	}
+
+	return b, nil
+}

kong/client_test.go

@@ -1,7 +1,9 @@
 package kong
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"net/http"
 	"os"
 	"testing"
@@ -205,3 +207,113 @@ func TestBaseRootURL(t *testing.T) {
 		require.Equal(t, client.BaseRootURL(), "https://customkong2.com")
 	})
 }
+
+func TestReloadDeclarativeRawConfig(t *testing.T) {
+	RunWhenDBMode(t, "off")
+
+	tests := []struct {
+		name    string
+		config  Configuration
+		wantErr bool
+	}{
+		{
+			name: "basic config works",
+			config: Configuration{
+				"_format_version": "1.1",
+				"services": []Configuration{
+					{
+						"host":     "mockbin.com",
+						"port":     443,
+						"protocol": "https",
+						"routes": []Configuration{
+							{"paths": []string{"/"}},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "missing _format_version fails",
+			config: Configuration{
+				"services": []Configuration{
+					{
+						"host":     "mockbin.com",
+						"port":     443,
+						"protocol": "https",
+						"routes": []Configuration{
+							{"paths": []string{"/"}},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid config fails",
+			config: Configuration{
+				"dummy_key": []Configuration{
+					{
+						"host":     "mockbin.com",
+						"port":     443,
+						"protocol": "https",
+					},
+				},
+			},
+			wantErr: true,
+		},
+	}
+
+	// POST /config actually interprets query string fields as additional fields within the config unless they're
+	// explicitly stripped by the API code (no idea why this behavior exists). Without stripping "flatten_errors" in
+	// https://github.com/Kong/kong/blob/master/kong/api/routes/config.lua#L71 Kong will actually reject configuration
+	// because it thinks "flatten_errors" is an actual (and invalid) field inside the config.
+	//
+	// This is the one test where we want version-dependent behavior, but only for one value within the test. We test
+	// config updates on all DB-less capable versions, but only set flattenErrors=true on 3.2+. To handle that, this
+	// snippet is borrowed from RunWhenKong, to allow toggling that behavior only on or off depending on the version.
+	var flattenErrors bool
+	client, err := NewTestClient(nil, nil)
+	require.NoError(t, err)
+	info, err := client.Root(defaultCtx)
+	require.NoError(t, err)
+	version := VersionFromInfo(info)
+	currentVersion, err := ParseSemanticVersion(version)
+	require.NoError(t, err)
+	r, err := NewRange(">=3.2.0")
+	require.NoError(t, err)
+
+	if r(currentVersion) {
+		t.Log("Kong version >=3.2, enabling flattenErrors for ReloadDeclarativeRawConfig")
+		flattenErrors = true
+	} else {
+		t.Log("Kong version <3.2, disabling flattenErrors for ReloadDeclarativeRawConfig")
+	}
+
+	for _, tt := range tests {
+		client, err := NewTestClient(nil, nil)
+		require.NoError(t, err)
+		require.NotNil(t, client)
+
+		tt := tt
+		t.Run("with_schema/"+tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			b, err := json.Marshal(tt.config)
+			require.NoError(t, err)
+
+			body, err := client.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true, flattenErrors)
+
+			if tt.wantErr {
+				assert.Errorf(t, err, "Client.SendConfig() got unexpected error")
+			} else {
+				assert.NoError(t, err)
+			}
+
+			// this is somewhat untrue: network or HTTP-level failures _can_ result in a nil response body. however,
+			// none of our test cases should cause network or HTTP-level failures, so fail if they do occur. if this
+			// _does_ encounter such a failure, we need to investigate and either update tests or fix some upstream bug
+			// if it's not some transient issue with the testing environment
+			require.NotNilf(t, body, "body was nil; should never be nil")
+		})
+	}
+}