Merge pull request #2191 from vincepri/component-config-readd

🌱 Re-add tests for component config

Author: k8s-ci-robot

.golangci.yml

@@ -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:
@@ -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

alias.go

@@ -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

pkg/config/config.go

@@ -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
@@ -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))
@@ -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()
 }

pkg/config/config_suite_test.go

@@ -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")
+}

pkg/config/config_test.go

@@ -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"))
+		})
+	})
+})