Skip to content

🌱 Re-add tests for component config #2191

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

Merged
Merged
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
203 changes: 103 additions & 100 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,44 +1,44 @@
linters:
disable-all: true
enable:
- asasalint
- asciicheck
- bidichk
- bodyclose
- depguard
- dogsled
- dupl
- errcheck
- errchkjson
- errorlint
- exhaustive
- exportloopref
- goconst
- gocritic
- gocyclo
- gofmt
- goimports
- goprintffuncname
- gosec
- gosimple
- govet
- importas
- ineffassign
- makezero
- misspell
- nakedret
- nilerr
- nolintlint
- prealloc
- revive
- staticcheck
- stylecheck
- tagliatelle
- typecheck
- unconvert
- unparam
- unused
- whitespace
- asasalint
- asciicheck
- bidichk
- bodyclose
- depguard
- dogsled
- dupl
- errcheck
- errchkjson
- errorlint
- exhaustive
- exportloopref
- goconst
- gocritic
- gocyclo
- gofmt
- goimports
- goprintffuncname
- gosec
- gosimple
- govet
- importas
- ineffassign
- makezero
- misspell
- nakedret
- nilerr
- nolintlint
- prealloc
- revive
- staticcheck
- stylecheck
- tagliatelle
- typecheck
- unconvert
- unparam
- unused
- whitespace

linters-settings:
importas:
Expand Down Expand Up @@ -75,71 +75,74 @@ issues:
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"
# 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 and 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"
- linters:
- dupl
path: _test\.go
- 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
- linters:
- staticcheck
text: "SA1019: .* The component config package has been deprecated and will be removed in a future release."
# 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 and 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"
- linters:
- dupl
path: _test\.go

run:
timeout: 10m
skip-files:
- "zz_generated.*\\.go$"
- ".*conversion.*\\.go$"
- "zz_generated.*\\.go$"
- ".*conversion.*\\.go$"
allow-parallel-runners: true
2 changes: 1 addition & 1 deletion alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ var (
// the manager.
//
// Deprecated: This is deprecated in favor of using Options directly.
ConfigFile = cfg.File //nolint:staticcheck
ConfigFile = cfg.File

// NewControllerManagedBy returns a new controller builder that will be started by the provided Manager.
NewControllerManagedBy = builder.ControllerManagedBy
Expand Down
14 changes: 7 additions & 7 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,24 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"sigs.k8s.io/controller-runtime/pkg/config/v1alpha1" //nolint:staticcheck
"sigs.k8s.io/controller-runtime/pkg/config/v1alpha1"
)

// ControllerManagerConfiguration defines the functions necessary to parse a config file
// and to configure the Options struct for the ctrl.Manager.
//
// Deprecated: This package has been deprecated and will be removed in a future release.
// Deprecated: The component config package has been deprecated and will be removed in a future release. Users should migrate to their own config implementation, please share feedback in https://github.com/kubernetes-sigs/controller-runtime/issues/895.
type ControllerManagerConfiguration interface {
runtime.Object

// Complete returns the versioned configuration
Complete() (v1alpha1.ControllerManagerConfigurationSpec, error) //nolint:staticcheck
Complete() (v1alpha1.ControllerManagerConfigurationSpec, error)
}

// DeferredFileLoader is used to configure the decoder for loading controller
// runtime component config types.
//
// Deprecated: This package has been deprecated and will be removed in a future release.
// Deprecated: The component config package has been deprecated and will be removed in a future release. Users should migrate to their own config implementation, please share feedback in https://github.com/kubernetes-sigs/controller-runtime/issues/895.
type DeferredFileLoader struct {
ControllerManagerConfiguration
path string
Expand All @@ -57,7 +57,7 @@ type DeferredFileLoader struct {
// * Path: "./config.yaml"
// * Kind: GenericControllerManagerConfiguration
//
// Deprecated: This package has been deprecated and will be removed in a future release.
// Deprecated: The component config package has been deprecated and will be removed in a future release. Users should migrate to their own config implementation, please share feedback in https://github.com/kubernetes-sigs/controller-runtime/issues/895.
func File() *DeferredFileLoader {
scheme := runtime.NewScheme()
utilruntime.Must(v1alpha1.AddToScheme(scheme))
Expand All @@ -69,10 +69,10 @@ func File() *DeferredFileLoader {
}

// Complete will use sync.Once to set the scheme.
func (d *DeferredFileLoader) Complete() (v1alpha1.ControllerManagerConfigurationSpec, error) { //nolint:staticcheck
func (d *DeferredFileLoader) Complete() (v1alpha1.ControllerManagerConfigurationSpec, error) {
d.once.Do(d.loadFile)
if d.err != nil {
return v1alpha1.ControllerManagerConfigurationSpec{}, d.err //nolint:staticcheck
return v1alpha1.ControllerManagerConfigurationSpec{}, d.err
}
return d.ControllerManagerConfiguration.Complete()
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/config/config_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
Copyright 2018 The Kubernetes 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 config_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestScheme(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Config Suite")
}
45 changes: 45 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
Copyright 2020 The Kubernetes 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 config_test

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"sigs.k8s.io/controller-runtime/pkg/config"
"sigs.k8s.io/controller-runtime/pkg/config/v1alpha1"
)

var _ = Describe("config", func() {
Describe("File", func() {

It("should error loading from non existent file", func() {
loader := config.File()
_, err := loader.Complete()
Expect(err).ToNot(BeNil())
})

It("should load a config from file", func() {
conf := v1alpha1.ControllerManagerConfiguration{}
loader := config.File().AtPath("./testdata/config.yaml").OfKind(&conf)
Expect(conf.CacheNamespace).To(Equal(""))

_, err := loader.Complete()
Expect(err).To(BeNil())

Expect(*conf.LeaderElection.LeaderElect).To(Equal(true))
Expect(conf.CacheNamespace).To(Equal("default"))
Expect(conf.Metrics.BindAddress).To(Equal(":8081"))
})
})
})
Loading