From 032831f69a4e52bb5cc7c10ca20d2167a4809d3e Mon Sep 17 00:00:00 2001 From: Carlo Teubner Date: Thu, 19 Dec 2024 23:08:25 +0000 Subject: [PATCH 1/2] golangci-lint: add nilerr & address issues Signed-off-by: Carlo Teubner --- .golangci.yml | 1 + pkg/common/container/process/winapi.go | 2 +- pkg/common/plugin/facade.go | 2 +- pkg/common/telemetry/statsd.go | 2 +- pkg/server/plugin/keymanager/awskms/awskms.go | 2 +- pkg/server/plugin/keymanager/awskms/awskms_test.go | 2 ++ 6 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9306540caf..b703f91a92 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,6 +19,7 @@ linters: - gosec - misspell - nakedret + - nilerr - unconvert - unparam - whitespace diff --git a/pkg/common/container/process/winapi.go b/pkg/common/container/process/winapi.go index 2634b319f3..232fda40f9 100644 --- a/pkg/common/container/process/winapi.go +++ b/pkg/common/container/process/winapi.go @@ -145,7 +145,7 @@ func (a *api) QuerySystemExtendedHandleInformation() ([]SystemHandleInformationE handlesList := (*SystemExtendedHandleInformation)(unsafe.Pointer(&buffer[0])) handles := unsafe.Slice(&handlesList.Handles[0], int(handlesList.NumberOfHandles)) - return handles, nil + return handles, nil //nolint:nilerr } return nil, status diff --git a/pkg/common/plugin/facade.go b/pkg/common/plugin/facade.go index 6468eb8130..f50329ef73 100644 --- a/pkg/common/plugin/facade.go +++ b/pkg/common/plugin/facade.go @@ -52,7 +52,7 @@ func (f *Facade) InitLog(log logrus.FieldLogger) { // that come out of plugin implementations. func (f *Facade) WrapErr(err error) error { if err == nil { - return err + return nil } // Embellish the gRPC status with the prefix, if necessary. diff --git a/pkg/common/telemetry/statsd.go b/pkg/common/telemetry/statsd.go index 69d1108d61..9e25cc9d13 100644 --- a/pkg/common/telemetry/statsd.go +++ b/pkg/common/telemetry/statsd.go @@ -16,7 +16,7 @@ func newStatsdRunner(c *MetricsConfig) (sinkRunner, error) { for _, sc := range c.FileConfig.Statsd { sink, err := metrics.NewStatsdSink(sc.Address) if err != nil { - return runner, nil + return nil, err } runner.loadedSinks = append(runner.loadedSinks, sink) diff --git a/pkg/server/plugin/keymanager/awskms/awskms.go b/pkg/server/plugin/keymanager/awskms/awskms.go index f2bdc74d1e..6a3b4589fc 100644 --- a/pkg/server/plugin/keymanager/awskms/awskms.go +++ b/pkg/server/plugin/keymanager/awskms/awskms.go @@ -807,7 +807,7 @@ func (p *Plugin) createDefaultPolicy(ctx context.Context) (*string, error) { roleName, err := roleNameFromARN(*result.Arn) if err != nil { // the server has not assumed any role, use default KMS policy and log a warn message - p.log.Warn("In a future version of SPIRE, it will be mandatory for the SPIRE servers to assume an AWS IAM Role when using the default AWS KMS key policy. Please assign an IAM role to this SPIRE Server instance.") + p.log.Warn("In a future version of SPIRE, it will be mandatory for the SPIRE servers to assume an AWS IAM Role when using the default AWS KMS key policy. Please assign an IAM role to this SPIRE Server instance.", reasonTag, err) return nil, nil } diff --git a/pkg/server/plugin/keymanager/awskms/awskms_test.go b/pkg/server/plugin/keymanager/awskms/awskms_test.go index 2a1d8695fb..0a2a8596df 100644 --- a/pkg/server/plugin/keymanager/awskms/awskms_test.go +++ b/pkg/server/plugin/keymanager/awskms/awskms_test.go @@ -719,6 +719,7 @@ func TestGenerateKey(t *testing.T) { { Level: logrus.WarnLevel, Message: "In a future version of SPIRE, it will be mandatory for the SPIRE servers to assume an AWS IAM Role when using the default AWS KMS key policy. Please assign an IAM role to this SPIRE Server instance.", + Data: logrus.Fields{reasonTag: `incomplete resource, expected 'resource-type/resource-id' but got "example-account-id"`}, }, }, }, @@ -734,6 +735,7 @@ func TestGenerateKey(t *testing.T) { { Level: logrus.WarnLevel, Message: "In a future version of SPIRE, it will be mandatory for the SPIRE servers to assume an AWS IAM Role when using the default AWS KMS key policy. Please assign an IAM role to this SPIRE Server instance.", + Data: logrus.Fields{reasonTag: `arn does not contain an assumed role: "arn:aws:sts::example-account-id:user/development"`}, }, }, }, From 76b30851e0c824d7ece9f35a499a954cea7ae12b Mon Sep 17 00:00:00 2001 From: Carlo Teubner Date: Thu, 19 Dec 2024 23:46:35 +0000 Subject: [PATCH 2/2] Change a few 'errors.New' to 'fmt.Errorf' These were not found by a linter but just something I noticed where it would be better to include a bit more info. Signed-off-by: Carlo Teubner --- pkg/server/server.go | 4 ++-- test/integration/setup/node-attestation/client.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index 6693e2fd2e..f34e143750 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -520,7 +520,7 @@ func (s *Server) CheckHealth() health.State { func (s *Server) tryGetBundle() error { client, err := server_util.NewServerClient(s.config.BindLocalAddress) if err != nil { - return errors.New("cannot create registration client") + return fmt.Errorf("cannot create registration client: %w", err) } defer client.Release() @@ -531,7 +531,7 @@ func (s *Server) tryGetBundle() error { // As currently coded however, the API isn't served until after // the server CA has been signed by upstream. if _, err := bundleClient.GetBundle(context.Background(), &bundlev1.GetBundleRequest{}); err != nil { - return errors.New("unable to fetch bundle") + return fmt.Errorf("unable to fetch bundle: %w", err) } return nil } diff --git a/test/integration/setup/node-attestation/client.go b/test/integration/setup/node-attestation/client.go index 8f113ee4a9..5b8ced7e57 100644 --- a/test/integration/setup/node-attestation/client.go +++ b/test/integration/setup/node-attestation/client.go @@ -230,7 +230,7 @@ func doX509popStep(ctx context.Context) error { // Ban agent if err := banAgent(ctx, client, svidResp.Id); err != nil { - return errors.New("failed to ban agent") + return fmt.Errorf("failed to ban agent: %w", err) } // Reattest banned agent, it MUST fail