Skip to content

Commit

Permalink
Compile the regexes once, and return an error if invalid.
Browse files Browse the repository at this point in the history 
* Further refactoring: simplify and test kind and namespace matching.

* Added tests to verify that invalid regular expressions are
  caught as recoverable error messages (and runtime testing
  showed that they were reported and backups continued).
  • Loading branch information
@ericpromislow
ericpromislow committed Feb 26, 2024
1 parent b9ca414 commit 18086f5
Show file tree
Hide file tree
Showing 3 changed files with 241 additions and 107 deletions.
168 changes: 69 additions & 99 deletions pkg/resourcesets/collector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (h *ResourceHandler) GatherResources(ctx context.Context, resourceSelectors
}

func (h *ResourceHandler) gatherResourcesForGroupVersion(filter v1.ResourceSelector) ([]k8sv1.APIResource, error) {
var resourceList, resourceListFromRegex, resourceListFromNames []k8sv1.APIResource
var resourceList []k8sv1.APIResource

groupVersion := filter.APIVersion
logrus.Infof("Gathering resources for groupVersion: %v", groupVersion)
Expand All @@ -119,71 +119,7 @@ func (h *ResourceHandler) gatherResourcesForGroupVersion(filter v1.ResourceSelec
}
return resourceList, err
}
if filter.KindsRegexp == "" && len(filter.Kinds) == 0 {
// if no filters for resource kind are given, return entire resource list
return resources.APIResources, nil
}

// "resources" list has all resources under given groupVersion, first filter based on KindsRegexp
if filter.KindsRegexp != "" {
if filter.KindsRegexp == "." {
// "." will match anything except the ones in the list of excludeKinds
for _, res := range resources.APIResources {
if isKindExcluded(filter.ExcludeKinds, res) {
continue
}
resourceListFromRegex = append(resourceListFromRegex, res)
}
return resourceListFromRegex, nil
}
// else filter out resource with regex match
for _, res := range resources.APIResources {
if isKindExcluded(filter.ExcludeKinds, res) {
continue
}
kindMatched, err := regexp.MatchString(filter.KindsRegexp, res.Kind)
if err != nil {
return resourceList, err
}
pluralNameMatched, err := regexp.MatchString(filter.KindsRegexp, res.Name)
if err != nil {
return resourceList, err
}
if !kindMatched && !pluralNameMatched {
continue
}

logrus.Debugf("resource kind %v, matched regex %v", res.Name, filter.KindsRegexp)
resourceListFromRegex = append(resourceListFromRegex, res)
}
}

// then add the resources from Kinds field to this list
if len(filter.Kinds) > 0 {
resourceListAfterRegexMatch := make(map[string]bool)
// avoid adding same resource twice by checking what's in resourceListFromRegex
for _, res := range resourceListFromRegex {
// adding in both, resource.Name (plural) and resource.Kind (singular camelcase)
resourceListAfterRegexMatch[res.Name] = true
resourceListAfterRegexMatch[res.Kind] = true
}
resourceTypesToInclude := make(map[string]bool)
for _, kind := range filter.Kinds {
resourceTypesToInclude[kind] = true
}
for _, res := range resources.APIResources {
// comparing whatever is specified in the Kinds field with both, resource Name (plural) and Kind (singular)
if resourceTypesToInclude[res.Name] || resourceTypesToInclude[res.Kind] {
if !resourceListAfterRegexMatch[res.Name] && !resourceListAfterRegexMatch[res.Kind] {
logrus.Infof("resource kind %v found in list of resources to include", res.Name)
resourceListFromNames = append(resourceListFromNames, res)
}
}
}
}
// combine both lists, obtained from regex and from iterating over the kinds list
resourceList = append(resourceListFromRegex, resourceListFromNames...)
return resourceList, nil
return h.filterByKind(filter, resources.APIResources)
}

