From 7f4455a28d50775ca86fb45c9b452f096fc27a2c Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 29 Oct 2025 12:52:41 +0800 Subject: [PATCH 1/2] feat(host): auto-generate certificates for host mode Auto-generates TLS certificates for host mode when they don't exist, eliminating the manual 'envoy-gateway certgen --local' step. Also fixes a data race in proxy context map by using atomic LoadAndDelete operation. Signed-off-by: Adrian Cole --- internal/crypto/certgen.go | 4 + internal/crypto/certgen_test.go | 30 +- internal/infrastructure/host/infra.go | 67 +++- internal/infrastructure/host/infra_test.go | 131 +++++++ internal/infrastructure/host/proxy_infra.go | 47 ++- .../infrastructure/host/proxy_infra_test.go | 337 +++++++++++++++--- release-notes/current.yaml | 2 + 7 files changed, 529 insertions(+), 89 deletions(-) create mode 100644 internal/infrastructure/host/infra_test.go diff --git a/internal/crypto/certgen.go b/internal/crypto/certgen.go index 561953b44d..678afeaaf5 100644 --- a/internal/crypto/certgen.go +++ b/internal/crypto/certgen.go @@ -110,6 +110,10 @@ func GenerateCerts(cfg *config.Server) (*Certificates, error) { case egv1a1.ProviderTypeKubernetes: egDNSNames = kubeServiceNames(DefaultEnvoyGatewayDNSPrefix, cfg.ControllerNamespace, cfg.DNSDomain) envoyDNSNames = append(envoyDNSNames, fmt.Sprintf("*.%s", cfg.ControllerNamespace)) + case egv1a1.ProviderTypeCustom: + // For custom provider (host mode), use localhost for xDS communication + egDNSNames = []string{"localhost"} + envoyDNSNames = []string{"localhost"} default: // Kubernetes is the only supported Envoy Gateway provider. return nil, fmt.Errorf("unsupported provider type %v", egProvider) diff --git a/internal/crypto/certgen_test.go b/internal/crypto/certgen_test.go index 12efa708e5..4a55c48cc8 100644 --- a/internal/crypto/certgen_test.go +++ b/internal/crypto/certgen_test.go @@ -16,26 +16,24 @@ import ( "github.com/stretchr/testify/require" + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/envoygateway/config" ) func TestGenerateCerts(t *testing.T) { type testcase struct { - certConfig *Configuration + cfg *config.Server wantEnvoyGatewayDNSName string wantEnvoyDNSName string } - cfg, err := config.New(os.Stdout, os.Stderr) - require.NoError(t, err) - run := func(t *testing.T, name string, tc testcase) { t.Helper() t.Run(name, func(t *testing.T) { t.Helper() - got, err := GenerateCerts(cfg) + got, err := GenerateCerts(tc.cfg) require.NoError(t, err) roots := x509.NewCertPool() @@ -52,11 +50,29 @@ func TestGenerateCerts(t *testing.T) { }) } - run(t, "no configuration - use defaults", testcase{ - certConfig: &Configuration{}, + // Test Kubernetes provider (default) + kubeCfg, err := config.New(os.Stdout, os.Stderr) + require.NoError(t, err) + + run(t, "kubernetes provider - use defaults", testcase{ + cfg: kubeCfg, wantEnvoyGatewayDNSName: DefaultEnvoyGatewayDNSPrefix, wantEnvoyDNSName: fmt.Sprintf("*.%s", config.DefaultNamespace), }) + + // Test Custom provider + customCfg, err := config.New(os.Stdout, os.Stderr) + require.NoError(t, err) + // Set provider type to Custom + customCfg.EnvoyGateway.Provider = &egv1a1.EnvoyGatewayProvider{ + Type: egv1a1.ProviderTypeCustom, + } + + run(t, "custom provider - use localhost", testcase{ + cfg: customCfg, + wantEnvoyGatewayDNSName: "localhost", + wantEnvoyDNSName: "localhost", + }) } func TestGeneratedValidKubeCerts(t *testing.T) { diff --git a/internal/infrastructure/host/infra.go b/internal/infrastructure/host/infra.go index c586f2c53d..039964744e 100644 --- a/internal/infrastructure/host/infra.go +++ b/internal/infrastructure/host/infra.go @@ -11,8 +11,13 @@ import ( "io" "os" "path/filepath" + "sync" + + func_e "github.com/tetratelabs/func-e" + func_e_api "github.com/tetratelabs/func-e/api" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/crypto" "github.com/envoyproxy/gateway/internal/envoygateway/config" "github.com/envoyproxy/gateway/internal/infrastructure/common" "github.com/envoyproxy/gateway/internal/logging" @@ -42,7 +47,7 @@ type Infra struct { EnvoyGateway *egv1a1.EnvoyGateway // proxyContextMap store the context of each running proxy by its name for lifecycle management. - proxyContextMap map[string]*proxyContext + proxyContextMap sync.Map // sdsConfigPath is the path to SDS configuration files. sdsConfigPath string @@ -54,6 +59,9 @@ type Infra struct { Stdout io.Writer // Stderr is the writer for error output (for Envoy stderr). Stderr io.Writer + + // envoyRunner runs Envoy (can be overridden in tests). + envoyRunner func_e_api.RunFunc } func NewInfra(runnerCtx context.Context, cfg *config.Server, logger logging.Logger) (*Infra, error) { @@ -75,10 +83,10 @@ func NewInfra(runnerCtx context.Context, cfg *config.Server, logger logging.Logg return nil, fmt.Errorf("failed to create data directory: %w", err) } - // Check local certificates dir exist + // Check if certificates exist, generate them if not certPath := paths.CertDir("envoy") - if _, err := os.Lstat(certPath); err != nil { - return nil, fmt.Errorf("failed to stat cert dir: %w", err) + if err := maybeGenerateCertificates(cfg, certPath); err != nil { + return nil, err } // Ensure the sds config exist @@ -90,11 +98,11 @@ func NewInfra(runnerCtx context.Context, cfg *config.Server, logger logging.Logg Paths: paths, Logger: logger, EnvoyGateway: cfg.EnvoyGateway, - proxyContextMap: make(map[string]*proxyContext), sdsConfigPath: certPath, defaultEnvoyImage: egv1a1.DefaultEnvoyProxyImage, Stdout: cfg.Stdout, Stderr: cfg.Stderr, + envoyRunner: func_e.Run, } return infra, nil } @@ -116,3 +124,52 @@ func createSdsConfig(dir string) error { return nil } + +// maybeGenerateCertificates checks if all required certificate files exist and generates them if any is missing. +func maybeGenerateCertificates(cfg *config.Server, certPath string) error { + certFiles := []string{"ca.crt", "tls.crt", "tls.key"} + + // Check if any cert file is missing + var missing bool + for _, filename := range certFiles { + filePath := filepath.Join(certPath, filename) + _, err := os.Lstat(filePath) + if os.IsNotExist(err) { + missing = true + break + } + if err != nil { + return fmt.Errorf("failed to stat %s: %w", filename, err) + } + } + + if !missing { + // All files exist, nothing to do + return nil + } + + // Generate certificates automatically + certs, err := crypto.GenerateCerts(cfg) + if err != nil { + return fmt.Errorf("failed to generate certificates: %w", err) + } + + // Create the cert directory + if err := os.MkdirAll(certPath, 0o750); err != nil { + return fmt.Errorf("failed to create cert directory: %w", err) + } + + // Write cert files + certMap := map[string][]byte{ + "ca.crt": certs.CACertificate, + "tls.crt": certs.EnvoyCertificate, + "tls.key": certs.EnvoyPrivateKey, + } + + for filename, content := range certMap { + if err := file.Write(string(content), filepath.Join(certPath, filename)); err != nil { + return fmt.Errorf("failed to write %s: %w", filename, err) + } + } + return nil +} diff --git a/internal/infrastructure/host/infra_test.go b/internal/infrastructure/host/infra_test.go new file mode 100644 index 0000000000..50b12c5007 --- /dev/null +++ b/internal/infrastructure/host/infra_test.go @@ -0,0 +1,131 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package host + +import ( + "io" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/envoyproxy/gateway/internal/envoygateway/config" + "github.com/envoyproxy/gateway/internal/infrastructure/common" + "github.com/envoyproxy/gateway/internal/utils/file" +) + +func TestMaybeGenerateCertificates(t *testing.T) { + cfg, err := config.New(io.Discard, io.Discard) + require.NoError(t, err) + + certFiles := []string{"ca.crt", "tls.crt", "tls.key"} + + t.Run("all_files_exist", func(t *testing.T) { + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "envoy") + + // Create directory and dummy files + require.NoError(t, os.MkdirAll(certPath, 0o750)) + for _, filename := range certFiles { + fpath := filepath.Join(certPath, filename) + require.NoError(t, os.WriteFile(fpath, []byte("dummy"), 0o600)) + } + + err := maybeGenerateCertificates(cfg, certPath) + require.NoError(t, err) + + // Verify files still exist and unchanged size + for _, filename := range certFiles { + data, err := os.ReadFile(filepath.Join(certPath, filename)) + require.NoError(t, err) + require.Len(t, data, 5) // "dummy" + } + }) + + t.Run("missing_files", func(t *testing.T) { + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "envoy") + + err := maybeGenerateCertificates(cfg, certPath) + require.NoError(t, err) + + // Verify directory created + info, err := os.Stat(certPath) + require.NoError(t, err) + require.True(t, info.IsDir()) + + // Verify all files created and non-empty + for _, filename := range certFiles { + data, err := os.ReadFile(filepath.Join(certPath, filename)) + require.NoError(t, err) + require.NotEmpty(t, data, filename) + } + }) + + t.Run("partial_files_missing", func(t *testing.T) { + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "envoy") + + require.NoError(t, os.MkdirAll(certPath, 0o750)) + + // Create only one file + require.NoError(t, os.WriteFile(filepath.Join(certPath, "ca.crt"), []byte("dummy"), 0o600)) + + err := maybeGenerateCertificates(cfg, certPath) + require.NoError(t, err) + + // Verify all files created and non-empty + for _, filename := range certFiles { + data, err := os.ReadFile(filepath.Join(certPath, filename)) + require.NoError(t, err) + require.NotEmpty(t, data, filename) + } + }) + + t.Run("cert_generation_fails", func(t *testing.T) { + tmpDir := t.TempDir() + // This tests mkdir fail by making parent unwritable + unwritableDir := filepath.Join(tmpDir, "unwritable") + require.NoError(t, os.Mkdir(unwritableDir, 0o555)) // Read-only + + badCertPath := filepath.Join(unwritableDir, "envoy") + err := maybeGenerateCertificates(cfg, badCertPath) + require.ErrorContains(t, err, "failed to create cert directory") + }) +} + +func TestCreateSdsConfig(t *testing.T) { + t.Run("success", func(t *testing.T) { + dir := t.TempDir() + // Create required cert files + require.NoError(t, file.Write("test ca", filepath.Join(dir, XdsTLSCaFilename))) + require.NoError(t, file.Write("test cert", filepath.Join(dir, XdsTLSCertFilename))) + require.NoError(t, file.Write("test key", filepath.Join(dir, XdsTLSKeyFilename))) + + err := createSdsConfig(dir) + require.NoError(t, err) + + // Verify CA config was created + caConfigPath := filepath.Join(dir, common.SdsCAFilename) + actualCAConfig, err := os.ReadFile(caConfigPath) + require.NoError(t, err) + require.NotEmpty(t, actualCAConfig) + + // Verify cert config was created + certConfigPath := filepath.Join(dir, common.SdsCertFilename) + actualCertConfig, err := os.ReadFile(certConfigPath) + require.NoError(t, err) + require.NotEmpty(t, actualCertConfig) + }) + + t.Run("error_writing_ca_config", func(t *testing.T) { + // Use invalid path to force file.Write to fail + invalidDir := filepath.Join("/", "nonexistent", "invalid", "path") + err := createSdsConfig(invalidDir) + require.Error(t, err) + }) +} diff --git a/internal/infrastructure/host/proxy_infra.go b/internal/infrastructure/host/proxy_infra.go index 033478971e..1a4b9d5fee 100644 --- a/internal/infrastructure/host/proxy_infra.go +++ b/internal/infrastructure/host/proxy_infra.go @@ -12,9 +12,9 @@ import ( "path/filepath" "regexp" "strings" + "sync" - func_e "github.com/tetratelabs/func-e" - "github.com/tetratelabs/func-e/api" + func_e_api "github.com/tetratelabs/func-e/api" "k8s.io/utils/ptr" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -34,9 +34,19 @@ type proxyContext struct { // Close implements the Manager interface. func (i *Infra) Close() error { - for name := range i.proxyContextMap { - i.stopEnvoy(name) - } + var wg sync.WaitGroup + + // Stop any Envoy subprocesses in parallel + i.proxyContextMap.Range(func(key, value any) bool { + wg.Add(1) + go func(name string) { + defer wg.Done() + i.stopEnvoy(name) + }(key.(string)) + return true + }) + + wg.Wait() return nil } @@ -53,7 +63,7 @@ func (i *Infra) CreateOrUpdateProxyInfra(ctx context.Context, infra *ir.Infra) e proxyInfra := infra.GetProxyInfra() proxyName := utils.GetHashedName(proxyInfra.Name, 64) // Return directly if the proxy is running. - if _, ok := i.proxyContextMap[proxyName]; ok { + if _, loaded := i.proxyContextMap.Load(proxyName); loaded { return nil } @@ -91,22 +101,22 @@ func (i *Infra) CreateOrUpdateProxyInfra(ctx context.Context, infra *ir.Infra) e func (i *Infra) runEnvoy(ctx context.Context, envoyVersion, name string, args []string) { pCtx, cancel := context.WithCancel(ctx) exit := make(chan struct{}, 1) - i.proxyContextMap[name] = &proxyContext{cancel: cancel, exit: exit} + i.proxyContextMap.Store(name, &proxyContext{cancel: cancel, exit: exit}) go func() { // Run blocks until pCtx is done or the process exits where the latter doesn't happen when // Envoy successfully starts up. So, this will not return until pCtx is done in practice. defer func() { exit <- struct{}{} }() - err := func_e.Run(pCtx, args, - api.ConfigHome(i.Paths.ConfigHome), - api.DataHome(i.Paths.DataHome), - api.StateHome(i.Paths.StateHome), - api.RuntimeDir(i.Paths.RuntimeDir), - api.Out(i.Stdout), - api.EnvoyOut(i.Stdout), - api.EnvoyErr(i.Stderr), - api.EnvoyVersion(envoyVersion)) + err := i.envoyRunner(pCtx, args, + func_e_api.ConfigHome(i.Paths.ConfigHome), + func_e_api.DataHome(i.Paths.DataHome), + func_e_api.StateHome(i.Paths.StateHome), + func_e_api.RuntimeDir(i.Paths.RuntimeDir), + func_e_api.Out(i.Stdout), + func_e_api.EnvoyOut(i.Stdout), + func_e_api.EnvoyErr(i.Stderr), + func_e_api.EnvoyVersion(envoyVersion)) if err != nil { i.Logger.Error(err, "failed to run envoy") } @@ -127,11 +137,12 @@ func (i *Infra) DeleteProxyInfra(_ context.Context, infra *ir.Infra) error { // stopEnvoy stops the Envoy process by its name. It will block until the process completely stopped. func (i *Infra) stopEnvoy(proxyName string) { - if pCtx, ok := i.proxyContextMap[proxyName]; ok { + value, ok := i.proxyContextMap.LoadAndDelete(proxyName) + if ok { + pCtx := value.(*proxyContext) pCtx.cancel() // Cancel causes the Envoy process to exit. <-pCtx.exit // Wait for the Envoy process to completely exit. close(pCtx.exit) // Close the channel to avoid leaking. - delete(i.proxyContextMap, proxyName) } } diff --git a/internal/infrastructure/host/proxy_infra_test.go b/internal/infrastructure/host/proxy_infra_test.go index 7ddebc5f88..bb8f945137 100644 --- a/internal/infrastructure/host/proxy_infra_test.go +++ b/internal/infrastructure/host/proxy_infra_test.go @@ -6,13 +6,17 @@ package host import ( + "context" + "fmt" "io" "os" "path" "testing" + "testing/synctest" "time" "github.com/stretchr/testify/require" + func_e_api "github.com/tetratelabs/func-e/api" "k8s.io/utils/ptr" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -21,11 +25,13 @@ import ( "github.com/envoyproxy/gateway/internal/infrastructure/common" "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/logging" + "github.com/envoyproxy/gateway/internal/utils" "github.com/envoyproxy/gateway/internal/utils/file" "github.com/envoyproxy/gateway/internal/xds/bootstrap" - "github.com/envoyproxy/gateway/test/utils" + testutils "github.com/envoyproxy/gateway/test/utils" ) +// newMockInfra doesn't actually run Envoy func newMockInfra(t *testing.T, cfg *config.Server) *Infra { t.Helper() homeDir := t.TempDir() @@ -51,51 +57,197 @@ func newMockInfra(t *testing.T, cfg *config.Server) *Infra { RuntimeDir: homeDir, } infra := &Infra{ - Paths: paths, - Logger: logging.DefaultLogger(io.Discard, egv1a1.LogLevelInfo), - EnvoyGateway: cfg.EnvoyGateway, - proxyContextMap: make(map[string]*proxyContext), - sdsConfigPath: proxyDir, - Stdout: io.Discard, - Stderr: io.Discard, + Paths: paths, + Logger: logging.DefaultLogger(io.Discard, egv1a1.LogLevelInfo), + EnvoyGateway: cfg.EnvoyGateway, + sdsConfigPath: proxyDir, + Stdout: io.Discard, + Stderr: io.Discard, + envoyRunner: func(ctx context.Context, args []string, options ...func_e_api.RunOption) error { + // Block until context is cancelled (mimics real Envoy blocking) + <-ctx.Done() + return ctx.Err() + }, } return infra } -func TestInfraCreateProxy(t *testing.T) { +func TestInfra_CreateOrUpdateProxyInfra(t *testing.T) { cfg, err := config.New(io.Discard, io.Discard) require.NoError(t, err) infra := newMockInfra(t, cfg) + t.Run("create new proxy", func(t *testing.T) { + infraIR := &ir.Infra{ + Proxy: &ir.ProxyInfra{ + Name: "test-proxy", + Namespace: "default", + Config: &egv1a1.EnvoyProxy{ + Spec: egv1a1.EnvoyProxySpec{ + Logging: egv1a1.ProxyLogging{ + Level: map[egv1a1.ProxyLogComponent]egv1a1.LogLevel{ + egv1a1.LogComponentDefault: egv1a1.LogLevelInfo, + }, + }, + }, + }, + }, + } + + hashedName := utils.GetHashedName("test-proxy", 64) + t.Cleanup(func() { infra.stopEnvoy(hashedName) }) + + err := infra.CreateOrUpdateProxyInfra(t.Context(), infraIR) + require.NoError(t, err) + + // Verify proxy context was stored + _, loaded := infra.proxyContextMap.Load(hashedName) + require.True(t, loaded, "proxy should be loaded after creation") + }) + + t.Run("idempotent - proxy already exists", func(t *testing.T) { + infraIR := &ir.Infra{ + Proxy: &ir.ProxyInfra{ + Name: "test-proxy-idempotent", + Namespace: "default", + Config: &egv1a1.EnvoyProxy{ + Spec: egv1a1.EnvoyProxySpec{ + Logging: egv1a1.ProxyLogging{ + Level: map[egv1a1.ProxyLogComponent]egv1a1.LogLevel{ + egv1a1.LogComponentDefault: egv1a1.LogLevelInfo, + }, + }, + }, + }, + }, + } + + hashedName := utils.GetHashedName("test-proxy-idempotent", 64) + t.Cleanup(func() { infra.stopEnvoy(hashedName) }) + + // First call creates the proxy + err := infra.CreateOrUpdateProxyInfra(t.Context(), infraIR) + require.NoError(t, err) + + _, loaded := infra.proxyContextMap.Load(hashedName) + require.True(t, loaded, "proxy should be loaded after first call") + + // Second call should be idempotent (early return without error) + err = infra.CreateOrUpdateProxyInfra(t.Context(), infraIR) + require.NoError(t, err) + + // Verify proxy is still loaded and wasn't recreated + _, loaded = infra.proxyContextMap.Load(hashedName) + require.True(t, loaded, "proxy should still be loaded after second call") + }) + testCases := []struct { - name string - expect bool - infra *ir.Infra + name string + infra *ir.Infra + expectedError string }{ { - name: "nil cfg", - expect: false, - infra: nil, + name: "nil cfg", + infra: nil, + expectedError: "infra ir is nil", }, { - name: "nil proxy", - expect: false, + name: "nil proxy", infra: &ir.Infra{ Proxy: nil, }, + expectedError: "infra proxy ir is nil", }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err = infra.CreateOrUpdateProxyInfra(t.Context(), tc.infra) - if tc.expect { - require.NoError(t, err) - } else { - require.Error(t, err) - } + actual := infra.CreateOrUpdateProxyInfra(t.Context(), tc.infra) + require.EqualError(t, actual, tc.expectedError) }) } + + t.Run("invalid_bootstrap_config", func(t *testing.T) { + infraIR := &ir.Infra{ + Proxy: &ir.ProxyInfra{ + Name: "test-proxy-invalid", + Namespace: "default", + Config: &egv1a1.EnvoyProxy{ + Spec: egv1a1.EnvoyProxySpec{ + Bootstrap: &egv1a1.ProxyBootstrap{ + Type: ptr.To(egv1a1.BootstrapTypeMerge), + // Invalid YAML that will cause bootstrap merge to fail + Value: ptr.To("invalid: yaml: [unclosed"), + }, + }, + }, + }, + } + + err := infra.CreateOrUpdateProxyInfra(t.Context(), infraIR) + require.Error(t, err) + }) +} + +func TestInfra_DeleteProxyInfra(t *testing.T) { + cfg, err := config.New(io.Discard, io.Discard) + require.NoError(t, err) + infra := newMockInfra(t, cfg) + + t.Run("delete existing proxy", func(t *testing.T) { + infraIR := &ir.Infra{ + Proxy: &ir.ProxyInfra{ + Name: "test-proxy-delete", + Namespace: "default", + Config: &egv1a1.EnvoyProxy{ + Spec: egv1a1.EnvoyProxySpec{ + Logging: egv1a1.ProxyLogging{ + Level: map[egv1a1.ProxyLogComponent]egv1a1.LogLevel{ + egv1a1.LogComponentDefault: egv1a1.LogLevelInfo, + }, + }, + }, + }, + }, + } + + // Create a proxy first + err := infra.CreateOrUpdateProxyInfra(t.Context(), infraIR) + require.NoError(t, err) + + hashedName := utils.GetHashedName("test-proxy-delete", 64) + t.Cleanup(func() { infra.stopEnvoy(hashedName) }) + + _, loaded := infra.proxyContextMap.Load(hashedName) + require.True(t, loaded, "proxy should be loaded before deletion") + + // Delete the proxy + err = infra.DeleteProxyInfra(t.Context(), infraIR) + require.NoError(t, err) + + // Verify deletion + _, loaded = infra.proxyContextMap.Load(hashedName) + require.False(t, loaded, "proxy should be removed after deletion") + }) + + t.Run("delete non-existent proxy", func(t *testing.T) { + infraIR := &ir.Infra{ + Proxy: &ir.ProxyInfra{ + Name: "non-existent-proxy", + Namespace: "default", + Config: &egv1a1.EnvoyProxy{}, + }, + } + + // Delete a proxy that was never created - should not error + err := infra.DeleteProxyInfra(t.Context(), infraIR) + require.NoError(t, err) + }) + + t.Run("nil infra", func(t *testing.T) { + err := infra.DeleteProxyInfra(t.Context(), nil) + require.EqualError(t, err, "infra ir is nil") + }) } func TestExtractSemver(t *testing.T) { @@ -126,8 +278,8 @@ func TestExtractSemver(t *testing.T) { } } -// TestInfra_runEnvoy verifies Envoy process lifecycle, output redirection, and XDG directory usage. -func TestInfra_runEnvoy(t *testing.T) { +// TestInfra_runEnvoy_integration verifies Envoy process lifecycle, output redirection, and XDG directory usage. +func TestInfra_runEnvoy_integration(t *testing.T) { // Create separate XDG directories baseDir := t.TempDir() configHome := path.Join(baseDir, "config") @@ -136,31 +288,39 @@ func TestInfra_runEnvoy(t *testing.T) { runtimeDir := path.Join(baseDir, "runtime") // Create separate buffers for stdout and stderr - buffers := utils.DumpLogsOnFail(t, "stdout", "stderr") + buffers := testutils.DumpLogsOnFail(t, "stdout", "stderr") stdout := buffers[0] stderr := buffers[1] - paths := &Paths{ - ConfigHome: configHome, - DataHome: dataHome, - StateHome: stateHome, - RuntimeDir: runtimeDir, - } - i := &Infra{ - proxyContextMap: make(map[string]*proxyContext), - Paths: paths, - Logger: logging.DefaultLogger(stdout, egv1a1.LogLevelInfo), - Stdout: stdout, - Stderr: stderr, + // Create config with custom paths + cfg, err := config.New(stdout, stderr) + require.NoError(t, err) + cfg.EnvoyGateway.Provider = &egv1a1.EnvoyGatewayProvider{ + Type: egv1a1.ProviderTypeCustom, + Custom: &egv1a1.EnvoyGatewayCustomProvider{ + Infrastructure: &egv1a1.EnvoyGatewayInfrastructureProvider{ + Type: egv1a1.InfrastructureProviderTypeHost, + Host: &egv1a1.EnvoyGatewayHostInfrastructureProvider{ + ConfigHome: ptr.To(configHome), + DataHome: ptr.To(dataHome), + StateHome: ptr.To(stateHome), + RuntimeDir: ptr.To(runtimeDir), + }, + }, + }, } + i, err := NewInfra(t.Context(), cfg, logging.DefaultLogger(stdout, egv1a1.LogLevelInfo)) + require.NoError(t, err) + // Run envoy once to let func-e set up all XDG directories args := []string{ "--config-yaml", - "admin: {address: {socket_address: {address: '127.0.0.1', port_value: 9901}}}", + "admin: {address: {socket_address: {address: '127.0.0.1', port_value: 0}}}", } i.runEnvoy(t.Context(), "", "test", args) - require.Len(t, i.proxyContextMap, 1) + _, ok := i.proxyContextMap.Load("test") + require.True(t, ok, "expected proxy context to be stored") // Wait for func-e to create all XDG directories require.Eventually(t, func() bool { @@ -169,7 +329,8 @@ func TestInfra_runEnvoy(t *testing.T) { }, 5*time.Second, 100*time.Millisecond, "envoy-version file should be created in configHome") i.stopEnvoy("test") - require.Empty(t, i.proxyContextMap) + _, ok = i.proxyContextMap.Load("test") + require.False(t, ok, "expected proxy context to be removed") t.Run("xdg_directory_state", func(t *testing.T) { // Verify XDG directories were created at configured paths by func-e @@ -189,11 +350,6 @@ func TestInfra_runEnvoy(t *testing.T) { // RuntimeDir must exist - func-e creates runID subdirectories with admin-address.txt require.DirExists(t, runtimeDir, "runtimeDir should exist at configured path") - - // Verify each XDG directory is separate (not the same path) - require.NotEqual(t, configHome, dataHome, "configHome and dataHome must be different") - require.NotEqual(t, dataHome, stateHome, "dataHome and stateHome must be different") - require.NotEqual(t, stateHome, runtimeDir, "stateHome and runtimeDir must be different") }) t.Run("output_redirection", func(t *testing.T) { @@ -201,21 +357,66 @@ func TestInfra_runEnvoy(t *testing.T) { totalOutput := stdout.Len() + stderr.Len() require.Positive(t, totalOutput, "expected some output to be captured in stdout or stderr buffers") }) +} - t.Run("stop_start_cycle", func(t *testing.T) { - // Ensures that run -> stop cycle works multiple times without issues - for range 5 { - args := []string{ - "--config-yaml", - "admin: {address: {socket_address: {address: '127.0.0.1', port_value: 9901}}}", - } - i.runEnvoy(t.Context(), "", "test", args) - require.Len(t, i.proxyContextMap, 1) - i.stopEnvoy("test") - require.Empty(t, i.proxyContextMap) - // If the cleanup didn't work, the error due to "address already in use" will be - // tried to be written to the nil logger, which will panic. +func TestInfra_StopStartCycle(t *testing.T) { + cfg, err := config.New(io.Discard, io.Discard) + require.NoError(t, err) + + // Use mock infra with fake runner - no actual Envoy process + infra := newMockInfra(t, cfg) + + // Verify concurrent run -> stop cycles work without races + synctest.Test(t, func(t *testing.T) { + for i := range 5 { + go func(id int) { + name := utils.GetHashedName(fmt.Sprintf("test-%d", id), 64) + infra.runEnvoy(t.Context(), "", name, []string{"--version"}) + _, ok := infra.proxyContextMap.Load(name) + require.True(t, ok, "expected proxy context to be stored") + + infra.stopEnvoy(name) + _, ok = infra.proxyContextMap.Load(name) + require.False(t, ok, "expected proxy context to be removed") + }(i) + } + }) +} + +func TestInfra_Close(t *testing.T) { + cfg, err := config.New(io.Discard, io.Discard) + require.NoError(t, err) + + infra := newMockInfra(t, cfg) + + synctest.Test(t, func(t *testing.T) { + for id := range 5 { + name := utils.GetHashedName(fmt.Sprintf("proxy-%d", id), 64) + infra.runEnvoy(t.Context(), "", name, []string{"--version"}) } + + // Wait for all runEnvoy calls to complete + synctest.Wait() + + // Verify all proxies are running + count := 0 + infra.proxyContextMap.Range(func(key, value any) bool { + count++ + return true + }) + require.Equal(t, 5, count, "expected 5 proxies to be running") + + // Close should stop all proxies concurrently + err := infra.Close() + require.NoError(t, err) + + // Verify all proxies were stopped + count = 0 + infra.proxyContextMap.Range(func(key, value any) bool { + count++ + return true + }) + require.Equal(t, 0, count, "expected all proxies to be stopped") }) } @@ -301,6 +502,24 @@ func TestGetEnvoyVersion(t *testing.T) { // TestTopologyInjectorDisabledInHostMode verifies we don't cause a 15+ second // startup delay in standalone mode as Envoy waits for endpoint discovery. // See: https://github.com/envoyproxy/gateway/issues/7080 +func TestNewInfra(t *testing.T) { + // This test verifies successful creation of Infra. + cfg, err := config.New(io.Discard, io.Discard) + require.NoError(t, err) + + actual, err := NewInfra(t.Context(), cfg, logging.DefaultLogger(io.Discard, egv1a1.LogLevelInfo)) + require.NoError(t, err) + require.NotNil(t, actual) + require.NotNil(t, actual.Paths) + require.NotEmpty(t, actual.sdsConfigPath) + require.NotNil(t, actual.Logger) + require.NotNil(t, actual.EnvoyGateway) + require.Equal(t, egv1a1.DefaultEnvoyProxyImage, actual.defaultEnvoyImage) + require.NotNil(t, actual.Stdout) + require.NotNil(t, actual.Stderr) + require.NotNil(t, actual.envoyRunner) +} + func TestTopologyInjectorDisabledInHostMode(t *testing.T) { testCases := []struct { name string diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 9524ec001c..12827a7e33 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -29,6 +29,7 @@ new features: | Added support for Envoy PreconnectPolicy in BackendTrafficPolicy. Added support for binaryData in ConfigMap referenced by HTTPRouteFilter for direct response. Added support PDB for Ratelimit service. + Auto-generates TLS certificates in host mode when they don't exist. bug fixes: | Fixed %ROUTE_KIND% operator to be lower-cased when used by clusterStatName in EnvoyProxy API. @@ -51,6 +52,7 @@ bug fixes: | Fixed an issue in EnvoyPatchPolicy where it didn't match the target Gateway/GatewayClass due to an incorrect name reference. Fixed certificate SAN overlap detection in gateway listeners. Fixed description and translation behavior for PreserveXRequestID + Fixed race condition in proxy context map used in host mode. # Enhancements that improve performance. performance improvements: | From 0c38d536b4cc6c792c9e10794eaf6b9c0a7d5e30 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Thu, 30 Oct 2025 08:53:03 +0800 Subject: [PATCH 2/2] polish Signed-off-by: Adrian Cole --- examples/extension-server/go.mod | 4 ++-- examples/extension-server/go.sum | 2 ++ internal/infrastructure/host/proxy_infra_test.go | 4 +--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/examples/extension-server/go.mod b/examples/extension-server/go.mod index e13f138b87..19f1732584 100644 --- a/examples/extension-server/go.mod +++ b/examples/extension-server/go.mod @@ -4,8 +4,8 @@ go 1.25.3 require ( github.com/envoyproxy/gateway v1.3.1 - github.com/envoyproxy/go-control-plane v0.13.5-0.20250929230642-07d3df27ff4f - github.com/envoyproxy/go-control-plane/envoy v1.35.1-0.20250929230642-07d3df27ff4f + github.com/envoyproxy/go-control-plane v0.13.5-0.20251022160057-de4316c523b7 + github.com/envoyproxy/go-control-plane/envoy v1.35.1-0.20251022160057-de4316c523b7 github.com/urfave/cli/v2 v2.27.7 google.golang.org/grpc v1.76.0 google.golang.org/protobuf v1.36.10 diff --git a/examples/extension-server/go.sum b/examples/extension-server/go.sum index 52cf37d103..5801c3dcdb 100644 --- a/examples/extension-server/go.sum +++ b/examples/extension-server/go.sum @@ -24,8 +24,10 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/envoyproxy/go-control-plane v0.13.5-0.20250929230642-07d3df27ff4f h1:36vvJBe/wXWfD7qrTb1WnbPVPMxNFDfEygztH8wgebw= github.com/envoyproxy/go-control-plane v0.13.5-0.20250929230642-07d3df27ff4f/go.mod h1:PTY7yDlLxB4bW7rEOO7e79uTDr9yXzpuI1QGIDfxEzc= +github.com/envoyproxy/go-control-plane v0.13.5-0.20251022160057-de4316c523b7/go.mod h1:Alz8LEClvR7xKsrq3qzoc4N0guvVNSS8KmSChGYr9hs= github.com/envoyproxy/go-control-plane/envoy v1.35.1-0.20250929230642-07d3df27ff4f h1:4efYrIQgVRwCmwCveby6ck+VpxqzibdOL1Out1rJqqc= github.com/envoyproxy/go-control-plane/envoy v1.35.1-0.20250929230642-07d3df27ff4f/go.mod h1:2LcmvJoXsDSrsGZIxGM0Gah9ykiwTn/kgjyQdnNH8Jc= +github.com/envoyproxy/go-control-plane/envoy v1.35.1-0.20251022160057-de4316c523b7/go.mod h1:ty89S1YCCVruQAm9OtKeEkQLTb+Lkz0k8v9W0Oxsv98= github.com/envoyproxy/protoc-gen-validate v1.2.1 h1:DEo3O99U8j4hBFwbJfrz9VtgcDfUKS7KJ7spH3d86P8= github.com/envoyproxy/protoc-gen-validate v1.2.1/go.mod h1:d/C80l/jxXLdfEIhX1W2TmLfsJ31lvEjwamM4DxlWXU= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= diff --git a/internal/infrastructure/host/proxy_infra_test.go b/internal/infrastructure/host/proxy_infra_test.go index bb8f945137..299db0d915 100644 --- a/internal/infrastructure/host/proxy_infra_test.go +++ b/internal/infrastructure/host/proxy_infra_test.go @@ -389,15 +389,13 @@ func TestInfra_Close(t *testing.T) { infra := newMockInfra(t, cfg) + // Use synctest as runEnvoy internally starts goroutines synctest.Test(t, func(t *testing.T) { for id := range 5 { name := utils.GetHashedName(fmt.Sprintf("proxy-%d", id), 64) infra.runEnvoy(t.Context(), "", name, []string{"--version"}) } - // Wait for all runEnvoy calls to complete - synctest.Wait() - // Verify all proxies are running count := 0 infra.proxyContextMap.Range(func(key, value any) bool {