Merge pull request #26911 from Honny1/refactro-build-rest-api

refactor: modularize build REST API with utility functions

pkg/api/handlers/utils/errors.go

@@ -108,3 +108,32 @@ func BadRequest(w http.ResponseWriter, key string, value string, err error) {
 func UnSupportedParameter(param string) {
 	log.Infof("API parameter %q: not supported", param)
 }
+
+type BuildError struct {
+	err  error
+	code int
+}
+
+func (e *BuildError) Error() string {
+	return e.err.Error()
+}
+
+func GetBadRequestError(key, value string, err error) *BuildError {
+	return &BuildError{code: http.StatusBadRequest, err: fmt.Errorf("failed to parse query parameter '%s': %q: %w", key, value, err)}
+}
+
+func GetGenericBadRequestError(err error) *BuildError {
+	return &BuildError{code: http.StatusBadRequest, err: err}
+}
+
+func GetInternalServerError(err error) *BuildError {
+	return &BuildError{code: http.StatusInternalServerError, err: err}
+}
+
+func ProcessBuildError(w http.ResponseWriter, err error) {
+	if buildErr, ok := err.(*BuildError); ok {
+		Error(w, buildErr.code, buildErr.err)
+		return
+	}
+	InternalServerError(w, err)
+}

pkg/api/handlers/utils/handler.go

@@ -8,16 +8,20 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"strconv"
 	"unsafe"
 
 	"github.com/blang/semver/v4"
+	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
 	jsoniter "github.com/json-iterator/go"
 	"github.com/sirupsen/logrus"
+	"go.podman.io/image/v5/types"
 
 	"github.com/containers/podman/v5/pkg/api/handlers/utils/apiutil"
 	api "github.com/containers/podman/v5/pkg/api/types"
+	"github.com/containers/podman/v5/pkg/bindings/images"
 )
 
 // IsLibpodRequest returns true if the request related to a libpod endpoint
@@ -147,3 +151,97 @@ func GetDecoder(r *http.Request) *schema.Decoder {
 	}
 	return r.Context().Value(api.CompatDecoderKey).(*schema.Decoder)
 }
+
+// ParseOptionalJSONField unmarshals a JSON string only if the field exists in query values.
+func ParseOptionalJSONField[T any](jsonStr, fieldName string, queryValues url.Values, target *T) error {
+	if _, found := queryValues[fieldName]; found {
+		return json.Unmarshal([]byte(jsonStr), target)
+	}
+	return nil
+}
+
+// ParseOptionalBool creates a types.OptionalBool if the field exists in query values.
+// Returns the OptionalBool and whether the field was found.
+func ParseOptionalBool(value bool, fieldName string, queryValues url.Values) (types.OptionalBool, bool) {
+	if _, found := queryValues[fieldName]; found {
+		return types.NewOptionalBool(value), true
+	}
+	return types.OptionalBoolUndefined, false
+}
+
+// ParseJSONOptionalSlice parses a JSON array string into a slice if the parameter exists.
+// Returns nil if the parameter is not found.
+func ParseJSONOptionalSlice(value string, query url.Values, paramName string) ([]string, error) {
+	if _, found := query[paramName]; found {
+		var result []string
+		if err := json.Unmarshal([]byte(value), &result); err != nil {
+			return nil, err
+		}
+		return result, nil
+	}
+	return nil, nil
+}
+
+// ResponseSender provides streaming JSON responses with automatic flushing.
+type ResponseSender struct {
+	encoder *jsoniter.Encoder
+	flusher func()
+}
+
+// NewBuildResponseSender creates a ResponseSender for streaming build responses.
+// Optionally writes to a debug file if PODMAN_RETAIN_BUILD_ARTIFACT is set.
+func NewBuildResponseSender(w http.ResponseWriter) *ResponseSender {
+	body := w.(io.Writer)
+	if logrus.IsLevelEnabled(logrus.DebugLevel) {
+		if v, found := os.LookupEnv("PODMAN_RETAIN_BUILD_ARTIFACT"); found {
+			if keep, _ := strconv.ParseBool(v); keep {
+				if t, err := os.CreateTemp("", "build_*_server"); err != nil {
+					logrus.Warnf("Failed to create temp file: %v", err)
+				} else {
+					defer t.Close()
+					body = io.MultiWriter(t, w)
+				}
+			}
+		}
+	}
+
+	enc := jsoniter.NewEncoder(body)
+	enc.SetEscapeHTML(true)
+
+	flusher := func() {
+		if f, ok := w.(http.Flusher); ok {
+			f.Flush()
+		}
+	}
+
+	return &ResponseSender{encoder: enc, flusher: flusher}
+}
+
+// Send encodes and sends a response object as JSON with automatic flushing.
+func (b *ResponseSender) Send(response any) {
+	if err := b.encoder.Encode(response); err != nil {
+		logrus.Warnf("Failed to json encode build response: %v", err)
+	}
+	b.flusher()
+}
+
+// SendBuildStream sends a build stream message to the client.
+func (b *ResponseSender) SendBuildStream(message string) {
+	b.Send(images.BuildResponse{Stream: message})
+}
+
+// SendBuildError sends an error message as a build response.
+func (b *ResponseSender) SendBuildError(message string) {
+	response := images.BuildResponse{
+		ErrorMessage: message,
+		Error: &jsonmessage.JSONError{
+			Message: message,
+		},
+	}
+	b.Send(response)
+}
+
+// SendBuildAux sends auxiliary data as part of a build response.
+func (b *ResponseSender) SendBuildAux(aux []byte) {
+	b.Send(images.BuildResponse{Aux: aux})
+}

