From fe7973c060d7d26069d45a6cf973650e12e4f6ba Mon Sep 17 00:00:00 2001 From: max furman Date: Thu, 19 Sep 2019 13:17:23 -0700 Subject: [PATCH 1/2] wip --- .golangci.yml | 5 +++-- acme/authority.go | 8 ++++---- acme/authz.go | 7 ------- acme/directory_test.go | 3 --- api/revoke_test.go | 1 - api/ssh_test.go | 7 ------- authority/authority.go | 1 - authority/db_test.go | 2 +- authority/provisioner/timeduration_test.go | 4 ---- authority/tls.go | 1 - ca/acmeClient_test.go | 3 --- ca/bootstrap_test.go | 2 +- ca/ca.go | 11 +++++------ logging/responselogger.go | 9 +++++---- 14 files changed, 19 insertions(+), 45 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0ac34cf4..4defd73f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -49,6 +49,8 @@ linters: - misspell - ineffassign - deadcode + - staticcheck + - unused run: skip-dirs: @@ -60,10 +62,9 @@ issues: - declaration of "err" shadows declaration at line - should have a package comment, unless it's in another file for this package - error strings should not be capitalized or end with punctuation or a newline - - declaration of "authz" shadows declaration at line # golangci.com configuration # https://github.com/golangci/golangci/wiki/Configuration service: - golangci-lint-version: 1.17.x # use the fixed version to not introduce new linters unexpectedly + golangci-lint-version: 1.18.x # use the fixed version to not introduce new linters unexpectedly prepare: - echo "here I can run custom commands, but no preparation needed for this repo" diff --git a/acme/authority.go b/acme/authority.go index a56d5ac2..a96b1f49 100644 --- a/acme/authority.go +++ b/acme/authority.go @@ -214,18 +214,18 @@ func (a *Authority) FinalizeOrder(p provisioner.Interface, accID, orderID string // GetAuthz retrieves and attempts to update the status on an ACME authz // before returning. func (a *Authority) GetAuthz(p provisioner.Interface, accID, authzID string) (*Authz, error) { - authz, err := getAuthz(a.db, authzID) + az, err := getAuthz(a.db, authzID) if err != nil { return nil, err } - if accID != authz.getAccountID() { + if accID != az.getAccountID() { return nil, UnauthorizedErr(errors.New("account does not own authz")) } - authz, err = authz.updateStatus(a.db) + az, err = az.updateStatus(a.db) if err != nil { return nil, Wrap(err, "error updating authz status") } - return authz.toACME(a.db, a.dir, p) + return az.toACME(a.db, a.dir, p) } // ValidateChallenge attempts to validate the challenge. diff --git a/acme/authz.go b/acme/authz.go index 132977e1..27e98051 100644 --- a/acme/authz.go +++ b/acme/authz.go @@ -195,13 +195,6 @@ func (ba *baseAuthz) clone() *baseAuthz { return &u } -func (ba *baseAuthz) storeAndReturnError(db nosql.DB, err *Error) error { - clone := ba.clone() - clone.Error = err - clone.save(db, ba) - return err -} - func (ba *baseAuthz) parent() authz { return &dnsAuthz{ba} } diff --git a/acme/directory_test.go b/acme/directory_test.go index 14fd421f..2cfc8bb8 100644 --- a/acme/directory_test.go +++ b/acme/directory_test.go @@ -16,9 +16,6 @@ func TestDirectoryGetLink(t *testing.T) { prov := newProv() provID := URLSafeProvisionerName(prov) - type newTest struct { - actual, expected string - } assert.Equals(t, dir.getLink(NewNonceLink, provID, true), fmt.Sprintf("https://ca.smallstep.com/acme/%s/new-nonce", provID)) assert.Equals(t, dir.getLink(NewNonceLink, provID, false), fmt.Sprintf("/%s/new-nonce", provID)) diff --git a/api/revoke_test.go b/api/revoke_test.go index fed4a178..477d90e8 100644 --- a/api/revoke_test.go +++ b/api/revoke_test.go @@ -74,7 +74,6 @@ func Test_caHandler_Revoke(t *testing.T) { input string auth Authority tls *tls.ConnectionState - err error statusCode int expected []byte } diff --git a/api/ssh_test.go b/api/ssh_test.go index f37bcad8..9deb5c88 100644 --- a/api/ssh_test.go +++ b/api/ssh_test.go @@ -260,13 +260,6 @@ func Test_caHandler_SignSSH(t *testing.T) { }) assert.FatalError(t, err) - type fields struct { - Authority Authority - } - type args struct { - w http.ResponseWriter - r *http.Request - } tests := []struct { name string req []byte diff --git a/authority/authority.go b/authority/authority.go index 399738d6..03fd1a99 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -26,7 +26,6 @@ type Authority struct { intermediateIdentity *x509util.Identity sshCAUserCertSignKey crypto.Signer sshCAHostCertSignKey crypto.Signer - validateOnce bool certificates *sync.Map startTime time.Time provisioners *provisioner.Collection diff --git a/authority/db_test.go b/authority/db_test.go index 870a3984..e3834b99 100644 --- a/authority/db_test.go +++ b/authority/db_test.go @@ -8,7 +8,7 @@ import ( type MockAuthDB struct { err error - ret1, ret2 interface{} + ret1 interface{} init func(*db.Config) (db.AuthDB, error) isRevoked func(string) (bool, error) revoke func(rci *db.RevokedCertificateInfo) error diff --git a/authority/provisioner/timeduration_test.go b/authority/provisioner/timeduration_test.go index 65bd6f96..bd855d5f 100644 --- a/authority/provisioner/timeduration_test.go +++ b/authority/provisioner/timeduration_test.go @@ -248,10 +248,6 @@ func TestTimeDuration_Unix(t *testing.T) { func TestTimeDuration_String(t *testing.T) { tm, fn := mockNow() defer fn() - type fields struct { - t time.Time - d time.Duration - } tests := []struct { name string timeDuration *TimeDuration diff --git a/authority/tls.go b/authority/tls.go index d54e4373..84227667 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -212,7 +212,6 @@ type RevokeOptions struct { MTLS bool Crt *x509.Certificate OTT string - errCtxt map[string]interface{} } // Revoke revokes a certificate. diff --git a/ca/acmeClient_test.go b/ca/acmeClient_test.go index d4218cd8..5163101a 100644 --- a/ca/acmeClient_test.go +++ b/ca/acmeClient_test.go @@ -22,7 +22,6 @@ import ( func TestNewACMEClient(t *testing.T) { type test struct { - endpoint string ops []ClientOption r1, r2 interface{} rc1, rc2 int @@ -357,8 +356,6 @@ func TestACMEClient_post(t *testing.T) { func TestACMEClient_NewOrder(t *testing.T) { type test struct { - payload []byte - jwk *jose.JSONWebKey ops []withHeaderOption r1, r2 interface{} rc1, rc2 int diff --git a/ca/bootstrap_test.go b/ca/bootstrap_test.go index 09565b1d..baa23d7e 100644 --- a/ca/bootstrap_test.go +++ b/ca/bootstrap_test.go @@ -570,8 +570,8 @@ func TestBootstrapListener(t *testing.T) { return } wg := new(sync.WaitGroup) + wg.Add(1) go func() { - wg.Add(1) http.Serve(lis, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("ok")) })) diff --git a/ca/ca.go b/ca/ca.go index 5b706b09..a8af2f23 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -63,12 +63,11 @@ func WithDatabase(db db.AuthDB) Option { // CA is the type used to build the complete certificate authority. It builds // the HTTP server, set ups the middlewares and the HTTP handlers. type CA struct { - auth *authority.Authority - acmeAuth *acme.Authority - config *authority.Config - srv *server.Server - opts *options - renewer *TLSRenewer + auth *authority.Authority + config *authority.Config + srv *server.Server + opts *options + renewer *TLSRenewer } // New creates and initializes the CA with the given configuration and options. diff --git a/logging/responselogger.go b/logging/responselogger.go index e93d4dba..8e052bfa 100644 --- a/logging/responselogger.go +++ b/logging/responselogger.go @@ -2,6 +2,7 @@ package logging import ( "bufio" + "context" "net" "net/http" ) @@ -30,7 +31,7 @@ func NewResponseLogger(w http.ResponseWriter) ResponseLogger { func wrapLogger(w http.ResponseWriter) (rw ResponseLogger) { rw = &rwDefault{w, 200, 0, nil} - if c, ok := w.(http.CloseNotifier); ok { + if c, ok := w.(context.Context); ok { rw = &rwCloseNotifier{rw, c} } if f, ok := w.(http.Flusher); ok { @@ -90,11 +91,11 @@ func (r *rwDefault) WithFields(fields map[string]interface{}) { type rwCloseNotifier struct { ResponseLogger - c http.CloseNotifier + ctx context.Context } -func (r *rwCloseNotifier) CloseNotify() <-chan bool { - return r.CloseNotify() +func (r *rwCloseNotifier) Done() <-chan struct{} { + return r.Done() } type rwFlusher struct { From 3d46bc13f52ed92145504c25516160aee0ffef4f Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 19 Sep 2019 14:36:11 -0700 Subject: [PATCH 2/2] Remove http.CloseNotifier wrapper. It's deprecated. --- logging/responselogger.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/logging/responselogger.go b/logging/responselogger.go index 8e052bfa..6f992ac0 100644 --- a/logging/responselogger.go +++ b/logging/responselogger.go @@ -2,7 +2,6 @@ package logging import ( "bufio" - "context" "net" "net/http" ) @@ -31,9 +30,6 @@ func NewResponseLogger(w http.ResponseWriter) ResponseLogger { func wrapLogger(w http.ResponseWriter) (rw ResponseLogger) { rw = &rwDefault{w, 200, 0, nil} - if c, ok := w.(context.Context); ok { - rw = &rwCloseNotifier{rw, c} - } if f, ok := w.(http.Flusher); ok { rw = &rwFlusher{rw, f} } @@ -89,15 +85,6 @@ func (r *rwDefault) WithFields(fields map[string]interface{}) { } } -type rwCloseNotifier struct { - ResponseLogger - ctx context.Context -} - -func (r *rwCloseNotifier) Done() <-chan struct{} { - return r.Done() -} - type rwFlusher struct { ResponseLogger f http.Flusher