Skip to content

Commit

Permalink
fix: avoiding possible data races when accessing shared maps
Browse files Browse the repository at this point in the history
  • Loading branch information
bl4ko committed Aug 22, 2024
1 parent 7d59906 commit be4e7e6
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 18 deletions.
54 changes: 54 additions & 0 deletions internal/netbox/inventory/get_items.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,57 @@ func (nbi *NetboxInventory) GetVlan(vlanGroupID int, vlanID int) *objects.Vlan {
}
return nbi.VlansIndexByVlanGroupIDAndVID[vlanGroupID][vlanID]
}

func (nbi *NetboxInventory) GetTenant(tenantName string) (*objects.Tenant, bool) {
nbi.TenantsLock.Lock()
defer nbi.TenantsLock.Unlock()
if _, ok := nbi.TenantsIndexByName[tenantName]; !ok {
return nil, false

Check warning on line 21 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L17-L21

Added lines #L17 - L21 were not covered by tests
}
return nbi.TenantsIndexByName[tenantName], true

Check warning on line 23 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L23

Added line #L23 was not covered by tests
}

func (nbi *NetboxInventory) GetSite(siteName string) (*objects.Site, bool) {
nbi.SitesLock.Lock()
defer nbi.SitesLock.Unlock()
if _, ok := nbi.SitesIndexByName[siteName]; !ok {
return nil, false

Check warning on line 30 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L26-L30

Added lines #L26 - L30 were not covered by tests
}
return nbi.SitesIndexByName[siteName], true

Check warning on line 32 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L32

Added line #L32 was not covered by tests
}

func (nbi *NetboxInventory) GetVlanGroup(vlanGroupName string) (*objects.VlanGroup, bool) {
nbi.VlanGroupsLock.Lock()
defer nbi.VlanGroupsLock.Unlock()
if _, ok := nbi.VlanGroupsIndexByName[vlanGroupName]; !ok {
return nil, false

Check warning on line 39 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L35-L39

Added lines #L35 - L39 were not covered by tests
}
return nbi.VlanGroupsIndexByName[vlanGroupName], true

Check warning on line 41 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L41

Added line #L41 was not covered by tests
}

func (nbi *NetboxInventory) GetClusterGroup(clusterGroupName string) (*objects.ClusterGroup, bool) {
nbi.ClusterGroupsLock.Lock()
defer nbi.ClusterGroupsLock.Unlock()
if _, ok := nbi.ClusterGroupsIndexByName[clusterGroupName]; !ok {
return nil, false

Check warning on line 48 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L44-L48

Added lines #L44 - L48 were not covered by tests
}
return nbi.ClusterGroupsIndexByName[clusterGroupName], true

Check warning on line 50 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L50

Added line #L50 was not covered by tests
}

func (nbi *NetboxInventory) GetCluster(clusterName string) (*objects.Cluster, bool) {
nbi.ClustersLock.Lock()
defer nbi.ClustersLock.Unlock()
if _, ok := nbi.ClustersIndexByName[clusterName]; !ok {
return nil, false

Check warning on line 57 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L53-L57

Added lines #L53 - L57 were not covered by tests
}
return nbi.ClustersIndexByName[clusterName], true

Check warning on line 59 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L59

Added line #L59 was not covered by tests
}

