Add linter config and fix linter issues

.golangci.yaml

@@ -0,0 +1,65 @@
+run:
+  build-tags:
+    - testtools
+  skip-dirs:
+    - internal/models
+
+linters-settings:
+  goimports:
+    local-prefixes: go.hollow.sh/serverservice
+  gofumpt:
+    extra-rules: true
+
+linters:
+  enable:
+    # default linters
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - typecheck
+    - unused
+
+    # additional linters
+    - bodyclose
+    - gocritic
+    - gocyclo
+    - goerr113
+    - gofmt
+    # - gofumpt
+    - goimports
+    - gomnd
+    - govet
+    - misspell
+    - noctx
+    - revive
+    - stylecheck
+    - whitespace
+    - wsl
+
+    # - bod
+issues:
+  exclude:
+    # Default excludes from `golangci-lint run --help` with EXC0002 removed
+    # EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok
+    - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
+    # EXC0002 golint: Annoying issue about not having a comment. The rare codebase has such comments
+    # - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)
+    # EXC0003 golint: False positive when tests are defined in package 'test'
+    - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this
+    # EXC0004 govet: Common false positives
+    - (possible misuse of unsafe.Pointer|should have signature)
+    # EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore
+    - ineffective break statement. Did you mean to break out of the outer loop
+    # EXC0006 gosec: Too many false-positives on 'unsafe' usage
+    - Use of unsafe calls should be audited
+    # EXC0007 gosec: Too many false-positives for parametrized shell calls
+    - Subprocess launch(ed with variable|ing should be audited)
+    # EXC0008 gosec: Duplicated errcheck checks
+    - (G104|G307)
+    # EXC0009 gosec: Too many issues in popular repos
+    - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)
+    # EXC0010 gosec: False positive is triggered by 'src, err := ioutil.ReadFile(filename)'
+    - Potential file inclusion via variable
+  exclude-use-default: false

cmd/doc.go

@@ -0,0 +1,2 @@
+// Package cmd is the package holding the commands to start the permissions-api and manage the schema
+package cmd

cmd/root.go

