From 53ac926584fbd4c39413cf0ac76a2c6ac7504459 Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Thu, 6 Jun 2019 14:45:10 -0700 Subject: [PATCH] :book: re-enable golangci-lint's godoc comment checking --- .golangci.yml | 52 +++++++++++-------- pkg/builder/options.go | 6 +++ pkg/builder/webhook.go | 1 + pkg/client/fake/doc.go | 3 +- pkg/client/options.go | 42 +++++++++++++++ .../unconventionallisttypecrd.go | 4 ++ .../testing/integration/internal/apiserver.go | 6 ++- .../testing/integration/internal/arguments.go | 1 + .../testing/integration/internal/etcd.go | 7 +++ .../internal/integration_tests/doc.go | 2 +- .../testing/integration/internal/process.go | 8 +++ .../testing/integration/internal/tinyca.go | 2 + 12 files changed, 108 insertions(+), 26 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index cc0a4caa29..44a915409d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,28 +1,36 @@ -linters: - disable-all: true - enable: - - misspell - - structcheck - - golint - - govet - - deadcode - - errcheck - - varcheck - - goconst - - unparam - - ineffassign - - nakedret - - interfacer - - gocyclo - - lll - - dupl - - goimports - +run: + deadline: 5m linters-settings: lll: line-length: 170 dupl: threshold: 400 +issues: + # don't skip warning about doc comments + exclude-use-default: false -run: - timeout: 5m + # restore some of the defaults + # (fill in the rest as needed) + exclude-rules: + - linters: [errcheck] + text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked" +linters: + disable-all: true + enable: + - misspell + - structcheck + - golint + - govet + - deadcode + - errcheck + - varcheck + - goconst + - unparam + - ineffassign + - nakedret + - interfacer + - gocyclo + - lll + - dupl + - goimports + - golint diff --git a/pkg/builder/options.go b/pkg/builder/options.go index e95f5de781..edd5d0156b 100644 --- a/pkg/builder/options.go +++ b/pkg/builder/options.go @@ -51,16 +51,22 @@ func WithPredicates(predicates ...predicate.Predicate) Predicates { } } +// Predicates filters events before enqueuing the keys. type Predicates struct { predicates []predicate.Predicate } +// ApplyToFor applies this configuration to the given ForInput options. func (w Predicates) ApplyToFor(opts *ForInput) { opts.predicates = w.predicates } + +// ApplyToOwns applies this configuration to the given OwnsInput options. func (w Predicates) ApplyToOwns(opts *OwnsInput) { opts.predicates = w.predicates } + +// ApplyToWatches applies this configuration to the given WatchesInput options. func (w Predicates) ApplyToWatches(opts *WatchesInput) { opts.predicates = w.predicates } diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index ada4ddd239..7ba398762a 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -38,6 +38,7 @@ type WebhookBuilder struct { config *rest.Config } +// WebhookManagedBy allows inform its manager.Manager func WebhookManagedBy(m manager.Manager) *WebhookBuilder { return &WebhookBuilder{mgr: m} } diff --git a/pkg/client/fake/doc.go b/pkg/client/fake/doc.go index 9d5d85648c..a45d703320 100644 --- a/pkg/client/fake/doc.go +++ b/pkg/client/fake/doc.go @@ -15,9 +15,10 @@ limitations under the License. */ /* +Package fake provides a fake client for testing. + Deprecated: please use pkg/envtest for testing. This package will be dropped before the v1.0.0 release. -Package fake provides a fake client for testing. An fake client is backed by its simple object store indexed by GroupVersionResource. You can create a fake client with optional objects. diff --git a/pkg/client/options.go b/pkg/client/options.go index 83a0e6a287..fe4bb2c41c 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -71,15 +71,22 @@ var DryRunAll = dryRunAll{} type dryRunAll struct{} +// ApplyToCreate applies this configuration to the given create options. func (dryRunAll) ApplyToCreate(opts *CreateOptions) { opts.DryRun = []string{metav1.DryRunAll} } + +// ApplyToUpdate applies this configuration to the given update options. func (dryRunAll) ApplyToUpdate(opts *UpdateOptions) { opts.DryRun = []string{metav1.DryRunAll} } + +// ApplyToPatch applies this configuration to the given patch options. func (dryRunAll) ApplyToPatch(opts *PatchOptions) { opts.DryRun = []string{metav1.DryRunAll} } + +// ApplyToPatch applies this configuration to the given delete options. func (dryRunAll) ApplyToDelete(opts *DeleteOptions) { opts.DryRun = []string{metav1.DryRunAll} } @@ -87,12 +94,17 @@ func (dryRunAll) ApplyToDelete(opts *DeleteOptions) { // FieldOwner set the field manager name for the given server-side apply patch. type FieldOwner string +// ApplyToPatch applies this configuration to the given patch options. func (f FieldOwner) ApplyToPatch(opts *PatchOptions) { opts.FieldManager = string(f) } + +// ApplyToCreate applies this configuration to the given create options. func (f FieldOwner) ApplyToCreate(opts *CreateOptions) { opts.FieldManager = string(f) } + +// ApplyToUpdate applies this configuration to the given update options. func (f FieldOwner) ApplyToUpdate(opts *UpdateOptions) { opts.FieldManager = string(f) } @@ -252,33 +264,49 @@ func (o *DeleteOptions) ApplyToDelete(do *DeleteOptions) { // to the given number of seconds. type GracePeriodSeconds int64 +// ApplyToDelete applies this configuration to the given delete options. func (s GracePeriodSeconds) ApplyToDelete(opts *DeleteOptions) { secs := int64(s) opts.GracePeriodSeconds = &secs } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (s GracePeriodSeconds) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { s.ApplyToDelete(&opts.DeleteOptions) } +// Preconditions must be fulfilled before an operation (update, delete, etc.) is carried out. type Preconditions metav1.Preconditions +// ApplyToDelete applies this configuration to the given delete options. func (p Preconditions) ApplyToDelete(opts *DeleteOptions) { preconds := metav1.Preconditions(p) opts.Preconditions = &preconds } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (p Preconditions) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { p.ApplyToDelete(&opts.DeleteOptions) } +// PropagationPolicy determined whether and how garbage collection will be +// performed. Either this field or OrphanDependents may be set, but not both. +// The default policy is decided by the existing finalizer set in the +// metadata.finalizers and the resource-specific default policy. +// Acceptable values are: 'Orphan' - orphan the dependents; 'Background' - +// allow the garbage collector to delete the dependents in the background; +// 'Foreground' - a cascading policy that deletes all dependents in the +// foreground. type PropagationPolicy metav1.DeletionPropagation +// ApplyToDelete applies the given delete options on these options. +// It will propagate to the dependents of the object to let the garbage collector handle it. func (p PropagationPolicy) ApplyToDelete(opts *DeleteOptions) { policy := metav1.DeletionPropagation(p) opts.PropagationPolicy = &policy } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (p PropagationPolicy) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { p.ApplyToDelete(&opts.DeleteOptions) } @@ -379,12 +407,14 @@ func (o *ListOptions) ApplyOptions(opts []ListOption) *ListOptions { // MatchingLabels filters the list/delete operation on the given set of labels. type MatchingLabels map[string]string +// ApplyToList applies this configuration to the given list options. func (m MatchingLabels) ApplyToList(opts *ListOptions) { // TODO(directxman12): can we avoid reserializing this over and over? sel := labels.SelectorFromValidatedSet(map[string]string(m)) opts.LabelSelector = sel } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (m MatchingLabels) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { m.ApplyToList(&opts.ListOptions) } @@ -393,6 +423,7 @@ func (m MatchingLabels) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { // without checking their values. type HasLabels []string +// ApplyToList applies this configuration to the given list options. func (m HasLabels) ApplyToList(opts *ListOptions) { sel := labels.NewSelector() for _, label := range m { @@ -404,6 +435,7 @@ func (m HasLabels) ApplyToList(opts *ListOptions) { opts.LabelSelector = sel } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (m HasLabels) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { m.ApplyToList(&opts.ListOptions) } @@ -415,10 +447,12 @@ type MatchingLabelsSelector struct { labels.Selector } +// ApplyToList applies this configuration to the given list options. func (m MatchingLabelsSelector) ApplyToList(opts *ListOptions) { opts.LabelSelector = m } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (m MatchingLabelsSelector) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { m.ApplyToList(&opts.ListOptions) } @@ -435,12 +469,14 @@ func MatchingField(name, val string) MatchingFields { // (or index in the case of cached lists). type MatchingFields fields.Set +// ApplyToList applies this configuration to the given list options. func (m MatchingFields) ApplyToList(opts *ListOptions) { // TODO(directxman12): can we avoid re-serializing this? sel := fields.Set(m).AsSelector() opts.FieldSelector = sel } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (m MatchingFields) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { m.ApplyToList(&opts.ListOptions) } @@ -452,10 +488,12 @@ type MatchingFieldsSelector struct { fields.Selector } +// ApplyToList applies this configuration to the given list options. func (m MatchingFieldsSelector) ApplyToList(opts *ListOptions) { opts.FieldSelector = m } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (m MatchingFieldsSelector) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { m.ApplyToList(&opts.ListOptions) } @@ -463,10 +501,12 @@ func (m MatchingFieldsSelector) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { // InNamespace restricts the list/delete operation to the given namespace. type InNamespace string +// ApplyToList applies this configuration to the given list options. func (n InNamespace) ApplyToList(opts *ListOptions) { opts.Namespace = string(n) } +// ApplyToDeleteAllOf applies this configuration to the given an List options. func (n InNamespace) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { n.ApplyToList(&opts.ListOptions) } @@ -476,6 +516,7 @@ func (n InNamespace) ApplyToDeleteAllOf(opts *DeleteAllOfOptions) { // does not support setting it for deletecollection operations. type Limit int64 +// ApplyToList applies this configuration to the given an list options. func (l Limit) ApplyToList(opts *ListOptions) { opts.Limit = int64(l) } @@ -485,6 +526,7 @@ func (l Limit) ApplyToList(opts *ListOptions) { // does not support setting it for deletecollection operations. type Continue string +// ApplyToList applies this configuration to the given an List options. func (c Continue) ApplyToList(opts *ListOptions) { opts.Continue = string(c) } diff --git a/pkg/controller/controllertest/unconventionallisttypecrd.go b/pkg/controller/controllertest/unconventionallisttypecrd.go index a2794ca307..a4d23f8abe 100644 --- a/pkg/controller/controllertest/unconventionallisttypecrd.go +++ b/pkg/controller/controllertest/unconventionallisttypecrd.go @@ -22,6 +22,8 @@ func (u *UnconventionalListType) DeepCopyObject() runtime.Object { return u.DeepCopy() } +// DeepCopy implements *UnconventionalListType +// Handwritten for simplicity. func (u *UnconventionalListType) DeepCopy() *UnconventionalListType { return &UnconventionalListType{ TypeMeta: u.TypeMeta, @@ -44,6 +46,8 @@ func (u *UnconventionalListTypeList) DeepCopyObject() runtime.Object { return u.DeepCopy() } +// DeepCopy implements *UnconventionalListTypeListt +// Handwritten for simplicity. func (u *UnconventionalListTypeList) DeepCopy() *UnconventionalListTypeList { out := &UnconventionalListTypeList{ TypeMeta: u.TypeMeta, diff --git a/pkg/internal/testing/integration/internal/apiserver.go b/pkg/internal/testing/integration/internal/apiserver.go index 0282b23b4d..0ad9bd19d0 100644 --- a/pkg/internal/testing/integration/internal/apiserver.go +++ b/pkg/internal/testing/integration/internal/apiserver.go @@ -1,8 +1,8 @@ package internal +// APIServerDefaultArgs allow tests to run offline, by preventing API server from attempting to +// use default route to determine its --advertise-address. var APIServerDefaultArgs = []string{ - // Allow tests to run offline, by preventing API server from attempting to - // use default route to determine its --advertise-address "--advertise-address=127.0.0.1", "--etcd-servers={{ if .EtcdURL }}{{ .EtcdURL.String }}{{ end }}", "--cert-dir={{ .CertDir }}", @@ -14,6 +14,8 @@ var APIServerDefaultArgs = []string{ "--allow-privileged=true", } +// DoAPIServerArgDefaulting will set default values to allow tests to run offline when the args are not informed. Otherwise, +// it will return the same []string arg passed as param. func DoAPIServerArgDefaulting(args []string) []string { if len(args) != 0 { return args diff --git a/pkg/internal/testing/integration/internal/arguments.go b/pkg/internal/testing/integration/internal/arguments.go index 00fe7935a1..573295d904 100644 --- a/pkg/internal/testing/integration/internal/arguments.go +++ b/pkg/internal/testing/integration/internal/arguments.go @@ -5,6 +5,7 @@ import ( "html/template" ) +// RenderTemplates returns an []string to render the templates func RenderTemplates(argTemplates []string, data interface{}) (args []string, err error) { var t *template.Template diff --git a/pkg/internal/testing/integration/internal/etcd.go b/pkg/internal/testing/integration/internal/etcd.go index 5a4511747a..2d108a3e82 100644 --- a/pkg/internal/testing/integration/internal/etcd.go +++ b/pkg/internal/testing/integration/internal/etcd.go @@ -4,6 +4,8 @@ import ( "net/url" ) +// EtcdDefaultArgs allow tests to run offline, by preventing API server from attempting to +// use default route to determine its urls. var EtcdDefaultArgs = []string{ "--listen-peer-urls=http://localhost:0", "--advertise-client-urls={{ if .URL }}{{ .URL.String }}{{ end }}", @@ -11,6 +13,8 @@ var EtcdDefaultArgs = []string{ "--data-dir={{ .DataDir }}", } +// DoEtcdArgDefaulting will set default values to allow tests to run offline when the args are not informed. Otherwise, +// it will return the same []string arg passed as param. func DoEtcdArgDefaulting(args []string) []string { if len(args) != 0 { return args @@ -19,6 +23,7 @@ func DoEtcdArgDefaulting(args []string) []string { return EtcdDefaultArgs } +// isSecureScheme returns false when the schema is insecure. func isSecureScheme(scheme string) bool { // https://github.com/coreos/etcd/blob/d9deeff49a080a88c982d328ad9d33f26d1ad7b6/pkg/transport/listener.go#L53 if scheme == "https" || scheme == "unixs" { @@ -27,6 +32,8 @@ func isSecureScheme(scheme string) bool { return false } +// GetEtcdStartMessage returns an start message to inform if the client is or not insecure. +// It will return true when the URL informed has the scheme == "https" || scheme == "unixs" func GetEtcdStartMessage(listenURL url.URL) string { if isSecureScheme(listenURL.Scheme) { // https://github.com/coreos/etcd/blob/a7f1fbe00ec216fcb3a1919397a103b41dca8413/embed/serve.go#L167 diff --git a/pkg/internal/testing/integration/internal/integration_tests/doc.go b/pkg/internal/testing/integration/internal/integration_tests/doc.go index c22a570af3..363a126ec5 100644 --- a/pkg/internal/testing/integration/internal/integration_tests/doc.go +++ b/pkg/internal/testing/integration/internal/integration_tests/doc.go @@ -1,5 +1,5 @@ /* -Package integrariontests is holding the integration tests to run against the +Package integrationtests holds the integration tests to run against the framework. This file's only purpose is to make godep happy. diff --git a/pkg/internal/testing/integration/internal/process.go b/pkg/internal/testing/integration/internal/process.go index 8ecf1afe5c..9651029f05 100644 --- a/pkg/internal/testing/integration/internal/process.go +++ b/pkg/internal/testing/integration/internal/process.go @@ -19,6 +19,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr" ) +// ProcessState define the state of the process. type ProcessState struct { DefaultedProcessInput Session *gexec.Session @@ -46,6 +47,7 @@ type ProcessState struct { ready bool } +// DefaultedProcessInput defines the default process input required to perform the test. type DefaultedProcessInput struct { URL url.URL Dir string @@ -55,6 +57,8 @@ type DefaultedProcessInput struct { StartTimeout time.Duration } +// DoDefaulting sets the default configuration according to the data informed and return an DefaultedProcessInput +// and an error if some requirement was not informed. func DoDefaulting( name string, listenURL *url.URL, @@ -112,6 +116,8 @@ func DoDefaulting( type stopChannel chan struct{} +// Start starts the apiserver, waits for it to come up, and returns an error, +// if occurred. func (ps *ProcessState) Start(stdout, stderr io.Writer) (err error) { if ps.ready { return nil @@ -187,6 +193,8 @@ func pollURLUntilOK(url url.URL, interval time.Duration, ready chan bool, stopCh } } +// Stop stops this process gracefully, waits for its termination, and cleans up +// the CertDir if necessary. func (ps *ProcessState) Stop() error { if ps.Session == nil { return nil diff --git a/pkg/internal/testing/integration/internal/tinyca.go b/pkg/internal/testing/integration/internal/tinyca.go index ca877ca9e3..034a659bcc 100644 --- a/pkg/internal/testing/integration/internal/tinyca.go +++ b/pkg/internal/testing/integration/internal/tinyca.go @@ -73,6 +73,8 @@ func newPrivateKey() (crypto.Signer, error) { return rsa.GenerateKey(crand.Reader, rsaKeySize) } +// NewTinyCA creates a new a tiny CA utility for provisioning serving certs and client certs FOR TESTING ONLY. +// Don't use this for anything else! func NewTinyCA() (*TinyCA, error) { caPrivateKey, err := newPrivateKey() if err != nil {