Skip to content

Commit f294f17

Browse files
R-LawtonR-Lawton
authored
Merge pull request #57 from Kuadrant/handle-openapi-fetching-errors
Handle OpenAPI spec fetch failures
2 parents aac5aea + 29c419e commit f294f17

2 files changed

Lines changed: 261 additions & 5 deletions

File tree

‎internal/controller/apiproduct_controller.go‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -452,12 +452,26 @@ func (r *APIProductReconciler) openAPIStatus(ctx context.Context, apiProductObj
452452

453453
req, err := http.NewRequestWithContext(ctx, http.MethodGet, openAPIURL, nil)
454454
if err != nil {
455-
return nil, fmt.Errorf("failed to create HTTP request for OpenAPI spec: %w", err)
455+
return &devportalv1alpha1.OpenAPIStatus{
456+
Raw: "",
457+
LastSyncTime: metav1.Now(),
458+
MaxSizeUsed: r.OpenAPISpecMaxSize},
459+
&OpenAPISpecErr{
460+
Reason: "RequestCreationFailed",
461+
Message: fmt.Sprintf("failed to create HTTP request for OpenAPI spec: %v. Controller will not retry; the spec needs to change", err),
462+
}
456463
}
457464

458465
resp, err := r.HTTPClient.Do(req)
459466
if err != nil {
460-
return nil, fmt.Errorf("failed to fetch OpenAPI spec from %s: %w", openAPIURL, err)
467+
return &devportalv1alpha1.OpenAPIStatus{
468+
Raw: "",
469+
LastSyncTime: metav1.Now(),
470+
MaxSizeUsed: r.OpenAPISpecMaxSize},
471+
&OpenAPISpecErr{
472+
Reason: "FetchFailed",
473+
Message: fmt.Sprintf("failed to fetch OpenAPI spec from %s: %v. Controller will not retry; the spec needs to change", openAPIURL, err),
474+
}
461475
}
462476
defer func() {
463477
if closeErr := resp.Body.Close(); closeErr != nil {
@@ -472,13 +486,20 @@ func (r *APIProductReconciler) openAPIStatus(ctx context.Context, apiProductObj
472486
MaxSizeUsed: r.OpenAPISpecMaxSize},
473487
&OpenAPISpecErr{
474488
Reason: "FetchFailed",
475-
Message: fmt.Sprintf("failed to fetch OpenAPI spec from %s: unexpected status code %d", openAPIURL, resp.StatusCode),
489+
Message: fmt.Sprintf("failed to fetch OpenAPI spec from %s: unexpected status code %d. Controller will not retry; the spec needs to change", openAPIURL, resp.StatusCode),
476490
}
477491
}
478492

479493
body, err := io.ReadAll(resp.Body)
480494
if err != nil {
481-
return nil, fmt.Errorf("failed to read OpenAPI spec response body: %w", err)
495+
return &devportalv1alpha1.OpenAPIStatus{
496+
Raw: "",
497+
LastSyncTime: metav1.Now(),
498+
MaxSizeUsed: r.OpenAPISpecMaxSize},
499+
&OpenAPISpecErr{
500+
Reason: "ReadFailed",
501+
Message: fmt.Sprintf("failed to read OpenAPI spec response body: %v. Controller will not retry; the spec needs to change", err),
502+
}
482503
}
483504

484505
openAPISize := len(body)
@@ -492,7 +513,7 @@ func (r *APIProductReconciler) openAPIStatus(ctx context.Context, apiProductObj
492513
MaxSizeUsed: r.OpenAPISpecMaxSize,
493514
}, &OpenAPISpecErr{
494515
Reason: "SpecSizeTooLarge",
495-
Message: fmt.Sprintf("OpenAPI spec exceeds size limit (%d bytes)", maxSize),
516+
Message: fmt.Sprintf("OpenAPI spec exceeds size limit (%d bytes). Controller will not retry; the spec needs to change", maxSize),
496517
}
497518

498519
}

‎internal/controller/apiproduct_controller_openapi_error_test.go‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
/*
2+
Copyright 2025.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller
18+
19+
import (
20+
"bytes"
21+
"context"
22+
"errors"
23+
"io"
24+
"net/http"
25+
"time"
26+
27+
. "github.com/onsi/ginkgo/v2"
28+
. "github.com/onsi/gomega"
29+
"k8s.io/apimachinery/pkg/api/meta"
30+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31+
"k8s.io/apimachinery/pkg/types"
32+
"k8s.io/utils/ptr"
33+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
34+
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
35+
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
36+
37+
devportalv1alpha1 "github.com/kuadrant/developer-portal-controller/api/v1alpha1"
38+
)
39+
40+
var _ = Describe("APIProduct OpenAPI Error Handling", func() {
41+
const (
42+
nodeTimeOut = NodeTimeout(30 * time.Second)
43+
)
44+
var (
45+
testNamespace string
46+
gateway *gwapiv1.Gateway
47+
route *gwapiv1.HTTPRoute
48+
)
49+
50+
BeforeEach(func(ctx SpecContext) {
51+
createNamespaceWithContext(ctx, &testNamespace)
52+
})
53+
54+
AfterEach(func(ctx SpecContext) {
55+
deleteNamespaceWithContext(ctx, testNamespace)
56+
}, nodeTimeOut)
57+
58+
Context("When OpenAPI fetch fails", func() {
59+
const (
60+
apiProductName = "test-apiproduct-openapi-error"
61+
testURL = "https://api2.example.com/spec.yaml"
62+
testGatewayName = "my-gateway-openapi-error"
63+
testHTTPRouteName = "my-route-openapi-error"
64+
)
65+
66+
ctx := context.Background()
67+
68+
var (
69+
apiProductKey types.NamespacedName
70+
apiproduct *devportalv1alpha1.APIProduct
71+
)
72+
73+
BeforeEach(func() {
74+
apiProductKey = types.NamespacedName{
75+
Name: apiProductName,
76+
Namespace: testNamespace,
77+
}
78+
apiproduct = &devportalv1alpha1.APIProduct{
79+
TypeMeta: metav1.TypeMeta{
80+
Kind: "APIProduct",
81+
APIVersion: devportalv1alpha1.GroupVersion.String(),
82+
},
83+
ObjectMeta: metav1.ObjectMeta{
84+
Name: apiProductKey.Name,
85+
Namespace: apiProductKey.Namespace,
86+
},
87+
Spec: devportalv1alpha1.APIProductSpec{
88+
TargetRef: gatewayapiv1alpha2.LocalPolicyTargetReference{
89+
Group: gwapiv1.GroupName,
90+
Name: testHTTPRouteName,
91+
Kind: "HTTPRoute",
92+
},
93+
PublishStatus: "Draft",
94+
ApprovalMode: "manual",
95+
Documentation: &devportalv1alpha1.DocumentationSpec{
96+
OpenAPISpecURL: ptr.To(testURL),
97+
},
98+
},
99+
}
100+
101+
gateway = buildBasicGateway(testGatewayName, testNamespace)
102+
Expect(k8sClient.Create(ctx, gateway)).To(Succeed())
103+
route = buildBasicHttpRoute(testHTTPRouteName, testGatewayName, testNamespace, []string{"openapi-error.example.com"})
104+
Expect(k8sClient.Create(ctx, route)).ToNot(HaveOccurred())
105+
addAcceptedCondition(route)
106+
Expect(k8sClient.Status().Update(ctx, route)).ToNot(HaveOccurred())
107+
Expect(k8sClient.Create(ctx, apiproduct)).ToNot(HaveOccurred())
108+
})
109+
110+
It("should handle network errors gracefully", func() {
111+
By("Reconciling with mock HTTP client that returns network error")
112+
mockClient := &mockHTTPClient{
113+
DoFunc: func(req *http.Request) (*http.Response, error) {
114+
// Simulate DNS lookup failure
115+
return nil, errors.New("dial tcp: lookup api2.example.com on 10.96.0.10:53: server misbehaving")
116+
},
117+
}
118+
119+
controllerReconciler := &APIProductReconciler{
120+
Client: k8sClient,
121+
Scheme: k8sClient.Scheme(),
122+
HTTPClient: mockClient,
123+
OpenAPISpecMaxSize: 500 * 1024,
124+
}
125+
126+
_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{})
127+
// Reconcile should NOT return an error - the error should be in the status
128+
Expect(err).NotTo(HaveOccurred())
129+
130+
err = k8sClient.Get(ctx, apiProductKey, apiproduct)
131+
Expect(err).NotTo(HaveOccurred())
132+
133+
By("Checking OpenAPISpecReady condition is False")
134+
condition := meta.FindStatusCondition(apiproduct.Status.Conditions, devportalv1alpha1.StatusConditionOpenAPISpecReady)
135+
Expect(condition).NotTo(BeNil())
136+
Expect(condition.Status).To(Equal(metav1.ConditionFalse))
137+
Expect(condition.Reason).To(Equal("FetchFailed"))
138+
Expect(condition.Message).To(ContainSubstring("failed to fetch OpenAPI spec"))
139+
Expect(condition.Message).To(ContainSubstring("api2.example.com"))
140+
141+
By("Checking OpenAPI status has empty content but timestamp")
142+
Expect(apiproduct.Status.OpenAPI).NotTo(BeNil())
143+
Expect(apiproduct.Status.OpenAPI.Raw).To(BeEmpty())
144+
Expect(apiproduct.Status.OpenAPI.LastSyncTime).NotTo(BeZero())
145+
})
146+
147+
It("should handle HTTP error codes gracefully", func() {
148+
By("Reconciling with mock HTTP client that returns 404")
149+
mockClient := &mockHTTPClient{
150+
DoFunc: func(req *http.Request) (*http.Response, error) {
151+
return &http.Response{
152+
StatusCode: http.StatusNotFound,
153+
Body: io.NopCloser(bytes.NewBufferString("")),
154+
}, nil
155+
},
156+
}
157+
158+
controllerReconciler := &APIProductReconciler{
159+
Client: k8sClient,
160+
Scheme: k8sClient.Scheme(),
161+
HTTPClient: mockClient,
162+
OpenAPISpecMaxSize: 500 * 1024,
163+
}
164+
165+
_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{})
166+
// Reconcile should NOT return an error - the error should be in the status
167+
Expect(err).NotTo(HaveOccurred())
168+
169+
err = k8sClient.Get(ctx, apiProductKey, apiproduct)
170+
Expect(err).NotTo(HaveOccurred())
171+
172+
By("Checking OpenAPISpecReady condition is False")
173+
condition := meta.FindStatusCondition(apiproduct.Status.Conditions, devportalv1alpha1.StatusConditionOpenAPISpecReady)
174+
Expect(condition).NotTo(BeNil())
175+
Expect(condition.Status).To(Equal(metav1.ConditionFalse))
176+
Expect(condition.Reason).To(Equal("FetchFailed"))
177+
Expect(condition.Message).To(ContainSubstring("unexpected status code 404"))
178+
})
179+
180+
It("should recover when fetch succeeds after previous failure", func() {
181+
By("First reconcile with network error")
182+
mockClient := &mockHTTPClient{
183+
DoFunc: func(req *http.Request) (*http.Response, error) {
184+
return nil, errors.New("network error")
185+
},
186+
}
187+
188+
controllerReconciler := &APIProductReconciler{
189+
Client: k8sClient,
190+
Scheme: k8sClient.Scheme(),
191+
HTTPClient: mockClient,
192+
OpenAPISpecMaxSize: 500 * 1024,
193+
}
194+
195+
_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{})
196+
Expect(err).NotTo(HaveOccurred())
197+
198+
err = k8sClient.Get(ctx, apiProductKey, apiproduct)
199+
Expect(err).NotTo(HaveOccurred())
200+
201+
condition := meta.FindStatusCondition(apiproduct.Status.Conditions, devportalv1alpha1.StatusConditionOpenAPISpecReady)
202+
Expect(condition).NotTo(BeNil())
203+
Expect(condition.Status).To(Equal(metav1.ConditionFalse))
204+
205+
By("Updating spec to trigger a new fetch")
206+
apiproduct.Spec.DisplayName = "Updated Name"
207+
Expect(k8sClient.Update(ctx, apiproduct)).To(Succeed())
208+
209+
By("Second reconcile with successful fetch")
210+
openAPIContent := `{"openapi": "3.0.0", "info": {"title": "Test API"}}`
211+
mockClient.DoFunc = func(req *http.Request) (*http.Response, error) {
212+
return &http.Response{
213+
StatusCode: http.StatusOK,
214+
Body: io.NopCloser(bytes.NewBufferString(openAPIContent)),
215+
}, nil
216+
}
217+
218+
_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{})
219+
Expect(err).NotTo(HaveOccurred())
220+
221+
err = k8sClient.Get(ctx, apiProductKey, apiproduct)
222+
Expect(err).NotTo(HaveOccurred())
223+
224+
By("Checking OpenAPISpecReady condition is now True")
225+
condition = meta.FindStatusCondition(apiproduct.Status.Conditions, devportalv1alpha1.StatusConditionOpenAPISpecReady)
226+
Expect(condition).NotTo(BeNil())
227+
Expect(condition.Status).To(Equal(metav1.ConditionTrue))
228+
Expect(condition.Reason).To(Equal("SpecFetched"))
229+
230+
By("Checking OpenAPI status has content")
231+
Expect(apiproduct.Status.OpenAPI).NotTo(BeNil())
232+
Expect(apiproduct.Status.OpenAPI.Raw).To(Equal(openAPIContent))
233+
})
234+
})
235+
})

0 commit comments

Comments
 (0)