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.

Sign up for GitHub

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

Clean up access list protos, add in conversion functions tests. #28787

Merged
merged 7 commits into from
Jul 13, 2023
Merged

Clean up access list protos, add in conversion functions tests. #28787

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
44deb0d
Clean up access list protos, add in conversion functions tests.
Jul 6, 2023
53584a4
Remove access list test (covered in other PR), run GCI.
Jul 7, 2023
90064d2
Rebase on master.
Jul 7, 2023
d3fa344
Move internal storage repreesentation of objects into lib.
Jul 11, 2023
bbe295f
Rename trait conversion From/ToProto.
Jul 11, 2023
b8368b1
GCI.
Jul 11, 2023
752d622
Reduce stuttering where possible.
Jul 12, 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
13 changes: 7 additions & 6 deletions ...n/proto/go/accesslist/v1/accesslist.pb.go → ...o/teleport/accesslist/v1/accesslist.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions api/gen/proto/go/header/v1/metadata.pb.go → ...roto/go/teleport/header/v1/metadata.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions ...n/proto/go/header/v1/resourceheader.pb.go → ...o/teleport/header/v1/resourceheader.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions api/gen/proto/go/trait/v1/trait.pb.go → ...en/proto/go/teleport/trait/v1/trait.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/proto/teleport/accesslist/v1/accesslist.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import "google/protobuf/timestamp.proto";
import "teleport/header/v1/resourceheader.proto";
import "teleport/trait/v1/trait.proto";

option go_package = "github.com/gravitational/teleport/api/gen/proto/go/accesslist/v1;accesslist";
option go_package = "github.com/gravitational/teleport/api/gen/proto/go/teleport/accesslist/v1;accesslistv1";

// AccessList describes the basic building block of access grants, which are
// similar to access requests but for longer lived permissions that need to be
Expand Down
2 changes: 1 addition & 1 deletion api/proto/teleport/header/v1/metadata.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package teleport.header.v1;

import "google/protobuf/timestamp.proto";

option go_package = "github.com/gravitational/teleport/api/gen/proto/go/header/v1;header";
option go_package = "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1;headerv1";

