diff --git a/pkg/controller/storageversiongc/gc_controller_test.go b/pkg/controller/storageversiongc/gc_controller_test.go index 9e1a039706d..4f11b7e33a9 100644 --- a/pkg/controller/storageversiongc/gc_controller_test.go +++ b/pkg/controller/storageversiongc/gc_controller_test.go @@ -48,7 +48,7 @@ func newKubeApiserverLease(name, holderIdentity string) *coordinationv1.Lease { Name: name, Namespace: metav1.NamespaceSystem, Labels: map[string]string{ - "k8s.io/component": "kube-apiserver", + "apiserver.kubernetes.io/identity": "kube-apiserver", }, }, Spec: coordinationv1.LeaseSpec{ diff --git a/pkg/controlplane/controller/apiserverleasegc/gc_controller_test.go b/pkg/controlplane/controller/apiserverleasegc/gc_controller_test.go index 0e86e70ff9f..38e18c3181f 100644 --- a/pkg/controlplane/controller/apiserverleasegc/gc_controller_test.go +++ b/pkg/controlplane/controller/apiserverleasegc/gc_controller_test.go @@ -44,7 +44,7 @@ func Test_Controller(t *testing.T) { Name: "kube-apiserver-12345", Namespace: metav1.NamespaceSystem, Labels: map[string]string{ - "k8s.io/component": "kube-apiserver", + "apiserver.kubernetes.io/identity": "kube-apiserver", }, }, Spec: coordinationv1.LeaseSpec{ @@ -62,7 +62,7 @@ func Test_Controller(t *testing.T) { Name: "kube-apiserver-12345", Namespace: metav1.NamespaceSystem, Labels: map[string]string{ - "k8s.io/component": "kube-controller-manager", + "apiserver.kubernetes.io/identity": "kube-controller-manager", }, }, Spec: coordinationv1.LeaseSpec{ @@ -80,7 +80,7 @@ func Test_Controller(t *testing.T) { Name: "kube-apiserver-12345", Namespace: metav1.NamespaceSystem, Labels: map[string]string{ - "k8s.io/component": "kube-apiserver", + "apiserver.kubernetes.io/identity": "kube-apiserver", }, }, Spec: coordinationv1.LeaseSpec{ @@ -98,7 +98,7 @@ func Test_Controller(t *testing.T) { Name: "kube-apiserver-12345", Namespace: metav1.NamespaceSystem, Labels: map[string]string{ - "k8s.io/component": "kube-apiserver", + "apiserver.kubernetes.io/identity": "kube-apiserver", }, }, Spec: coordinationv1.LeaseSpec{ @@ -116,7 +116,7 @@ func Test_Controller(t *testing.T) { Name: "kube-apiserver-12345", Namespace: metav1.NamespaceSystem, Labels: map[string]string{ - "k8s.io/component": "kube-apiserver", + "apiserver.kubernetes.io/identity": "kube-apiserver", }, }, Spec: coordinationv1.LeaseSpec{ @@ -132,7 +132,7 @@ func Test_Controller(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { clientset := fake.NewSimpleClientset(test.lease) - controller := NewAPIServerLeaseGC(clientset, 100*time.Millisecond, metav1.NamespaceSystem, "k8s.io/component=kube-apiserver") + controller := NewAPIServerLeaseGC(clientset, 100*time.Millisecond, metav1.NamespaceSystem, "apiserver.kubernetes.io/identity=kube-apiserver") go controller.Run(nil) time.Sleep(time.Second) diff --git a/pkg/controlplane/instance.go b/pkg/controlplane/instance.go index db60aae7db0..b3c87c07bdc 100644 --- a/pkg/controlplane/instance.go +++ b/pkg/controlplane/instance.go @@ -123,9 +123,11 @@ const ( // IdentityLeaseComponentLabelKey is used to apply a component label to identity lease objects, indicating: // 1. the lease is an identity lease (different from leader election leases) // 2. which component owns this lease - IdentityLeaseComponentLabelKey = "k8s.io/component" + IdentityLeaseComponentLabelKey = "apiserver.kubernetes.io/identity" // KubeAPIServer defines variable used internally when referring to kube-apiserver component KubeAPIServer = "kube-apiserver" + // DeprecatedKubeAPIServerIdentityLeaseLabelSelector selects kube-apiserver identity leases + DeprecatedKubeAPIServerIdentityLeaseLabelSelector = "k8s.io/component=kube-apiserver" // KubeAPIServerIdentityLeaseLabelSelector selects kube-apiserver identity leases KubeAPIServerIdentityLeaseLabelSelector = IdentityLeaseComponentLabelKey + "=" + KubeAPIServer // repairLoopInterval defines the interval used to run the Services ClusterIP and NodePort repair loops @@ -505,10 +507,28 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) IdentityLeaseRenewIntervalPeriod, leaseName, metav1.NamespaceSystem, - labelAPIServerHeartbeat) + // TODO: receive identity label value as a parameter when post start hook is moved to generic apiserver. + labelAPIServerHeartbeatFunc(KubeAPIServer)) go controller.Run(hookContext.StopCh) return nil }) + // Labels for apiserver idenitiy leases switched from k8s.io/component=kube-apiserver to apiserver.kubernetes.io/identity=kube-apiserver. + // For compatibility, garbage collect leases with both labels for at least 1 release + // TODO: remove in Kubernetes 1.28 + m.GenericAPIServer.AddPostStartHookOrDie("start-deprecated-kube-apiserver-identity-lease-garbage-collector", func(hookContext genericapiserver.PostStartHookContext) error { + kubeClient, err := kubernetes.NewForConfig(hookContext.LoopbackClientConfig) + if err != nil { + return err + } + go apiserverleasegc.NewAPIServerLeaseGC( + kubeClient, + IdentityLeaseGCPeriod, + metav1.NamespaceSystem, + DeprecatedKubeAPIServerIdentityLeaseLabelSelector, + ).Run(hookContext.StopCh) + return nil + }) + // TODO: move this into generic apiserver and make the lease identity value configurable m.GenericAPIServer.AddPostStartHookOrDie("start-kube-apiserver-identity-lease-garbage-collector", func(hookContext genericapiserver.PostStartHookContext) error { kubeClient, err := kubernetes.NewForConfig(hookContext.LoopbackClientConfig) if err != nil { @@ -536,21 +556,24 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) return m, nil } -func labelAPIServerHeartbeat(lease *coordinationapiv1.Lease) error { - if lease.Labels == nil { - lease.Labels = map[string]string{} - } - // This label indicates that kube-apiserver owns this identity lease object - lease.Labels[IdentityLeaseComponentLabelKey] = KubeAPIServer +func labelAPIServerHeartbeatFunc(identity string) lease.ProcessLeaseFunc { + return func(lease *coordinationapiv1.Lease) error { + if lease.Labels == nil { + lease.Labels = map[string]string{} + } - hostname, err := os.Hostname() - if err != nil { - return err - } + // This label indiciates the identity of the lease object. + lease.Labels[IdentityLeaseComponentLabelKey] = identity - // convenience label to easily map a lease object to a specific apiserver - lease.Labels[apiv1.LabelHostname] = hostname - return nil + hostname, err := os.Hostname() + if err != nil { + return err + } + + // convenience label to easily map a lease object to a specific apiserver + lease.Labels[apiv1.LabelHostname] = hostname + return nil + } } // InstallLegacyAPI will install the legacy APIs for the restStorageProviders if they are enabled. diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 8c5da2db524..86f8eca6d8e 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -34,6 +34,7 @@ import ( jsonpatch "github.com/evanphx/json-patch" "github.com/google/uuid" + "golang.org/x/crypto/cryptobyte" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -345,8 +346,24 @@ func NewConfig(codecs serializer.CodecFactory) *Config { klog.Fatalf("error getting hostname for apiserver identity: %v", err) } - hash := sha256.Sum256([]byte(hostname)) - id = "kube-apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16])) + // Since the hash needs to be unique across each kube-apiserver and aggregated apiservers, + // the hash used for the identity should include both the hostname and the identity value. + // TODO: receive the identity value as a parameter once the apiserver identity lease controller + // post start hook is moved to generic apiserver. + b := cryptobyte.NewBuilder(nil) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte(hostname)) + }) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte("kube-apiserver")) + }) + hashData, err := b.Bytes() + if err != nil { + klog.Fatalf("error building hash data for apiserver identity: %v", err) + } + + hash := sha256.Sum256(hashData) + id = "apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16])) } lifecycleSignals := newLifecycleSignals() diff --git a/staging/src/k8s.io/controller-manager/go.mod b/staging/src/k8s.io/controller-manager/go.mod index 8f11faaffe5..6d9e02baf5b 100644 --- a/staging/src/k8s.io/controller-manager/go.mod +++ b/staging/src/k8s.io/controller-manager/go.mod @@ -79,6 +79,7 @@ require ( go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.19.0 // indirect + golang.org/x/crypto v0.1.0 // indirect golang.org/x/net v0.4.0 // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/sys v0.3.0 // indirect diff --git a/staging/src/k8s.io/controller-manager/go.sum b/staging/src/k8s.io/controller-manager/go.sum index f05ca808758..83b538d889e 100644 --- a/staging/src/k8s.io/controller-manager/go.sum +++ b/staging/src/k8s.io/controller-manager/go.sum @@ -381,6 +381,7 @@ golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= +golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= diff --git a/test/e2e/apimachinery/apiserver_identity.go b/test/e2e/apimachinery/apiserver_identity.go index 72db85329d7..c641b4a8b86 100644 --- a/test/e2e/apimachinery/apiserver_identity.go +++ b/test/e2e/apimachinery/apiserver_identity.go @@ -27,6 +27,8 @@ import ( "time" "github.com/onsi/ginkgo/v2" + "golang.org/x/crypto/cryptobyte" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" @@ -115,7 +117,7 @@ var _ = SIGDescribe("kube-apiserver identity [Feature:APIServerIdentity]", func( } leases, err := client.CoordinationV1().Leases(metav1.NamespaceSystem).List(context.TODO(), metav1.ListOptions{ - LabelSelector: "k8s.io/component=kube-apiserver", + LabelSelector: "apiserver.kubernetes.io/identity=kube-apiserver", }) framework.ExpectNoError(err) framework.ExpectEqual(len(leases.Items), len(controlPlaneNodes), "unexpected number of leases") @@ -124,8 +126,18 @@ var _ = SIGDescribe("kube-apiserver identity [Feature:APIServerIdentity]", func( hostname, err := getControlPlaneHostname(ctx, &node) framework.ExpectNoError(err) - hash := sha256.Sum256([]byte(hostname)) - leaseName := "kube-apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16])) + b := cryptobyte.NewBuilder(nil) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte(hostname)) + }) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte("kube-apiserver")) + }) + + hashData, err := b.Bytes() + framework.ExpectNoError(err) + hash := sha256.Sum256(hashData) + leaseName := "apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16])) lease, err := client.CoordinationV1().Leases(metav1.NamespaceSystem).Get(context.TODO(), leaseName, metav1.GetOptions{}) framework.ExpectNoError(err) @@ -161,7 +173,7 @@ var _ = SIGDescribe("kube-apiserver identity [Feature:APIServerIdentity]", func( // As long as the hostname of kube-apiserver is unchanged, a restart should not result in new Lease objects. // Check that the number of lease objects remains the same after restarting kube-apiserver. leases, err = client.CoordinationV1().Leases(metav1.NamespaceSystem).List(context.TODO(), metav1.ListOptions{ - LabelSelector: "k8s.io/component=kube-apiserver", + LabelSelector: "apiserver.kubernetes.io/identity=kube-apiserver", }) framework.ExpectNoError(err) framework.ExpectEqual(len(leases.Items), len(controlPlaneNodes), "unexpected number of leases") diff --git a/test/integration/controlplane/apiserver_identity_test.go b/test/integration/controlplane/apiserver_identity_test.go index 6c735035913..981a979a655 100644 --- a/test/integration/controlplane/apiserver_identity_test.go +++ b/test/integration/controlplane/apiserver_identity_test.go @@ -26,6 +26,8 @@ import ( "testing" "time" + "golang.org/x/crypto/cryptobyte" + coordinationv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -45,9 +47,21 @@ const ( testLeaseName = "apiserver-lease-test" ) -func expectedAPIServerIdentity(hostname string) string { - hash := sha256.Sum256([]byte(hostname)) - return "kube-apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16])) +func expectedAPIServerIdentity(t *testing.T, hostname string) string { + b := cryptobyte.NewBuilder(nil) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte(hostname)) + }) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte("kube-apiserver")) + }) + hashData, err := b.Bytes() + if err != nil { + t.Fatalf("error building hash data for apiserver identity: %v", err) + } + + hash := sha256.Sum256(hashData) + return "apiserver-" + strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:16])) } func TestCreateLeaseOnStart(t *testing.T) { @@ -84,8 +98,8 @@ func TestCreateLeaseOnStart(t *testing.T) { } lease := leases.Items[0] - if lease.Name != expectedAPIServerIdentity(hostname) { - return false, fmt.Errorf("unexpected apiserver identity, got: %v, expected: %v", lease.Name, expectedAPIServerIdentity(hostname)) + if lease.Name != expectedAPIServerIdentity(t, hostname) { + return false, fmt.Errorf("unexpected apiserver identity, got: %v, expected: %v", lease.Name, expectedAPIServerIdentity(t, hostname)) } if lease.Labels[corev1.LabelHostname] != hostname { @@ -134,12 +148,54 @@ func TestLeaseGarbageCollection(t *testing.T) { t.Run("expired non-identity lease should not be garbage collected", testLeaseNotGarbageCollected(t, kubeclient, expiredLease)) - // identity leases (with k8s.io/component label) created in user namespaces should not be GC'ed + // identity leases (with apiserver.kubernetes.io/identity label) created in user namespaces should not be GC'ed expiredNonKubeSystemLease := newTestLease(time.Now().Add(-2*time.Hour), metav1.NamespaceDefault) t.Run("expired non-system identity lease should not be garbage collected", testLeaseNotGarbageCollected(t, kubeclient, expiredNonKubeSystemLease)) } +func TestLeaseGarbageCollectionWithDeprecatedLabels(t *testing.T) { + oldIdentityLeaseDurationSeconds := controlplane.IdentityLeaseDurationSeconds + oldIdentityLeaseGCPeriod := controlplane.IdentityLeaseGCPeriod + oldIdentityLeaseRenewIntervalPeriod := controlplane.IdentityLeaseRenewIntervalPeriod + defer func() { + // reset the default values for leases after this test + controlplane.IdentityLeaseDurationSeconds = oldIdentityLeaseDurationSeconds + controlplane.IdentityLeaseGCPeriod = oldIdentityLeaseGCPeriod + controlplane.IdentityLeaseRenewIntervalPeriod = oldIdentityLeaseRenewIntervalPeriod + }() + + // Shorten lease parameters so GC behavior can be exercised in integration tests + controlplane.IdentityLeaseDurationSeconds = 1 + controlplane.IdentityLeaseGCPeriod = time.Second + controlplane.IdentityLeaseRenewIntervalPeriod = time.Second + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIServerIdentity, true)() + result := kubeapiservertesting.StartTestServerOrDie(t, nil, nil, framework.SharedEtcd()) + defer result.TearDownFn() + + kubeclient, err := kubernetes.NewForConfig(result.ClientConfig) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expiredLease := newTestLeaseWithDeprecatedLabels(time.Now().Add(-2*time.Hour), metav1.NamespaceSystem) + t.Run("expired apiserver lease should be garbage collected", + testLeaseGarbageCollected(t, kubeclient, expiredLease)) + + freshLease := newTestLeaseWithDeprecatedLabels(time.Now().Add(-2*time.Minute), metav1.NamespaceSystem) + t.Run("fresh apiserver lease should not be garbage collected", + testLeaseNotGarbageCollected(t, kubeclient, freshLease)) + + expiredLease.Labels = nil + t.Run("expired non-identity lease should not be garbage collected", + testLeaseNotGarbageCollected(t, kubeclient, expiredLease)) + + // identity leases (with k8s.io/component label) created in user namespaces should not be GC'ed + expiredNonKubeSystemLease := newTestLeaseWithDeprecatedLabels(time.Now().Add(-2*time.Hour), metav1.NamespaceDefault) + t.Run("expired non-system identity lease should not be garbage collected", + testLeaseNotGarbageCollected(t, kubeclient, expiredNonKubeSystemLease)) +} + func testLeaseGarbageCollected(t *testing.T, client kubernetes.Interface, lease *coordinationv1.Lease) func(t *testing.T) { return func(t *testing.T) { ns := lease.Namespace @@ -203,3 +259,21 @@ func newTestLease(acquireTime time.Time, namespace string) *coordinationv1.Lease }, } } + +func newTestLeaseWithDeprecatedLabels(acquireTime time.Time, namespace string) *coordinationv1.Lease { + return &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: testLeaseName, + Namespace: namespace, + Labels: map[string]string{ + "k8s.io/component": "kube-apiserver", + }, + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: pointer.StringPtr(testLeaseName), + LeaseDurationSeconds: pointer.Int32Ptr(3600), + AcquireTime: &metav1.MicroTime{Time: acquireTime}, + RenewTime: &metav1.MicroTime{Time: acquireTime}, + }, + } +}