Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

read endpoint #18268

Merged
merged 38 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
183298e
init commit
xwa153 Jul 17, 2023
eaf297d
pass x-consul-token to the grpc server
xwa153 Jul 20, 2023
97dacbc
query params and add logger
xwa153 Jul 20, 2023
7062429
log message
xwa153 Jul 20, 2023
e69aa2d
change log message
xwa153 Jul 20, 2023
4975291
fix unit test
xwa153 Jul 20, 2023
f5e9707
remove read
xwa153 Jul 21, 2023
f1bfb5d
add detailed error handling message
xwa153 Jul 21, 2023
7c19794
Merge branch 'main' into NET-2678/xw-http-api-write
xwa153 Jul 21, 2023
42d6ccf
fix unit tests
xwa153 Jul 21, 2023
595ac3e
fix unit tests
xwa153 Jul 21, 2023
4ccddf3
read endpoint
xwa153 Jul 24, 2023
eb96780
general refactor
xwa153 Jul 25, 2023
1c2efc2
add more tests
xwa153 Jul 25, 2023
884b79d
Merge branch 'main' into NET-2678/xw-http-api-write
xwa153 Jul 25, 2023
dacbda0
refactor test
xwa153 Jul 27, 2023
b5edf4a
add add owner and remove extra check
xwa153 Jul 27, 2023
8db104c
add more tests
xwa153 Aug 2, 2023
30938f1
Merge branch 'main' into NET-2678/xw-http-api-write
xwa153 Aug 2, 2023
0079424
merge write branch
xwa153 Aug 3, 2023
f326d53
refactor checkUrl function
xwa153 Aug 3, 2023
388a465
write the error msg back
xwa153 Aug 3, 2023
c3775a7
add owner test
xwa153 Aug 3, 2023
f769295
merge write
xwa153 Aug 3, 2023
7e2c9fa
could use the consistent mode
xwa153 Aug 3, 2023
adfab07
add new test
xwa153 Aug 3, 2023
996f9e8
add tests regarding the version query parameter
xwa153 Aug 3, 2023
52bbf65
Merge branch 'NET-2678/xw-http-api-write' into NET-2705/xw-http-api-read
xwa153 Aug 3, 2023
071d612
remove duplicate test case
xwa153 Aug 3, 2023
00bab51
add test case
xwa153 Aug 3, 2023
c043f45
code refactor
xwa153 Aug 3, 2023
b1682ef
Merge branch 'NET-2678/xw-http-api-write' into NET-2705/xw-http-api-read
xwa153 Aug 3, 2023
0c13388
merge main
xwa153 Aug 4, 2023
4ae4941
merge main
xwa153 Aug 11, 2023
0a189db
compare whole body in test
xwa153 Aug 11, 2023
e04c8df
refactor
xwa153 Aug 11, 2023
76a0a9f
linter
xwa153 Aug 11, 2023
129bd83
refactor tests
xwa153 Aug 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 65 additions & 29 deletions internal/resource/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ func (h *resourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case http.MethodPut:
h.handleWrite(w, r, ctx)
case http.MethodGet:
h.handleRead(w, r, ctx)
case http.MethodDelete:
h.handleDelete(w, r, ctx)
default:
Expand Down Expand Up @@ -88,17 +90,17 @@ func (h *resourceHandler) handleWrite(w http.ResponseWriter, r *http.Request, ct
return
}

tenancyInfo, resourceName, version := parseParams(r)
tenancyInfo, params := parseParams(r)

rsp, err := h.client.Write(ctx, &pbresource.WriteRequest{
Resource: &pbresource.Resource{
Id: &pbresource.ID{
Type: h.reg.Type,
Tenancy: tenancyInfo,
Name: resourceName,
Name: params["resourceName"],
},
Owner: req.Owner,
Version: version,
Version: params["version"],
Metadata: req.Metadata,
Data: anyProtoMsg,
},
Expand All @@ -117,18 +119,71 @@ func (h *resourceHandler) handleWrite(w http.ResponseWriter, r *http.Request, ct
w.Write(output)
}

func parseParams(r *http.Request) (tenancy *pbresource.Tenancy, resourceName string, version string) {
params := r.URL.Query()
func (h *resourceHandler) handleRead(w http.ResponseWriter, r *http.Request, ctx context.Context) {
tenancyInfo, params := parseParams(r)
if params["consistent"] != "" {
ctx = metadata.AppendToOutgoingContext(ctx, "x-consul-consistency-mode", "consistent")
}

rsp, err := h.client.Read(ctx, &pbresource.ReadRequest{
Id: &pbresource.ID{
Type: h.reg.Type,
Tenancy: tenancyInfo,
Name: params["resourceName"],
},
})
if err != nil {
handleResponseError(err, w, h)
return
}

output, err := jsonMarshal(rsp.Resource)
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
h.logger.Error("Failed to unmarshal GRPC resource response", "error", err)
return
}
w.Write(output)
}

// Note: The HTTP endpoints do not accept UID since it is quite unlikely that the user will have access to it
func (h *resourceHandler) handleDelete(w http.ResponseWriter, r *http.Request, ctx context.Context) {
tenancyInfo, params := parseParams(r)
_, err := h.client.Delete(ctx, &pbresource.DeleteRequest{
Id: &pbresource.ID{
Type: h.reg.Type,
Tenancy: tenancyInfo,
Name: params["resourceName"],
},
Version: params["version"],
})
if err != nil {
handleResponseError(err, w, h)
return
}
w.WriteHeader(http.StatusNoContent)
w.Write([]byte("{}"))
}

func parseParams(r *http.Request) (tenancy *pbresource.Tenancy, params map[string]string) {
query := r.URL.Query()
tenancy = &pbresource.Tenancy{
Partition: params.Get("partition"),
PeerName: params.Get("peer_name"),
Namespace: params.Get("namespace"),
Partition: query.Get("partition"),
PeerName: query.Get("peer_name"),
Namespace: query.Get("namespace"),
}
resourceName = path.Base(r.URL.Path)

resourceName := path.Base(r.URL.Path)
if resourceName == "." || resourceName == "/" {
resourceName = ""
}
version = params.Get("version")

params = make(map[string]string)
params["resourceName"] = resourceName
params["version"] = query.Get("version")
if _, ok := query["consistent"]; ok {
params["consistent"] = "true"
}

return
}
Expand Down Expand Up @@ -173,22 +228,3 @@ func handleResponseError(err error, w http.ResponseWriter, h *resourceHandler) {
}
w.Write([]byte(err.Error()))
}

// Note: The HTTP endpoints do not accept UID since it is quite unlikely that the user will have access to it
func (h *resourceHandler) handleDelete(w http.ResponseWriter, r *http.Request, ctx context.Context) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not deleted 😂
I grouped it right after the handleRead function for readability

tenancyInfo, resourceName, version := parseParams(r)
_, err := h.client.Delete(ctx, &pbresource.DeleteRequest{
Id: &pbresource.ID{
Type: h.reg.Type,
Tenancy: tenancyInfo,
Name: resourceName,
},
Version: version,
})
if err != nil {
handleResponseError(err, w, h)
return
}
w.WriteHeader(http.StatusNoContent)
w.Write([]byte("{}"))
}
119 changes: 79 additions & 40 deletions internal/resource/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

const testACLTokenArtistReadPolicy = "00000000-0000-0000-0000-000000000001"
const testACLTokenArtistWritePolicy = "00000000-0000-0000-0000-000000000002"
const fakeToken = "fake-token"

func parseToken(req *http.Request, token *string) {
*token = req.Header.Get("x-consul-token")
Expand Down Expand Up @@ -109,25 +110,9 @@ func TestResourceWriteHandler(t *testing.T) {

client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes)

v1ArtistHandler := resourceHandler{
resource.Registration{
Type: demo.TypeV1Artist,
Proto: &pbdemov1.Artist{},
},
client,
parseToken,
hclog.NewNullLogger(),
}

v2ArtistHandler := resourceHandler{
resource.Registration{
Type: demo.TypeV2Artist,
Proto: &pbdemov2.Artist{},
},
client,
parseToken,
hclog.NewNullLogger(),
}
r := resource.NewRegistry()
demo.RegisterTypes(r)
handler := NewHandler(client, r, parseToken, hclog.NewNullLogger())

t.Run("should be blocked if the token is not authorized", func(t *testing.T) {
rsp := httptest.NewRecorder()
Expand All @@ -145,7 +130,7 @@ func TestResourceWriteHandler(t *testing.T) {

req.Header.Add("x-consul-token", testACLTokenArtistReadPolicy)

v2ArtistHandler.ServeHTTP(rsp, req)
handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusForbidden, rsp.Result().StatusCode)
})
Expand All @@ -166,7 +151,7 @@ func TestResourceWriteHandler(t *testing.T) {

req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy)

v2ArtistHandler.ServeHTTP(rsp, req)
handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusOK, rsp.Result().StatusCode)

Expand Down Expand Up @@ -206,7 +191,7 @@ func TestResourceWriteHandler(t *testing.T) {

req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy)

v2ArtistHandler.ServeHTTP(rsp, req)
handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusOK, rsp.Result().StatusCode)
var result map[string]any
Expand All @@ -231,7 +216,7 @@ func TestResourceWriteHandler(t *testing.T) {

req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy)

