Skip to content

Commit 4337e57

Browse files
fix: thread stdout/stderr from cobra through to func-e for Host provider (#7142)
* fix: thread stdout/stderr from cobra through to func-e for Host provider Fixes an issue where the Host infrastructure provider hardcoded `os.Stdout` when starting Envoy via func-e, bypassing output redirection configured through Cobra commands. This prevented applications like ai-gateway from properly capturing or redirecting Envoy's output when using the Host provider. The fix threads stdout and stderr writers from Cobra's `cmd.OutOrStdout()` and `cmd.ErrOrStderr()` through the configuration chain to the `runEnvoy` function, which now uses func-e's three writer options: `api.Out()` for func-e status messages, `api.EnvoyOut()` for Envoy stdout, and `api.EnvoyErr()` for Envoy stderr. Changes: * Add Stdout and Stderr fields to config.Server struct * Thread writers from server command through getConfig chain * Add Stdout and Stderr fields to host.Infra struct * Update runEnvoy to use i.Stdout/i.Stderr instead of hardcoded os.Stdout * Add tests for output redirection behavior Signed-off-by: Adrian Cole <[email protected]>
1 parent cdf7037 commit 4337e57

27 files changed

+253
-64
lines changed

internal/cmd/certgen.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func GetCertGenCommand() *cobra.Command {
6161

6262
// certGen generates control plane certificates.
6363
func certGen(ctx context.Context, logOut io.Writer, local bool) error {
64-
cfg, err := config.New(logOut)
64+
cfg, err := config.New(logOut, io.Discard)
6565
if err != nil {
6666
return err
6767
}

internal/cmd/certgen_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2222

2323
"github.com/envoyproxy/gateway/internal/crypto"
24+
"github.com/envoyproxy/gateway/internal/envoygateway/config"
2425
)
2526

2627
func TestGetCertgenCommand(t *testing.T) {
@@ -29,7 +30,7 @@ func TestGetCertgenCommand(t *testing.T) {
2930
}
3031

3132
func TestOutputCertsForLocal(t *testing.T) {
32-
cfg, err := getConfig(os.Stdout)
33+
cfg, err := config.New(os.Stdout, os.Stderr)
3334
require.NoError(t, err)
3435

3536
certs, err := crypto.GenerateCerts(cfg)
@@ -52,7 +53,7 @@ func TestOutputCertsForLocal(t *testing.T) {
5253
}
5354

5455
func TestPatchTopologyWebhook(t *testing.T) {
55-
cfg, err := getConfig(os.Stdout)
56+
cfg, err := config.New(os.Stdout, os.Stderr)
5657
require.NoError(t, err)
5758

5859
cases := []struct {

internal/cmd/server.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func GetServerCommand() *cobra.Command {
4545
Aliases: []string{"serve"},
4646
Short: "Serve Envoy Gateway",
4747
RunE: func(cmd *cobra.Command, args []string) error {
48-
return server(cmd.Context(), cmd.OutOrStdout())
48+
return server(cmd.Context(), cmd.OutOrStdout(), cmd.ErrOrStderr())
4949
},
5050
}
5151
cmd.PersistentFlags().StringVarP(&cfgPath, "config-path", "c", "",
@@ -55,8 +55,8 @@ func GetServerCommand() *cobra.Command {
5555
}
5656

5757
// server serves Envoy Gateway.
58-
func server(ctx context.Context, logOut io.Writer) error {
59-
cfg, err := getConfig(logOut)
58+
func server(ctx context.Context, stdout, stderr io.Writer) error {
59+
cfg, err := getConfig(stdout, stderr)
6060
if err != nil {
6161
return err
6262
}
@@ -72,7 +72,7 @@ func server(ctx context.Context, logOut io.Writer) error {
7272
return nil
7373
}
7474
l := loader.New(cfgPath, cfg, hook)
75-
if err := l.Start(ctx, logOut); err != nil {
75+
if err := l.Start(ctx, stdout); err != nil {
7676
return err
7777
}
7878

@@ -88,14 +88,14 @@ func server(ctx context.Context, logOut io.Writer) error {
8888
}
8989

9090
// getConfig gets the Server configuration
91-
func getConfig(logOut io.Writer) (*config.Server, error) {
92-
return getConfigByPath(logOut, cfgPath)
91+
func getConfig(stdout, stderr io.Writer) (*config.Server, error) {
92+
return getConfigByPath(stdout, stderr, cfgPath)
9393
}
9494

9595
// make `cfgPath` an argument to test it without polluting the global var
96-
func getConfigByPath(logOut io.Writer, cfgPath string) (*config.Server, error) {
96+
func getConfigByPath(stdout, stderr io.Writer, cfgPath string) (*config.Server, error) {
9797
// Initialize with default config parameters.
98-
cfg, err := config.New(logOut)
98+
cfg, err := config.New(stdout, stderr)
9999
if err != nil {
100100
return nil, err
101101
}
@@ -118,7 +118,7 @@ func getConfigByPath(logOut io.Writer, cfgPath string) (*config.Server, error) {
118118
cfg.EnvoyGateway = eg
119119
// update cfg logger
120120
eg.Logging.SetEnvoyGatewayLoggingDefaults()
121-
cfg.Logger = logging.NewLogger(logOut, eg.Logging)
121+
cfg.Logger = logging.NewLogger(stdout, eg.Logging)
122122
}
123123

124124
if err := cfg.Validate(); err != nil {

internal/cmd/server_test.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package cmd
77

88
import (
9+
"bytes"
10+
"io"
911
"os"
1012
"testing"
1113

@@ -60,7 +62,7 @@ func TestGetConfigValidate(t *testing.T) {
6062
_, err = file.WriteString(test.input)
6163
require.NoError(t, err)
6264

63-
_, err = getConfigByPath(os.Stderr, file.Name())
65+
_, err = getConfigByPath(io.Discard, io.Discard, file.Name())
6466
if test.errors == nil {
6567
require.NoError(t, err)
6668
} else {
@@ -71,3 +73,26 @@ func TestGetConfigValidate(t *testing.T) {
7173
})
7274
}
7375
}
76+
77+
// TestServerCommand_OutputRedirection verifies that the server command respects output redirection.
78+
func TestServerCommand_OutputRedirection(t *testing.T) {
79+
file, err := os.CreateTemp("", "config")
80+
require.NoError(t, err)
81+
defer os.Remove(file.Name())
82+
83+
_, err = file.WriteString(validGatewayConfig)
84+
require.NoError(t, err)
85+
86+
// Create separate buffers for stdout and stderr
87+
stdout := &bytes.Buffer{}
88+
stderr := &bytes.Buffer{}
89+
90+
// Test that getConfigByPath uses the provided writers
91+
cfg, err := getConfigByPath(stdout, stderr, file.Name())
92+
require.NoError(t, err)
93+
require.NotNil(t, cfg)
94+
95+
// Verify the config has the writers set
96+
require.Equal(t, stdout, cfg.Stdout)
97+
require.Equal(t, stderr, cfg.Stderr)
98+
}

internal/crypto/certgen_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TestGenerateCerts(t *testing.T) {
2626
wantEnvoyDNSName string
2727
}
2828

29-
cfg, err := config.New(os.Stdout)
29+
cfg, err := config.New(os.Stdout, os.Stderr)
3030
require.NoError(t, err)
3131

3232
run := func(t *testing.T, name string, tc testcase) {

internal/envoygateway/config/config.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,21 @@ type Server struct {
3939
Logger logging.Logger
4040
// Elected chan is used to signal when an EG instance is elected as leader.
4141
Elected chan struct{}
42+
// Stdout is the writer for standard output.
43+
Stdout io.Writer
44+
// Stderr is the writer for error output.
45+
Stderr io.Writer
4246
}
4347

4448
// New returns a Server with default parameters.
45-
func New(logOut io.Writer) (*Server, error) {
49+
func New(stdout, stderr io.Writer) (*Server, error) {
4650
return &Server{
4751
EnvoyGateway: egv1a1.DefaultEnvoyGateway(),
4852
ControllerNamespace: env.Lookup("ENVOY_GATEWAY_NAMESPACE", DefaultNamespace),
4953
DNSDomain: env.Lookup("KUBERNETES_CLUSTER_DOMAIN", DefaultDNSDomain),
50-
Logger: logging.DefaultLogger(logOut, egv1a1.LogLevelInfo),
54+
Logger: logging.DefaultLogger(stdout, egv1a1.LogLevelInfo),
55+
Stdout: stdout,
56+
Stderr: stderr,
5157
Elected: make(chan struct{}),
5258
}, nil
5359
}

internal/envoygateway/config/config_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ var (
2222
)
2323

2424
func TestValidate(t *testing.T) {
25-
cfg, err := New(os.Stdout)
25+
cfg, err := New(os.Stdout, os.Stderr)
2626
require.NoError(t, err)
2727

2828
testCases := []struct {

internal/envoygateway/config/loader/configloader_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestConfigLoader(t *testing.T) {
3232

3333
cfgPath := tmpDir + "/config.yaml"
3434
require.NoError(t, os.WriteFile(cfgPath, []byte(defaultConfig), 0o600))
35-
s, err := config.New(os.Stdout)
35+
s, err := config.New(os.Stdout, os.Stderr)
3636
require.NoError(t, err)
3737

3838
ctx, cancel := context.WithCancel(context.TODO())

internal/gatewayapi/runner/runner_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestRunner(t *testing.T) {
3030
pResources := new(message.ProviderResources)
3131
xdsIR := new(message.XdsIR)
3232
infraIR := new(message.InfraIR)
33-
cfg, err := config.New(os.Stdout)
33+
cfg, err := config.New(os.Stdout, os.Stderr)
3434
require.NoError(t, err)
3535
extMgr, closeFunc, err := registry.NewInMemoryManager(&egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
3636
require.NoError(t, err)
@@ -70,7 +70,7 @@ func setupTestRunner(t *testing.T) (*Runner, []types.NamespacedName) {
7070
pResources := new(message.ProviderResources)
7171
xdsIR := new(message.XdsIR)
7272
infraIR := new(message.InfraIR)
73-
cfg, err := config.New(os.Stdout)
73+
cfg, err := config.New(os.Stdout, os.Stderr)
7474
require.NoError(t, err)
7575
extMgr, closeFunc, err := registry.NewInMemoryManager(&egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
7676
require.NoError(t, err)

internal/globalratelimit/runner/runner_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func Test_subscribeAndTranslate(t *testing.T) {
210210
defer cancel()
211211
xdsIR := new(message.XdsIR)
212212
defer xdsIR.Close()
213-
cfg, err := config.New(os.Stdout)
213+
cfg, err := config.New(os.Stdout, os.Stderr)
214214
require.NoError(t, err)
215215

216216
r := New(&Config{

0 commit comments

Comments
 (0)