func (nbi *NetboxInventory) GetDevice(deviceName string, siteID int) (*objects.Device, bool) {
nbi.DevicesLock.Lock()
defer nbi.DevicesLock.Unlock()
if device, ok := nbi.DevicesIndexByNameAndSiteID[deviceName][siteID]; ok {
return device, true

Check warning on line 66 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L62-L66

Added lines #L62 - L66 were not covered by tests
}
return nil, false

Check warning on line 68 in internal/netbox/inventory/get_items.go

View check run for this annotation

Codecov / codecov/patch

internal/netbox/inventory/get_items.go#L68

Added line #L68 was not covered by tests
}
23 changes: 13 additions & 10 deletions internal/source/common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func MatchClusterToTenant(ctx context.Context, nbi *inventory.NetboxInventory, c
return nil, fmt.Errorf("matching cluster to tenant: %s", err)
}
if tenantName != "" {
tenant, ok := nbi.TenantsIndexByName[tenantName]
tenant, ok := nbi.GetTenant(tenantName)

Check warning on line 25 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L25

Added line #L25 was not covered by tests
if !ok {
tenant, err := nbi.AddTenant(ctx, &objects.Tenant{
Name: tenantName,
Expand Down Expand Up @@ -50,7 +50,7 @@ func MatchClusterToSite(ctx context.Context, nbi *inventory.NetboxInventory, clu
return nil, fmt.Errorf("matching cluster to tenant: %s", err)
}
if siteName != "" {
site, ok := nbi.SitesIndexByName[siteName]
site, ok := nbi.GetSite(siteName)

Check warning on line 53 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L53

Added line #L53 was not covered by tests
if !ok {
newSite, err := nbi.AddSite(ctx, &objects.Site{
Name: siteName,
Expand All @@ -71,14 +71,15 @@ func MatchClusterToSite(ctx context.Context, nbi *inventory.NetboxInventory, clu
// In case there is no match or regexRelations is nil, it will return default VlanGroup.
func MatchVlanToGroup(ctx context.Context, nbi *inventory.NetboxInventory, vlanName string, regexRelations map[string]string) (*objects.VlanGroup, error) {
if regexRelations == nil {
return nbi.VlanGroupsIndexByName[objects.DefaultVlanGroupName], nil
vlanGroup, _ := nbi.GetVlanGroup(objects.DefaultVlanGroupName)
return vlanGroup, nil

Check warning on line 75 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L74-L75

Added lines #L74 - L75 were not covered by tests
}
vlanGroupName, err := utils.MatchStringToValue(vlanName, regexRelations)
if err != nil {
return nil, fmt.Errorf("matching vlan to group: %s", err)
}
if vlanGroupName != "" {
vlanGroup, ok := nbi.VlanGroupsIndexByName[vlanGroupName]
vlanGroup, ok := nbi.GetVlanGroup(vlanGroupName)

Check warning on line 82 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L82

Added line #L82 was not covered by tests
if !ok {
// Create vlanGroup
vlanGroup, err = nbi.AddVlanGroup(ctx, &objects.VlanGroup{
Expand All @@ -94,7 +95,8 @@ func MatchVlanToGroup(ctx context.Context, nbi *inventory.NetboxInventory, vlanN
return vlanGroup, nil
}

return nbi.VlanGroupsIndexByName[objects.DefaultVlanGroupName], nil
vlanGroup, _ := nbi.GetVlanGroup(objects.DefaultVlanGroupName)
return vlanGroup, nil

Check warning on line 99 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L98-L99

Added lines #L98 - L99 were not covered by tests
}

// Function that matches vlanName to tenant using vlanTenantRelations regex relations map.
Expand All @@ -109,7 +111,7 @@ func MatchVlanToTenant(ctx context.Context, nbi *inventory.NetboxInventory, vlan
return nil, fmt.Errorf("matching vlan to tenant: %s", err)
}
if tenantName != "" {
tenant, ok := nbi.TenantsIndexByName[tenantName]
tenant, ok := nbi.GetTenant(tenantName)

Check warning on line 114 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L114

Added line #L114 was not covered by tests
if !ok {
tenant, err := nbi.AddTenant(ctx, &objects.Tenant{
Name: tenantName,
Expand Down Expand Up @@ -138,7 +140,7 @@ func MatchHostToSite(ctx context.Context, nbi *inventory.NetboxInventory, hostNa
return nil, fmt.Errorf("matching host to site: %s", err)
}
if siteName != "" {
site, ok := nbi.SitesIndexByName[siteName]
site, ok := nbi.GetSite(siteName)

Check warning on line 143 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L143

Added line #L143 was not covered by tests
if !ok {
newSite, err := nbi.AddSite(ctx, &objects.Site{
Name: siteName,
Expand All @@ -151,7 +153,8 @@ func MatchHostToSite(ctx context.Context, nbi *inventory.NetboxInventory, hostNa
}
return site, nil
}
return nbi.SitesIndexByName[constants.DefaultSite], nil
site, _ := nbi.GetSite(constants.DefaultSite)
return site, nil

Check warning on line 157 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L156-L157

Added lines #L156 - L157 were not covered by tests
}

// Function that matches Host from hostName to Tenant using hostTenantRelations.
Expand All @@ -166,7 +169,7 @@ func MatchHostToTenant(ctx context.Context, nbi *inventory.NetboxInventory, host
return nil, fmt.Errorf("matching host to tenant: %s", err)
}
if tenantName != "" {
site, ok := nbi.TenantsIndexByName[tenantName]
site, ok := nbi.GetTenant(tenantName)

Check warning on line 172 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L172

Added line #L172 was not covered by tests
if !ok {
tenant, err := nbi.AddTenant(ctx, &objects.Tenant{
Name: tenantName,
Expand Down Expand Up @@ -194,7 +197,7 @@ func MatchVMToTenant(ctx context.Context, nbi *inventory.NetboxInventory, vmName
return nil, fmt.Errorf("matching vm to tenant: %s", err)
}
if tenantName != "" {
site, ok := nbi.TenantsIndexByName[tenantName]
site, ok := nbi.GetTenant(tenantName)

Check warning on line 200 in internal/source/common/utils.go

View check run for this annotation

Codecov / codecov/patch

internal/source/common/utils.go#L200

Added line #L200 was not covered by tests
if !ok {
tenant, err := nbi.AddTenant(ctx, &objects.Tenant{
Name: tenantName,
Expand Down
10 changes: 5 additions & 5 deletions internal/source/ovirt/ovirt_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (o *OVirtSource) syncClusters(nbi *inventory.NetboxInventory) error {
if mappedName, ok := o.DatacenterClusterGroupRelations[clusterGroupName]; ok {
clusterGroupName = mappedName
}
clusterGroup = nbi.ClusterGroupsIndexByName[clusterGroupName]
clusterGroup, _ = nbi.GetClusterGroup(clusterGroupName)

Check warning on line 129 in internal/source/ovirt/ovirt_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/ovirt/ovirt_sync.go#L129

Added line #L129 was not covered by tests
}

clusterSite, err := common.MatchClusterToSite(o.Ctx, nbi, clusterName, o.ClusterSiteRelations)
Expand Down Expand Up @@ -170,7 +170,7 @@ func (o *OVirtSource) syncHosts(nbi *inventory.NetboxInventory) error {
if !exists {
o.Logger.Warningf(o.Ctx, "name of host with id=%s is empty", hostID)
}
hostCluster := nbi.ClustersIndexByName[o.Clusters[host.MustCluster().MustId()].MustName()]
hostCluster, _ := nbi.GetCluster(o.Clusters[host.MustCluster().MustId()].MustName())

Check warning on line 173 in internal/source/ovirt/ovirt_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/ovirt/ovirt_sync.go#L173

Added line #L173 was not covered by tests

hostSite, err := common.MatchHostToSite(o.Ctx, nbi, hostName, o.HostSiteRelations)
if err != nil {
Expand Down Expand Up @@ -706,7 +706,7 @@ func (o *OVirtSource) extractVMData(nbi *inventory.NetboxInventory, vmID string,
cluster, exists := vm.Cluster()
if exists {
if _, ok := o.Clusters[cluster.MustId()]; ok {
vmCluster = nbi.ClustersIndexByName[o.Clusters[cluster.MustId()].MustName()]
vmCluster, _ = nbi.GetCluster(o.Clusters[cluster.MustId()].MustName())

Check warning on line 709 in internal/source/ovirt/ovirt_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/ovirt/ovirt_sync.go#L709

Added line #L709 was not covered by tests
}
}

Expand Down Expand Up @@ -737,7 +737,7 @@ func (o *OVirtSource) extractVMData(nbi *inventory.NetboxInventory, vmID string,
if host, exists := vm.Host(); exists {
if oHost, ok := o.Hosts[host.MustId()]; ok {
if oHostName, ok := oHost.Name(); ok {
vmHostDevice = nbi.DevicesIndexByNameAndSiteID[oHostName][vmSite.ID]
vmHostDevice, _ = nbi.GetDevice(oHostName, vmSite.ID)

Check warning on line 740 in internal/source/ovirt/ovirt_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/ovirt/ovirt_sync.go#L740

Added line #L740 was not covered by tests
}
}
}
Expand Down Expand Up @@ -1017,7 +1017,7 @@ func (o *OVirtSource) syncVMNics(nbi *inventory.NetboxInventory, ovirtVM *ovirts
o.Logger.Warningf(o.Ctx, "match vlan to group: %s", err)
continue
}
nicVlans = []*objects.Vlan{nbi.VlansIndexByVlanGroupIDAndVID[vlanGroup.ID][int(vlanID)]}
nicVlans = []*objects.Vlan{nbi.GetVlan(vlanGroup.ID, int(vlanID))}

Check warning on line 1020 in internal/source/ovirt/ovirt_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/ovirt/ovirt_sync.go#L1020

Added line #L1020 was not covered by tests
nicMode = &objects.VMInterfaceModeTagged
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/source/vmware/vmware_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (vc *VmwareSource) syncClusters(nbi *inventory.NetboxInventory) error {
if mappedName, ok := vc.DatacenterClusterGroupRelations[clusterGroupName]; ok {
clusterGroupName = mappedName
}
clusterGroup = nbi.ClusterGroupsIndexByName[clusterGroupName]
clusterGroup, _ = nbi.GetClusterGroup(clusterGroupName)

Check warning on line 98 in internal/source/vmware/vmware_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/vmware/vmware_sync.go#L98

Added line #L98 was not covered by tests

clusterSite, err := common.MatchClusterToSite(vc.Ctx, nbi, clusterName, vc.ClusterSiteRelations)
if err != nil {
Expand Down Expand Up @@ -146,7 +146,7 @@ func (vc *VmwareSource) syncHosts(nbi *inventory.NetboxInventory) error {
return fmt.Errorf("hostTenant: %s", err)
}

hostCluster := nbi.ClustersIndexByName[vc.Clusters[vc.Host2Cluster[hostID]].Name]
hostCluster, _ := nbi.GetCluster(vc.Clusters[vc.Host2Cluster[hostID]].Name)
if hostCluster == nil {

Check warning on line 150 in internal/source/vmware/vmware_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/vmware/vmware_sync.go#L149-L150

Added lines #L149 - L150 were not covered by tests
// Create a hypothetical cluster https://github.com/bl4ko/netbox-ssot/issues/141
hostCluster, err = vc.createHypotheticalCluster(nbi, hostName, hostSite, hostTenant)
Expand Down Expand Up @@ -736,7 +736,7 @@ func (vc *VmwareSource) syncVM(nbi *inventory.NetboxInventory, vmKey string, vm
if err != nil {
return fmt.Errorf("vm's Site: %s", err)
}
vmHost := nbi.DevicesIndexByNameAndSiteID[vmHostName][vmSite.ID]
vmHost, _ := nbi.GetDevice(vmHostName, vmSite.ID)

Check warning on line 739 in internal/source/vmware/vmware_sync.go

View check run for this annotation

Codecov / codecov/patch

internal/source/vmware/vmware_sync.go#L739

Added line #L739 was not covered by tests

// Cluster of the vm is same as the host
vmCluster := vmHost.Cluster
Expand Down

0 comments on commit be4e7e6

Please sign in to comment.