v2ArtistHandler.ServeHTTP(rsp, req)
handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusConflict, rsp.Result().StatusCode)
})
Expand Down Expand Up @@ -265,7 +250,7 @@ func TestResourceWriteHandler(t *testing.T) {

req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy)

v1ArtistHandler.ServeHTTP(rsp, req)
handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusOK, rsp.Result().StatusCode)

Expand All @@ -291,7 +276,7 @@ func TestResourceWriteHandler(t *testing.T) {
})
}

func createResource(t *testing.T, artistHandler resourceHandler) {
func createResource(t *testing.T, artistHandler http.Handler) map[string]any {
rsp := httptest.NewRecorder()
req := httptest.NewRequest("PUT", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader(`
{
Expand All @@ -309,6 +294,65 @@ func createResource(t *testing.T, artistHandler resourceHandler) {

artistHandler.ServeHTTP(rsp, req)
require.Equal(t, http.StatusOK, rsp.Result().StatusCode)

var result map[string]any
require.NoError(t, json.NewDecoder(rsp.Body).Decode(&result))
return result
}

func TestResourceReadHandler(t *testing.T) {
aclResolver := &resourceSvc.MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistReadPolicy, mock.Anything, mock.Anything).
Return(svctest.AuthorizerFrom(t, demo.ArtistV1ReadPolicy, demo.ArtistV2ReadPolicy), nil)
aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything).
Return(svctest.AuthorizerFrom(t, demo.ArtistV1WritePolicy, demo.ArtistV2WritePolicy), nil)
aclResolver.On("ResolveTokenAndDefaultMeta", fakeToken, mock.Anything, mock.Anything).
Return(svctest.AuthorizerFrom(t, ""), nil)

client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes)

r := resource.NewRegistry()
demo.RegisterTypes(r)
handler := NewHandler(client, r, parseToken, hclog.NewNullLogger())

createdResource := createResource(t, handler)

t.Run("Read resource", func(t *testing.T) {
rsp := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&consistent", nil)

req.Header.Add("x-consul-token", testACLTokenArtistReadPolicy)

handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusOK, rsp.Result().StatusCode)

var result map[string]any
require.NoError(t, json.NewDecoder(rsp.Body).Decode(&result))
require.Equal(t, result, createdResource)
})

t.Run("should not be found if resource not exist", func(t *testing.T) {
rsp := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/demo/v2/artist/keith-not-exist?partition=default&peer_name=local&namespace=default&consistent", nil)

req.Header.Add("x-consul-token", testACLTokenArtistReadPolicy)

handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusNotFound, rsp.Result().StatusCode)
})

t.Run("should be blocked if the token is not authorized", func(t *testing.T) {
rsp := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&consistent", nil)

req.Header.Add("x-consul-token", fakeToken)

handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusForbidden, rsp.Result().StatusCode)
})
}

func TestResourceDeleteHandler(t *testing.T) {
Expand All @@ -320,38 +364,33 @@ func TestResourceDeleteHandler(t *testing.T) {

client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes)

v2ArtistHandler := resourceHandler{
resource.Registration{
Type: demo.TypeV2Artist,
Proto: &pbdemov2.Artist{},
},
client,
parseToken,
hclog.NewNullLogger(),
}
r := resource.NewRegistry()
demo.RegisterTypes(r)

handler := NewHandler(client, r, parseToken, hclog.NewNullLogger())

t.Run("should surface PermissionDenied error from resource service", func(t *testing.T) {
createResource(t, v2ArtistHandler)
createResource(t, handler)

deleteRsp := httptest.NewRecorder()
deletReq := httptest.NewRequest("DELETE", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader(""))

deletReq.Header.Add("x-consul-token", testACLTokenArtistReadPolicy)

v2ArtistHandler.ServeHTTP(deleteRsp, deletReq)
handler.ServeHTTP(deleteRsp, deletReq)

require.Equal(t, http.StatusForbidden, deleteRsp.Result().StatusCode)
})

t.Run("should delete a resource without version", func(t *testing.T) {
createResource(t, v2ArtistHandler)
createResource(t, handler)

deleteRsp := httptest.NewRecorder()
deletReq := httptest.NewRequest("DELETE", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader(""))

deletReq.Header.Add("x-consul-token", testACLTokenArtistWritePolicy)

v2ArtistHandler.ServeHTTP(deleteRsp, deletReq)
handler.ServeHTTP(deleteRsp, deletReq)

require.Equal(t, http.StatusNoContent, deleteRsp.Result().StatusCode)

Expand All @@ -370,14 +409,14 @@ func TestResourceDeleteHandler(t *testing.T) {
})

t.Run("should delete a resource with version", func(t *testing.T) {
createResource(t, v2ArtistHandler)
createResource(t, handler)

rsp := httptest.NewRecorder()
req := httptest.NewRequest("DELETE", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&version=1", strings.NewReader(""))

req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy)

v2ArtistHandler.ServeHTTP(rsp, req)
handler.ServeHTTP(rsp, req)

require.Equal(t, http.StatusNoContent, rsp.Result().StatusCode)

Expand Down