Fix the typecheck error reported by the lint GitHub action.

Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>

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

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

.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,8 +140,6 @@ 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
   gomodguard:
@@ -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,18 @@ type itemCollector struct {
 }
 
 // nsTracker is used to integrate several namespace filters together.
-//  1. backup's namespace include and exclude filters;
+//  1. Backup's namespace Include/Exclude filters;
-//  2. backup's LabelSelector and OrLabelSelector selected namespace;
+//  2. Backup's (Or)LabelSelector selected namespace;
-//  3. resources namespaces selected by label selectors.
+//  3. Backup's (Or)LabelSelector selected resources' namespaces.
+//
+// Rules:
+//
+//	a. When backup namespace Include/Exclude filters get everything,
+//	The namespaces, which do not have backup including resources,
+//	are not collected.
+//
+//	b. If the namespace I/E filters and the (Or)LabelSelectors selected
+//	namespaces are different. The tracker takes the union of them.
 type nsTracker struct {
 	singleLabelSelector labels.Selector
 	orLabelSelector     []labels.Selector
@@ -67,6 +76,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,6 +87,8 @@ 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]
@@ -84,6 +96,8 @@ func (nt *nsTracker) isTracked(ns string) bool {
 	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,
@@ -91,18 +105,21 @@ func (nt *nsTracker) init(
 	namespaceFilter *collections.IncludesExcludes,
 	logger logrus.FieldLogger,
 ) {
+	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.singleLabelSelector.Matches(labels.Set(namespace.GetLabels())) {
-			nt.logger.Debugf("Track namespace %s,", namespace.GetName(),
+			nt.logger.Debugf(`Track namespace %s, 
-				"because its labels match backup LabelSelector.")
+				because its labels match backup LabelSelector.`,
+				namespace.GetName(),
+			)
 
 			nt.track(namespace.GetName())
 			continue
@@ -110,9 +127,11 @@ func (nt *nsTracker) init(
 
 		if len(nt.orLabelSelector) > 0 {
 			for _, selector := range nt.orLabelSelector {
-				if selector.Matches(labels.Set(namespace.GetLabels())) == true {
+				if selector.Matches(labels.Set(namespace.GetLabels())) {
-					nt.logger.Debugf("Track namespace %s", namespace.GetName(),
+					nt.logger.Debugf(`Track namespace %s",
-						"because its labels match the backup OrLabelSelector.")
+						"because its labels match the backup OrLabelSelector.`,
+						namespace.GetName(),
+					)
 					nt.track(namespace.GetName())
 					continue
 				}
@@ -120,25 +139,26 @@ func (nt *nsTracker) init(
 		}
 
 		// 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 nt.namespaceFilter.IncludeEverything() &&
 			(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 {
+		if nt.namespaceFilter.ShouldInclude(namespace.GetName()) {
-			nt.logger.Debugf("Track namespace %s", namespace.GetName(),
+			nt.logger.Debugf(`Track namespace %s,
-				"because its name match the backup namespace filter.")
+				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 {
@@ -146,7 +166,7 @@ func (nt *nsTracker) filterNamespaces(
 
 	for _, resource := range resources {
 		if resource.groupResource != kuberesource.Namespaces ||
-			nt.isTracked(resource.name) == true {
+			nt.isTracked(resource.name) {
 			result = append(result, resource)
 		}
 	}
@@ -160,8 +180,8 @@ type kubernetesResource struct {
 	namespace, name, path string
 }
 
-// getItemsFromResourceIdentifiers converts ResourceIdentifiers to
+// getItemsFromResourceIdentifiers get the kubernetesResources
-// kubernetesResources
+// specified by the input parameter resourceIDs.
 func (r *itemCollector) getItemsFromResourceIdentifiers(
 	resourceIDs []velero.ResourceIdentifier,
 ) []*kubernetesResource {
@@ -173,16 +193,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 +433,10 @@ func (r *itemCollector) getResourceItems(
 	}
 
 	// Handle namespace resource here.
-	// Namespace are only filtered by namespace include/exclude filters.
+	// Namespace are filtered by namespace include/exclude filters,
-	// Label selectors are not checked.
+	// backup LabelSelectors and OrLabelSelectors are checked too.
 	if gr == kuberesource.Namespaces {
-		return r.backupNamespaces(
+		return r.collectNamespaces(
 			resource,
 			gv,
 			gr,
@@ -697,8 +721,8 @@ func (r *itemCollector) listItemsForLabel(
 	return unstructuredItems, nil
 }
 
-// backupNamespaces process namespace resource according to namespace filters.
+// collectNamespaces process namespace resource according to namespace filters.
-func (r *itemCollector) backupNamespaces(
+func (r *itemCollector) collectNamespaces(
 	resource metav1.APIResource,
 	gv schema.GroupVersion,
 	gr schema.GroupResource,
@@ -750,17 +774,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,
 		})
 	}

pkg/backup/item_collector_test.go

@@ -206,12 +206,23 @@ func TestItemCollectorBackupNamespaces(t *testing.T) {
 			backup: builder.ForBackup("velero", "backup").OrLabelSelector([]*metav1.LabelSelector{
 				{MatchLabels: map[string]string{"name": "ns1"}},
 			}).Result(),
-			ie: collections.NewIncludesExcludes().Excludes("ns1"),
+			ie: collections.NewIncludesExcludes().Excludes("ns1", "ns2"),
+			namespaces: []*corev1.Namespace{
+				builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(),
+				builder.ForNamespace("ns2").Result(),
+				builder.ForNamespace("ns3").Result(),
+			},
+			expectedTrackedNS: []string{"ns1", "ns3"},
+		},
+		{
+			name:   "No ns filters",
+			backup: builder.ForBackup("velero", "backup").Result(),
+			ie:     collections.NewIncludesExcludes().Includes("*"),
 			namespaces: []*corev1.Namespace{
 				builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(),
 				builder.ForNamespace("ns2").Result(),
 			},
-			expectedTrackedNS: []string{"ns1"},
+			expectedTrackedNS: []string{"ns1", "ns2"},
 		},
 	}
 
@@ -248,7 +259,7 @@ func TestItemCollectorBackupNamespaces(t *testing.T) {
 				dir:            tempDir,
 			}
 
-			r.backupNamespaces(
+			r.collectNamespaces(
 				metav1.APIResource{
 					Name:       "Namespace",
 					Kind:       "Namespace",

pkg/client/client_test.go

@@ -79,7 +79,7 @@ func TestConfig(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			client, err := Config(test.kubeconfig, test.kubecontext, "velero", test.QPS, test.burst)
-			assert.Equal(t, err, nil)
+			assert.Nil(t, err)
 			assert.Equal(t, test.expectedHost, client.Host)
 			assert.Equal(t, test.QPS, client.QPS)
 			assert.Equal(t, test.burst, client.Burst)

pkg/client/config_test.go

@@ -62,13 +62,13 @@ func TestConfigOperations(t *testing.T) {
 
 	// Remove config file if it exists
 	err := removeConfigfileName()
-	assert.Equal(t, err, nil)
+	assert.NoError(t, err)
 
 	// Test LoadConfig: expect an empty velero config
 	expectedConfig := VeleroConfig{}
 	config, err := LoadConfig()
 
-	assert.Equal(t, err, nil)
+	assert.NoError(t, err)
 	assert.True(t, reflect.DeepEqual(expectedConfig, config))
 
 	// Test savedConfig
@@ -84,9 +84,9 @@ func TestConfigOperations(t *testing.T) {
 
 	err = SaveConfig(config)
 
-	assert.Equal(t, err, nil)
+	assert.NoError(t, err)
 	savedConfig, err := LoadConfig()
-	assert.Equal(t, err, nil)
+	assert.NoError(t, err)
 
 	// Test Features
 	feature := savedConfig.Features()
@@ -107,7 +107,7 @@ func TestConfigOperations(t *testing.T) {
 
 	t.Cleanup(func() {
 		err = removeConfigfileName()
-		assert.Equal(t, err, nil)
+		assert.NoError(t, err)
 		os.Unsetenv("HOME")
 		os.Setenv("HOME", preHomeEnv)
 	})

pkg/client/factory_test.go

@@ -95,7 +95,7 @@ func TestFactory(t *testing.T) {
 
 	baseName := "velero-bn"
 	config, err := LoadConfig()
-	assert.Equal(t, err, nil)
+	assert.NoError(t, err)
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			f = NewFactory(baseName, config)

pkg/cmd/cli/backup/delete_test.go

@@ -62,15 +62,15 @@ func TestDeleteCommand(t *testing.T) {
 	args := []string{backup1, backup2}
 
 	e := o.Complete(f, args)
-	require.Equal(t, nil, e)
+	require.NoError(t, e)
 
 	e = o.Validate(c, f, args)
-	require.Equal(t, nil, e)
+	require.NoError(t, e)
 
 	Run(o)
 
 	e = c.Execute()
-	require.Equal(t, nil, e)
+	require.NoError(t, e)
 
 	if os.Getenv(cmdtest.CaptureFlag) == "1" {
 		return

pkg/cmd/cli/backuplocation/create_test.go

@@ -148,7 +148,7 @@ func TestCreateCommand_Run(t *testing.T) {
 
 	o.Complete(args, f)
 	e := o.Validate(c, args, f)
-	assert.Equal(t, e, nil)
+	assert.Nil(t, e)
 
 	e = o.Run(c, f)
 	assert.Contains(t, e.Error(), fmt.Sprintf("%s: no such file or directory", caCertFile))

pkg/cmd/cli/backuplocation/set_test.go

@@ -66,7 +66,7 @@ func TestNewSetCommand(t *testing.T) {
 	args := []string{backupName}
 	o.Complete(args, f)
 	e := o.Validate(c, args, f)
-	assert.Equal(t, e, nil)
+	assert.Nil(t, e)
 
 	e = o.Run(c, f)
 	assert.Contains(t, e.Error(), fmt.Sprintf("%s: no such file or directory", cacert))

pkg/cmd/cli/restore/delete_test.go

@@ -62,15 +62,15 @@ func TestDeleteCommand(t *testing.T) {
 	args := []string{restore1, restore2}
 
 	e := o.Complete(f, args)
-	require.Equal(t, nil, e)
+	require.NoError(t, e)
 
 	e = o.Validate(c, f, args)
-	require.Equal(t, nil, e)
+	require.NoError(t, e)
 
 	Run(o)
 
 	e = c.Execute()
-	require.Equal(t, nil, e)
+	require.NoError(t, e)
 
 	if os.Getenv(cmdtest.CaptureFlag) == "1" {
 		return

pkg/datapath/file_system_test.go

@@ -101,7 +101,7 @@ func TestAsyncBackup(t *testing.T) {
 			fs.callbacks = test.callbacks
 
 			err := fs.StartBackup(AccessPoint{ByPath: test.path}, "", "", false, nil, map[string]string{})
-			require.Equal(t, nil, err)
+			require.NoError(t, err)
 
 			<-finish
 
@@ -184,7 +184,7 @@ func TestAsyncRestore(t *testing.T) {
 			fs.callbacks = test.callbacks
 
 			err := fs.StartRestore(test.snapshot, AccessPoint{ByPath: test.path}, map[string]string{})
-			require.Equal(t, nil, err)
+			require.NoError(t, err)
 
 			<-finish
 

pkg/datapath/manager_test.go

@@ -36,7 +36,7 @@ func TestManager(t *testing.T) {
 	assert.Equal(t, ConcurrentLimitExceed, err)
 
 	ret := m.GetAsyncBR("job-0")
-	assert.Equal(t, nil, ret)
+	assert.Nil(t, ret)
 
 	ret = m.GetAsyncBR("job-1")
 	assert.Equal(t, async_job_1, ret)
@@ -48,5 +48,5 @@ func TestManager(t *testing.T) {
 	assert.Len(t, m.tracker, 1)
 
 	ret = m.GetAsyncBR("job-1")
-	assert.Equal(t, nil, ret)
+	assert.Nil(t, ret)
 }