diff --git a/lib/resourcebuilder/apiext.go b/lib/resourcebuilder/apiext.go index cef543d15..45d1ab20b 100644 --- a/lib/resourcebuilder/apiext.go +++ b/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) +} diff --git a/lib/resourcebuilder/apiext_test.go b/lib/resourcebuilder/apiext_test.go new file mode 100644 index 000000000..feb8c9658 --- /dev/null +++ b/lib/resourcebuilder/apiext_test.go @@ -0,0 +1,141 @@ +package resourcebuilder + +import ( + "strings" + "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 !strings.Contains(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) + } +} +