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>

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

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
 }

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:]...)
 }
 

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._

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")
+}

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))
+}

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")
+			})
+		})
+	})
+})