diff --git a/internal/client/clienttools.go b/internal/client/clienttools.go index dd1fadd..25f3e84 100644 --- a/internal/client/clienttools.go +++ b/internal/client/clienttools.go @@ -48,13 +48,14 @@ func ToType(ty string) (AuthType, error) { func buildPath(baseUrl string, path string, parameters map[string]string, query map[string]string) *url.URL { for key, param := range parameters { - path = strings.Replace(path, fmt.Sprintf("{%s}", key), fmt.Sprintf("%v", param), 1) + param = url.PathEscape(param) + path = strings.Replace(path, fmt.Sprintf("{%s}", key), param, 1) } params := url.Values{} - for key, param := range query { - params.Add(key, param) + queryParam := url.QueryEscape(param) + params.Add(key, queryParam) } parsed, err := url.Parse(baseUrl) @@ -62,11 +63,15 @@ func buildPath(baseUrl string, path string, parameters map[string]string, query return nil } - parsed.Path = pathutil.Join(parsed.Path, path) + // Remove trailing slash from base path if present + parsed.Opaque = "//" + pathutil.Join(parsed.Host, parsed.Path, path) parsed.RawQuery = params.Encode() + parsed, err = url.Parse(parsed.String()) + if err != nil { + return nil + } return parsed } - func getValidResponseCode(codes *orderedmap.Map[string, *v3.Response]) ([]int, error) { var validCodes []int for code := codes.First(); code != nil; code = code.Next() { diff --git a/internal/client/clienttools_test.go b/internal/client/clienttools_test.go new file mode 100644 index 0000000..c2d2d4d --- /dev/null +++ b/internal/client/clienttools_test.go @@ -0,0 +1,152 @@ +package restclient + +import ( + "fmt" + "net/url" + "reflect" + "testing" +) + +func TestBuildPath_Basic(t *testing.T) { + baseUrl := "https://api.example.com/v1" + path := "/users/{userId}/posts/{postId}" + parameters := map[string]string{ + "userId": "42", + "postId": "99", + } + query := map[string]string{ + "sort": "desc", + "page": "2", + } + + got := buildPath(baseUrl, path, parameters, query) + if got == nil { + t.Fatalf("buildPath returned nil") + } + + got, err := url.Parse(got.String()) + if err != nil { + t.Fatalf("failed to parse base URL: %v", err) + } + + expectedPath := "/v1/users/42/posts/99" + if got.Path != "/v1/users/42/posts/99" { + t.Errorf("expected path %q, got %q", expectedPath, got.Path) + } + + wantQuery := url.Values{"sort": {"desc"}, "page": {"2"}}.Encode() + if got.RawQuery != wantQuery && got.RawQuery != "page=2&sort=desc" { + t.Errorf("expected query %q, got %q", wantQuery, got.RawQuery) + } +} + +func TestBuildPath_WithPathParamsWithSlashes(t *testing.T) { + baseUrl := "https://api.example.com/v1" + path := "/users/{userId}/posts/{postId}" + parameters := map[string]string{ + "userId": "42/123", + "postId": "99", + } + query := map[string]string{} + + got := buildPath(baseUrl, path, parameters, query) + if got == nil { + t.Fatalf("buildPath returned nil") + } + + fmt.Println("Got Path:", got.RawQuery) + + wantPath := baseUrl + "/users/42%2F123/posts/99" + if got.String() != wantPath { + t.Errorf("expected path %q, got %q", wantPath, got.String()) + } +} + +func TestBuildPath_NoParams(t *testing.T) { + baseUrl := "https://api.example.com" + path := "/status" + parameters := map[string]string{} + query := map[string]string{} + + got := buildPath(baseUrl, path, parameters, query) + if got == nil { + t.Fatalf("buildPath returned nil") + } + got, err := url.Parse(got.String()) + if err != nil { + t.Fatalf("failed to parse base URL: %v", err) + } + + wantPath := "/status" + if got.Path != wantPath { + t.Errorf("expected path %q, got %q", wantPath, got.Path) + } + if got.RawQuery != "" { + t.Errorf("expected empty query, got %q", got.RawQuery) + } +} + +func TestBuildPath_InvalidBaseURL(t *testing.T) { + baseUrl := "://bad-url" + path := "/foo" + parameters := map[string]string{} + query := map[string]string{} + + got := buildPath(baseUrl, path, parameters, query) + if got != nil { + t.Errorf("expected nil for invalid baseUrl, got %v", got) + } +} + +func TestBuildPath_MultipleQueryParams(t *testing.T) { + baseUrl := "https://api.example.com" + path := "/search" + parameters := map[string]string{} + query := map[string]string{ + "q": "golang", + "lang": "en", + } + + got := buildPath(baseUrl, path, parameters, query) + if got == nil { + t.Fatalf("buildPath returned nil") + } + + got, err := url.Parse(got.String()) + if err != nil { + t.Fatalf("failed to parse base URL: %v", err) + } + + wantPath := "/search" + if got.Path != wantPath { + t.Errorf("expected path %q, got %q", wantPath, got.Path) + } + + parsedQuery, _ := url.ParseQuery(got.RawQuery) + expectedQuery := url.Values{"q": {"golang"}, "lang": {"en"}} + if !reflect.DeepEqual(parsedQuery, expectedQuery) { + t.Errorf("expected query %v, got %v", expectedQuery, parsedQuery) + } +} + +func TestBuildPath_PathWithNoLeadingSlash(t *testing.T) { + baseUrl := "https://api.example.com/api" + path := "foo/bar" + parameters := map[string]string{} + query := map[string]string{} + + got := buildPath(baseUrl, path, parameters, query) + if got == nil { + t.Fatalf("buildPath returned nil") + } + + got, err := url.Parse(got.String()) + if err != nil { + t.Fatalf("failed to parse base URL: %v", err) + } + + wantPath := "/api/foo/bar" + if got.Path != wantPath { + t.Errorf("expected path %q, got %q", wantPath, got.Path) + } +} diff --git a/internal/client/restclient_test.go b/internal/client/restclient_test.go index 3237194..86f3328 100644 --- a/internal/client/restclient_test.go +++ b/internal/client/restclient_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "os" "testing" @@ -42,6 +43,25 @@ func TestCallWithRecorder(t *testing.T) { expected interface{} expectedError string }{ + { + name: "path with slash in path parameter", + handler: func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "GET", r.Method) + r.URL, _ = url.Parse(r.URL.String()) + assert.Equal(t, "/api/test/123%2F456", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(map[string]interface{}{"message": "success"}) + }, + path: "/api/test/{id}", + opts: &RequestConfiguration{ + Method: "GET", + Parameters: map[string]string{ + "id": "123%2F456", + }, + }, + expected: map[string]interface{}{"message": "success"}, + }, { name: "successful GET request", handler: func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/client/testdata/openapi.yaml b/internal/client/testdata/openapi.yaml index db66560..ffceb4e 100644 --- a/internal/client/testdata/openapi.yaml +++ b/internal/client/testdata/openapi.yaml @@ -7,6 +7,30 @@ servers: description: Test server paths: + /api/test/{id}: + get: + summary: Get test data by ID + operationId: getTestById + parameters: + - name: id + in: path + required: true + schema: + type: string + responses: + '200': + description: Successful operation + content: + application/json: + schema: + type: object + properties: + id: + type: string + name: + type: string + '404': + description: Not found /api/test: get: summary: Get test data diff --git a/internal/restResources/restResources.go b/internal/restResources/restResources.go index ba796c8..6ba3362 100644 --- a/internal/restResources/restResources.go +++ b/internal/restResources/restResources.go @@ -61,6 +61,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c if mg == nil { return controller.ExternalObservation{}, fmt.Errorf("custom resource is nil") } + log := h.logger.WithValues("op", "Observe"). WithValues("apiVersion", mg.GetAPIVersion()). WithValues("kind", mg.GetKind()). diff --git a/internal/restResources/support_test.go b/internal/restResources/support_test.go new file mode 100644 index 0000000..4f3c52f --- /dev/null +++ b/internal/restResources/support_test.go @@ -0,0 +1,161 @@ +package restResources + +import ( + "fmt" + "reflect" + "testing" + + restclient "github.com/krateoplatformops/rest-dynamic-controller/internal/client" + "github.com/krateoplatformops/rest-dynamic-controller/internal/text" +) + +func TestBuildCallConfig_AllFields(t *testing.T) { + callInfo := &CallInfo{ + Method: "POST", + ReqParams: &RequestedParams{ + Parameters: text.StringSet{"param1": {}, "param2": {}}, + Query: text.StringSet{"query1": {}, "query2": {}}, + Body: text.StringSet{"body1": {}, "body2": {}}, + }, + } + + statusFields := map[string]interface{}{ + "param1": "statusParam1", + "query1": "statusQuery1", + "body1": "statusBody1", + } + specFields := map[string]interface{}{ + "param2": "spec%2FParam2", + "query2": "specQuery2", + "body2": "specBody2", + } + + got := BuildCallConfig(callInfo, statusFields, specFields) + + wantParams := map[string]string{ + "param1": "statusParam1", + "param2": "spec%2FParam2", + } + wantQuery := map[string]string{ + "query1": "statusQuery1", + "query2": "specQuery2", + } + wantBody := map[string]interface{}{ + "body1": "statusBody1", + "body2": "specBody2", + } + + if !reflect.DeepEqual(got.Parameters, wantParams) { + t.Errorf("Parameters = %v, want %v", got.Parameters, wantParams) + } + if !reflect.DeepEqual(got.Query, wantQuery) { + t.Errorf("Query = %v, want %v", got.Query, wantQuery) + } + if !reflect.DeepEqual(got.Body, wantBody) { + t.Errorf("Body = %v, want %v", got.Body, wantBody) + } + if got.Method != "POST" { + t.Errorf("Method = %v, want POST", got.Method) + } +} + +func TestBuildCallConfig_EmptyFields(t *testing.T) { + callInfo := &CallInfo{ + Method: "GET", + ReqParams: &RequestedParams{ + Parameters: text.StringSet{}, + Query: text.StringSet{}, + Body: text.StringSet{}, + }, + } + statusFields := map[string]interface{}{} + specFields := map[string]interface{}{} + + got := BuildCallConfig(callInfo, statusFields, specFields) + + if len(got.Parameters) != 0 { + t.Errorf("Parameters should be empty, got %v", got.Parameters) + } + if len(got.Query) != 0 { + t.Errorf("Query should be empty, got %v", got.Query) + } + bodyMap, ok := got.Body.(map[string]interface{}) + if !ok { + t.Errorf("Body should be a map[string]interface{}, got %T", got.Body) + } else if len(bodyMap) != 0 { + t.Errorf("Body should be empty, got %v", got.Body) + } + if got.Method != "GET" { + t.Errorf("Method = %v, want GET", got.Method) + } +} + +func TestProcessFields_PrioritizeNonEmpty(t *testing.T) { + callInfo := &CallInfo{ + ReqParams: &RequestedParams{ + Parameters: text.StringSet{"foo": {}}, + Query: text.StringSet{}, + Body: text.StringSet{}, + }, + } + reqConf := &restclient.RequestConfiguration{ + Parameters: map[string]string{"foo": "existing"}, + Query: map[string]string{}, + } + mapBody := map[string]interface{}{} + + fields := map[string]interface{}{ + "foo": "", + } + processFields(callInfo, fields, reqConf, mapBody) + // Should not overwrite existing non-empty value with empty string + if reqConf.Parameters["foo"] != "existing" { + t.Errorf("Expected 'foo' to remain 'existing', got %q", reqConf.Parameters["foo"]) + } +} + +func TestProcessFields_BodyField(t *testing.T) { + callInfo := &CallInfo{ + ReqParams: &RequestedParams{ + Parameters: text.StringSet{}, + Query: text.StringSet{}, + Body: text.StringSet{"bar": {}}, + }, + } + reqConf := &restclient.RequestConfiguration{ + Parameters: map[string]string{}, + Query: map[string]string{}, + } + mapBody := map[string]interface{}{} + + fields := map[string]interface{}{ + "bar": 123, + } + processFields(callInfo, fields, reqConf, mapBody) + if v, ok := mapBody["bar"]; !ok || v != 123 { + t.Errorf("Expected mapBody['bar'] = 123, got %v", mapBody["bar"]) + } +} + +func TestProcessFields_QueryField(t *testing.T) { + callInfo := &CallInfo{ + ReqParams: &RequestedParams{ + Parameters: text.StringSet{}, + Query: text.StringSet{"baz": {}}, + Body: text.StringSet{}, + }, + } + reqConf := &restclient.RequestConfiguration{ + Parameters: map[string]string{}, + Query: map[string]string{}, + } + mapBody := map[string]interface{}{} + + fields := map[string]interface{}{ + "baz": 456, + } + processFields(callInfo, fields, reqConf, mapBody) + if v, ok := reqConf.Query["baz"]; !ok || v != fmt.Sprintf("%v", 456) { + t.Errorf("Expected Query['baz'] = '456', got %v", reqConf.Query["baz"]) + } +}