From e481d9996591331d6173e4cb146afcaf5e0308f1 Mon Sep 17 00:00:00 2001 From: Jian Zeng Date: Mon, 3 May 2021 00:01:49 +0800 Subject: [PATCH 1/3] refactor: disable insecure serving in controller-manager Now the following flags have no effect and would be removed in v1.24: * `--port` * `--address` The insecure port flags `--port` may only be set to 0 now. Signed-off-by: Jian Zeng --- .../kube-controller-manager.manifest | 3 +- .../app/controllermanager.go | 29 +++++++---- .../app/options/options.go | 51 +++++++++---------- .../app/options/options_test.go | 13 +---- .../app/testing/testserver.go | 9 ---- pkg/cluster/ports/ports.go | 4 -- pkg/registry/core/rest/storage_core.go | 3 +- 7 files changed, 50 insertions(+), 62 deletions(-) diff --git a/cluster/gce/manifests/kube-controller-manager.manifest b/cluster/gce/manifests/kube-controller-manager.manifest index 09754285822..3c5e4eac587 100644 --- a/cluster/gce/manifests/kube-controller-manager.manifest +++ b/cluster/gce/manifests/kube-controller-manager.manifest @@ -46,7 +46,8 @@ "livenessProbe": { "httpGet": { "host": "127.0.0.1", - "port": 10252, + "port": 10257, + "scheme": "HTTPS", "path": "/healthz" }, "initialDelaySeconds": 15, diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 74713640102..28910e08fbb 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -30,6 +30,7 @@ import ( "time" "github.com/spf13/cobra" + "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -37,7 +38,6 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" genericfeatures "k8s.io/apiserver/pkg/features" - "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/healthz" "k8s.io/apiserver/pkg/server/mux" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -62,6 +62,7 @@ import ( "k8s.io/controller-manager/pkg/clientbuilder" "k8s.io/controller-manager/pkg/informerfactory" "k8s.io/klog/v2" + "k8s.io/kubernetes/cmd/kube-controller-manager/app/config" "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" kubectrlmgrconfig "k8s.io/kubernetes/pkg/controller/apis/config" @@ -86,6 +87,18 @@ const ( ExternalLoops ) +// TODO: delete this check after insecure flags removed in v1.24 +func checkNonZeroInsecurePort(fs *pflag.FlagSet) error { + val, err := fs.GetInt("port") + if err != nil { + return err + } + if val != 0 { + return fmt.Errorf("invalid port value %d: only zero is allowed", val) + } + return nil +} + // NewControllerManagerCommand creates a *cobra.Command object with default parameters func NewControllerManagerCommand() *cobra.Command { s, err := options.NewKubeControllerManagerOptions() @@ -114,6 +127,12 @@ controller, and serviceaccounts controller.`, verflag.PrintAndExitIfRequested() cliflag.PrintFlags(cmd.Flags()) + err := checkNonZeroInsecurePort(cmd.Flags()) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + os.Exit(1) + } + c, err := s.Config(KnownControllers(), ControllersDisabledByDefault.List()) if err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) @@ -198,14 +217,6 @@ func Run(c *config.CompletedConfig, stopCh <-chan struct{}) error { return err } } - if c.InsecureServing != nil { - unsecuredMux = genericcontrollermanager.NewBaseHandler(&c.ComponentConfig.Generic.Debugging, checks...) - insecureSuperuserAuthn := server.AuthenticationInfo{Authenticator: &server.InsecureSuperuser{}} - handler := genericcontrollermanager.BuildHandlerChain(unsecuredMux, nil, &insecureSuperuserAuthn) - if err := c.InsecureServing.Serve(handler, 0, stopCh); err != nil { - return err - } - } run := func(ctx context.Context) { rootClientBuilder := clientbuilder.SimpleControllerClientBuilder{ diff --git a/cmd/kube-controller-manager/app/options/options.go b/cmd/kube-controller-manager/app/options/options.go index a4a559715f0..f8d7c38c082 100644 --- a/cmd/kube-controller-manager/app/options/options.go +++ b/cmd/kube-controller-manager/app/options/options.go @@ -22,6 +22,7 @@ import ( "fmt" "net" + "github.com/spf13/pflag" v1 "k8s.io/api/core/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" apiserveroptions "k8s.io/apiserver/pkg/server/options" @@ -84,13 +85,11 @@ type KubeControllerManagerOptions struct { SAController *SAControllerOptions TTLAfterFinishedController *TTLAfterFinishedControllerOptions - SecureServing *apiserveroptions.SecureServingOptionsWithLoopback - // TODO: remove insecure serving mode - InsecureServing *apiserveroptions.DeprecatedInsecureServingOptionsWithLoopback - Authentication *apiserveroptions.DelegatingAuthenticationOptions - Authorization *apiserveroptions.DelegatingAuthorizationOptions - Metrics *metrics.Options - Logs *logs.Options + SecureServing *apiserveroptions.SecureServingOptionsWithLoopback + Authentication *apiserveroptions.DelegatingAuthenticationOptions + Authorization *apiserveroptions.DelegatingAuthorizationOptions + Metrics *metrics.Options + Logs *logs.Options Master string Kubeconfig string @@ -99,7 +98,7 @@ type KubeControllerManagerOptions struct { // NewKubeControllerManagerOptions creates a new KubeControllerManagerOptions with a default config. func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) { - componentConfig, err := NewDefaultComponentConfig(ports.InsecureKubeControllerManagerPort) + componentConfig, err := NewDefaultComponentConfig() if err != nil { return nil, err } @@ -179,12 +178,7 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) { TTLAfterFinishedController: &TTLAfterFinishedControllerOptions{ &componentConfig.TTLAfterFinishedController, }, - SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(), - InsecureServing: (&apiserveroptions.DeprecatedInsecureServingOptions{ - BindAddress: net.ParseIP(componentConfig.Generic.Address), - BindPort: int(componentConfig.Generic.Port), - BindNetwork: "tcp", - }).WithLoopback(), + SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(), Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(), Authorization: apiserveroptions.NewDelegatingAuthorizationOptions(), Metrics: metrics.NewOptions(), @@ -212,7 +206,7 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) { } // NewDefaultComponentConfig returns kube-controller manager configuration object. -func NewDefaultComponentConfig(insecurePort int32) (kubectrlmgrconfig.KubeControllerManagerConfiguration, error) { +func NewDefaultComponentConfig() (kubectrlmgrconfig.KubeControllerManagerConfiguration, error) { versioned := kubectrlmgrconfigv1alpha1.KubeControllerManagerConfiguration{} kubectrlmgrconfigscheme.Scheme.Default(&versioned) @@ -220,10 +214,23 @@ func NewDefaultComponentConfig(insecurePort int32) (kubectrlmgrconfig.KubeContro if err := kubectrlmgrconfigscheme.Scheme.Convert(&versioned, &internal, nil); err != nil { return internal, err } - internal.Generic.Port = insecurePort return internal, nil } +// TODO: remove these insecure flags in v1.24 +func addDummyInsecureFlags(fs *pflag.FlagSet) { + var ( + bindAddr = net.IPv4(127, 0, 0, 1) + bindPort = 0 + ) + fs.IPVar(&bindAddr, "address", bindAddr, + "The IP address on which to serve the insecure --port (set to 0.0.0.0 for all IPv4 interfaces and :: for all IPv6 interfaces).") + fs.MarkDeprecated("address", "This flag has no effect now and will be removed in v1.24.") + + fs.IntVar(&bindPort, "port", bindPort, "The port on which to serve unsecured, unauthenticated access. Set to 0 to disable.") + fs.MarkDeprecated("port", "This flag has no effect now and will be removed in v1.24.") +} + // Flags returns flags for a specific APIServer by section name func (s *KubeControllerManagerOptions) Flags(allControllers []string, disabledByDefaultControllers []string) cliflag.NamedFlagSets { fss := cliflag.NamedFlagSets{} @@ -232,7 +239,7 @@ func (s *KubeControllerManagerOptions) Flags(allControllers []string, disabledBy s.ServiceController.AddFlags(fss.FlagSet("service controller")) s.SecureServing.AddFlags(fss.FlagSet("secure serving")) - s.InsecureServing.AddUnqualifiedFlags(fss.FlagSet("insecure serving")) + addDummyInsecureFlags(fss.FlagSet("insecure serving")) s.Authentication.AddFlags(fss.FlagSet("authentication")) s.Authorization.AddFlags(fss.FlagSet("authorization")) @@ -350,9 +357,6 @@ func (s *KubeControllerManagerOptions) ApplyTo(c *kubecontrollerconfig.Config) e if err := s.TTLAfterFinishedController.ApplyTo(&c.ComponentConfig.TTLAfterFinishedController); err != nil { return err } - if err := s.InsecureServing.ApplyTo(&c.InsecureServing, &c.LoopbackClientConfig); err != nil { - return err - } if err := s.SecureServing.ApplyTo(&c.SecureServing, &c.LoopbackClientConfig); err != nil { return err } @@ -364,12 +368,6 @@ func (s *KubeControllerManagerOptions) ApplyTo(c *kubecontrollerconfig.Config) e return err } } - - // sync back to component config - // TODO: find more elegant way than syncing back the values. - c.ComponentConfig.Generic.Port = int32(s.InsecureServing.BindPort) - c.ComponentConfig.Generic.Address = s.InsecureServing.BindAddress.String() - return nil } @@ -404,7 +402,6 @@ func (s *KubeControllerManagerOptions) Validate(allControllers []string, disable errs = append(errs, s.ServiceController.Validate()...) errs = append(errs, s.TTLAfterFinishedController.Validate()...) errs = append(errs, s.SecureServing.Validate()...) - errs = append(errs, s.InsecureServing.Validate()...) errs = append(errs, s.Authentication.Validate()...) errs = append(errs, s.Authorization.Validate()...) errs = append(errs, s.Metrics.Validate()...) diff --git a/cmd/kube-controller-manager/app/options/options_test.go b/cmd/kube-controller-manager/app/options/options_test.go index 83b85e3d25a..e26086a082f 100644 --- a/cmd/kube-controller-manager/app/options/options_test.go +++ b/cmd/kube-controller-manager/app/options/options_test.go @@ -62,7 +62,6 @@ import ( ) var args = []string{ - "--address=192.168.4.10", "--allocate-node-cidrs=true", "--attach-detach-reconcile-sync-period=30s", "--cidr-allocator-type=CloudAllocator", @@ -136,7 +135,6 @@ var args = []string{ "--node-monitor-period=10s", "--node-startup-grace-period=30s", "--pod-eviction-timeout=2m", - "--port=10000", "--profiling=false", "--pv-recycler-increment-timeout-nfs=45", "--pv-recycler-minimum-timeout-hostpath=45", @@ -171,8 +169,7 @@ func TestAddFlags(t *testing.T) { expected := &KubeControllerManagerOptions{ Generic: &cmoptions.GenericControllerManagerConfigurationOptions{ GenericControllerManagerConfiguration: &cmconfig.GenericControllerManagerConfiguration{ - Port: 10252, // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config - Address: "0.0.0.0", // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config + Address: "0.0.0.0", // Note: This field should have no effect in CM now, and "0.0.0.0" is the default value. MinResyncPeriod: metav1.Duration{Duration: 8 * time.Hour}, ClientConnection: componentbaseconfig.ClientConnectionConfiguration{ ContentType: "application/json", @@ -405,11 +402,6 @@ func TestAddFlags(t *testing.T) { }, HTTP2MaxStreamsPerConnection: 47, }).WithLoopback(), - InsecureServing: (&apiserveroptions.DeprecatedInsecureServingOptions{ - BindAddress: net.ParseIP("192.168.4.10"), - BindPort: int(10000), - BindNetwork: "tcp", - }).WithLoopback(), Authentication: &apiserveroptions.DelegatingAuthenticationOptions{ CacheTTL: 10 * time.Second, ClientTimeout: 10 * time.Second, @@ -462,8 +454,7 @@ func TestApplyTo(t *testing.T) { expected := &kubecontrollerconfig.Config{ ComponentConfig: kubectrlmgrconfig.KubeControllerManagerConfiguration{ Generic: cmconfig.GenericControllerManagerConfiguration{ - Port: 10252, // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config - Address: "0.0.0.0", // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config + Address: "0.0.0.0", // Note: This field should have no effect in CM now, and "0.0.0.0" is the default value. MinResyncPeriod: metav1.Duration{Duration: 8 * time.Hour}, ClientConnection: componentbaseconfig.ClientConnectionConfiguration{ ContentType: "application/json", diff --git a/cmd/kube-controller-manager/app/testing/testserver.go b/cmd/kube-controller-manager/app/testing/testserver.go index 7d3f0c5579a..8c1e1402e70 100644 --- a/cmd/kube-controller-manager/app/testing/testserver.go +++ b/cmd/kube-controller-manager/app/testing/testserver.go @@ -101,15 +101,6 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err t.Logf("kube-controller-manager will listen securely on port %d...", s.SecureServing.BindPort) } - if s.InsecureServing.BindPort != 0 { - s.InsecureServing.Listener, s.InsecureServing.BindPort, err = createListenerOnFreePort() - if err != nil { - return result, fmt.Errorf("failed to create listener: %v", err) - } - - t.Logf("kube-controller-manager will listen insecurely on port %d...", s.InsecureServing.BindPort) - } - config, err := s.Config(all, disabled) if err != nil { return result, fmt.Errorf("failed to create config from options: %v", err) diff --git a/pkg/cluster/ports/ports.go b/pkg/cluster/ports/ports.go index 7407060d920..8fd44e01b40 100644 --- a/pkg/cluster/ports/ports.go +++ b/pkg/cluster/ports/ports.go @@ -25,10 +25,6 @@ const ( // KubeletPort is the default port for the kubelet server on each host machine. // May be overridden by a flag at startup. KubeletPort = 10250 - // InsecureKubeControllerManagerPort is the default port for the controller manager status server. - // May be overridden by a flag at startup. - // Deprecated: use the secure KubeControllerManagerPort instead. - InsecureKubeControllerManagerPort = 10252 // KubeletReadOnlyPort exposes basic read-only services from the kubelet. // May be overridden by a flag at startup. // This is necessary for heapster to collect monitoring stats from the kubelet diff --git a/pkg/registry/core/rest/storage_core.go b/pkg/registry/core/rest/storage_core.go index 40aed6226c7..750dcb79043 100644 --- a/pkg/registry/core/rest/storage_core.go +++ b/pkg/registry/core/rest/storage_core.go @@ -17,6 +17,7 @@ limitations under the License. package rest import ( + "crypto/tls" "fmt" "net" "net/http" @@ -343,7 +344,7 @@ func (s componentStatusStorage) serversToValidate() map[string]*componentstatus. // this is fragile, which assumes that the default port is being used // TODO: switch to secure port until these components remove the ability to serve insecurely. serversToValidate := map[string]*componentstatus.Server{ - "controller-manager": {Addr: "127.0.0.1", Port: ports.InsecureKubeControllerManagerPort, Path: "/healthz"}, + "controller-manager": {EnableHTTPS: true, TLSConfig: &tls.Config{InsecureSkipVerify: true}, Addr: "127.0.0.1", Port: ports.KubeControllerManagerPort, Path: "/healthz"}, "scheduler": {Addr: "127.0.0.1", Port: kubeschedulerconfig.DefaultInsecureSchedulerPort, Path: "/healthz"}, } From c4c25747789d7a394ae31501f7c0b5beb92e711d Mon Sep 17 00:00:00 2001 From: Jian Zeng Date: Mon, 3 May 2021 00:12:06 +0800 Subject: [PATCH 2/3] refactor(e2e): grab metrics from controller-manager via nginx Signed-off-by: Jian Zeng --- test/e2e/e2e.go | 6 + test/e2e/framework/metrics/metrics_grabber.go | 25 +-- test/e2e/framework/metrics/metrics_proxy.go | 199 ++++++++++++++++++ test/e2e/framework/pod/resource.go | 2 +- test/e2e/framework/ports.go | 4 - test/e2e/framework/providers/gce/firewall.go | 2 +- .../monitoring/metrics_grabber.go | 6 +- 7 files changed, 221 insertions(+), 23 deletions(-) create mode 100644 test/e2e/framework/metrics/metrics_proxy.go diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index 9b826b06112..28f72057122 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -46,6 +46,7 @@ import ( "k8s.io/kubernetes/test/e2e/framework" e2ekubectl "k8s.io/kubernetes/test/e2e/framework/kubectl" e2emanifest "k8s.io/kubernetes/test/e2e/framework/manifest" + "k8s.io/kubernetes/test/e2e/framework/metrics" e2enode "k8s.io/kubernetes/test/e2e/framework/node" e2epod "k8s.io/kubernetes/test/e2e/framework/pod" e2ereporters "k8s.io/kubernetes/test/e2e/reporters" @@ -307,6 +308,11 @@ func setupSuite() { nodeKiller := framework.NewNodeKiller(framework.TestContext.NodeKiller, c, framework.TestContext.Provider) go nodeKiller.Run(framework.TestContext.NodeKiller.NodeKillerStopCh) } + + err = metrics.SetupMetricsProxy(c) + if err != nil { + framework.Logf("Fail to setup metrics proxy: %v", err) + } } // logClusterImageSources writes out cluster image sources. diff --git a/test/e2e/framework/metrics/metrics_grabber.go b/test/e2e/framework/metrics/metrics_grabber.go index 1d341cc9360..24d97a7c0fd 100644 --- a/test/e2e/framework/metrics/metrics_grabber.go +++ b/test/e2e/framework/metrics/metrics_grabber.go @@ -27,20 +27,17 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" - e2epod "k8s.io/kubernetes/test/e2e/framework/pod" - "k8s.io/klog/v2" + + e2epod "k8s.io/kubernetes/test/e2e/framework/pod" ) const ( - // insecureSchedulerPort is the default port for the scheduler status server. - // May be overridden by a flag at startup. - // Deprecated: use the secure KubeSchedulerPort instead. - insecureSchedulerPort = 10251 - // insecureKubeControllerManagerPort is the default port for the controller manager status server. - // May be overridden by a flag at startup. - // Deprecated: use the secure KubeControllerManagerPort instead. - insecureKubeControllerManagerPort = 10252 + // kubeSchedulerPort is the default port for the scheduler status server. + kubeSchedulerPort = 10259 + // kubeControllerManagerPort is the default port for the controller manager status server. + kubeControllerManagerPort = 10257 + metricsProxyPod = "metrics-proxy" ) // Collection is metrics collection of components @@ -152,7 +149,7 @@ func (g *Grabber) GrabFromScheduler() (SchedulerMetrics, error) { if g.kubeScheduler == "" { return SchedulerMetrics{}, fmt.Errorf("kube-scheduler pod is not registered. Skipping Scheduler's metrics gathering") } - output, err := g.getMetricsFromPod(g.client, g.kubeScheduler, metav1.NamespaceSystem, insecureSchedulerPort) + output, err := g.getMetricsFromPod(g.client, metricsProxyPod, metav1.NamespaceSystem, kubeSchedulerPort) if err != nil { return SchedulerMetrics{}, err } @@ -196,7 +193,7 @@ func (g *Grabber) GrabFromControllerManager() (ControllerManagerMetrics, error) var lastMetricsFetchErr error if metricsWaitErr := wait.PollImmediate(time.Second, time.Minute, func() (bool, error) { - _, lastMetricsFetchErr = g.getMetricsFromPod(g.client, podName, metav1.NamespaceSystem, insecureKubeControllerManagerPort) + _, lastMetricsFetchErr = g.getMetricsFromPod(g.client, metricsProxyPod, metav1.NamespaceSystem, kubeControllerManagerPort) return lastMetricsFetchErr == nil, nil }); metricsWaitErr != nil { err = fmt.Errorf("error waiting for controller manager pod to expose metrics: %v; %v", metricsWaitErr, lastMetricsFetchErr) @@ -207,7 +204,7 @@ func (g *Grabber) GrabFromControllerManager() (ControllerManagerMetrics, error) return ControllerManagerMetrics{}, err } - output, err := g.getMetricsFromPod(g.client, podName, metav1.NamespaceSystem, insecureKubeControllerManagerPort) + output, err := g.getMetricsFromPod(g.client, metricsProxyPod, metav1.NamespaceSystem, kubeControllerManagerPort) if err != nil { return ControllerManagerMetrics{}, err } @@ -286,7 +283,7 @@ func (g *Grabber) getMetricsFromPod(client clientset.Interface, podName string, Namespace(namespace). Resource("pods"). SubResource("proxy"). - Name(fmt.Sprintf("%v:%v", podName, port)). + Name(fmt.Sprintf("%s:%d", podName, port)). Suffix("metrics"). Do(context.TODO()).Raw() if err != nil { diff --git a/test/e2e/framework/metrics/metrics_proxy.go b/test/e2e/framework/metrics/metrics_proxy.go new file mode 100644 index 00000000000..f096238cf8f --- /dev/null +++ b/test/e2e/framework/metrics/metrics_proxy.go @@ -0,0 +1,199 @@ +/* +Copyright 2021 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 metrics + +import ( + "context" + "fmt" + "strings" + "time" + + v1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" + + e2epod "k8s.io/kubernetes/test/e2e/framework/pod" + imageutils "k8s.io/kubernetes/test/utils/image" +) + +type componentInfo struct { + Port int + IP string +} + +// SetupMetricsProxy creates a nginx Pod to expose metrics from the secure port of kube-scheduler and kube-controller-manager in tests. +func SetupMetricsProxy(c clientset.Interface) error { + podList, err := c.CoreV1().Pods(metav1.NamespaceSystem).List(context.TODO(), metav1.ListOptions{}) + if err != nil { + return err + } + var infos []componentInfo + for _, pod := range podList.Items { + switch { + case strings.HasPrefix(pod.Name, "kube-scheduler-"): + infos = append(infos, componentInfo{ + Port: kubeSchedulerPort, + IP: pod.Status.PodIP, + }) + case strings.HasPrefix(pod.Name, "kube-controller-manager-"): + infos = append(infos, componentInfo{ + Port: kubeControllerManagerPort, + IP: pod.Status.PodIP, + }) + } + if len(infos) == 2 { + break + } + } + if len(infos) == 0 { + klog.Warningf("Can't find any pods in namespace %s to grab metrics from", metav1.NamespaceSystem) + return nil + } + + const name = metricsProxyPod + _, err = c.CoreV1().ServiceAccounts(metav1.NamespaceSystem).Create(context.TODO(), &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + }, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("create serviceAccount: %w", err) + } + _, err = c.RbacV1().ClusterRoles().Create(context.TODO(), &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Rules: []rbacv1.PolicyRule{ + { + NonResourceURLs: []string{"/metrics"}, + Verbs: []string{"get"}, + }, + }, + }, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("create clusterRole: %w", err) + } + _, err = c.RbacV1().ClusterRoleBindings().Create(context.TODO(), &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.ServiceAccountKind, + Name: name, + Namespace: metav1.NamespaceSystem, + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: name, + }, + }, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("create clusterRoleBinding: %w", err) + } + + var token string + err = wait.PollImmediate(time.Second*5, time.Minute*5, func() (done bool, err error) { + sa, err := c.CoreV1().ServiceAccounts(metav1.NamespaceSystem).Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + klog.Warningf("Fail to get serviceAccount %s: %v", name, err) + return false, nil + } + if len(sa.Secrets) < 1 { + klog.Warningf("No secret found in serviceAccount %s", name) + return false, nil + } + secretRef := sa.Secrets[0] + secret, err := c.CoreV1().Secrets(metav1.NamespaceSystem).Get(context.TODO(), secretRef.Name, metav1.GetOptions{}) + if err != nil { + klog.Warningf("Fail to get secret %s", secretRef.Name) + return false, nil + } + token = string(secret.Data["token"]) + if len(token) == 0 { + klog.Warningf("Token in secret %s is empty", secretRef.Name) + return false, nil + } + return true, nil + }) + if err != nil { + return err + } + + var nginxConfig string + for _, info := range infos { + nginxConfig += fmt.Sprintf(` +server { + listen %d; + server_name _; + proxy_set_header Authorization "Bearer %s"; + proxy_ssl_verify off; + location /metrics { + proxy_pass https://%s:%d; + } +} +`, info.Port, token, info.IP, info.Port) + } + _, err = c.CoreV1().ConfigMaps(metav1.NamespaceSystem).Create(context.TODO(), &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceSystem, + }, + Data: map[string]string{ + "metrics.conf": nginxConfig, + }, + }, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("create nginx configmap: %w", err) + } + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceSystem, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "nginx", + Image: imageutils.GetE2EImage(imageutils.Nginx), + VolumeMounts: []v1.VolumeMount{{ + Name: "config", + MountPath: "/etc/nginx/conf.d", + ReadOnly: true, + }}, + }}, + Volumes: []v1.Volume{{ + Name: "config", + VolumeSource: v1.VolumeSource{ + ConfigMap: &v1.ConfigMapVolumeSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: name, + }, + }, + }, + }}, + }, + } + _, err = c.CoreV1().Pods(metav1.NamespaceSystem).Create(context.TODO(), pod, metav1.CreateOptions{}) + if err != nil { + return err + } + err = e2epod.WaitForPodNameRunningInNamespace(c, name, metav1.NamespaceSystem) + if err != nil { + return err + } + klog.Info("Successfully setup metrics-proxy") + return nil +} diff --git a/test/e2e/framework/pod/resource.go b/test/e2e/framework/pod/resource.go index 668cf263ad7..9c34125b0d1 100644 --- a/test/e2e/framework/pod/resource.go +++ b/test/e2e/framework/pod/resource.go @@ -25,7 +25,6 @@ import ( "github.com/onsi/ginkgo" "github.com/onsi/gomega" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -34,6 +33,7 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/util/podutils" + e2elog "k8s.io/kubernetes/test/e2e/framework/log" testutils "k8s.io/kubernetes/test/utils" imageutils "k8s.io/kubernetes/test/utils/image" diff --git a/test/e2e/framework/ports.go b/test/e2e/framework/ports.go index 5eb08c6a8fb..38326a12234 100644 --- a/test/e2e/framework/ports.go +++ b/test/e2e/framework/ports.go @@ -22,10 +22,6 @@ const ( // KubeletPort is the default port for the kubelet server on each host machine. // May be overridden by a flag at startup. KubeletPort = 10250 - // InsecureKubeControllerManagerPort is the default port for the controller manager status server. - // May be overridden by a flag at startup. - // Deprecated: use the secure KubeControllerManagerPort instead. - InsecureKubeControllerManagerPort = 10252 // KubeControllerManagerPort is the default port for the controller manager status server. // May be overridden by a flag at startup. KubeControllerManagerPort = 10257 diff --git a/test/e2e/framework/providers/gce/firewall.go b/test/e2e/framework/providers/gce/firewall.go index 6126a9a6238..76478598f9e 100644 --- a/test/e2e/framework/providers/gce/firewall.go +++ b/test/e2e/framework/providers/gce/firewall.go @@ -25,7 +25,7 @@ import ( compute "google.golang.org/api/compute/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" cloudprovider "k8s.io/cloud-provider" diff --git a/test/e2e/instrumentation/monitoring/metrics_grabber.go b/test/e2e/instrumentation/monitoring/metrics_grabber.go index 4ddede2d5d1..b4f2fbaab1b 100644 --- a/test/e2e/instrumentation/monitoring/metrics_grabber.go +++ b/test/e2e/instrumentation/monitoring/metrics_grabber.go @@ -22,15 +22,15 @@ import ( "strings" "time" + "github.com/onsi/ginkgo" + "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" + "k8s.io/kubernetes/test/e2e/framework" e2emetrics "k8s.io/kubernetes/test/e2e/framework/metrics" e2enode "k8s.io/kubernetes/test/e2e/framework/node" instrumentation "k8s.io/kubernetes/test/e2e/instrumentation/common" - - "github.com/onsi/ginkgo" - "github.com/onsi/gomega" ) var _ = instrumentation.SIGDescribe("MetricsGrabber", func() { From 97b5d2a3003ef1a36363d46b037fe2a8726691b2 Mon Sep 17 00:00:00 2001 From: Jian Zeng Date: Mon, 3 May 2021 00:35:16 +0800 Subject: [PATCH 3/3] test: update test cases of TestComponentSecureServingAndAuth --- test/integration/serving/serving_test.go | 135 +++++++++++++++++++++-- 1 file changed, 123 insertions(+), 12 deletions(-) diff --git a/test/integration/serving/serving_test.go b/test/integration/serving/serving_test.go index 95b914b058b..926fe64afb5 100644 --- a/test/integration/serving/serving_test.go +++ b/test/integration/serving/serving_test.go @@ -30,7 +30,7 @@ import ( "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/options" - "k8s.io/cloud-provider" + cloudprovider "k8s.io/cloud-provider" cloudctrlmgrtesting "k8s.io/cloud-provider/app/testing" "k8s.io/cloud-provider/fake" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" @@ -159,17 +159,23 @@ users: brokenApiserverConfig.Close() tests := []struct { - name string - tester componentTester - extraFlags []string + name string + tester componentTester + extraFlags []string + insecureDisabled bool }{ - {"kube-controller-manager", kubeControllerManagerTester{}, nil}, - {"cloud-controller-manager", cloudControllerManagerTester{}, []string{"--cloud-provider=fake"}}, - {"kube-scheduler", kubeSchedulerTester{}, nil}, + {"kube-controller-manager", kubeControllerManagerTester{}, nil, true}, + {"cloud-controller-manager", cloudControllerManagerTester{}, []string{"--cloud-provider=fake"}, false}, + {"kube-scheduler", kubeSchedulerTester{}, nil, false}, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - testComponent(t, tt.tester, apiserverConfig.Name(), brokenApiserverConfig.Name(), token, tt.extraFlags) + if tt.insecureDisabled { + testComponentWithSecureServing(t, tt.tester, apiserverConfig.Name(), brokenApiserverConfig.Name(), token, tt.extraFlags) + } else { + testComponent(t, tt.tester, apiserverConfig.Name(), brokenApiserverConfig.Name(), token, tt.extraFlags) + } }) } } @@ -215,7 +221,7 @@ func testComponent(t *testing.T, tester componentTester, kubeconfig, brokenKubec }, "/healthz", false, false, intPtr(http.StatusOK), nil}, {"authorization skipped for /healthz with BROKEN authn/authz", []string{ "--port=0", - "--authentication-skip-lookup", // to survive unaccessible extensions-apiserver-authentication configmap + "--authentication-skip-lookup", // to survive inaccessible extensions-apiserver-authentication configmap "--authentication-kubeconfig", brokenKubeconfig, "--authorization-kubeconfig", brokenKubeconfig, "--kubeconfig", kubeconfig, @@ -237,9 +243,9 @@ func testComponent(t *testing.T, tester componentTester, kubeconfig, brokenKubec }, "/metrics", false, false, intPtr(http.StatusInternalServerError), intPtr(http.StatusOK)}, {"always-allowed /metrics with BROKEN authn/authz", []string{ "--port=0", - "--authentication-skip-lookup", // to survive unaccessible extensions-apiserver-authentication configmap - "--authentication-kubeconfig", kubeconfig, - "--authorization-kubeconfig", kubeconfig, + "--authentication-skip-lookup", // to survive inaccessible extensions-apiserver-authentication configmap + "--authentication-kubeconfig", brokenKubeconfig, + "--authorization-kubeconfig", brokenKubeconfig, "--authorization-always-allow-paths", "/healthz,/metrics", "--kubeconfig", kubeconfig, "--leader-elect=false", @@ -322,6 +328,111 @@ func testComponent(t *testing.T, tester componentTester, kubeconfig, brokenKubec } } +func testComponentWithSecureServing(t *testing.T, tester componentTester, kubeconfig, brokenKubeconfig, token string, extraFlags []string) { + tests := []struct { + name string + flags []string + path string + anonymous bool // to use the token or not + wantErr bool + wantSecureCode *int + }{ + {"no-flags", nil, "/healthz", false, true, nil}, + {"/healthz without authn/authz", []string{ + "--kubeconfig", kubeconfig, + "--leader-elect=false", + }, "/healthz", true, false, intPtr(http.StatusOK)}, + {"/metrics without authn/authz", []string{ + "--kubeconfig", kubeconfig, + "--leader-elect=false", + }, "/metrics", true, false, intPtr(http.StatusForbidden)}, + {"authorization skipped for /healthz with authn/authz", []string{ + "--authentication-kubeconfig", kubeconfig, + "--authorization-kubeconfig", kubeconfig, + "--kubeconfig", kubeconfig, + "--leader-elect=false", + }, "/healthz", false, false, intPtr(http.StatusOK)}, + {"authorization skipped for /healthz with BROKEN authn/authz", []string{ + "--authentication-skip-lookup", // to survive inaccessible extensions-apiserver-authentication configmap + "--authentication-kubeconfig", brokenKubeconfig, + "--authorization-kubeconfig", brokenKubeconfig, + "--kubeconfig", kubeconfig, + "--leader-elect=false", + }, "/healthz", false, false, intPtr(http.StatusOK)}, + {"not authorized /metrics with BROKEN authn/authz", []string{ + "--authentication-kubeconfig", kubeconfig, + "--authorization-kubeconfig", brokenKubeconfig, + "--kubeconfig", kubeconfig, + "--leader-elect=false", + }, "/metrics", false, false, intPtr(http.StatusInternalServerError)}, + {"always-allowed /metrics with BROKEN authn/authz", []string{ + "--authentication-skip-lookup", // to survive inaccessible extensions-apiserver-authentication configmap + "--authentication-kubeconfig", brokenKubeconfig, + "--authorization-kubeconfig", brokenKubeconfig, + "--authorization-always-allow-paths", "/healthz,/metrics", + "--kubeconfig", kubeconfig, + "--leader-elect=false", + }, "/metrics", false, false, intPtr(http.StatusOK)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + secureOptions, secureInfo, _, tearDownFn, err := tester.StartTestServer(t, append(append([]string{}, tt.flags...), extraFlags...)) + if tearDownFn != nil { + defer tearDownFn() + } + if (err != nil) != tt.wantErr { + t.Fatalf("StartTestServer() error = %v, wantErr %v", err, tt.wantErr) + } + if err != nil { + return + } + + if want, got := tt.wantSecureCode != nil, secureInfo != nil; want != got { + t.Errorf("SecureServing enabled: expected=%v got=%v", want, got) + } else if want { + url := fmt.Sprintf("https://%s%s", secureInfo.Listener.Addr().String(), tt.path) + url = strings.Replace(url, "[::]", "127.0.0.1", -1) // switch to IPv4 because the self-signed cert does not support [::] + + // read self-signed server cert disk + pool := x509.NewCertPool() + serverCertPath := path.Join(secureOptions.ServerCert.CertDirectory, secureOptions.ServerCert.PairName+".crt") + serverCert, err := ioutil.ReadFile(serverCertPath) + if err != nil { + t.Fatalf("Failed to read component server cert %q: %v", serverCertPath, err) + } + pool.AppendCertsFromPEM(serverCert) + tr := &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: pool, + }, + } + + client := &http.Client{Transport: tr} + req, err := http.NewRequest("GET", url, nil) + if err != nil { + t.Fatal(err) + } + if !tt.anonymous { + req.Header.Add("Authorization", fmt.Sprintf("Token %s", token)) + } + r, err := client.Do(req) + if err != nil { + t.Fatalf("failed to GET %s from component: %v", tt.path, err) + } + + body, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Fatalf("failed to read response body: %v", err) + } + defer r.Body.Close() + if got, expected := r.StatusCode, *tt.wantSecureCode; got != expected { + t.Fatalf("expected http %d at %s of component, got: %d %q", expected, tt.path, got, string(body)) + } + } + }) + } +} + func intPtr(x int) *int { return &x }