Remove error output for all Validate() plugin commands.

This permits getting the entire output of the notes, and aligns with the desired output in the code review. As we don't know which plugin note was the failure, an additional note is added when the plugin's configuration is invalid. Signed-off-by: Edwin Buck <edwbuck@gmail.com>
spiffe · Dec 4, 2024 · d05c2d1 · d05c2d1
1 parent c0615b9
commit d05c2d1
Show file tree

Hide file tree

Showing 20 changed files with 20 additions and 24 deletions.
diff --git a/pkg/common/catalog/configure.go b/pkg/common/catalog/configure.go
@@ -85,7 +85,6 @@ func ValidatePlugin(ctx context.Context, coreConfig CoreConfig, configurer Confi
 		status.ReportErrorf("failed to load plugin data: %s", err.Error())
 		return status, err
 	}
-	status.ReportInfo("<validating plugin>")
 
 	return configurer.Validate(ctx, coreConfig, data)
 }
@@ -232,12 +231,9 @@ func (v1 *configurerV1) Validate(ctx context.Context, coreConfig CoreConfig, hcl
 		HclConfiguration:  hclConfiguration,
 	})
 	result := NewValidateResult()
-	// TODO: The GRPC default behavior is to not return a *ValidateResponse when there is an error, and that destroys many
-	//       of the validation messages, except for the error message.  We may wish to not consider validation errors
-	//       as errors going forward.
 	result.Notes = response.GetNotes()
 	if !response.GetValid() {
-		result.ReportErrorf("invalid configuration: %s", err.Error())
+		result.ReportError("plugin configuration is invalid")
 	}
 	return result, err
 }

