Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run additional linters as pre-commit hooks #1798

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,15 @@ repos:
rev: v1.56.2
hooks:
- id: golangci-lint
- repo: https://github.com/crate-ci/typos
rev: v1.28.2
hooks:
- id: typos
# Override the default arguments, which include writing back all
# suggested changes. We don't want false positives to mangle files.
args: [--force-exclude, --sort]
- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.10.0.1
hooks:
- id: shellcheck
args: [--severity=warning]
25 changes: 25 additions & 0 deletions _typos.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Configure 'typos' to reduce false positives.
# For more information, see <https://github.com/crate-ci/typos>.

[default]
extend-ignore-identifiers-re = [
## Ignore TLAs. Two and three letter acronyms are not worth fixing.
"^[A-Za-z]{2,3}$",
## Ignore the 'dne' identifiers in server_utils/origin_test.go.
"(.*)_dne$",
]

[default.extend-identifiers]
## The 'cipher' library uses "Encrypter"; 'typos' prefers "Encryptor".
NewCFBEncrypter = "NewCFBEncrypter"
## HTCondor's daemon names follow the usual "{word} + 'd'" convention.
StartdAttrs = "StartdAttrs"

[files]
extend-exclude = [
## Ignore configuration files and the like.
"go.mod",
"xrootd/resources/osdf-authfile",
## Ignore this script because of its inline certificates.
"github_scripts/osx_install.sh",
]
4 changes: 2 additions & 2 deletions broker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func ConnectToOrigin(ctx context.Context, brokerUrl, prefix, originName string)
if err = config.GenerateCACert(); err != nil {
return
}
caCert, err := config.LoadCertficate(param.Server_TLSCACertificateFile.GetString())
caCert, err := config.LoadCertificate(param.Server_TLSCACertificateFile.GetString())
if err != nil {
return
}
Expand Down Expand Up @@ -529,7 +529,7 @@ func doCallback(ctx context.Context, brokerResp reversalRequest) (listener net.L
hj.realConn.Close()
newConn, err := net.FileConn(fp)
if err != nil {
err = errors.Wrap(err, "Failure when making scoket from duplicated connection")
err = errors.Wrap(err, "Failure when making socket from duplicated connection")
return
}
fp.Close()
Expand Down
2 changes: 1 addition & 1 deletion cache/cache_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func LaunchDirectorTestFileCleanup(ctx context.Context) {
}
directorItems = append(directorItems, item)
}
if len(directorItems) <= 2 { // At mininum there are the test file and .cinfo file, and we don't want to remove the last two
if len(directorItems) <= 2 { // At minimum there are the test file and .cinfo file, and we don't want to remove the last two
return nil
}
for idx, item := range directorItems {
Expand Down
2 changes: 1 addition & 1 deletion client/acquire_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func (tci *tokenContentIterator) discoverHTCondorTokenLocations(tokenName string

func (tci *tokenContentIterator) next() (string, bool) {
/*
Search for the location of the authentiction token. It can be set explicitly on the command line,
Search for the location of the authentication token. It can be set explicitly on the command line,
with the environment variable "BEARER_TOKEN", or it can be searched in the standard HTCondor directory pointed
to by the environment variable "_CONDOR_CREDS".
*/
Expand Down
4 changes: 2 additions & 2 deletions client/director_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,15 @@ func TestGetDirectorInfoForPath(t *testing.T) {
expectedError: "",
},
{
name: "PUT request changes verb", // also generates 405, although this is a feauture of the director
name: "PUT request changes verb", // also generates 405, although this is a feature of the director
resourcePath: "/test",
directorUrl: ts.URL,
isPut: true,
query: "",
expectedError: "error 405: No writeable origins were found",
},
{
name: "Queries are propagated", // also generates 405, although this is a feauture of the director
name: "Queries are propagated", // also generates 405, although this is a feature of the director
resourcePath: "/test",
directorUrl: ts.URL,
isPut: false,
Expand Down
10 changes: 5 additions & 5 deletions client/handle_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ type (
err error
}

// StatusCodeError is a wrapper around grab.StatusCodeErorr that indicates the server returned
// StatusCodeError is a wrapper around grab.StatusCodeError that indicates the server returned
// a non-200 code.
//
// The wrapper is done to provide a Pelican-based error hierarchy in case we ever decide to have
Expand Down Expand Up @@ -1114,7 +1114,7 @@ func (tj *TransferJob) GetLookupStatus() (ok bool, err error) {
// Submit the transfer job to the client for processing
func (tc *TransferClient) Submit(tj *TransferJob) error {
// Ensure that a tj.Wait() immediately after Submit will always block.
log.Debugln("Submiting transfer job", tj.uuid.String())
log.Debugln("Submitting transfer job", tj.uuid.String())
select {
case <-tc.ctx.Done():
return tc.ctx.Err()
Expand Down Expand Up @@ -1873,7 +1873,7 @@ func downloadHTTP(ctx context.Context, te *TransferEngine, callback TransferCall
if ageSec, err := strconv.Atoi(ageStr); err == nil {
cacheAge = time.Duration(ageSec) * time.Second
} else {
log.WithFields(fields).Debugf("Server at %s gave unparseable Age header (%s) in response: %s", transfer.Url.Host, ageStr, err.Error())
log.WithFields(fields).Debugf("Server at %s gave unparsable Age header (%s) in response: %s", transfer.Url.Host, ageStr, err.Error())
}
}
if cacheAge == 0 {
Expand Down Expand Up @@ -1912,7 +1912,7 @@ func downloadHTTP(ctx context.Context, te *TransferEngine, callback TransferCall
callback(dest, 0, totalSize, false)
}

stoppedTransferTimeout := compatToDuration(param.Client_StoppedTransferTimeout.GetDuration(), "Client.StoppedTranferTimeout")
stoppedTransferTimeout := compatToDuration(param.Client_StoppedTransferTimeout.GetDuration(), "Client.StoppedTransferTimeout")
slowTransferRampupTime := compatToDuration(param.Client_SlowTransferRampupTime.GetDuration(), "Client.SlowTransferRampupTime")
slowTransferWindow := compatToDuration(param.Client_SlowTransferWindow.GetDuration(), "Client.SlowTransferWindow")
stoppedTransferDebugLine.Do(func() {
Expand Down Expand Up @@ -2091,7 +2091,7 @@ func (pr *ProgressReader) Read(p []byte) (n int, err error) {
return n, err
}

// Close implments the close function of io.Closer
// Close implements the close function of io.Closer
func (pr *ProgressReader) Close() error {
err := pr.reader.Close()
// Also, send the closed channel a message
Expand Down
2 changes: 1 addition & 1 deletion client/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestGenerateSortedObjectServers(t *testing.T) {
})

// Test our prepend works with multiple preferred caches
t.Run("testMutliPreferredCachesPrepend", func(t *testing.T) {
t.Run("testMultiPreferredCachesPrepend", func(t *testing.T) {
preferredOServers := []*url.URL{
{Scheme: "https", Host: "preferred1.com", Path: "/foo"},
{Scheme: "https", Host: "preferred2.com", Path: "/foo"},
Expand Down
6 changes: 3 additions & 3 deletions cmd/config_printer/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ func initClientAndServerConfig(v *viper.Viper) *param.Config {
config.SetFederation(globalFedInfo)
}

exapandedConfig, err := param.UnmarshalConfig(v)
expandedConfig, err := param.UnmarshalConfig(v)
if err != nil {
log.Errorf("Error unmarshaling config: %v", err)
log.Errorf("Error unmarshalling config: %v", err)
}
return exapandedConfig
return expandedConfig
}

// printConfig is used to print a config.
Expand Down
2 changes: 1 addition & 1 deletion cmd/plugin_stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func stagePluginMain(cmd *cobra.Command, args []string) {
var sources, extraSources []string
var exitCode int

// If not a condor hook, our souces come from our args
// If not a condor hook, our sources come from our args
if !isHook {
log.Debugln("Len of source:", len(args))
if len(args) < 1 {
Expand Down
6 changes: 3 additions & 3 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ func setupTransport() {
if param.TLSSkipVerify.GetBool() {
transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
}
if caCert, err := LoadCertficate(param.Server_TLSCACertificateFile.GetString()); err == nil {
if caCert, err := LoadCertificate(param.Server_TLSCACertificateFile.GetString()); err == nil {
systemPool, err := x509.SystemCertPool()
if err == nil {
systemPool.AddCert(caCert)
Expand Down Expand Up @@ -591,7 +591,7 @@ func GetTransport() *http.Transport {
return transport
}

// Get singleton global validte method for field validation
// Get singleton global validate method for field validation
func GetValidate() *validator.Validate {
return validate
}
Expand Down Expand Up @@ -1041,7 +1041,7 @@ func SetServerDefaults(v *viper.Viper) error {
v.SetDefault(param.Origin_StorageType.GetName(), "posix")
v.SetDefault(param.Origin_SelfTest.GetName(), true)
v.SetDefault(param.Origin_DirectorTest.GetName(), true)
// Set up the default S3 URL style to be path-style here as opposed to in the defaults.yaml becase
// Set up the default S3 URL style to be path-style here as opposed to in the defaults.yaml because
// we want to be able to check if this is user-provided (which we can't do for defaults.yaml)
v.SetDefault(param.Origin_S3UrlStyle.GetName(), "path")

Expand Down
12 changes: 6 additions & 6 deletions config/csrf.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import (

var (
// Global CSRF handler that shares the same auth key
csrfHanlder gin.HandlerFunc
onceCSRFHanlder sync.Once
csrfHandler gin.HandlerFunc
onceCSRFHandler sync.Once
)

func setupCSRFHandler() {
Expand All @@ -52,15 +52,15 @@ func setupCSRFHandler() {
}
})),
)
csrfHanlder = adapter.Wrap(CSRF)
csrfHandler = adapter.Wrap(CSRF)
}

func GetCSRFHandler() (gin.HandlerFunc, error) {
onceCSRFHanlder.Do(func() {
onceCSRFHandler.Do(func() {
setupCSRFHandler()
})
if csrfHanlder == nil {
if csrfHandler == nil {
return nil, errors.New("Error setting up the CSRF handler")
}
return csrfHanlder, nil
return csrfHandler, nil
}
12 changes: 6 additions & 6 deletions config/init_server_creds.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,10 @@ func GenerateCACert() error {
return nil
}

// Read a PEM-encoded TLS certficate file, parse and return the first
// Read a PEM-encoded TLS certificate file, parse and return the first
// certificate appeared in the chain. Return error if there's no cert
// present in the file
func LoadCertficate(certFile string) (*x509.Certificate, error) {
func LoadCertificate(certFile string) (*x509.Certificate, error) {
rest, err := os.ReadFile(certFile)
if err != nil {
return nil, err
Expand All @@ -343,7 +343,7 @@ func LoadCertficate(certFile string) (*x509.Certificate, error) {
}

// Generate a TLS certificate (host certificate) and its private key
// for non-production environment if the requied TLS files are not present
// for non-production environment if the required TLS files are not present
func GenerateCert() error {
gid, err := GetDaemonGID()
if err != nil {
Expand Down Expand Up @@ -373,7 +373,7 @@ func GenerateCert() error {
if _, err := os.Open(caCert); err == nil {
file.Close()
// Check that the CA is a valid CA
if _, err := LoadCertficate(caCert); err != nil {
if _, err := LoadCertificate(caCert); err != nil {
return errors.Wrap(err, "Failed to load CA cert")
} else {
// TODO: Check that the private key is a pair of the server cert
Expand All @@ -399,15 +399,15 @@ func GenerateCert() error {
if err := GenerateCACert(); err != nil {
return err
}
caCert, err := LoadCertficate(param.Server_TLSCACertificateFile.GetString())
caCert, err := LoadCertificate(param.Server_TLSCACertificateFile.GetString())
if err != nil {
return err
}

// However, if only CA is missing but TLS cert and private key are present, we simply
// generate the CA and return
if tlsCertPrivateKeyExists {
log.Debug("TLS Certficiate and its private key are present. Generated a CA and returns.")
log.Debug("TLS Certificate and its private key are present. Generated a CA and returns.")
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion dev/dev_pelican.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
TLSSkipVerify: true # Turn off the TLS check for developmetn
TLSSkipVerify: true # Turn off the TLS check for development
Logging:
Level: "debug" # Debug level will log more verbose messages
Federation:
Expand Down
2 changes: 1 addition & 1 deletion director/advertise_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func mockTopoDowntimeXMLHandler(w http.ResponseWriter, r *http.Request) {
downtimeInfo := server_structs.TopoCurrentDowntimes{
Downtimes: []server_structs.TopoServerDowntime{
{
// Current time falls in start-end window. SHould be filtered
// Current time falls in start-end window. Should be filtered
ResourceName: "BOISE_INTERNET2_OSDF_CACHE",
ResourceFQDN: "dtn-pas.bois.nrp.internet2.edu",
StartTime: time.Now().Add(-24 * time.Hour).Format("Jan 2, 2006 03:04 PM MST"),
Expand Down
12 changes: 6 additions & 6 deletions director/director.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,9 @@ func generateXNamespaceHeader(ginCtx *gin.Context, namespaceAd server_structs.Na
ginCtx.Writer.Header()["X-Pelican-Namespace"] = []string{xPelicanNamespace}
}

func getFinalRedirectURL(rurl url.URL, requstParams url.Values) string {
func getFinalRedirectURL(rurl url.URL, requestParams url.Values) string {
rQuery := rurl.Query()
for key, vals := range requstParams {
for key, vals := range requestParams {
for _, val := range vals {
rQuery.Add(key, val)
}
Expand Down Expand Up @@ -924,7 +924,7 @@ func ShortcutMiddleware(defaultResponse string) gin.HandlerFunc {
}
}

// Check for the DirectRead query paramater and redirect to the origin if it's set if the origin allows DirectReads
// Check for the DirectRead query parameter and redirect to the origin if it's set if the origin allows DirectReads
if c.Request.URL.Query().Has(pelican_url.QueryDirectRead) {
log.Debugln("directread query parameter detected, redirecting to origin")
// We'll redirect to origin here and the origin will decide if it can serve the request (if direct reads are enabled)
Expand Down Expand Up @@ -1175,7 +1175,7 @@ func serverAdMetricMiddleware(ctx *gin.Context) {
if len(namespacePath) == 0 {
continue
}
metrics.PelicanDirectorAdvertisementsRecievedTotal.With(
metrics.PelicanDirectorAdvertisementsReceivedTotal.With(
prometheus.Labels{
"server_name": serverName,
"server_web_url": serverWebUrl,
Expand Down Expand Up @@ -1351,9 +1351,9 @@ func collectClientVersionMetric(reqVer *version.Version, service string) {
fmt.Sprintf("%d", versionSegments[1]),
}

shortendVersion := strings.Join(strSegments, ".")
shortenedVersion := strings.Join(strSegments, ".")

metrics.PelicanDirectorClientVersionTotal.With(prometheus.Labels{"version": shortendVersion, "service": service}).Inc()
metrics.PelicanDirectorClientVersionTotal.With(prometheus.Labels{"version": shortenedVersion, "service": service}).Inc()
}

func collectDirectorRedirectionMetric(ctx *gin.Context, destination string) {
Expand Down
2 changes: 1 addition & 1 deletion director/director_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func hookServerAdsCache() {
}

// Populate internal filteredServers map using Director.FilteredServers param and director db
func ConfigFilterdServers() {
func ConfigFilteredServers() {
filteredServersMutex.Lock()
defer filteredServersMutex.Unlock()

Expand Down
2 changes: 1 addition & 1 deletion director/director_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func getAllServerDowntimes() ([]ServerDowntime, error) {
// Set the downtime info (filterType) of a given server
func setServerDowntime(serverName string, filterType filterType) error {
var serverDowntime ServerDowntime
// slience the logger for this query because there's definitely an ErrRecordNotFound when a new downtime info entry inserted
// silence the logger for this query because there's definitely an ErrRecordNotFound when a new downtime info entry inserted
err := db.Session(&gorm.Session{Logger: db.Logger.LogMode(logger.Silent)}).First(&serverDowntime, "name = ?", serverName).Error

// If the server doesn't exist in director db, create a new entry for it
Expand Down
10 changes: 5 additions & 5 deletions director/director_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,9 @@ func TestDirectorRegistration(t *testing.T) {
return tok
}

setupRequest := func(c *gin.Context, r *gin.Engine, bodyByt []byte, token string, stype server_structs.ServerType) {
setupRequest := func(c *gin.Context, r *gin.Engine, bodyByte []byte, token string, stype server_structs.ServerType) {
r.POST("/", func(gctx *gin.Context) { registerServeAd(ctx, gctx, stype) })
c.Request, _ = http.NewRequest(http.MethodPost, "/", bytes.NewBuffer(bodyByt))
c.Request, _ = http.NewRequest(http.MethodPost, "/", bytes.NewBuffer(bodyByte))
c.Request.Header.Set("Authorization", "Bearer "+token)
c.Request.Header.Set("Content-Type", "application/json")
// Hard code the current min version. When this test starts failing because of new stuff in the Director,
Expand Down Expand Up @@ -1328,7 +1328,7 @@ func TestRedirects(t *testing.T) {
router := gin.Default()
router.GET("/api/v1.0/director/origin/*any", redirectToOrigin)

// Check that the checkkHostnameRedirects uses the pre-configured hostnames to redirect
// Check that the checkHostnameRedirects uses the pre-configured hostnames to redirect
// requests that come in at the default paths, but not if the request is made
// specifically for an object or a cache via the API.
t.Run("redirect-check-hostnames", func(t *testing.T) {
Expand Down Expand Up @@ -1792,7 +1792,7 @@ func TestHandleFilterServer(t *testing.T) {

resB, err := io.ReadAll(w.Body)
require.NoError(t, err)
assert.Contains(t, string(resB), "Can't filter a server that already has been fitlered")
assert.Contains(t, string(resB), "Can't filter a server that already has been filtered")
})
t.Run("filter-server-w-tempFiltered", func(t *testing.T) {
// Create a request to the endpoint
Expand Down Expand Up @@ -1821,7 +1821,7 @@ func TestHandleFilterServer(t *testing.T) {

resB, err := io.ReadAll(w.Body)
require.NoError(t, err)
assert.Contains(t, string(resB), "Can't filter a server that already has been fitlered")
assert.Contains(t, string(resB), "Can't filter a server that already has been filtered")
})
t.Run("filter-server-w-tempAllowed", func(t *testing.T) {
// Create a request to the endpoint
Expand Down
Loading