From c8aa6c3ce3762f6215714ab6ce75df41b1f20e80 Mon Sep 17 00:00:00 2001 From: Sam Heilbron Date: Mon, 8 Apr 2024 18:05:46 -0600 Subject: [PATCH] utils: improve envoyutils and curl requestutils (#9335) * Update cmdutils * add envoyutils * add curl_request util * Move curl request utils to standard location, rely on functional arguments * expand usage of admin client, and use it in envoy service * improve name of envoy client constant * expand tests for curl request * fix envoyInstance: getConfigDump, mark it deprecated * add changelog * cleanup client tests, simplify api * remove unnecesary context * fix test * add retry logic to kube2e curl helper * cleanup envoyutils * add test flake issue reference in changelong * improve api for kubectl.Cli * attempt to fix setup_syncer test flake * add FlakeAttempts decorator to staged_transformation test * use testHelper as source of truth * include longer interval in eventually * reduce retries in test that are known to fail * include query parametres * PR feedback: improvements to api, documentation * expand envoy client behavior * fix curl request method * fix test assertion --------- Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> --- .../v1.17.0-beta17/gg-envoy-admin-cli.yaml | 26 +++ pkg/utils/cmdutils/local.go | 11 +- pkg/utils/cmdutils/run_error.go | 16 ++ pkg/utils/envoyutils/admincli/README.md | 19 ++ .../admincli/admincli_suite_test.go | 13 ++ pkg/utils/envoyutils/admincli/client.go | 184 +++++++++++++++ pkg/utils/envoyutils/admincli/client_test.go | 81 +++++++ pkg/utils/kubeutils/kubectl/cli.go | 28 ++- pkg/utils/protoutils/utils.go | 10 + .../requestutils/curl/curl_suite_test.go | 13 ++ pkg/utils/requestutils/curl/option.go | 175 ++++++++++++++ pkg/utils/requestutils/curl/request.go | 140 ++++++++++++ pkg/utils/requestutils/curl/request_test.go | 55 +++++ projects/gloo/cli/pkg/cmd/install/knative.go | 9 +- .../gloo/cli/pkg/cmd/install/uninstall.go | 2 +- projects/gloo/pkg/defaults/port.go | 4 +- test/e2e/happypath_test.go | 12 +- test/e2e/staged_transformation_test.go | 4 +- test/helpers/kube_dump.go | 2 +- test/kube2e/gateway/gateway_suite_test.go | 2 +- test/kube2e/gateway/gateway_test.go | 7 + test/kube2e/gateway/robustness_test.go | 14 +- test/kube2e/gloo/gloo_suite_test.go | 6 + test/kube2e/gloo/setup_syncer_test.go | 70 +++--- test/kube2e/helper/curl.go | 64 ++++-- test/services/envoy/instance.go | 96 ++++---- test/testutils/curl_request.go | 215 ------------------ 27 files changed, 912 insertions(+), 366 deletions(-) create mode 100644 changelog/v1.17.0-beta17/gg-envoy-admin-cli.yaml create mode 100644 pkg/utils/envoyutils/admincli/README.md create mode 100644 pkg/utils/envoyutils/admincli/admincli_suite_test.go create mode 100644 pkg/utils/envoyutils/admincli/client.go create mode 100644 pkg/utils/envoyutils/admincli/client_test.go create mode 100644 pkg/utils/requestutils/curl/curl_suite_test.go create mode 100644 pkg/utils/requestutils/curl/option.go create mode 100644 pkg/utils/requestutils/curl/request.go create mode 100644 pkg/utils/requestutils/curl/request_test.go delete mode 100644 test/testutils/curl_request.go diff --git a/changelog/v1.17.0-beta17/gg-envoy-admin-cli.yaml b/changelog/v1.17.0-beta17/gg-envoy-admin-cli.yaml new file mode 100644 index 00000000000..07842814c46 --- /dev/null +++ b/changelog/v1.17.0-beta17/gg-envoy-admin-cli.yaml @@ -0,0 +1,26 @@ +changelog: + - type: NON_USER_FACING + issueLink: https://github.com/solo-io/solo-projects/issues/5723 + resolvesIssue: false + description: >- + Introduce a client for the Envoy Admin APi, improve how curl requests are built. + - type: NON_USER_FACING + issueLink: https://github.com/solo-io/gloo/issues/9291 + resolvesIssue: false + description: >- + Introduce retries to the kube2e curl helper, so that curl requests which may fail due to + network issues, will not cause tests to fail. This has demonstrated to reduce flakes, + though it has not proven that it will fix this test flake completely, so it is not + marked as resolving the issue. + - type: NON_USER_FACING + issueLink: https://github.com/solo-io/gloo/issues/9306 + resolvesIssue: false + description: >- + Attempt to resolve a test flake which occurs when a port-forward command fails. + The proposed solution is to rely on a new port forwarding utility, which includes + retries in the request, by default. + - type: NON_USER_FACING + issueLink: https://github.com/solo-io/gloo/issues/9292 + resolvesIssue: false + description: >- + Add a FlakeAttempts decorator, to try to reduce the impact of the staged_transformation test flakes \ No newline at end of file diff --git a/pkg/utils/cmdutils/local.go b/pkg/utils/cmdutils/local.go index bb00cdb6d30..1a72c5b52f8 100644 --- a/pkg/utils/cmdutils/local.go +++ b/pkg/utils/cmdutils/local.go @@ -3,6 +3,7 @@ package cmdutils import ( "context" "io" + "os" "os/exec" "strings" @@ -24,14 +25,18 @@ func Command(ctx context.Context, command string, args ...string) Cmd { // LocalCmder is a factory for LocalCmd, implementing Cmder type LocalCmder struct{} -// Command is like Command but includes a context +// Command returns a Cmd which includes the running process's `Environment` func (c *LocalCmder) Command(ctx context.Context, name string, arg ...string) Cmd { - return &LocalCmd{ + cmd := &LocalCmd{ Cmd: exec.CommandContext(ctx, name, arg...), } + + // By default, assign the env variables for the command + // Consumers of this Cmd can then override it, if they want + return cmd.WithEnv(os.Environ()...) } -// LocalCmd wraps os/exec.Cmd, implementing the kind/pkg/exec.Cmd interface +// LocalCmd wraps os/exec.Cmd, implementing the cmdutils.Cmd interface type LocalCmd struct { *exec.Cmd } diff --git a/pkg/utils/cmdutils/run_error.go b/pkg/utils/cmdutils/run_error.go index 83cbb6e3a3f..2944e9c4b66 100644 --- a/pkg/utils/cmdutils/run_error.go +++ b/pkg/utils/cmdutils/run_error.go @@ -17,12 +17,28 @@ type RunError struct { var _ error = &RunError{} func (e *RunError) Error() string { + if e == nil { + return "" + } return fmt.Sprintf("command \"%s\" failed with error: %v", e.PrettyCommand(), e.inner) } // PrettyCommand pretty prints the command in a way that could be pasted // into a shell func (e *RunError) PrettyCommand() string { + if e == nil { + return "RunError is nil" + } + + if len(e.command) == 0 { + return "no command args" + } + + if len(e.command) == 1 { + return e.command[0] + } + + // The above cases should not happen, but we defend against it return PrettyCommand(e.command[0], e.command[1:]...) } diff --git a/pkg/utils/envoyutils/admincli/README.md b/pkg/utils/envoyutils/admincli/README.md new file mode 100644 index 00000000000..7858ecb2857 --- /dev/null +++ b/pkg/utils/envoyutils/admincli/README.md @@ -0,0 +1,19 @@ +# Admincli + +> **Warning** +> This code is not intended to be used within the Control Plane. + +## Client +This is the Go client that should be used whenever communicating with the Envoy Admin API. Within the Gloo project, it is used inside of tests and our CLI. + +### Philosophy +We expose methods that return a [Command](/pkg/utils/cmdutils/cmd.go) which can be run by the calling code. Any methods that fit this structure, should end in `Cmd`: +```go +func StatsCmd(ctx context.Context) cmdutils.Cmd {} +``` + +There are also methods that the client exposes which are [syntactic sugar](https://en.wikipedia.org/wiki/Syntactic_sugar) on top of this command API. These methods tend to follow the naming convention: `GetX`: +```go +func GetStats(ctx context.Context) (string, error) {} +``` +_As a general practice, these methods should return a concrete type, whenever possible._ \ No newline at end of file diff --git a/pkg/utils/envoyutils/admincli/admincli_suite_test.go b/pkg/utils/envoyutils/admincli/admincli_suite_test.go new file mode 100644 index 00000000000..b5ebfb0a400 --- /dev/null +++ b/pkg/utils/envoyutils/admincli/admincli_suite_test.go @@ -0,0 +1,13 @@ +package admincli_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestAdminCli(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "AdminCli Suite") +} diff --git a/pkg/utils/envoyutils/admincli/client.go b/pkg/utils/envoyutils/admincli/client.go new file mode 100644 index 00000000000..90a1c4c88ce --- /dev/null +++ b/pkg/utils/envoyutils/admincli/client.go @@ -0,0 +1,184 @@ +package admincli + +import ( + "context" + "fmt" + "io" + "net/http" + + adminv3 "github.com/envoyproxy/go-control-plane/envoy/admin/v3" + "github.com/solo-io/gloo/pkg/utils/cmdutils" + "github.com/solo-io/gloo/pkg/utils/protoutils" + "github.com/solo-io/gloo/pkg/utils/requestutils/curl" + "github.com/solo-io/go-utils/threadsafe" +) + +const ( + ConfigDumpPath = "config_dump" + StatsPath = "stats" + ClustersPath = "clusters" + ListenersPath = "listeners" + ModifyRuntimePath = "runtime_modify" + ShutdownServerPath = "quitquitquit" + HealthCheckPath = "healthcheck" + LoggingPath = "logging" + + DefaultAdminPort = 19000 +) + +// Client is a utility for executing requests against the Envoy Admin API +// The Admin API handlers can be found here: +// https://github.com/envoyproxy/envoy/blob/63bc9b564b1a76a22a0d029bcac35abeffff2a61/source/server/admin/admin.cc#L127 +type Client struct { + // receiver is the default destination for the curl stdout and stderr + receiver io.Writer + + // curlOptions is the set of default Option that the Client will use for curl commands + curlOptions []curl.Option +} + +// NewClient returns an implementation of the admincli.Client +func NewClient() *Client { + return &Client{ + receiver: io.Discard, + curlOptions: []curl.Option{ + curl.WithScheme("http"), + curl.WithHost("127.0.0.1"), + curl.WithPort(DefaultAdminPort), + // 3 retries, exponential back-off, 10 second max + curl.WithRetries(3, 0, 10), + }, + } +} + +// WithReceiver sets the io.Writer that will be used by default for the stdout and stderr +// of cmdutils.Cmd created by the Client +func (c *Client) WithReceiver(receiver io.Writer) *Client { + c.receiver = receiver + return c +} + +// WithCurlOptions sets the default set of curl.Option that will be used by default with +// the cmdutils.Cmd created by the Client +func (c *Client) WithCurlOptions(options ...curl.Option) *Client { + c.curlOptions = append(c.curlOptions, options...) + return c +} + +// Command returns a curl Command, using the provided curl.Option as well as the client.curlOptions +func (c *Client) Command(ctx context.Context, options ...curl.Option) cmdutils.Cmd { + commandCurlOptions := append( + c.curlOptions, + // Ensure any options defined for this command can override any defaults that the Client has defined + options...) + curlArgs := curl.BuildArgs(commandCurlOptions...) + + return cmdutils.Command(ctx, "curl", curlArgs...). + // For convenience, we set the stdout and stderr to the receiver + // This can still be overwritten by consumers who use the commands + WithStdout(c.receiver). + WithStderr(c.receiver) +} + +// RunCommand executes a curl Command, using the provided curl.Option as well as the client.curlOptions +func (c *Client) RunCommand(ctx context.Context, options ...curl.Option) error { + return c.Command(ctx, options...).Run().Cause() +} + +// RequestPathCmd returns the cmdutils.Cmd that can be run, and will execute a request against the provided path +func (c *Client) RequestPathCmd(ctx context.Context, path string) cmdutils.Cmd { + return c.Command(ctx, curl.WithPath(path)) +} + +// StatsCmd returns the cmdutils.Cmd that can be run to request data from the stats endpoint +func (c *Client) StatsCmd(ctx context.Context) cmdutils.Cmd { + return c.RequestPathCmd(ctx, StatsPath) +} + +// GetStats returns the data that is available at the stats endpoint +func (c *Client) GetStats(ctx context.Context) (string, error) { + var outLocation threadsafe.Buffer + + err := c.StatsCmd(ctx).WithStdout(&outLocation).Run().Cause() + if err != nil { + return "", err + } + + return outLocation.String(), nil +} + +// ClustersCmd returns the cmdutils.Cmd that can be run to request data from the clusters endpoint +func (c *Client) ClustersCmd(ctx context.Context) cmdutils.Cmd { + return c.RequestPathCmd(ctx, ClustersPath) +} + +// ListenersCmd returns the cmdutils.Cmd that can be run to request data from the listeners endpoint +func (c *Client) ListenersCmd(ctx context.Context) cmdutils.Cmd { + return c.RequestPathCmd(ctx, ListenersPath) +} + +// ConfigDumpCmd returns the cmdutils.Cmd that can be run to request data from the config_dump endpoint +func (c *Client) ConfigDumpCmd(ctx context.Context) cmdutils.Cmd { + return c.RequestPathCmd(ctx, ConfigDumpPath) +} + +// GetConfigDump returns the structured data that is available at the config_dump endpoint +func (c *Client) GetConfigDump(ctx context.Context) (*adminv3.ConfigDump, error) { + var ( + cfgDump adminv3.ConfigDump + outLocation threadsafe.Buffer + ) + + err := c.ConfigDumpCmd(ctx).WithStdout(&outLocation).Run().Cause() + if err != nil { + return nil, err + } + + // Ever since upgrading the go-control-plane to v0.10.1 the standard unmarshal fails with the following error: + // unknown field \"hidden_envoy_deprecated_build_version\" in envoy.config.core.v3.Node" + // To get around this, we rely on an unmarshaler with AllowUnknownFields set to true + if err = protoutils.UnmarshalAllowUnknown(&outLocation, &cfgDump); err != nil { + return nil, err + } + + return &cfgDump, nil +} + +// ModifyRuntimeConfiguration passes the queryParameters to the runtime_modify endpoint +func (c *Client) ModifyRuntimeConfiguration(ctx context.Context, queryParameters map[string]string) error { + return c.RunCommand(ctx, + curl.WithPath(ModifyRuntimePath), + curl.WithQueryParameters(queryParameters), + curl.WithMethod(http.MethodPost)) +} + +// ShutdownServer calls the shutdown server endpoint +func (c *Client) ShutdownServer(ctx context.Context) error { + return c.RunCommand(ctx, + curl.WithPath(ShutdownServerPath), + curl.WithMethod(http.MethodPost)) +} + +// FailHealthCheck calls the endpoint to have the server start failing health checks +func (c *Client) FailHealthCheck(ctx context.Context) error { + return c.RunCommand(ctx, + curl.WithPath(fmt.Sprintf("%s/fail", HealthCheckPath)), + curl.WithMethod(http.MethodPost)) +} + +// PassHealthCheck calls the endpoint to have the server start passing health checks +func (c *Client) PassHealthCheck(ctx context.Context) error { + return c.RunCommand(ctx, + curl.WithPath(fmt.Sprintf("%s/ok", HealthCheckPath)), + curl.WithMethod(http.MethodPost)) +} + +// SetLogLevel calls the endpoint to change the log level for the server +func (c *Client) SetLogLevel(ctx context.Context, logLevel string) error { + return c.RunCommand(ctx, + curl.WithPath(LoggingPath), + curl.WithQueryParameters(map[string]string{ + "level": logLevel, + }), + curl.WithMethod(http.MethodPost)) +} diff --git a/pkg/utils/envoyutils/admincli/client_test.go b/pkg/utils/envoyutils/admincli/client_test.go new file mode 100644 index 00000000000..f89bf4b72c0 --- /dev/null +++ b/pkg/utils/envoyutils/admincli/client_test.go @@ -0,0 +1,81 @@ +package admincli_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/solo-io/gloo/pkg/utils/envoyutils/admincli" + "github.com/solo-io/gloo/pkg/utils/requestutils/curl" + "github.com/solo-io/go-utils/threadsafe" +) + +var _ = Describe("Client", func() { + + var ( + ctx context.Context + ) + + BeforeEach(func() { + ctx = context.Background() + }) + + Context("Client tests", func() { + + It("WithCurlOptions can append and override default curl.Option", func() { + client := admincli.NewClient().WithCurlOptions( + curl.WithRetries(1, 1, 1), // override + curl.Silent(), // new value + ) + + curlCommand := client.Command(ctx).Run().PrettyCommand() + Expect(curlCommand).To(And( + ContainSubstring("\"--retry\" \"1\""), + ContainSubstring("\"--retry-delay\" \"1\""), + ContainSubstring("\"--retry-max-time\" \"1\""), + ContainSubstring(" \"-s\""), + )) + }) + + }) + + Context("Integration tests", func() { + + When("Admin API is reachable", func() { + // We do not YET write additional integration tests for when the Admin API is reachable + // This utility is used in our test/services/envoy.Instance, which is the core service + // for our in-memory e2e (test/e2e) tests. + // todo: we should introduce integration tests to validate this behavior + }) + + When("Admin API is not reachable", func() { + + It("emits an error to configured locations", func() { + var ( + defaultOutputLocation, errLocation, outLocation threadsafe.Buffer + ) + + // Create a client that points to an address where Envoy is NOT running + client := admincli.NewClient(). + WithReceiver(&defaultOutputLocation). + WithCurlOptions( + curl.WithScheme("http"), + curl.WithHost("127.0.0.1"), + curl.WithPort(1111), + // Since we expect this test to fail, we don't need to use all the reties that the client defaults to use + curl.WithoutRetries(), + ) + + statsCmd := client.StatsCmd(ctx). + WithStdout(&outLocation). + WithStderr(&errLocation) + + err := statsCmd.Run().Cause() + Expect(err).To(HaveOccurred(), "running the command should return an error") + Expect(defaultOutputLocation.Bytes()).To(BeEmpty(), "defaultOutputLocation should not be used") + Expect(outLocation.Bytes()).To(BeEmpty(), "failed request should not output to Stdout") + Expect(string(errLocation.Bytes())).To(ContainSubstring("Failed to connect to 127.0.0.1 port 1111"), "failed request should output to Stderr") + }) + }) + }) +}) diff --git a/pkg/utils/kubeutils/kubectl/cli.go b/pkg/utils/kubeutils/kubectl/cli.go index e474730ac21..0c720cc9d95 100644 --- a/pkg/utils/kubeutils/kubectl/cli.go +++ b/pkg/utils/kubeutils/kubectl/cli.go @@ -7,7 +7,6 @@ import ( "github.com/solo-io/gloo/pkg/utils/cmdutils" "io" - "os" "time" "github.com/avast/retry-go/v4" @@ -25,13 +24,21 @@ type Cli struct { } // NewCli returns an implementation of the kubectl.Cli -func NewCli(receiver io.Writer) *Cli { +func NewCli() *Cli { return &Cli{ - receiver: receiver, + receiver: io.Discard, kubeContext: "", } } +// WithReceiver sets the io.Writer that will be used by default for the stdout and stderr +// of cmdutils.Cmd created by the Cli +func (c *Cli) WithReceiver(receiver io.Writer) *Cli { + c.receiver = receiver + return c +} + +// WithKubeContext sets the --context for the kubectl command invoked by the Cli func (c *Cli) WithKubeContext(kubeContext string) *Cli { c.kubeContext = kubeContext return c @@ -42,17 +49,14 @@ func (c *Cli) Command(ctx context.Context, args ...string) cmdutils.Cmd { args = append([]string{"--context", c.kubeContext}, args...) } - cmd := cmdutils.Command(ctx, "kubectl", args...) - cmd.WithEnv(os.Environ()...) - - // For convenience, we set the stdout and stderr to the receiver - // This can still be overwritten by consumers who use the commands - cmd.WithStdout(c.receiver) - cmd.WithStderr(c.receiver) - return cmd + return cmdutils.Command(ctx, "kubectl", args...). + // For convenience, we set the stdout and stderr to the receiver + // This can still be overwritten by consumers who use the commands + WithStdout(c.receiver). + WithStderr(c.receiver) } -func (c *Cli) ExecuteCommand(ctx context.Context, args ...string) error { +func (c *Cli) RunCommand(ctx context.Context, args ...string) error { return c.Command(ctx, args...).Run().Cause() } diff --git a/pkg/utils/protoutils/utils.go b/pkg/utils/protoutils/utils.go index 61f0b405e50..a3399b6ced5 100644 --- a/pkg/utils/protoutils/utils.go +++ b/pkg/utils/protoutils/utils.go @@ -2,6 +2,7 @@ package protoutils import ( "bytes" + "io" "github.com/ghodss/yaml" "github.com/golang/protobuf/jsonpb" @@ -14,6 +15,7 @@ var ( jsonpbMarshaler = &jsonpb.Marshaler{OrigName: false} jsonpbMarshalerEmitZeroValues = &jsonpb.Marshaler{OrigName: false, EmitDefaults: true} jsonpbMarshalerIndented = &jsonpb.Marshaler{OrigName: false, Indent: " "} + jsonpbUnmarshalerAllowUnknown = &jsonpb.Unmarshaler{AllowUnknownFields: true} NilStructError = eris.New("cannot unmarshal nil struct") ) @@ -62,6 +64,14 @@ func UnmarshalBytes(data []byte, into proto.Message) error { return jsonpb.Unmarshal(bytes.NewBuffer(data), into) } +func UnmarshalBytesAllowUnknown(data []byte, into proto.Message) error { + return UnmarshalAllowUnknown(bytes.NewBuffer(data), into) +} + +func UnmarshalAllowUnknown(r io.Reader, into proto.Message) error { + return jsonpbUnmarshalerAllowUnknown.Unmarshal(r, into) +} + func UnmarshalYaml(data []byte, into proto.Message) error { jsn, err := yaml.YAMLToJSON(data) if err != nil { diff --git a/pkg/utils/requestutils/curl/curl_suite_test.go b/pkg/utils/requestutils/curl/curl_suite_test.go new file mode 100644 index 00000000000..6caac804c5b --- /dev/null +++ b/pkg/utils/requestutils/curl/curl_suite_test.go @@ -0,0 +1,13 @@ +package curl_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCurl(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Curl Suite") +} diff --git a/pkg/utils/requestutils/curl/option.go b/pkg/utils/requestutils/curl/option.go new file mode 100644 index 00000000000..6847cebec14 --- /dev/null +++ b/pkg/utils/requestutils/curl/option.go @@ -0,0 +1,175 @@ +package curl + +import ( + "net/http" + "strings" +) + +// Option represents an option for a curl request. +type Option func(config *requestConfig) + +// VerboseOutput returns the Option to emit a verbose output for the curl request +// https://curl.se/docs/manpage.html#-v +func VerboseOutput() Option { + return func(config *requestConfig) { + config.verbose = true + } +} + +// IgnoreServerCert returns the Option to ignore the server certificate in the curl request +// https://curl.se/docs/manpage.html#-k +func IgnoreServerCert() Option { + return func(config *requestConfig) { + config.ignoreServerCert = true + } +} + +// Silent returns the Option to enable silent mode for the curl request +// https://curl.se/docs/manpage.html#-s +func Silent() Option { + return func(config *requestConfig) { + config.silent = true + } +} + +// WithHeadersOnly returns the Option to only return headers with the curl response +// https://curl.se/docs/manpage.html#-I +func WithHeadersOnly() Option { + return func(config *requestConfig) { + config.headersOnly = true + } +} + +// WithConnectionTimeout returns the Option to set a connection timeout on the curl request +// https://curl.se/docs/manpage.html#--connect-timeout +// https://curl.se/docs/manpage.html#-m +func WithConnectionTimeout(seconds int) Option { + return func(config *requestConfig) { + config.connectionTimeout = seconds + } +} + +// WithMethod returns the Option to set the method for the curl request +// https://curl.se/docs/manpage.html#-X +func WithMethod(method string) Option { + return func(config *requestConfig) { + config.method = method + } +} + +// WithPort returns the Option to set the port for the curl request +func WithPort(port int) Option { + return func(config *requestConfig) { + config.port = port + } +} + +// WithHost returns the Option to set the host for the curl request +func WithHost(host string) Option { + return func(config *requestConfig) { + config.host = host + } +} + +// WithSni returns the Option to configure a custom address to connect to +// https://curl.se/docs/manpage.html#--resolve +func WithSni(sni string) Option { + return func(config *requestConfig) { + config.sni = sni + } +} + +// WithCaFile returns the Option to configure the certificate file used to verify the peer +// https://curl.se/docs/manpage.html#--cacert +func WithCaFile(caFile string) Option { + return func(config *requestConfig) { + config.caFile = caFile + } +} + +// WithPath returns the Option to configure the path of the curl request +// The provided path is expected to not contain a leading `/`, +// so if it is provided, it will be trimmed +func WithPath(path string) Option { + return func(config *requestConfig) { + config.path = strings.TrimPrefix(path, "/") + } +} + +// WithQueryParameters returns the Option to configure the query parameters of the curl request +func WithQueryParameters(parameters map[string]string) Option { + return func(config *requestConfig) { + config.queryParameters = parameters + } +} + +// WithRetries returns the Option to configure the retries for the curl request +func WithRetries(retry, retryDelay, retryMaxTime int) Option { + return func(config *requestConfig) { + config.retry = retry + config.retryDelay = retryDelay + config.retryMaxTime = retryMaxTime + } +} + +// WithoutRetries returns the Option to disable retries for the curl request +func WithoutRetries() Option { + return func(config *requestConfig) { + WithRetries(0, -1, 0)(config) + } +} + +// WithPostBody returns the Option to configure a curl request to execute a post request with the provided json body +func WithPostBody(body string) Option { + return func(config *requestConfig) { + WithMethod(http.MethodPost)(config) + WithBody(body)(config) + WithContentType("application/json")(config) + } +} + +// WithBody returns the Option to configure the body for a curl request +// https://curl.se/docs/manpage.html#-d +func WithBody(body string) Option { + return func(config *requestConfig) { + config.body = body + } +} + +// WithContentType returns the Option to configure the Content-Type header for the curl request +func WithContentType(contentType string) Option { + return func(config *requestConfig) { + WithHeader("Content-Type", contentType)(config) + } +} + +// WithHostHeader returns the Option to configure the Host header for the curl request +func WithHostHeader(host string) Option { + return func(config *requestConfig) { + WithHeader("Host", host)(config) + } +} + +// WithHeader returns the Option to configure a header for the curl request +// https://curl.se/docs/manpage.html#-H +func WithHeader(key, value string) Option { + return func(config *requestConfig) { + config.headers[key] = value + } +} + +// WithScheme returns the Option to configure the scheme for the curl request +func WithScheme(scheme string) Option { + return func(config *requestConfig) { + config.scheme = scheme + } +} + +// WithArgs allows developers to append arbitrary args to the curl request +// This should mainly be used for debugging purposes. If there is an argument that the current Option +// set doesn't yet support, it should be added explicitly, to make it easier for developers to utilize +func WithArgs(args []string) Option { + return func(config *requestConfig) { + config.additionalArgs = args + } +} diff --git a/pkg/utils/requestutils/curl/request.go b/pkg/utils/requestutils/curl/request.go new file mode 100644 index 00000000000..b3dc07239b3 --- /dev/null +++ b/pkg/utils/requestutils/curl/request.go @@ -0,0 +1,140 @@ +package curl + +import ( + "fmt" + "net/http" + "net/url" +) + +// BuildArgs accepts a set of curl.Option and generates the list of arguments +// that can be used to execute a curl request +// If multiple Option modify the same argument, the last defined one will win: +// +// Example: +// BuildArgs(WithMethod("GET"), WithMethod("POST")) +// will return a curl with using a post method +// +// A notable exception to this is the WithHeader option, which will always modify +// the map of headers used in the curl request. +func BuildArgs(options ...Option) []string { + config := &requestConfig{ + verbose: false, + ignoreServerCert: false, + silent: false, + connectionTimeout: 3, + headersOnly: false, + method: http.MethodGet, + host: "127.0.0.1", + port: 8080, + headers: make(map[string]string), + scheme: "http", // https://github.com/golang/go/issues/40587 + sni: "", + caFile: "", + path: "", + retry: 0, // do not retry + retryDelay: -1, + retryMaxTime: 0, + + additionalArgs: []string{}, + } + + for _, opt := range options { + opt(config) + } + + return config.generateArgs() +} + +// requestConfig contains the set of options that can be used to configure a curl request +type requestConfig struct { + verbose bool + ignoreServerCert bool + silent bool + connectionTimeout int // seconds + headersOnly bool + method string + host string + port int + headers map[string]string + body string + sni string + caFile string + path string + queryParameters map[string]string + + scheme string + + retry int + retryDelay int + retryMaxTime int + + additionalArgs []string +} + +func (c *requestConfig) generateArgs() []string { + var args []string + + if c.verbose { + args = append(args, "-v") + } + if c.ignoreServerCert { + args = append(args, "-k") + } + if c.silent { + args = append(args, "-s") + } + if c.connectionTimeout > 0 { + seconds := fmt.Sprintf("%v", c.connectionTimeout) + args = append(args, "--connect-timeout", seconds, "--max-time", seconds) + } + if c.headersOnly { + args = append(args, "-I") + } + + // We prefer to be explicit, and always set the method + args = append(args, "--request", c.method) + + for h, v := range c.headers { + args = append(args, "-H", fmt.Sprintf("%v: %v", h, v)) + } + if c.caFile != "" { + args = append(args, "--cacert", c.caFile) + } + if c.body != "" { + args = append(args, "-d", c.body) + } + if c.retry != 0 { + args = append(args, "--retry", fmt.Sprintf("%d", c.retry)) + } + if c.retryDelay != -1 { + args = append(args, "--retry-delay", fmt.Sprintf("%d", c.retryDelay)) + } + if c.retryMaxTime != 0 { + args = append(args, "--retry-max-time", fmt.Sprintf("%d", c.retryMaxTime)) + } + + if len(c.additionalArgs) > 0 { + args = append(args, c.additionalArgs...) + } + + // Todo: rely on url.Url to construct the address + var fullAddress string + + if c.sni != "" { + sniResolution := fmt.Sprintf("%s:%d:%s", c.sni, c.port, c.host) + fullAddress = fmt.Sprintf("%s://%s:%d", c.scheme, c.sni, c.port) + args = append(args, "--resolve", sniResolution) + } else { + fullAddress = fmt.Sprintf("%v://%s:%v/%s", c.scheme, c.host, c.port, c.path) + if len(c.queryParameters) > 0 { + values := url.Values{} + for k, v := range c.queryParameters { + values.Add(k, v) + } + fullAddress = fmt.Sprintf("%s?%s", fullAddress, values.Encode()) + } + } + + args = append(args, fullAddress) + return args +} diff --git a/pkg/utils/requestutils/curl/request_test.go b/pkg/utils/requestutils/curl/request_test.go new file mode 100644 index 00000000000..1b9e34719ce --- /dev/null +++ b/pkg/utils/requestutils/curl/request_test.go @@ -0,0 +1,55 @@ +package curl_test + +import ( + . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" + "github.com/solo-io/gloo/pkg/utils/requestutils/curl" + + . "github.com/onsi/ginkgo/v2" +) + +var _ = Describe("Curl", func() { + + Context("BuildArgs", func() { + + DescribeTable("it builds the args using the provided option", + func(option curl.Option, expectedMatcher types.GomegaMatcher) { + Expect(curl.BuildArgs(option)).To(expectedMatcher) + }, + Entry("VerboseOutput", + curl.VerboseOutput(), + ContainElement("-v"), + ), + Entry("IgnoreServerCert", + curl.IgnoreServerCert(), + ContainElement("-k"), + ), + Entry("Silent", + curl.Silent(), + ContainElement("-s"), + ), + Entry("WithHeadersOnly", + curl.WithHeadersOnly(), + ContainElement("-I"), + ), + Entry("WithCaFile", + curl.WithCaFile("caFile"), + ContainElement("--cacert"), + ), + Entry("WithBody", + curl.WithBody("body"), + ContainElement("-d"), + ), + Entry("WithRetries", + curl.WithRetries(1, 1, 1), + ContainElements("--retry", "--retry-delay", "--retry-max-time"), + ), + Entry("WithArgs", + curl.WithArgs([]string{"--custom-args"}), + ContainElement("--custom-args"), + ), + ) + + }) + +}) diff --git a/projects/gloo/cli/pkg/cmd/install/knative.go b/projects/gloo/cli/pkg/cmd/install/knative.go index af0d2afc333..72174a98f9b 100644 --- a/projects/gloo/cli/pkg/cmd/install/knative.go +++ b/projects/gloo/cli/pkg/cmd/install/knative.go @@ -12,11 +12,12 @@ import ( "strings" "time" + "github.com/solo-io/gloo/pkg/cliutil" + "github.com/solo-io/gloo/pkg/utils/kubeutils/kubectl" "github.com/avast/retry-go" "github.com/rotisserie/eris" - "github.com/solo-io/gloo/pkg/cliutil" "github.com/solo-io/go-utils/contextutils" "github.com/solo-io/k8s-utils/kubeutils" "github.com/spf13/cobra" @@ -154,7 +155,9 @@ func knativeCmd(opts *options.Options) *cobra.Command { func installKnativeServing(opts *options.Options) error { knativeOpts := opts.Install.Knative - kubeCli := kubectl.NewCli(cliutil.GetLogger()).WithKubeContext(opts.Top.KubeContext) + kubeCli := kubectl.NewCli(). + WithReceiver(cliutil.GetLogger()). + WithKubeContext(opts.Top.KubeContext) // store the opts as a label on the knative-serving namespace // we can use this to uninstall later on @@ -200,7 +203,7 @@ func installKnativeServing(opts *options.Options) error { } } // label the knative-serving namespace as belonging to us - if err := kubeCli.ExecuteCommand( + if err := kubeCli.RunCommand( opts.Top.Ctx, "annotate", "namespace", diff --git a/projects/gloo/cli/pkg/cmd/install/uninstall.go b/projects/gloo/cli/pkg/cmd/install/uninstall.go index e5d586f79ad..ab1459c1da2 100644 --- a/projects/gloo/cli/pkg/cmd/install/uninstall.go +++ b/projects/gloo/cli/pkg/cmd/install/uninstall.go @@ -206,7 +206,7 @@ func (u *uninstaller) uninstallKnativeIfNecessary(ctx context.Context) { _, _ = fmt.Fprintf(u.output, "Could not determine which knative components to remove. Continuing...\n") return } - if err := kubectl.NewCli(u.output).Delete(ctx, []byte(manifests), "--ignore-not-found"); err != nil { + if err := kubectl.NewCli().WithReceiver(u.output).Delete(ctx, []byte(manifests), "--ignore-not-found"); err != nil { _, _ = fmt.Fprintf(u.output, "Unable to delete knative. Continuing...\n") } } diff --git a/projects/gloo/pkg/defaults/port.go b/projects/gloo/pkg/defaults/port.go index c791ad33bdd..166a9ba781a 100644 --- a/projects/gloo/pkg/defaults/port.go +++ b/projects/gloo/pkg/defaults/port.go @@ -2,13 +2,15 @@ package defaults import ( "time" + + "github.com/solo-io/gloo/pkg/utils/envoyutils/admincli" ) const GlooRestXdsName = "rest_xds_cluster" var HttpPort uint32 = 8080 var HttpsPort uint32 = 8443 -var EnvoyAdminPort uint32 = 19000 +var EnvoyAdminPort uint32 = admincli.DefaultAdminPort var GlooAdminPort uint32 = 9091 var GlooProxyDebugPort = 9966 var GlooRestXdsPort = 9976 diff --git a/test/e2e/happypath_test.go b/test/e2e/happypath_test.go index 48597e00ea3..d5928b3114d 100644 --- a/test/e2e/happypath_test.go +++ b/test/e2e/happypath_test.go @@ -4,7 +4,6 @@ import ( "context" "crypto/tls" "fmt" - "io" "net" "net/http" "time" @@ -139,18 +138,11 @@ var _ = Describe("Happy path", func() { // This will hit the virtual host with the above virtual cluster config TestUpstreamReachable() - response, err := http.Get(fmt.Sprintf("http://localhost:%d/stats", envoyInstance.AdminPort)) + stats, err := envoyInstance.AdminClient().GetStats(ctx) Expect(err).NotTo(HaveOccurred()) - Expect(response).NotTo(BeNil()) - //goland:noinspection GoUnhandledErrorResult - defer response.Body.Close() - - body, err := io.ReadAll(response.Body) - Expect(err).NotTo(HaveOccurred()) - statsString := string(body) // Verify that stats for the above virtual cluster are present - Expect(statsString).To(ContainSubstring("vhost.virt1.vcluster.test-vc.")) + Expect(stats).To(ContainSubstring("vhost.virt1.vcluster.test-vc.")) }) It("it correctly passes the suppress envoy headers config", func() { diff --git a/test/e2e/staged_transformation_test.go b/test/e2e/staged_transformation_test.go index 78f59727b15..31cf4cf48ab 100644 --- a/test/e2e/staged_transformation_test.go +++ b/test/e2e/staged_transformation_test.go @@ -28,7 +28,9 @@ import ( "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/transformation" ) -var _ = Describe("Staged Transformation", func() { +var _ = Describe("Staged Transformation", FlakeAttempts(3), func() { + // We added the FlakeAttempts decorator to try to reduce the impact of the flakes outlined in: + // https://github.com/solo-io/gloo/issues/9292 var ( testContext *e2e.TestContext diff --git a/test/helpers/kube_dump.go b/test/helpers/kube_dump.go index 4c65b3fea80..b63ffe7aa22 100644 --- a/test/helpers/kube_dump.go +++ b/test/helpers/kube_dump.go @@ -214,7 +214,7 @@ func kubeGet(namespace string, kubeType string, name string) (string, error) { } func kubeExecute(args []string) (string, error) { - cli := kubectl.NewCli(ginkgo.GinkgoWriter) + cli := kubectl.NewCli().WithReceiver(ginkgo.GinkgoWriter) runError := cli.Command(context.Background(), args...).Run() return runError.OutputString(), runError.Cause() diff --git a/test/kube2e/gateway/gateway_suite_test.go b/test/kube2e/gateway/gateway_suite_test.go index 6ca32ae71b0..220dd87e72b 100644 --- a/test/kube2e/gateway/gateway_suite_test.go +++ b/test/kube2e/gateway/gateway_suite_test.go @@ -63,7 +63,7 @@ func StartTestHelper() { Expect(err).NotTo(HaveOccurred()) skhelpers.RegisterPreFailHandler(helpers.StandardGlooDumpOnFail(GinkgoWriter, testHelper.InstallNamespace)) - kubeCli = kubectl.NewCli(GinkgoWriter) + kubeCli = kubectl.NewCli().WithReceiver(GinkgoWriter) // Allow skipping of install step for running multiple times if !kubeutils2.ShouldSkipInstall() { diff --git a/test/kube2e/gateway/gateway_test.go b/test/kube2e/gateway/gateway_test.go index b4422695556..7390b051f8b 100644 --- a/test/kube2e/gateway/gateway_test.go +++ b/test/kube2e/gateway/gateway_test.go @@ -1679,6 +1679,13 @@ var _ = Describe("Kube2e: gateway", func() { Port: gatewayPort, ConnectionTimeout: 1, WithoutStats: true, + // These redOpts are used in a curl that is expected to consistently pass + // We rely on curl retries to prevent network flakes from causing test flakes + Retries: struct { + Retry int + RetryDelay int + RetryMaxTime int + }{Retry: 3, RetryDelay: 0, RetryMaxTime: 5}, } // try it 10 times diff --git a/test/kube2e/gateway/robustness_test.go b/test/kube2e/gateway/robustness_test.go index 7daf098d2b1..4217ce6c7c8 100644 --- a/test/kube2e/gateway/robustness_test.go +++ b/test/kube2e/gateway/robustness_test.go @@ -324,6 +324,16 @@ var _ = Describe("Robustness tests", func() { }, "15s", "0.5s").Should(BeTrue()) }) + // See helper.CurlOpts.Retries for explanation about why retries are useful + withBasicRetries := func(opts helper.CurlOpts) helper.CurlOpts { + opts.Retries = struct { + Retry int + RetryDelay int + RetryMaxTime int + }{Retry: 3, RetryDelay: 0, RetryMaxTime: 5} + return opts + } + firstRouteCurlOpts := func() helper.CurlOpts { return helper.CurlOpts{ Protocol: "http", @@ -366,7 +376,7 @@ var _ = Describe("Robustness tests", func() { return testHelper.Curl(firstRouteCurlOpts()) }, "30s", "1s").Should(ContainSubstring(validRouteResponse)) g.Consistently(func() (string, error) { - return testHelper.Curl(firstRouteCurlOpts()) + return testHelper.Curl(withBasicRetries(firstRouteCurlOpts())) }, "5s", "1s").Should(ContainSubstring(validRouteResponse)) // can no longer route to appName2 since its k8s service has been removed @@ -377,7 +387,7 @@ var _ = Describe("Robustness tests", func() { return testHelper.Curl(secondRouteCurlOpts()) }, "30s", "1s").Should(ContainSubstring(invalidRouteResponse)) g.Consistently(func() (string, error) { - return testHelper.Curl(secondRouteCurlOpts()) + return testHelper.Curl(withBasicRetries(secondRouteCurlOpts())) }, "5s", "1s").Should(ContainSubstring(invalidRouteResponse)) }, "30s").Should(Succeed()) diff --git a/test/kube2e/gloo/gloo_suite_test.go b/test/kube2e/gloo/gloo_suite_test.go index d6f23782d74..06cb8fafeb0 100644 --- a/test/kube2e/gloo/gloo_suite_test.go +++ b/test/kube2e/gloo/gloo_suite_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "github.com/solo-io/gloo/pkg/utils/kubeutils/kubectl" + "github.com/avast/retry-go" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -49,6 +51,8 @@ var ( envoyFactory envoy.Factory vaultFactory *services.VaultFactory + + kubeCli *kubectl.Cli ) var _ = BeforeSuite(func() { @@ -61,6 +65,8 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) skhelpers.RegisterPreFailHandler(helpers.StandardGlooDumpOnFail(GinkgoWriter, testHelper.InstallNamespace)) + kubeCli = kubectl.NewCli().WithReceiver(GinkgoWriter) + // Allow skipping of install step for running multiple times if !glootestutils.ShouldSkipInstall() { installGloo() diff --git a/test/kube2e/gloo/setup_syncer_test.go b/test/kube2e/gloo/setup_syncer_test.go index 5bce8bdd826..f6a1716570f 100644 --- a/test/kube2e/gloo/setup_syncer_test.go +++ b/test/kube2e/gloo/setup_syncer_test.go @@ -3,12 +3,12 @@ package gloo_test import ( "context" "net" - "os" - "os/exec" - "strconv" "sync" "time" + "github.com/solo-io/gloo/pkg/utils/kubeutils" + "github.com/solo-io/gloo/pkg/utils/kubeutils/portforward" + "github.com/solo-io/gloo/pkg/utils/settingsutil" "github.com/solo-io/gloo/pkg/bootstrap/leaderelector" @@ -113,53 +113,41 @@ var _ = Describe("Setup Syncer", func() { }).NotTo(Panic()) }) - setupTestGrpcClient := func() func() error { - var cc *grpc.ClientConn - var err error - Eventually(func() error { - cc, err = grpc.DialContext(ctx, "localhost:9988", grpc.WithInsecure(), grpc.WithBlock(), grpc.FailOnNonTempDialError(true)) - return err - }, "10s", "1s").Should(BeNil()) - // setup a gRPC client to make sure connection is persistent across invocations - client := validation.NewGlooValidationServiceClient(cc) - req := &validation.GlooValidationServiceRequest{Proxy: &v1.Proxy{Listeners: []*v1.Listener{{Name: "test-listener"}}}} - return func() error { - _, err := client.Validate(ctx, req) - return err - } - } - - startPortFwd := func() *os.Process { - validationPort := strconv.Itoa(defaults.GlooValidationPort) - portFwd := exec.Command("kubectl", "port-forward", "-n", namespace, - "deployment/gloo", validationPort) - portFwd.Stdout = os.Stderr - portFwd.Stderr = os.Stderr - err := portFwd.Start() - Expect(err).ToNot(HaveOccurred()) - return portFwd.Process - } - It("restarts validation grpc server when settings change", func() { - // setup port forward - portFwdProc := startPortFwd() + portForwarder, err := kubeCli.StartPortForward(ctx, + portforward.WithDeployment(kubeutils.GlooDeploymentName, testHelper.InstallNamespace), + portforward.WithRemotePort(defaults.GlooValidationPort), + ) + Expect(err).NotTo(HaveOccurred()) defer func() { - if portFwdProc != nil { - portFwdProc.Kill() - } + portForwarder.Close() + portForwarder.WaitForStop() }() - testFunc := setupTestGrpcClient() - err := testFunc() + cc, err := grpc.DialContext(ctx, portForwarder.Address(), grpc.WithInsecure()) Expect(err).NotTo(HaveOccurred()) + validationClient := validation.NewGlooValidationServiceClient(cc) + validationRequest := &validation.GlooValidationServiceRequest{ + Proxy: &v1.Proxy{ + Listeners: []*v1.Listener{ + {Name: "test-listener"}, + }, + }, + } + + Eventually(func(g Gomega) { + _, err := validationClient.Validate(ctx, validationRequest) + g.Expect(err).NotTo(HaveOccurred()) + }, "10s", "1s").Should(Succeed(), "validation request should succeed") kube2e.UpdateSettings(ctx, func(settings *v1.Settings) { settings.Gateway.Validation.ValidationServerGrpcMaxSizeBytes = &wrappers.Int32Value{Value: 1} - }, namespace) + }, testHelper.InstallNamespace) - err = testFunc() - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("received message larger than max (19 vs. 1)")) + Eventually(func(g Gomega) { + _, err := validationClient.Validate(ctx, validationRequest) + g.Expect(err).To(MatchError(ContainSubstring("received message larger than max (19 vs. 1)"))) + }, "10s", "1s").Should(Succeed(), "validation request should fail") }) }) }) diff --git a/test/kube2e/helper/curl.go b/test/kube2e/helper/curl.go index 0297a01382d..09f15800df5 100644 --- a/test/kube2e/helper/curl.go +++ b/test/kube2e/helper/curl.go @@ -8,15 +8,15 @@ import ( "strings" "time" + "github.com/solo-io/gloo/pkg/utils/requestutils/curl" + "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega/types" - "github.com/solo-io/gloo/test/gomega/matchers" - "github.com/solo-io/gloo/test/gomega/transforms" - "github.com/solo-io/gloo/test/testutils" - . "github.com/onsi/gomega" "github.com/pkg/errors" + "github.com/solo-io/gloo/test/gomega/matchers" + "github.com/solo-io/gloo/test/gomega/transforms" "github.com/solo-io/go-utils/log" ) @@ -40,6 +40,15 @@ type CurlOpts struct { // Optional SNI name to resolve domain to when sending request Sni string SelfSigned bool + + // Retries on Curl requests are disabled by default because they historically were not configurable + // Curls to a remote container may be subject to network flakes and therefore using retries + // can be a useful mechanism to avoid test flakes + Retries struct { + Retry int + RetryDelay int + RetryMaxTime int + } } var ( @@ -187,68 +196,77 @@ func getExpectedResponseMatcher(expectedOutput interface{}) types.GomegaMatcher } func (t *testContainer) buildCurlArgs(opts CurlOpts) []string { - curlRequestBuilder := testutils.DefaultCurlRequestBuilder() + var curlOptions []curl.Option + + appendOption := func(option curl.Option) { + curlOptions = append(curlOptions, option) + } // The testContainer relies on the transforms.WithCurlHttpResponse to validate the response is what // we would expect // For this transform to behave appropriately, we must execute the request with verbose=true - curlRequestBuilder.VerboseOutput() + appendOption(curl.VerboseOutput()) + + if opts.Retries.Retry != 0 { + appendOption(curl.WithRetries(opts.Retries.Retry, opts.Retries.RetryDelay, opts.Retries.RetryMaxTime)) + } if opts.WithoutStats { - curlRequestBuilder.WithoutStats() + appendOption(curl.Silent()) + } if opts.ReturnHeaders { - curlRequestBuilder.WithReturnHeaders() + appendOption(curl.WithHeadersOnly()) } - curlRequestBuilder.WithConnectionTimeout(opts.ConnectionTimeout) - - curlRequestBuilder.WithPath(opts.Path) + appendOption(curl.WithConnectionTimeout(opts.ConnectionTimeout)) + appendOption(curl.WithPath(opts.Path)) if opts.Method != "" { - curlRequestBuilder.WithMethod(opts.Method) + appendOption(curl.WithMethod(opts.Method)) } if opts.CaFile != "" { - curlRequestBuilder.WithCaFile(opts.CaFile) + appendOption(curl.WithCaFile(opts.CaFile)) } if opts.Host != "" { - curlRequestBuilder.WithHost(opts.Host) + appendOption(curl.WithHostHeader(opts.Host)) } if opts.Body != "" { - curlRequestBuilder.WithPostBody(opts.Body) + appendOption(curl.WithPostBody(opts.Body)) } for h, v := range opts.Headers { - curlRequestBuilder.WithHeader(h, v) + appendOption(curl.WithHeader(h, v)) } if opts.AllowInsecure { - curlRequestBuilder.AllowInsecure() + appendOption(curl.IgnoreServerCert()) } port := opts.Port if port == 0 { port = 8080 } - curlRequestBuilder.WithPort(port) + appendOption(curl.WithPort(port)) if opts.Protocol != "" { - curlRequestBuilder.WithScheme(opts.Protocol) + appendOption(curl.WithScheme(opts.Protocol)) } service := opts.Service if service == "" { service = "test-ingress" } - curlRequestBuilder.WithService(service) + appendOption(curl.WithHost(service)) if opts.SelfSigned { - curlRequestBuilder.SelfSigned() + appendOption(curl.IgnoreServerCert()) } if opts.Sni != "" { - curlRequestBuilder.WithSni(opts.Sni) + appendOption(curl.WithSni(opts.Sni)) } - args := curlRequestBuilder.BuildArgs() + args := append([]string{"curl"}, curl.BuildArgs(curlOptions...)...) log.Printf("running: %v", strings.Join(args, " ")) + return args } diff --git a/test/services/envoy/instance.go b/test/services/envoy/instance.go index d81c19c9c39..b3c632fec7c 100644 --- a/test/services/envoy/instance.go +++ b/test/services/envoy/instance.go @@ -6,18 +6,21 @@ import ( "fmt" "io" "net" - "net/http" "os/exec" - "github.com/solo-io/gloo/test/services" + "github.com/solo-io/go-utils/threadsafe" - adminv3 "github.com/envoyproxy/go-control-plane/envoy/admin/v3" - "github.com/golang/protobuf/jsonpb" + "github.com/solo-io/gloo/pkg/utils/envoyutils/admincli" + "github.com/solo-io/gloo/pkg/utils/requestutils/curl" + + "github.com/solo-io/gloo/test/services" "sync" "text/template" "time" + adminv3 "github.com/envoyproxy/go-control-plane/envoy/admin/v3" + "github.com/onsi/ginkgo/v2" errors "github.com/rotisserie/eris" @@ -57,6 +60,9 @@ type Instance struct { DockerContainerName string *RequestPorts + + // adminApiClient represents the client that can be used to access the Envoy Admin API + adminApiClient *admincli.Client } // RequestPorts are the ports that the Instance will listen on for requests @@ -144,6 +150,18 @@ func (ei *Instance) runWithAll(runConfig RunConfig, bootstrapBuilder bootstrapBu ei.RestXdsPort = runConfig.RestXdsPort ei.envoycfg = bootstrapBuilder.Build(ei) + // construct a client that can be used to access the Admin API + ei.adminApiClient = admincli.NewClient(). + WithReceiver(ginkgo.GinkgoWriter). + WithCurlOptions( + curl.WithPort(int(ei.AdminPort)), + // We include the verbose output of requests so that we have more information + // if a test fails + curl.VerboseOutput(), + // To reduce potential test flakes, we rely on some basic retries in requests to the Envoy Admin API + curl.WithRetries(3, 0, 10), + ) + if ei.UseDocker { return ei.runContainer(runConfig.Context) } @@ -179,30 +197,28 @@ func (ei *Instance) LocalAddr() string { } func (ei *Instance) EnablePanicMode() error { - return ei.setRuntimeConfiguration(fmt.Sprintf("upstream.healthy_panic_threshold=%d", 100)) + return ei.setRuntimeConfiguration( + map[string]string{ + "upstream.healthy_panic_threshold": "100", + }) } func (ei *Instance) DisablePanicMode() error { - return ei.setRuntimeConfiguration(fmt.Sprintf("upstream.healthy_panic_threshold=%d", 0)) + return ei.setRuntimeConfiguration( + map[string]string{ + "upstream.healthy_panic_threshold": "0", + }) } -func (ei *Instance) setRuntimeConfiguration(queryParameters string) error { - resp, err := http.Post(fmt.Sprintf("http://localhost:%d/runtime_modify?%s", ei.AdminPort, queryParameters), "", nil) - if err != nil { - return err - } - resp.Body.Close() - return nil +func (ei *Instance) setRuntimeConfiguration(queryParameters map[string]string) error { + return ei.AdminClient().ModifyRuntimeConfiguration(context.Background(), queryParameters) } func (ei *Instance) Clean() { if ei == nil { return } - resp, err := http.Post(fmt.Sprintf("http://localhost:%d/quitquitquit", ei.AdminPort), "", nil) - if err == nil { - resp.Body.Close() - } + _ = ei.AdminClient().ShutdownServer(context.Background()) if ei.cmd != nil { ei.cmd.Process.Kill() @@ -292,52 +308,28 @@ func (ei *Instance) Logs() (string, error) { return ei.logs.String(), nil } +// Deprecated: Prefer StructuredConfigDump func (ei *Instance) ConfigDump() (string, error) { - return ei.getAdminEndpointData("config_dump") -} + var outLocation threadsafe.Buffer -func (ei *Instance) StructuredConfigDump() (*adminv3.ConfigDump, error) { - adminUrl := fmt.Sprintf("http://%s:%d/%s", ei.LocalAddr(), ei.AdminPort, "config_dump") - response, err := http.Get(adminUrl) + err := ei.AdminClient().ConfigDumpCmd(context.Background()).WithStdout(&outLocation).Run().Cause() if err != nil { - return nil, err - } - - defer response.Body.Close() - - jsonpbMarshaler := &jsonpb.Unmarshaler{ - // Ever since upgrading the go-control-plane to v0.10.1 this test fails with the following error: - // unknown field \"hidden_envoy_deprecated_build_version\" in envoy.config.core.v3.Node" - // Set AllowUnknownFields to true to get around this - AllowUnknownFields: true, + return "", err } - var cfgDump adminv3.ConfigDump - if err = jsonpbMarshaler.Unmarshal(response.Body, &cfgDump); err != nil { - return nil, err - } + return outLocation.String(), nil +} - return &cfgDump, nil +func (ei *Instance) StructuredConfigDump() (*adminv3.ConfigDump, error) { + return ei.AdminClient().GetConfigDump(context.Background()) } func (ei *Instance) Statistics() (string, error) { - return ei.getAdminEndpointData("stats") + return ei.AdminClient().GetStats(context.Background()) } -func (ei *Instance) getAdminEndpointData(endpoint string) (string, error) { - adminUrl := fmt.Sprintf("http://%s:%d/%s", ei.LocalAddr(), ei.AdminPort, endpoint) - response, err := http.Get(adminUrl) - if err != nil { - return "", err - } - - responseBytes := new(bytes.Buffer) - defer response.Body.Close() - if _, err := io.Copy(responseBytes, response.Body); err != nil { - return "", err - } - - return responseBytes.String(), nil +func (ei *Instance) AdminClient() *admincli.Client { + return ei.adminApiClient } // SafeBuffer is a goroutine safe bytes.Buffer diff --git a/test/testutils/curl_request.go b/test/testutils/curl_request.go deleted file mode 100644 index 44cf4b0b41e..00000000000 --- a/test/testutils/curl_request.go +++ /dev/null @@ -1,215 +0,0 @@ -package testutils - -import ( - "errors" - "fmt" - "net/http" - - "github.com/onsi/ginkgo/v2" -) - -// CurlRequestBuilder simplifies the process of generating curl requests in tests -type CurlRequestBuilder struct { - verbose bool - allowInsecure bool - selfSigned bool - withoutStats bool - connectionTimeout int // seconds - returnHeaders bool - method string - host string - port int - headers map[string]string - body string - service string - sni string - caFile string - path string - - scheme string - - additionalArgs []string -} - -// DefaultCurlRequestBuilder returns a CurlRequestBuilder with some default values -func DefaultCurlRequestBuilder() *CurlRequestBuilder { - return &CurlRequestBuilder{ - verbose: false, - allowInsecure: false, - selfSigned: false, - withoutStats: false, - connectionTimeout: 3, - returnHeaders: false, - method: http.MethodGet, - host: "", - port: 8080, - headers: make(map[string]string), - scheme: "http", // https://github.com/golang/go/issues/40587 - service: "", - sni: "", - caFile: "", - path: "", - - additionalArgs: []string{}, - } -} - -func (c *CurlRequestBuilder) VerboseOutput() *CurlRequestBuilder { - c.verbose = true - return c -} - -func (c *CurlRequestBuilder) AllowInsecure() *CurlRequestBuilder { - c.verbose = true - return c -} - -func (c *CurlRequestBuilder) SelfSigned() *CurlRequestBuilder { - c.selfSigned = true - return c -} - -func (c *CurlRequestBuilder) WithoutStats() *CurlRequestBuilder { - c.withoutStats = true - return c -} - -func (c *CurlRequestBuilder) WithReturnHeaders() *CurlRequestBuilder { - c.returnHeaders = true - return c -} - -func (c *CurlRequestBuilder) WithConnectionTimeout(seconds int) *CurlRequestBuilder { - c.connectionTimeout = seconds - return c -} - -func (c *CurlRequestBuilder) WithMethod(method string) *CurlRequestBuilder { - c.method = method - return c -} - -func (c *CurlRequestBuilder) WithPort(port int) *CurlRequestBuilder { - c.port = port - return c -} - -func (c *CurlRequestBuilder) WithService(service string) *CurlRequestBuilder { - c.service = service - return c -} - -func (c *CurlRequestBuilder) WithSni(sni string) *CurlRequestBuilder { - c.sni = sni - return c -} - -func (c *CurlRequestBuilder) WithCaFile(caFile string) *CurlRequestBuilder { - c.caFile = caFile - return c -} - -func (c *CurlRequestBuilder) WithPath(path string) *CurlRequestBuilder { - c.path = path - return c -} - -func (c *CurlRequestBuilder) WithPostBody(body string) *CurlRequestBuilder { - return c.WithBody(body).WithContentType("application/json") -} - -func (c *CurlRequestBuilder) WithBody(body string) *CurlRequestBuilder { - c.body = body - return c -} - -func (c *CurlRequestBuilder) WithContentType(contentType string) *CurlRequestBuilder { - return c.WithHeader("Content-Type", contentType) -} - -func (c *CurlRequestBuilder) WithHost(host string) *CurlRequestBuilder { - return c.WithHeader("Host", host) -} - -func (c *CurlRequestBuilder) WithHeader(key, value string) *CurlRequestBuilder { - c.headers[key] = value - return c -} - -func (c *CurlRequestBuilder) WithScheme(scheme string) *CurlRequestBuilder { - c.scheme = scheme - return c -} - -// WithArgs allows developers to append arbitrary args to the CurlRequestBuilder -// This should mainly be used for debugging purposes. If there is an argument that the builder -// doesn't yet support, it should be added explicitly, to make it easier for developers to utilize -func (c *CurlRequestBuilder) WithArgs(args []string) *CurlRequestBuilder { - c.additionalArgs = args - return c -} - -func (c *CurlRequestBuilder) errorIfInvalid() error { - if c.service == "" { - return errors.New("service is empty, but required") - } - - return nil -} - -func (c *CurlRequestBuilder) BuildArgs() []string { - ginkgo.GinkgoHelper() - - if err := c.errorIfInvalid(); err != nil { - // We error loudly here - // These types of errors are intended to prevent developers from creating resources - // which are semantically correct, but lead to test flakes/confusion - ginkgo.Fail(err.Error()) - } - - args := []string{"curl"} - - if c.verbose { - args = append(args, "-v") - } - if c.allowInsecure { - args = append(args, "-k") - } - if c.withoutStats { - args = append(args, "-s") - } - if c.connectionTimeout > 0 { - seconds := fmt.Sprintf("%v", c.connectionTimeout) - args = append(args, "--connect-timeout", seconds, "--max-time", seconds) - } - if c.returnHeaders { - args = append(args, "-I") - } - if c.method != http.MethodGet && c.method != "" { - args = append(args, "-X"+c.method) - } - for h, v := range c.headers { - args = append(args, "-H", fmt.Sprintf("%v: %v", h, v)) - } - if c.caFile != "" { - args = append(args, "--cacert", c.caFile) - } - if c.body != "" { - args = append(args, "-d", c.body) - } - if c.selfSigned { - args = append(args, "-k") - } - if len(c.additionalArgs) > 0 { - args = append(args, c.additionalArgs...) - } - if c.sni != "" { - sniResolution := fmt.Sprintf("%s:%d:%s", c.sni, c.port, c.service) - fullAddress := fmt.Sprintf("%s://%s:%d", c.scheme, c.sni, c.port) - args = append(args, "--resolve", sniResolution, fullAddress) - } else { - args = append(args, fmt.Sprintf("%v://%s:%v%s", c.scheme, c.service, c.port, c.path)) - } - - return args -}