Don't contact trust_bundle_url needlessly

cmd/spire-agent/cli/run/run.go

@@ -471,18 +471,15 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool)
 	}
 	ac.DisableSPIFFECertValidation = c.Agent.SDS.DisableSPIFFECertValidation
 
-	ac.InsecureBootstrap = c.Agent.InsecureBootstrap
 	ts := &trustbundlesources.Config{
 		InsecureBootstrap:     c.Agent.InsecureBootstrap,
 		TrustBundleFormat:     c.Agent.TrustBundleFormat,
 		TrustBundlePath:       c.Agent.TrustBundlePath,
 		TrustBundleURL:        c.Agent.TrustBundleURL,
 		TrustBundleUnixSocket: c.Agent.TrustBundleUnixSocket,
 	}
-	err = trustbundlesources.SetupTrustBundle(ac, ts)
-	if err != nil {
-		return nil, err
-	}
+
+	ac.TrustBundleSources = trustbundlesources.New(ts, ac.Log.WithField("Logger", "TrustBundleSources"))
 
 	ac.WorkloadKeyType = workloadkey.ECP256
 	if c.Agent.WorkloadX509SVIDKeyType != "" {

cmd/spire-agent/cli/run/run_test.go

@@ -566,7 +566,7 @@ func TestNewAgentConfig(t *testing.T) {
 				c.Agent.InsecureBootstrap = false
 			},
 			test: func(t *testing.T, c *agent.Config) {
-				require.False(t, c.InsecureBootstrap)
+				require.False(t, c.TrustBundleSources.GetInsecureBootstrap())
 			},
 		},
 		{
@@ -578,7 +578,7 @@ func TestNewAgentConfig(t *testing.T) {
 				c.Agent.InsecureBootstrap = true
 			},
 			test: func(t *testing.T, c *agent.Config) {
-				require.True(t, c.InsecureBootstrap)
+				require.True(t, c.TrustBundleSources.GetInsecureBootstrap())
 			},
 		},
 		{

pkg/agent/agent.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"crypto/x509"
 	"errors"
 	"fmt"
 	"net/http"
@@ -114,13 +115,20 @@ func (a *Agent) Run(ctx context.Context) error {
 		)
 
 		for {
-			as, err = a.attest(ctx, sto, cat, metrics, nodeAttestor)
-			if err == nil {
-				break
+			insecureBootstrap := false
+			bootstrapTrustBundle, err := sto.LoadBundle()
+			if errors.Is(err, storage.ErrNotCached) {
+				bootstrapTrustBundle, insecureBootstrap, err = a.c.TrustBundleSources.GetBundle()
 			}
-
-			if status.Code(err) == codes.PermissionDenied {
-				return err
+			if err == nil {
+				as, err = a.attest(ctx, sto, cat, metrics, nodeAttestor, bootstrapTrustBundle, insecureBootstrap)
+				if err == nil {
+					break
+				}
+
+				if status.Code(err) == codes.PermissionDenied {
+					return err
+				}
 			}
 
 			nextDuration := attBackoff.NextBackOff()
@@ -141,7 +149,15 @@ func (a *Agent) Run(ctx context.Context) error {
 			}
 		}
 	} else {
-		as, err = a.attest(ctx, sto, cat, metrics, nodeAttestor)
+		insecureBootstrap := false
+		bootstrapTrustBundle, err := sto.LoadBundle()
+		if errors.Is(err, storage.ErrNotCached) {
+			bootstrapTrustBundle, insecureBootstrap, err = a.c.TrustBundleSources.GetBundle()
+		}
+		if err != nil {
+			return err
+		}
+		as, err = a.attest(ctx, sto, cat, metrics, nodeAttestor, bootstrapTrustBundle, insecureBootstrap)
 		if err != nil {
 			return err
 		}
@@ -249,19 +265,19 @@ func (a *Agent) setupProfiling(ctx context.Context) (stop func()) {
 	}
 }
 
-func (a *Agent) attest(ctx context.Context, sto storage.Storage, cat catalog.Catalog, metrics telemetry.Metrics, na nodeattestor.NodeAttestor) (*node_attestor.AttestationResult, error) {
+func (a *Agent) attest(ctx context.Context, sto storage.Storage, cat catalog.Catalog, metrics telemetry.Metrics, na nodeattestor.NodeAttestor, bootstrapTrustBundle []*x509.Certificate, insecureBootstrap bool) (*node_attestor.AttestationResult, error) {
 	config := node_attestor.Config{
-		Catalog:           cat,
-		Metrics:           metrics,
-		JoinToken:         a.c.JoinToken,
-		TrustDomain:       a.c.TrustDomain,
-		TrustBundle:       a.c.TrustBundle,
-		InsecureBootstrap: a.c.InsecureBootstrap,
-		Storage:           sto,
-		Log:               a.c.Log.WithField(telemetry.SubsystemName, telemetry.Attestor),
-		ServerAddress:     a.c.ServerAddress,
-		NodeAttestor:      na,
-		TLSPolicy:         a.c.TLSPolicy,
+		Catalog:              cat,
+		Metrics:              metrics,
+		JoinToken:            a.c.JoinToken,
+		TrustDomain:          a.c.TrustDomain,
+		BootstrapTrustBundle: bootstrapTrustBundle,
+		InsecureBootstrap:    insecureBootstrap,
+		Storage:              sto,
+		Log:                  a.c.Log.WithField(telemetry.SubsystemName, telemetry.Attestor),
+		ServerAddress:        a.c.ServerAddress,
+		NodeAttestor:         na,
+		TLSPolicy:            a.c.TLSPolicy,
 	}
 	return node_attestor.New(&config).Attest(ctx)
 }

pkg/agent/attestor/node/node.go

@@ -48,17 +48,17 @@ type Attestor interface {
 }
 
 type Config struct {
-	Catalog           catalog.Catalog
-	Metrics           telemetry.Metrics
-	JoinToken         string
-	TrustDomain       spiffeid.TrustDomain
-	TrustBundle       []*x509.Certificate
-	InsecureBootstrap bool
-	Storage           storage.Storage
-	Log               logrus.FieldLogger
-	ServerAddress     string
-	NodeAttestor      nodeattestor.NodeAttestor
-	TLSPolicy         tlspolicy.Policy
+	Catalog              catalog.Catalog
+	Metrics              telemetry.Metrics
+	JoinToken            string
+	TrustDomain          spiffeid.TrustDomain
+	BootstrapTrustBundle []*x509.Certificate
+	InsecureBootstrap    bool
+	Storage              storage.Storage
+	Log                  logrus.FieldLogger
+	ServerAddress        string
+	NodeAttestor         nodeattestor.NodeAttestor
+	TLSPolicy            tlspolicy.Policy
 }
 
 type attestor struct {
@@ -157,12 +157,12 @@ func (a *attestor) loadBundle() (*spiffebundle.Bundle, error) {
 	bundle, err := a.c.Storage.LoadBundle()
 	if errors.Is(err, storage.ErrNotCached) {
 		if a.c.InsecureBootstrap {
-			if len(a.c.TrustBundle) > 0 {
+			if len(a.c.BootstrapTrustBundle) > 0 {
 				a.c.Log.Warn("Trust bundle will be ignored; performing insecure bootstrap")
 			}
 			return nil, nil
 		}
-		bundle = a.c.TrustBundle
+		bundle = a.c.BootstrapTrustBundle
 	} else if err != nil {
 		return nil, fmt.Errorf("load bundle: %w", err)
 	}

pkg/agent/attestor/node/node_test.go

@@ -317,16 +317,16 @@ func TestAttestor(t *testing.T) {
 			// create the attestor
 			log, _ := test.NewNullLogger()
 			attestor := attestor.New(&attestor.Config{
-				Catalog:           catalog,
-				Metrics:           telemetry.Blackhole{},
-				JoinToken:         testCase.agentService.joinToken,
-				Storage:           sto,
-				Log:               log,
-				TrustDomain:       trustDomain,
-				TrustBundle:       makeTrustBundle(testCase.bootstrapBundle),
-				InsecureBootstrap: testCase.insecureBootstrap,
-				ServerAddress:     listener.Addr().String(),
-				NodeAttestor:      agentNA,
+				Catalog:              catalog,
+				Metrics:              telemetry.Blackhole{},
+				JoinToken:            testCase.agentService.joinToken,
+				Storage:              sto,
+				Log:                  log,
+				TrustDomain:          trustDomain,
+				BootstrapTrustBundle: makeTrustBundle(testCase.bootstrapBundle),
+				InsecureBootstrap:    testCase.insecureBootstrap,
+				ServerAddress:        listener.Addr().String(),
+				NodeAttestor:         agentNA,
 			})
 
 			// perform attestation

pkg/agent/config.go

@@ -2,12 +2,12 @@ package agent
 
 import (
 	"context"
-	"crypto/x509"
 	"net"
 	"time"
 
 	"github.com/sirupsen/logrus"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
+	"github.com/spiffe/spire/pkg/agent/trustbundlesources"
 	"github.com/spiffe/spire/pkg/agent/workloadkey"
 	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/spiffe/spire/pkg/common/health"
@@ -37,9 +37,6 @@ type Config struct {
 	// The TLS Certificate resource name to use for the default X509-SVID with Envoy SDS
 	DefaultSVIDName string
 
-	// If true, the agent will bootstrap insecurely with the server
-	InsecureBootstrap bool
-
 	// If true, the agent retries bootstrap with backoff
 	RetryBootstrap bool
 
@@ -75,7 +72,9 @@ type Config struct {
 
 	// Trust domain and associated CA bundle
 	TrustDomain spiffeid.TrustDomain
-	TrustBundle []*x509.Certificate
+
+	// Sources for getting Trust Bundles
+	TrustBundleSources *trustbundlesources.Bundle
 
 	// Join token to use for attestation, if needed
 	JoinToken string

pkg/agent/trustbundlesources/bundle.go

@@ -10,50 +10,60 @@ import (
 	"net/http"
 	"os"
 
+	"github.com/sirupsen/logrus"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
-	"github.com/spiffe/spire/pkg/agent"
 	"github.com/spiffe/spire/pkg/common/bundleutil"
 	"github.com/spiffe/spire/pkg/common/pemutil"
 )
 
-func SetupTrustBundle(ac *agent.Config, bconfig *Config) error {
-	// Either download the trust bundle if TrustBundleURL is set, or read it
-	// from disk if TrustBundlePath is set
-	ac.InsecureBootstrap = bconfig.InsecureBootstrap
+type Bundle struct {
+	config *Config
+	log    logrus.FieldLogger
+}
 
+func New(config *Config, log logrus.FieldLogger) *Bundle {
+	return &Bundle{
+		config: config,
+		log:    log,
+	}
+}
+
+func (b *Bundle) GetBundle() ([]*x509.Certificate, bool, error) {
 	var bundleBytes []byte
 	var err error
 
 	switch {
-	case bconfig.TrustBundleURL != "":
-		bundleBytes, err = downloadTrustBundle(bconfig.TrustBundleURL, bconfig.TrustBundleUnixSocket)
+	case b.config.TrustBundleURL != "":
+		bundleBytes, err = downloadTrustBundle(b.config.TrustBundleURL, b.config.TrustBundleUnixSocket)
 		if err != nil {
-			return err
+			return nil, false, err
 		}
-	case bconfig.TrustBundlePath != "":
-		bundleBytes, err = loadTrustBundle(bconfig.TrustBundlePath)
+	case b.config.TrustBundlePath != "":
+		bundleBytes, err = loadTrustBundle(b.config.TrustBundlePath)
 		if err != nil {
-			return fmt.Errorf("could not parse trust bundle: %w", err)
+			return nil, false, fmt.Errorf("could not parse trust bundle: %w", err)
 		}
 	default:
 		// If InsecureBootstrap is configured, the bundle is not required
-		if bconfig.InsecureBootstrap {
-			return nil
+		if b.config.InsecureBootstrap {
+			return nil, true, nil
 		}
 	}
 
-	bundle, err := parseTrustBundle(bundleBytes, bconfig.TrustBundleFormat)
+	bundle, err := parseTrustBundle(bundleBytes, b.config.TrustBundleFormat)
 	if err != nil {
-		return err
+		return nil, false, err
 	}
 
 	if len(bundle) == 0 {
-		return errors.New("no certificates found in trust bundle")
+		return nil, false, errors.New("no certificates found in trust bundle")
 	}
 
-	ac.TrustBundle = bundle
+	return bundle, false, nil
+}
 
-	return nil
+func (b *Bundle) GetInsecureBootstrap() bool {
+	return b.config.InsecureBootstrap
 }
 
 func parseTrustBundle(bundleBytes []byte, trustBundleContentType string) ([]*x509.Certificate, error) {

pkg/agent/trustbundlesources/bundle_test.go

@@ -10,12 +10,12 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/spiffe/spire/pkg/agent"
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/spiffe/spire/test/util"
 	"github.com/stretchr/testify/require"
 )
 
-func TestSetupTrustBundle(t *testing.T) {
+func TestGetBundle(t *testing.T) {
 	testTrustBundlePath := path.Join(util.ProjectRoot(), "conf/agent/dummy_root_ca.crt")
 	testTBSPIFFE := `{
     "keys": [
@@ -85,7 +85,6 @@ func TestSetupTrustBundle(t *testing.T) {
 	for _, testCase := range cases {
 		t.Run(testCase.msg, func(t *testing.T) {
 			var err error
-			var ac agent.Config
 			var c Config = Config{
 				InsecureBootstrap:     testCase.insecureBootstrap,
 				TrustBundlePath:       testCase.trustBundlePath,
@@ -100,14 +99,18 @@ func TestSetupTrustBundle(t *testing.T) {
 			if testCase.trustBundleURL {
 				c.TrustBundleURL = testServer.URL
 			}
-			err = SetupTrustBundle(&ac, &c)
+			log, _ := test.NewNullLogger()
+			tbs := New(&c, log)
+			require.NoError(t, err)
+
+			trustBundle, insecureBootstrap, err := tbs.GetBundle()
 			if testCase.error {
 				require.Error(t, err)
 			} else {
 				require.NoError(t, err)
-				require.Equal(t, ac.InsecureBootstrap, c.InsecureBootstrap)
+				require.Equal(t, insecureBootstrap, testCase.insecureBootstrap)
 				if testCase.trustBundlePath != "" {
-					require.Equal(t, len(ac.TrustBundle), 1)
+					require.Equal(t, len(trustBundle), 1)
 				}
 			}
 		})