feat: improve observation validation for POST requests

Signed-off-by: Ariel Septon <arielsepton@Ariels-MBP.lan>

examples/sample/disposablerequest-jwt.yaml

@@ -14,17 +14,13 @@ spec:
     url:  http://flask-api.default.svc.cluster.local/v1/login
     method: POST
 
+    # Indicates whether the reconciliation should loop indefinitely. If `rollbackRetriesLimit` is set and the request returns an error, it will stop reconciliation once the limit is reached.
     shouldLoopInfinitely: true
+    # Specifies the duration after which the next reconcile should occur.
     nextReconcile: 72h # 3 days
 
     # waitTimeout: 5m
 
-    # Indicates whether the reconciliation should loop indefinitely. If `rollbackRetriesLimit` is set and the request returns an error, it will stop reconciliation once the limit is reached.
-    # shouldLoopInfinitely: true
-
-    # Specifies the duration after which the next reconcile should occur.
-    # nextReconcile: 3m 
-
     # Secrets receiving patches from response data
     secretInjectionConfigs: 
       - secretRef:

examples/sample/request.yaml

@@ -61,7 +61,7 @@ spec:
         url: (.payload.baseUrl + "/" + (.response.body.id|tostring))
 
     # expectedResponseCheck is optional. If not specified or if the type is "DEFAULT", 
-    # the resource is considered up to date if the GET response matches the PUT body.
+    # the resource is considered up to date if the GET response containes the PUT body.
     # If specified, the JQ logic determines if the resource is up to date:
     # - If the JQ query is false, a PUT request is sent to update the resource.
     # - If true, the resource is considered up to date.

internal/clients/http/client.go