diff --git a/pkg/server/plugin/bundlepublisher/awsrolesanywhere/awsrolesanywhere.go b/pkg/server/plugin/bundlepublisher/awsrolesanywhere/awsrolesanywhere.go
@@ -114,7 +114,7 @@ func (p *Plugin) Validate(ctx context.Context, req *configv1.ValidateRequest) (*
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 // PublishBundle puts the bundle in the Roles Anywhere trust anchor, with

diff --git a/pkg/server/plugin/bundlepublisher/awss3/awss3.go b/pkg/server/plugin/bundlepublisher/awss3/awss3.go
@@ -139,7 +139,7 @@ func (p *Plugin) Validate(ctx context.Context, req *configv1.ValidateRequest) (*
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 // PublishBundle puts the bundle in the configured S3 bucket name and

diff --git a/pkg/server/plugin/bundlepublisher/gcpcloudstorage/gcpcloudstorage.go b/pkg/server/plugin/bundlepublisher/gcpcloudstorage/gcpcloudstorage.go
@@ -138,7 +138,7 @@ func (p *Plugin) Validate(ctx context.Context, req *configv1.ValidateRequest) (*
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 // PublishBundle puts the bundle in the configured GCS bucket and object name.

diff --git a/pkg/server/plugin/keymanager/awskms/awskms.go b/pkg/server/plugin/keymanager/awskms/awskms.go
@@ -244,7 +244,7 @@ func (p *Plugin) Validate(ctx context.Context, req *configv1.ValidateRequest) (*
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 // GenerateKey creates a key in KMS. If a key already exists in the local storage, it is updated.

diff --git a/pkg/server/plugin/keymanager/disk/disk.go b/pkg/server/plugin/keymanager/disk/disk.go
@@ -91,7 +91,7 @@ func (m *KeyManager) Validate(_ context.Context, req *configv1.ValidateRequest)
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (m *KeyManager) configure(config *configuration) error {

diff --git a/pkg/server/plugin/nodeattestor/jointoken/join_token.go b/pkg/server/plugin/nodeattestor/jointoken/join_token.go
@@ -74,5 +74,5 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
diff --git a/pkg/server/plugin/nodeattestor/k8spsat/psat.go b/pkg/server/plugin/nodeattestor/k8spsat/psat.go
@@ -288,7 +288,7 @@ func (p *AttestorPlugin) Validate(ctx context.Context, req *configv1.ValidateReq
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *AttestorPlugin) getConfig() (*attestorConfig, error) {

diff --git a/pkg/server/plugin/nodeattestor/k8ssat/sat.go b/pkg/server/plugin/nodeattestor/k8ssat/sat.go
@@ -356,7 +356,7 @@ func (p *AttestorPlugin) Validate(_ context.Context, req *configv1.ValidateReque
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *AttestorPlugin) getConfig() (*attestorConfig, error) {

diff --git a/pkg/server/plugin/nodeattestor/tpmdevid/devid.go b/pkg/server/plugin/nodeattestor/tpmdevid/devid.go
@@ -242,7 +242,7 @@ func (p *Plugin) Validate(ctx context.Context, req *configv1.ValidateRequest) (*
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) getConfiguration() *config {

diff --git a/pkg/server/plugin/nodeattestor/x509pop/x509pop.go b/pkg/server/plugin/nodeattestor/x509pop/x509pop.go
@@ -223,7 +223,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) getConfig() (*configuration, error) {

diff --git a/pkg/server/plugin/notifier/gcsbundle/gcsbundle.go b/pkg/server/plugin/notifier/gcsbundle/gcsbundle.go
@@ -145,7 +145,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (res
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) getConfig() (*configuration, error) {

diff --git a/pkg/server/plugin/notifier/k8sbundle/k8sbundle.go b/pkg/server/plugin/notifier/k8sbundle/k8sbundle.go
@@ -175,7 +175,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 // startInformers creates informers to set CA Bundle in objects created after server has started

diff --git a/pkg/server/plugin/upstreamauthority/awspca/pca.go b/pkg/server/plugin/upstreamauthority/awspca/pca.go
@@ -196,7 +196,7 @@ func (p *PCAPlugin) Validate(ctx context.Context, req *configv1.ValidateRequest)
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 // MintX509CA mints an X509CA by submitting the CSR to ACM to be signed by the certificate authority

diff --git a/pkg/server/plugin/upstreamauthority/awssecret/awssecret.go b/pkg/server/plugin/upstreamauthority/awssecret/awssecret.go
@@ -157,7 +157,7 @@ func (p *Plugin) Validate(ctx context.Context, req *configv1.ValidateRequest) (*
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 // MintX509CAAndSubscribe mints an X509CA by signing presented CSR with root CA fetched from AWS Secrets Manager

diff --git a/pkg/server/plugin/upstreamauthority/certmanager/certmanager.go b/pkg/server/plugin/upstreamauthority/certmanager/certmanager.go
@@ -155,7 +155,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) MintX509CAAndSubscribe(request *upstreamauthorityv1.MintX509CARequest, stream upstreamauthorityv1.UpstreamAuthority_MintX509CAAndSubscribeServer) error {

diff --git a/pkg/server/plugin/upstreamauthority/disk/disk.go b/pkg/server/plugin/upstreamauthority/disk/disk.go
@@ -120,7 +120,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) MintX509CAAndSubscribe(request *upstreamauthorityv1.MintX509CARequest, stream upstreamauthorityv1.UpstreamAuthority_MintX509CAAndSubscribeServer) error {

diff --git a/pkg/server/plugin/upstreamauthority/gcpcas/gcpcas.go b/pkg/server/plugin/upstreamauthority/gcpcas/gcpcas.go
@@ -184,7 +184,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) getConfig() (*Configuration, error) {

diff --git a/pkg/server/plugin/upstreamauthority/spire/spire.go b/pkg/server/plugin/upstreamauthority/spire/spire.go
@@ -138,7 +138,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) SetLogger(log hclog.Logger) {

diff --git a/pkg/server/plugin/upstreamauthority/vault/vault.go b/pkg/server/plugin/upstreamauthority/vault/vault.go
@@ -186,7 +186,7 @@ func (p *Plugin) Validate(_ context.Context, req *configv1.ValidateRequest) (*co
 	return &configv1.ValidateResponse{
 		Valid: err == nil,
 		Notes: notes,
-	}, err
+	}, nil
 }
 
 func (p *Plugin) MintX509CAAndSubscribe(req *upstreamauthorityv1.MintX509CARequest, stream upstreamauthorityv1.UpstreamAuthority_MintX509CAAndSubscribeServer) error {