func (h *ResourceHandler) gatherObjectsForResource(ctx context.Context, res k8sv1.APIResource, gv schema.GroupVersion, filter v1.ResourceSelector) ([]unstructured.Unstructured, error) {
Expand Down Expand Up @@ -221,12 +157,50 @@ func (h *ResourceHandler) filterByLabel(ctx context.Context, dr dynamic.Resource
logrus.Debugf("Listing objects using label selector %v", labelSelector)
}

resourceObjectsList, err := unrollPaginatedListResult(ctx, dr, k8sv1.ListOptions{LabelSelector: labelSelector})
if err != nil {
return nil, err
return unrollPaginatedListResult(ctx, dr, k8sv1.ListOptions{LabelSelector: labelSelector})
}

func (h *ResourceHandler) filterByKind(filter v1.ResourceSelector, apiResources []k8sv1.APIResource) ([]k8sv1.APIResource, error) {
var resourceList []k8sv1.APIResource
var kindRegexp *regexp.Regexp
if filter.KindsRegexp == "" && len(filter.Kinds) == 0 {
// if no filters for resource kind are given, return entire resource list
return apiResources, nil
}
if filter.KindsRegexp != "" {
var err error
kindRegexp, err = regexp.Compile(filter.KindsRegexp)
if err != nil {
return nil, fmt.Errorf("error in kindsRegexp pattern %s: %w", filter.KindsRegexp, err)
}
}
allowedKinds := make(map[string]bool)
disallowedKinds := make(map[string]bool)
for _, name := range filter.Kinds {
allowedKinds[name] = true
}
for _, name := range filter.ExcludeKinds {
disallowedKinds[name] = true
}

return resourceObjectsList, nil
// "resources" list has all resources under given groupVersion, first filter based on KindsRegexp
// Look for a match in either `Kind` (singular name) or `Name` (plural name).
// If we match by regexp, we need to consider exclusions by both `Kind` and `Name`
// This means we can regexp-match on `Name` but will exclude due to matching on `Kind`, for example
for _, resObj := range apiResources {
if allowedKinds[resObj.Kind] || allowedKinds[resObj.Name] {
resourceList = append(resourceList, resObj)
continue
}
if kindRegexp == nil {
continue
}
if (filter.KindsRegexp == "." || kindRegexp.MatchString(resObj.Kind) || kindRegexp.MatchString(resObj.Name)) &&
!disallowedKinds[resObj.Kind] && !disallowedKinds[resObj.Name] {
resourceList = append(resourceList, resObj)
}
}
return resourceList, nil
}

func (h *ResourceHandler) filterByName(filter v1.ResourceSelector, resourceObjectsList []unstructured.Unstructured) ([]unstructured.Unstructured, error) {
Expand All @@ -244,31 +218,37 @@ func (h *ResourceHandler) filterByName(filter v1.ResourceSelector, resourceObjec
}

var includeRegex *regexp.Regexp
var err error
hasRegEx := false
if filter.ResourceNameRegexp != "" {
includeRegex = regexp.MustCompile(filter.ResourceNameRegexp)
includeRegex, err = regexp.Compile(filter.ResourceNameRegexp)
if err != nil {
return nil, fmt.Errorf("error in resource-name pattern %s: %w", filter.ResourceNameRegexp, err)
}
hasRegEx = true
}
var excludeRegex *regexp.Regexp
if filter.ExcludeResourceNameRegexp != "" {
excludeRegex = regexp.MustCompile(filter.ExcludeResourceNameRegexp)
excludeRegex, err = regexp.Compile(filter.ExcludeResourceNameRegexp)
if err != nil {
return nil, fmt.Errorf("error in exclude-resource-name pattern %s: %w", filter.ResourceNameRegexp, err)
}
hasRegEx = true
}

for resObjID := range resourceObjectsList {
for _, resObj := range resourceObjectsList {
// Check if the name is in the includeNames list
name := resourceObjectsList[resObjID].GetName()
name := resObj.GetName()
if includeNameMap[name] {
filteredByName = append(filteredByName, resourceObjectsList[resObjID])
filteredByName = append(filteredByName, resObj)
logrus.Debugf("Including [%s] because it matched a name in ResourceNames", name)
} else {
if filter.ResourceNameRegexp != "" || filter.ExcludeResourceNameRegexp != "" {
includeMatch := includeRegex == nil || includeRegex.MatchString(name)

if includeMatch {
// ExcludeResourceNameRegexp can override ResourceNameRegexp, if given and it matches
excludeMatch := excludeRegex != nil && excludeRegex.MatchString(name)
if !excludeMatch {
filteredByName = append(filteredByName, resourceObjectsList[resObjID])
logrus.Debugf("Including [%s] because it matched ResourceNameRegexp [%s] and didn't match ExcludeResourceNameRegexp [%s]", name, filter.ResourceNameRegexp, filter.ExcludeResourceNameRegexp)
}
} else if hasRegEx {
includeMatch := includeRegex == nil || includeRegex.MatchString(name)
if includeMatch {
excludeMatch := excludeRegex != nil && excludeRegex.MatchString(name)
if !excludeMatch {
filteredByName = append(filteredByName, resObj)
logrus.Debugf("Including [%s] because it matched ResourceNameRegexp [%s] and didn't match ExcludeResourceNameRegexp [%s]", name, filter.ResourceNameRegexp, filter.ExcludeResourceNameRegexp)
}
}
}
Expand All @@ -289,7 +269,7 @@ func (h *ResourceHandler) filterByNamespace(filter v1.ResourceSelector, filtered
logrus.Debugf("Using NamespaceRegexp %s to filter resource names", filter.NamespaceRegexp)
namespaceRegexp, err = regexp.Compile(filter.NamespaceRegexp)
if err != nil {
return nil, err
return nil, fmt.Errorf("error in namespace pattern %s: %w", filter.NamespaceRegexp, err)
}
}
allowedNamespaces := make(map[string]bool)
Expand Down Expand Up @@ -353,7 +333,7 @@ func (h *ResourceHandler) gatherObjectsForNonListResource(ctx context.Context, r
for _, name := range filter.ResourceNames {
obj, err := dr.Get(ctx, name, k8sv1.GetOptions{})
if err != nil {
return gatheredObjects, err
return nil, err
}
gatheredObjects = append(gatheredObjects, *obj)
}
Expand Down Expand Up @@ -454,7 +434,7 @@ func writeToBackup(ctx context.Context, resource map[string]interface{}, backupP
return fmt.Errorf("error converting resource to JSON: %v", err)
}
if transformer != nil {
encrypted, err := transformer.TransformToStorage(ctx, resourceBytes, value.DefaultContext([]byte(additionalAuthenticatedData)))
encrypted, err := transformer.TransformToStorage(ctx, resourceBytes, value.DefaultContext(additionalAuthenticatedData))
if err != nil {
return fmt.Errorf("error converting resource to JSON: %v", err)
}
Expand Down Expand Up @@ -490,13 +470,3 @@ func canGetResource(verbs k8sv1.Verbs) bool {
}
return false
}

func isKindExcluded(excludes []string, res k8sv1.APIResource) bool {
for _, exclude := range excludes {
if exclude == res.Name || exclude == res.Kind {
return true
}
}

return false
}
Loading

0 comments on commit 18086f5

Please sign in to comment.