From 6b58d6866deed74ddef855861c38b166f1036d6a Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 15:04:01 -0700 Subject: [PATCH 01/12] egress: serve more than one credential provider The egress gateway dialed one credential provider and refused any credential URI naming another. Make the provider name, address and server name flags repeatable, dial each provider, and send each URI to the provider its host names. One of each flag behaves as before. Signed-off-by: Max Thompson --- cmd/atenet/internal/router/cmd.go | 6 +-- cmd/atenet/internal/router/config.go | 50 ++++++++++++++----- cmd/atenet/internal/router/config_test.go | 32 ++++++++++++ .../internal/router/egress/credentials.go | 19 +++---- .../router/egress/credentials_test.go | 48 ++++++++++++++++-- cmd/atenet/internal/router/egress/egress.go | 24 ++++----- .../internal/router/egress/egress_test.go | 8 +-- .../internal/router/egress/request_test.go | 8 +-- .../router/extproc/egress_websocket_test.go | 2 +- cmd/atenet/internal/router/router.go | 23 +++++---- 10 files changed, 157 insertions(+), 63 deletions(-) diff --git a/cmd/atenet/internal/router/cmd.go b/cmd/atenet/internal/router/cmd.go index 42ccd65da9..5908d84c40 100644 --- a/cmd/atenet/internal/router/cmd.go +++ b/cmd/atenet/internal/router/cmd.go @@ -86,11 +86,11 @@ func bindRouterFlags(fs *pflag.FlagSet, cfg *routerConfig) { // and only when injection is enabled: with an empty // --credential-provider-address, a request matching an EgressPolicy rule // that requires injection is denied. - fs.StringVar(&cfg.CredentialProvider.Name, "credential-provider-name", "", "Credential provider this egress gateway serves, as the host of its ate-secret:// credential URIs (e.g. k8s.io); a policy credential URI naming any other provider is refused. Empty disables the check (dev only)") - fs.StringVar(&cfg.CredentialProvider.Address, "credential-provider-address", "", "gRPC dial target of the credential provider the MITM-leg injector resolves secrets through. Empty (the default) disables egress credential injection, and requests matching a rule that requires it are denied") + fs.StringArrayVar(&cfg.CredentialProvider.Names, "credential-provider-name", nil, "Credential provider this egress gateway serves, as the host of its ate-secret:// credential URIs (e.g. k8s.io). Repeat once per --credential-provider-address, in the same order; a policy credential URI naming no configured provider is refused. A single empty name disables the check (dev only)") + fs.StringArrayVar(&cfg.CredentialProvider.Addresses, "credential-provider-address", nil, "gRPC dial target of a credential provider the MITM-leg injector resolves secrets through. Repeat once per provider. None (the default) disables egress credential injection, and requests matching a rule that requires it are denied") fs.StringVar(&cfg.CredentialProvider.CAFile, "credential-provider-ca-file", "", "CA the credential provider's serving certificate must chain to; required unless --credential-provider-insecure is set") fs.StringVar(&cfg.CredentialProvider.ClientCert, "credential-provider-client-cert", "", "Credential bundle presented to the credential provider as the client certificate; required unless --credential-provider-insecure is set") - fs.StringVar(&cfg.CredentialProvider.ServerName, "credential-provider-server-name", "", "SAN/SNI expected on the credential provider's serving certificate") + fs.StringArrayVar(&cfg.CredentialProvider.ServerNames, "credential-provider-server-name", nil, "SAN/SNI expected on a credential provider's serving certificate. Repeat once per --credential-provider-address, in the same order, or omit") fs.BoolVar(&cfg.CredentialProvider.Insecure, "credential-provider-insecure", false, "Dial the credential provider WITHOUT TLS. Development only: secrets cross the network in the clear. Without this, a missing --credential-provider-ca-file or --credential-provider-client-cert fails startup instead of silently downgrading") // Envoy learns the collector over xDS rather than from its own environment, diff --git a/cmd/atenet/internal/router/config.go b/cmd/atenet/internal/router/config.go index 8d3fe2cfde..c92f10b47f 100644 --- a/cmd/atenet/internal/router/config.go +++ b/cmd/atenet/internal/router/config.go @@ -16,6 +16,7 @@ package router import ( "fmt" + "slices" "time" "github.com/spiffe/go-spiffe/v2/spiffeid" @@ -87,29 +88,49 @@ type authConfig struct { AteapiServerName string } -// credentialProviderConfig holds the egress gateway's connection to the -// credential provider the MITM-leg injection resolves secrets through. A -// non-empty Address enables injection; the rest configures the mTLS the gateway -// dials the provider with. See cmd/atenet/internal/router/egress. +// credentialProviderConfig holds the egress gateway's connections to the +// credential providers the MITM-leg injection resolves secrets through. Names, +// Addresses and ServerNames pair up by position; the rest configures the mTLS +// the gateway dials every provider with. See cmd/atenet/internal/router/egress. type credentialProviderConfig struct { - // Name is the provider this gateway serves, as a ate-secret:// - // prefix; a policy credential URI naming any other provider is refused. - // Empty disables the check (dev only). - Name string - // Address is the provider's gRPC dial target. Empty disables injection. - Address string + // Names are the providers this gateway serves, as ate-secret:// + // prefixes; a policy credential URI naming any other provider is refused. + // A lone empty name disables the check (dev only). + Names []string + // Addresses are the providers' gRPC dial targets. None disables injection. + Addresses []string // CAFile is the CA the provider's serving certificate must chain to. // Required unless Insecure is set. CAFile string // ClientCert is the credential bundle presented to the provider as the // client certificate. Required unless Insecure is set. ClientCert string - // ServerName is the SAN/SNI expected on the provider's serving certificate. - ServerName string + // ServerNames are the SAN/SNI expected on each provider's serving + // certificate. Empty leaves them all unset. + ServerNames []string // Insecure dials the provider without TLS; explicit development-only opt-in. Insecure bool } +// validate checks that the repeated provider flags pair up. +func (p credentialProviderConfig) validate() error { + if len(p.Names) != len(p.Addresses) { + return fmt.Errorf("got %d --credential-provider-name for %d --credential-provider-address, want one per address", len(p.Names), len(p.Addresses)) + } + if len(p.ServerNames) != 0 && len(p.ServerNames) != len(p.Addresses) { + return fmt.Errorf("got %d --credential-provider-server-name for %d --credential-provider-address, want one per address or none", len(p.ServerNames), len(p.Addresses)) + } + for i, name := range p.Names { + if name == "" && len(p.Names) > 1 { + return fmt.Errorf("an empty --credential-provider-name serves every credential URI, so it must be the only provider") + } + if slices.Contains(p.Names[:i], name) { + return fmt.Errorf("--credential-provider-name %q repeats", name) + } + } + return nil +} + // routerConfig holds deployment setup and endpoint options for the router node instance. type routerConfig struct { // Mode is the one traffic direction this instance serves. Required. @@ -157,7 +178,7 @@ type routerConfig struct { // CredentialProvider configures egress credential injection on the MITM leg. // Only the egress gateway sets it, and only when injection is enabled: an - // empty CredentialProvider.Address leaves the injector disabled, so a rule + // CredentialProvider with no Addresses leaves the injector disabled, so a rule // that requires an injection is skipped and the request passes through // without the credential. See egress.Handler. CredentialProvider credentialProviderConfig @@ -281,6 +302,9 @@ func (c routerConfig) validate() error { if err := c.validateIngressAuth(); err != nil { return err } + if err := c.CredentialProvider.validate(); err != nil { + return err + } if c.ExtProcMaxRequests < 0 { return fmt.Errorf("--extproc-max-requests must not be negative, got %d (0 derives it from --parked-request-max)", c.ExtProcMaxRequests) diff --git a/cmd/atenet/internal/router/config_test.go b/cmd/atenet/internal/router/config_test.go index 43744e6e04..371e494bce 100644 --- a/cmd/atenet/internal/router/config_test.go +++ b/cmd/atenet/internal/router/config_test.go @@ -47,6 +47,38 @@ func TestRouterConfigValidate(t *testing.T) { cfg: routerConfig{Mode: ModeIngress, AtenetRouter: "blah"}, wantErr: "--atenet-dataplane must be", }, + { + name: "credential providers paired by position are valid", + cfg: routerConfig{Mode: ModeEgress, CredentialProvider: credentialProviderConfig{ + Names: []string{"k8s.io", "sts.example.com"}, Addresses: []string{"a:1", "b:1"}, ServerNames: []string{"a", "b"}, + }}, + }, + { + name: "credential provider name without an address rejected", + cfg: routerConfig{Mode: ModeEgress, CredentialProvider: credentialProviderConfig{Names: []string{"k8s.io"}}}, + wantErr: "want one per address", + }, + { + name: "credential provider server names not paired rejected", + cfg: routerConfig{Mode: ModeEgress, CredentialProvider: credentialProviderConfig{ + Names: []string{"k8s.io", "sts.example.com"}, Addresses: []string{"a:1", "b:1"}, ServerNames: []string{"a"}, + }}, + wantErr: "want one per address or none", + }, + { + name: "repeated credential provider name rejected", + cfg: routerConfig{Mode: ModeEgress, CredentialProvider: credentialProviderConfig{ + Names: []string{"k8s.io", "k8s.io"}, Addresses: []string{"a:1", "b:1"}, + }}, + wantErr: `"k8s.io" repeats`, + }, + { + name: "empty credential provider name next to another rejected", + cfg: routerConfig{Mode: ModeEgress, CredentialProvider: credentialProviderConfig{ + Names: []string{"", "k8s.io"}, Addresses: []string{"a:1", "b:1"}, + }}, + wantErr: "must be the only provider", + }, { name: "negative extproc-max-requests rejected", cfg: routerConfig{Mode: ModeIngress, ExtProcMaxRequests: -1, ParkedRequest: ingress.ParkedRequestConfig{Max: 0}}, diff --git a/cmd/atenet/internal/router/egress/credentials.go b/cmd/atenet/internal/router/egress/credentials.go index 4814a155ad..68be8151c2 100644 --- a/cmd/atenet/internal/router/egress/credentials.go +++ b/cmd/atenet/internal/router/egress/credentials.go @@ -18,6 +18,8 @@ import ( "context" "fmt" "log/slog" + "maps" + "slices" "strings" "time" @@ -139,34 +141,33 @@ func (h *Handler) actorJWT(ctx context.Context, ref resources.ActorRef, dest egr // providerCredential fetches the secret uri names from the credential // provider. Every error it returns is already a client-facing denial. func (h *Handler) providerCredential(ctx context.Context, ref resources.ActorRef, dest egresspolicy.Destination, uri string) ([]byte, error) { - if h.provider == nil { + if len(h.providers) == 0 { slog.ErrorContext(ctx, "egress denied: policy requires credential injection but no credential provider is configured", slog.Any("actor", ref), slog.String("host", dest.Hostname)) return nil, extproc.NewReqError(envoy_type.StatusCode_InternalServerError, deniedBody) } - // Confirm the credential URI names the provider this gateway serves - // before dialing: the configured connection fronts one provider, so a URI - // naming another cannot be resolved here and must fail closed rather than - // be sent to the wrong provider. - if h.providerName != "" { + // Send the URI to the provider its host names. A URI naming no configured + // provider fails closed rather than going to the wrong one. + provider, ok := h.providers[""] + if !ok { name, err := providerNameFromURI(uri) if err != nil { slog.ErrorContext(ctx, "egress denied: policy names an unparseable credential URI", slog.Any("actor", ref), slog.String("host", dest.Hostname), slog.String("uri", uri), slog.Any("err", err)) return nil, extproc.WrapReqError(envoy_type.StatusCode_InternalServerError, err, deniedBody) } - if name != h.providerName { + if provider, ok = h.providers[name]; !ok { slog.ErrorContext(ctx, "egress denied: credential URI names a provider this gateway does not serve", slog.Any("actor", ref), slog.String("host", dest.Hostname), slog.String("uri", uri), - slog.String("provider", name), slog.String("serves", h.providerName)) + slog.String("provider", name), slog.Any("serves", slices.Sorted(maps.Keys(h.providers)))) return nil, extproc.NewReqError(envoy_type.StatusCode_InternalServerError, deniedBody) } } // Atunnel connected to us with a worker-for-actor SPIFFE ID; translate it // to a pure actor SPIFFE ID for plugins to make decisions on. - resp, err := h.provider.FetchSecret(ctx, &credprovider.FetchSecretRequest{ + resp, err := provider.FetchSecret(ctx, &credprovider.FetchSecretRequest{ Uri: uri, ActorSpiffeId: resources.ActorSPIFFEID(ref).String(), }) diff --git a/cmd/atenet/internal/router/egress/credentials_test.go b/cmd/atenet/internal/router/egress/credentials_test.go index b69fb4af84..ddc43c7c0f 100644 --- a/cmd/atenet/internal/router/egress/credentials_test.go +++ b/cmd/atenet/internal/router/egress/credentials_test.go @@ -18,6 +18,7 @@ import ( "cmp" "context" "errors" + "maps" "testing" "testing/synctest" "time" @@ -57,7 +58,11 @@ func bearerTokenResponse(token string) *credprovider.FetchSecretResponse { } func injectionHandlerFor(policy *ateapi.EgressPolicy, provider credprovider.CredentialProviderClient, providerName string) *Handler { - return New(&egressMockClient{actor: runningActor(), policy: policy}, nil, 0, provider, providerName) + var providers map[string]credprovider.CredentialProviderClient + if provider != nil { + providers = map[string]credprovider.CredentialProviderClient{providerName: provider} + } + return New(&egressMockClient{actor: runningActor(), policy: policy}, nil, 0, providers) } // withAuthorization adds the placeholder authorization header the sample @@ -113,6 +118,41 @@ func TestInjectionOnRequestLegs(t *testing.T) { } } +func TestInjectionPicksProviderByName(t *testing.T) { + policy := &ateapi.EgressPolicy{Rules: []*ateapi.EgressRule{{Https: &ateapi.HttpsRule{ + Hostnames: []string{"api.example.com"}, + Effects: &ateapi.HttpRuleEffects{ReplaceHeaders: []*ateapi.CredentialHeader{ + {Header: "authorization", Prefix: "Bearer ", CredentialUri: "ate-secret://k8s/default/token"}, + {Header: "x-api-key", CredentialUri: "ate-secret://vault/team/key"}, + }}, + }}}} + k8s := &fakeProvider{resp: bearerTokenResponse("from-k8s")} + vault := &fakeProvider{resp: bearerTokenResponse("from-vault")} + h := New(&egressMockClient{actor: runningActor(), policy: policy}, nil, 0, + map[string]credprovider.CredentialProviderClient{"k8s": k8s, "vault": vault}) + + md := withAuthorization(innerMetadata(extproc.EgressTLSMITMFilterChainName, "GET", "api.example.com", nil)) + md.Headers["x-api-key"] = "placeholder" + res, err := h.HandleRequestHeaders(context.Background(), md) + if err != nil { + t.Fatalf("HandleRequestHeaders: %v", err) + } + got := map[string]string{} + for _, hv := range res.Response.GetResponse().GetHeaderMutation().GetSetHeaders() { + got[hv.GetHeader().GetKey()] = string(hv.GetHeader().GetRawValue()) + } + want := map[string]string{"authorization": "Bearer from-k8s", "x-api-key": "from-vault"} + if !maps.Equal(got, want) { + t.Errorf("injected headers = %v, want %v", got, want) + } + if uri := k8s.got.GetUri(); uri != "ate-secret://k8s/default/token" { + t.Errorf("k8s provider got URI %q", uri) + } + if uri := vault.got.GetUri(); uri != "ate-secret://vault/team/key" { + t.Errorf("vault provider got URI %q", uri) + } +} + // A request that does not carry the header a rule replaces goes out unchanged: // nothing is fetched, added, or denied, even where injecting would fail. func TestInjectionSkippedWithoutHeader(t *testing.T) { @@ -312,7 +352,7 @@ func TestActorJWTInjection(t *testing.T) { actorJWTHeader("authorization"), &ateapi.CredentialHeader{Header: "x-api-key", CredentialUri: "ate-secret://k8s/default/token"}, )} - h := New(client, nil, 0, nil, "") + h := New(client, nil, 0, nil) res, err := h.HandleRequestHeaders(context.Background(), withAuthorization(innerMetadata(leg, "GET", "api.example.com", nil))) @@ -347,7 +387,7 @@ func TestActorJWTInjection(t *testing.T) { func TestActorJWTInjectionSkippedWithoutHeader(t *testing.T) { client := &egressMockClient{actor: runningActor(), policy: replaceHeadersPolicy(extproc.EgressTLSMITMFilterChainName, actorJWTHeader("authorization"))} - h := New(client, nil, 0, nil, "") + h := New(client, nil, 0, nil) res, err := h.HandleRequestHeaders(context.Background(), innerMetadata(extproc.EgressTLSMITMFilterChainName, "GET", "api.example.com", nil)) if err != nil { @@ -385,7 +425,7 @@ func TestActorJWTInjectionDenials(t *testing.T) { policy: replaceHeadersPolicy(leg, actorJWTHeader(header)), mintErr: tc.mintErr, } - h := New(mock, nil, 0, nil, "") + h := New(mock, nil, 0, nil) md := innerMetadata(leg, "GET", "api.example.com", nil) md.Headers[header] = "placeholder" _, err := h.HandleRequestHeaders(context.Background(), md) diff --git a/cmd/atenet/internal/router/egress/egress.go b/cmd/atenet/internal/router/egress/egress.go index 9e8de155ff..35fd25e451 100644 --- a/cmd/atenet/internal/router/egress/egress.go +++ b/cmd/atenet/internal/router/egress/egress.go @@ -85,13 +85,8 @@ type Handler struct { policies *policyCache // actorJWTs mints the actor JWTs that credential injections ask for. actorJWTs *actorjwt.Minter - // provider resolves an egress policy's credential_uri injections. Nil - // means none are configured, and a request that needs one is denied. - provider credprovider.CredentialProviderClient - // providerName, when set, is the provider this gateway serves (the host of - // its ate-secret:// credential URIs); a credential URI naming another - // provider is refused. - providerName string + // providers resolve credential_uri injections by provider name; see New. + providers map[string]credprovider.CredentialProviderClient } // New builds the egress handler. loadActorIdentityRoots yields the egress @@ -100,12 +95,12 @@ type Handler struct { // the next tunnel. A nil loadActorIdentityRoots fails every CONNECT with 503. // policyCacheTTL of 0 fetches the policy on every callout. // -// provider resolves an allowed rule's credential_uri injections on the -// TLS-terminated MITM leg; nil leaves them off, so a request that needs one is -// denied. Actor JWT injections need no provider. providerName, when set, is -// the provider this gateway serves; a credential URI naming another provider -// is refused. -func New(apiClient ateapi.ControlClient, loadActorIdentityRoots func() (*x509.CertPool, error), policyCacheTTL time.Duration, provider credprovider.CredentialProviderClient, providerName string) *Handler { +// providers resolve an allowed rule's credential_uri injections on the +// TLS-terminated MITM leg, keyed by provider name. A credential URI goes to the +// provider its host names, and one naming no provider is refused; a lone "" +// key serves every URI. No providers leaves injections off, so a request that +// needs one is denied. Actor JWT injections need no provider. +func New(apiClient ateapi.ControlClient, loadActorIdentityRoots func() (*x509.CertPool, error), policyCacheTTL time.Duration, providers map[string]credprovider.CredentialProviderClient) *Handler { if loadActorIdentityRoots == nil { loadActorIdentityRoots = func() (*x509.CertPool, error) { return nil, errors.New("no actor-identity CA configured") @@ -116,8 +111,7 @@ func New(apiClient ateapi.ControlClient, loadActorIdentityRoots func() (*x509.Ce loadActorIdentityRoots: loadActorIdentityRoots, policies: newPolicyCache(apiClient, policyCacheTTL), actorJWTs: actorjwt.New(apiClient), - provider: provider, - providerName: providerName, + providers: providers, } } diff --git a/cmd/atenet/internal/router/egress/egress_test.go b/cmd/atenet/internal/router/egress/egress_test.go index 79d475cb83..703db9034e 100644 --- a/cmd/atenet/internal/router/egress/egress_test.go +++ b/cmd/atenet/internal/router/egress/egress_test.go @@ -167,7 +167,7 @@ func xfccHeaderDER(chain ...[]byte) string { // egressHandler builds a Handler with an allow-everything policy, so the // identity tests are about identity alone. func egressHandler(loadRoots func() (*x509.CertPool, error), actor *ateapi.Actor, err error) *Handler { - return New(&egressMockClient{actor: actor, err: err, policy: allowAllPolicy()}, loadRoots, DefaultPolicyCacheTTL, nil, "") + return New(&egressMockClient{actor: actor, err: err, policy: allowAllPolicy()}, loadRoots, DefaultPolicyCacheTTL, nil) } // egressMockClient is the slice of ateapi the egress handler talks to. @@ -409,7 +409,7 @@ func TestConnectLegOpensForAnyRules(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - h := New(&egressMockClient{actor: runningActor(), policy: tc.policy}, ca.loadRoots, 0, nil, "") + h := New(&egressMockClient{actor: runningActor(), policy: tc.policy}, ca.loadRoots, 0, nil) md := egressMetadata(xfccHeader(leaf)) md.Host = "93.184.216.34:443" if tc.dialed != "" { @@ -468,7 +468,7 @@ func TestConnectLegWithoutRequestLegs(t *testing.T) { leaf := ca.issueActorCert(t, "spiffe://substrate-actor.local/worker-for-actor/foo/bar", actorCertOptions{}) certificate := string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: leaf.Raw})) - h := New(&egressMockClient{actor: runningActor(), policy: httpPolicy("api.example.com")}, ca.loadRoots, 0, nil, "") + h := New(&egressMockClient{actor: runningActor(), policy: httpPolicy("api.example.com")}, ca.loadRoots, 0, nil) res, err := h.HandleRequestHeaders(context.Background(), agentgatewayEgressMetadata(certificate)) if err != nil { t.Fatalf("HandleRequestHeaders() error = %v, want the tunnel to open", err) @@ -476,7 +476,7 @@ func TestConnectLegWithoutRequestLegs(t *testing.T) { if got := dialedPortOf(res); got != "80" { t.Errorf("dialed port = %q, want %q", got, "80") } - h = New(&egressMockClient{actor: runningActor()}, ca.loadRoots, 0, nil, "") + h = New(&egressMockClient{actor: runningActor()}, ca.loadRoots, 0, nil) _, err = h.HandleRequestHeaders(context.Background(), agentgatewayEgressMetadata(certificate)) wantStatus(t, err, envoy_type.StatusCode_Forbidden) } diff --git a/cmd/atenet/internal/router/egress/request_test.go b/cmd/atenet/internal/router/egress/request_test.go index 89d8527717..174bcc899e 100644 --- a/cmd/atenet/internal/router/egress/request_test.go +++ b/cmd/atenet/internal/router/egress/request_test.go @@ -66,7 +66,7 @@ func cleartextInjectionPolicy(pattern string) *ateapi.EgressPolicy { // policyHandler builds a Handler for an actor whose policy is policy (nil // means none) with the cache disabled, so each callout sees the mock as is. func policyHandler(policy *ateapi.EgressPolicy) *Handler { - return New(&egressMockClient{actor: runningActor(), policy: policy}, nil, 0, nil, "") + return New(&egressMockClient{actor: runningActor(), policy: policy}, nil, 0, nil) } // testSNI is the server name the test actor's TLS connection presented, on @@ -420,7 +420,7 @@ func TestRequestLegPolicyLookup(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - h := New(tc.client, nil, 0, nil, "") + h := New(tc.client, nil, 0, nil) _, err := h.HandleRequestHeaders(context.Background(), requestMetadata("api.example.com")) wantStatus(t, err, tc.want) }) @@ -446,7 +446,7 @@ func TestConnectLegRequiresAPolicy(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - h := New(tc.client, ca.loadRoots, DefaultPolicyCacheTTL, nil, "") + h := New(tc.client, ca.loadRoots, DefaultPolicyCacheTTL, nil) res, err := h.HandleRequestHeaders(context.Background(), egressMetadata(xfccHeader(leaf))) if tc.want == 0 { wantAllowed(t, res, err) @@ -503,7 +503,7 @@ func TestDenialBodyIsUniform(t *testing.T) { // A caller that gives up mid-fetch is neither a denial nor an outage. func TestCanceledCallerIsNotAPolicyFailure(t *testing.T) { client := &egressMockClient{policy: allowAllPolicy(), policyGate: make(chan struct{})} - h := New(client, nil, 0, nil, "") + h := New(client, nil, 0, nil) ctx, cancel := context.WithCancel(context.Background()) done := make(chan error, 1) go func() { diff --git a/cmd/atenet/internal/router/extproc/egress_websocket_test.go b/cmd/atenet/internal/router/extproc/egress_websocket_test.go index dc25406859..04596139d5 100644 --- a/cmd/atenet/internal/router/extproc/egress_websocket_test.go +++ b/cmd/atenet/internal/router/extproc/egress_websocket_test.go @@ -74,7 +74,7 @@ func TestProcessReturnsWebSocketDenialAsImmediateResponse(t *testing.T) { } stream := &websocketProcessStream{ctx: context.Background(), requests: []*extprocv3.ProcessingRequest{request}} server := extproc.NewServer(0, nil, extproc.Handlers{ - extproc.DirectionEgress: egress.New(nil, nil, 0, nil, ""), + extproc.DirectionEgress: egress.New(nil, nil, 0, nil), }) if err := server.Process(stream); err != nil { diff --git a/cmd/atenet/internal/router/router.go b/cmd/atenet/internal/router/router.go index cd352ec377..dfa53a1370 100644 --- a/cmd/atenet/internal/router/router.go +++ b/cmd/atenet/internal/router/router.go @@ -220,31 +220,34 @@ func (s *RouterServer) Run(ctx context.Context) error { // is set: dial the provider over mTLS and hand the client to the handler. // With no address the handler gets no provider, and a request matching // an injection-requiring rule is denied (see egress.applyEffects). - var provider credprovider.CredentialProviderClient - var providerName string - if s.cfg.CredentialProvider.Address != "" { - providerName, err = egress.ProviderName(s.cfg.CredentialProvider.Name) + providers := map[string]credprovider.CredentialProviderClient{} + for i, address := range s.cfg.CredentialProvider.Addresses { + providerName, err := egress.ProviderName(s.cfg.CredentialProvider.Names[i]) if err != nil { return fmt.Errorf("--credential-provider-name: %w", err) } + var serverName string + if len(s.cfg.CredentialProvider.ServerNames) > 0 { + serverName = s.cfg.CredentialProvider.ServerNames[i] + } providerConn, err := egress.DialProvider(ctx, egress.ProviderDialConfig{ - Address: s.cfg.CredentialProvider.Address, + Address: address, CAFile: s.cfg.CredentialProvider.CAFile, ClientCert: s.cfg.CredentialProvider.ClientCert, - ServerName: s.cfg.CredentialProvider.ServerName, + ServerName: serverName, Insecure: s.cfg.CredentialProvider.Insecure, }) if err != nil { return fmt.Errorf("dial credential provider: %w", err) } defer providerConn.Close() - provider = credprovider.NewCredentialProviderClient(providerConn) + providers[providerName] = credprovider.NewCredentialProviderClient(providerConn) slog.InfoContext(ctx, "egress credential injection enabled", - slog.String("provider", s.cfg.CredentialProvider.Address), - slog.String("provider_name", s.cfg.CredentialProvider.Name)) + slog.String("provider", address), + slog.String("provider_name", providerName)) } - egressHandler := egress.New(s.apiClient, loadActorIdentityRoots, s.cfg.EgressPolicyCacheTTL, provider, providerName) + egressHandler := egress.New(s.apiClient, loadActorIdentityRoots, s.cfg.EgressPolicyCacheTTL, providers) handlers[egressHandler.Direction()] = egressHandler } From 285fcfc2e0bfef4c96dce841f09ff671d50666e5 Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 15:07:43 -0700 Subject: [PATCH 02/12] ate-setup: accept a list of credential providers --credential-provider also takes a JSON array of provider objects. ate-setup renders one set of egress flags, or one agentgateway credentialProviders entry, per provider, and deploys the bundled Kubernetes Secrets provider when k8s.io is among them. Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/config.go | 55 ++++++++++-- cmd/ate-setup/internal/config/config_test.go | 69 ++++++++++++-- cmd/ate-setup/internal/config/errors_test.go | 4 +- cmd/ate-setup/internal/config/load.go | 2 +- cmd/ate-setup/internal/steps/deploy.go | 17 ++-- cmd/ate-setup/internal/steps/overlay.go | 45 ++++++---- cmd/ate-setup/internal/steps/overlay_test.go | 95 ++++++++++++-------- 7 files changed, 210 insertions(+), 77 deletions(-) diff --git a/cmd/ate-setup/internal/config/config.go b/cmd/ate-setup/internal/config/config.go index a44fdf816a..c447f3f514 100644 --- a/cmd/ate-setup/internal/config/config.go +++ b/cmd/ate-setup/internal/config/config.go @@ -25,6 +25,7 @@ import ( "net" "os" "path/filepath" + "slices" "sort" "strconv" "strings" @@ -238,7 +239,7 @@ type Config struct { AdditionalEgressExtprocService string // CredentialProviderJSON is the --credential-provider value; see - // CredentialProvider for its schema. + // CredentialProviders for its schema. CredentialProviderJSON string // AnthropicAPIKey is required only by the claude-code-multiplex demo. @@ -581,7 +582,8 @@ const credentialProviderKey = "atenet.egress.credentialProvider" // credentialProviderUsage lists the accepted --credential-provider values. const credentialProviderUsage = `{"name":"k8s.io"} for the bundled Kubernetes Secrets provider, ` + `{"enabled":false} to turn egress credential injection off, ` + - `or {"name":"","address":":"} for a provider you deploy` + `or {"name":"","address":":"} for a provider you deploy; ` + + `a JSON array of provider objects selects several` // credentialProviderSpec is the schema of the --credential-provider JSON. type credentialProviderSpec struct { @@ -607,6 +609,11 @@ func (p CredentialProvider) Enabled() bool { return p.Name != "" } // Kubernetes reports whether the provider is the bundled one. func (p CredentialProvider) Kubernetes() bool { return p.Name == K8sCredentialProviderName } +// HasKubernetesProvider reports whether providers include the bundled one. +func HasKubernetesProvider(providers []CredentialProvider) bool { + return slices.ContainsFunc(providers, CredentialProvider.Kubernetes) +} + // ServerName is the SAN the gateway expects on the provider's serving // certificate: the address without its port. func (p CredentialProvider) ServerName() string { @@ -616,12 +623,11 @@ func (p CredentialProvider) ServerName() string { return p.Address } -// CredentialProvider parses --credential-provider, a JSON object that either +// credentialProvider parses a --credential-provider JSON object that either // turns injection off with enabled false or names a provider and its address. // Only the bundled provider has a default address. A missing value is reported here rather than in validate // because only the deploys that render the egress gateway need it. -func (c *Config) CredentialProvider() (CredentialProvider, error) { - raw := c.CredentialProviderJSON +func credentialProvider(raw string) (CredentialProvider, error) { if raw == "" { // Every channel is named, not just the flag: a provider set in the // configuration document is the documented way to do it, and a reader @@ -666,3 +672,42 @@ func (c *Config) CredentialProvider() (CredentialProvider, error) { } return CredentialProvider{Name: spec.Name, Address: spec.Address}, nil } + +// CredentialProviders parses --credential-provider: one provider object, which +// credentialProvider reads, or a JSON array of them. None means injection is +// off. +func (c *Config) CredentialProviders() ([]CredentialProvider, error) { + raw := c.CredentialProviderJSON + if !strings.HasPrefix(strings.TrimSpace(raw), "[") { + p, err := credentialProvider(raw) + if err != nil || !p.Enabled() { + return nil, err + } + return []CredentialProvider{p}, nil + } + invalid := func(format string, args ...any) ([]CredentialProvider, error) { + return nil, fmt.Errorf("invalid --credential-provider '%s': %s", raw, fmt.Sprintf(format, args...)) + } + var elems []json.RawMessage + if err := json.Unmarshal([]byte(raw), &elems); err != nil { + return invalid("%v; want %s", err, credentialProviderUsage) + } + if len(elems) == 0 { + return invalid(`the array names no provider; {"enabled":false} turns injection off`) + } + var providers []CredentialProvider + for _, elem := range elems { + p, err := credentialProvider(string(elem)) + if err != nil { + return nil, err + } + if !p.Enabled() { + return invalid("enabled false is not allowed in an array") + } + if slices.ContainsFunc(providers, func(q CredentialProvider) bool { return q.Name == p.Name }) { + return invalid("provider name %q repeats", p.Name) + } + providers = append(providers, p) + } + return providers, nil +} diff --git a/cmd/ate-setup/internal/config/config_test.go b/cmd/ate-setup/internal/config/config_test.go index 5429ece623..274ac78372 100644 --- a/cmd/ate-setup/internal/config/config_test.go +++ b/cmd/ate-setup/internal/config/config_test.go @@ -1059,6 +1059,10 @@ func TestLoadRejectsInvalidValues(t *testing.T) { {"kubernetes address without a port", Options{CredentialProvider: `{"name":"k8s.io","address":"secrets.ate-system.svc"}`}}, {"other provider without an address", Options{CredentialProvider: `{"name":"vault.example.com"}`}}, {"other provider address without a port", Options{CredentialProvider: `{"name":"vault.example.com","address":"vault.ate-system.svc"}`}}, + {"provider array empty", Options{CredentialProvider: `[]`}}, + {"provider array entry disabled", Options{CredentialProvider: `[{"enabled":false}]`}}, + {"provider array with a repeated name", Options{CredentialProvider: `[{"name":"k8s.io"},{"name":"k8s.io"}]`}}, + {"provider array entry without an address", Options{CredentialProvider: `[{"name":"k8s.io"},{"name":"vault.example.com"}]`}}, } { t.Run(tc.name, func(t *testing.T) { if _, err := Load(tc.opts); err == nil { @@ -1069,7 +1073,7 @@ func TestLoadRejectsInvalidValues(t *testing.T) { } // The provider selection is required only where the egress gateway is -// rendered, so Load accepts an absent value and CredentialProvider refuses it. +// rendered, so Load accepts an absent value and CredentialProviders refuses it. func TestCredentialProvider(t *testing.T) { loadEnv(t) @@ -1137,26 +1141,75 @@ func TestCredentialProvider(t *testing.T) { if err != nil { t.Fatalf("Load() error = %v", err) } - got, err := cfg.CredentialProvider() + got, err := cfg.CredentialProviders() if tc.wantErr { if err == nil { - t.Fatal("CredentialProvider() succeeded, want an error") + t.Fatal("CredentialProviders() succeeded, want an error") } return } if err != nil { - t.Fatalf("CredentialProvider() error = %v", err) + t.Fatalf("CredentialProviders() error = %v", err) } - if got != tc.want { - t.Errorf("CredentialProvider() = %+v, want %+v", got, tc.want) + var want []CredentialProvider + if tc.want.Enabled() { + want = []CredentialProvider{tc.want} } - if got.Enabled() != (tc.want.Name != "") { - t.Errorf("Enabled() = %v, want %v", got.Enabled(), tc.want.Name != "") + if !slices.Equal(got, want) { + t.Errorf("CredentialProviders() = %+v, want %+v", got, want) } }) } } +func TestCredentialProvidersArray(t *testing.T) { + loadEnv(t) + k8s := CredentialProvider{Name: K8sCredentialProviderName, Address: K8sCredentialProviderAddress} + for _, tc := range []struct { + name string + raw string + want []CredentialProvider + }{ + {name: "an array of one", raw: `[{"name":"k8s.io"}]`, want: []CredentialProvider{k8s}}, + { + name: "several providers", + raw: `[{"name":"k8s.io"},{"name":"sts.example.com","address":"sts.ate-system.svc:50051"}]`, + want: []CredentialProvider{k8s, {Name: "sts.example.com", Address: "sts.ate-system.svc:50051"}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg, err := Load(Options{CredentialProvider: tc.raw}) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + got, err := cfg.CredentialProviders() + if err != nil { + t.Fatalf("CredentialProviders() error = %v", err) + } + if !slices.Equal(got, tc.want) { + t.Errorf("CredentialProviders() = %+v, want %+v", got, tc.want) + } + }) + } +} + +func TestHasKubernetesProvider(t *testing.T) { + k8s := CredentialProvider{Name: K8sCredentialProviderName} + vault := CredentialProvider{Name: "vault.example.com"} + for _, tc := range []struct { + providers []CredentialProvider + want bool + }{ + {nil, false}, + {[]CredentialProvider{vault}, false}, + {[]CredentialProvider{vault, k8s}, true}, + } { + if got := HasKubernetesProvider(tc.providers); got != tc.want { + t.Errorf("HasKubernetesProvider(%v) = %v, want %v", tc.providers, got, tc.want) + } + } +} + func TestCredentialProviderServerName(t *testing.T) { for addr, want := range map[string]string{ "k8s-credential-provider.ate-system.svc:50051": "k8s-credential-provider.ate-system.svc", diff --git a/cmd/ate-setup/internal/config/errors_test.go b/cmd/ate-setup/internal/config/errors_test.go index aacc43f296..0e59c8349f 100644 --- a/cmd/ate-setup/internal/config/errors_test.go +++ b/cmd/ate-setup/internal/config/errors_test.go @@ -248,9 +248,9 @@ func TestValidationErrorsNameTheSupplyingChannel(t *testing.T) { func TestCredentialProviderErrorNamesTheFileChannel(t *testing.T) { useFixtures(t) cfg := &Config{} - _, err := cfg.CredentialProvider() + _, err := cfg.CredentialProviders() if err == nil { - t.Fatal("CredentialProvider() with nothing set returned no error") + t.Fatal("CredentialProviders() with nothing set returned no error") } for _, want := range []string{ "--credential-provider", diff --git a/cmd/ate-setup/internal/config/load.go b/cmd/ate-setup/internal/config/load.go index 056e4e815b..d585b17bbb 100644 --- a/cmd/ate-setup/internal/config/load.go +++ b/cmd/ate-setup/internal/config/load.go @@ -459,7 +459,7 @@ func validateResolved(cfg *Config, r *Resolved) error { // configuration rather than part-way through a deploy. Whether one is // required at all is the deploy's question, not this one's. if cfg.CredentialProviderJSON != "" { - if _, err := cfg.CredentialProvider(); err != nil { + if _, err := cfg.CredentialProviders(); err != nil { // The parser's own message says which part is wrong and what to // write instead; only the channel is added. Replacing it with the // generic "must be " would drop the reason. diff --git a/cmd/ate-setup/internal/steps/deploy.go b/cmd/ate-setup/internal/steps/deploy.go index bacf30a4e5..68dba51c63 100644 --- a/cmd/ate-setup/internal/steps/deploy.go +++ b/cmd/ate-setup/internal/steps/deploy.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "go.ate.dev/substrate/cmd/ate-setup/internal/config" "go.ate.dev/substrate/cmd/ate-setup/internal/kube" "go.ate.dev/substrate/cmd/ate-setup/internal/log" ) @@ -64,7 +65,7 @@ func (e *Env) DeployAteSystem(ctx context.Context, opts DeployOptions) error { } // Fail fast on an unusable credential provider selection before touching // the cluster. - provider, err := e.Cfg.CredentialProvider() + providers, err := e.Cfg.CredentialProviders() if err != nil { return err } @@ -159,7 +160,7 @@ func (e *Env) DeployAteSystem(ctx context.Context, opts DeployOptions) error { } // After the podcertificate controller: the provider serves with a // projected pod certificate. - if provider.Kubernetes() { + if config.HasKubernetesProvider(providers) { err = e.deployK8sCredentialProvider(ctx) } else { err = e.removeK8sCredentialProvider(ctx) @@ -167,7 +168,7 @@ func (e *Env) DeployAteSystem(ctx context.Context, opts DeployOptions) error { if err != nil { return err } - if err := e.applyAtenetEgress(ctx, provider); err != nil { + if err := e.applyAtenetEgress(ctx, providers); err != nil { return err } @@ -182,7 +183,7 @@ func (e *Env) DeployAteSystem(ctx context.Context, opts DeployOptions) error { rollout{kube.KindDeployment, "atenet-egress"}, rollout{kube.KindDaemonSet, "atelet"}, ) - if provider.Kubernetes() { + if config.HasKubernetesProvider(providers) { waits = append(waits, rollout{kube.KindDeployment, k8sCredentialProviderDeployment}) } for _, w := range waits { @@ -530,7 +531,7 @@ func (e *Env) ateletManifestPath() string { func (e *Env) DeployAtenet(ctx context.Context) error { log.Step("deploy_atenet") - provider, err := e.Cfg.CredentialProvider() + providers, err := e.Cfg.CredentialProviders() if err != nil { return err } @@ -557,7 +558,7 @@ func (e *Env) DeployAtenet(ctx context.Context) error { if err := e.EnsureEgressMITMCAPoolSecret(ctx); err != nil { return err } - if provider.Kubernetes() { + if config.HasKubernetesProvider(providers) { err = e.deployK8sCredentialProvider(ctx) } else { err = e.removeK8sCredentialProvider(ctx) @@ -565,12 +566,12 @@ func (e *Env) DeployAtenet(ctx context.Context) error { if err != nil { return err } - if err := e.applyAtenetEgress(ctx, provider); err != nil { + if err := e.applyAtenetEgress(ctx, providers); err != nil { return err } deployments := []string{"atenet-router", "atenet-egress"} - if provider.Kubernetes() { + if config.HasKubernetesProvider(providers) { deployments = append(deployments, k8sCredentialProviderDeployment) } for _, name := range deployments { diff --git a/cmd/ate-setup/internal/steps/overlay.go b/cmd/ate-setup/internal/steps/overlay.go index 401b5b2b0d..1cf30003c0 100644 --- a/cmd/ate-setup/internal/steps/overlay.go +++ b/cmd/ate-setup/internal/steps/overlay.go @@ -228,7 +228,7 @@ func (e *Env) atenetEgressManifestPath() string { } // renderAtenetEgressManifest produces the atenet egress manifest. -func (e *Env) renderAtenetEgressManifest(ctx context.Context, provider config.CredentialProvider) ([]byte, error) { +func (e *Env) renderAtenetEgressManifest(ctx context.Context, providers []config.CredentialProvider) ([]byte, error) { general := e.Cfg.AdditionalEgressExtprocService != "" if e.Cfg.Router == config.RouterAgentgateway { @@ -246,7 +246,7 @@ func (e *Env) renderAtenetEgressManifest(ctx context.Context, provider config.Cr if err != nil { return nil, err } - raw, err = patchAgentgatewayEgressInject(raw, provider) + raw, err = patchAgentgatewayEgressInject(raw, providers) if err != nil { return nil, err } @@ -271,7 +271,7 @@ func (e *Env) renderAtenetEgressManifest(ctx context.Context, provider config.Cr if err != nil { return nil, err } - raw, err = e.patchAtenetEgressInject(raw, provider) + raw, err = e.patchAtenetEgressInject(raw, providers) if err != nil { return nil, err } @@ -360,19 +360,22 @@ func (e *Env) patchEnvoyDataplaneImage(raw []byte, imageRef string) []byte { // egress sidecar's args with the credential-provider flags, or removes it when // injection is off, which leaves the gateway with no provider. It takes the // manifest bytes so it can run after the general patch. -func (e *Env) patchAtenetEgressInject(raw []byte, provider config.CredentialProvider) ([]byte, error) { +func (e *Env) patchAtenetEgressInject(raw []byte, providers []config.CredentialProvider) ([]byte, error) { var flagsBlock string - if provider.Enabled() { - flagsBlock = emitEgressInjectFlags(provider.Name, provider.Address, provider.ServerName()) + if len(providers) > 0 { + flagsBlock = emitEgressInjectFlags(providers) } return replaceManifestMarker(raw, "#ATE_EGRESS_INJECT_FLAGS", flagsBlock) } // The HTTP and HTTPS listeners share the egress policy and its providers. -func patchAgentgatewayEgressInject(raw []byte, provider config.CredentialProvider) ([]byte, error) { +func patchAgentgatewayEgressInject(raw []byte, providers []config.CredentialProvider) ([]byte, error) { var block string - if provider.Enabled() { - block = fmt.Sprintf(`credentialProviders: + for i, p := range providers { + if i == 0 { + block = "credentialProviders:" + } + block += fmt.Sprintf(` - uriAuthority: %q target: host: %q @@ -381,7 +384,7 @@ func patchAgentgatewayEgressInject(raw []byte, provider config.CredentialProvide hostname: %q cert: /run/podidentity.podcert.ate.dev/credential-bundle.pem key: /run/podidentity.podcert.ate.dev/credential-bundle.pem - root: /run/servicedns-ca/trust-bundle.pem`, provider.Name, provider.Address, provider.ServerName()) + root: /run/servicedns-ca/trust-bundle.pem`, p.Name, p.Address, p.ServerName()) } return replaceManifestMarker(raw, "#ATE_AGENTGATEWAY_CREDENTIAL_PROVIDERS", block) } @@ -408,12 +411,18 @@ func replaceManifestMarker(raw []byte, marker, block string) ([]byte, error) { return []byte(strings.Join(out, "\n")), nil } -func emitEgressInjectFlags(name, address, serverName string) string { - return fmt.Sprintf(`- --credential-provider-name=%s +// emitEgressInjectFlags writes the name, address and server name flags of each +// provider in order, then the CA and client certificate every provider shares. +func emitEgressInjectFlags(providers []config.CredentialProvider) string { + var flags string + for _, p := range providers { + flags += fmt.Sprintf(`- --credential-provider-name=%s - --credential-provider-address=%s -- --credential-provider-ca-file=/run/servicedns.podcert.ate.dev/trust-bundle.pem -- --credential-provider-client-cert=/run/podidentity.podcert.ate.dev/credential-bundle.pem -- --credential-provider-server-name=%s`, name, address, serverName) +- --credential-provider-server-name=%s +`, p.Name, p.Address, p.ServerName()) + } + return flags + `- --credential-provider-ca-file=/run/servicedns.podcert.ate.dev/trust-bundle.pem +- --credential-provider-client-cert=/run/podidentity.podcert.ate.dev/credential-bundle.pem` } func (e *Env) patchAtenetEgressManifest() ([]byte, error) { @@ -551,8 +560,8 @@ func emitAdditionalEgressExtprocCluster(address, port, serverName string) string port_value: %s`, additionalEgressExtprocCluster, serverName, serverName, additionalEgressExtprocCluster, address, port) } -func (e *Env) applyAtenetEgress(ctx context.Context, provider config.CredentialProvider) error { - manifests, err := e.renderAtenetEgressManifest(ctx, provider) +func (e *Env) applyAtenetEgress(ctx context.Context, providers []config.CredentialProvider) error { + manifests, err := e.renderAtenetEgressManifest(ctx, providers) if err != nil { return err } @@ -566,7 +575,7 @@ func (e *Env) applyAtenetEgress(ctx context.Context, provider config.CredentialP return err } - if running && (e.Cfg.AdditionalEgressExtprocService != "" || provider.Enabled()) { + if running && (e.Cfg.AdditionalEgressExtprocService != "" || len(providers) > 0) { if err := e.Kube.RolloutRestartDeployment(ctx, e.Namespace(), "atenet-egress", time.Now()); err != nil { return err } diff --git a/cmd/ate-setup/internal/steps/overlay_test.go b/cmd/ate-setup/internal/steps/overlay_test.go index 7de3d99195..d209e6cb00 100644 --- a/cmd/ate-setup/internal/steps/overlay_test.go +++ b/cmd/ate-setup/internal/steps/overlay_test.go @@ -57,14 +57,16 @@ func TestPatchAtenetEgressInject(t *testing.T) { t.Fatalf("reading egress manifest: %v", err) } + k8s := config.CredentialProvider{Name: config.K8sCredentialProviderName, Address: config.K8sCredentialProviderAddress} + vault := config.CredentialProvider{Name: "vault.example.com", Address: "vault.ate-system.svc:8200"} for _, tc := range []struct { - name string - provider config.CredentialProvider - want []string + name string + providers []config.CredentialProvider + want []string }{ { - name: "kubernetes", - provider: config.CredentialProvider{Name: config.K8sCredentialProviderName, Address: config.K8sCredentialProviderAddress}, + name: "kubernetes", + providers: []config.CredentialProvider{k8s}, want: []string{ "--credential-provider-name=k8s.io", "--credential-provider-address=k8s-credential-provider.ate-system.svc:50051", @@ -74,18 +76,31 @@ func TestPatchAtenetEgressInject(t *testing.T) { }, }, { - name: "another provider", - provider: config.CredentialProvider{Name: "vault.example.com", Address: "vault.ate-system.svc:8200"}, + name: "another provider", + providers: []config.CredentialProvider{vault}, + want: []string{ + "--credential-provider-name=vault.example.com", + "--credential-provider-address=vault.ate-system.svc:8200", + "--credential-provider-server-name=vault.ate-system.svc", + }, + }, + { + name: "several providers", + providers: []config.CredentialProvider{k8s, vault}, want: []string{ + "--credential-provider-name=k8s.io", + "--credential-provider-address=k8s-credential-provider.ate-system.svc:50051", + "--credential-provider-server-name=k8s-credential-provider.ate-system.svc", "--credential-provider-name=vault.example.com", "--credential-provider-address=vault.ate-system.svc:8200", "--credential-provider-server-name=vault.ate-system.svc", + "--credential-provider-ca-file=", }, }, {name: "off"}, } { t.Run(tc.name, func(t *testing.T) { - patched, err := env.patchAtenetEgressInject(raw, tc.provider) + patched, err := env.patchAtenetEgressInject(raw, tc.providers) if err != nil { t.Fatalf("patchAtenetEgressInject failed: %v", err) } @@ -94,12 +109,16 @@ func TestPatchAtenetEgressInject(t *testing.T) { t.Errorf("patched manifest still contains an unreplaced marker: %q", line) } } + // The gateway pairs the repeated flags up by position. + rest := string(patched) for _, want := range tc.want { - if !strings.Contains(string(patched), want) { - t.Errorf("patched manifest is missing spliced flag %q", want) + i := strings.Index(rest, want) + if i < 0 { + t.Fatalf("patched manifest is missing spliced flag %q, or has it out of order", want) } + rest = rest[i+len(want):] } - if !tc.provider.Enabled() && strings.Contains(string(patched), "- --credential-provider-") { + if len(tc.providers) == 0 && strings.Contains(string(patched), "- --credential-provider-") { t.Error("patched manifest carries provider flags with injection off") } assertEgressManifestParses(t, patched) @@ -108,7 +127,7 @@ func TestPatchAtenetEgressInject(t *testing.T) { // The marker is what the splice keys on; a manifest without it is a // broken install, not a silent no-op. - if _, err := env.patchAtenetEgressInject([]byte("kind: ConfigMap\n"), config.CredentialProvider{}); err == nil { + if _, err := env.patchAtenetEgressInject([]byte("kind: ConfigMap\n"), nil); err == nil { t.Error("patchAtenetEgressInject accepted a manifest without the marker") } } @@ -986,7 +1005,7 @@ func TestRenderAgentgatewayStaticMTLS(t *testing.T) { { name: "egress redeploy", render: func(e *Env) ([]byte, error) { - return e.renderAtenetEgressManifest(t.Context(), config.CredentialProvider{}) + return e.renderAtenetEgressManifest(t.Context(), nil) }, }, } { @@ -1231,13 +1250,16 @@ func TestAgentgatewayEgressOverlay(t *testing.T) { } func TestRenderAgentgatewayCredentialProvider(t *testing.T) { + k8s := config.CredentialProvider{Name: config.K8sCredentialProviderName, Address: config.K8sCredentialProviderAddress} + vault := config.CredentialProvider{Name: "vault.example.com", Address: "vault.ate-system.svc:8200"} for _, tc := range []struct { - name string - provider config.CredentialProvider + name string + providers []config.CredentialProvider }{ {name: "disabled"}, - {name: "kubernetes", provider: config.CredentialProvider{Name: config.K8sCredentialProviderName, Address: config.K8sCredentialProviderAddress}}, - {name: "custom", provider: config.CredentialProvider{Name: "vault.example.com", Address: "vault.ate-system.svc:8200"}}, + {name: "kubernetes", providers: []config.CredentialProvider{k8s}}, + {name: "custom", providers: []config.CredentialProvider{vault}}, + {name: "several providers", providers: []config.CredentialProvider{k8s, vault}}, } { t.Run(tc.name, func(t *testing.T) { src := images.Source{Repo: "example.com/substrate", Tag: "v1.2.3"} @@ -1252,7 +1274,7 @@ func TestRenderAgentgatewayCredentialProvider(t *testing.T) { return "sha256:" + strings.Repeat("2", 64), nil }), } - built, err := e.renderAtenetEgressManifest(t.Context(), tc.provider) + built, err := e.renderAtenetEgressManifest(t.Context(), tc.providers) if err != nil { t.Fatal(err) } @@ -1296,24 +1318,27 @@ func TestRenderAgentgatewayCredentialProvider(t *testing.T) { if err := policy.YNode().Decode(&got); err != nil { t.Fatal(err) } - if !tc.provider.Enabled() { + if len(tc.providers) == 0 { if len(got.Providers) != 0 { t.Errorf("%s unexpectedly has credential providers: %v", protocol, got.Providers) } continue } - want := []map[string]any{{ - "uriAuthority": tc.provider.Name, - "target": map[string]any{ - "host": tc.provider.Address, - "policies": map[string]any{"backendTLS": map[string]any{ - "hostname": tc.provider.ServerName(), - "cert": "/run/podidentity.podcert.ate.dev/credential-bundle.pem", - "key": "/run/podidentity.podcert.ate.dev/credential-bundle.pem", - "root": "/run/servicedns-ca/trust-bundle.pem", - }}, - }, - }} + var want []map[string]any + for _, p := range tc.providers { + want = append(want, map[string]any{ + "uriAuthority": p.Name, + "target": map[string]any{ + "host": p.Address, + "policies": map[string]any{"backendTLS": map[string]any{ + "hostname": p.ServerName(), + "cert": "/run/podidentity.podcert.ate.dev/credential-bundle.pem", + "key": "/run/podidentity.podcert.ate.dev/credential-bundle.pem", + "root": "/run/servicedns-ca/trust-bundle.pem", + }}, + }, + }) + } if !reflect.DeepEqual(got.Providers, want) { t.Errorf("credential providers = %v, want %v", got.Providers, want) } @@ -1328,7 +1353,7 @@ func TestRenderAgentgatewayCredentialProvider(t *testing.T) { func TestPatchAgentgatewayCredentialProviderRequiresOneMarker(t *testing.T) { for _, raw := range []string{"", "#ATE_AGENTGATEWAY_CREDENTIAL_PROVIDERS\n#ATE_AGENTGATEWAY_CREDENTIAL_PROVIDERS\n"} { - if _, err := patchAgentgatewayEgressInject([]byte(raw), config.CredentialProvider{}); err == nil { + if _, err := patchAgentgatewayEgressInject([]byte(raw), nil); err == nil { t.Errorf("accepted a manifest without exactly one marker: %q", raw) } } @@ -1450,7 +1475,7 @@ func TestRenderAtenetEgressManifestPrebuilt(t *testing.T) { }{ { name: "egress", - render: func() ([]byte, error) { return e.renderAtenetEgressManifest(t.Context(), config.CredentialProvider{}) }, + render: func() ([]byte, error) { return e.renderAtenetEgressManifest(t.Context(), nil) }, }, { name: "router", @@ -1546,7 +1571,7 @@ func TestRenderAtenetEgressManifestEnvoyConcurrency(t *testing.T) { return digest, nil }), } - out, err := e.renderAtenetEgressManifest(t.Context(), config.CredentialProvider{}) + out, err := e.renderAtenetEgressManifest(t.Context(), nil) if err != nil { t.Fatalf("renderAtenetEgressManifest() error = %v", err) } @@ -1568,7 +1593,7 @@ func TestRenderAtenetEgressManifestEnvoyConcurrency(t *testing.T) { return digest, nil }), } - out, err := e.renderAtenetEgressManifest(t.Context(), config.CredentialProvider{}) + out, err := e.renderAtenetEgressManifest(t.Context(), nil) if err != nil { t.Fatalf("renderAtenetEgressManifest() error = %v", err) } From c8f7752de0d9c50652be86fb21900c5aaae39811 Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 15:33:46 -0700 Subject: [PATCH 03/12] ate-setup: tell a provider object from an array by its JSON type Decode --credential-provider once and switch on whether it is an object or an array, instead of looking for a leading bracket, and document the three accepted forms. Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/config.go | 56 +++++++++++++++---------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/cmd/ate-setup/internal/config/config.go b/cmd/ate-setup/internal/config/config.go index c447f3f514..eb0cb98c2e 100644 --- a/cmd/ate-setup/internal/config/config.go +++ b/cmd/ate-setup/internal/config/config.go @@ -625,16 +625,8 @@ func (p CredentialProvider) ServerName() string { // credentialProvider parses a --credential-provider JSON object that either // turns injection off with enabled false or names a provider and its address. -// Only the bundled provider has a default address. A missing value is reported here rather than in validate -// because only the deploys that render the egress gateway need it. +// Only the bundled provider has a default address. func credentialProvider(raw string) (CredentialProvider, error) { - if raw == "" { - // Every channel is named, not just the flag: a provider set in the - // configuration document is the documented way to do it, and a reader - // told only about the flag goes looking in the wrong place. - return CredentialProvider{}, fmt.Errorf("a credential provider is required; set one of %s: %s", - channelsFor(credentialProviderKey), credentialProviderUsage) - } invalid := func(format string, args ...any) (CredentialProvider, error) { return CredentialProvider{}, fmt.Errorf("invalid --credential-provider '%s': %s", raw, fmt.Sprintf(format, args...)) } @@ -673,31 +665,53 @@ func credentialProvider(raw string) (CredentialProvider, error) { return CredentialProvider{Name: spec.Name, Address: spec.Address}, nil } -// CredentialProviders parses --credential-provider: one provider object, which -// credentialProvider reads, or a JSON array of them. None means injection is -// off. +// CredentialProviders parses --credential-provider, which is one of: +// +// {"name":"k8s.io"} one provider +// [{"name":"k8s.io"},{"name":"","address":""}] several providers +// {"enabled":false} none: injection off +// +// credentialProvider checks each provider object. A missing value is reported +// here rather than in validate because only the deploys that render the egress +// gateway need it. func (c *Config) CredentialProviders() ([]CredentialProvider, error) { raw := c.CredentialProviderJSON - if !strings.HasPrefix(strings.TrimSpace(raw), "[") { - p, err := credentialProvider(raw) - if err != nil || !p.Enabled() { - return nil, err - } - return []CredentialProvider{p}, nil + if raw == "" { + // Every channel is named, not just the flag: a provider set in the + // configuration document is the documented way to do it, and a reader + // told only about the flag goes looking in the wrong place. + return nil, fmt.Errorf("a credential provider is required; set one of %s: %s", + channelsFor(credentialProviderKey), credentialProviderUsage) } invalid := func(format string, args ...any) ([]CredentialProvider, error) { return nil, fmt.Errorf("invalid --credential-provider '%s': %s", raw, fmt.Sprintf(format, args...)) } - var elems []json.RawMessage - if err := json.Unmarshal([]byte(raw), &elems); err != nil { + var value any + if err := json.Unmarshal([]byte(raw), &value); err != nil { return invalid("%v; want %s", err, credentialProviderUsage) } + var elems []any + switch v := value.(type) { + case map[string]any: + p, err := credentialProvider(raw) + if err != nil || !p.Enabled() { + return nil, err + } + return []CredentialProvider{p}, nil + case []any: + elems = v + default: + return invalid("want a JSON object or an array of them: %s", credentialProviderUsage) + } if len(elems) == 0 { return invalid(`the array names no provider; {"enabled":false} turns injection off`) } var providers []CredentialProvider for _, elem := range elems { - p, err := credentialProvider(string(elem)) + // Re-encoded so each entry gets credentialProvider's strict decoding. A + // value json.Unmarshal produced always encodes. + obj, _ := json.Marshal(elem) + p, err := credentialProvider(string(obj)) if err != nil { return nil, err } From f7dc46852dc2fbba72ee37b2e2aca7bf1b549609 Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 15:43:01 -0700 Subject: [PATCH 04/12] ate-setup: drop a comment the error message already explains Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/config.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/ate-setup/internal/config/config.go b/cmd/ate-setup/internal/config/config.go index eb0cb98c2e..0a0589f212 100644 --- a/cmd/ate-setup/internal/config/config.go +++ b/cmd/ate-setup/internal/config/config.go @@ -677,9 +677,6 @@ func credentialProvider(raw string) (CredentialProvider, error) { func (c *Config) CredentialProviders() ([]CredentialProvider, error) { raw := c.CredentialProviderJSON if raw == "" { - // Every channel is named, not just the flag: a provider set in the - // configuration document is the documented way to do it, and a reader - // told only about the flag goes looking in the wrong place. return nil, fmt.Errorf("a credential provider is required; set one of %s: %s", channelsFor(credentialProviderKey), credentialProviderUsage) } From 9875d67340bef6e798631216a601ee0db951e261 Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 16:14:52 -0700 Subject: [PATCH 05/12] egress: reject an empty credential provider address With the repeatable flag, --credential-provider-address= became one provider with an empty dial target instead of turning injection off. Refuse it at startup, and correct the comments on what happens with no provider: a request that needs an injection is denied. Signed-off-by: Max Thompson --- cmd/atenet/internal/router/cmd.go | 2 +- cmd/atenet/internal/router/config.go | 10 ++++++---- cmd/atenet/internal/router/config_test.go | 5 +++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cmd/atenet/internal/router/cmd.go b/cmd/atenet/internal/router/cmd.go index 5908d84c40..90f02d1a0d 100644 --- a/cmd/atenet/internal/router/cmd.go +++ b/cmd/atenet/internal/router/cmd.go @@ -83,7 +83,7 @@ func bindRouterFlags(fs *pflag.FlagSet, cfg *routerConfig) { fs.DurationVar(&cfg.EgressPolicyCacheTTL, "egress-policy-cache-ttl", egress.DefaultPolicyCacheTTL, "How long the egress gateway keeps acting on an actor's EgressPolicy before fetching it from ateapi again, which bounds the lag between a policy change and its effect on new requests. 0 disables the cache (concurrent callouts for one actor still share a fetch)") // Egress credential injection (MITM leg). Only the egress gateway sets these, - // and only when injection is enabled: with an empty + // and only when injection is enabled: with no // --credential-provider-address, a request matching an EgressPolicy rule // that requires injection is denied. fs.StringArrayVar(&cfg.CredentialProvider.Names, "credential-provider-name", nil, "Credential provider this egress gateway serves, as the host of its ate-secret:// credential URIs (e.g. k8s.io). Repeat once per --credential-provider-address, in the same order; a policy credential URI naming no configured provider is refused. A single empty name disables the check (dev only)") diff --git a/cmd/atenet/internal/router/config.go b/cmd/atenet/internal/router/config.go index c92f10b47f..61527158c6 100644 --- a/cmd/atenet/internal/router/config.go +++ b/cmd/atenet/internal/router/config.go @@ -121,6 +121,9 @@ func (p credentialProviderConfig) validate() error { return fmt.Errorf("got %d --credential-provider-server-name for %d --credential-provider-address, want one per address or none", len(p.ServerNames), len(p.Addresses)) } for i, name := range p.Names { + if p.Addresses[i] == "" { + return fmt.Errorf("--credential-provider-address must not be empty") + } if name == "" && len(p.Names) > 1 { return fmt.Errorf("an empty --credential-provider-name serves every credential URI, so it must be the only provider") } @@ -177,10 +180,9 @@ type routerConfig struct { EgressPolicyCacheTTL time.Duration // CredentialProvider configures egress credential injection on the MITM leg. - // Only the egress gateway sets it, and only when injection is enabled: an - // CredentialProvider with no Addresses leaves the injector disabled, so a rule - // that requires an injection is skipped and the request passes through - // without the credential. See egress.Handler. + // Only the egress gateway sets it, and only when injection is enabled: a + // CredentialProvider with no Addresses leaves the injector disabled, so a + // request that needs an injection is denied. See egress.Handler. CredentialProvider credentialProviderConfig LogLevel string diff --git a/cmd/atenet/internal/router/config_test.go b/cmd/atenet/internal/router/config_test.go index 371e494bce..746e563878 100644 --- a/cmd/atenet/internal/router/config_test.go +++ b/cmd/atenet/internal/router/config_test.go @@ -53,6 +53,11 @@ func TestRouterConfigValidate(t *testing.T) { Names: []string{"k8s.io", "sts.example.com"}, Addresses: []string{"a:1", "b:1"}, ServerNames: []string{"a", "b"}, }}, }, + { + name: "empty credential provider address rejected", + cfg: routerConfig{Mode: ModeEgress, CredentialProvider: credentialProviderConfig{Names: []string{"k8s.io"}, Addresses: []string{""}}}, + wantErr: "--credential-provider-address must not be empty", + }, { name: "credential provider name without an address rejected", cfg: routerConfig{Mode: ModeEgress, CredentialProvider: credentialProviderConfig{Names: []string{"k8s.io"}}}, From c197c74ce86951fb0f01db940c428691028d5a7e Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 16:16:41 -0700 Subject: [PATCH 06/12] ate-setup: require each provider array entry to be an object An entry such as null reported "name is required". Reject a non-object entry by its JSON type, and handle the error from re-encoding each entry. Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/config.go | 10 +++++++--- cmd/ate-setup/internal/config/config_test.go | 15 ++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cmd/ate-setup/internal/config/config.go b/cmd/ate-setup/internal/config/config.go index 0a0589f212..fda58cfa1e 100644 --- a/cmd/ate-setup/internal/config/config.go +++ b/cmd/ate-setup/internal/config/config.go @@ -705,9 +705,13 @@ func (c *Config) CredentialProviders() ([]CredentialProvider, error) { } var providers []CredentialProvider for _, elem := range elems { - // Re-encoded so each entry gets credentialProvider's strict decoding. A - // value json.Unmarshal produced always encodes. - obj, _ := json.Marshal(elem) + if _, ok := elem.(map[string]any); !ok { + return invalid("array entries must be JSON objects; want %s", credentialProviderUsage) + } + obj, err := json.Marshal(elem) + if err != nil { + return invalid("%v", err) + } p, err := credentialProvider(string(obj)) if err != nil { return nil, err diff --git a/cmd/ate-setup/internal/config/config_test.go b/cmd/ate-setup/internal/config/config_test.go index 274ac78372..1fef3fed4f 100644 --- a/cmd/ate-setup/internal/config/config_test.go +++ b/cmd/ate-setup/internal/config/config_test.go @@ -1166,9 +1166,10 @@ func TestCredentialProvidersArray(t *testing.T) { loadEnv(t) k8s := CredentialProvider{Name: K8sCredentialProviderName, Address: K8sCredentialProviderAddress} for _, tc := range []struct { - name string - raw string - want []CredentialProvider + name string + raw string + want []CredentialProvider + wantErr string }{ {name: "an array of one", raw: `[{"name":"k8s.io"}]`, want: []CredentialProvider{k8s}}, { @@ -1176,9 +1177,17 @@ func TestCredentialProvidersArray(t *testing.T) { raw: `[{"name":"k8s.io"},{"name":"sts.example.com","address":"sts.ate-system.svc:50051"}]`, want: []CredentialProvider{k8s, {Name: "sts.example.com", Address: "sts.ate-system.svc:50051"}}, }, + {name: "a null entry", raw: `[null]`, wantErr: "array entries must be JSON objects"}, + {name: "a string entry", raw: `["k8s.io"]`, wantErr: "array entries must be JSON objects"}, } { t.Run(tc.name, func(t *testing.T) { cfg, err := Load(Options{CredentialProvider: tc.raw}) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("Load() error = %v, want one containing %q", err, tc.wantErr) + } + return + } if err != nil { t.Fatalf("Load() error = %v", err) } From f42dac1633a8cb072868a6b8be212cb719a70795 Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 16:34:07 -0700 Subject: [PATCH 07/12] egress: match every URI only with a lone "" provider Document the providers map on the Handler field and point New at it. The catch-all now applies only when "" is the sole entry, as the field says, instead of whenever a "" key is present. Signed-off-by: Max Thompson --- .../internal/router/egress/credentials.go | 12 ++++---- .../router/egress/credentials_test.go | 28 +++++++++++++++++++ cmd/atenet/internal/router/egress/egress.go | 13 +++++---- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/cmd/atenet/internal/router/egress/credentials.go b/cmd/atenet/internal/router/egress/credentials.go index 68be8151c2..5712a7aca2 100644 --- a/cmd/atenet/internal/router/egress/credentials.go +++ b/cmd/atenet/internal/router/egress/credentials.go @@ -147,17 +147,19 @@ func (h *Handler) providerCredential(ctx context.Context, ref resources.ActorRef return nil, extproc.NewReqError(envoy_type.StatusCode_InternalServerError, deniedBody) } - // Send the URI to the provider its host names. A URI naming no configured - // provider fails closed rather than going to the wrong one. - provider, ok := h.providers[""] - if !ok { + // If the only provider is keyed "", it serves every URI. Otherwise find the + // provider the URI's host names; a URI naming none fails closed. + var provider credprovider.CredentialProviderClient + if p, ok := h.providers[""]; ok && len(h.providers) == 1 { + provider = p + } else { name, err := providerNameFromURI(uri) if err != nil { slog.ErrorContext(ctx, "egress denied: policy names an unparseable credential URI", slog.Any("actor", ref), slog.String("host", dest.Hostname), slog.String("uri", uri), slog.Any("err", err)) return nil, extproc.WrapReqError(envoy_type.StatusCode_InternalServerError, err, deniedBody) } - if provider, ok = h.providers[name]; !ok { + if provider = h.providers[name]; provider == nil { slog.ErrorContext(ctx, "egress denied: credential URI names a provider this gateway does not serve", slog.Any("actor", ref), slog.String("host", dest.Hostname), slog.String("uri", uri), slog.String("provider", name), slog.Any("serves", slices.Sorted(maps.Keys(h.providers)))) diff --git a/cmd/atenet/internal/router/egress/credentials_test.go b/cmd/atenet/internal/router/egress/credentials_test.go index ddc43c7c0f..0d5bc731fa 100644 --- a/cmd/atenet/internal/router/egress/credentials_test.go +++ b/cmd/atenet/internal/router/egress/credentials_test.go @@ -153,6 +153,34 @@ func TestInjectionPicksProviderByName(t *testing.T) { } } +func TestInjectionCatchAllProvider(t *testing.T) { + policy := credentialInjectionPolicySample("api.example.com") + md := func() *extproc.RequestMetadata { + return withAuthorization(innerMetadata(extproc.EgressTLSMITMFilterChainName, "GET", "api.example.com", nil)) + } + + catchAll := &fakeProvider{resp: bearerTokenResponse("s3cr3t")} + h := New(&egressMockClient{actor: runningActor(), policy: policy}, nil, 0, + map[string]credprovider.CredentialProviderClient{"": catchAll}) + if _, err := h.HandleRequestHeaders(context.Background(), md()); err != nil { + t.Fatalf("HandleRequestHeaders with a lone \"\" provider: %v", err) + } + if catchAll.got == nil { + t.Error("a lone \"\" provider was not asked for the credential") + } + + catchAll = &fakeProvider{resp: bearerTokenResponse("s3cr3t")} + k8s := &fakeProvider{resp: bearerTokenResponse("s3cr3t")} + h = New(&egressMockClient{actor: runningActor(), policy: policy}, nil, 0, + map[string]credprovider.CredentialProviderClient{"": catchAll, injectionProviderName: k8s}) + if _, err := h.HandleRequestHeaders(context.Background(), md()); err != nil { + t.Fatalf("HandleRequestHeaders with a \"\" provider next to another: %v", err) + } + if catchAll.got != nil || k8s.got == nil { + t.Errorf("next to another provider, the \"\" entry was asked (%v) and %s was asked (%v); want only %s", catchAll.got != nil, injectionProviderName, k8s.got != nil, injectionProviderName) + } +} + // A request that does not carry the header a rule replaces goes out unchanged: // nothing is fetched, added, or denied, even where injecting would fail. func TestInjectionSkippedWithoutHeader(t *testing.T) { diff --git a/cmd/atenet/internal/router/egress/egress.go b/cmd/atenet/internal/router/egress/egress.go index 35fd25e451..7c0edd34b1 100644 --- a/cmd/atenet/internal/router/egress/egress.go +++ b/cmd/atenet/internal/router/egress/egress.go @@ -85,7 +85,11 @@ type Handler struct { policies *policyCache // actorJWTs mints the actor JWTs that credential injections ask for. actorJWTs *actorjwt.Minter - // providers resolve credential_uri injections by provider name; see New. + // providers is a map of provider name (from the URI) to the credential + // provider. + // + // A special case of a map consisting of a single entry with empty string + // ("") key will match all provider URIs. providers map[string]credprovider.CredentialProviderClient } @@ -96,10 +100,9 @@ type Handler struct { // policyCacheTTL of 0 fetches the policy on every callout. // // providers resolve an allowed rule's credential_uri injections on the -// TLS-terminated MITM leg, keyed by provider name. A credential URI goes to the -// provider its host names, and one naming no provider is refused; a lone "" -// key serves every URI. No providers leaves injections off, so a request that -// needs one is denied. Actor JWT injections need no provider. +// TLS-terminated MITM leg; none leaves them off, so a request that needs one is +// denied. Actor JWT injections need no provider. See Handler.providers for the +// map definition. func New(apiClient ateapi.ControlClient, loadActorIdentityRoots func() (*x509.CertPool, error), policyCacheTTL time.Duration, providers map[string]credprovider.CredentialProviderClient) *Handler { if loadActorIdentityRoots == nil { loadActorIdentityRoots = func() (*x509.CertPool, error) { From e8622bd3ac02ff6d8888ee2ff2cc9b617832852d Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 16:49:07 -0700 Subject: [PATCH 08/12] egress: say when the "" provider matches every URI Signed-off-by: Max Thompson --- cmd/atenet/internal/router/egress/credentials.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/atenet/internal/router/egress/credentials.go b/cmd/atenet/internal/router/egress/credentials.go index 5712a7aca2..85bca5063b 100644 --- a/cmd/atenet/internal/router/egress/credentials.go +++ b/cmd/atenet/internal/router/egress/credentials.go @@ -147,8 +147,9 @@ func (h *Handler) providerCredential(ctx context.Context, ref resources.ActorRef return nil, extproc.NewReqError(envoy_type.StatusCode_InternalServerError, deniedBody) } - // If the only provider is keyed "", it serves every URI. Otherwise find the - // provider the URI's host names; a URI naming none fails closed. + // When h.providers has exactly one entry and its key is "", that provider + // serves every URI. Otherwise look up the provider named by the URI's host, + // and fail closed if none matches. var provider credprovider.CredentialProviderClient if p, ok := h.providers[""]; ok && len(h.providers) == 1 { provider = p From c5a0cdce9f214955921714b40018a6fc9f9b9b88 Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 19:57:23 -0700 Subject: [PATCH 09/12] ate-setup: repeat --credential-provider instead of taking a JSON array Each --credential-provider flag now takes one provider object. The environment variable and the config file can't repeat, so they take the objects separated by whitespace, and Resolve joins repeated flags with spaces so every channel parses the same way. Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/config.go | 54 ++++++++------------ cmd/ate-setup/internal/config/config_test.go | 54 ++++++++++++++------ cmd/ate-setup/internal/config/resolve.go | 8 +++ cmd/ate-setup/internal/config/setting.go | 23 +++++---- 4 files changed, 80 insertions(+), 59 deletions(-) diff --git a/cmd/ate-setup/internal/config/config.go b/cmd/ate-setup/internal/config/config.go index fda58cfa1e..05cf555d9d 100644 --- a/cmd/ate-setup/internal/config/config.go +++ b/cmd/ate-setup/internal/config/config.go @@ -583,7 +583,7 @@ const credentialProviderKey = "atenet.egress.credentialProvider" const credentialProviderUsage = `{"name":"k8s.io"} for the bundled Kubernetes Secrets provider, ` + `{"enabled":false} to turn egress credential injection off, ` + `or {"name":"","address":":"} for a provider you deploy; ` + - `a JSON array of provider objects selects several` + `for several providers, repeat the flag or separate the objects with whitespace` // credentialProviderSpec is the schema of the --credential-provider JSON. type credentialProviderSpec struct { @@ -665,10 +665,11 @@ func credentialProvider(raw string) (CredentialProvider, error) { return CredentialProvider{Name: spec.Name, Address: spec.Address}, nil } -// CredentialProviders parses --credential-provider, which is one of: +// CredentialProviders parses --credential-provider, one JSON object per +// provider, separated by whitespace: // // {"name":"k8s.io"} one provider -// [{"name":"k8s.io"},{"name":"","address":""}] several providers +// {"name":"k8s.io"} {"name":"","address":""} several providers // {"enabled":false} none: injection off // // credentialProvider checks each provider object. A missing value is reported @@ -676,53 +677,40 @@ func credentialProvider(raw string) (CredentialProvider, error) { // gateway need it. func (c *Config) CredentialProviders() ([]CredentialProvider, error) { raw := c.CredentialProviderJSON - if raw == "" { - return nil, fmt.Errorf("a credential provider is required; set one of %s: %s", - channelsFor(credentialProviderKey), credentialProviderUsage) - } invalid := func(format string, args ...any) ([]CredentialProvider, error) { return nil, fmt.Errorf("invalid --credential-provider '%s': %s", raw, fmt.Sprintf(format, args...)) } - var value any - if err := json.Unmarshal([]byte(raw), &value); err != nil { - return invalid("%v; want %s", err, credentialProviderUsage) - } - var elems []any - switch v := value.(type) { - case map[string]any: - p, err := credentialProvider(raw) - if err != nil || !p.Enabled() { - return nil, err - } - return []CredentialProvider{p}, nil - case []any: - elems = v - default: - return invalid("want a JSON object or an array of them: %s", credentialProviderUsage) - } - if len(elems) == 0 { - return invalid(`the array names no provider; {"enabled":false} turns injection off`) - } var providers []CredentialProvider - for _, elem := range elems { - if _, ok := elem.(map[string]any); !ok { - return invalid("array entries must be JSON objects; want %s", credentialProviderUsage) + disabled := false + dec := json.NewDecoder(strings.NewReader(raw)) + for { + var obj json.RawMessage + err := dec.Decode(&obj) + if err == io.EOF { + break } - obj, err := json.Marshal(elem) if err != nil { - return invalid("%v", err) + return invalid("%v; want %s", err, credentialProviderUsage) } p, err := credentialProvider(string(obj)) if err != nil { return nil, err } if !p.Enabled() { - return invalid("enabled false is not allowed in an array") + disabled = true + continue } if slices.ContainsFunc(providers, func(q CredentialProvider) bool { return q.Name == p.Name }) { return invalid("provider name %q repeats", p.Name) } providers = append(providers, p) } + switch { + case disabled && len(providers) > 0: + return invalid(`{"enabled":false} cannot be combined with a provider`) + case !disabled && len(providers) == 0: + return nil, fmt.Errorf("a credential provider is required; set one of %s: %s", + channelsFor(credentialProviderKey), credentialProviderUsage) + } return providers, nil } diff --git a/cmd/ate-setup/internal/config/config_test.go b/cmd/ate-setup/internal/config/config_test.go index 1fef3fed4f..09fc9b11e9 100644 --- a/cmd/ate-setup/internal/config/config_test.go +++ b/cmd/ate-setup/internal/config/config_test.go @@ -1047,7 +1047,7 @@ func TestLoadRejectsInvalidValues(t *testing.T) { {"provider null", Options{CredentialProvider: `null`}}, {"provider without a name", Options{CredentialProvider: `{"address":"vault.ate-system.svc:8200"}`}}, {"provider with an unknown key", Options{CredentialProvider: `{"name":"k8s.io","adress":"x:1"}`}}, - {"provider with trailing data", Options{CredentialProvider: `{"enabled":false} {"name":"k8s.io"}`}}, + {"disabled beside a provider", Options{CredentialProvider: `{"enabled":false} {"name":"k8s.io"}`}}, {"provider name with a scheme", Options{CredentialProvider: `{"name":"ate-secret://k8s.io"}`}}, {"provider name not lowercase", Options{CredentialProvider: `{"name":"Vault.example.com","address":"vault.ate-system.svc:8200"}`}}, {"provider name with a port", Options{CredentialProvider: `{"name":"k8s.io:443"}`}}, @@ -1059,10 +1059,6 @@ func TestLoadRejectsInvalidValues(t *testing.T) { {"kubernetes address without a port", Options{CredentialProvider: `{"name":"k8s.io","address":"secrets.ate-system.svc"}`}}, {"other provider without an address", Options{CredentialProvider: `{"name":"vault.example.com"}`}}, {"other provider address without a port", Options{CredentialProvider: `{"name":"vault.example.com","address":"vault.ate-system.svc"}`}}, - {"provider array empty", Options{CredentialProvider: `[]`}}, - {"provider array entry disabled", Options{CredentialProvider: `[{"enabled":false}]`}}, - {"provider array with a repeated name", Options{CredentialProvider: `[{"name":"k8s.io"},{"name":"k8s.io"}]`}}, - {"provider array entry without an address", Options{CredentialProvider: `[{"name":"k8s.io"},{"name":"vault.example.com"}]`}}, } { t.Run(tc.name, func(t *testing.T) { if _, err := Load(tc.opts); err == nil { @@ -1162,34 +1158,58 @@ func TestCredentialProvider(t *testing.T) { } } -func TestCredentialProvidersArray(t *testing.T) { +func TestSeveralCredentialProviders(t *testing.T) { loadEnv(t) - k8s := CredentialProvider{Name: K8sCredentialProviderName, Address: K8sCredentialProviderAddress} + const k8sJSON = `{"name":"k8s.io"}` + const stsJSON = `{"name":"sts.example.com","address":"sts.ate-system.svc:50051"}` + both := []CredentialProvider{ + {Name: K8sCredentialProviderName, Address: K8sCredentialProviderAddress}, + {Name: "sts.example.com", Address: "sts.ate-system.svc:50051"}, + } for _, tc := range []struct { name string - raw string + flags []string + env string + file string want []CredentialProvider wantErr string }{ - {name: "an array of one", raw: `[{"name":"k8s.io"}]`, want: []CredentialProvider{k8s}}, + {name: "repeated flags", flags: []string{k8sJSON, stsJSON}, want: both}, + {name: "environment", env: k8sJSON + " " + stsJSON, want: both}, { - name: "several providers", - raw: `[{"name":"k8s.io"},{"name":"sts.example.com","address":"sts.ate-system.svc:50051"}]`, - want: []CredentialProvider{k8s, {Name: "sts.example.com", Address: "sts.ate-system.svc:50051"}}, + name: "file with one object per line", + file: "atenet:\n egress:\n credentialProvider: |\n " + k8sJSON + "\n " + stsJSON + "\n", + want: both, }, - {name: "a null entry", raw: `[null]`, wantErr: "array entries must be JSON objects"}, - {name: "a string entry", raw: `["k8s.io"]`, wantErr: "array entries must be JSON objects"}, + {name: "a repeated name", flags: []string{k8sJSON, k8sJSON}, wantErr: `provider name "k8s.io" repeats`}, + {name: "disabled beside a provider", flags: []string{`{"enabled":false}`, k8sJSON}, wantErr: "cannot be combined"}, + {name: "a JSON array", env: "[" + k8sJSON + "," + stsJSON + "]", wantErr: "cannot unmarshal array"}, + {name: "only whitespace", env: " \n", wantErr: "a credential provider is required"}, } { t.Run(tc.name, func(t *testing.T) { - cfg, err := Load(Options{CredentialProvider: tc.raw}) + if tc.env != "" { + t.Setenv("ATE_CREDENTIAL_PROVIDER", tc.env) + } + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + BindFlags(fs) + for _, v := range tc.flags { + if err := fs.Set("credential-provider", v); err != nil { + t.Fatal(err) + } + } + var lo LoadOptions + if tc.file != "" { + lo.ConfigPath = writeConfig(t, validHeader+tc.file) + } + cfg, err := LoadFlags(fs, lo) if tc.wantErr != "" { if err == nil || !strings.Contains(err.Error(), tc.wantErr) { - t.Fatalf("Load() error = %v, want one containing %q", err, tc.wantErr) + t.Fatalf("LoadFlags() error = %v, want one containing %q", err, tc.wantErr) } return } if err != nil { - t.Fatalf("Load() error = %v", err) + t.Fatalf("LoadFlags() error = %v", err) } got, err := cfg.CredentialProviders() if err != nil { diff --git a/cmd/ate-setup/internal/config/resolve.go b/cmd/ate-setup/internal/config/resolve.go index ea79c34a43..b1a5455dbe 100644 --- a/cmd/ate-setup/internal/config/resolve.go +++ b/cmd/ate-setup/internal/config/resolve.go @@ -17,6 +17,7 @@ package config import ( "fmt" "sort" + "strings" "time" "github.com/spf13/pflag" @@ -123,6 +124,13 @@ func Resolve(fs *pflag.FlagSet, opts ResolveOptions) (*Resolved, error) { // flag's printed form, which round-trips for bool and int. raw = fs.Lookup(s.Flag).Value.String() } + if s.Repeated { + values, err := fs.GetStringArray(s.Flag) + if err != nil { + return nil, fmt.Errorf("reading --%s: %w", s.Flag, err) + } + raw = strings.Join(values, " ") + } v = Value{Setting: s, Raw: raw, From: OriginFlag, Supplied: true} } diff --git a/cmd/ate-setup/internal/config/setting.go b/cmd/ate-setup/internal/config/setting.go index 6fe546a358..6a966c4b06 100644 --- a/cmd/ate-setup/internal/config/setting.go +++ b/cmd/ate-setup/internal/config/setting.go @@ -50,6 +50,9 @@ type Setting struct { Flag string // Kind is the value type. Resolve parses raw strings against it. Kind ValueKind + // Repeated lets the flag be given more than once. Resolve joins the + // values with spaces, so every channel must accept space-separated values. + Repeated bool // Default applies when no channel supplies a value. Default string // Secret suppresses the value wherever configuration is displayed. @@ -195,11 +198,9 @@ var Registry = []Setting{ "certificates. The podidentity signer issues spiffe://cluster.local/ns//sa/", }, { - // The egress gateway's credential provider, as a JSON object. Upstream - // takes it as one value on one flag; it is carried the same way here - // so the two channels accept the same text. + // The egress gateway's credential providers; see CredentialProviders. Key: "atenet.egress.credentialProvider", Env: "ATE_CREDENTIAL_PROVIDER", - Flag: "credential-provider", Kind: KindString, + Flag: "credential-provider", Kind: KindString, Repeated: true, Usage: "Credential provider the egress gateway injects credentials from, as JSON; " + "required by deploy ate-system and deploy atenet. " + credentialProviderUsage + ". A provider requires --atenet-dataplane=envoy", @@ -533,12 +534,16 @@ func (s Setting) bindable() bool { return s.Flag != "" && !s.Secret } // only from Set, never from registration, and Resolve reads a flag only when // Changed reports true. // -// Only bools get a typed flag. Ints and durations register as strings so that -// Setting.parse is the one thing deciding what they accept: pflag's Int reads -// base 0, so it takes "0x10" as 16 where parse rejects it, and its errors name -// no canonical key. One vocabulary across all three channels is worth more -// than the type shown in help. +// Only bools and repeated settings get a typed flag. Ints and durations +// register as strings so that Setting.parse is the one thing deciding what +// they accept: pflag's Int reads base 0, so it takes "0x10" as 16 where parse +// rejects it, and its errors name no canonical key. One vocabulary across all +// three channels is worth more than the type shown in help. func bind(fs *pflag.FlagSet, s Setting) { + if s.Repeated { + fs.StringArray(s.Flag, nil, s.Usage) + return + } if s.Kind == KindBool { // An empty Default yields nil from parse; the assertion then gives // false, which is right for a bool with no declared default. From 62b589c05cd080dc002bde749c8af9c105eeb44d Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 21:26:27 -0700 Subject: [PATCH 10/12] ate-setup: take several credential providers as a JSON array again Repeated --credential-provider flags were joined with spaces and split again by the JSON decoder. That only works because JSON objects mark their own boundaries, so a general repeated setting can't use it. Go back to one JSON object or array on every channel until ate-setup has a list setting type. Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/config.go | 54 ++++++++++++-------- cmd/ate-setup/internal/config/config_test.go | 54 ++++++-------------- cmd/ate-setup/internal/config/resolve.go | 8 --- cmd/ate-setup/internal/config/setting.go | 23 ++++----- 4 files changed, 59 insertions(+), 80 deletions(-) diff --git a/cmd/ate-setup/internal/config/config.go b/cmd/ate-setup/internal/config/config.go index 05cf555d9d..fda58cfa1e 100644 --- a/cmd/ate-setup/internal/config/config.go +++ b/cmd/ate-setup/internal/config/config.go @@ -583,7 +583,7 @@ const credentialProviderKey = "atenet.egress.credentialProvider" const credentialProviderUsage = `{"name":"k8s.io"} for the bundled Kubernetes Secrets provider, ` + `{"enabled":false} to turn egress credential injection off, ` + `or {"name":"","address":":"} for a provider you deploy; ` + - `for several providers, repeat the flag or separate the objects with whitespace` + `a JSON array of provider objects selects several` // credentialProviderSpec is the schema of the --credential-provider JSON. type credentialProviderSpec struct { @@ -665,11 +665,10 @@ func credentialProvider(raw string) (CredentialProvider, error) { return CredentialProvider{Name: spec.Name, Address: spec.Address}, nil } -// CredentialProviders parses --credential-provider, one JSON object per -// provider, separated by whitespace: +// CredentialProviders parses --credential-provider, which is one of: // // {"name":"k8s.io"} one provider -// {"name":"k8s.io"} {"name":"","address":""} several providers +// [{"name":"k8s.io"},{"name":"","address":""}] several providers // {"enabled":false} none: injection off // // credentialProvider checks each provider object. A missing value is reported @@ -677,40 +676,53 @@ func credentialProvider(raw string) (CredentialProvider, error) { // gateway need it. func (c *Config) CredentialProviders() ([]CredentialProvider, error) { raw := c.CredentialProviderJSON + if raw == "" { + return nil, fmt.Errorf("a credential provider is required; set one of %s: %s", + channelsFor(credentialProviderKey), credentialProviderUsage) + } invalid := func(format string, args ...any) ([]CredentialProvider, error) { return nil, fmt.Errorf("invalid --credential-provider '%s': %s", raw, fmt.Sprintf(format, args...)) } + var value any + if err := json.Unmarshal([]byte(raw), &value); err != nil { + return invalid("%v; want %s", err, credentialProviderUsage) + } + var elems []any + switch v := value.(type) { + case map[string]any: + p, err := credentialProvider(raw) + if err != nil || !p.Enabled() { + return nil, err + } + return []CredentialProvider{p}, nil + case []any: + elems = v + default: + return invalid("want a JSON object or an array of them: %s", credentialProviderUsage) + } + if len(elems) == 0 { + return invalid(`the array names no provider; {"enabled":false} turns injection off`) + } var providers []CredentialProvider - disabled := false - dec := json.NewDecoder(strings.NewReader(raw)) - for { - var obj json.RawMessage - err := dec.Decode(&obj) - if err == io.EOF { - break + for _, elem := range elems { + if _, ok := elem.(map[string]any); !ok { + return invalid("array entries must be JSON objects; want %s", credentialProviderUsage) } + obj, err := json.Marshal(elem) if err != nil { - return invalid("%v; want %s", err, credentialProviderUsage) + return invalid("%v", err) } p, err := credentialProvider(string(obj)) if err != nil { return nil, err } if !p.Enabled() { - disabled = true - continue + return invalid("enabled false is not allowed in an array") } if slices.ContainsFunc(providers, func(q CredentialProvider) bool { return q.Name == p.Name }) { return invalid("provider name %q repeats", p.Name) } providers = append(providers, p) } - switch { - case disabled && len(providers) > 0: - return invalid(`{"enabled":false} cannot be combined with a provider`) - case !disabled && len(providers) == 0: - return nil, fmt.Errorf("a credential provider is required; set one of %s: %s", - channelsFor(credentialProviderKey), credentialProviderUsage) - } return providers, nil } diff --git a/cmd/ate-setup/internal/config/config_test.go b/cmd/ate-setup/internal/config/config_test.go index 09fc9b11e9..1fef3fed4f 100644 --- a/cmd/ate-setup/internal/config/config_test.go +++ b/cmd/ate-setup/internal/config/config_test.go @@ -1047,7 +1047,7 @@ func TestLoadRejectsInvalidValues(t *testing.T) { {"provider null", Options{CredentialProvider: `null`}}, {"provider without a name", Options{CredentialProvider: `{"address":"vault.ate-system.svc:8200"}`}}, {"provider with an unknown key", Options{CredentialProvider: `{"name":"k8s.io","adress":"x:1"}`}}, - {"disabled beside a provider", Options{CredentialProvider: `{"enabled":false} {"name":"k8s.io"}`}}, + {"provider with trailing data", Options{CredentialProvider: `{"enabled":false} {"name":"k8s.io"}`}}, {"provider name with a scheme", Options{CredentialProvider: `{"name":"ate-secret://k8s.io"}`}}, {"provider name not lowercase", Options{CredentialProvider: `{"name":"Vault.example.com","address":"vault.ate-system.svc:8200"}`}}, {"provider name with a port", Options{CredentialProvider: `{"name":"k8s.io:443"}`}}, @@ -1059,6 +1059,10 @@ func TestLoadRejectsInvalidValues(t *testing.T) { {"kubernetes address without a port", Options{CredentialProvider: `{"name":"k8s.io","address":"secrets.ate-system.svc"}`}}, {"other provider without an address", Options{CredentialProvider: `{"name":"vault.example.com"}`}}, {"other provider address without a port", Options{CredentialProvider: `{"name":"vault.example.com","address":"vault.ate-system.svc"}`}}, + {"provider array empty", Options{CredentialProvider: `[]`}}, + {"provider array entry disabled", Options{CredentialProvider: `[{"enabled":false}]`}}, + {"provider array with a repeated name", Options{CredentialProvider: `[{"name":"k8s.io"},{"name":"k8s.io"}]`}}, + {"provider array entry without an address", Options{CredentialProvider: `[{"name":"k8s.io"},{"name":"vault.example.com"}]`}}, } { t.Run(tc.name, func(t *testing.T) { if _, err := Load(tc.opts); err == nil { @@ -1158,58 +1162,34 @@ func TestCredentialProvider(t *testing.T) { } } -func TestSeveralCredentialProviders(t *testing.T) { +func TestCredentialProvidersArray(t *testing.T) { loadEnv(t) - const k8sJSON = `{"name":"k8s.io"}` - const stsJSON = `{"name":"sts.example.com","address":"sts.ate-system.svc:50051"}` - both := []CredentialProvider{ - {Name: K8sCredentialProviderName, Address: K8sCredentialProviderAddress}, - {Name: "sts.example.com", Address: "sts.ate-system.svc:50051"}, - } + k8s := CredentialProvider{Name: K8sCredentialProviderName, Address: K8sCredentialProviderAddress} for _, tc := range []struct { name string - flags []string - env string - file string + raw string want []CredentialProvider wantErr string }{ - {name: "repeated flags", flags: []string{k8sJSON, stsJSON}, want: both}, - {name: "environment", env: k8sJSON + " " + stsJSON, want: both}, + {name: "an array of one", raw: `[{"name":"k8s.io"}]`, want: []CredentialProvider{k8s}}, { - name: "file with one object per line", - file: "atenet:\n egress:\n credentialProvider: |\n " + k8sJSON + "\n " + stsJSON + "\n", - want: both, + name: "several providers", + raw: `[{"name":"k8s.io"},{"name":"sts.example.com","address":"sts.ate-system.svc:50051"}]`, + want: []CredentialProvider{k8s, {Name: "sts.example.com", Address: "sts.ate-system.svc:50051"}}, }, - {name: "a repeated name", flags: []string{k8sJSON, k8sJSON}, wantErr: `provider name "k8s.io" repeats`}, - {name: "disabled beside a provider", flags: []string{`{"enabled":false}`, k8sJSON}, wantErr: "cannot be combined"}, - {name: "a JSON array", env: "[" + k8sJSON + "," + stsJSON + "]", wantErr: "cannot unmarshal array"}, - {name: "only whitespace", env: " \n", wantErr: "a credential provider is required"}, + {name: "a null entry", raw: `[null]`, wantErr: "array entries must be JSON objects"}, + {name: "a string entry", raw: `["k8s.io"]`, wantErr: "array entries must be JSON objects"}, } { t.Run(tc.name, func(t *testing.T) { - if tc.env != "" { - t.Setenv("ATE_CREDENTIAL_PROVIDER", tc.env) - } - fs := pflag.NewFlagSet("test", pflag.ContinueOnError) - BindFlags(fs) - for _, v := range tc.flags { - if err := fs.Set("credential-provider", v); err != nil { - t.Fatal(err) - } - } - var lo LoadOptions - if tc.file != "" { - lo.ConfigPath = writeConfig(t, validHeader+tc.file) - } - cfg, err := LoadFlags(fs, lo) + cfg, err := Load(Options{CredentialProvider: tc.raw}) if tc.wantErr != "" { if err == nil || !strings.Contains(err.Error(), tc.wantErr) { - t.Fatalf("LoadFlags() error = %v, want one containing %q", err, tc.wantErr) + t.Fatalf("Load() error = %v, want one containing %q", err, tc.wantErr) } return } if err != nil { - t.Fatalf("LoadFlags() error = %v", err) + t.Fatalf("Load() error = %v", err) } got, err := cfg.CredentialProviders() if err != nil { diff --git a/cmd/ate-setup/internal/config/resolve.go b/cmd/ate-setup/internal/config/resolve.go index b1a5455dbe..ea79c34a43 100644 --- a/cmd/ate-setup/internal/config/resolve.go +++ b/cmd/ate-setup/internal/config/resolve.go @@ -17,7 +17,6 @@ package config import ( "fmt" "sort" - "strings" "time" "github.com/spf13/pflag" @@ -124,13 +123,6 @@ func Resolve(fs *pflag.FlagSet, opts ResolveOptions) (*Resolved, error) { // flag's printed form, which round-trips for bool and int. raw = fs.Lookup(s.Flag).Value.String() } - if s.Repeated { - values, err := fs.GetStringArray(s.Flag) - if err != nil { - return nil, fmt.Errorf("reading --%s: %w", s.Flag, err) - } - raw = strings.Join(values, " ") - } v = Value{Setting: s, Raw: raw, From: OriginFlag, Supplied: true} } diff --git a/cmd/ate-setup/internal/config/setting.go b/cmd/ate-setup/internal/config/setting.go index 6a966c4b06..6fe546a358 100644 --- a/cmd/ate-setup/internal/config/setting.go +++ b/cmd/ate-setup/internal/config/setting.go @@ -50,9 +50,6 @@ type Setting struct { Flag string // Kind is the value type. Resolve parses raw strings against it. Kind ValueKind - // Repeated lets the flag be given more than once. Resolve joins the - // values with spaces, so every channel must accept space-separated values. - Repeated bool // Default applies when no channel supplies a value. Default string // Secret suppresses the value wherever configuration is displayed. @@ -198,9 +195,11 @@ var Registry = []Setting{ "certificates. The podidentity signer issues spiffe://cluster.local/ns//sa/", }, { - // The egress gateway's credential providers; see CredentialProviders. + // The egress gateway's credential provider, as a JSON object. Upstream + // takes it as one value on one flag; it is carried the same way here + // so the two channels accept the same text. Key: "atenet.egress.credentialProvider", Env: "ATE_CREDENTIAL_PROVIDER", - Flag: "credential-provider", Kind: KindString, Repeated: true, + Flag: "credential-provider", Kind: KindString, Usage: "Credential provider the egress gateway injects credentials from, as JSON; " + "required by deploy ate-system and deploy atenet. " + credentialProviderUsage + ". A provider requires --atenet-dataplane=envoy", @@ -534,16 +533,12 @@ func (s Setting) bindable() bool { return s.Flag != "" && !s.Secret } // only from Set, never from registration, and Resolve reads a flag only when // Changed reports true. // -// Only bools and repeated settings get a typed flag. Ints and durations -// register as strings so that Setting.parse is the one thing deciding what -// they accept: pflag's Int reads base 0, so it takes "0x10" as 16 where parse -// rejects it, and its errors name no canonical key. One vocabulary across all -// three channels is worth more than the type shown in help. +// Only bools get a typed flag. Ints and durations register as strings so that +// Setting.parse is the one thing deciding what they accept: pflag's Int reads +// base 0, so it takes "0x10" as 16 where parse rejects it, and its errors name +// no canonical key. One vocabulary across all three channels is worth more +// than the type shown in help. func bind(fs *pflag.FlagSet, s Setting) { - if s.Repeated { - fs.StringArray(s.Flag, nil, s.Usage) - return - } if s.Kind == KindBool { // An empty Default yields nil from parse; the assertion then gives // false, which is right for a bool with no declared default. From 746d65b184ebc65168f0c5a660e6a8261e85544f Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 21:46:18 -0700 Subject: [PATCH 11/12] ate-setup: say why the credential provider setting is a JSON array The old comment described one JSON object and an upstream shell flag that no longer exists. Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/setting.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/ate-setup/internal/config/setting.go b/cmd/ate-setup/internal/config/setting.go index 6fe546a358..d3779d7422 100644 --- a/cmd/ate-setup/internal/config/setting.go +++ b/cmd/ate-setup/internal/config/setting.go @@ -195,9 +195,9 @@ var Registry = []Setting{ "certificates. The podidentity signer issues spiffe://cluster.local/ns//sa/", }, { - // The egress gateway's credential provider, as a JSON object. Upstream - // takes it as one value on one flag; it is carried the same way here - // so the two channels accept the same text. + // The egress gateway's credential providers: one JSON object, or a JSON array + // of them for several. A repeated flag would be ideal, but a Setting holds one + // string on every channel, so an array is the best available option. Key: "atenet.egress.credentialProvider", Env: "ATE_CREDENTIAL_PROVIDER", Flag: "credential-provider", Kind: KindString, Usage: "Credential provider the egress gateway injects credentials from, as JSON; " + From eb2d809aafa176d59d21cd9b9fbf7f39a22b2d1c Mon Sep 17 00:00:00 2001 From: Max Thompson Date: Wed, 28 Oct 2026 21:51:52 -0700 Subject: [PATCH 12/12] ate-setup: call the JSON array the least worst option Signed-off-by: Max Thompson --- cmd/ate-setup/internal/config/setting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ate-setup/internal/config/setting.go b/cmd/ate-setup/internal/config/setting.go index d3779d7422..9d9d2f4d13 100644 --- a/cmd/ate-setup/internal/config/setting.go +++ b/cmd/ate-setup/internal/config/setting.go @@ -197,7 +197,7 @@ var Registry = []Setting{ { // The egress gateway's credential providers: one JSON object, or a JSON array // of them for several. A repeated flag would be ideal, but a Setting holds one - // string on every channel, so an array is the best available option. + // string on every channel, so an array is the least worst option. Key: "atenet.egress.credentialProvider", Env: "ATE_CREDENTIAL_PROVIDER", Flag: "credential-provider", Kind: KindString, Usage: "Credential provider the egress gateway injects credentials from, as JSON; " +