diff --git a/management/server/networks/resources/types/resource.go b/management/server/networks/resources/types/resource.go index 643f9cdd6..ce4a55efa 100644 --- a/management/server/networks/resources/types/resource.go +++ b/management/server/networks/resources/types/resource.go @@ -33,7 +33,7 @@ type NetworkResource struct { ID string `gorm:"primaryKey"` NetworkID string `gorm:"index"` AccountID string `gorm:"index"` - PublicID string `json:"-"` + PublicID string `json:"-" gorm:"index"` Name string Description string Type NetworkResourceType diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 99bb2c2c1..f335da758 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -58,6 +58,7 @@ const ( keyQueryCondition = "key = ?" mysqlKeyQueryCondition = "`key` = ?" accountAndIDQueryCondition = "account_id = ? and id = ?" + accountAndAnyIDQueryCondition = "account_id = ? and (id = ? or public_id = ?)" accountAndPeerIDQueryCondition = "account_id = ? and peer_id = ?" accountAndIDsQueryCondition = "account_id = ? AND id IN ?" accountIDCondition = "account_id = ?" @@ -4038,6 +4039,30 @@ func (s *SqlStore) GetPolicyByID(ctx context.Context, lockStrength LockingStreng return policy, nil } +// GetPolicyByIDOrPublicID retrieves a policy by either its ID or its PublicID. Peers report +// whichever of the two the network map they were served carries, so callers resolving a +// peer-reported reference cannot know upfront which namespace it belongs to. +func (s *SqlStore) GetPolicyByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types.Policy, error) { + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var policy *types.Policy + + result := tx.Preload(clause.Associations). + Take(&policy, accountAndAnyIDQueryCondition, accountID, policyID, policyID) + if err := result.Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, status.NewPolicyNotFoundError(policyID) + } + log.WithContext(ctx).Errorf("failed to get policy from store: %s", err) + return nil, status.Errorf(status.Internal, "failed to get policy from store") + } + + return policy, nil +} + func (s *SqlStore) CreatePolicy(ctx context.Context, policy *types.Policy) error { result := s.db.Create(policy) if result.Error != nil { @@ -4223,6 +4248,27 @@ func (s *SqlStore) GetRouteByID(ctx context.Context, lockStrength LockingStrengt return route, nil } +// GetRouteByIDOrPublicID retrieves a route by either its ID or its PublicID. See +// GetPolicyByIDOrPublicID for why peer-reported references need both. +func (s *SqlStore) GetRouteByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID string, routeID string) (*route.Route, error) { + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var route *route.Route + result := tx.Take(&route, accountAndAnyIDQueryCondition, accountID, routeID, routeID) + if err := result.Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, status.NewRouteNotFoundError(routeID) + } + log.WithContext(ctx).Errorf("failed to get route from the store: %s", err) + return nil, status.Errorf(status.Internal, "failed to get route from store") + } + + return route, nil +} + // SaveRoute saves a route to the database. func (s *SqlStore) SaveRoute(ctx context.Context, route *route.Route) error { result := s.db.Save(route) @@ -4617,6 +4663,28 @@ func (s *SqlStore) GetNetworkResourceByID(ctx context.Context, lockStrength Lock return netResources, nil } +// GetNetworkResourceByIDOrPublicID retrieves a network resource by either its ID or its +// PublicID. See GetPolicyByIDOrPublicID for why peer-reported references need both. +func (s *SqlStore) GetNetworkResourceByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*resourceTypes.NetworkResource, error) { + tx := s.db + if lockStrength != LockingStrengthNone { + tx = tx.Clauses(clause.Locking{Strength: string(lockStrength)}) + } + + var netResources *resourceTypes.NetworkResource + result := tx. + Take(&netResources, accountAndAnyIDQueryCondition, accountID, resourceID, resourceID) + if result.Error != nil { + if errors.Is(result.Error, gorm.ErrRecordNotFound) { + return nil, status.NewNetworkResourceNotFoundError(resourceID) + } + log.WithContext(ctx).Errorf("failed to get network resource from store: %v", result.Error) + return nil, status.Errorf(status.Internal, "failed to get network resource from store") + } + + return netResources, nil +} + func (s *SqlStore) GetNetworkResourceByName(ctx context.Context, lockStrength LockingStrength, accountID, resourceName string) (*resourceTypes.NetworkResource, error) { tx := s.db if lockStrength != LockingStrengthNone { diff --git a/management/server/store/sql_store_test.go b/management/server/store/sql_store_test.go index ed3419dd7..142a9ac35 100644 --- a/management/server/store/sql_store_test.go +++ b/management/server/store/sql_store_test.go @@ -1972,6 +1972,32 @@ func TestSqlStore_GetPolicyByID(t *testing.T) { } } +func TestSqlStore_GetPolicyByIDOrPublicID(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + + accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + policyID := "cs1tnh0hhcjnqoiuebf0" + + policy, err := store.GetPolicyByID(context.Background(), LockingStrengthNone, accountID, policyID) + require.NoError(t, err) + require.NotEmpty(t, policy.PublicID) + + for _, id := range []string{policyID, policy.PublicID} { + policy, err := store.GetPolicyByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, id) + require.NoError(t, err) + require.Equal(t, policyID, policy.ID) + } + + policy, err = store.GetPolicyByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, "non-existing") + require.Error(t, err) + sErr, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, sErr.Type(), status.NotFound) + require.Nil(t, policy) +} + func TestSqlStore_CreatePolicy(t *testing.T) { store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) t.Cleanup(cleanup) @@ -2631,6 +2657,32 @@ func TestSqlStore_GetNetworkResourceByID(t *testing.T) { } } +func TestSqlStore_GetNetworkResourceByIDOrPublicID(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + + accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + netResourceID := "ctc4nci7qv9061u6ilfg" + + netResource, err := store.GetNetworkResourceByID(context.Background(), LockingStrengthNone, accountID, netResourceID) + require.NoError(t, err) + require.NotEmpty(t, netResource.PublicID) + + for _, id := range []string{netResourceID, netResource.PublicID} { + netResource, err := store.GetNetworkResourceByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, id) + require.NoError(t, err) + require.Equal(t, netResourceID, netResource.ID) + } + + netResource, err = store.GetNetworkResourceByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, "non-existing") + require.Error(t, err) + sErr, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, sErr.Type(), status.NotFound) + require.Nil(t, netResource) +} + func TestSqlStore_SaveNetworkResource(t *testing.T) { store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/store.sql", t.TempDir()) t.Cleanup(cleanup) @@ -3748,6 +3800,32 @@ func TestSqlStore_GetRouteByID(t *testing.T) { } } +func TestSqlStore_GetRouteByIDOrPublicID(t *testing.T) { + store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/extended-store.sql", t.TempDir()) + t.Cleanup(cleanup) + require.NoError(t, err) + + accountID := "bf1c8084-ba50-4ce7-9439-34653001fc3b" + routeID := "ct03t427qv97vmtmglog" + + route, err := store.GetRouteByID(context.Background(), LockingStrengthNone, accountID, routeID) + require.NoError(t, err) + require.NotEmpty(t, route.PublicID) + + for _, id := range []string{routeID, route.PublicID} { + route, err := store.GetRouteByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, id) + require.NoError(t, err) + require.Equal(t, routeID, string(route.ID)) + } + + route, err = store.GetRouteByIDOrPublicID(context.Background(), LockingStrengthNone, accountID, "non-existing") + require.Error(t, err) + sErr, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, sErr.Type(), status.NotFound) + require.Nil(t, route) +} + func TestSqlStore_SaveRoute(t *testing.T) { store, cleanup, err := NewTestStoreFromSQL(context.Background(), "../testdata/extended-store.sql", t.TempDir()) t.Cleanup(cleanup) diff --git a/management/server/store/store.go b/management/server/store/store.go index 869e8dab5..77011f0b3 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -136,6 +136,7 @@ type Store interface { GetAccountPolicies(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*types.Policy, error) GetPolicyByID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types.Policy, error) + GetPolicyByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types.Policy, error) CreatePolicy(ctx context.Context, policy *types.Policy) error SavePolicy(ctx context.Context, policy *types.Policy) error DeletePolicy(ctx context.Context, accountID, policyID string) error @@ -206,6 +207,7 @@ type Store interface { GetAccountRoutes(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*route.Route, error) GetRouteByID(ctx context.Context, lockStrength LockingStrength, accountID, routeID string) (*route.Route, error) + GetRouteByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, routeID string) (*route.Route, error) SaveRoute(ctx context.Context, route *route.Route) error DeleteRoute(ctx context.Context, accountID, routeID string) error @@ -246,6 +248,7 @@ type Store interface { GetNetworkResourcesByNetID(ctx context.Context, lockStrength LockingStrength, accountID, netID string) ([]*resourceTypes.NetworkResource, error) GetNetworkResourcesByAccountID(ctx context.Context, lockStrength LockingStrength, accountID string) ([]*resourceTypes.NetworkResource, error) GetNetworkResourceByID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*resourceTypes.NetworkResource, error) + GetNetworkResourceByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*resourceTypes.NetworkResource, error) GetNetworkResourceByName(ctx context.Context, lockStrength LockingStrength, accountID, resourceName string) (*resourceTypes.NetworkResource, error) SaveNetworkResource(ctx context.Context, resource *resourceTypes.NetworkResource) error DeleteNetworkResource(ctx context.Context, accountID, resourceID string) error diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go index 3d9e160ba..fedab357d 100644 --- a/management/server/store/store_mock.go +++ b/management/server/store/store_mock.go @@ -2115,6 +2115,21 @@ func (mr *MockStoreMockRecorder) GetNetworkResourceByID(ctx, lockStrength, accou return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkResourceByID", reflect.TypeOf((*MockStore)(nil).GetNetworkResourceByID), ctx, lockStrength, accountID, resourceID) } +// GetNetworkResourceByIDOrPublicID mocks base method. +func (m *MockStore) GetNetworkResourceByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, resourceID string) (*types0.NetworkResource, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetworkResourceByIDOrPublicID", ctx, lockStrength, accountID, resourceID) + ret0, _ := ret[0].(*types0.NetworkResource) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNetworkResourceByIDOrPublicID indicates an expected call of GetNetworkResourceByIDOrPublicID. +func (mr *MockStoreMockRecorder) GetNetworkResourceByIDOrPublicID(ctx, lockStrength, accountID, resourceID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkResourceByIDOrPublicID", reflect.TypeOf((*MockStore)(nil).GetNetworkResourceByIDOrPublicID), ctx, lockStrength, accountID, resourceID) +} + // GetNetworkResourceByName mocks base method. func (m *MockStore) GetNetworkResourceByName(ctx context.Context, lockStrength LockingStrength, accountID, resourceName string) (*types0.NetworkResource, error) { m.ctrl.T.Helper() @@ -2445,6 +2460,21 @@ func (mr *MockStoreMockRecorder) GetPolicyByID(ctx, lockStrength, accountID, pol return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPolicyByID", reflect.TypeOf((*MockStore)(nil).GetPolicyByID), ctx, lockStrength, accountID, policyID) } +// GetPolicyByIDOrPublicID mocks base method. +func (m *MockStore) GetPolicyByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, policyID string) (*types3.Policy, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPolicyByIDOrPublicID", ctx, lockStrength, accountID, policyID) + ret0, _ := ret[0].(*types3.Policy) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPolicyByIDOrPublicID indicates an expected call of GetPolicyByIDOrPublicID. +func (mr *MockStoreMockRecorder) GetPolicyByIDOrPublicID(ctx, lockStrength, accountID, policyID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPolicyByIDOrPublicID", reflect.TypeOf((*MockStore)(nil).GetPolicyByIDOrPublicID), ctx, lockStrength, accountID, policyID) +} + // GetPolicyRulesByResourceID mocks base method. func (m *MockStore) GetPolicyRulesByResourceID(ctx context.Context, lockStrength LockingStrength, accountID, peerID string) ([]*types3.PolicyRule, error) { m.ctrl.T.Helper() @@ -2625,6 +2655,21 @@ func (mr *MockStoreMockRecorder) GetRouteByID(ctx, lockStrength, accountID, rout return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRouteByID", reflect.TypeOf((*MockStore)(nil).GetRouteByID), ctx, lockStrength, accountID, routeID) } +// GetRouteByIDOrPublicID mocks base method. +func (m *MockStore) GetRouteByIDOrPublicID(ctx context.Context, lockStrength LockingStrength, accountID, routeID string) (*route.Route, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRouteByIDOrPublicID", ctx, lockStrength, accountID, routeID) + ret0, _ := ret[0].(*route.Route) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRouteByIDOrPublicID indicates an expected call of GetRouteByIDOrPublicID. +func (mr *MockStoreMockRecorder) GetRouteByIDOrPublicID(ctx, lockStrength, accountID, routeID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRouteByIDOrPublicID", reflect.TypeOf((*MockStore)(nil).GetRouteByIDOrPublicID), ctx, lockStrength, accountID, routeID) +} + // GetRoutingPeerNetworks mocks base method. func (m *MockStore) GetRoutingPeerNetworks(ctx context.Context, accountID, peerID string) ([]string, error) { m.ctrl.T.Helper() diff --git a/route/route.go b/route/route.go index 3bdb0a3a1..ef9a39ef7 100644 --- a/route/route.go +++ b/route/route.go @@ -95,7 +95,7 @@ type Route struct { ID ID `gorm:"primaryKey"` // AccountID is a reference to Account that this object belongs AccountID string `gorm:"index"` - PublicID string `json:"-"` + PublicID string `json:"-" gorm:"index"` // Network and Domains are mutually exclusive Network netip.Prefix `gorm:"serializer:json"` Domains domain.List `gorm:"serializer:json"` diff --git a/shared/management/types/policy.go b/shared/management/types/policy.go index b8f605b94..55fe580d3 100644 --- a/shared/management/types/policy.go +++ b/shared/management/types/policy.go @@ -56,7 +56,7 @@ type Policy struct { // ID of the policy' ID string `gorm:"primaryKey"` - PublicID string `json:"-"` + PublicID string `json:"-" gorm:"index"` // AccountID is a reference to Account that this object belongs AccountID string `json:"-" gorm:"index"`