Update all plugins and plugin unit tests to work with new plugin API.

The old API performed all configuration checks coupled with plugin
reconfiguration under the Configure() func.

The new API adds a Validation() func that only performs configuration
checks but has no impact on the running plugin.

To facilitate easier user, the pluginconf package was added that makes
it easier to handle the merged code streams through a pluginconf.Status
struct that will capture the first error (for integration with
Configure() while permitting the Validation() to capture all errors
that can be captured.

Unit tests had to be reworked, as a side-effect of using the new
pluginconf package is that all plugins now automatically check their
trustdomain, instead of each plugin checking it in a haphazard manner.

Occasionally, very small fixes were performed on plugins, and plugin
coding standards were tweaked in small ways to be more similar to each
other.

Signed-off-by: Edwin Buck <edwbuck@gmail.com>

go.mod

@@ -73,7 +73,7 @@ require (
 	github.com/sirupsen/logrus v1.9.3
 	github.com/spiffe/go-spiffe/v2 v2.3.0
 	github.com/spiffe/spire-api-sdk v1.2.5-0.20240807182354-18e423ce2c1c
-	github.com/spiffe/spire-plugin-sdk v1.4.4-0.20230721151831-bf67dde4721d
+	github.com/spiffe/spire-plugin-sdk v1.4.4-0.20240701180828-594312f4444d
 	github.com/stretchr/testify v1.9.0
 	github.com/uber-go/tally/v4 v4.1.16
 	github.com/valyala/fastjson v1.6.4

go.sum

@@ -1395,8 +1395,8 @@ github.com/spiffe/go-spiffe/v2 v2.3.0 h1:g2jYNb/PDMB8I7mBGL2Zuq/Ur6hUhoroxGQFyD6
 github.com/spiffe/go-spiffe/v2 v2.3.0/go.mod h1:Oxsaio7DBgSNqhAO9i/9tLClaVlfRok7zvJnTV8ZyIY=
 github.com/spiffe/spire-api-sdk v1.2.5-0.20240807182354-18e423ce2c1c h1:lK/B2paDUiqbngUGsLxDBmNX/BsG2yKxS8W/iGT+x2c=
 github.com/spiffe/spire-api-sdk v1.2.5-0.20240807182354-18e423ce2c1c/go.mod h1:4uuhFlN6KBWjACRP3xXwrOTNnvaLp1zJs8Lribtr4fI=
-github.com/spiffe/spire-plugin-sdk v1.4.4-0.20230721151831-bf67dde4721d h1:LCRQGU6vOqKLfRrG+GJQrwMwDILcAddAEIf4/1PaSVc=
-github.com/spiffe/spire-plugin-sdk v1.4.4-0.20230721151831-bf67dde4721d/go.mod h1:GA6o2PVLwyJdevT6KKt5ZXCY/ziAPna13y/seGk49Ik=
+github.com/spiffe/spire-plugin-sdk v1.4.4-0.20240701180828-594312f4444d h1:Upcyq8u1aWFHTQSEskwxBE2PehobpY+M21LXXDS/mPw=
+github.com/spiffe/spire-plugin-sdk v1.4.4-0.20240701180828-594312f4444d/go.mod h1:GA6o2PVLwyJdevT6KKt5ZXCY/ziAPna13y/seGk49Ik=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=

pkg/agent/plugin/nodeattestor/awsiid/iid.go

@@ -15,6 +15,7 @@ import (
 	configv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/service/common/config/v1"
 	"github.com/spiffe/spire/pkg/common/catalog"
 	caws "github.com/spiffe/spire/pkg/common/plugin/aws"
+	"github.com/spiffe/spire/pkg/common/pluginconf"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -40,6 +41,16 @@ type IIDAttestorConfig struct {
 	EC2MetadataEndpoint string `hcl:"ec2_metadata_endpoint"`
 }
 
+func buildConfig(coreConfig catalog.CoreConfig, hclText string, status *pluginconf.Status) *IIDAttestorConfig {
+	newConfig := &IIDAttestorConfig{}
+	if err := hcl.Decode(newConfig, hclText); err != nil {
+		status.ReportErrorf("unable to decode configuration: %v", err)
+		return nil
+	}
+
+	return newConfig
+}
+
 // IIDAttestorPlugin implements aws nodeattestation in the agent.
 type IIDAttestorPlugin struct {
 	nodeattestorv1.UnsafeNodeAttestorServer
@@ -155,20 +166,27 @@ func readStringAndClose(r io.ReadCloser) (string, error) {
 
 // Configure implements the Config interface method of the same name
 func (p *IIDAttestorPlugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (*configv1.ConfigureResponse, error) {
-	// Parse HCL config payload into config struct
-	config := &IIDAttestorConfig{}
-	if err := hcl.Decode(config, req.HclConfiguration); err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "unable to decode configuration: %v", err)
+	newConfig, _, err := pluginconf.Build(req, buildConfig)
+	if err != nil {
+		return nil, err
 	}
 
 	p.mtx.Lock()
 	defer p.mtx.Unlock()
-
-	p.config = config
+	p.config = newConfig
 
 	return &configv1.ConfigureResponse{}, nil
 }
 
+func (p *IIDAttestorPlugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*configv1.ValidateResponse, error) {
+	_, notes, err := pluginconf.Build(req, buildConfig)
+
+	return &configv1.ValidateResponse{
+		Valid: err == nil,
+		Notes: notes,
+	}, nil
+}
+
 func (p *IIDAttestorPlugin) getConfig() (*IIDAttestorConfig, error) {
 	p.mtx.RLock()
 	defer p.mtx.RUnlock()

pkg/agent/plugin/nodeattestor/awsiid/iid_test.go

@@ -18,8 +18,10 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
 	"github.com/fullsailor/pkcs7"
+	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	"github.com/spiffe/spire/pkg/agent/plugin/nodeattestor"
 	nodeattestortest "github.com/spiffe/spire/pkg/agent/plugin/nodeattestor/test"
+	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/spiffe/spire/pkg/common/pemutil"
 	"github.com/spiffe/spire/pkg/common/plugin/aws"
 	"github.com/spiffe/spire/test/plugintest"
@@ -93,6 +95,9 @@ func (s *Suite) SetupTest() {
 	}))
 
 	s.p = s.loadPlugin(
+		plugintest.CoreConfig(catalog.CoreConfig{ 
+			TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+		}),
 		plugintest.Configuref(`ec2_metadata_endpoint = "http://%s/latest"`, s.server.Listener.Addr()),
 	)
 
@@ -104,7 +109,8 @@ func (s *Suite) TearDownTest() {
 }
 
 func (s *Suite) TestErrorWhenNotConfigured() {
-	p := s.loadPlugin()
+	p := s.loadPlugin(
+	)
 
 	err := p.Attest(context.Background(), nil)
 	s.RequireGRPCStatus(err, codes.FailedPrecondition, "nodeattestor(aws_iid): not configured")
@@ -141,13 +147,21 @@ func (s *Suite) TestConfigure() {
 
 	var err error
 	s.loadPlugin(
+		plugintest.CoreConfig(catalog.CoreConfig{ 
+			TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+		}),
 		plugintest.CaptureConfigureError(&err),
 		plugintest.Configure("malformed"),
 	)
 	require.Error(err)
 
 	// success
-	s.loadPlugin(plugintest.Configure(""))
+	s.loadPlugin(
+		plugintest.CoreConfig(catalog.CoreConfig{ 
+			TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+		}),
+		plugintest.Configure(""),
+	)
 }
 
 func (s *Suite) loadPlugin(opts ...plugintest.Option) nodeattestor.NodeAttestor {

pkg/agent/plugin/nodeattestor/azuremsi/msi.go

@@ -11,6 +11,7 @@ import (
 	configv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/service/common/config/v1"
 	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/spiffe/spire/pkg/common/plugin/azure"
+	"github.com/spiffe/spire/pkg/common/pluginconf"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -40,6 +41,21 @@ type MSIAttestorConfig struct {
 	ResourceID string `hcl:"resource_id"`
 }
 
+func buildConfig(coreConfig catalog.CoreConfig, hclText string, status *pluginconf.Status) *MSIAttestorConfig {
+	newConfig := new(MSIAttestorConfig)
+	if err := hcl.Decode(newConfig, hclText); err != nil {
+		status.ReportErrorf("unable to decode configuration: %v", err)
+		return nil
+	}
+
+	if newConfig.ResourceID == "" {
+		newConfig.ResourceID = azure.DefaultMSIResourceID
+	}
+
+
+	return newConfig
+}
+
 type MSIAttestorPlugin struct {
 	nodeattestorv1.UnsafeNodeAttestorServer
 	configv1.UnsafeConfigServer
@@ -85,19 +101,27 @@ func (p *MSIAttestorPlugin) AidAttestation(stream nodeattestorv1.NodeAttestor_Ai
 }
 
 func (p *MSIAttestorPlugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (*configv1.ConfigureResponse, error) {
-	config := new(MSIAttestorConfig)
-	if err := hcl.Decode(config, req.HclConfiguration); err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "unable to decode configuration: %v", err)
+	newConfig, _, err := pluginconf.Build(req, buildConfig)
+	if err != nil {
+		return nil, err
 	}
 
-	if config.ResourceID == "" {
-		config.ResourceID = azure.DefaultMSIResourceID
-	}
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	p.config = newConfig
 
-	p.setConfig(config)
 	return &configv1.ConfigureResponse{}, nil
 }
 
+func (p *MSIAttestorPlugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*configv1.ValidateResponse, error) {
+	_, notes, err := pluginconf.Build(req, buildConfig)
+
+	return &configv1.ValidateResponse{
+		Valid: err == nil,
+		Notes: notes,
+	}, nil
+}
+
 func (p *MSIAttestorPlugin) getConfig() (*MSIAttestorConfig, error) {
 	p.mu.RLock()
 	defer p.mu.RUnlock()
@@ -107,8 +131,3 @@ func (p *MSIAttestorPlugin) getConfig() (*MSIAttestorConfig, error) {
 	return p.config, nil
 }
 
-func (p *MSIAttestorPlugin) setConfig(config *MSIAttestorConfig) {
-	p.mu.Lock()
-	defer p.mu.Unlock()
-	p.config = config
-}

pkg/agent/plugin/nodeattestor/azuremsi/msi_test.go

@@ -9,9 +9,11 @@ import (
 
 	jose "github.com/go-jose/go-jose/v4"
 	"github.com/go-jose/go-jose/v4/jwt"
+	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	"github.com/spiffe/spire/pkg/agent/plugin/nodeattestor"
 	nodeattestortest "github.com/spiffe/spire/pkg/agent/plugin/nodeattestor/test"
 	"github.com/spiffe/spire/pkg/common/plugin/azure"
+	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/spiffe/spire/test/plugintest"
 	"github.com/spiffe/spire/test/spiretest"
 	"google.golang.org/grpc/codes"
@@ -49,7 +51,12 @@ func (s *MSIAttestorSuite) TestAidAttestationNotConfigured() {
 func (s *MSIAttestorSuite) TestAidAttestationFailedToObtainToken() {
 	s.tokenErr = errors.New("FAILED")
 
-	attestor := s.loadAttestor(plugintest.Configure(""))
+	attestor := s.loadAttestor(
+		plugintest.CoreConfig(catalog.CoreConfig{
+                        TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+                }),
+		plugintest.Configure(""),
+	)
 	err := attestor.Attest(context.Background(), streamBuilder.Build())
 	s.RequireGRPCStatus(err, codes.Internal, "nodeattestor(azure_msi): unable to fetch token: FAILED")
 }
@@ -59,23 +66,46 @@ func (s *MSIAttestorSuite) TestAidAttestationSuccess() {
 
 	expectPayload := []byte(fmt.Sprintf(`{"token":%q}`, s.token))
 
-	attestor := s.loadAttestor(plugintest.Configure(""))
+	attestor := s.loadAttestor(
+		plugintest.CoreConfig(catalog.CoreConfig{
+                        TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+                }),
+		plugintest.Configure(""),
+	)
 	err := attestor.Attest(context.Background(), streamBuilder.ExpectAndBuild(expectPayload))
 	s.Require().NoError(err)
 }
 
 func (s *MSIAttestorSuite) TestConfigure() {
 	// malformed configuration
 	var err error
-	s.loadAttestor(plugintest.CaptureConfigureError(&err), plugintest.Configure("blah"))
+	s.loadAttestor(
+		plugintest.CaptureConfigureError(&err),
+		plugintest.CoreConfig(catalog.CoreConfig{
+                        TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+                }),
+		plugintest.Configure("blah"),
+	)
 	s.RequireGRPCStatusContains(err, codes.InvalidArgument, "unable to decode configuration")
 
 	// success
-	s.loadAttestor(plugintest.CaptureConfigureError(&err), plugintest.Configure(""))
+	s.loadAttestor(
+		plugintest.CaptureConfigureError(&err),
+		plugintest.CoreConfig(catalog.CoreConfig{
+                        TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+                }),
+		plugintest.Configure(""),
+	)
 	s.Require().NoError(err)
 
 	// success with resource_id
-	s.loadAttestor(plugintest.CaptureConfigureError(&err), plugintest.Configure(`resource_id = "foo"`))
+	s.loadAttestor(
+		plugintest.CaptureConfigureError(&err),
+		plugintest.CoreConfig(catalog.CoreConfig{
+                        TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"),
+                }),
+		plugintest.Configure(`resource_id = "foo"`),
+	)
 	s.Require().NoError(err)
 }
 

pkg/agent/plugin/nodeattestor/gcpiit/iit.go

@@ -16,6 +16,7 @@ import (
 	configv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/service/common/config/v1"
 	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/spiffe/spire/pkg/common/plugin/gcp"
+	"github.com/spiffe/spire/pkg/common/pluginconf"
 )
 
 const (
@@ -50,6 +51,24 @@ type IITAttestorConfig struct {
 	ServiceAccount    string `hcl:"service_account"`
 }
 
+func buildConfig(coreConfig catalog.CoreConfig, hclText string, status *pluginconf.Status) *IITAttestorConfig {
+	newConfig := &IITAttestorConfig{}
+	if err := hcl.Decode(newConfig, hclText); err != nil {
+		status.ReportErrorf("unable to decode configuration: %v", err)
+		return nil
+	}
+
+	if newConfig.ServiceAccount == "" {
+		newConfig.ServiceAccount = defaultServiceAccount
+	}
+
+	if newConfig.IdentityTokenHost == "" {
+		newConfig.IdentityTokenHost = defaultIdentityTokenHost
+	}
+
+	return newConfig
+}
+
 // NewIITAttestorPlugin creates a new IITAttestorPlugin.
 func New() *IITAttestorPlugin {
 	return &IITAttestorPlugin{}
@@ -76,26 +95,27 @@ func (p *IITAttestorPlugin) AidAttestation(stream nodeattestorv1.NodeAttestor_Ai
 }
 
 func (p *IITAttestorPlugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (*configv1.ConfigureResponse, error) {
-	config := &IITAttestorConfig{}
-	if err := hcl.Decode(config, req.HclConfiguration); err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "unable to decode configuration: %v", err)
-	}
-
-	if config.ServiceAccount == "" {
-		config.ServiceAccount = defaultServiceAccount
-	}
-
-	if config.IdentityTokenHost == "" {
-		config.IdentityTokenHost = defaultIdentityTokenHost
+	newConfig, _, err := pluginconf.Build(req, buildConfig)
+	if err != nil {
+		return nil, err
 	}
 
 	p.mtx.Lock()
 	defer p.mtx.Unlock()
-	p.config = config
+	p.config = newConfig
 
 	return &configv1.ConfigureResponse{}, nil
 }
 
+func (p *IITAttestorPlugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*configv1.ValidateResponse, error) {
+	_, notes, err := pluginconf.Build(req, buildConfig)
+
+	return &configv1.ValidateResponse{
+		Valid: err == nil,
+		Notes: notes,
+	}, nil
+}
+
 func (p *IITAttestorPlugin) getConfig() (*IITAttestorConfig, error) {
 	p.mtx.Lock()
 	defer p.mtx.Unlock()