diff --git a/integration_tests/management/network_map_db/pgsql/domain_test.go b/integration_tests/management/network_map_db/pgsql/domain_test.go index 8a6e7fe8f..afb8645e0 100644 --- a/integration_tests/management/network_map_db/pgsql/domain_test.go +++ b/integration_tests/management/network_map_db/pgsql/domain_test.go @@ -12,15 +12,15 @@ import ( func TestGetDomains(t *testing.T) { ctx := context.TODO() - _, err := pgstore.Pool.Query(ctx, + _, err := pgstore.Pool.Exec(ctx, `insert into domains (id, account_id, domain, target_cluster) VALUES('domain-1','account-1','test-1.com','target-1.cluster.local')`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into domains (id, account_id, domain, target_cluster) VALUES('domain-2','account-1','test-2.com','target-2.cluster.local')`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into domains (id, account_id, domain, target_cluster) VALUES('domain-3','account-1',null,null)`) assert.NoError(t, err) diff --git a/integration_tests/management/network_map_db/pgsql/group_test.go b/integration_tests/management/network_map_db/pgsql/group_test.go index 417cb0bcc..343daab96 100644 --- a/integration_tests/management/network_map_db/pgsql/group_test.go +++ b/integration_tests/management/network_map_db/pgsql/group_test.go @@ -48,7 +48,7 @@ func TestGetGroupsWithoutExpectedFields(t *testing.T) { acctId := xid.New().String() - _, err = s.Pool.Query(ctx, + _, err = s.Pool.Exec(ctx, "insert into accounts (id) VALUES($1)", acctId) assert.NoError(t, err) diff --git a/integration_tests/management/network_map_db/pgsql/nameserver_test.go b/integration_tests/management/network_map_db/pgsql/nameserver_test.go index e23a5091c..d2879719f 100644 --- a/integration_tests/management/network_map_db/pgsql/nameserver_test.go +++ b/integration_tests/management/network_map_db/pgsql/nameserver_test.go @@ -13,15 +13,15 @@ import ( func TestGetNameServerGroups(t *testing.T) { ctx := context.TODO() - _, err := pgstore.Pool.Query(ctx, + _, err := pgstore.Pool.Exec(ctx, `insert into name_server_groups (id, public_id, name, description, name_servers, groups, domains, enabled, search_domains_enabled, "primary", account_id) VALUES('nsgroup-1','nsgroup-1-public','nsgroup-1','nsgroup-1','[{"IP":"192.168.31.2","NSType":1,"Port":53}]','["group-one-resource-id"]','["test-1.com"]',TRUE,FALSE,TRUE,'account-1')`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into name_server_groups (id, public_id, name, description, name_servers, groups, domains, enabled, search_domains_enabled,"primary",account_id) VALUES('nsgroup-2','nsgroup-2-public','nsgroup-2','nsgroup-2','[{"IP":"192.168.32.3","NSType":1,"Port":53}]','["group-one-resource-id","group-no-resources-id"]','["test-1.com","test-2.com"]',TRUE,FALSE,TRUE,'account-1')`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into name_server_groups (id, public_id, name, description, name_servers, groups, domains, enabled, search_domains_enabled,"primary",account_id) VALUES('nsgroup-3','nsgroup-3-public',null,null,null,null,null,TRUE,FALSE,FALSE,'account-1')`) assert.NoError(t, err) diff --git a/integration_tests/management/network_map_db/pgsql/network_resource_test.go b/integration_tests/management/network_map_db/pgsql/network_resource_test.go index b3d07a2b4..7356831e5 100644 --- a/integration_tests/management/network_map_db/pgsql/network_resource_test.go +++ b/integration_tests/management/network_map_db/pgsql/network_resource_test.go @@ -16,15 +16,18 @@ func TestGetNetworkResources(t *testing.T) { s, err := networkmap_pgsql.NewPostgresqlStore(ctx, dsn) assert.NoError(t, err) - _, err = s.Pool.Query(ctx, + _, err = s.Pool.Exec(ctx, `insert into network_resources (id, account_id, network_id, public_id, name, description, type, domain, prefix, enabled) VALUES('net-resource-1','account-1','network-1','net-resource-public-1','network-resource-1','network-resource-1','subnet','','"10.0.0.0/16"',TRUE)`) - _, err = s.Pool.Query(ctx, + assert.NoError(t, err) + _, err = s.Pool.Exec(ctx, `insert into network_resources (id, account_id, network_id, public_id, name, description, type, domain, prefix, enabled) VALUES('net-resource-2','account-1','network-2','net-resource-public-2','network-resource-2','network-resource-2','domain','test.com','',TRUE)`) - _, err = s.Pool.Query(ctx, + assert.NoError(t, err) + _, err = s.Pool.Exec(ctx, `insert into network_resources (id, account_id, network_id, public_id, name, description, type, domain, prefix, enabled) VALUES('net-resource-3','account-1','network-3','net-resource-public-3','network-resource-3','network-resource-3','host','','"10.0.0.1/32"',TRUE)`) + assert.NoError(t, err) resources, err := s.GetNetworkResources(ctx, "account-1") assert.NoError(t, err) diff --git a/integration_tests/management/network_map_db/pgsql/network_router_test.go b/integration_tests/management/network_map_db/pgsql/network_router_test.go index 7d56f1780..1f555bb58 100644 --- a/integration_tests/management/network_map_db/pgsql/network_router_test.go +++ b/integration_tests/management/network_map_db/pgsql/network_router_test.go @@ -15,12 +15,14 @@ func TestGetNetworkRouters(t *testing.T) { s, err := networkmap_pgsql.NewPostgresqlStore(ctx, dsn) assert.NoError(t, err) - _, err = s.Pool.Query(ctx, + _, err = s.Pool.Exec(ctx, `insert into network_routers (id, account_id, public_id, peer, network_id, masquerade, metric, enabled, peer_groups) VALUES('test-nr-id-1','account-1','public-id-1','peer-id-1','network-id-1',TRUE,999,TRUE,'["group-one-resource-id"]')`) - _, err = s.Pool.Query(ctx, + assert.NoError(t, err) + _, err = s.Pool.Exec(ctx, `insert into network_routers (id, account_id, public_id, peer, network_id, masquerade, metric, enabled, peer_groups) VALUES('test-nr-id-2','account-1','public-id-2','','network-id-2',TRUE,333,TRUE,'["group-two-resources-id","group-no-resources-id"]')`) + assert.NoError(t, err) routers, err := s.GetNetworkRouters(ctx, "account-1") assert.NoError(t, err) diff --git a/integration_tests/management/network_map_db/pgsql/networks_test.go b/integration_tests/management/network_map_db/pgsql/networks_test.go index 3e09e9681..4cf6a27cd 100644 --- a/integration_tests/management/network_map_db/pgsql/networks_test.go +++ b/integration_tests/management/network_map_db/pgsql/networks_test.go @@ -11,10 +11,10 @@ import ( func TestGetNetworks(t *testing.T) { ctx := context.TODO() - _, err := pgstore.Pool.Query(ctx, + _, err := pgstore.Pool.Exec(ctx, `insert into networks (id, account_id, public_id) VALUES('network-1','account-1','network-1-public')`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into networks (id, account_id, public_id) VALUES('network-2','account-1','network-2-public')`) assert.NoError(t, err) diff --git a/integration_tests/management/network_map_db/pgsql/posture_test.go b/integration_tests/management/network_map_db/pgsql/posture_test.go index 70da08abd..4de8f64ef 100644 --- a/integration_tests/management/network_map_db/pgsql/posture_test.go +++ b/integration_tests/management/network_map_db/pgsql/posture_test.go @@ -13,7 +13,7 @@ import ( func TestGetPostureChecks(t *testing.T) { ctx := context.TODO() - _, err := pgstore.Pool.Query(ctx, + _, err := pgstore.Pool.Exec(ctx, `insert into posture_checks (id, account_id, public_id, checks) VALUES('posturecheck-1','account-1','posturecheck-1-public', '{"NBVersionCheck":{"MinVersion":"0.25.0"}, @@ -22,7 +22,7 @@ func TestGetPostureChecks(t *testing.T) { "PeerNetworkRangeCheck":{"Action":"deny","Ranges":["192.168.0.1/24"]}}')`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into posture_checks (id, account_id, public_id, checks) VALUES('posturecheck-2','account-1','posturecheck-2-public', '{"NBVersionCheck":{"MinVersion":"0.25.0"}, @@ -30,7 +30,7 @@ func TestGetPostureChecks(t *testing.T) { "GeoLocationCheck":{"Locations":[{"CountryCode":"US","CityName":"Harker Heights"}],"Action":"allow"}, "PeerNetworkRangeCheck":{"Action":"allow","Ranges":["0.0.0.0/0"]}}')`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into posture_checks (id, account_id, public_id, checks) VALUES('posturecheck-3','account-1','posturecheck-3-public', null)`) assert.NoError(t, err) diff --git a/integration_tests/management/network_map_db/pgsql/route_test.go b/integration_tests/management/network_map_db/pgsql/route_test.go index c3d3552e4..6a7287122 100644 --- a/integration_tests/management/network_map_db/pgsql/route_test.go +++ b/integration_tests/management/network_map_db/pgsql/route_test.go @@ -14,7 +14,7 @@ import ( func TestGetRoutes(t *testing.T) { ctx := context.TODO() - _, err := pgstore.Pool.Query(ctx, + _, err := pgstore.Pool.Exec(ctx, `insert into routes (id, account_id, public_id, network, domains, keep_route, net_id, description, peer, peer_groups, network_type, masquerade, metric, enabled, groups, access_control_groups, skip_auto_apply) @@ -22,7 +22,7 @@ func TestGetRoutes(t *testing.T) { 'peer-id-1','["group-one-resource-id"]',1,true,9999,true, '["group-one-resource-id"]','["group-one-resource-id"]',false)`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into routes (id, account_id, public_id, network, domains, keep_route, net_id, description, peer, peer_groups, network_type, masquerade, metric, enabled, groups, access_control_groups, skip_auto_apply) @@ -30,7 +30,7 @@ func TestGetRoutes(t *testing.T) { 'peer-id-2','["group-two-resources-id"]',1,true,9999,true, '["group-two-resources-id"]','["group-two-resources-id"]',false)`) assert.NoError(t, err) - _, err = pgstore.Pool.Query(ctx, + _, err = pgstore.Pool.Exec(ctx, `insert into routes (id, account_id, public_id, network, domains, keep_route, net_id, description, peer, peer_groups, network_type, masquerade, metric, enabled, groups, access_control_groups, skip_auto_apply) diff --git a/management/internals/network_map_db/db_store.go b/management/internals/network_map_db/db_store.go index f1ca00826..0dfa88e25 100644 --- a/management/internals/network_map_db/db_store.go +++ b/management/internals/network_map_db/db_store.go @@ -23,7 +23,7 @@ const ( NMAP_MAP_TO = "map_to" ) -type NetworkMapDBStore interface { +type NetworkMapDBStore interface { //nolint:revive // established name across the codebase GetGroups(ctx context.Context, accountId string) ([]nmdata.Group, map[string]map[string]any, error) GetPeers(ctx context.Context, accountId string) ([]nmdata.Peer, map[string][]*nmdata.Peer, error) GetPolicies(ctx context.Context, accountId string) ([]nmdata.Policy, map[string]map[string]any, map[string]map[string]any, error) @@ -40,7 +40,7 @@ type NetworkMapDBStore interface { GetDnsSettings(ctx context.Context, accountId string) (nmdata.DNSSettings, error) } -type NetworkMapDBStoreImpl struct { +type NetworkMapDBStoreImpl struct { //nolint:revive // established name across the codebase store NetworkMapDBStore integratedPeerValidator integrated_validator.IntegratedValidator extraSettingsManager settings.Manager @@ -138,7 +138,9 @@ func FromSqlTypesToSharedTypes(src reflect.Value, dst reflect.Value) error { } case "json.RawMessage": s := srcField.Interface().(json.RawMessage) - json.Unmarshal(s, dstField.Addr().Interface()) + if err := json.Unmarshal(s, dstField.Addr().Interface()); err != nil { + return err + } case "[]string": if srcField.IsNil() { return nil diff --git a/management/internals/network_map_db/pgsql/dns.go b/management/internals/network_map_db/pgsql/dns.go index 05d3e75d2..268aa4de4 100644 --- a/management/internals/network_map_db/pgsql/dns.go +++ b/management/internals/network_map_db/pgsql/dns.go @@ -15,7 +15,7 @@ import ( "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" ) -var DnsUnsupportedRecordTypeError = errors.New("unsupported record type") +var ErrDnsUnsupportedRecordType = errors.New("unsupported record type") const ( GetAccountZonesQuery = ` @@ -73,7 +73,7 @@ func GetAppliedZoneCandidatesViaPgxConnection(ctx context.Context, conn *pgx.Con rtype, rdata, err := recordTypeAndRdata(z.RecordType.String, z.RecordRData.String) if err != nil { - if errors.Is(err, DnsUnsupportedRecordTypeError) { + if errors.Is(err, ErrDnsUnsupportedRecordType) { continue } return nil, err @@ -112,7 +112,7 @@ func recordTypeAndRdata(t, rdata string) (int, string, error) { case "CNAME": return int(dns.TypeCNAME), dns.Fqdn(rdata), nil default: - return 0, "", fmt.Errorf("record type: %s %w", t, DnsUnsupportedRecordTypeError) + return 0, "", fmt.Errorf("record type: %s %w", t, ErrDnsUnsupportedRecordType) } } diff --git a/management/internals/network_map_db/pgsql/dns_test.go b/management/internals/network_map_db/pgsql/dns_test.go index 62e34e795..4a997f5f5 100644 --- a/management/internals/network_map_db/pgsql/dns_test.go +++ b/management/internals/network_map_db/pgsql/dns_test.go @@ -18,7 +18,7 @@ func TestRecordTypeAndRdata(t *testing.T) { {recordType: "AAAA", expectedRecordType: 28, rdata: "test.com", expectedRdata: "test.com", expectedErr: nil}, {recordType: "CNAME", expectedRecordType: 5, rdata: "test.com", expectedRdata: "test.com.", expectedErr: nil}, {recordType: "CNAME", expectedRecordType: 5, rdata: "test.com.", expectedRdata: "test.com.", expectedErr: nil}, - {recordType: "TypeMX", expectedErr: DnsUnsupportedRecordTypeError}, + {recordType: "TypeMX", expectedErr: ErrDnsUnsupportedRecordType}, } for _, tt := range tests { @@ -26,7 +26,7 @@ func TestRecordTypeAndRdata(t *testing.T) { recordType, rdata, err := recordTypeAndRdata(tt.recordType, tt.rdata) if tt.expectedErr != nil { - assert.ErrorIs(t, err, DnsUnsupportedRecordTypeError) + assert.ErrorIs(t, err, ErrDnsUnsupportedRecordType) return } diff --git a/management/internals/network_map_db/pgsql/group_test.go b/management/internals/network_map_db/pgsql/group_test.go index 9774747d6..156d24d08 100644 --- a/management/internals/network_map_db/pgsql/group_test.go +++ b/management/internals/network_map_db/pgsql/group_test.go @@ -2,7 +2,6 @@ package networkmap_pgsql import ( "context" - "fmt" "testing" "time" @@ -20,7 +19,7 @@ func TestGetGroups(t *testing.T) { // err = loadSQL(ctx, s.pool, initDb) //assert.NoError(t, err) - _, err = s.Pool.Query(ctx, "insert into groups (id, account_id, name, resources, public_id) VALUES('test-group-id-1','ck7bnf2t2r9s739pkug0','test-group-1', '[{\"ID\":\"cui7q2jl0ubs73d8qpi0\",\"Type\":\"host\"}]','public-id-1')") + _, err = s.Pool.Exec(ctx, "insert into groups (id, account_id, name, resources, public_id) VALUES('test-group-id-1','ck7bnf2t2r9s739pkug0','test-group-1', '[{\"ID\":\"cui7q2jl0ubs73d8qpi0\",\"Type\":\"host\"}]','public-id-1')") assert.NoError(t, err) groups, _, err := s.GetGroups(ctx, "ck7bnf2t2r9s739pkug0") //"ckd7ee2fic3c73dtendg") @@ -47,7 +46,7 @@ func TestGetPeers(t *testing.T) { assert.NoError(t, err) assert.NotEmpty(t, clusterToPeerIdx) - fmt.Print(peers) + t.Log(peers) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -71,7 +70,7 @@ func TestGetPolicies(t *testing.T) { assert.NotEmpty(t, idx1) assert.NotEmpty(t, idx2) - fmt.Print(peers) + t.Log(peers) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -93,7 +92,7 @@ func TestGetRoutes(t *testing.T) { peers, err := s.GetRoutes(ctx, "csg5iabl0ubs7398nf1g") //"ckd7ee2fic3c73dtendg") assert.NoError(t, err) - fmt.Print(peers) + t.Log(peers) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -115,7 +114,7 @@ func TestGetNSGroups(t *testing.T) { groups, err := s.GetNameServerGroups(ctx, "cl3h77qfic3c738mkja0") //"ckd7ee2fic3c73dtendg") assert.NoError(t, err) - fmt.Print(groups) + t.Log(groups) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -137,7 +136,7 @@ func TestGetNetworkResources(t *testing.T) { res, err := s.GetNetworkResources(ctx, "cag86v2t2r9s73d0416g") //"ckd7ee2fic3c73dtendg") assert.NoError(t, err) - fmt.Print(res) + t.Log(res) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -160,7 +159,7 @@ func TestGetNetworkRouters(t *testing.T) { res, err := GetNetworkRoutersViaPgxConnection(ctx, c.Conn(), "d29f99jl0ubs73cm8ce0") //"ckd7ee2fic3c73dtendg") assert.NoError(t, err) - fmt.Print(res) + t.Log(res) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -182,7 +181,7 @@ func TestGetNetwork(t *testing.T) { n, err := s.GetNetwork(ctx, "d29f99jl0ubs73cm8ce0") //"ckd7ee2fic3c73dtendg") assert.NoError(t, err) - fmt.Print(n) + t.Log(n) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -204,7 +203,7 @@ func TestGetAccountZones(t *testing.T) { zones, err := s.GetAppliedZoneCandidates(ctx, "d4g66rjl0ubs73b2q3b0") //"ckd7ee2fic3c73dtendg") assert.NoError(t, err) - fmt.Print(zones) + t.Log(zones) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, @@ -245,7 +244,7 @@ func TestGetPostureChecks(t *testing.T) { assert.NoError(t, err) assert.NotEmpty(t, idx) - fmt.Print(checks) + t.Log(checks) // assert.Contains(t, // groups, // nmdata.Group{Name: "test-group-1", PublicID: "public-id-1", Resources: []nmdata.Resource{{ID: "cui7q2jl0ubs73d8qpi0", Type: "host"}}}, diff --git a/management/internals/network_map_db/pgsql/network_map_data.go b/management/internals/network_map_db/pgsql/network_map_data.go index f8ef30f2c..a949c01c4 100644 --- a/management/internals/network_map_db/pgsql/network_map_data.go +++ b/management/internals/network_map_db/pgsql/network_map_data.go @@ -7,6 +7,8 @@ import ( "github.com/jackc/pgx/v5" "github.com/miekg/dns" + log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/shared/management/networkmap" "github.com/netbirdio/netbird/shared/management/networkmap/nmdata" ) @@ -111,9 +113,8 @@ func (pg *PgStore) GetNetworkMapData(ctx context.Context, accountId string) (*ne } } - err = tx.Commit(ctx) - if err != nil { - // TODO log and ignore? + if err = tx.Commit(ctx); err != nil { + log.WithContext(ctx).Warnf("failed to commit network map read transaction: %v", err) } toret := networkmap.NetworkMapData{ @@ -143,7 +144,7 @@ func (pg *PgStore) GetNetworkMapData(ctx context.Context, accountId string) (*ne func rollbackAndReturnError(ctx context.Context, tx pgx.Tx, err error) (*networkmap.NetworkMapData, error) { if errr := tx.Rollback(ctx); errr != nil { - // TODO log and ignore? + log.WithContext(ctx).Warnf("failed to rollback network map read transaction: %v", errr) } return nil, err } diff --git a/management/server/types/account.go b/management/server/types/account.go index b68f66e24..4f2b4f717 100644 --- a/management/server/types/account.go +++ b/management/server/types/account.go @@ -31,7 +31,6 @@ import ( ) const ( - defaultTTL = 300 // privateServiceDNSRecordTTL is short so proxy-peer changes propagate quickly to clients. privateServiceDNSRecordTTL = 5 DefaultPeerLoginExpiration = 24 * time.Hour diff --git a/shared/management/networkmap/networkmapdata.go b/shared/management/networkmap/networkmapdata.go index 272f6152a..71bb0ba96 100644 --- a/shared/management/networkmap/networkmapdata.go +++ b/shared/management/networkmap/networkmapdata.go @@ -9,7 +9,7 @@ import ( // fresh nmdata twin types. A builder converts an Account into a NetworkMapData // once per account; the per-peer components calculation then runs on this twin // with no reference back to the Account. -type NetworkMapData struct { +type NetworkMapData struct { //nolint:revive // established name across the codebase Peers map[string]*nmdata.Peer Groups map[string]*nmdata.Group Policies []*nmdata.Policy