Skip to content

Commit

Permalink
Merge pull request #2191 from vincepri/component-config-readd
Browse files Browse the repository at this point in the history
🌱 Re-add tests for component config
  • Loading branch information
k8s-ci-robot committed Feb 13, 2023
2 parents 9f4e5ed + c0a72ba commit c98b7fd
Show file tree
Hide file tree
Showing 14 changed files with 495 additions and 121 deletions.
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

0 comments on commit c98b7fd

Please sign in to comment.