Skip to content

Commit

Permalink
Bump golangci lint to v1.49.0
Browse files Browse the repository at this point in the history
  • Loading branch information
sbueringer committed Aug 25, 2022
0 parents commit b9a9086
Show file tree
Hide file tree
Showing 404 changed files with 66,210 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE/breaking_change.md
@@ -0,0 +1,5 @@
<!-- please add a :warning: (`:warning:`) to the title of this PR, and delete this line and similar ones -->

<!-- What does this do, and why do we need it? -->

<!-- Why does this have to be a breaking change (what else did you consider)? -->
5 changes: 5 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE/bug_fix.md
@@ -0,0 +1,5 @@
<!-- please add a :bug: (`:bug:`) to the title of this PR, and delete this line and similar ones -->

<!-- What does this do, and why do we need it? -->

<!-- What issue does this fix (e.g. Fixes #XYZ) -->
3 changes: 3 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE/compat_feature.md
@@ -0,0 +1,3 @@
<!-- please add a :sparkles: (`:sparkles:`) to the title of this PR, and delete this line and similar ones -->

<!-- What does this do, and why do we need it? -->
3 changes: 3 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE/docs.md
@@ -0,0 +1,3 @@
<!-- please add a :book: (`:book:`) to the title of this PR, and delete this line and similar ones -->

<!-- What docs does this change, and why? -->
3 changes: 3 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE/other.md
@@ -0,0 +1,3 @@
<!-- please add a :seedling: (`:seedling:`) to the title of this PR, and delete this line and similar ones -->

<!-- What does this do, and why do we need it? -->
4 changes: 4 additions & 0 deletions .github/pull_request_template.md
@@ -0,0 +1,4 @@
<!-- please add an icon to the title of this PR (see VERSIONING.md), and delete this line and similar ones -->
<!-- the icon will be either ⚠ (:warning:, major), ✨ (:sparkles:, minor), 🐛 (:bug:, patch), 📖 (:book:, docs), or 🌱 (:seedling:, other) -->

<!-- What does this do, and why do we need it? -->
23 changes: 23 additions & 0 deletions .github/workflows/golangci-lint.yml
@@ -0,0 +1,23 @@
name: golangci-lint
on:
pull_request:
types: [opened, edited, synchronize, reopened]
branches:
- main
- master
jobs:
golangci:
name: lint
runs-on: ubuntu-latest
strategy:
matrix:
working-directory:
- ""
- tools/setup-envtest
steps:
- uses: actions/checkout@v2
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
version: v1.49.0
working-directory: ${{matrix.working-directory}}
14 changes: 14 additions & 0 deletions .github/workflows/verify.yml
@@ -0,0 +1,14 @@
on:
pull_request_target:
types: [opened, edited, reopened, synchronize]

jobs:
verify:
runs-on: ubuntu-latest
name: verify PR contents
steps:
- name: Verifier action
id: verifier
uses: kubernetes-sigs/kubebuilder-release-tools@v0.1
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
24 changes: 24 additions & 0 deletions .gitignore
@@ -0,0 +1,24 @@
# Binaries for programs and plugins
*.exe
*.exe~
*.dll
*.so
*.dylib

# Test binary, build with `go test -c`
*.test

# Output of the go coverage tool, specifically when used with LiteIDE
*.out

# editor and IDE paraphernalia
.idea
*.swp
*.swo
*~

# Vscode files
.vscode

# Tools binaries.
hack/tools/bin
143 changes: 143 additions & 0 deletions .golangci.yml
@@ -0,0 +1,143 @@
linters:
disable-all: true
enable:
- asciicheck
- bodyclose
- deadcode
- depguard
- dogsled
- errcheck
- errorlint
- exportloopref
- goconst
- gocritic
- gocyclo
- gofmt
- goimports
- goprintffuncname
- gosec
- gosimple
- govet
- ifshort
- importas
- ineffassign
- misspell
- nakedret
- nilerr
- nolintlint
- prealloc
- revive
- rowserrcheck
- staticcheck
- structcheck
- stylecheck
- typecheck
- unconvert
- unparam
- unused
- varcheck
- whitespace

linters-settings:
ifshort:
# Maximum length of variable declaration measured in number of characters, after which linter won't suggest using short syntax.
max-decl-chars: 50
importas:
no-unaliased: true
alias:
# Kubernetes
- pkg: k8s.io/api/core/v1
alias: corev1
- pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
alias: apiextensionsv1
- pkg: k8s.io/apimachinery/pkg/apis/meta/v1
alias: metav1
- pkg: k8s.io/apimachinery/pkg/api/errors
alias: apierrors
- pkg: k8s.io/apimachinery/pkg/util/errors
alias: kerrors
# Controller Runtime
- pkg: sigs.k8s.io/controller-runtime
alias: ctrl
staticcheck:
go: "1.19"
stylecheck:
go: "1.19"
depguard:
include-go-root: true
packages:
- io/ioutil # https://go.dev/doc/go1.16#ioutil

issues:
max-same-issues: 0
max-issues-per-linter: 0
# We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
# changes in PRs and avoid nitpicking.
exclude-use-default: false
# List of regexps of issue texts to exclude, empty list by default.
exclude:
# The following are being worked on to remove their exclusion. This list should be reduced or go away all together over time.
# If it is decided they will not be addressed they should be moved above this comment.
- Subprocess launch(ed with variable|ing should be audited)
- (G204|G104|G307)
- "ST1000: at least one file in a package should have a package comment"
exclude-rules:
- linters:
- gosec
text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
- linters:
- revive
text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
- linters:
- errcheck
text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
# With Go 1.16, the new embed directive can be used with an un-named import,
# revive (previously, golint) only allows these to be imported in a main.go, which wouldn't work for us.
# This directive allows the embed package to be imported with an underscore everywhere.
- linters:
- revive
source: _ "embed"
# Exclude some packages or code to require comments, for example test code, or fake clients.
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
source: (func|type).*Fake.*
- linters:
- revive
text: exported (method|function|type|const) (.+) should have comment or be unexported
path: fake_\.go
# Disable unparam "always receives" which might not be really
# useful when building libraries.
- linters:
- unparam
text: always receives
# Dot imports for gomega or ginkgo are allowed
# within test files.
- path: _test\.go
text: should not use dot imports
- path: _test\.go
text: cyclomatic complexity
- path: _test\.go
text: "G107: Potential HTTP request made with variable url"
# Append should be able to assign to a different var/slice.
- linters:
- gocritic
text: "appendAssign: append result not assigned to the same slice"
- linters:
- gocritic
text: "singleCaseSwitch: should rewrite switch statement to if statement"
# It considers all file access to a filename that comes from a variable problematic,
# which is naiv at best.
- linters:
- gosec
text: "G304: Potential file inclusion via variable"
- linters:
- revive
text: "package-comments: should have a package comment"

run:
timeout: 10m
skip-files:
- "zz_generated.*\\.go$"
- ".*conversion.*\\.go$"
allow-parallel-runners: true
19 changes: 19 additions & 0 deletions CONTRIBUTING.md
@@ -0,0 +1,19 @@
# Contributing guidelines

## Sign the CLA

Kubernetes projects require that you sign a Contributor License Agreement (CLA) before we can accept your pull requests.

Please see https://git.k8s.io/community/CLA.md for more info

## Contributing steps

1. Submit an issue describing your proposed change to the repo in question.
1. The [repo owners](OWNERS) will respond to your issue promptly.
1. If your proposed change is accepted, and you haven't already done so, sign a Contributor License Agreement (see details above).
1. Fork the desired repo, develop and test your code changes.
1. Submit a pull request.

## Test locally

Run the command `make test` to test the changes locally.
81 changes: 81 additions & 0 deletions FAQ.md
@@ -0,0 +1,81 @@
# FAQ

### Q: How do I know which type of object a controller references?

**A**: Each controller should only reconcile one object type. Other
affected objects should be mapped to a single type of root object, using
the `EnqueueRequestForOwner` or `EnqueueRequestsFromMapFunc` event
handlers, and potentially indices. Then, your Reconcile method should
attempt to reconcile *all* state for that given root objects.

### Q: How do I have different logic in my reconciler for different types of events (e.g. create, update, delete)?

**A**: You should not. Reconcile functions should be idempotent, and
should always reconcile state by reading all the state it needs, then
writing updates. This allows your reconciler to correctly respond to
generic events, adjust to skipped or coalesced events, and easily deal
with application startup. The controller will enqueue reconcile requests
for both old and new objects if a mapping changes, but it's your
responsibility to make sure you have enough information to be able clean
up state that's no longer referenced.

### Q: My cache might be stale if I read from a cache! How should I deal with that?

**A**: There are several different approaches that can be taken, depending
on your situation.

- When you can, take advantage of optimistic locking: use deterministic
names for objects you create, so that the Kubernetes API server will
warn you if the object already exists. Many controllers in Kubernetes
take this approach: the StatefulSet controller appends a specific number
to each pod that it creates, while the Deployment controller hashes the
pod template spec and appends that.

- In the few cases when you cannot take advantage of deterministic names
(e.g. when using generateName), it may be useful in to track which
actions you took, and assume that they need to be repeated if they don't
occur after a given time (e.g. using a requeue result). This is what
the ReplicaSet controller does.

In general, write your controller with the assumption that information
will eventually be correct, but may be slightly out of date. Make sure
that your reconcile function enforces the entire state of the world each
time it runs. If none of this works for you, you can always construct
a client that reads directly from the API server, but this is generally
considered to be a last resort, and the two approaches above should
generally cover most circumstances.

### Q: Where's the fake client? How do I use it?

**A**: The fake client
[exists](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/client/fake),
but we generally recommend using
[envtest.Environment](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/envtest#Environment)
to test against a real API server. In our experience, tests using fake
clients gradually re-implement poorly-written impressions of a real API
server, which leads to hard-to-maintain, complex test code.

### Q: How should I write tests? Any suggestions for getting started?

- Use the aforementioned
[envtest.Environment](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/envtest#Environment)
to spin up a real API server instead of trying to mock one out.

- Structure your tests to check that the state of the world is as you
expect it, *not* that a particular set of API calls were made, when
working with Kubernetes APIs. This will allow you to more easily
refactor and improve the internals of your controllers without changing
your tests.

- Remember that any time you're interacting with the API server, changes
may have some delay between write time and reconcile time.

### Q: What are these errors about no Kind being registered for a type?

**A**: You're probably missing a fully-set-up Scheme. Schemes record the
mapping between Go types and group-version-kinds in Kubernetes. In
general, your application should have its own Scheme containing the types
from the API groups that it needs (be they Kubernetes types or your own).
See the [scheme builder
docs](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/scheme) for
more information.

0 comments on commit b9a9086

Please sign in to comment.