Add flagz implementation and enablement in apiserver

richabanker · richabanker · commit da8dc433e9b5 · 2024-11-07T17:11:49.000-08:00
diff --git a/cmd/kube-apiserver/app/options/completion.go b/cmd/kube-apiserver/app/options/completion.go
@@ -26,14 +26,14 @@ import (
 	_ "k8s.io/component-base/metrics/prometheus/workqueue"
 	netutils "k8s.io/utils/net"
 
-	controlplane "k8s.io/kubernetes/pkg/controlplane/apiserver/options"
+	cp "k8s.io/kubernetes/pkg/controlplane/apiserver/options"
 	"k8s.io/kubernetes/pkg/kubeapiserver"
 	kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options"
 )
 
 // completedOptions is a private wrapper that enforces a call of Complete() before Run can be invoked.
 type completedOptions struct {
-	controlplane.CompletedOptions
+	cp.CompletedOptions
 	CloudProvider *kubeoptions.CloudProviderOptions
 
 	Extra
@@ -57,7 +57,7 @@ func (s *ServerRunOptions) Complete(ctx context.Context) (CompletedOptions, erro
 	if err != nil {
 		return CompletedOptions{}, err
 	}
-	controlplane, err := s.Options.Complete(ctx, []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes"}, []net.IP{apiServerServiceIP})
+	controlplane, err := s.Options.Complete(ctx, s.Flags(), []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes"}, []net.IP{apiServerServiceIP})
 	if err != nil {
 		return CompletedOptions{}, err
 	}
@@ -107,7 +107,7 @@ func getServiceIPAndRanges(serviceClusterIPRanges string) (net.IP, net.IPNet, ne
 	// nothing provided by user, use default range (only applies to the Primary)
 	if len(serviceClusterIPRangeList) == 0 {
 		var primaryServiceClusterCIDR net.IPNet
-		primaryServiceIPRange, apiServerServiceIP, err = controlplane.ServiceIPRange(primaryServiceClusterCIDR)
+		primaryServiceIPRange, apiServerServiceIP, err = cp.ServiceIPRange(primaryServiceClusterCIDR)
 		if err != nil {
 			return net.IP{}, net.IPNet{}, net.IPNet{}, fmt.Errorf("error determining service IP ranges: %v", err)
 		}
@@ -119,7 +119,7 @@ func getServiceIPAndRanges(serviceClusterIPRanges string) (net.IP, net.IPNet, ne
 		return net.IP{}, net.IPNet{}, net.IPNet{}, fmt.Errorf("service-cluster-ip-range[0] is not a valid cidr")
 	}
 
-	primaryServiceIPRange, apiServerServiceIP, err = controlplane.ServiceIPRange(*primaryServiceClusterCIDR)
+	primaryServiceIPRange, apiServerServiceIP, err = cp.ServiceIPRange(*primaryServiceClusterCIDR)
 	if err != nil {
 		return net.IP{}, net.IPNet{}, net.IPNet{}, fmt.Errorf("error determining service IP ranges for primary service cidr: %v", err)
 	}
diff --git a/pkg/controlplane/apiserver/config.go b/pkg/controlplane/apiserver/config.go
@@ -119,6 +119,7 @@ func BuildGenericConfig(
 	lastErr error,
 ) {
 	genericConfig = genericapiserver.NewConfig(legacyscheme.Codecs)
+	genericConfig.Flagz = s.Flagz
 	genericConfig.MergedResourceConfig = resourceConfig
 
 	if lastErr = s.GenericServerRunOptions.ApplyTo(genericConfig); lastErr != nil {
diff --git a/pkg/controlplane/apiserver/config_test.go b/pkg/controlplane/apiserver/config_test.go
@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
+	cliflag "k8s.io/component-base/cli/flag"
 	aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/controlplane/apiserver/options"
@@ -46,7 +47,7 @@ func TestBuildGenericConfig(t *testing.T) {
 	s.BindPort = ln.Addr().(*net.TCPAddr).Port
 	opts.SecureServing = s
 
-	completedOptions, err := opts.Complete(context.TODO(), nil, nil)
+	completedOptions, err := opts.Complete(context.TODO(), cliflag.NamedFlagSets{}, nil, nil)
 	if err != nil {
 		t.Fatalf("Failed to complete apiserver options: %v", err)
 	}
diff --git a/pkg/controlplane/apiserver/options/options.go b/pkg/controlplane/apiserver/options/options.go
@@ -34,6 +34,7 @@ import (
 	"k8s.io/component-base/logs"
 	logsapi "k8s.io/component-base/logs/api/v1"
 	"k8s.io/component-base/metrics"
+	"k8s.io/component-base/zpages/flagz"
 	"k8s.io/klog/v2"
 	netutil "k8s.io/utils/net"
 
@@ -47,6 +48,7 @@ import (
 // Options define the flags and validation for a generic controlplane. If the
 // structs are nil, the options are not added to the command line and not validated.
 type Options struct {
+	Flagz                   flagz.Reader
 	GenericServerRunOptions *genericoptions.ServerRunOptions
 	Etcd                    *genericoptions.EtcdOptions
 	SecureServing           *genericoptions.SecureServingOptionsWithLoopback
@@ -201,7 +203,7 @@ func (s *Options) AddFlags(fss *cliflag.NamedFlagSets) {
 		"Path to socket where a external JWT signer is listening. This flag is mutually exclusive with --service-account-signing-key-file and --service-account-key-file. Requires enabling feature gate (ExternalServiceAccountTokenSigner)")
 }
 
-func (o *Options) Complete(ctx context.Context, alternateDNS []string, alternateIPs []net.IP) (CompletedOptions, error) {
+func (o *Options) Complete(ctx context.Context, fss cliflag.NamedFlagSets, alternateDNS []string, alternateIPs []net.IP) (CompletedOptions, error) {
 	if o == nil {
 		return CompletedOptions{completedOptions: &completedOptions{}}, nil
 	}
@@ -257,6 +259,8 @@ func (o *Options) Complete(ctx context.Context, alternateDNS []string, alternate
 		}
 	}
 
+	completed.Flagz = flagz.NamedFlagSetsReader{FlagSets: fss}
+
 	return CompletedOptions{
 		completedOptions: &completed,
 	}, nil
diff --git a/pkg/controlplane/apiserver/samples/generic/server/server.go b/pkg/controlplane/apiserver/samples/generic/server/server.go
@@ -24,6 +24,7 @@ import (
 	"path/filepath"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -85,7 +86,7 @@ APIs.`,
 
 			ctx := genericapiserver.SetupSignalContext()
 
-			completedOptions, err := s.Complete(ctx, []string{}, []net.IP{})
+			completedOptions, err := s.Complete(ctx, cliflag.NamedFlagSets{FlagSets: map[string]*pflag.FlagSet{"sample_generic_controlplane": fs}}, []string{}, []net.IP{})
 			if err != nil {
 				return err
 			}
diff --git a/pkg/controlplane/apiserver/samples/generic/server/testing/testserver.go b/pkg/controlplane/apiserver/samples/generic/server/testing/testserver.go
@@ -164,7 +164,7 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions,
 	o.Authentication.ServiceAccounts.Issuers = []string{"https://foo.bar.example.com"}
 	o.Authentication.ServiceAccounts.KeyFiles = []string{saSigningKeyFile.Name()}
 
-	completedOptions, err := o.Complete(tCtx, nil, nil)
+	completedOptions, err := o.Complete(tCtx, fss, nil, nil)
 	if err != nil {
 		return result, fmt.Errorf("failed to set default ServerRunOptions: %w", err)
 	}
diff --git a/pkg/controlplane/apiserver/server.go b/pkg/controlplane/apiserver/server.go
@@ -37,6 +37,7 @@ import (
 	clientgoinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	zpagesfeatures "k8s.io/component-base/zpages/features"
+	"k8s.io/component-base/zpages/flagz"
 	"k8s.io/component-base/zpages/statusz"
 	"k8s.io/component-helpers/apimachinery/lease"
 	"k8s.io/klog/v2"
@@ -153,6 +154,12 @@ func (c completedConfig) New(name string, delegationTarget genericapiserver.Dele
 		return nil, fmt.Errorf("failed to get listener address: %w", err)
 	}
 
+	if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) {
+		if c.Generic.Flagz != nil {
+			flagz.Install(s.GenericAPIServer.Handler.NonGoRestfulMux, name, c.Generic.Flagz)
+		}
+	}
+
 	if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentStatusz) {
 		statusz.Install(s.GenericAPIServer.Handler.NonGoRestfulMux, name, statusz.NewRegistry())
 	}
diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go
@@ -803,6 +803,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 		{Version: version.MustParse("1.26"), Default: true, PreRelease: featuregate.Alpha},
 	},
 
+	zpagesfeatures.ComponentFlagz: {
+		{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
+	},
+
 	zpagesfeatures.ComponentStatusz: {
 		{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
 	},
diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go
@@ -203,6 +203,10 @@ func ClusterRoles() []rbacv1.ClusterRole {
 		).RuleOrDie(),
 	}
 
+	if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) {
+		monitoringRules = append(monitoringRules, rbacv1helpers.NewRule("get").URLs("/flagz").RuleOrDie())
+	}
+
 	if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentStatusz) {
 		monitoringRules = append(monitoringRules, rbacv1helpers.NewRule("get").URLs("/statusz").RuleOrDie())
 	}
diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go
@@ -79,6 +79,7 @@ import (
 	"k8s.io/component-base/metrics/prometheus/slis"
 	"k8s.io/component-base/tracing"
 	utilversion "k8s.io/component-base/version"
+	"k8s.io/component-base/zpages/flagz"
 	"k8s.io/klog/v2"
 	openapicommon "k8s.io/kube-openapi/pkg/common"
 	"k8s.io/kube-openapi/pkg/spec3"
@@ -189,6 +190,7 @@ type Config struct {
 	LivezChecks []healthz.HealthChecker
 	// The default set of readyz-only checks. There might be more added via AddReadyzChecks dynamically.
 	ReadyzChecks []healthz.HealthChecker
+	Flagz        flagz.Reader
 	// LegacyAPIGroupPrefixes is used to set up URL parsing for authorization and for validating requests
 	// to InstallLegacyAPIGroup. New API servers don't generally have legacy groups at all.
 	LegacyAPIGroupPrefixes sets.String
diff --git a/staging/src/k8s.io/component-base/zpages/features/kube_features.go b/staging/src/k8s.io/component-base/zpages/features/kube_features.go
@@ -22,6 +22,10 @@ import (
 )
 
 const (
+	// owner: @richabanker
+	// kep: https://kep.k8s.io/4828
+	ComponentFlagz featuregate.Feature = "ComponentFlagz"
+
 	// owner: @richabanker
 	// kep: https://kep.k8s.io/4827
 	// alpha: v1.32
@@ -33,6 +37,9 @@ const (
 
 func featureGates() map[featuregate.Feature]featuregate.VersionedSpecs {
 	return map[featuregate.Feature]featuregate.VersionedSpecs{
+		ComponentFlagz: {
+			{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
+		},
 		ComponentStatusz: {
 			{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
 		},
diff --git a/staging/src/k8s.io/component-base/zpages/flagz/flagreader.go b/staging/src/k8s.io/component-base/zpages/flagz/flagreader.go
@@ -0,0 +1,52 @@
+/*
+Copyright 2024 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 flagz
+
+import (
+	"github.com/spf13/pflag"
+	cliflag "k8s.io/component-base/cli/flag"
+)
+
+type Reader interface {
+	GetFlagz() map[string]string
+}
+
+// NamedFlagSetsGetter implements Reader for cliflag.NamedFlagSets
+type NamedFlagSetsReader struct {
+	FlagSets cliflag.NamedFlagSets
+}
+
+func (n NamedFlagSetsReader) GetFlagz() map[string]string {
+	return convertNamedFlagSetToFlags(&n.FlagSets)
+}
+
+func convertNamedFlagSetToFlags(flagSets *cliflag.NamedFlagSets) map[string]string {
+	flags := make(map[string]string)
+	for _, fs := range flagSets.FlagSets {
+		fs.VisitAll(func(flag *pflag.Flag) {
+			if flag.Value != nil {
+				value := flag.Value.String()
+				if set, ok := flag.Annotations["classified"]; ok && len(set) > 0 {
+					value = "CLASSIFIED"
+				}
+				flags[flag.Name] = value
+			}
+		})
+	}
+
+	return flags
+}
diff --git a/staging/src/k8s.io/component-base/zpages/flagz/flagreader_test.go b/staging/src/k8s.io/component-base/zpages/flagz/flagreader_test.go
@@ -0,0 +1,95 @@
+/*
+Copyright 2024 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 flagz
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/spf13/pflag"
+
+	"k8s.io/component-base/cli/flag"
+)
+
+func TestConvertNamedFlagSetToFlags(t *testing.T) {
+	tests := []struct {
+		name     string
+		flagSets *flag.NamedFlagSets
+		want     map[string]string
+	}{
+		{
+			name: "basic flags",
+			flagSets: &flag.NamedFlagSets{
+				FlagSets: map[string]*pflag.FlagSet{
+					"test": flagSet(t, map[string]flagValue{
+						"flag1": {value: "value1", sensitive: false},
+						"flag2": {value: "value2", sensitive: false},
+					}),
+				},
+			},
+			want: map[string]string{
+				"flag1": "value1",
+				"flag2": "value2",
+			},
+		},
+		{
+			name: "classified flags",
+			flagSets: &flag.NamedFlagSets{
+				FlagSets: map[string]*pflag.FlagSet{
+					"test": flagSet(t, map[string]flagValue{
+						"secret1": {value: "value1", sensitive: true},
+						"flag2":   {value: "value2", sensitive: false},
+					}),
+				},
+			},
+			want: map[string]string{
+				"flag2":   "value2",
+				"secret1": "CLASSIFIED",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := convertNamedFlagSetToFlags(tt.flagSets)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ConvertNamedFlagSetToFlags() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+type flagValue struct {
+	value     string
+	sensitive bool
+}
+
+func flagSet(t *testing.T, flags map[string]flagValue) *pflag.FlagSet {
+	fs := pflag.NewFlagSet("test-set", pflag.ContinueOnError)
+	for flagName, flagVal := range flags {
+		flagValue := ""
+		fs.StringVar(&flagValue, flagName, flagVal.value, "test-usage")
+		if flagVal.sensitive {
+			err := fs.SetAnnotation(flagName, "classified", []string{"true"})
+			if err != nil {
+				t.Fatalf("unexpected error when setting flag annotation: %v", err)
+			}
+		}
+	}
+
+	return fs
+}
diff --git a/staging/src/k8s.io/component-base/zpages/flagz/flagz.go b/staging/src/k8s.io/component-base/zpages/flagz/flagz.go
diff --git a/staging/src/k8s.io/component-base/zpages/flagz/flagz_test.go b/staging/src/k8s.io/component-base/zpages/flagz/flagz_test.go
diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml

Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,7 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions,`
`164`	`164`	`o.Authentication.ServiceAccounts.Issuers = []string{"https://foo.bar.example.com"}`
`165`	`165`	`o.Authentication.ServiceAccounts.KeyFiles = []string{saSigningKeyFile.Name()}`
`166`	`166`
`167`		`- completedOptions, err := o.Complete(tCtx, nil, nil)`
	`167`	`+ completedOptions, err := o.Complete(tCtx, fss, nil, nil)`
`168`	`168`	`if err != nil {`
`169`	`169`	`return result, fmt.Errorf("failed to set default ServerRunOptions: %w", err)`
`170`	`170`	`}`
Original file line number	Diff line number	Diff line change
`@@ -203,6 +203,10 @@ func ClusterRoles() []rbacv1.ClusterRole {`
`203`	`203`	`).RuleOrDie(),`
`204`	`204`	`}`
`205`	`205`
	`206`	`+ if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) {`
	`207`	`+ monitoringRules = append(monitoringRules, rbacv1helpers.NewRule("get").URLs("/flagz").RuleOrDie())`
	`208`	`+ }`
	`209`	`+`
`206`	`210`	`if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentStatusz) {`
`207`	`211`	`monitoringRules = append(monitoringRules, rbacv1helpers.NewRule("get").URLs("/statusz").RuleOrDie())`
`208`	`212`	`}`