@@ -54,7 +54,11 @@ type HttpDetails struct {
 // SendRequest sends an HTTP request to the specified URL with the given method, body, headers and skipTLSVerify.
 func (hc *client) SendRequest(ctx context.Context, method string, url string, body Data, headers Data, skipTLSVerify bool) (details HttpDetails, err error) {
 	requestBody := []byte(body.Decrypted.(string))
+
+	// request contains the HTTP request that will be sent.
 	request, err := http.NewRequestWithContext(ctx, method, url, bytes.NewBuffer(requestBody))
+
+	// requestDetails contains the request details that will be logged.
 	requestDetails := HttpRequest{
 		URL:     url,
 		Body:    body.Encrypted.(string),

internal/controller/request/observe.go

@@ -81,7 +81,7 @@ func (c *external) determineResponseCheck(ctx context.Context, cr *v1alpha2.Requ
 
 // isObjectValidForObservation checks if the object is valid for observation
 func (c *external) isObjectValidForObservation(cr *v1alpha2.Request) bool {
-	return cr.Status.Response.Body != "" &&
+	return cr.Status.Response.StatusCode != 0 &&
 		!(cr.Status.RequestDetails.Method == http.MethodPost && utils.IsHTTPError(cr.Status.Response.StatusCode))
 }
 

internal/controller/request/observe_test.go

@@ -59,7 +59,7 @@ func Test_isUpToDate(t *testing.T) {
 		args args
 		want want
 	}{
-		"ObjectNotFoundEmptyBody": {
+		"ObjectNotFoundEmptyStatus": {
 			args: args{
 				http: &MockHttpClient{
 					MockSendRequest: func(ctx context.Context, method string, url string, body, headers httpClient.Data, skipTLSVerify bool) (resp httpClient.HttpDetails, err error) {
@@ -71,6 +71,7 @@ func Test_isUpToDate(t *testing.T) {
 				},
 				mg: httpRequest(func(r *v1alpha2.Request) {
 					r.Status.Response.Body = ""
+					r.Status.Response.StatusCode = 0
 				}),
 			},
 			want: want{
@@ -100,14 +101,19 @@ func Test_isUpToDate(t *testing.T) {
 			args: args{
 				http: &MockHttpClient{
 					MockSendRequest: func(ctx context.Context, method string, url string, body, headers httpClient.Data, skipTLSVerify bool) (resp httpClient.HttpDetails, err error) {
-						return httpClient.HttpDetails{}, nil
+						return httpClient.HttpDetails{
+							HttpResponse: httpClient.HttpResponse{
+								Body:       "",
+								StatusCode: http.StatusNotFound,
+							},
+						}, nil
 					},
 				},
 				localKube: &test.MockClient{
 					MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil),
 				},
 				mg: httpRequest(func(r *v1alpha2.Request) {
-					r.Status.Response.StatusCode = 404
+					r.Status.Response.StatusCode = http.StatusNotFound
 				}),
 			},
 			want: want{
@@ -130,6 +136,7 @@ func Test_isUpToDate(t *testing.T) {
 				},
 				mg: httpRequest(func(r *v1alpha2.Request) {
 					r.Status.Response.Body = `{"username":"john_doe_new_username"}`
+					r.Status.Response.StatusCode = http.StatusOK
 				}),
 			},
 			want: want{
@@ -153,6 +160,7 @@ func Test_isUpToDate(t *testing.T) {
 				},
 				mg: httpRequest(func(r *v1alpha2.Request) {
 					r.Status.Response.Body = `{"username":"john_doe_new_username"}`
+					r.Status.Response.StatusCode = http.StatusOK
 				}),
 			},
 			want: want{
@@ -474,12 +482,13 @@ func Test_isObjectValidForObservation(t *testing.T) {
 		args args
 		want want
 	}{
-		"ValidResponseBody": {
+		"ValidStatusCode": {
 			args: args{
 				cr: &v1alpha2.Request{
 					Status: v1alpha2.RequestStatus{
 						Response: v1alpha2.Response{
-							Body: "some response",
+							Body:       "",
+							StatusCode: http.StatusOK,
 						},
 						RequestDetails: v1alpha2.Mapping{
 							Method: http.MethodGet,
@@ -491,12 +500,13 @@ func Test_isObjectValidForObservation(t *testing.T) {
 				valid: true,
 			},
 		},
-		"EmptyResponseBody": {
+		"EmptyStatusCode": {
 			args: args{
 				cr: &v1alpha2.Request{
 					Status: v1alpha2.RequestStatus{
 						Response: v1alpha2.Response{
-							Body: "",
+							Body:       "",
+							StatusCode: 0,
 						},
 					},
 				},

internal/controller/request/statushandler/status.go

@@ -2,6 +2,7 @@ package statushandler
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"strconv"
 
@@ -38,6 +39,7 @@ type requestStatusHandler struct {
 // based on the outcome of the HTTP request and the presence of an error.
 func (r *requestStatusHandler) SetRequestStatus() error {
 	if r.responseError != nil {
+		r.logger.Debug("error occurred during HTTP request", "error", r.responseError)
 		return r.setErrorAndReturn(r.responseError)
 	}
 
@@ -65,21 +67,25 @@ func (r *requestStatusHandler) SetRequestStatus() error {
 	return nil
 }
 
+// setErrorAndReturn sets the error message in the status of the Request.
 func (r *requestStatusHandler) setErrorAndReturn(err error) error {
+	r.logger.Debug("Error occurred during HTTP request", "error", err)
 	if settingError := utils.SetRequestResourceStatus(*r.resource, r.resource.SetError(err)); settingError != nil {
 		return errors.Wrap(settingError, utils.ErrFailedToSetStatus)
 	}
 
 	return err
 }
 
+// incrementFailuresAndReturn increments the failures counter and sets the error message in the status of the Request.
 func (r *requestStatusHandler) incrementFailuresAndReturn(combinedSetters []utils.SetRequestStatusFunc) error {
 	combinedSetters = append(combinedSetters, r.resource.SetError(nil)) // should increment failures counter
 
 	if settingError := utils.SetRequestResourceStatus(*r.resource, combinedSetters...); settingError != nil {
 		return errors.Wrap(settingError, utils.ErrFailedToSetStatus)
 	}
 
+	r.logger.Debug(fmt.Sprintf("HTTP request failed with status code %s, and response %s", strconv.Itoa(r.resource.HttpResponse.StatusCode), r.resource.HttpResponse.Body))
 	return errors.Errorf(utils.ErrStatusCode, r.resource.HttpRequest.Method, strconv.Itoa(r.resource.HttpResponse.StatusCode))
 }
 
@@ -108,6 +114,7 @@ func (r *requestStatusHandler) shouldSetCache(forProvider v1alpha2.RequestParame
 	return true
 }
 
+// ResetFailures resets the failures counter in the status of the Request.
 func (r *requestStatusHandler) ResetFailures() {
 	if r.extraSetters == nil {
 		r.extraSetters = &[]utils.SetRequestStatusFunc{}

internal/utils/retry.go

@@ -10,25 +10,30 @@ const (
 	defaultWaitTimeout = 5 * time.Minute
 )
 
+// ShouldRetry determines if the request should be retried based on the status of the request and the rollback retries limit.
 func ShouldRetry(rollbackRetriesLimit *int32, statusFailed int32) bool {
 	return RollBackEnabled(rollbackRetriesLimit) && statusFailed != 0
 }
 
+// RollBackEnabled determines if the rollback retries limit is enabled.
 func RollBackEnabled(rollbackRetriesLimit *int32) bool {
 	return rollbackRetriesLimit != nil
 }
 
+// RetriesLimitReached determines if the rollback retries limit has been reached.
 func RetriesLimitReached(statusFailed int32, rollbackRetriesLimit *int32) bool {
 	return statusFailed >= *rollbackRetriesLimit
 }
 
+// WaitTimeout returns the wait timeout duration.
 func WaitTimeout(timeout *v1.Duration) time.Duration {
 	if timeout != nil {
 		return timeout.Duration
 	}
 	return defaultWaitTimeout
 }
 
+// GetRollbackRetriesLimit returns the rollback retries limit.
 func GetRollbackRetriesLimit(rollbackRetriesLimit *int32) int32 {
 	limit := int32(1)
 	if rollbackRetriesLimit != nil {

internal/utils/set_status.go

@@ -11,8 +11,10 @@ const (
 	ErrFailedToSetStatus = "failed to update status"
 )
 
+// SetRequestStatusFunc is a function that sets the status of a resource.
 type SetRequestStatusFunc func()
 
+// RequestResource is a struct that holds the resource, request context, http response, http request, and local client.
 type RequestResource struct {
 	Resource       client.Object
 	RequestContext context.Context
@@ -101,36 +103,44 @@ func (rr *RequestResource) ResetFailures() SetRequestStatusFunc {
 	}
 }
 
+// ResponseSetter is an interface that defines the methods to set the status code, headers, and body of a resource.
 type ResponseSetter interface {
 	SetStatusCode(statusCode int)
 	SetHeaders(headers map[string][]string)
 	SetBody(body string)
 }
 
+// CacheSetter is an interface that defines the method to set the cache of a resource.
 type CacheSetter interface {
 	SetCache(statusCode int, headers map[string][]string, body string)
 }
 
+// SyncedSetter is an interface that defines the method to set the synced status of a resource.
 type SyncedSetter interface {
 	SetSynced(synced bool)
 }
 
+// ErrorSetter is an interface that defines the method to set the error of a resource.
 type ErrorSetter interface {
 	SetError(err error)
 }
 
+// ResetFailures is an interface that defines the method to reset the failures of a resource.
 type ResetFailures interface {
 	ResetFailures()
 }
 
+// LastReconcileTimeSetter is an interface that defines the method to set the last reconcile time of a resource.
 type LastReconcileTimeSetter interface {
 	SetLastReconcileTime()
 }
 
+// RequestDetailsSetter is an interface that defines the method to set the request details of a resource.
 type RequestDetailsSetter interface {
 	SetRequestDetails(url, method, body string, headers map[string][]string)
 }
 
+// SetRequestResourceStatus sets the status of a resource.
 func SetRequestResourceStatus(rr RequestResource, statusFuncs ...SetRequestStatusFunc) error {
 	for _, updateStatusFunc := range statusFuncs {
 		updateStatusFunc()

internal/utils/validate.go

@@ -12,6 +12,7 @@ const (
 	ErrStatusCode  = "HTTP %s request failed with status code: %s"
 )
 
+// IsRequestValid checks if an HTTP request is valid.
 func IsRequestValid(method string, url string) error {
 	if method == "" {
 		return errors.New(errEmptyMethod)