fix(controller): handle long rule names in bootstrap annotation keys

internal/controller/helper.go

@@ -17,12 +17,48 @@ limitations under the License.
 package controller
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const (
+	// bootstrapAnnotationPrefix is the prefix used for node annotations tracking bootstrap completion.
+	bootstrapAnnotationPrefix = "readiness.k8s.io/bootstrap-completed-"
+)
+
+// getBootstrapAnnotationKey generates a valid Kubernetes annotation key for a given rule name.
+// It handles rule names longer than 43 characters by using a hybrid approach:
+// it truncates the name and appends a short hash. This keeps the key human-readable
+// while staying within the 63-character limit for the name part.
+func getBootstrapAnnotationKey(ruleName string) string {
+	const maxNamePartLen = 63
+	const namePartPrefix = "bootstrap-completed-"
+	// availableSpace is the space left for the ruleName in the annotation name part (after "bootstrap-completed-")
+	availableSpace := maxNamePartLen - len(namePartPrefix) // 43
+
+	if len(ruleName) <= availableSpace {
+		return fmt.Sprintf("%s%s", bootstrapAnnotationPrefix, ruleName)
+	}
+
+	// For names longer than 43 chars, use a hybrid approach: truncated-name-shorthash
+	const hashLen = 8
+	// -1 for the hyphen separator
+	humanPartLen := availableSpace - hashLen - 1 // 34
+
+	hash := sha256.Sum256([]byte(ruleName))
+	shortHash := hex.EncodeToString(hash[:])[:hashLen]
+
+	// Use first 34 chars of ruleName, then a hyphen, then 8 chars of hash.
+	// Total name part: 34 + 1 + 8 = 43.
+	// This fits well within the 63 char limit and leaves room for the prefix.
+	namePart := fmt.Sprintf("%s-%s", ruleName[:humanPartLen], shortHash)
+	return fmt.Sprintf("%s%s", bootstrapAnnotationPrefix, namePart)
+}
+
 // nodeSelectorChanged checks if nodeSelector has changed.
 func nodeSelectorChanged(current, previous metav1.LabelSelector) bool {
 	// Compare matchLabels

internal/controller/helper_unit_test.go

@@ -0,0 +1,78 @@
+/*
+Copyright 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 controller
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func TestGetBootstrapAnnotationKey(t *testing.T) {
+	g := NewWithT(t)
+
+	tests := []struct {
+		name     string
+		ruleName string
+		expected string
+	}{
+		{
+			name:     "Short name",
+			ruleName: "short-rule",
+			expected: "readiness.k8s.io/bootstrap-completed-short-rule",
+		},
+		{
+			name:     "Exactly 43 characters",
+			ruleName: "this-rule-name-is-exactly-43-chars-long-123",
+			expected: "readiness.k8s.io/bootstrap-completed-this-rule-name-is-exactly-43-chars-long-123",
+		},
+		{
+			name:     "Long name (44 characters)",
+			ruleName: "this-rule-name-is-exactly-44-chars-long-1234",
+		},
+		{
+			name:     "Very long name",
+			ruleName: "my-very-long-rule-name-that-is-definitely-going-to-exceed-the-limit-of-sixty-three-characters-in-the-annotation-key-name-part",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := getBootstrapAnnotationKey(tt.ruleName)
+
+			// Verify prefix
+			g.Expect(result).To(HavePrefix(bootstrapAnnotationPrefix))
+
+			if len(tt.ruleName) <= 43 {
+				g.Expect(result).To(Equal(bootstrapAnnotationPrefix + tt.ruleName))
+			} else {
+				// Verify name part length is exactly 20 (prefix) + 34 (human) + 1 (hyphen) + 8 (hash) = 63
+				// Total key length: 17 (domain) + 63 (name part) = 80
+				g.Expect(len(result)).To(Equal(80))
+				// Verify it has the hyphen separator before the hash (last 9 characters are -XXXXXXXX)
+				g.Expect(result[len(result)-9 : len(result)-8]).To(Equal("-"))
+			}
+
+			// Verify name part length is <= 63
+			namePart := result[len("readiness.k8s.io/"):]
+			g.Expect(len(namePart)).To(BeNumerically("<=", 63))
+
+			// Verify determinism
+			g.Expect(getBootstrapAnnotationKey(tt.ruleName)).To(Equal(result))
+		})
+	}
+}

internal/controller/node_controller.go

@@ -316,23 +316,22 @@ func (r *RuleReadinessController) removeTaintBySpec(ctx context.Context, node *c
 	})
 }
 
-// Bootstrap completion tracking.
 func (r *RuleReadinessController) isBootstrapCompleted(ctx context.Context, nodeName, ruleName string) bool {
 	// Check node annotation
 	node := &corev1.Node{}
 	if err := r.Get(ctx, client.ObjectKey{Name: nodeName}, node); err != nil {
 		return false
 	}
 
-	annotationKey := fmt.Sprintf("readiness.k8s.io/bootstrap-completed-%s", ruleName)
+	annotationKey := getBootstrapAnnotationKey(ruleName)
 	_, exists := node.Annotations[annotationKey]
 	return exists
 }
 
 func (r *RuleReadinessController) markBootstrapCompleted(ctx context.Context, nodeName, ruleName string) {
 	log := ctrl.LoggerFrom(ctx)
 
-	annotationKey := fmt.Sprintf("readiness.k8s.io/bootstrap-completed-%s", ruleName)
+	annotationKey := getBootstrapAnnotationKey(ruleName)
 	marked := false
 
 	// retry to handle conflict with concurrent node updates

internal/controller/node_controller_reproduction_test.go

@@ -0,0 +1,131 @@
+/*
+Copyright 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 controller
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/tools/record"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	nodereadinessiov1alpha1 "sigs.k8s.io/node-readiness-controller/api/v1alpha1"
+)
+
+var _ = Describe("Node Controller Reproduction", func() {
+	Context("when reconciling a node with a very long rule name", func() {
+		var (
+			ctx                 context.Context
+			readinessController *RuleReadinessController
+			nodeReconciler      *NodeReconciler
+			fakeClientset       *fake.Clientset
+			node                *corev1.Node
+			rule                *nodereadinessiov1alpha1.NodeReadinessRule
+			longRuleName        = "my-very-long-rule-name-that-exceeds-the-annotation-key-limit-strictly"
+		)
+
+		BeforeEach(func() {
+			ctx = context.Background()
+
+			fakeClientset = fake.NewSimpleClientset()
+			readinessController = &RuleReadinessController{
+				Client:        k8sClient,
+				Scheme:        k8sClient.Scheme(),
+				clientset:     fakeClientset,
+				ruleCache:     make(map[string]*nodereadinessiov1alpha1.NodeReadinessRule),
+				EventRecorder: record.NewFakeRecorder(10),
+			}
+
+			nodeReconciler = &NodeReconciler{
+				Client:     k8sClient,
+				Scheme:     k8sClient.Scheme(),
+				Controller: readinessController,
+			}
+
+			node = &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "repro-node",
+					Labels: map[string]string{"env": "repro"},
+				},
+				Status: corev1.NodeStatus{
+					Conditions: []corev1.NodeCondition{
+						{Type: "ReproCondition", Status: corev1.ConditionTrue},
+					},
+				},
+			}
+
+			rule = &nodereadinessiov1alpha1.NodeReadinessRule{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: longRuleName,
+				},
+				Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
+					Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
+						{Type: "ReproCondition", RequiredStatus: corev1.ConditionTrue},
+					},
+					Taint: corev1.Taint{
+						Key:    "readiness.k8s.io/repro-taint",
+						Effect: corev1.TaintEffectNoSchedule,
+					},
+					NodeSelector: metav1.LabelSelector{
+						MatchLabels: map[string]string{"env": "repro"},
+					},
+					EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
+				},
+			}
+		})
+
+		JustBeforeEach(func() {
+			Expect(k8sClient.Create(ctx, node)).To(Succeed())
+			Expect(k8sClient.Create(ctx, rule)).To(Succeed())
+			readinessController.updateRuleCache(ctx, rule)
+		})
+
+		AfterEach(func() {
+			_ = k8sClient.Delete(ctx, node)
+			updatedRule := &nodereadinessiov1alpha1.NodeReadinessRule{}
+			if err := k8sClient.Get(ctx, types.NamespacedName{Name: longRuleName}, updatedRule); err == nil {
+				updatedRule.Finalizers = nil
+				_ = k8sClient.Update(ctx, updatedRule)
+				_ = k8sClient.Delete(ctx, updatedRule)
+			}
+			readinessController.removeRuleFromCache(ctx, longRuleName)
+		})
+
+		It("should successfully mark bootstrap completed by using a hashed annotation key for long rule names", func() {
+			// Trigger reconciliation
+			_, err := nodeReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "repro-node"}})
+			Expect(err).NotTo(HaveOccurred())
+
+			recheckedNode := &corev1.Node{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "repro-node"}, recheckedNode)).To(Succeed())
+
+			// The key should now be the hashed version: readiness.k8s.io/bootstrap-completed-<md5-hash>
+			expectedKey := getBootstrapAnnotationKey(longRuleName)
+			Expect(recheckedNode.Annotations).To(HaveKey(expectedKey),
+				"Annotation SHOULD be present with the hashed key")
+
+			// Verify the length of the name part is exactly 63
+			// Total length: 17 (domain) + 63 (name part) = 80
+			Expect(len(expectedKey)).To(Equal(17 + 63))
+		})
+	})
+})