luxolus/open-consul
2
0
Fork 0
Code Issues 1 Pull requests Packages Releases Wiki Activity

backport of commit f2e26c36eced387f8b5396e5969f37c8d4b6493c (#17870)

Browse source 
Co-authored-by: Daniel Upton <daniel@floppy.co>
This commit is contained in:
hc-github-team-consul-core 2023-06-26 07:40:50 -05:00 committed by GitHub
parent 45747a760d
commit 7b03d95d6f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 129 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
agent/grpc-external/services/resource/list_by_owner_test.go
View file

@ -74,7 +74,7 @@ func TestListByOwner_TypeNotRegistered(t *testing.T) {
})
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource type demo.v2.artist not registered")
require.Contains(t, err.Error(), "resource type demo.v2.Artist not registered")
}
func TestListByOwner_Empty(t *testing.T) {
@ -126,7 +126,7 @@ func TestListByOwner_Many(t *testing.T) {
}
func TestListByOwner_ACL_PerTypeDenied(t *testing.T) {
authz := AuthorizerFrom(t, `key_prefix "resource/demo.v2.album/" { policy = "deny" }`)
authz := AuthorizerFrom(t, `key_prefix "resource/demo.v2.Album/" { policy = "deny" }`)
_, rsp, err := roundTripListByOwner(t, authz)
// verify resource filtered out, hence no results
@ -135,7 +135,7 @@ func TestListByOwner_ACL_PerTypeDenied(t *testing.T) {
}
func TestListByOwner_ACL_PerTypeAllowed(t *testing.T) {
authz := AuthorizerFrom(t, `key_prefix "resource/demo.v2.album/" { policy = "read" }`)
authz := AuthorizerFrom(t, `key_prefix "resource/demo.v2.Album/" { policy = "read" }`)
album, rsp, err := roundTripListByOwner(t, authz)
// verify resource not filtered out

4
agent/grpc-external/services/resource/list_test.go
View file

@ -58,7 +58,7 @@ func TestList_TypeNotFound(t *testing.T) {
})
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource type demo.v2.artist not registered")
require.Contains(t, err.Error(), "resource type demo.v2.Artist not registered")
}
func TestList_Empty(t *testing.T) {
@ -178,7 +178,7 @@ func TestList_ACL_ListAllowed_ReadDenied(t *testing.T) {
// allow list, deny read
authz := AuthorizerFrom(t, demo.ArtistV2ListPolicy,
`key_prefix "resource/demo.v2.artist/" { policy = "deny" }`)
`key_prefix "resource/demo.v2.Artist/" { policy = "deny" }`)
_, rsp, err := roundTripList(t, authz)
// verify resource filtered out by key:read denied hence no results

2
agent/grpc-external/services/resource/read_test.go
View file

@ -71,7 +71,7 @@ func TestRead_TypeNotFound(t *testing.T) {
_, err = client.Read(context.Background(), &pbresource.ReadRequest{Id: artist.Id})
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource type demo.v2.artist not registered")
require.Contains(t, err.Error(), "resource type demo.v2.Artist not registered")
}
func TestRead_ResourceNotFound(t *testing.T) {

6
agent/grpc-external/services/resource/watch_test.go
View file

@ -66,7 +66,7 @@ func TestWatchList_TypeNotFound(t *testing.T) {
err = mustGetError(t, rspCh)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource type demo.v2.artist not registered")
require.Contains(t, err.Error(), "resource type demo.v2.Artist not registered")
}
func TestWatchList_GroupVersionMatches(t *testing.T) {
@ -172,7 +172,7 @@ func TestWatchList_ACL_ListAllowed_ReadDenied(t *testing.T) {
// allow list, deny read
authz := AuthorizerFrom(t, `
key_prefix "resource/" { policy = "list" }
key_prefix "resource/demo.v2.artist/" { policy = "deny" }
key_prefix "resource/demo.v2.Artist/" { policy = "deny" }
`)
rspCh, _ := roundTripACL(t, authz)
@ -187,7 +187,7 @@ func TestWatchList_ACL_ListAllowed_ReadAllowed(t *testing.T) {
// allow list, allow read
authz := AuthorizerFrom(t, `
key_prefix "resource/" { policy = "list" }
key_prefix "resource/demo.v2.artist/" { policy = "read" }
key_prefix "resource/demo.v2.Artist/" { policy = "read" }
`)
rspCh, artist := roundTripACL(t, authz)

2
agent/grpc-external/services/resource/write_status_test.go
View file

@ -180,7 +180,7 @@ func TestWriteStatus_TypeNotFound(t *testing.T) {
_, err = client.WriteStatus(testContext(t), validWriteStatusRequest(t, res))
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource type demo.v2.artist not registered")
require.Contains(t, err.Error(), "resource type demo.v2.Artist not registered")
}
func TestWriteStatus_ResourceNotFound(t *testing.T) {

2
agent/grpc-external/services/resource/write_test.go
View file

@ -151,7 +151,7 @@ func TestWrite_TypeNotFound(t *testing.T) {
_, err = client.Write(testContext(t), &pbresource.WriteRequest{Resource: res})
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource type demo.v2.artist not registered")
require.Contains(t, err.Error(), "resource type demo.v2.Artist not registered")
}
func TestWrite_ACLs(t *testing.T) {

4
internal/controller/api_test.go
View file

@ -190,7 +190,7 @@ func TestController_String(t *testing.T) {
WithPlacement(controller.PlacementEachServer)
require.Equal(t,
`<Controller managed_type="demo.v2.artist", watched_types=["demo.v2.album"], backoff=<base="5s", max="1h0m0s">, placement="each-server">`,
`<Controller managed_type="demo.v2.Artist", watched_types=["demo.v2.Album"], backoff=<base="5s", max="1h0m0s">, placement="each-server">`,
ctrl.String(),
)
}
@ -201,7 +201,7 @@ func TestController_NoReconciler(t *testing.T) {
ctrl := controller.ForType(demo.TypeV2Artist)
require.PanicsWithValue(t,
`cannot register controller without a reconciler <Controller managed_type="demo.v2.artist", watched_types=[], backoff=<base="5ms", max="16m40s">, placement="singleton">`,
`cannot register controller without a reconciler <Controller managed_type="demo.v2.Artist", watched_types=[], backoff=<base="5ms", max="16m40s">, placement="singleton">`,
func() { mgr.Register(ctrl) })
}

16
internal/resource/demo/demo.go
View file

@ -33,36 +33,36 @@ var (
TypeV1Artist = &pbresource.Type{
Group: "demo",
GroupVersion: "v1",
Kind: "artist",
Kind: "Artist",
}
// TypeV1Album represents a collection of an artist's songs.
TypeV1Album = &pbresource.Type{
Group: "demo",
GroupVersion: "v1",
Kind: "album",
Kind: "Album",
}
// TypeV2Artist represents a musician or group of musicians.
TypeV2Artist = &pbresource.Type{
Group: "demo",
GroupVersion: "v2",
Kind: "artist",
Kind: "Artist",
}
// TypeV2Album represents a collection of an artist's songs.
TypeV2Album = &pbresource.Type{
Group: "demo",
GroupVersion: "v2",
Kind: "album",
Kind: "Album",
}
)
const (
ArtistV1ReadPolicy = `key_prefix "resource/demo.v1.artist/" { policy = "read" }`
ArtistV1WritePolicy = `key_prefix "resource/demo.v1.artist/" { policy = "write" }`
ArtistV2ReadPolicy = `key_prefix "resource/demo.v2.artist/" { policy = "read" }`
ArtistV2WritePolicy = `key_prefix "resource/demo.v2.artist/" { policy = "write" }`
ArtistV1ReadPolicy = `key_prefix "resource/demo.v1.Artist/" { policy = "read" }`
ArtistV1WritePolicy = `key_prefix "resource/demo.v1.Artist/" { policy = "write" }`
ArtistV2ReadPolicy = `key_prefix "resource/demo.v2.Artist/" { policy = "read" }`
ArtistV2WritePolicy = `key_prefix "resource/demo.v2.Artist/" { policy = "write" }`
ArtistV2ListPolicy = `key_prefix "resource/" { policy = "list" }`
)

22
internal/resource/registry.go
View file

@ -5,6 +5,7 @@ package resource
import (
"fmt"
"regexp"
"sync"
"google.golang.org/protobuf/proto"
@ -13,6 +14,12 @@ import (
"github.com/hashicorp/consul/proto-public/pbresource"
)
var (
groupRegexp = regexp.MustCompile(`^[a-z][a-z\d_]+$`)
groupVersionRegexp = regexp.MustCompile(`^v([a-z\d]+)?\d$`)
kindRegexp = regexp.MustCompile(`^[A-Z][A-Za-z\d]+$`)
)
type Registry interface {
// Register the given resource type and its hooks.
Register(reg Registration)
@ -82,14 +89,23 @@ func NewRegistry() Registry {
}
func (r *TypeRegistry) Register(registration Registration) {
r.lock.Lock()
defer r.lock.Unlock()
typ := registration.Type
if typ.Group == "" || typ.GroupVersion == "" || typ.Kind == "" {
panic("type field(s) cannot be empty")
}
switch {
case !groupRegexp.MatchString(typ.Group):
panic(fmt.Sprintf("Type.Group must be in snake_case. Got: %q", typ.Group))
case !groupVersionRegexp.MatchString(typ.GroupVersion):
panic(fmt.Sprintf("Type.GroupVersion must be lowercase, start with `v`, and end with a number (e.g. `v2` or `v1alpha1`). Got: %q", typ.Group))
case !kindRegexp.MatchString(typ.Kind):
panic(fmt.Sprintf("Type.Kind must be in PascalCase. Got: %q", typ.Kind))
}
r.lock.Lock()
defer r.lock.Unlock()
key := ToGVK(registration.Type)
if _, ok := r.registrations[key]; ok {
panic(fmt.Sprintf("resource type %s already registered", key))

117
internal/resource/registry_test.go
View file

@ -28,36 +28,9 @@ func TestRegister(t *testing.T) {
require.True(t, proto.Equal(demo.TypeV2Artist, actual.Type))
// register existing should panic
require.PanicsWithValue(t, "resource type demo.v2.artist already registered", func() {
require.PanicsWithValue(t, "resource type demo.v2.Artist already registered", func() {
r.Register(reg)
})
// type missing required fields should panic
testcases := map[string]*pbresource.Type{
"empty group": {
Group: "",
GroupVersion: "v2",
Kind: "artist",
},
"empty group version": {
Group: "",
GroupVersion: "v2",
Kind: "artist",
},
"empty kind": {
Group: "demo",
GroupVersion: "v2",
Kind: "",
},
}
for desc, typ := range testcases {
t.Run(desc, func(t *testing.T) {
require.PanicsWithValue(t, "type field(s) cannot be empty", func() {
r.Register(resource.Registration{Type: typ})
})
})
}
}
func TestRegister_Defaults(t *testing.T) {
@ -102,7 +75,7 @@ func TestResolve(t *testing.T) {
serviceType := &pbresource.Type{
Group: "mesh",
GroupVersion: "v1",
Kind: "service",
Kind: "Service",
}
// not found
@ -115,3 +88,89 @@ func TestResolve(t *testing.T) {
assert.True(t, ok)
assert.Equal(t, registration.Type, serviceType)
}
func TestRegister_TypeValidation(t *testing.T) {
registry := resource.NewRegistry()
testCases := map[string]struct {
fn func(*pbresource.Type)
valid bool
}{
"Valid": {valid: true},
"Group empty": {
fn: func(t *pbresource.Type) { t.Group = "" },
valid: false,
},
"Group PascalCase": {
fn: func(t *pbresource.Type) { t.Group = "Foo" },
valid: false,
},
"Group kebab-case": {
fn: func(t *pbresource.Type) { t.Group = "foo-bar" },
valid: false,
},
"Group snake_case": {
fn: func(t *pbresource.Type) { t.Group = "foo_bar" },
valid: true,
},
"GroupVersion empty": {
fn: func(t *pbresource.Type) { t.GroupVersion = "" },
valid: false,
},
"GroupVersion snake_case": {
fn: func(t *pbresource.Type) { t.GroupVersion = "v_1" },
valid: false,
},
"GroupVersion kebab-case": {
fn: func(t *pbresource.Type) { t.GroupVersion = "v-1" },
valid: false,
},
"GroupVersion no leading v": {
fn: func(t *pbresource.Type) { t.GroupVersion = "1" },
valid: false,
},
"GroupVersion no trailing number": {
fn: func(t *pbresource.Type) { t.GroupVersion = "OnePointOh" },
valid: false,
},
"Kind PascalCase with numbers": {
fn: func(t *pbresource.Type) { t.Kind = "Number1" },
valid: true,
},
"Kind camelCase": {
fn: func(t *pbresource.Type) { t.Kind = "barBaz" },
valid: false,
},
"Kind snake_case": {
fn: func(t *pbresource.Type) { t.Kind = "bar_baz" },
valid: false,
},
"Kind empty": {
fn: func(t *pbresource.Type) { t.Kind = "" },
valid: false,
},
}
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
reg := func() {
typ := &pbresource.Type{
Group: "foo",
GroupVersion: "v1",
Kind: "Bar",
}
if tc.fn != nil {
tc.fn(typ)
}
registry.Register(resource.Registration{
Type: typ,
})
}
if tc.valid {
require.NotPanics(t, reg)
} else {
require.Panics(t, reg)
}
})
}
}

2
internal/resource/tombstone.go
View file

@ -6,6 +6,6 @@ var (
TypeV1Tombstone = &pbresource.Type{
Group: "internal",
GroupVersion: "v1",
Kind: "tombstone",
Kind: "Tombstone",
}
)