pkg/api/handlers/utils/handler_test.go

@@ -4,9 +4,14 @@ package utils
 
 import (
 	"net/http/httptest"
+	"net/url"
 	"reflect"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.podman.io/image/v5/types"
 )
 
 func TestErrorEncoderFuncOmit(t *testing.T) {
@@ -107,3 +112,306 @@ func TestWriteJSONNoHTMLEscape(t *testing.T) {
 		t.Errorf("Parsed message doesn't match original: got %v, want %v", parsed, testData)
 	}
 }
+
+func TestParseOptionalJSONField(t *testing.T) {
+	t.Run("field exists with valid JSON", func(t *testing.T) {
+		jsonStr := `["item1", "item2"]`
+		queryValues := url.Values{"testfield": []string{jsonStr}}
+		var target []string
+
+		err := ParseOptionalJSONField(jsonStr, "testfield", queryValues, &target)
+
+		assert.NoError(t, err)
+		assert.Equal(t, []string{"item1", "item2"}, target)
+	})
+
+	t.Run("field does not exist", func(t *testing.T) {
+		jsonStr := `["item1", "item2"]`
+		queryValues := url.Values{"otherfield": []string{jsonStr}}
+		var target []string
+		originalLen := len(target)
+
+		err := ParseOptionalJSONField(jsonStr, "testfield", queryValues, &target)
+
+		assert.NoError(t, err)
+		assert.Len(t, target, originalLen) // Should remain unchanged
+	})
+
+	t.Run("field exists with invalid JSON", func(t *testing.T) {
+		jsonStr := `{invalid json}`
+		queryValues := url.Values{"testfield": []string{jsonStr}}
+		var target map[string]string
+
+		err := ParseOptionalJSONField(jsonStr, "testfield", queryValues, &target)
+
+		assert.Error(t, err)
+	})
+
+	t.Run("complex object parsing", func(t *testing.T) {
+		jsonStr := `{"buildargs": {"ARG1": "value1", "ARG2": "value2"}}`
+		queryValues := url.Values{"config": []string{jsonStr}}
+		var target map[string]map[string]string
+
+		err := ParseOptionalJSONField(jsonStr, "config", queryValues, &target)
+
+		assert.NoError(t, err)
+		expected := map[string]map[string]string{
+			"buildargs": {"ARG1": "value1", "ARG2": "value2"},
+		}
+		assert.Equal(t, expected, target)
+	})
+}
+
+func TestParseOptionalBool(t *testing.T) {
+	t.Run("field exists with true value", func(t *testing.T) {
+		queryValues := url.Values{"testfield": []string{"true"}}
+		result, found := ParseOptionalBool(true, "testfield", queryValues)
+
+		assert.True(t, found)
+		assert.Equal(t, types.NewOptionalBool(true), result)
+	})
+
+	t.Run("field exists with false value", func(t *testing.T) {
+		queryValues := url.Values{"testfield": []string{"false"}}
+		result, found := ParseOptionalBool(false, "testfield", queryValues)
+
+		assert.True(t, found)
+		assert.Equal(t, types.NewOptionalBool(false), result)
+	})
+
+	t.Run("field does not exist", func(t *testing.T) {
+		queryValues := url.Values{"otherfield": []string{"value"}}
+		result, found := ParseOptionalBool(true, "testfield", queryValues)
+
+		assert.False(t, found)
+		var empty types.OptionalBool
+		assert.Equal(t, empty, result)
+	})
+
+	t.Run("multiple values for same field", func(t *testing.T) {
+		queryValues := url.Values{"testfield": []string{"true", "false"}}
+		result, found := ParseOptionalBool(true, "testfield", queryValues)
+
+		assert.True(t, found)
+		assert.Equal(t, types.NewOptionalBool(true), result)
+	})
+}
+
+func TestParseJSONOptionalSlice(t *testing.T) {
+	t.Run("parameter exists with valid JSON array", func(t *testing.T) {
+		value := `["item1", "item2", "item3"]`
+		queryValues := url.Values{"testparam": []string{value}}
+
+		result, err := ParseJSONOptionalSlice(value, queryValues, "testparam")
+
+		assert.NoError(t, err)
+		assert.Equal(t, []string{"item1", "item2", "item3"}, result)
+	})
+
+	t.Run("parameter does not exist", func(t *testing.T) {
+		value := `["item1", "item2"]`
+		queryValues := url.Values{"otherparam": []string{value}}
+
+		result, err := ParseJSONOptionalSlice(value, queryValues, "testparam")
+
+		assert.NoError(t, err)
+		assert.Nil(t, result)
+	})
+
+	t.Run("parameter exists with invalid JSON", func(t *testing.T) {
+		value := `[invalid json]`
+		queryValues := url.Values{"testparam": []string{value}}
+
+		result, err := ParseJSONOptionalSlice(value, queryValues, "testparam")
+
+		assert.Error(t, err)
+		assert.Nil(t, result)
+	})
+
+	t.Run("parameter exists with empty array", func(t *testing.T) {
+		value := `[]`
+		queryValues := url.Values{"testparam": []string{value}}
+
+		result, err := ParseJSONOptionalSlice(value, queryValues, "testparam")
+
+		assert.NoError(t, err)
+		assert.Equal(t, []string{}, result)
+	})
+
+	t.Run("parameter exists with single item", func(t *testing.T) {
+		value := `["single"]`
+		queryValues := url.Values{"testparam": []string{value}}
+
+		result, err := ParseJSONOptionalSlice(value, queryValues, "testparam")
+
+		assert.NoError(t, err)
+		assert.Equal(t, []string{"single"}, result)
+	})
+}
+
+func TestNewBuildResponseSender(t *testing.T) {
+	t.Run("normal operation", func(t *testing.T) {
+		w := httptest.NewRecorder()
+		sender := NewBuildResponseSender(w)
+
+		assert.NotNil(t, sender)
+		assert.NotNil(t, sender.encoder)
+		assert.NotNil(t, sender.flusher)
+	})
+}
+
+func TestResponseSender_Send(t *testing.T) {
+	w := httptest.NewRecorder()
+	sender := NewBuildResponseSender(w)
+
+	testResponse := map[string]interface{}{
+		"stream": "test message",
+		"id":     "12345",
+	}
+
+	sender.Send(testResponse)
+
+	// Check that the response was written
+	assert.NotEmpty(t, w.Body.String())
+
+	// Verify the JSON was properly encoded
+	var decoded map[string]interface{}
+	err := json.Unmarshal(w.Body.Bytes(), &decoded)
+	assert.NoError(t, err)
+	assert.Equal(t, "test message", decoded["stream"])
+	assert.Equal(t, "12345", decoded["id"])
+}
+
+func TestResponseSender_SendBuildStream(t *testing.T) {
+	w := httptest.NewRecorder()
+	sender := NewBuildResponseSender(w)
+
+	message := "Building step 1/5"
+	sender.SendBuildStream(message)
+
+	// Verify the response structure
+	var response map[string]interface{}
+	err := json.Unmarshal(w.Body.Bytes(), &response)
+	assert.NoError(t, err)
+	assert.Equal(t, message, response["stream"])
+}
+
+func TestResponseSender_SendBuildError(t *testing.T) {
+	w := httptest.NewRecorder()
+	sender := NewBuildResponseSender(w)
+
+	errorMessage := "Build failed: syntax error"
+	sender.SendBuildError(errorMessage)
+
+	// Verify the response structure
+	var response map[string]interface{}
+	err := json.Unmarshal(w.Body.Bytes(), &response)
+	assert.NoError(t, err)
+
+	// ErrorMessage field maps to "error" in JSON
+	assert.Equal(t, errorMessage, response["error"])
+	assert.NotNil(t, response["errorDetail"])
+
+	// Check the nested error structure (errorDetail)
+	errorObj := response["errorDetail"].(map[string]interface{})
+	assert.Equal(t, errorMessage, errorObj["message"])
+}
+
+func TestResponseSender_SendBuildAux(t *testing.T) {
+	w := httptest.NewRecorder()
+	sender := NewBuildResponseSender(w)
+
+	auxData := []byte(`{"ID":"sha256:1234567890abcdef"}`)
+	sender.SendBuildAux(auxData)
+
+	// Verify the response structure
+	var response map[string]interface{}
+	err := json.Unmarshal(w.Body.Bytes(), &response)
+	assert.NoError(t, err)
+
+	assert.NotNil(t, response["aux"])
+
+	// The aux field should contain the raw JSON data
+	auxBytes, err := json.Marshal(response["aux"])
+	assert.NoError(t, err)
+	assert.Equal(t, auxData, auxBytes)
+}
+
+func TestResponseSender_SendInvalidJSON(t *testing.T) {
+	w := httptest.NewRecorder()
+	sender := NewBuildResponseSender(w)
+
+	// Create a value that can't be JSON encoded (contains channels)
+	invalidValue := map[string]interface{}{
+		"channel": make(chan string),
+	}
+
+	// This should not panic, but should log a warning
+	sender.Send(invalidValue)
+
+	// The body should be empty since encoding failed
+	assert.Empty(t, w.Body.String())
+}
+
+// Test integration scenarios
+func TestParseOptionalJSONFieldIntegration(t *testing.T) {
+	// Simulate a real query parameter scenario
+	queryValues := url.Values{
+		"buildargs": []string{`{"ARG1":"value1","ARG2":"value2"}`},
+		"labels":    []string{`{"app":"myapp","version":"1.0"}`},
+	}
+
+	t.Run("parse build args", func(t *testing.T) {
+		var buildArgs map[string]string
+		err := ParseOptionalJSONField(queryValues.Get("buildargs"), "buildargs", queryValues, &buildArgs)
+
+		require.NoError(t, err)
+		expected := map[string]string{"ARG1": "value1", "ARG2": "value2"}
+		assert.Equal(t, expected, buildArgs)
+	})
+
+	t.Run("parse labels", func(t *testing.T) {
+		var labels map[string]string
+		err := ParseOptionalJSONField(queryValues.Get("labels"), "labels", queryValues, &labels)
+
+		require.NoError(t, err)
+		expected := map[string]string{"app": "myapp", "version": "1.0"}
+		assert.Equal(t, expected, labels)
+	})
+
+	t.Run("parse non-existent field", func(t *testing.T) {
+		var nonExistent map[string]string
+		err := ParseOptionalJSONField("", "nonexistent", queryValues, &nonExistent)
+
+		assert.NoError(t, err)
+		assert.Nil(t, nonExistent)
+	})
+}
+
+func TestResponseSenderFlushBehavior(t *testing.T) {
+	// Create a custom ResponseWriter that tracks flush calls
+	flushCalled := false
+	w := &testResponseWriter{
+		ResponseRecorder: httptest.NewRecorder(),
+		onFlush: func() {
+			flushCalled = true
+		},
+	}
+
+	sender := NewBuildResponseSender(w)
+	sender.Send(map[string]string{"test": "message"})
+
+	assert.True(t, flushCalled, "Flush should have been called")
+}
+
+// Helper type for testing flush behavior
+type testResponseWriter struct {
+	*httptest.ResponseRecorder
+	onFlush func()
+}
+
+func (t *testResponseWriter) Flush() {
+	if t.onFlush != nil {
+		t.onFlush()
+	}
+}