Fix the typecheck error reported by the lint GitHub action.

Signed-off-by: Xun Jiang <[email protected]>

.github/workflows/pr-linter-check.yml

@@ -7,8 +7,16 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
+    - name: Set up Go
+      uses: actions/setup-go@v5
+      with:
+        go-version: '1.22'
+
     - name: Check out the code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v4
 
-    - name: Linter check
-      run: make lint
+    - name: golangci-lint
+      uses: golangci/golangci-lint-action@v4
+      with:
+        version: v1.57.2
+        args: --out-format=colored-line-number

golangci.yaml → .golangci.yaml

@@ -12,14 +12,6 @@ run:
   # exit code when at least one issue was found, default is 1
   issues-exit-code: 1
 
-  # which dirs to skip: issues from them won't be reported;
-  # can use regexp here: generated.*, regexp is applied on full path;
-  # default value is empty list, but default dirs are skipped independently
-  # from this option's value (see skip-dirs-use-default).
-  # "/" will be replaced by current OS file path separator to properly work
-  # on Windows.
-  skip-dirs:
-    - pkg/plugin/generated/*
 
   # default is true. Enables skipping of directories:
   #   vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
@@ -44,7 +36,7 @@ run:
 # output configuration options
 output:
   # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
-  format: colored-line-number
+  formats: colored-line-number
 
   # print lines of code with issue, default is true
   print-issued-lines: true
@@ -148,10 +140,8 @@ linters-settings:
     # minimal confidence for issues, default is 0.8
     min-confidence: 0.8
   gomnd:
-    settings:
-      mnd:
-        # the list of enabled checks, see https://github.com/tommy-muehle/go-mnd/#checks for description.
-        checks: argument,case,condition,operation,return,assign
+    # the list of enabled checks, see https://github.com/tommy-muehle/go-mnd/#checks for description.
+    checks: argument,case,condition,operation,return,assign
   gomodguard:
     allowed:
       modules:                                                        # List of allowed modules
@@ -389,6 +379,15 @@ issues:
   # Show only new issues created after git revision `REV`
   # new-from-rev: origin/main
 
+  # which dirs to skip: issues from them won't be reported;
+  # can use regexp here: generated.*, regexp is applied on full path;
+  # default value is empty list, but default dirs are skipped independently
+  # from this option's value (see skip-dirs-use-default).
+  # "/" will be replaced by current OS file path separator to properly work
+  # on Windows.
+  exclude-dirs:
+    - pkg/plugin/generated/*
+
 severity:
   # Default value is empty string.
   # Set the default severity for issues. If severity rules are defined and the issues 

changelogs/unreleased/7697-blackpiglet

@@ -1 +1 @@
-Modify namespace filter logic for backup with label selector
+When Included/ExcludedNamespaces are omitted, and LabelSelector or OrLabelSelector is used, namespaces without selected items are excluded from backup.

hack/build-image/Dockerfile

@@ -93,7 +93,7 @@ RUN ARCH=$(go env GOARCH) && \
     chmod +x /usr/bin/goreleaser
 
 # get golangci-lint
-RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.0
+RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.57.2
 
 # install kubectl
 RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/$(go env GOARCH)/kubectl

hack/lint.sh

@@ -14,14 +14,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-HACK_DIR=$(dirname "${BASH_SOURCE[0]}")
-
 # Printing out cache status
 golangci-lint cache status
 
 # Enable GL_DEBUG line below for debug messages for golangci-lint
 # export GL_DEBUG=loader,gocritic,env
-CMD="golangci-lint run -c $HACK_DIR/../golangci.yaml"
+CMD="golangci-lint run"
 echo "Running $CMD"
 
 eval $CMD

internal/hook/item_hook_handler_test.go

@@ -1976,7 +1976,7 @@ func TestValidateContainer(t *testing.T) {
 	expectedError := fmt.Errorf("invalid InitContainer in restore hook, it doesn't have Command, Name or Image field")
 
 	// valid string should return nil as result.
-	assert.Equal(t, nil, ValidateContainer([]byte(valid)))
+	assert.Nil(t, ValidateContainer([]byte(valid)))
 
 	// noName string should return expected error as result.
 	assert.Equal(t, expectedError, ValidateContainer([]byte(noName)))

pkg/backup/backup_test.go

@@ -4334,8 +4334,7 @@ func TestBackupNamespaces(t *testing.T) {
 			backup: defaultBackup().OrLabelSelector([]*metav1.LabelSelector{
 				{MatchLabels: map[string]string{"a": "b"}},
 				{MatchLabels: map[string]string{"c": "d"}},
-			}).
-				Result(),
+			}).Result(),
 			apiResources: []*test.APIResource{
 				test.Namespaces(
 					builder.ForNamespace("ns-1").Result(),
@@ -4379,6 +4378,27 @@ func TestBackupNamespaces(t *testing.T) {
 				"resources/deployments.apps/v1-preferredversion/namespaces/ns-1/deploy-1.json",
 			},
 		},
+		{
+			name: "LabelSelector and Namespace exclude filtering test",
+			backup: defaultBackup().ExcludedNamespaces("ns-1", "ns-2").LabelSelector(&metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}}).
+				Result(),
+			apiResources: []*test.APIResource{
+				test.Namespaces(
+					builder.ForNamespace("ns-1").ObjectMeta(builder.WithLabels("a", "b")).Result(),
+					builder.ForNamespace("ns-2").Result(),
+					builder.ForNamespace("ns-3").Result(),
+				),
+				test.Deployments(
+					builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("a", "b")).Result(),
+				),
+			},
+			want: []string{
+				"resources/namespaces/cluster/ns-1.json",
+				"resources/namespaces/v1-preferredversion/cluster/ns-1.json",
+				"resources/namespaces/cluster/ns-3.json",
+				"resources/namespaces/v1-preferredversion/cluster/ns-3.json",
+			},
+		},
 		{
 			name:   "Empty namespace test",
 			backup: defaultBackup().IncludedNamespaces("invalid*").Result(),

pkg/backup/item_collector.go

@@ -55,9 +55,24 @@ type itemCollector struct {
 }
 
 // nsTracker is used to integrate several namespace filters together.
-//  1. backup's namespace include and exclude filters;
-//  2. backup's LabelSelector and OrLabelSelector selected namespace;
-//  3. resources namespaces selected by label selectors.
+//  1. Backup's namespace Include/Exclude filters;
+//  2. Backup's (Or)LabelSelector selected namespace;
+//  3. Backup's (Or)LabelSelector selected resources' namespaces.
+//
+// Rules:
+//
+//	a. When backup has default namespace Include/Exclude filters,
+//	The namespaces, which do not have backup including resources,
+//	are not collected.
+//
+//	b. If the namespace I/E filters and the (Or)LabelSelectors have
+//	different results. The tracker takes the union of them.
+//
+//	c. If the namespace I/E filters and the (Or)LabelSelectors selected
+//	namespaces are different. The tracker takes the union of them.
+//
+//	d. Namespace I/E filters should always have the same result with
+//	(Or)LabelSelector selected resources' namespaces.
 type nsTracker struct {
 	singleLabelSelector labels.Selector
 	orLabelSelector     []labels.Selector
@@ -67,6 +82,7 @@ type nsTracker struct {
 	namespaceMap map[string]bool
 }
 
+// track add the namespace into the namespaceMap.
 func (nt *nsTracker) track(ns string) {
 	if nt.namespaceMap == nil {
 		nt.namespaceMap = make(map[string]bool)
@@ -77,76 +93,86 @@ func (nt *nsTracker) track(ns string) {
 	}
 }
 
+// isTracked check whether the namespace's name exists in
+// namespaceMap.
 func (nt *nsTracker) isTracked(ns string) bool {
 	if nt.namespaceMap != nil {
 		return nt.namespaceMap[ns]
 	}
 	return false
 }
 
+// init initialize the namespaceMap, and add elements according to
+// namespace include/exclude filters and the backup label selectors.
 func (nt *nsTracker) init(
 	unstructuredNSs []unstructured.Unstructured,
 	singleLabelSelector labels.Selector,
 	orLabelSelector []labels.Selector,
 	namespaceFilter *collections.IncludesExcludes,
 	logger logrus.FieldLogger,
 ) {
-	nt.namespaceMap = make(map[string]bool)
+	if nt.namespaceMap == nil {
+		nt.namespaceMap = make(map[string]bool)
+	}
 	nt.singleLabelSelector = singleLabelSelector
 	nt.orLabelSelector = orLabelSelector
 	nt.namespaceFilter = namespaceFilter
 	nt.logger = logger
 
 	for _, namespace := range unstructuredNSs {
-
 		if nt.singleLabelSelector != nil &&
-			nt.singleLabelSelector.Matches(labels.Set(namespace.GetLabels())) == true {
-			nt.logger.Debugf("Track namespace %s,", namespace.GetName(),
-				"because its labels match backup LabelSelector.")
+			nt.singleLabelSelector.Matches(labels.Set(namespace.GetLabels())) {
+			nt.logger.Debugf(`Track namespace %s, 
+				because its labels match backup LabelSelector.`,
+				namespace.GetName(),
+			)
 
 			nt.track(namespace.GetName())
 			continue
 		}
 
 		if len(nt.orLabelSelector) > 0 {
 			for _, selector := range nt.orLabelSelector {
-				if selector.Matches(labels.Set(namespace.GetLabels())) == true {
-					nt.logger.Debugf("Track namespace %s", namespace.GetName(),
-						"because its labels match the backup OrLabelSelector.")
+				if selector.Matches(labels.Set(namespace.GetLabels())) {
+					nt.logger.Debugf(`Track namespace %s",
+						"because its labels match the backup OrLabelSelector.`,
+						namespace.GetName(),
+					)
 					nt.track(namespace.GetName())
 					continue
 				}
 			}
 		}
 
 		// Skip the backup when the backup's namespace filter has
-		// default value(*), and the namespace doesn't match backup
+		// default value, and the namespace doesn't match backup
 		// LabelSelector and OrLabelSelector.
 		// https://github.com/vmware-tanzu/velero/issues/7105
-		if nt.namespaceFilter.ShouldInclude("*") == true &&
+		if collections.ValueIsDefault(nt.namespaceFilter) &&
 			(nt.singleLabelSelector != nil || len(nt.orLabelSelector) > 0) {
-			nt.logger.Debugf("Skip namespace %s,", namespace.GetName(),
-				"because backup's namespace filter is *, and backup has label selector.")
 			continue
 		}
 
-		if nt.namespaceFilter.ShouldInclude(namespace.GetName()) == true {
-			nt.logger.Debugf("Track namespace %s", namespace.GetName(),
-				"because its name match the backup namespace filter.")
+		if nt.namespaceFilter.ShouldInclude(namespace.GetName()) {
+			nt.logger.Debugf(`Track namespace %s,
+				because its name match the backup namespace filter.`,
+				namespace.GetName(),
+			)
 			nt.track(namespace.GetName())
 		}
 	}
 }
 
-// filterNamespaces only keeps namespace tracked by the nsTracker
+// filterNamespaces filters the input resource list to remove the
+// namespaces not tracked by the nsTracker.
 func (nt *nsTracker) filterNamespaces(
 	resources []*kubernetesResource,
 ) []*kubernetesResource {
 	result := make([]*kubernetesResource, 0)
 
 	for _, resource := range resources {
 		if resource.groupResource != kuberesource.Namespaces ||
-			nt.isTracked(resource.name) == true {
+			nt.isTracked(resource.name) {
 			result = append(result, resource)
 		}
 	}
@@ -160,8 +186,8 @@ type kubernetesResource struct {
 	namespace, name, path string
 }
 
-// getItemsFromResourceIdentifiers converts ResourceIdentifiers to
-// kubernetesResources
+// getItemsFromResourceIdentifiers get the kubernetesResources
+// specified by the input parameter resourceIDs.
 func (r *itemCollector) getItemsFromResourceIdentifiers(
 	resourceIDs []velero.ResourceIdentifier,
 ) []*kubernetesResource {
@@ -173,16 +199,20 @@ func (r *itemCollector) getItemsFromResourceIdentifiers(
 	return r.getItems(grResourceIDsMap)
 }
 
-// getAllItems gets all relevant items from all API groups.
+// getAllItems gets all backup-relevant items from all API groups.
 func (r *itemCollector) getAllItems() []*kubernetesResource {
 	resources := r.getItems(nil)
 
 	return r.nsTracker.filterNamespaces(resources)
 }
 
-// getItems gets all relevant items from all API groups.
+// getItems gets all backup-relevant items from all API groups,
+//
 // If resourceIDsMap is nil, then all items from the cluster are
-// pulled for each API group, subject to include/exclude rules.
+// pulled for each API group, subject to include/exclude rules,
+// except the namespace, because the namespace filtering depends on
+// all namespaced-scoped resources.
+//
 // If resourceIDsMap is supplied, then only those resources are
 // returned, with the appropriate APIGroup information filled in. In
 // this case, include/exclude rules are not invoked, since we already
@@ -409,10 +439,10 @@ func (r *itemCollector) getResourceItems(
 	}
 
 	// Handle namespace resource here.
-	// Namespace are only filtered by namespace include/exclude filters.
-	// Label selectors are not checked.
+	// Namespace are filtered by namespace include/exclude filters,
+	// backup LabelSelectors and OrLabelSelectors are checked too.
 	if gr == kuberesource.Namespaces {
-		return r.backupNamespaces(
+		return r.collectNamespaces(
 			resource,
 			gv,
 			gr,
@@ -697,8 +727,8 @@ func (r *itemCollector) listItemsForLabel(
 	return unstructuredItems, nil
 }
 
-// backupNamespaces process namespace resource according to namespace filters.
-func (r *itemCollector) backupNamespaces(
+// collectNamespaces process namespace resource according to namespace filters.
+func (r *itemCollector) collectNamespaces(
 	resource metav1.APIResource,
 	gv schema.GroupVersion,
 	gr schema.GroupResource,
@@ -750,17 +780,18 @@ func (r *itemCollector) backupNamespaces(
 
 	var items []*kubernetesResource
 
-	for index, unstructured := range unstructuredList.Items {
+	for index := range unstructuredList.Items {
 		path, err := r.writeToFile(&unstructuredList.Items[index])
 		if err != nil {
-			log.WithError(err).Error("Error writing item to file")
+			log.WithError(err).Errorf("Error writing item %s to file",
+				unstructuredList.Items[index].GetName())
 			continue
 		}
 
 		items = append(items, &kubernetesResource{
 			groupResource: gr,
 			preferredGVR:  preferredGVR,
-			name:          unstructured.GetName(),
+			name:          unstructuredList.Items[index].GetName(),
 			path:          path,
 		})
 	}