github-vet / github-vet/rangeloop-pointer-findings
rancher/backup-restore-operator: pkg/controllers/restore/controller.go; 114 LoC
- Dominant language
- No language data
- Stars
- 0
- Forks
- 0
- PR merge metrics
- PR metrics pending
Description
Found a possible issue in [rancher/backup-restore-operator](https://www.github.com/rancher/backup-restore-operator) at [pkg/controllers/restore/controller.go](https://github.com/rancher/backup-restore-operator/blob/65da05520ac06d3a914267cc0004c03adba701fd/pkg/controllers/restore/controller.go#L357-L470)
Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first issue it finds, so please do not limit your consideration to the contents of the below message.
> reference to resourceData was used in a composite literal at line 368
[Click here to see the code in its original context.](https://github.com/rancher/backup-restore-operator/blob/65da05520ac06d3a914267cc0004c03adba701fd/pkg/controllers/restore/controller.go#L357-L470)
Click here to show the 114 line(s) of Go which triggered the analyzer.
```go
for resourceInfo, resourceData := range resourceInfoToData {
// add to adjacency list
name := resourceInfo.Name
namespace := resourceInfo.Namespace
gvr := resourceInfo.GVR
// TODO: Maybe restoreObj won't be needed
currRestoreObj := restoreObj{
Name: name,
Namespace: namespace,
ResourceConfigPath: resourceInfo.ConfigPath,
GVR: gvr,
Data: &resourceData,
}
metadata := resourceData.Object[metadataMapKey].(map[string]interface{})
ownerRefs, ownerRefsFound := metadata[ownerRefsMapKey].([]interface{})
if !ownerRefsFound {
// has no owners, so no need to add to adjacency list, add to restoreResources list
*toRestore = append(*toRestore, currRestoreObj)
continue
}
numOwners := 0
logrus.Infof("Checking ownerRefs for resource %v of type %v", name, gvr.String())
errCheckingOwnerRefs := false
for _, owner := range ownerRefs {
ownerRefData, ok := owner.(map[string]interface{})
if !ok {
errCheckingOwnerRefs = true
logrus.Errorf("Invalid ownerRef for resource %v of type %v", name, gvr.String())
continue
}
groupVersion := ownerRefData["apiVersion"].(string)
gv, err := schema.ParseGroupVersion(groupVersion)
if err != nil {
errCheckingOwnerRefs = true
logrus.Errorf("Error parsing ownerRef apiVersion %v for resource %v: %v", groupVersion, name, err)
continue
}
kind := ownerRefData["kind"].(string)
gvk := gv.WithKind(kind)
logrus.Infof("Getting GVR for ownerRef %v of resource %v", gvk.String(), name)
ownerGVR, isOwnerNamespaced, err := h.sharedClientFactory.ResourceForGVK(gvk)
if err != nil {
// Prior to Rancher 2.4.5, following resources had roles&rolebindings with malformed ownerRefs:
// Secrets for cloud creds; NodeTemplates; ClusterTemplates & Revisions; Multiclusterapps & GlobalDNS
// Kind was replaced by the resource name in plural and APIVersion field only contained the group and not version
// Error is of the kind: Kind=nodetemplates: no matches for kind "nodetemplates" in version "management.cattle.io"
// this is an invalid ownerRef, can't restore current resource with this ownerRef. But if we continue and this resource has no valid ownerRef it won't get restored
// so don't count this as owner. if the curr object has at least one valid ownerRef, it will get added to ownersToDependents list
// if not, for objects like the rancher 2.4.5 nodetemplate, check at the end of this loop if even a single ownerRef is found, if not add it to toRestore list
errCheckingOwnerRefs = true
logrus.Errorf("Invalid ownerRef %v, either of the fields is incorrect: APIVersion or Kind", gvk.String())
logrus.Errorf("Error getting ownerRef %v for object %v(of %v): %v", gvk.String(), name, gvr.String(), err)
continue
}
var apiGroup, version string
split := strings.SplitN(groupVersion, "/", 2)
if len(split) == 1 {
// resources under v1 version
version = split[0]
} else {
apiGroup = split[0]
version = split[1]
}
// kind + "." + apigroup + "#" + version
ownerDirPath := fmt.Sprintf("%s.%s#%s", ownerGVR.Resource, apiGroup, version)
ownerName := ownerRefData["name"].(string)
// Store resourceConfigPath of owner Ref because that's what we check for in "Created" map
ownerObj := restoreObj{
Name: ownerName,
ResourceConfigPath: filepath.Join(ownerDirPath, ownerName+".json"),
GVR: ownerGVR,
}
// If we are generating graph for the namespaced resources, and the ownerRef is clusterscoped, it should have been created by now
// So we can check its presence in "created" map skip adding this ownerRef to ownerToDependentsList for the current resource
if !isOwnerNamespaced {
if created[ownerObj.ResourceConfigPath] {
continue
}
}
if isOwnerNamespaced {
// if owner object is namespaced, then it has to be the same ns as the current dependent object as per k8s design
ownerObj.Namespace = currRestoreObj.Namespace
// the owner object's resourceFile in backup would also have namespace in the filename, so update
// ownerObj.ResourceConfigPath to include namespace subdir before the filename for owner
ownerFilename := filepath.Join(currRestoreObj.Namespace, ownerName+".json")
ownerObj.ResourceConfigPath = filepath.Join(ownerDirPath, ownerFilename)
}
ownerObjDependents, ok := ownerToDependentsList[ownerObj.ResourceConfigPath]
if !ok {
ownerToDependentsList[ownerObj.ResourceConfigPath] = []restoreObj{currRestoreObj}
} else {
ownerToDependentsList[ownerObj.ResourceConfigPath] = append(ownerObjDependents, currRestoreObj)
}
numOwners++
}
if numOwners > 0 {
numOwnerReferences[currRestoreObj.ResourceConfigPath] = numOwners
} else {
if !errCheckingOwnerRefs {
// owners already exist (this will happen when generating dependency graph for namespaced resources that have
// clusterscoped owners), so no need to add this namespaced resource to adjacency list, add to toRestore list
*toRestore = append(*toRestore, currRestoreObj)
continue
}
// Errors were encountered while processing ownerRefs for this object, so it should get restored without any ownerRefs,
// add it to toRestore
logrus.Warnf("Resource %v of type %v has invalid ownerRefs, adding it to restore queue by dropping the ownerRefs", name, gvr.String())
delete(currRestoreObj.Data.Object[metadataMapKey].(map[string]interface{}), ownerRefsMapKey)
*toRestore = append(*toRestore, currRestoreObj)
}
}
```
Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.
commit ID: 65da05520ac06d3a914267cc0004c03adba701fd
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.