From 4c4e1d0b5849f495d7a83e2760170c240727b510 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Wed, 25 Jan 2023 15:37:50 -0800 Subject: [PATCH 01/17] feat: return error response bodies in config svc --- kong/config_service.go | 27 +++++++++++---------------- kong/config_service_test.go | 2 +- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/kong/config_service.go b/kong/config_service.go index 1a2740fb3..e97a2dd09 100644 --- a/kong/config_service.go +++ b/kong/config_service.go @@ -1,4 +1,3 @@ -//nolint:lll package kong import ( @@ -13,7 +12,7 @@ type AbstractConfigService interface { // 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 - ReloadDeclarativeRawConfig(ctx context.Context, config io.Reader, checkHash bool) error + ReloadDeclarativeRawConfig(ctx context.Context, config io.Reader, checkHash bool) ([]byte, error) } // ConfigService handles Config in Kong. @@ -27,7 +26,7 @@ func (c *ConfigService) ReloadDeclarativeRawConfig( ctx context.Context, config io.Reader, checkHash bool, -) error { +) ([]byte, error) { type sendConfigParams struct { CheckHash int `url:"check_hash"` } @@ -37,29 +36,25 @@ func (c *ConfigService) ReloadDeclarativeRawConfig( } req, err := c.client.NewRequest("POST", "/config", sendConfigParams{CheckHash: checkHashI}, config) if err != nil { - return fmt.Errorf("creating new HTTP request for /config: %w", err) + return []byte{}, fmt.Errorf("creating new HTTP request for /config: %w", err) } resp, err := c.client.DoRAW(ctx, req) if err != nil { - return fmt.Errorf("failed posting new config to /config: %w", err) + return []byte{}, fmt.Errorf("failed posting new config to /config: %w", err) } defer resp.Body.Close() + var b []byte if resp.StatusCode < 200 || resp.StatusCode >= 400 { - b, err := io.ReadAll(resp.Body) + b, err = io.ReadAll(resp.Body) if err != nil { - return fmt.Errorf( - "failed posting new config to /config: got status code %d (and failed to read the response body): %w", - resp.StatusCode, err, - ) + return []byte{}, + fmt.Errorf(`failed posting new config to /config: got status code %d + (and failed to read the response body): %w`, + resp.StatusCode, err) } - - return fmt.Errorf( - "failed posting new config to /config: got status code %d, body: %s", - resp.StatusCode, b, - ) } - return nil + return b, nil } diff --git a/kong/config_service_test.go b/kong/config_service_test.go index 0345f0c3f..9270ac55c 100644 --- a/kong/config_service_test.go +++ b/kong/config_service_test.go @@ -76,7 +76,7 @@ func TestConfigService(t *testing.T) { b, err := json.Marshal(tt.config) require.NoError(t, err) - if err := client.Configs.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true); (err != nil) != tt.wantErr { + if err, _ := client.Configs.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true); (err != nil) != tt.wantErr { t.Errorf("Client.SendConfig() error = %v, wantErr %v", err, tt.wantErr) } }) From 7dd2a063fc18b5b3716fd70b05d69925632c93de Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Mon, 30 Jan 2023 18:01:47 -0800 Subject: [PATCH 02/17] chore: linter ignore long comments --- .golangci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 7891f86fb..511eec851 100644 --- a/.golangci.yml +++ b/.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 \ No newline at end of file + - sigs.k8s.io/yaml From 0bff000261f8256c8999c459980dfb0f801752b1 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Wed, 1 Feb 2023 13:53:39 -0800 Subject: [PATCH 03/17] pr: review suggestions --- kong/config_service.go | 2 ++ kong/config_service_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/kong/config_service.go b/kong/config_service.go index e97a2dd09..55234d4a3 100644 --- a/kong/config_service.go +++ b/kong/config_service.go @@ -12,6 +12,7 @@ type AbstractConfigService interface { // 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. ReloadDeclarativeRawConfig(ctx context.Context, config io.Reader, checkHash bool) ([]byte, error) } @@ -22,6 +23,7 @@ type ConfigService service // 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 *ConfigService) ReloadDeclarativeRawConfig( ctx context.Context, config io.Reader, diff --git a/kong/config_service_test.go b/kong/config_service_test.go index 9270ac55c..2cbbad8ad 100644 --- a/kong/config_service_test.go +++ b/kong/config_service_test.go @@ -76,7 +76,7 @@ func TestConfigService(t *testing.T) { b, err := json.Marshal(tt.config) require.NoError(t, err) - if err, _ := client.Configs.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true); (err != nil) != tt.wantErr { + if _, err := client.Configs.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true); (err != nil) != tt.wantErr { t.Errorf("Client.SendConfig() error = %v, wantErr %v", err, tt.wantErr) } }) From a255015aa3cf63c474f7daeabba9067d32e9943c Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Wed, 1 Feb 2023 14:04:36 -0800 Subject: [PATCH 04/17] chore: check config response body in tests --- kong/config_service_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kong/config_service_test.go b/kong/config_service_test.go index 2cbbad8ad..37c29659a 100644 --- a/kong/config_service_test.go +++ b/kong/config_service_test.go @@ -76,9 +76,15 @@ func TestConfigService(t *testing.T) { b, err := json.Marshal(tt.config) require.NoError(t, err) - if _, err := client.Configs.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true); (err != nil) != tt.wantErr { + body, err := client.Configs.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true) + if (err != nil) != tt.wantErr { t.Errorf("Client.SendConfig() error = %v, wantErr %v", err, tt.wantErr) } + // 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") }) } } From 5962d965fc92837c2b0559a27f245fe1e2bcb687 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 13:19:36 -0800 Subject: [PATCH 05/17] pr: nil instead of empty slice --- kong/config_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/config_service.go b/kong/config_service.go index 55234d4a3..5bc693165 100644 --- a/kong/config_service.go +++ b/kong/config_service.go @@ -51,7 +51,7 @@ func (c *ConfigService) ReloadDeclarativeRawConfig( if resp.StatusCode < 200 || resp.StatusCode >= 400 { b, err = io.ReadAll(resp.Body) if err != nil { - return []byte{}, + return nil, fmt.Errorf(`failed posting new config to /config: got status code %d (and failed to read the response body): %w`, resp.StatusCode, err) From c4556a01d857a2dfd57ec356eeb4c79a0140c8ef Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 13:20:38 -0800 Subject: [PATCH 06/17] chore: release v0.37.0 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a69156ff..6600dea87 100644 --- a/CHANGELOG.md +++ b/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,14 @@ - [0.2.0](#020) - [0.1.0](#010) +## [v0.37.0] + +> Release date: 2023/02/03 + +- **Breaking change** to `ConfigService.ReloadDeclarativeRawConfig()`. Its + response signature is now `([]byte, error)`. The byte slice is the config + response body. The error is unchanged. + ## [v0.36.0] > Release date: 2023/01/24 @@ -666,6 +675,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 From 48ce556560a67db731bf9b9513952384b6c0272a Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 13:36:14 -0800 Subject: [PATCH 07/17] chore: silence linter --- kong/client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kong/client.go b/kong/client.go index 447253bd5..dbde0ab86 100644 --- a/kong/client.go +++ b/kong/client.go @@ -238,7 +238,8 @@ 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) { - resp, err := c.DoRAW(ctx, req) + // 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 } From 7008ad752510aefda4990703fd74886694c210bc Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 14:15:01 -0800 Subject: [PATCH 08/17] chore: refactor declarative config functionality Remove the config service and wrap its functionality into the Kong client. --- CHANGELOG.md | 11 +++-- kong/client.go | 44 +++++++++++++++++- kong/config_service.go | 62 ------------------------- kong/config_service_test.go | 90 ------------------------------------- 4 files changed, 50 insertions(+), 157 deletions(-) delete mode 100644 kong/config_service.go delete mode 100644 kong/config_service_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6600dea87..a1693ab90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,9 +50,14 @@ > Release date: 2023/02/03 -- **Breaking change** to `ConfigService.ReloadDeclarativeRawConfig()`. Its - response signature is now `([]byte, error)`. The byte slice is the config - response body. The error is unchanged. +- **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. + [#273](https://github.com/Kong/go-kong/pull/273) ## [v0.36.0] diff --git a/kong/client.go b/kong/client.go index dbde0ab86..a94d9eccf 100644 --- a/kong/client.go +++ b/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) @@ -378,3 +376,45 @@ 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, +) ([]byte, error) { + type sendConfigParams struct { + CheckHash int `url:"check_hash"` + } + var checkHashI int + if checkHash { + checkHashI = 1 + } + req, err := c.NewRequest("POST", "/config", sendConfigParams{CheckHash: checkHashI}, config) + if err != nil { + return []byte{}, fmt.Errorf("creating new HTTP request for /config: %w", err) + } + + resp, err := c.DoRAW(ctx, req) + if err != nil { + return []byte{}, fmt.Errorf("failed posting new config to /config: %w", err) + } + defer resp.Body.Close() + + var b []byte + if resp.StatusCode < 200 || resp.StatusCode >= 400 { + b, err = io.ReadAll(resp.Body) + if err != nil { + return nil, + fmt.Errorf(`failed posting new config to /config: got status code %d + (and failed to read the response body): %w`, + resp.StatusCode, err) + } + } + + return b, nil +} diff --git a/kong/config_service.go b/kong/config_service.go deleted file mode 100644 index 5bc693165..000000000 --- a/kong/config_service.go +++ /dev/null @@ -1,62 +0,0 @@ -package kong - -import ( - "context" - "fmt" - "io" -) - -// AbstractConfigService handles Config in Kong. -type AbstractConfigService interface { - // 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. - ReloadDeclarativeRawConfig(ctx context.Context, config io.Reader, checkHash bool) ([]byte, error) -} - -// ConfigService handles Config in Kong. -type ConfigService service - -// 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 *ConfigService) ReloadDeclarativeRawConfig( - ctx context.Context, - config io.Reader, - checkHash bool, -) ([]byte, error) { - type sendConfigParams struct { - CheckHash int `url:"check_hash"` - } - var checkHashI int - if checkHash { - checkHashI = 1 - } - req, err := c.client.NewRequest("POST", "/config", sendConfigParams{CheckHash: checkHashI}, config) - if err != nil { - return []byte{}, fmt.Errorf("creating new HTTP request for /config: %w", err) - } - - resp, err := c.client.DoRAW(ctx, req) - if err != nil { - return []byte{}, fmt.Errorf("failed posting new config to /config: %w", err) - } - defer resp.Body.Close() - - var b []byte - if resp.StatusCode < 200 || resp.StatusCode >= 400 { - b, err = io.ReadAll(resp.Body) - if err != nil { - return nil, - fmt.Errorf(`failed posting new config to /config: got status code %d - (and failed to read the response body): %w`, - resp.StatusCode, err) - } - } - - return b, nil -} diff --git a/kong/config_service_test.go b/kong/config_service_test.go deleted file mode 100644 index 37c29659a..000000000 --- a/kong/config_service_test.go +++ /dev/null @@ -1,90 +0,0 @@ -package kong - -import ( - "bytes" - "context" - "encoding/json" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestConfigService(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, - }, - } - - 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.Configs.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true) - if (err != nil) != tt.wantErr { - t.Errorf("Client.SendConfig() error = %v, wantErr %v", err, tt.wantErr) - } - // 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") - }) - } -} From 6a88d74095232a710a6c08b7bc67aeab668e87e8 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 14:33:01 -0800 Subject: [PATCH 09/17] pr: convert more empty slices to nils --- kong/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kong/client.go b/kong/client.go index a94d9eccf..8dfdb425f 100644 --- a/kong/client.go +++ b/kong/client.go @@ -396,12 +396,12 @@ func (c *Client) ReloadDeclarativeRawConfig( } req, err := c.NewRequest("POST", "/config", sendConfigParams{CheckHash: checkHashI}, config) if err != nil { - return []byte{}, fmt.Errorf("creating new HTTP request for /config: %w", err) + return nil, fmt.Errorf("creating new HTTP request for /config: %w", err) } resp, err := c.DoRAW(ctx, req) if err != nil { - return []byte{}, fmt.Errorf("failed posting new config to /config: %w", err) + return nil, fmt.Errorf("failed posting new config to /config: %w", err) } defer resp.Body.Close() From f7b64c0cc33fd92646133257ac035075091f62a5 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 17:58:07 -0800 Subject: [PATCH 10/17] pr: re-add ReloadDeclarativeRawConfig test --- kong/client_test.go | 86 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/kong/client_test.go b/kong/client_test.go index e10717902..5aebf0d6f 100644 --- a/kong/client_test.go +++ b/kong/client_test.go @@ -1,7 +1,9 @@ package kong import ( + "bytes" "context" + "encoding/json" "io" "net/http" "os" @@ -209,3 +211,87 @@ 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, + }, + } + + 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) + stringBody := string(body) + if stringBody == "" { + t.Errorf("wat") + } + if (err != nil) != tt.wantErr { + t.Errorf("Client.SendConfig() error = %v, wantErr %v", err, tt.wantErr) + } + // 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") + }) + } +} From ab5b18a9aa926a850b54412951dcea34398c6ca6 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 17:58:15 -0800 Subject: [PATCH 11/17] fix: rework ReloadDeclarativeRawConfig error path Now that ReloadDeclarativeRawConfig returns config bodies always, the original error checking path didn't quite make sense. This extracts the body read and error check from the status error check and removes the response body from the status error. --- kong/client.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/kong/client.go b/kong/client.go index 8dfdb425f..5c9cc15b2 100644 --- a/kong/client.go +++ b/kong/client.go @@ -405,15 +405,12 @@ func (c *Client) ReloadDeclarativeRawConfig( } defer resp.Body.Close() - var b []byte + 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 { - b, err = io.ReadAll(resp.Body) - if err != nil { - return nil, - fmt.Errorf(`failed posting new config to /config: got status code %d - (and failed to read the response body): %w`, - resp.StatusCode, err) - } + return b, fmt.Errorf("failed posting new config to /config: got status code %d", resp.StatusCode) } return b, nil From a61b0d55a40fd6449185268437cf9172224a4bda Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 2 Feb 2023 18:37:36 -0800 Subject: [PATCH 12/17] feat: add flat error support to config update --- CHANGELOG.md | 4 ++++ kong/client.go | 15 +++++++++++++-- kong/client_test.go | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1693ab90..e85a97dbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ `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] diff --git a/kong/client.go b/kong/client.go index 5c9cc15b2..62a6a14dd 100644 --- a/kong/client.go +++ b/kong/client.go @@ -386,15 +386,26 @@ func (c *Client) ReloadDeclarativeRawConfig( ctx context.Context, config io.Reader, checkHash bool, + flattenErrors bool, ) ([]byte, error) { type sendConfigParams struct { - CheckHash int `url:"check_hash"` + CheckHash int `url:"check_hash"` + FlattenErrors int `url:"flatten_errors"` } var checkHashI int if checkHash { checkHashI = 1 } - req, err := c.NewRequest("POST", "/config", sendConfigParams{CheckHash: checkHashI}, config) + 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) } diff --git a/kong/client_test.go b/kong/client_test.go index 5aebf0d6f..8b60d003c 100644 --- a/kong/client_test.go +++ b/kong/client_test.go @@ -279,7 +279,7 @@ func TestReloadDeclarativeRawConfig(t *testing.T) { b, err := json.Marshal(tt.config) require.NoError(t, err) - body, err := client.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true) + body, err := client.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true, true) stringBody := string(body) if stringBody == "" { t.Errorf("wat") From 74cdcdad9ad6f6081f1cbd53a2a114fc2d683808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Fri, 3 Feb 2023 10:08:37 +0100 Subject: [PATCH 13/17] tests: more explicit asserts in TestReloadDeclarativeRawConfig --- kong/client.go | 2 +- kong/client_test.go | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/kong/client.go b/kong/client.go index 62a6a14dd..c19bae539 100644 --- a/kong/client.go +++ b/kong/client.go @@ -421,7 +421,7 @@ func (c *Client) ReloadDeclarativeRawConfig( 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", resp.StatusCode) + return b, fmt.Errorf("failed posting new config to /config: got status code %d and body %s", resp.StatusCode, b) } return b, nil diff --git a/kong/client_test.go b/kong/client_test.go index 8b60d003c..5601766d4 100644 --- a/kong/client_test.go +++ b/kong/client_test.go @@ -280,13 +280,16 @@ func TestReloadDeclarativeRawConfig(t *testing.T) { require.NoError(t, err) body, err := client.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true, true) - stringBody := string(body) - if stringBody == "" { - t.Errorf("wat") - } - if (err != nil) != tt.wantErr { - t.Errorf("Client.SendConfig() error = %v, wantErr %v", err, tt.wantErr) + // We only get empty body when there's a transient network error or + // we fail to read the response body which shouldn't happen in tests. + assert.NotEmpty(t, string(body)) + + if tt.wantErr { + assert.Errorf(t, err, "Client.SendConfig() got unexpected error = %v", err) + } 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 From 348dd0881afdcf75753dfefde955c3749e3f0e40 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Fri, 3 Feb 2023 09:36:53 -0800 Subject: [PATCH 14/17] fix: only flatten errors on 3.2+ --- kong/client_test.go | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/kong/client_test.go b/kong/client_test.go index 5601766d4..8d96ae808 100644 --- a/kong/client_test.go +++ b/kong/client_test.go @@ -268,6 +268,39 @@ func TestReloadDeclarativeRawConfig(t *testing.T) { }, } + // 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) + if err != nil { + t.Error(err) + } + info, err := client.Root(defaultCtx) + if err != nil { + t.Error(err) + } + version := VersionFromInfo(info) + currentVersion, err := ParseSemanticVersion(version) + if err != nil { + t.Error(err) + } + r, err := NewRange(">=3.2.0") + if err != nil { + t.Error(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) @@ -279,7 +312,7 @@ func TestReloadDeclarativeRawConfig(t *testing.T) { b, err := json.Marshal(tt.config) require.NoError(t, err) - body, err := client.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true, true) + body, err := client.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true, flattenErrors) // We only get empty body when there's a transient network error or // we fail to read the response body which shouldn't happen in tests. assert.NotEmpty(t, string(body)) From 74687195ef98202514ae773074e87df1f50ca62c Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Fri, 3 Feb 2023 10:55:39 -0800 Subject: [PATCH 15/17] fix: omit config params if unset If CheckHash or FlattenErrors are not set (i.e. zero value false) when calling ReloadDeclarativeRawConfig, omit them from the query string. Kong versions that do not recognize these parameters interpret them as part of the config and reject the config as invalid due to unrecognized fields: https://github.com/Kong/go-kong/pull/271#issuecomment-1416212852 --- kong/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kong/client.go b/kong/client.go index c19bae539..38c713126 100644 --- a/kong/client.go +++ b/kong/client.go @@ -389,8 +389,8 @@ func (c *Client) ReloadDeclarativeRawConfig( flattenErrors bool, ) ([]byte, error) { type sendConfigParams struct { - CheckHash int `url:"check_hash"` - FlattenErrors int `url:"flatten_errors"` + CheckHash int `url:"check_hash,omitempty"` + FlattenErrors int `url:"flatten_errors,omitempty"` } var checkHashI int if checkHash { From d9bef6d678c77bd3ed6f0c31da55b2e10246d761 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Fri, 3 Feb 2023 12:41:06 -0800 Subject: [PATCH 16/17] pr: clean up error checks --- kong/client_test.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/kong/client_test.go b/kong/client_test.go index 8b6050426..40fb245d7 100644 --- a/kong/client_test.go +++ b/kong/client_test.go @@ -274,22 +274,15 @@ func TestReloadDeclarativeRawConfig(t *testing.T) { // 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) - if err != nil { - t.Error(err) - } + require.NoError(t, err) info, err := client.Root(defaultCtx) - if err != nil { - t.Error(err) - } + require.NoError(t, err) version := VersionFromInfo(info) currentVersion, err := ParseSemanticVersion(version) - if err != nil { - t.Error(err) - } + require.NoError(t, err) r, err := NewRange(">=3.2.0") - if err != nil { - t.Error(err) - } + require.NoError(t, err) + if r(currentVersion) { t.Log("Kong version >=3.2, enabling flattenErrors for ReloadDeclarativeRawConfig") flattenErrors = true From 5b2500a6137bb79999f44a08d51c55cdfe8f17c0 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Fri, 3 Feb 2023 13:24:11 -0800 Subject: [PATCH 17/17] pr: more error cleanup --- kong/client_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/kong/client_test.go b/kong/client_test.go index 40fb245d7..658d979d7 100644 --- a/kong/client_test.go +++ b/kong/client_test.go @@ -302,12 +302,9 @@ func TestReloadDeclarativeRawConfig(t *testing.T) { require.NoError(t, err) body, err := client.ReloadDeclarativeRawConfig(ctx, bytes.NewBuffer(b), true, flattenErrors) - // We only get empty body when there's a transient network error or - // we fail to read the response body which shouldn't happen in tests. - assert.NotEmpty(t, string(body)) if tt.wantErr { - assert.Errorf(t, err, "Client.SendConfig() got unexpected error = %v", err) + assert.Errorf(t, err, "Client.SendConfig() got unexpected error") } else { assert.NoError(t, err) }