OCPBUGS-86798: Bug: Poll for CRD Established status after creation to fix TOCTOU race

lib/resourcebuilder/apiext.go

@@ -3,15 +3,22 @@ package resourcebuilder
 import (
 	"context"
 	"fmt"
+	"time"
 
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/klog/v2"
 )
 
-func (b *builder) checkCustomResourceDefinitionHealth(ctx context.Context, crd *apiextv1.CustomResourceDefinition) error {
-	if b.mode == InitializingMode {
-		return nil
-	}
+const (
+	crdEstablishedPollInterval = 100 * time.Millisecond
+	crdEstablishedPollTimeout  = 5 * time.Second
+)
 
+// checkCRDEstablished checks whether a CRD has Established=True in its status
+// conditions. Returns nil if established, or an error describing the current state.
+func checkCRDEstablished(crd *apiextv1.CustomResourceDefinition) error {
 	for _, condition := range crd.Status.Conditions {
 		if condition.Type == apiextv1.Established {
 			if condition.Status == apiextv1.ConditionTrue {
@@ -23,3 +30,36 @@ func (b *builder) checkCustomResourceDefinitionHealth(ctx context.Context, crd *
 
 	return fmt.Errorf("CustomResourceDefinition %s does not declare an Established status condition: %v", crd.Name, crd.Status.Conditions)
 }
+
+func (b *builder) checkCustomResourceDefinitionHealth(ctx context.Context, crd *apiextv1.CustomResourceDefinition) error {
+	if b.mode == InitializingMode {
+		return nil
+	}
+
+	if err := checkCRDEstablished(crd); err == nil {
+		return nil
+	}
+
+	// When status conditions are empty, the CRD was likely just created and the
+	// API server has not yet populated conditions. This is a known TOCTOU race:
+	// CVO creates the CRD and immediately checks Established, but the API server
+	// needs time to validate the schema and set conditions. Poll briefly to allow
+	// the API server to complete CRD registration before returning an error.
+	if len(crd.Status.Conditions) == 0 {
+		klog.V(4).Infof("CRD %s has empty status conditions, polling for Established", crd.Name)
+		pollErr := wait.PollUntilContextTimeout(ctx, crdEstablishedPollInterval, crdEstablishedPollTimeout, false, func(ctx context.Context) (bool, error) {
+			updated, err := b.apiextensionsClientv1.CustomResourceDefinitions().Get(ctx, crd.Name, metav1.GetOptions{})
+			if err != nil {
+				return false, err
+			}
+			crd = updated
+			return checkCRDEstablished(crd) == nil, nil
+		})
+		if pollErr == nil {
+			return nil
+		}
+		klog.V(2).Infof("CRD %s did not reach Established=True after polling for %s: %v", crd.Name, crdEstablishedPollTimeout, pollErr)
+	}
+
+	return checkCRDEstablished(crd)
+}

lib/resourcebuilder/apiext_test.go

@@ -0,0 +1,152 @@
+package resourcebuilder
+
+import (
+	"testing"
+
+	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestCheckCRDEstablished(t *testing.T) {
+	tests := []struct {
+		name      string
+		crd       *apiextv1.CustomResourceDefinition
+		wantErr   bool
+		errSubstr string
+	}{
+		{
+			name: "Established=True returns nil",
+			crd: &apiextv1.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "test.example.io"},
+				Status: apiextv1.CustomResourceDefinitionStatus{
+					Conditions: []apiextv1.CustomResourceDefinitionCondition{
+						{
+							Type:   apiextv1.NamesAccepted,
+							Status: apiextv1.ConditionTrue,
+						},
+						{
+							Type:   apiextv1.Established,
+							Status: apiextv1.ConditionTrue,
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Established=False returns error with reason",
+			crd: &apiextv1.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "test.example.io"},
+				Status: apiextv1.CustomResourceDefinitionStatus{
+					Conditions: []apiextv1.CustomResourceDefinitionCondition{
+						{
+							Type:    apiextv1.Established,
+							Status:  apiextv1.ConditionFalse,
+							Reason:  "Installing",
+							Message: "not yet established",
+						},
+					},
+				},
+			},
+			wantErr:   true,
+			errSubstr: "Established=False",
+		},
+		{
+			name: "empty conditions returns error",
+			crd: &apiextv1.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "test.example.io"},
+				Status: apiextv1.CustomResourceDefinitionStatus{
+					Conditions: []apiextv1.CustomResourceDefinitionCondition{},
+				},
+			},
+			wantErr:   true,
+			errSubstr: "does not declare an Established status condition",
+		},
+		{
+			name: "nil conditions returns error",
+			crd: &apiextv1.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "test.example.io"},
+				Status:     apiextv1.CustomResourceDefinitionStatus{},
+			},
+			wantErr:   true,
+			errSubstr: "does not declare an Established status condition",
+		},
+		{
+			name: "NamesAccepted but no Established returns error",
+			crd: &apiextv1.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "test.example.io"},
+				Status: apiextv1.CustomResourceDefinitionStatus{
+					Conditions: []apiextv1.CustomResourceDefinitionCondition{
+						{
+							Type:   apiextv1.NamesAccepted,
+							Status: apiextv1.ConditionTrue,
+						},
+					},
+				},
+			},
+			wantErr:   true,
+			errSubstr: "does not declare an Established status condition",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := checkCRDEstablished(tt.crd)
+			if tt.wantErr {
+				if err == nil {
+					t.Error("expected error, got nil")
+				} else if tt.errSubstr != "" {
+					if !containsSubstring(err.Error(), tt.errSubstr) {
+						t.Errorf("error %q does not contain %q", err.Error(), tt.errSubstr)
+					}
+				}
+			} else {
+				if err != nil {
+					t.Errorf("expected nil error, got: %v", err)
+				}
+			}
+		})
+	}
+}
+
+func TestCheckCustomResourceDefinitionHealthInitializingMode(t *testing.T) {
+	b := &builder{mode: InitializingMode}
+	crd := &apiextv1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{Name: "test.example.io"},
+		Status:     apiextv1.CustomResourceDefinitionStatus{},
+	}
+	if err := b.checkCustomResourceDefinitionHealth(t.Context(), crd); err != nil {
+		t.Errorf("InitializingMode should skip health check, got: %v", err)
+	}
+}
+
+func TestCheckCustomResourceDefinitionHealthEstablished(t *testing.T) {
+	b := &builder{mode: ReconcilingMode}
+	crd := &apiextv1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{Name: "test.example.io"},
+		Status: apiextv1.CustomResourceDefinitionStatus{
+			Conditions: []apiextv1.CustomResourceDefinitionCondition{
+				{
+					Type:   apiextv1.Established,
+					Status: apiextv1.ConditionTrue,
+				},
+			},
+		},
+	}
+	if err := b.checkCustomResourceDefinitionHealth(t.Context(), crd); err != nil {
+		t.Errorf("Established=True CRD should pass health check, got: %v", err)
+	}
+}
+
+func containsSubstring(s, substr string) bool {
+	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstringImpl(s, substr))
+}
+
+func containsSubstringImpl(s, substr string) bool {
+	for i := 0; i <= len(s)-len(substr); i++ {
+		if s[i:i+len(substr)] == substr {
+			return true
+		}
+	}
+	return false
+}