Fix usage of readonly (readonly is not allowed in a write request) an…

…d better error messages
projectsyn · May 27, 2024 · 6804d53 · 6804d53
1 parent e846d04
commit 6804d53
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 90 deletions.
diff --git a/openapi.yaml b/openapi.yaml
@@ -33,7 +33,6 @@ components:
       description: >
         A unique object identifier string. Automatically generated by the API on creation (in the form
         "<letter>-<adjective>-<noun>-<digits>" where all letters are lowercase, max 63 characters in total).
-      readOnly: true
     OIDCConfig:
       type: object
       required:

diff --git a/pkg/api/openapi.go b/pkg/api/openapi.go
diff --git a/pkg/service/api_service.go b/pkg/service/api_service.go
@@ -54,6 +54,11 @@ func NewAPIServer(conf APIConfig, k8sMiddleware ...KubernetesAuth) (*echo.Echo,
 	if err != nil {
 		return nil, fmt.Errorf("error loading swagger spec: %w", err)
 	}
+
+	// Clear out the servers array in the swagger spec, that skips validating
+	// that server names match. We don't know how/where this thing will be run.
+	swagger.Servers = nil
+
 	swaggerJSON, err = swagger.MarshalJSON()
 	if err != nil {
 		return nil, fmt.Errorf("error marshalling swagger spec: %w", err)

diff --git a/pkg/service/api_service_test.go b/pkg/service/api_service_test.go
@@ -257,15 +257,15 @@ func TestHealthz(t *testing.T) {
 	e, _ := setupTest(t)
 
 	result := testutil.NewRequest().Get("/healthz").Go(t, e)
-	assert.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	assert.Equal(t, "ok", string(result.Recorder.Body.String()))
 }
 
 func TestOpenAPI(t *testing.T) {
 	e, _ := setupTest(t)
 
 	result := testutil.NewRequest().Get("/openapi.json").Go(t, e)
-	assert.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	swaggerSpec := &openapi3.T{}
 	err := json.Unmarshal(result.Recorder.Body.Bytes(), swaggerSpec)
 	assert.NoError(t, err)
@@ -277,15 +277,15 @@ func TestSwaggerUI(t *testing.T) {
 	e, _ := setupTest(t)
 
 	result := testutil.NewRequest().Get("/docs").Go(t, e)
-	assert.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	assert.NotEmpty(t, result.Recorder.Body.Bytes)
 }
 
 func TestDiscovery(t *testing.T) {
 	e, _ := setupTest(t)
 
 	result := testutil.NewRequest().Get("/").Go(t, e)
-	assert.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	assert.NotEmpty(t, result.Recorder.Body.Bytes)
 	metadata := api.Metadata{}
 	err := json.Unmarshal(result.Recorder.Body.Bytes(), &metadata)

diff --git a/pkg/service/cluster_test.go b/pkg/service/cluster_test.go
@@ -26,7 +26,7 @@ func TestListCluster(t *testing.T) {
 		Get("/clusters").
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	require.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	clusters := make([]api.Cluster, 0)
 	err := result.UnmarshalJsonToObject(&clusters)
 	assert.NoError(t, err)
@@ -47,7 +47,7 @@ func TestListCluster_FilteredByTenant(t *testing.T) {
 		Get("/clusters?tenant="+tenantA.Name).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	require.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	clusters := make([]api.Cluster, 0)
 	err := result.UnmarshalJsonToObject(&clusters)
 	assert.NoError(t, err)
@@ -98,7 +98,7 @@ func TestListCluster_Sort(t *testing.T) {
 				Get(fmt.Sprintf("/clusters?sort_by=%s", tc.sortBy)).
 				WithHeader(echo.HeaderAuthorization, bearerToken).
 				Go(t, e)
-			require.Equal(t, http.StatusOK, result.Code())
+			requireHTTPCode(t, http.StatusOK, result)
 			clusters := make([]api.Cluster, 0)
 			err := result.UnmarshalJsonToObject(&clusters)
 			assert.NoError(t, err)
@@ -116,7 +116,7 @@ func TestListClusterMissingBearer(t *testing.T) {
 	result := testutil.NewRequest().
 		Get("/clusters").
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 }
 
 func TestListClusterWrongToken(t *testing.T) {
@@ -126,7 +126,7 @@ func TestListClusterWrongToken(t *testing.T) {
 		Get("/clusters").
 		WithHeader(echo.HeaderAuthorization, "asdf").
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 }
 
 func TestCreateCluster(t *testing.T) {
@@ -158,7 +158,7 @@ func TestCreateCluster(t *testing.T) {
 		WithJsonBody(newCluster).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusCreated, result.Code())
+	requireHTTPCode(t, http.StatusCreated, result)
 
 	cluster := &api.Cluster{}
 	err = result.UnmarshalJsonToObject(cluster)
@@ -206,7 +206,7 @@ func TestCreateClusterWithId(t *testing.T) {
 				WithJsonBody(request).
 				WithHeader(echo.HeaderAuthorization, bearerToken).
 				Go(t, e)
-			assert.Equal(t, http.StatusCreated, result.Code())
+			requireHTTPCode(t, http.StatusCreated, result)
 			cluster := &api.Cluster{}
 			err := result.UnmarshalJsonToObject(cluster)
 			assert.NoError(t, err)
@@ -238,7 +238,7 @@ func TestCreateClusterInstanceFact(t *testing.T) {
 		WithJsonBody(newCluster).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusCreated, result.Code())
+	requireHTTPCode(t, http.StatusCreated, result)
 
 	cluster := &api.Cluster{}
 	err = result.UnmarshalJsonToObject(cluster)
@@ -252,7 +252,7 @@ func TestCreateClusterInstanceFact(t *testing.T) {
 		WithJsonBody(newCluster).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusCreated, result.Code())
+	requireHTTPCode(t, http.StatusCreated, result)
 	cluster = &api.Cluster{}
 	err = result.UnmarshalJsonToObject(cluster)
 	assert.NoError(t, err)
@@ -269,7 +269,7 @@ func TestCreateClusterNoJSON(t *testing.T) {
 		WithBody([]byte("invalid-body")).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -287,7 +287,7 @@ func TestCreateClusterNoTenant(t *testing.T) {
 		WithJsonBody(createCluster).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -302,7 +302,7 @@ func TestCreateClusterEmpty(t *testing.T) {
 		WithJsonContentType().
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -316,7 +316,7 @@ func TestClusterDelete(t *testing.T) {
 		Delete("/clusters/"+clusterA.Name).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusNoContent, result.Code())
+	requireHTTPCode(t, http.StatusNoContent, result)
 }
 
 func TestClusterGet(t *testing.T) {
@@ -326,7 +326,7 @@ func TestClusterGet(t *testing.T) {
 		Get("/clusters/"+clusterA.Name).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	cluster := &api.Cluster{}
 	err := result.UnmarshalJsonToObject(cluster)
 	assert.NoError(t, err)
@@ -345,7 +345,7 @@ func TestClusterGetNoToken(t *testing.T) {
 		Get("/clusters/"+clusterB.Name).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	cluster := &api.Cluster{}
 	err := result.UnmarshalJsonToObject(cluster)
 	assert.NoError(t, err)
@@ -362,7 +362,7 @@ func TestClusterGetNotFound(t *testing.T) {
 		Get("/clusters/not-existing").
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusNotFound, result.Code())
+	requireHTTPCode(t, http.StatusNotFound, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -376,7 +376,7 @@ func TestClusterUpdateEmpty(t *testing.T) {
 		Patch("/clusters/1").
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -396,7 +396,7 @@ func TestClusterUpdateTenant(t *testing.T) {
 		WithContentType(api.ContentJSONPatch).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -418,7 +418,7 @@ func TestClusterUpdateUnknown(t *testing.T) {
 		WithContentType(api.ContentJSONPatch).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -440,7 +440,7 @@ func TestClusterUpdateIllegalDeployKey(t *testing.T) {
 		WithContentType(api.ContentJSONPatch).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -462,7 +462,7 @@ func TestClusterUpdateNotManagedDeployKey(t *testing.T) {
 		WithContentType(api.ContentJSONPatch).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	assert.Equal(t, http.StatusBadRequest, result.Code())
+	requireHTTPCode(t, http.StatusBadRequest, result)
 	reason := &api.Reason{}
 	err := result.UnmarshalJsonToObject(reason)
 	assert.NoError(t, err)
@@ -499,7 +499,7 @@ func TestClusterUpdate(t *testing.T) {
 		WithContentType(api.ContentJSONPatch).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	require.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	cluster := &api.Cluster{}
 	err := result.UnmarshalJsonToObject(cluster)
 	require.NoError(t, err)
@@ -532,7 +532,7 @@ func TestClusterUpdateDisplayName(t *testing.T) {
 		WithContentType(api.ContentJSONPatch).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	require.Equal(t, http.StatusOK, result.Code())
+	requireHTTPCode(t, http.StatusOK, result)
 	cluster := &api.Cluster{}
 	err := result.UnmarshalJsonToObject(cluster)
 	require.NoError(t, err)
@@ -613,7 +613,7 @@ func TestClusterPut(t *testing.T) {
 				WithJsonBody(tc.cluster).
 				WithHeader(echo.HeaderAuthorization, bearerToken).
 				Go(t, e)
-			require.Equal(t, tc.code, result.Code())
+			requireHTTPCode(t, tc.code, result)
 
 			res := &api.Cluster{}
 			err := result.UnmarshalJsonToObject(res)
@@ -649,7 +649,7 @@ func TestClusterPutCreateNameMissmatch(t *testing.T) {
 		WithJsonBody(cluster).
 		WithHeader(echo.HeaderAuthorization, bearerToken).
 		Go(t, e)
-	require.Equal(t, http.StatusCreated, result.Code())
+	requireHTTPCode(t, http.StatusCreated, result)
 
 	res := &api.Cluster{}
 	err := result.UnmarshalJsonToObject(res)
@@ -664,3 +664,9 @@ func TestClusterPutCreateNameMissmatch(t *testing.T) {
 	}, clusterObj)
 	require.NotNil(t, clusterObj)
 }
+
+// requireHTTPCode is a helper function to check the HTTP status code of a response and log the response body if the code is not as expected.
+func requireHTTPCode(t *testing.T, expected int, result *testutil.CompletedRequest) {
+	t.Helper()
+	require.Equalf(t, expected, result.Code(), "Unexpected response code: %d, body: %s", result.Code(), string(result.Recorder.Body.String()))
+}