@@ -1,19 +1,3 @@
-/*
-Copyright © 2022 The Infratographer Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-	http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
 package cmd
 
 import (

cmd/schema.go

@@ -1,18 +1,3 @@
-/*
-Copyright © 2022 The Infratographer Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-	http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
 package cmd
 
 import (

cmd/server.go

@@ -1,19 +1,3 @@
-/*
-Copyright © 2022 The Infratographer Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-	http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
 package cmd
 
 import (
@@ -33,7 +17,7 @@ import (
 )
 
 var (
-	APIDefaultListen = "0.0.0.0:7602"
+	apiDefaultListen = "0.0.0.0:7602"
 )
 
 var serverCmd = &cobra.Command{
@@ -49,7 +33,7 @@ func init() {
 
 	v := viper.GetViper()
 
-	ginx.MustViperFlags(v, serverCmd.Flags(), APIDefaultListen)
+	ginx.MustViperFlags(v, serverCmd.Flags(), apiDefaultListen)
 	otelx.MustViperFlags(v, serverCmd.Flags())
 	ginjwt.RegisterViperOIDCFlags(v, serverCmd)
 }

internal/api/auth.go

@@ -1,3 +1,4 @@
+// Package api defines the server for permissions-api
 package api
 
 import (

internal/api/pagination.go

@@ -20,6 +20,7 @@ type Pagination struct {
 	Order string
 }
 
+// ParsePagination parses the pagination query parameters from the gin context
 func ParsePagination(c *gin.Context) *Pagination {
 	// Initializing default
 	limit := DefaultPaginationSize
@@ -86,6 +87,7 @@ func parseLimit(l int) int {
 // 	return (page - 1) * p.Limit
 // }
 
+// SetHeaders sets the pagination headers on a response
 func (p *Pagination) SetHeaders(c *gin.Context, count int) {
 	c.Header("Pagination-Count", strconv.Itoa(count))
 	c.Header("Pagination-Limit", strconv.Itoa(p.Limit))

internal/api/response.go

@@ -11,7 +11,12 @@ type ErrorResponse struct {
 }
 
 var (
-	ErrResourceNotFound      = errors.New("resource not found")
-	ErrSearchNotFound        = errors.New("search term not found")
+	// ErrResourceNotFound is returned when the requested resource isn't found
+	ErrResourceNotFound = errors.New("resource not found")
+
+	// ErrSearchNotFound is returned when the requested search term isn't found
+	ErrSearchNotFound = errors.New("search term not found")
+
+	// ErrResourceAlreadyExists is returned when the resource already exists
 	ErrResourceAlreadyExists = errors.New("resource already exists")
 )

internal/api/router.go

@@ -18,6 +18,7 @@ type Router struct {
 	logger *zap.SugaredLogger
 }
 
+// NewRouter returns a new api router
 func NewRouter(authCfg ginjwt.AuthConfig, engine *query.Engine, l *zap.SugaredLogger) (*Router, error) {
 	authMW, err := newAuthMiddleware(authCfg)
 	if err != nil {

internal/config/config.go

@@ -1,3 +1,4 @@
+// Package config defines the application configuration
 package config
 
 import (
@@ -9,6 +10,7 @@ import (
 	"go.infratographer.com/permissions-api/internal/spicedbx"
 )
 
+// AppConfig is the struct used for configuring the app
 type AppConfig struct {
 	OIDC    ginjwt.AuthConfig
 	Logging loggingx.Config

internal/query/doc.go

@@ -0,0 +1,2 @@
+// Package query provides the client for querying spicedb
+package query

internal/query/tenants.go

@@ -155,6 +155,7 @@ func (e *Engine) roleRelationships(role types.Role, resource types.Resource) []*
 	return rels
 }
 
+// GetResourceTypes returns the list of resource types
 func GetResourceTypes() []types.ResourceType {
 	return []types.ResourceType{
 		{
@@ -170,6 +171,7 @@ func GetResourceTypes() []types.ResourceType {
 	}
 }
 
+// NewResourceFromURN returns a new resource struct from a given urn
 func (e *Engine) NewResourceFromURN(urn *urnx.URN) (types.Resource, error) {
 	if urn.Namespace != e.namespace {
 		return types.Resource{}, errorInvalidNamespace

internal/spicedbx/client.go

@@ -1,3 +1,4 @@
+// Package spicedbx is the wrapper around spicedb client interaction
 package spicedbx
 
 import (
@@ -20,6 +21,7 @@ type Config struct {
 	Prefix   string
 }
 
+// NewClient returns a new spicedb/authzed client
 func NewClient(cfg Config, enableTracing bool) (*authzed.Client, error) {
 	clientOpts := []grpc.DialOption{}
 
@@ -58,6 +60,7 @@ func NewClient(cfg Config, enableTracing bool) (*authzed.Client, error) {
 	return authzed.NewClient(cfg.Endpoint, clientOpts...)
 }
 
+// Healthcheck does nothing :laughing:
 func Healthcheck(client *authzed.Client) func(ctx context.Context) error {
 	return func(ctx context.Context) error {
 		return nil

internal/spicedbx/doc.go

@@ -0,0 +1,2 @@
+// Package spicedbx is the wrapper around the spicedb client
+package spicedbx

internal/spicedbx/errors.go

@@ -3,5 +3,6 @@ package spicedbx
 import "errors"
 
 var (
+	// ErrorNoNamespace is returned when no namespace is provided with a query
 	ErrorNoNamespace = errors.New("no namespace provided")
 )

internal/spicedbx/schema.go

@@ -43,6 +43,7 @@ definition {{$namespace}}/{{$typeName}} {
 {{end}}`))
 )
 
+// GenerateSchema generates the spicedb schema from the template
 func GenerateSchema(namespace string, resourceTypes []types.ResourceType) (string, error) {
 	if namespace == "" {
 		return "", ErrorNoNamespace
@@ -66,6 +67,7 @@ func GenerateSchema(namespace string, resourceTypes []types.ResourceType) (strin
 	return out.String(), nil
 }
 
+// GeneratedSchema generated the schema for a namespace
 func GeneratedSchema(namespace string) string {
 	resourceTypes := []types.ResourceType{
 		{

internal/types/types.go

@@ -1,29 +1,34 @@
-// Package type exposes domain types for permissions-api.
+// Package types exposes domain types for permissions-api.
 package types
 
 import "github.com/google/uuid"
 
+// RoleTemplate is a template for a role
 type RoleTemplate struct {
 	Actions []string
 }
 
+// Role is a collection of permissions
 type Role struct {
 	ID      uuid.UUID
 	Actions []string
 }
 
+// ResourceRelationship is a relationship for a resource type
 type ResourceRelationship struct {
 	Name     string
 	Type     string
 	Optional bool
 }
 
+// ResourceType defines a type of resource managed by the api
 type ResourceType struct {
 	Name          string
 	Relationships []ResourceRelationship
 	TenantActions []string
 }
 
+// Resource is the object to be acted upon by an subject
 type Resource struct {
 	Type string
 	ID   uuid.UUID

pkg/client/v1/auth.go

@@ -1,3 +1,4 @@
+// Package permissions defines the permissions-api client
 package permissions
 
 import (
@@ -28,11 +29,13 @@ type Doer interface {
 	Do(*http.Request) (*http.Response, error)
 }
 
+// Client is used to interact with the api
 type Client struct {
 	url        string
 	httpClient Doer
 }
 
+// New returns a new permissions client
 func New(url string, doerClient Doer) (*Client, error) {
 	if url == "" {
 		return nil, ErrMissingURI
@@ -55,6 +58,7 @@ func New(url string, doerClient Doer) (*Client, error) {
 	return c, nil
 }
 
+// Allowed checks if the client subject is permitted exec the action on the resource
 func (c *Client) Allowed(ctx context.Context, action string, resourceURNPrefix string) (bool, error) {
 	ctx, span := tracer.Start(ctx, "SubjectHasAction", trace.WithAttributes(
 		attribute.String("action", action),
@@ -130,7 +134,7 @@ func ensureValidServerResponse(resp *http.Response) error {
 			return ErrPermissionDenied
 		}
 
-		return errors.New("bad response from server")
+		return ErrBadResponse
 	}
 
 	return nil

pkg/client/v1/errors.go

@@ -3,12 +3,15 @@ package permissions
 import "errors"
 
 var (
-	// ErrNoNextPage is the error returned when there is not an additional page of resources
+	// ErrMissingURI is the error returned when there uri provided to the client
 	ErrMissingURI = errors.New("no uri provided for client")
 
 	// ErrNoAuthToken is the error returned when there is no auth token provided for the API request
 	ErrNoAuthToken = errors.New("no auth token provided for client")
 
 	// ErrPermissionDenied is the error returned when permission is denied to a call
 	ErrPermissionDenied = errors.New("subject doesn't have access")
+
+	// ErrBadResponse is the error returned when we receive a bad response from the server
+	ErrBadResponse = errors.New("bad response from server")
 )