diff --git a/README.md b/README.md index cd358b94f9..484881dce4 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ in sig apimachinery. You can reach the maintainers of this project at: -- Slack channel: [#kubebuilder](http://slack.k8s.io/#kubebuilder) +- Slack channel: [#controller-runtime](https://kubernetes.slack.com/archives/C02MRBMN00Z) - Google Group: [kubebuilder@googlegroups.com](https://groups.google.com/forum/#!forum/kubebuilder) ## Contributing diff --git a/TMP-LOGGING.md b/TMP-LOGGING.md index 9a3f1418a3..97e091fd48 100644 --- a/TMP-LOGGING.md +++ b/TMP-LOGGING.md @@ -21,7 +21,7 @@ log.Printf("starting reconciliation for pod %s/%s", podNamespace, podName) In controller-runtime, we'd instead write: ```go -logger.Info("starting reconciliation", "pod", req.NamespacedNamed) +logger.Info("starting reconciliation", "pod", req.NamespacedName) ``` or even write diff --git a/alias.go b/alias.go index 29f964dcbe..35cba30be5 100644 --- a/alias.go +++ b/alias.go @@ -70,6 +70,10 @@ type TypeMeta = metav1.TypeMeta type ObjectMeta = metav1.ObjectMeta var ( + // RegisterFlags registers flag variables to the given FlagSet if not already registered. + // It uses the default command line FlagSet, if none is provided. Currently, it only registers the kubeconfig flag. + RegisterFlags = config.RegisterFlags + // GetConfigOrDie creates a *rest.Config for talking to a Kubernetes apiserver. // If --kubeconfig is set, will use the kubeconfig file at that location. Otherwise will assume running // in cluster and use the cluster provided kubeconfig. diff --git a/pkg/cache/internal/cache_reader.go b/pkg/cache/internal/cache_reader.go index 9c2255123c..107f20fa6b 100644 --- a/pkg/cache/internal/cache_reader.go +++ b/pkg/cache/internal/cache_reader.go @@ -23,12 +23,11 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/selection" "k8s.io/client-go/tools/cache" + "sigs.k8s.io/controller-runtime/pkg/internal/field/selector" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -116,7 +115,7 @@ func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...cli case listOpts.FieldSelector != nil: // TODO(directxman12): support more complicated field selectors by // combining multiple indices, GetIndexers, etc - field, val, requiresExact := requiresExactMatch(listOpts.FieldSelector) + field, val, requiresExact := selector.RequiresExactMatch(listOpts.FieldSelector) if !requiresExact { return fmt.Errorf("non-exact field matches are not supported by the cache") } @@ -186,19 +185,6 @@ func objectKeyToStoreKey(k client.ObjectKey) string { return k.Namespace + "/" + k.Name } -// requiresExactMatch checks if the given field selector is of the form `k=v` or `k==v`. -func requiresExactMatch(sel fields.Selector) (field, val string, required bool) { - reqs := sel.Requirements() - if len(reqs) != 1 { - return "", "", false - } - req := reqs[0] - if req.Operator != selection.Equals && req.Operator != selection.DoubleEquals { - return "", "", false - } - return req.Field, req.Value, true -} - // FieldIndexName constructs the name of the index over the given field, // for use with an indexer. func FieldIndexName(field string) string { diff --git a/pkg/client/config/config.go b/pkg/client/config/config.go index ff44a225fe..a81b1a878d 100644 --- a/pkg/client/config/config.go +++ b/pkg/client/config/config.go @@ -29,15 +29,32 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/internal/log" ) +// KubeconfigFlagName is the name of the kubeconfig flag +const KubeconfigFlagName = "kubeconfig" + var ( kubeconfig string log = logf.RuntimeLog.WithName("client").WithName("config") ) +// init registers the "kubeconfig" flag to the default command line FlagSet. +// TODO: This should be removed, as it potentially leads to redefined flag errors for users, if they already +// have registered the "kubeconfig" flag to the command line FlagSet in other parts of their code. func init() { - // TODO: Fix this to allow double vendoring this library but still register flags on behalf of users - flag.StringVar(&kubeconfig, "kubeconfig", "", - "Paths to a kubeconfig. Only required if out-of-cluster.") + RegisterFlags(flag.CommandLine) +} + +// RegisterFlags registers flag variables to the given FlagSet if not already registered. +// It uses the default command line FlagSet, if none is provided. Currently, it only registers the kubeconfig flag. +func RegisterFlags(fs *flag.FlagSet) { + if fs == nil { + fs = flag.CommandLine + } + if f := fs.Lookup(KubeconfigFlagName); f != nil { + kubeconfig = f.Value.String() + } else { + fs.StringVar(&kubeconfig, KubeconfigFlagName, "", "Paths to a kubeconfig. Only required if out-of-cluster.") + } } // GetConfig creates a *rest.Config for talking to a Kubernetes API server. diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index b7ca2de47a..4ae74b883e 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -30,6 +30,8 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilrand "k8s.io/apimachinery/pkg/util/rand" @@ -37,6 +39,7 @@ import ( "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/testing" + "sigs.k8s.io/controller-runtime/pkg/internal/field/selector" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" @@ -49,9 +52,14 @@ type versionedTracker struct { } type fakeClient struct { - tracker versionedTracker - scheme *runtime.Scheme - restMapper meta.RESTMapper + tracker versionedTracker + scheme *runtime.Scheme + restMapper meta.RESTMapper + + // indexes maps each GroupVersionKind (GVK) to the indexes registered for that GVK. + // The inner map maps from index name to IndexerFunc. + indexes map[schema.GroupVersionKind]map[string]client.IndexerFunc + schemeWriteLock sync.Mutex } @@ -93,6 +101,10 @@ type ClientBuilder struct { initLists []client.ObjectList initRuntimeObjects []runtime.Object objectTracker testing.ObjectTracker + + // indexes maps each GroupVersionKind (GVK) to the indexes registered for that GVK. + // The inner map maps from index name to IndexerFunc. + indexes map[schema.GroupVersionKind]map[string]client.IndexerFunc } // WithScheme sets this builder's internal scheme. @@ -135,6 +147,44 @@ func (f *ClientBuilder) WithObjectTracker(ot testing.ObjectTracker) *ClientBuild return f } +// WithIndex can be optionally used to register an index with name `field` and indexer `extractValue` +// for API objects of the same GroupVersionKind (GVK) as `obj` in the fake client. +// It can be invoked multiple times, both with objects of the same GVK or different ones. +// Invoking WithIndex twice with the same `field` and GVK (via `obj`) arguments will panic. +// WithIndex retrieves the GVK of `obj` using the scheme registered via WithScheme if +// WithScheme was previously invoked, the default scheme otherwise. +func (f *ClientBuilder) WithIndex(obj runtime.Object, field string, extractValue client.IndexerFunc) *ClientBuilder { + objScheme := f.scheme + if objScheme == nil { + objScheme = scheme.Scheme + } + + gvk, err := apiutil.GVKForObject(obj, objScheme) + if err != nil { + panic(err) + } + + // If this is the first index being registered, we initialize the map storing all the indexes. + if f.indexes == nil { + f.indexes = make(map[schema.GroupVersionKind]map[string]client.IndexerFunc) + } + + // If this is the first index being registered for the GroupVersionKind of `obj`, we initialize + // the map storing the indexes for that GroupVersionKind. + if f.indexes[gvk] == nil { + f.indexes[gvk] = make(map[string]client.IndexerFunc) + } + + if _, fieldAlreadyIndexed := f.indexes[gvk][field]; fieldAlreadyIndexed { + panic(fmt.Errorf("indexer conflict: field %s for GroupVersionKind %v is already indexed", + field, gvk)) + } + + f.indexes[gvk][field] = extractValue + + return f +} + // Build builds and returns a new fake client. func (f *ClientBuilder) Build() client.WithWatch { if f.scheme == nil { @@ -171,6 +221,7 @@ func (f *ClientBuilder) Build() client.WithWatch { tracker: tracker, scheme: f.scheme, restMapper: f.restMapper, + indexes: f.indexes, } } @@ -420,21 +471,88 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl return err } - if listOpts.LabelSelector != nil { - objs, err := meta.ExtractList(obj) + if listOpts.LabelSelector == nil && listOpts.FieldSelector == nil { + return nil + } + + // If we're here, either a label or field selector are specified (or both), so before we return + // the list we must filter it. If both selectors are set, they are ANDed. + objs, err := meta.ExtractList(obj) + if err != nil { + return err + } + + filteredList, err := c.filterList(objs, gvk, listOpts.LabelSelector, listOpts.FieldSelector) + if err != nil { + return err + } + + return meta.SetList(obj, filteredList) +} + +func (c *fakeClient) filterList(list []runtime.Object, gvk schema.GroupVersionKind, ls labels.Selector, fs fields.Selector) ([]runtime.Object, error) { + // Filter the objects with the label selector + filteredList := list + if ls != nil { + objsFilteredByLabel, err := objectutil.FilterWithLabels(list, ls) if err != nil { - return err + return nil, err } - filteredObjs, err := objectutil.FilterWithLabels(objs, listOpts.LabelSelector) + filteredList = objsFilteredByLabel + } + + // Filter the result of the previous pass with the field selector + if fs != nil { + objsFilteredByField, err := c.filterWithFields(filteredList, gvk, fs) if err != nil { - return err + return nil, err } - err = meta.SetList(obj, filteredObjs) - if err != nil { - return err + filteredList = objsFilteredByField + } + + return filteredList, nil +} + +func (c *fakeClient) filterWithFields(list []runtime.Object, gvk schema.GroupVersionKind, fs fields.Selector) ([]runtime.Object, error) { + // We only allow filtering on the basis of a single field to ensure consistency with the + // behavior of the cache reader (which we're faking here). + fieldKey, fieldVal, requiresExact := selector.RequiresExactMatch(fs) + if !requiresExact { + return nil, fmt.Errorf("field selector %s is not in one of the two supported forms \"key==val\" or \"key=val\"", + fs) + } + + // Field selection is mimicked via indexes, so there's no sane answer this function can give + // if there are no indexes registered for the GroupVersionKind of the objects in the list. + indexes := c.indexes[gvk] + if len(indexes) == 0 || indexes[fieldKey] == nil { + return nil, fmt.Errorf("List on GroupVersionKind %v specifies selector on field %s, but no "+ + "index with name %s has been registered for GroupVersionKind %v", gvk, fieldKey, fieldKey, gvk) + } + + indexExtractor := indexes[fieldKey] + filteredList := make([]runtime.Object, 0, len(list)) + for _, obj := range list { + if c.objMatchesFieldSelector(obj, indexExtractor, fieldVal) { + filteredList = append(filteredList, obj) } } - return nil + return filteredList, nil +} + +func (c *fakeClient) objMatchesFieldSelector(o runtime.Object, extractIndex client.IndexerFunc, val string) bool { + obj, isClientObject := o.(client.Object) + if !isClientObject { + panic(fmt.Errorf("expected object %v to be of type client.Object, but it's not", o)) + } + + for _, extractedVal := range extractIndex(obj) { + if extractedVal == val { + return true + } + } + + return false } func (c *fakeClient) Scheme() *runtime.Scheme { diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index f95a05d9d4..570cd744ad 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -20,11 +20,14 @@ import ( "context" "encoding/json" "fmt" + "strconv" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes/fake" appsv1 "k8s.io/api/apps/v1" @@ -45,6 +48,7 @@ var _ = Describe("Fake client", func() { var cl client.WithWatch BeforeEach(func() { + replicas := int32(1) dep = &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ APIVersion: "apps/v1", @@ -55,6 +59,12 @@ var _ = Describe("Fake client", func() { Namespace: "ns1", ResourceVersion: trackerAddResourceVersion, }, + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Strategy: appsv1.DeploymentStrategy{ + Type: appsv1.RecreateDeploymentStrategyType, + }, + }, } dep2 = &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -69,6 +79,9 @@ var _ = Describe("Fake client", func() { }, ResourceVersion: trackerAddResourceVersion, }, + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + }, } cm = &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -86,7 +99,7 @@ var _ = Describe("Fake client", func() { } }) - AssertClientBehavior := func() { + AssertClientWithoutIndexBehavior := func() { It("should be able to Get", func() { By("Getting a deployment") namespacedName := types.NamespacedName{ @@ -967,7 +980,7 @@ var _ = Describe("Fake client", func() { WithObjects(dep, dep2, cm). Build() }) - AssertClientBehavior() + AssertClientWithoutIndexBehavior() }) Context("with given scheme", func() { @@ -982,7 +995,162 @@ var _ = Describe("Fake client", func() { WithLists(&appsv1.DeploymentList{Items: []appsv1.Deployment{*dep, *dep2}}). Build() }) - AssertClientBehavior() + AssertClientWithoutIndexBehavior() + }) + + Context("with Indexes", func() { + depReplicasIndexer := func(obj client.Object) []string { + dep, ok := obj.(*appsv1.Deployment) + if !ok { + panic(fmt.Errorf("indexer function for type %T's spec.replicas field received"+ + " object of type %T, this should never happen", appsv1.Deployment{}, obj)) + } + indexVal := "" + if dep.Spec.Replicas != nil { + indexVal = strconv.Itoa(int(*dep.Spec.Replicas)) + } + return []string{indexVal} + } + + depStrategyTypeIndexer := func(obj client.Object) []string { + dep, ok := obj.(*appsv1.Deployment) + if !ok { + panic(fmt.Errorf("indexer function for type %T's spec.strategy.type field received"+ + " object of type %T, this should never happen", appsv1.Deployment{}, obj)) + } + return []string{string(dep.Spec.Strategy.Type)} + } + + var cb *ClientBuilder + BeforeEach(func() { + cb = NewClientBuilder(). + WithObjects(dep, dep2, cm). + WithIndex(&appsv1.Deployment{}, "spec.replicas", depReplicasIndexer) + }) + + Context("client has just one Index", func() { + BeforeEach(func() { cl = cb.Build() }) + + Context("behavior that doesn't use an Index", func() { + AssertClientWithoutIndexBehavior() + }) + + Context("filtered List using field selector", func() { + It("errors when there's no Index for the GroupVersionResource", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("key", "val"), + } + err := cl.List(context.Background(), &corev1.ConfigMapList{}, listOpts) + Expect(err).NotTo(BeNil()) + }) + + It("errors when there's no Index matching the field name", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.paused", "false"), + } + err := cl.List(context.Background(), &appsv1.DeploymentList{}, listOpts) + Expect(err).NotTo(BeNil()) + }) + + It("errors when field selector uses two requirements", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.AndSelectors( + fields.OneTermEqualSelector("spec.replicas", "1"), + fields.OneTermEqualSelector("spec.strategy.type", string(appsv1.RecreateDeploymentStrategyType)), + )} + err := cl.List(context.Background(), &appsv1.DeploymentList{}, listOpts) + Expect(err).NotTo(BeNil()) + }) + + It("returns two deployments that match the only field selector requirement", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.replicas", "1"), + } + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(ConsistOf(*dep, *dep2)) + }) + + It("returns no object because no object matches the only field selector requirement", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.replicas", "2"), + } + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(BeEmpty()) + }) + + It("returns deployment that matches both the field and label selectors", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.replicas", "1"), + LabelSelector: labels.SelectorFromSet(dep2.Labels), + } + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(ConsistOf(*dep2)) + }) + + It("returns no object even if field selector matches because label selector doesn't", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.replicas", "1"), + LabelSelector: labels.Nothing(), + } + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(BeEmpty()) + }) + + It("returns no object even if label selector matches because field selector doesn't", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.replicas", "2"), + LabelSelector: labels.Everything(), + } + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(BeEmpty()) + }) + }) + }) + + Context("client has two Indexes", func() { + BeforeEach(func() { + cl = cb.WithIndex(&appsv1.Deployment{}, "spec.strategy.type", depStrategyTypeIndexer).Build() + }) + + Context("behavior that doesn't use an Index", func() { + AssertClientWithoutIndexBehavior() + }) + + Context("filtered List using field selector", func() { + It("uses the second index to retrieve the indexed objects when there are matches", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.strategy.type", string(appsv1.RecreateDeploymentStrategyType)), + } + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(ConsistOf(*dep)) + }) + + It("uses the second index to retrieve the indexed objects when there are no matches", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("spec.strategy.type", string(appsv1.RollingUpdateDeploymentStrategyType)), + } + list := &appsv1.DeploymentList{} + Expect(cl.List(context.Background(), list, listOpts)).To(Succeed()) + Expect(list.Items).To(BeEmpty()) + }) + + It("errors when field selector uses two requirements", func() { + listOpts := &client.ListOptions{ + FieldSelector: fields.AndSelectors( + fields.OneTermEqualSelector("spec.replicas", "1"), + fields.OneTermEqualSelector("spec.strategy.type", string(appsv1.RecreateDeploymentStrategyType)), + )} + err := cl.List(context.Background(), &appsv1.DeploymentList{}, listOpts) + Expect(err).NotTo(BeNil()) + }) + }) + }) }) It("should set the ResourceVersion to 999 when adding an object to the tracker", func() { @@ -1053,3 +1221,18 @@ var _ = Describe("Fake client", func() { Expect(obj).To(Equal(dep3)) }) }) + +var _ = Describe("Fake client builder", func() { + It("panics when an index with the same name and GroupVersionKind is registered twice", func() { + // We need any realistic GroupVersionKind, the choice of apps/v1 Deployment is arbitrary. + cb := NewClientBuilder().WithIndex(&appsv1.Deployment{}, + "test-name", + func(client.Object) []string { return nil }) + + Expect(func() { + cb.WithIndex(&appsv1.Deployment{}, + "test-name", + func(client.Object) []string { return []string{"foo"} }) + }).To(Panic()) + }) +}) diff --git a/pkg/client/interfaces.go b/pkg/client/interfaces.go index 7f8f8f31c6..10d9057510 100644 --- a/pkg/client/interfaces.go +++ b/pkg/client/interfaces.go @@ -60,7 +60,8 @@ type Reader interface { // Writer knows how to create, delete, and update Kubernetes objects. type Writer interface { - // Create saves the object obj in the Kubernetes cluster. + // Create saves the object obj in the Kubernetes cluster. obj must be a + // struct pointer so that obj can be updated with the content returned by the Server. Create(ctx context.Context, obj Object, opts ...CreateOption) error // Delete deletes the given obj from Kubernetes cluster. diff --git a/pkg/envtest/webhook_test.go b/pkg/envtest/webhook_test.go index bb1726cf01..ca4e936780 100644 --- a/pkg/envtest/webhook_test.go +++ b/pkg/envtest/webhook_test.go @@ -18,6 +18,7 @@ package envtest import ( "context" + "crypto/tls" "path/filepath" "time" @@ -41,6 +42,9 @@ var _ = Describe("Test", func() { Port: env.WebhookInstallOptions.LocalServingPort, Host: env.WebhookInstallOptions.LocalServingHost, CertDir: env.WebhookInstallOptions.LocalServingCertDir, + TLSOpts: []func(*tls.Config){ + func(config *tls.Config) {}, + }, }) // we need manager here just to leverage manager.SetFields Expect(err).NotTo(HaveOccurred()) server := m.GetWebhookServer() diff --git a/pkg/internal/field/selector/utils.go b/pkg/internal/field/selector/utils.go new file mode 100644 index 0000000000..4f6d084318 --- /dev/null +++ b/pkg/internal/field/selector/utils.go @@ -0,0 +1,35 @@ +/* +Copyright 2022 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 selector + +import ( + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/selection" +) + +// RequiresExactMatch checks if the given field selector is of the form `k=v` or `k==v`. +func RequiresExactMatch(sel fields.Selector) (field, val string, required bool) { + reqs := sel.Requirements() + if len(reqs) != 1 { + return "", "", false + } + req := reqs[0] + if req.Operator != selection.Equals && req.Operator != selection.DoubleEquals { + return "", "", false + } + return req.Field, req.Value, true +} diff --git a/pkg/internal/field/selector/utils_suite_test.go b/pkg/internal/field/selector/utils_suite_test.go new file mode 100644 index 0000000000..dd42f1d1ac --- /dev/null +++ b/pkg/internal/field/selector/utils_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2022 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 selector_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestSource(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Fields Selector Utils Suite") +} diff --git a/pkg/internal/field/selector/utils_test.go b/pkg/internal/field/selector/utils_test.go new file mode 100644 index 0000000000..fba214ff16 --- /dev/null +++ b/pkg/internal/field/selector/utils_test.go @@ -0,0 +1,88 @@ +/* +Copyright 2022 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 selector_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/fields" + + . "sigs.k8s.io/controller-runtime/pkg/internal/field/selector" +) + +var _ = Describe("RequiresExactMatch function", func() { + + It("Returns false when the selector matches everything", func() { + _, _, requiresExactMatch := RequiresExactMatch(fields.Everything()) + Expect(requiresExactMatch).To(BeFalse()) + }) + + It("Returns false when the selector matches nothing", func() { + _, _, requiresExactMatch := RequiresExactMatch(fields.Nothing()) + Expect(requiresExactMatch).To(BeFalse()) + }) + + It("Returns false when the selector has the form key!=val", func() { + _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val")) + Expect(requiresExactMatch).To(BeFalse()) + }) + + It("Returns false when the selector has the form key1==val1,key2==val2", func() { + _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key1==val1,key2==val2")) + Expect(requiresExactMatch).To(BeFalse()) + }) + + It("Returns true when the selector has the form key==val", func() { + _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key==val")) + Expect(requiresExactMatch).To(BeTrue()) + }) + + It("Returns true when the selector has the form key=val", func() { + _, _, requiresExactMatch := RequiresExactMatch(fields.ParseSelectorOrDie("key=val")) + Expect(requiresExactMatch).To(BeTrue()) + }) + + It("Returns empty key and value when the selector matches everything", func() { + key, val, _ := RequiresExactMatch(fields.Everything()) + Expect(key).To(Equal("")) + Expect(val).To(Equal("")) + }) + + It("Returns empty key and value when the selector matches nothing", func() { + key, val, _ := RequiresExactMatch(fields.Nothing()) + Expect(key).To(Equal("")) + Expect(val).To(Equal("")) + }) + + It("Returns empty key and value when the selector has the form key!=val", func() { + key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key!=val")) + Expect(key).To(Equal("")) + Expect(val).To(Equal("")) + }) + + It("Returns key and value when the selector has the form key==val", func() { + key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key==val")) + Expect(key).To(Equal("key")) + Expect(val).To(Equal("val")) + }) + + It("Returns key and value when the selector has the form key=val", func() { + key, val, _ := RequiresExactMatch(fields.ParseSelectorOrDie("key=val")) + Expect(key).To(Equal("key")) + Expect(val).To(Equal("val")) + }) +}) diff --git a/pkg/log/zap/zap.go b/pkg/log/zap/zap.go index 1a27ad09b9..ee89a7c6a4 100644 --- a/pkg/log/zap/zap.go +++ b/pkg/log/zap/zap.go @@ -168,7 +168,7 @@ type Options struct { // underlying Zap logger. ZapOpts []zap.Option // TimeEncoder specifies the encoder for the timestamps in log messages. - // Defaults to EpochTimeEncoder as this is the default in Zap currently. + // Defaults to RFC3339TimeEncoder. TimeEncoder zapcore.TimeEncoder } @@ -217,7 +217,7 @@ func (o *Options) addDefaults() { } if o.TimeEncoder == nil { - o.TimeEncoder = zapcore.EpochTimeEncoder + o.TimeEncoder = zapcore.RFC3339TimeEncoder } f := func(ecfg *zapcore.EncoderConfig) { ecfg.EncodeTime = o.TimeEncoder diff --git a/pkg/log/zap/zap_test.go b/pkg/log/zap/zap_test.go index 748923b9f1..1a8b3995c2 100644 --- a/pkg/log/zap/zap_test.go +++ b/pkg/log/zap/zap_test.go @@ -502,7 +502,7 @@ var _ = Describe("Zap log level flag options setup", func() { Expect(optVal.Pointer()).To(Equal(expVal.Pointer())) }) - It("Should default to 'epoch' time encoding", func() { + It("Should default to 'rfc3339' time encoding", func() { args := []string{""} fromFlags.BindFlags(&fs) err := fs.Parse(args) @@ -513,7 +513,7 @@ var _ = Describe("Zap log level flag options setup", func() { opt.addDefaults() optVal := reflect.ValueOf(opt.TimeEncoder) - expVal := reflect.ValueOf(zapcore.EpochTimeEncoder) + expVal := reflect.ValueOf(zapcore.RFC3339TimeEncoder) Expect(optVal.Pointer()).To(Equal(expVal.Pointer())) }) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 5b22c628f9..fb79c55441 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -18,6 +18,7 @@ package manager import ( "context" + "crypto/tls" "errors" "fmt" "net" @@ -135,6 +136,8 @@ type controllerManager struct { // if not set, webhook server would look up the server key and certificate in // {TempDir}/k8s-webhook-server/serving-certs certDir string + // tlsOpts is used to allow configuring the TLS config used for the webhook server. + tlsOpts []func(*tls.Config) webhookServer *webhook.Server // webhookServerOnce will be called in GetWebhookServer() to optionally initialize @@ -305,6 +308,7 @@ func (cm *controllerManager) GetWebhookServer() *webhook.Server { Port: cm.port, Host: cm.host, CertDir: cm.certDir, + TLSOpts: cm.tlsOpts, } } if err := cm.Add(cm.webhookServer); err != nil { diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 028d929d96..072919058a 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -18,6 +18,7 @@ package manager import ( "context" + "crypto/tls" "fmt" "net" "net/http" @@ -193,6 +194,12 @@ type Options struct { // LeaseDuration time first. LeaderElectionReleaseOnCancel bool + // LeaderElectionResourceLockInterface allows to provide a custom resourcelock.Interface that was created outside + // of the controller-runtime. If this value is set the options LeaderElectionID, LeaderElectionNamespace, + // LeaderElectionResourceLock, LeaseDuration, RenewDeadline and RetryPeriod will be ignored. This can be useful if you + // want to use a locking mechanism that is currently not supported, like a MultiLock across two Kubernetes clusters. + LeaderElectionResourceLockInterface resourcelock.Interface + // LeaseDuration is the duration that non-leader candidates will // wait to force acquire leadership. This is measured against time of // last observed ack. Default is 15 seconds. @@ -242,6 +249,9 @@ type Options struct { // It is used to set webhook.Server.CertDir if WebhookServer is not set. CertDir string + // TLSOpts is used to allow configuring the TLS config used for the webhook server. + TLSOpts []func(*tls.Config) + // WebhookServer is an externally configured webhook.Server. By default, // a Manager will create a default server using Port, Host, and CertDir; // if this is set, the Manager will use this server instead. @@ -377,14 +387,19 @@ func New(config *rest.Config, options Options) (Manager, error) { } } - resourceLock, err := options.newResourceLock(leaderConfig, leaderRecorderProvider, leaderelection.Options{ - LeaderElection: options.LeaderElection, - LeaderElectionResourceLock: options.LeaderElectionResourceLock, - LeaderElectionID: options.LeaderElectionID, - LeaderElectionNamespace: options.LeaderElectionNamespace, - }) - if err != nil { - return nil, err + var resourceLock resourcelock.Interface + if options.LeaderElectionResourceLockInterface != nil && options.LeaderElection { + resourceLock = options.LeaderElectionResourceLockInterface + } else { + resourceLock, err = options.newResourceLock(leaderConfig, leaderRecorderProvider, leaderelection.Options{ + LeaderElection: options.LeaderElection, + LeaderElectionResourceLock: options.LeaderElectionResourceLock, + LeaderElectionID: options.LeaderElectionID, + LeaderElectionNamespace: options.LeaderElectionNamespace, + }) + if err != nil { + return nil, err + } } // Create the metrics listener. This will throw an error if the metrics bind @@ -422,6 +437,7 @@ func New(config *rest.Config, options Options) (Manager, error) { port: options.Port, host: options.Host, certDir: options.CertDir, + tlsOpts: options.TLSOpts, webhookServer: options.WebhookServer, leaseDuration: *options.LeaseDuration, renewDeadline: *options.RenewDeadline, diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index b5aef683e6..6b01d48293 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -18,6 +18,7 @@ package manager import ( "context" + "crypto/tls" "errors" "fmt" "io" @@ -211,6 +212,9 @@ var _ = Describe("manger.Manager", func() { }, } + optionsTlSOptsFuncs := []func(*tls.Config){ + func(config *tls.Config) {}, + } m, err := Options{ SyncPeriod: &optDuration, LeaderElection: true, @@ -228,6 +232,7 @@ var _ = Describe("manger.Manager", func() { Port: 8080, Host: "example.com", CertDir: "/pki", + TLSOpts: optionsTlSOptsFuncs, }.AndFrom(&fakeDeferredLoader{ccfg}) Expect(err).To(BeNil()) @@ -247,6 +252,7 @@ var _ = Describe("manger.Manager", func() { Expect(m.Port).To(Equal(8080)) Expect(m.Host).To(Equal("example.com")) Expect(m.CertDir).To(Equal("/pki")) + Expect(m.TLSOpts).To(Equal(optionsTlSOptsFuncs)) }) It("should lazily initialize a webhook server if needed", func() { @@ -511,6 +517,25 @@ var _ = Describe("manger.Manager", func() { Expect(err).To(BeNil()) Expect(record.HolderIdentity).To(BeEmpty()) }) + When("using a custom LeaderElectionResourceLockInterface", func() { + It("should use the custom LeaderElectionResourceLockInterface", func() { + rl, err := fakeleaderelection.NewResourceLock(nil, nil, leaderelection.Options{}) + Expect(err).NotTo(HaveOccurred()) + + m, err := New(cfg, Options{ + LeaderElection: true, + LeaderElectionResourceLockInterface: rl, + newResourceLock: func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) { + return nil, fmt.Errorf("this should not be called") + }, + }) + Expect(m).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + cm, ok := m.(*controllerManager) + Expect(ok).To(BeTrue()) + Expect(cm.resourceLock).To(Equal(rl)) + }) + }) }) It("should create a listener for the metrics if a valid address is provided", func() { diff --git a/pkg/webhook/webhook_integration_test.go b/pkg/webhook/webhook_integration_test.go index 3f0f0d42a1..54cd8ca8b5 100644 --- a/pkg/webhook/webhook_integration_test.go +++ b/pkg/webhook/webhook_integration_test.go @@ -85,6 +85,7 @@ var _ = Describe("Webhook", func() { Port: testenv.WebhookInstallOptions.LocalServingPort, Host: testenv.WebhookInstallOptions.LocalServingHost, CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, + TLSOpts: []func(*tls.Config){func(config *tls.Config) {}}, }) // we need manager here just to leverage manager.SetFields Expect(err).NotTo(HaveOccurred()) server := m.GetWebhookServer() @@ -108,6 +109,7 @@ var _ = Describe("Webhook", func() { Port: testenv.WebhookInstallOptions.LocalServingPort, Host: testenv.WebhookInstallOptions.LocalServingHost, CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, + TLSOpts: []func(*tls.Config){func(config *tls.Config) {}}, }) // we need manager here just to leverage manager.SetFields Expect(err).NotTo(HaveOccurred()) server := m.GetWebhookServer()