// Metadata is resource metadata.
message Metadata {
Expand Down
2 changes: 1 addition & 1 deletion api/proto/teleport/header/v1/resourceheader.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package teleport.header.v1;

import "teleport/header/v1/metadata.proto";

option go_package = "github.com/gravitational/teleport/api/gen/proto/go/header/v1;header";
option go_package = "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1;headerv1";

// ResourceHeader is a shared resource header.
message ResourceHeader {
Expand Down
2 changes: 1 addition & 1 deletion api/proto/teleport/trait/v1/trait.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ syntax = "proto3";

package teleport.trait.v1;

option go_package = "github.com/gravitational/teleport/api/gen/proto/go/trait/v1;trait";
option go_package = "github.com/gravitational/teleport/api/gen/proto/go/teleport/trait/v1;traitv1";

// Trait is a trait that can be use in various resources.
message Trait {
Expand Down
12 changes: 0 additions & 12 deletions api/types/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types/common"
"github.com/gravitational/teleport/api/types/header"
"github.com/gravitational/teleport/api/utils"
)

Expand Down Expand Up @@ -368,17 +367,6 @@ func (h *ResourceHeader) CheckAndSetDefaults() error {
return trace.Wrap(h.Metadata.CheckAndSetDefaults())
}

// FromHeaderMetadata will convert a *header.Metadata object to this metadata object.
// TODO: Remove this once we get rid of the old Metadata object.
func FromHeaderMetadata(metadata header.Metadata) Metadata {
return Metadata{
ID: metadata.ID,
Name: metadata.Name,
Description: metadata.Description,
Labels: metadata.Labels,
}
}

// GetID returns resource ID
func (m *Metadata) GetID() int64 {
return m.ID
Expand Down
14 changes: 7 additions & 7 deletions lib/services/access_list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,32 @@ import (

"github.com/gravitational/trace"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/types/accesslist"
"github.com/gravitational/teleport/lib/utils"
)

// AccessListsGetter defines an interface for reading access lists.
type AccessListsGetter interface {
// GetAccessLists returns a list of all access lists.
GetAccessLists(context.Context) ([]*types.AccessList, error)
GetAccessLists(context.Context) ([]*accesslist.AccessList, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a huge fan of the name stuttering on the calling side here - but unfortunately I don't have a good suggestion for the AccessList type itself.

// GetAccessList returns the specified access list resource.
GetAccessList(context.Context, string) (*types.AccessList, error)
GetAccessList(context.Context, string) (*accesslist.AccessList, error)
}

// AccessLists defines an interface for managing AccessLists.
type AccessLists interface {
AccessListsGetter

// UpsertAccessList creates or updates an access list resource.
UpsertAccessList(context.Context, *types.AccessList) error
UpsertAccessList(context.Context, *accesslist.AccessList) error
// DeleteAccessList removes the specified access list resource.
DeleteAccessList(context.Context, string) error
// DeleteAllAccessLists removes all access lists.
DeleteAllAccessLists(context.Context) error
}

// MarshalAccessList marshals the access list resource to JSON.
func MarshalAccessList(accessList *types.AccessList, opts ...MarshalOption) ([]byte, error) {
func MarshalAccessList(accessList *accesslist.AccessList, opts ...MarshalOption) ([]byte, error) {
if err := accessList.CheckAndSetDefaults(); err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -65,15 +65,15 @@ func MarshalAccessList(accessList *types.AccessList, opts ...MarshalOption) ([]b
}

// UnmarshalAccessList unmarshals the access list resource from JSON.
func UnmarshalAccessList(data []byte, opts ...MarshalOption) (*types.AccessList, error) {
func UnmarshalAccessList(data []byte, opts ...MarshalOption) (*accesslist.AccessList, error) {
if len(data) == 0 {
return nil, trace.BadParameter("missing access list data")
}
cfg, err := CollectOptions(opts)
if err != nil {
return nil, trace.Wrap(err)
}
var accessList *types.AccessList
var accessList *accesslist.AccessList
if err := utils.FastUnmarshal(data, &accessList); err != nil {
return nil, trace.BadParameter(err.Error())
}
Expand Down
36 changes: 18 additions & 18 deletions lib/services/access_list_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@ import (

"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/header"
"github.com/gravitational/teleport/lib/types/accesslist"
"github.com/gravitational/teleport/lib/types/header"
"github.com/gravitational/teleport/lib/utils"
)

// TestAccessListUnmarshal verifies an access list resource can be unmarshaled.
func TestAccessListUnmarshal(t *testing.T) {
expected, err := types.NewAccessList(
expected, err := accesslist.NewAccessList(
header.Metadata{
Name: "test-access-list",
},
types.AccessListSpec{
accesslist.AccessListSpec{
Description: "test access list",
Owners: []types.AccessListOwner{
Owners: []accesslist.AccessListOwner{
mdwn marked this conversation as resolved.
Show resolved Hide resolved
{
Name: "test-user1",
Description: "test user 1",
Expand All @@ -45,31 +45,31 @@ func TestAccessListUnmarshal(t *testing.T) {
Description: "test user 2",
},
},
Audit: types.AccessListAudit{
Audit: accesslist.AccessListAudit{
Frequency: time.Hour,
},
MembershipRequires: types.AccessListRequires{
MembershipRequires: accesslist.AccessListRequires{
Roles: []string{"mrole1", "mrole2"},
Traits: map[string][]string{
"mtrait1": {"mvalue1", "mvalue2"},
"mtrait2": {"mvalue3", "mvalue4"},
},
},
OwnershipRequires: types.AccessListRequires{
OwnershipRequires: accesslist.AccessListRequires{
Roles: []string{"orole1", "orole2"},
Traits: map[string][]string{
"otrait1": {"ovalue1", "ovalue2"},
"otrait2": {"ovalue3", "ovalue4"},
},
},
Grants: types.AccessListGrants{
Grants: accesslist.AccessListGrants{
Roles: []string{"grole1", "grole2"},
Traits: map[string][]string{
"gtrait1": {"gvalue1", "gvalue2"},
"gtrait2": {"gvalue3", "gvalue4"},
},
},
Members: []types.AccessListMember{
Members: []accesslist.AccessListMember{
{
Name: "member1",
Joined: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
Expand Down Expand Up @@ -97,13 +97,13 @@ func TestAccessListUnmarshal(t *testing.T) {

// TestAccessListMarshal verifies a marshaled access list resource can be unmarshaled back.
func TestAccessListMarshal(t *testing.T) {
expected, err := types.NewAccessList(
expected, err := accesslist.NewAccessList(
header.Metadata{
Name: "test-rule",
},
types.AccessListSpec{
accesslist.AccessListSpec{
Description: "test access list",
Owners: []types.AccessListOwner{
Owners: []accesslist.AccessListOwner{
{
Name: "test-user1",
Description: "test user 1",
Expand All @@ -113,31 +113,31 @@ func TestAccessListMarshal(t *testing.T) {
Description: "test user 2",
},
},
Audit: types.AccessListAudit{
Audit: accesslist.AccessListAudit{
Frequency: time.Hour,
},
MembershipRequires: types.AccessListRequires{
MembershipRequires: accesslist.AccessListRequires{
Roles: []string{"mrole1", "mrole2"},
Traits: map[string][]string{
"mtrait1": {"mvalue1", "mvalue2"},
"mtrait2": {"mvalue3", "mvalue4"},
},
},
OwnershipRequires: types.AccessListRequires{
OwnershipRequires: accesslist.AccessListRequires{
Roles: []string{"orole1", "orole2"},
Traits: map[string][]string{
"otrait1": {"ovalue1", "ovalue2"},
"otrait2": {"ovalue3", "ovalue4"},
},
},
Grants: types.AccessListGrants{
Grants: accesslist.AccessListGrants{
Roles: []string{"grole1", "grole2"},
Traits: map[string][]string{
"gtrait1": {"gvalue1", "gvalue2"},
"gtrait2": {"gvalue3", "gvalue4"},
},
},
Members: []types.AccessListMember{
Members: []accesslist.AccessListMember{
{
Name: "member1",
Joined: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
Expand Down
11 changes: 6 additions & 5 deletions lib/services/local/access_list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/local/generic"
"github.com/gravitational/teleport/lib/types/accesslist"
)

const (
Expand All @@ -37,12 +38,12 @@ const (
type AccessListService struct {
log logrus.FieldLogger
clock clockwork.Clock
svc *generic.Service[*types.AccessList]
svc *generic.Service[*accesslist.AccessList]
}

// NewAccessListService creates a new AccessListService.
func NewAccessListService(backend backend.Backend, clock clockwork.Clock) (*AccessListService, error) {
svc, err := generic.NewService(&generic.ServiceConfig[*types.AccessList]{
svc, err := generic.NewService(&generic.ServiceConfig[*accesslist.AccessList]{
Backend: backend,
ResourceKind: types.KindAccessList,
BackendPrefix: accessListPrefix,
Expand All @@ -61,19 +62,19 @@ func NewAccessListService(backend backend.Backend, clock clockwork.Clock) (*Acce
}

// GetAccessLists returns a list of all access lists.
func (a *AccessListService) GetAccessLists(ctx context.Context) ([]*types.AccessList, error) {
func (a *AccessListService) GetAccessLists(ctx context.Context) ([]*accesslist.AccessList, error) {
accessLists, err := a.svc.GetResources(ctx)
return accessLists, trace.Wrap(err)
}

// GetAccessList returns the specified access list resource.
func (a *AccessListService) GetAccessList(ctx context.Context, name string) (*types.AccessList, error) {
func (a *AccessListService) GetAccessList(ctx context.Context, name string) (*accesslist.AccessList, error) {
accessList, err := a.svc.GetResource(ctx, name)
return accessList, trace.Wrap(err)
}

// UpsertAccessList creates or updates an access list resource.
func (a *AccessListService) UpsertAccessList(ctx context.Context, accessList *types.AccessList) error {
func (a *AccessListService) UpsertAccessList(ctx context.Context, accessList *accesslist.AccessList) error {
return trace.Wrap(a.svc.UpsertResource(ctx, accessList))
}

Expand Down
Loading