diff --git a/.golangci.yml b/.golangci.yml index 526d47e3..1ece977b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,12 +1,29 @@ run: timeout: 5m +linters-settings: + gocritic: + enabled-tags: + - performance + lll: + line-length: 200 + misspell: + locale: US + staticcheck: + go: "1.20" + linters: disable-all: true enable: + - errcheck + - exportloopref + - forcetypeassert - goconst + - gocritic - gocyclo + - godot - gofmt + - gofumpt - goimports - gosec - gosimple @@ -17,4 +34,6 @@ linters: - prealloc - revive - staticcheck + - typecheck - unused + - whitespace diff --git a/cmd/server/main.go b/cmd/server/main.go index de17b7a5..ce1b5607 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -62,7 +62,10 @@ func main() { } if *versionInfo { - version.PrintVersion() + if err := version.PrintVersion(); err != nil { + klog.ErrorS(err, "failed to print version") + os.Exit(1) + } os.Exit(0) } @@ -117,7 +120,12 @@ func main() { pb.RegisterKeyManagementServiceServer(s, kmsServer) klog.InfoS("Listening for connections", "addr", listener.Addr().String()) - go s.Serve(listener) + go func() { + if err := s.Serve(listener); err != nil { + klog.ErrorS(err, "failed to serve") + os.Exit(1) + } + }() healthz := &plugin.HealthZ{ KMSServer: kmsServer, diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index d1bedd16..59d0e558 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -23,7 +23,7 @@ import ( "k8s.io/klog/v2" ) -// GetKeyvaultToken() returns token for Keyvault endpoint +// GetKeyvaultToken() returns token for Keyvault endpoint. func GetKeyvaultToken(config *config.AzureConfig, env *azure.Environment, resource string, proxyMode bool) (authorizer autorest.Authorizer, err error) { servicePrincipalToken, err := GetServicePrincipalToken(config, env.ActiveDirectoryEndpoint, resource, proxyMode) if err != nil { @@ -33,7 +33,7 @@ func GetKeyvaultToken(config *config.AzureConfig, env *azure.Environment, resour return authorizer, nil } -// GetServicePrincipalToken creates a new service principal token based on the configuration +// GetServicePrincipalToken creates a new service principal token based on the configuration. func GetServicePrincipalToken(config *config.AzureConfig, aadEndpoint, resource string, proxyMode bool) (adal.OAuthTokenProvider, error) { oauthConfig, err := adal.NewOAuthConfig(aadEndpoint, config.TenantID) if err != nil { @@ -106,7 +106,7 @@ func GetServicePrincipalToken(config *config.AzureConfig, aadEndpoint, resource return nil, fmt.Errorf("no credentials provided for accessing keyvault") } -// ParseAzureEnvironment returns azure environment by name +// ParseAzureEnvironment returns azure environment by name. func ParseAzureEnvironment(cloudName string) (*azure.Environment, error) { var env azure.Environment var err error @@ -119,7 +119,7 @@ func ParseAzureEnvironment(cloudName string) (*azure.Environment, error) { } // decodePkcs12 decodes a PKCS#12 client certificate by extracting the public certificate and -// the private RSA key +// the private RSA key. func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.PrivateKey, error) { privateKey, certificate, err := pkcs12.Decode(pkcs, password) if err != nil { @@ -133,13 +133,13 @@ func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.Private return certificate, rsaPrivateKey, nil } -// redactClientCredentials applies regex to a sensitive string and return the redacted value +// redactClientCredentials applies regex to a sensitive string and return the redacted value. func redactClientCredentials(sensitiveString string) string { - r, _ := regexp.Compile(`^(\S{4})(\S|\s)*(\S{4})$`) + r := regexp.MustCompile(`^(\S{4})(\S|\s)*(\S{4})$`) return r.ReplaceAllString(sensitiveString, "$1##### REDACTED #####$3") } -// addTargetTypeHeader adds the target header if proxy mode is enabled +// addTargetTypeHeader adds the target header if proxy mode is enabled. func addTargetTypeHeader(spt *adal.ServicePrincipalToken) *adal.ServicePrincipalToken { spt.SetSender(autorest.CreateSender( (func() autorest.SendDecorator { diff --git a/pkg/config/azure_config.go b/pkg/config/azure_config.go index d7f8cee4..bdd3f00d 100644 --- a/pkg/config/azure_config.go +++ b/pkg/config/azure_config.go @@ -8,7 +8,7 @@ import ( "k8s.io/klog/v2" ) -// AzureConfig is representing /etc/kubernetes/azure.json +// AzureConfig is representing /etc/kubernetes/azure.json. type AzureConfig struct { Cloud string `json:"cloud" yaml:"cloud"` TenantID string `json:"tenantId" yaml:"tenantId"` @@ -20,7 +20,7 @@ type AzureConfig struct { AADClientCertPassword string `json:"aadClientCertPassword" yaml:"aadClientCertPassword"` } -// GetAzureConfig returns configs in the azure.json cloud provider file +// GetAzureConfig returns configs in the azure.json cloud provider file. func GetAzureConfig(configFile string) (config *AzureConfig, err error) { cfg := AzureConfig{} diff --git a/pkg/metrics/exporter.go b/pkg/metrics/exporter.go index be580882..1031e076 100644 --- a/pkg/metrics/exporter.go +++ b/pkg/metrics/exporter.go @@ -11,7 +11,7 @@ const ( prometheusExporter = "prometheus" ) -// InitMetricsExporter initializes new exporter +// InitMetricsExporter initializes new exporter. func InitMetricsExporter(metricsBackend, metricsAddress string) error { exporter := strings.ToLower(metricsBackend) klog.InfoS("metrics backend", "exporter", exporter) diff --git a/pkg/metrics/prometheus_exporter.go b/pkg/metrics/prometheus_exporter.go index aab05f8a..2f5c22bb 100644 --- a/pkg/metrics/prometheus_exporter.go +++ b/pkg/metrics/prometheus_exporter.go @@ -18,7 +18,8 @@ func initPrometheusExporter(metricsAddress string) error { exporter, err := prometheus.InstallNewPipeline(prometheus.Config{ DefaultHistogramBoundaries: []float64{ 0.1, 0.2, 0.3, 0.4, 0.5, 1, 1.5, 2, 2.5, 3.0, 5.0, 10.0, 15.0, 30.0, - }}, + }, + }, ) if err != nil { return fmt.Errorf("failed to register prometheus exporter: %v", err) diff --git a/pkg/metrics/stats_reporter.go b/pkg/metrics/stats_reporter.go index a1cca3b5..83ebd2c2 100644 --- a/pkg/metrics/stats_reporter.go +++ b/pkg/metrics/stats_reporter.go @@ -14,32 +14,30 @@ const ( statusTypeKey = "status" operationTypeKey = "operation" kmsRequestMetricName = "kms_request" - // ErrorStatusTypeValue sets status tag to "error" + // ErrorStatusTypeValue sets status tag to "error". ErrorStatusTypeValue = "error" - // SuccessStatusTypeValue sets status tag to "success" + // SuccessStatusTypeValue sets status tag to "success". SuccessStatusTypeValue = "success" - // EncryptOperationTypeValue sets operation tag to "encrypt" + // EncryptOperationTypeValue sets operation tag to "encrypt". EncryptOperationTypeValue = "encrypt" - // DecryptOperationTypeValue sets operation tag to "decrypt" + // DecryptOperationTypeValue sets operation tag to "decrypt". DecryptOperationTypeValue = "decrypt" - // GrpcOperationTypeValue sets operation tag to "grpc" + // GrpcOperationTypeValue sets operation tag to "grpc". GrpcOperationTypeValue = "grpc" ) -var ( - kmsRequest metric.Float64ValueRecorder -) +var kmsRequest metric.Float64ValueRecorder type reporter struct { meter metric.Meter } -// StatsReporter reports metrics +// StatsReporter reports metrics. type StatsReporter interface { ReportRequest(ctx context.Context, operationType, status string, duration float64, errors ...string) } -// NewStatsReporter instantiates otel reporter +// NewStatsReporter instantiates otel reporter. func NewStatsReporter() StatsReporter { meter := global.Meter(instrumentationName) diff --git a/pkg/plugin/healthz.go b/pkg/plugin/healthz.go index dd0c8bc2..89d489f3 100644 --- a/pkg/plugin/healthz.go +++ b/pkg/plugin/healthz.go @@ -33,7 +33,7 @@ type HealthZ struct { RPCTimeout time.Duration } -// Serve creates the http handler for serving health requests +// Serve creates the http handler for serving health requests. func (h *HealthZ) Serve() { serveMux := http.NewServeMux() serveMux.HandleFunc(h.HealthCheckURL.EscapedPath(), h.ServeHTTP) @@ -85,7 +85,10 @@ func (h *HealthZ) ServeHTTP(w http.ResponseWriter, _ *http.Request) { return } w.WriteHeader(http.StatusOK) - w.Write([]byte("ok")) + if _, err = w.Write([]byte("ok")); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } klog.V(5).Info("Completed health check") } diff --git a/pkg/plugin/healthz_test.go b/pkg/plugin/healthz_test.go index 20819b3d..49fb9eee 100644 --- a/pkg/plugin/healthz_test.go +++ b/pkg/plugin/healthz_test.go @@ -144,7 +144,9 @@ func setupFakeKMSServer(socketPath string) (*KeyManagementServiceServer, *mockke } s := grpc.NewServer() pb.RegisterKeyManagementServiceServer(s, fakeKMSServer) - go s.Serve(listener) + go func() { + _ = s.Serve(listener) + }() return fakeKMSServer, kvClient, nil } diff --git a/pkg/plugin/keyvault.go b/pkg/plugin/keyvault.go index e376337c..276002a1 100644 --- a/pkg/plugin/keyvault.go +++ b/pkg/plugin/keyvault.go @@ -24,7 +24,7 @@ import ( "k8s.io/klog/v2" ) -// Client interface for interacting with Keyvault +// Client interface for interacting with Keyvault. type Client interface { Encrypt(ctx context.Context, cipher []byte) ([]byte, error) Decrypt(ctx context.Context, plain []byte) ([]byte, error) @@ -40,14 +40,15 @@ type keyVaultClient struct { azureEnvironment *azure.Environment } -// NewKeyVaultClient returns a new key vault client to use for kms operations +// NewKeyVaultClient returns a new key vault client to use for kms operations. func newKeyVaultClient( config *config.AzureConfig, vaultName, keyName, keyVersion string, proxyMode bool, proxyAddress string, proxyPort int, - managedHSM bool) (*keyVaultClient, error) { + managedHSM bool, +) (*keyVaultClient, error) { // Sanitize vaultName, keyName, keyVersion. (https://github.com/Azure/kubernetes-kms/issues/85) vaultName = utils.SanitizeString(vaultName) keyName = utils.SanitizeString(keyName) diff --git a/pkg/plugin/server.go b/pkg/plugin/server.go index 3b46410c..45ab8bc0 100644 --- a/pkg/plugin/server.go +++ b/pkg/plugin/server.go @@ -50,7 +50,7 @@ func New(pc *Config) (*KeyManagementServiceServer, error) { }, nil } -// Version of kms +// Version of kms. func (s *KeyManagementServiceServer) Version(_ context.Context, _ *k8spb.VersionRequest) (*k8spb.VersionResponse, error) { return &k8spb.VersionResponse{ Version: version.APIVersion, @@ -59,7 +59,7 @@ func (s *KeyManagementServiceServer) Version(_ context.Context, _ *k8spb.Version }, nil } -// Encrypt message +// Encrypt message. func (s *KeyManagementServiceServer) Encrypt(ctx context.Context, request *k8spb.EncryptRequest) (*k8spb.EncryptResponse, error) { start := time.Now() @@ -84,7 +84,7 @@ func (s *KeyManagementServiceServer) Encrypt(ctx context.Context, request *k8spb return &k8spb.EncryptResponse{Cipher: cipher}, nil } -// Decrypt message +// Decrypt message. func (s *KeyManagementServiceServer) Decrypt(ctx context.Context, request *k8spb.DecryptRequest) (*k8spb.DecryptResponse, error) { start := time.Now() diff --git a/pkg/plugin/server_test.go b/pkg/plugin/server_test.go index a75fcded..00780ae1 100644 --- a/pkg/plugin/server_test.go +++ b/pkg/plugin/server_test.go @@ -6,6 +6,7 @@ package plugin import ( + "bytes" "context" "fmt" "testing" @@ -54,7 +55,7 @@ func TestEncrypt(t *testing.T) { if err != test.err { t.Fatalf("expected err: %v, got: %v", test.err, err) } - if string(out.GetCipher()) != string(test.output) { + if !bytes.Equal(out.GetCipher(), test.output) { t.Fatalf("expected out: %v, got: %v", test.output, out) } }) @@ -98,7 +99,7 @@ func TestDecrypt(t *testing.T) { if err != test.err { t.Fatalf("expected err: %v, got: %v", test.err, err) } - if string(out.GetPlain()) != string(test.output) { + if !bytes.Equal(out.GetPlain(), test.output) { t.Fatalf("expected out: %v, got: %v", test.output, out) } }) diff --git a/pkg/utils/grpc.go b/pkg/utils/grpc.go index 6a08abe9..a31b8271 100644 --- a/pkg/utils/grpc.go +++ b/pkg/utils/grpc.go @@ -12,7 +12,7 @@ import ( "k8s.io/klog/v2" ) -// ParseEndpoint returns unix socket's protocol and address +// ParseEndpoint returns unix socket's protocol and address. func ParseEndpoint(ep string) (string, string, error) { if strings.HasPrefix(strings.ToLower(ep), "unix://") { s := strings.SplitN(ep, "://", 2) diff --git a/pkg/utils/sanitize.go b/pkg/utils/sanitize.go index 89a5bf1c..9bb8b430 100644 --- a/pkg/utils/sanitize.go +++ b/pkg/utils/sanitize.go @@ -2,7 +2,7 @@ package utils import "strings" -// SanitizeString returns a string that does not have white spaces and double quotes +// SanitizeString returns a string that does not have white spaces and double quotes. func SanitizeString(s string) string { return strings.TrimSpace(strings.Trim(strings.TrimSpace(s), "\"")) } diff --git a/pkg/version/version.go b/pkg/version/version.go index 192bc22e..9b3251ee 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -7,17 +7,17 @@ import ( ) var ( - // BuildDate is the date when the binary was built + // BuildDate is the date when the binary was built. BuildDate string - // GitCommit is the commit hash when the binary was built + // GitCommit is the commit hash when the binary was built. GitCommit string - // BuildVersion is the version of the KMS binary + // BuildVersion is the version of the KMS binary. BuildVersion string APIVersion = "v1beta1" Runtime = "Microsoft AzureKMS" ) -// PrintVersion prints the current KMS plugin version +// PrintVersion prints the current KMS plugin version. func PrintVersion() (err error) { pv := struct { BuildVersion string diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go index c6a2407c..52398fef 100644 --- a/pkg/version/version_test.go +++ b/pkg/version/version_test.go @@ -25,7 +25,7 @@ func TestPrintVersion(t *testing.T) { // copy the output in a separate goroutine so printing can't block indefinitely go func() { var buf bytes.Buffer - io.Copy(&buf, r) + _, _ = io.Copy(&buf, r) outC <- strings.TrimSpace(buf.String()) }() @@ -52,6 +52,5 @@ func TestGetUserAgent(t *testing.T) { expectedUserAgent := fmt.Sprintf("k8s-kms-keyvault/version (%s/%s) hash/Now", runtime.GOOS, runtime.GOARCH) if !strings.EqualFold(userAgent, expectedUserAgent) { t.Fatalf("string doesn't match, expected %s, got %s", expectedUserAgent, userAgent) - } } diff --git a/tests/client/client_test.go b/tests/client/client_test.go index 69188d86..fb28fb3b 100644 --- a/tests/client/client_test.go +++ b/tests/client/client_test.go @@ -1,6 +1,7 @@ package test import ( + "bytes" "fmt" "net" "testing" @@ -53,7 +54,6 @@ func TestEncryptDecrypt(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - encryptRequest := k8spb.EncryptRequest{Version: version, Plain: tc.want} encryptResponse, err := client.Encrypt(context.Background(), &encryptRequest) if err != nil { @@ -62,7 +62,7 @@ func TestEncryptDecrypt(t *testing.T) { decryptRequest := k8spb.DecryptRequest{Version: version, Cipher: encryptResponse.Cipher} decryptResponse, err := client.Decrypt(context.Background(), &decryptRequest) - if string(decryptResponse.Plain) != string(tc.want) { + if !bytes.Equal(decryptResponse.Plain, tc.want) { t.Fatalf("Expected secret, but got %s - %v", string(decryptResponse.Plain), err) } }) @@ -85,7 +85,6 @@ func TestVersion(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - request := &k8spb.VersionRequest{Version: tc.want} response, err := client.Version(context.Background(), request) if err != nil {