From 3c5d41919b9e2b7943151b8ca1f7dcd3c7fe633a Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 6 Aug 2026 14:04:51 +0000 Subject: [PATCH 1/2] fix(lint): resolve unparam findings Removed unused return values and constant-valued parameters across 9 findings, updating every call site: - pkg/copy/copy.go: parseUserSpec drops its unused group return. - pkg/daemon/platform/local_server.go: newLocalServer and watchPlatform drop their always-nil error returns (unexported, single callers); ListenAndServe/Close synchronization via stopChan is unaffected. - pkg/driver/kubernetes/init_container.go: getInitContainers drops its always-nil error return. - pkg/driver/kubernetes/registry.go: validateIndexName drops its always-nil error return; propagated into newIndexInfo (its only caller), which had the same always-nil pattern once validateIndexName's signature simplified -- not in the original 11 but a direct consequence of this fix, so simplified alongside to keep the lint run clean. - pkg/platform/kubeconfig.go: newKubeConfig drops its always-true insecure param (InsecureSkipTLSVerify is always true across its 4 call sites; no test coverage depends on making it configurable). - pkg/telemetry/collect.go: newCLICollector drops its always-nil error return (analytics.NewClient() cannot fail). - pkg/workspace/list.go: listProWorkspaces drops its always-nil error return; propagated into reconcileProWorkspaces (its only caller, whose own error return became always-nil as a result) for the same reason as newIndexInfo above. - e2e/tests/up-docker-compose/helper.go: getAppContainer drops its unused ids return; setupDockerProvider drops its always-"docker" dockerPath param (all 5 call sites in this package use "docker"). Suppressed rather than fixed: - e2e/tests/up-features/helper.go: setupDockerProvider's dockerPath looks constant ("docker") from every non-Windows call site, but wsl.go (a windows-only build-tagged file in the same package) calls it with "podman". Verified with GOOS=windows go build. Added //nolint:unparam noting the windows-only caller. go build, go vet (aside from a pre-existing, unrelated ptytest.go vet finding), and tests for every touched package all pass. Also verified GOOS=windows go build ./e2e/... to confirm the up-features suppression. --- e2e/tests/up-docker-compose/build.go | 2 +- e2e/tests/up-docker-compose/config.go | 12 +++++------ e2e/tests/up-docker-compose/helper.go | 13 ++++++------ .../up-docker-compose/up_docker_compose.go | 2 +- e2e/tests/up-features/helper.go | 1 + pkg/copy/copy.go | 11 ++++------ pkg/daemon/platform/daemon.go | 5 +---- pkg/daemon/platform/local_server.go | 12 +++++------ pkg/driver/kubernetes/init_container.go | 8 ++++---- pkg/driver/kubernetes/registry.go | 19 +++++------------- pkg/driver/kubernetes/run.go | 5 +---- pkg/platform/kubeconfig.go | 8 +++----- pkg/telemetry/collect.go | 12 +++-------- pkg/workspace/list.go | 20 +++++++------------ 14 files changed, 49 insertions(+), 81 deletions(-) diff --git a/e2e/tests/up-docker-compose/build.go b/e2e/tests/up-docker-compose/build.go index 1dd5fa8b7..5b798fe31 100644 --- a/e2e/tests/up-docker-compose/build.go +++ b/e2e/tests/up-docker-compose/build.go @@ -37,7 +37,7 @@ var _ = ginkgo.Describe( composeHelper, err = compose.NewComposeHelper(dockerHelper) framework.ExpectNoError(err) - f, err = setupDockerProvider(initialDir+"/bin", "docker") + f, err = setupDockerProvider(initialDir + "/bin") framework.ExpectNoError(err) }) diff --git a/e2e/tests/up-docker-compose/config.go b/e2e/tests/up-docker-compose/config.go index fa2db08fc..616a861ff 100644 --- a/e2e/tests/up-docker-compose/config.go +++ b/e2e/tests/up-docker-compose/config.go @@ -36,7 +36,7 @@ var _ = ginkgo.Describe( tc.composeHelper, err = compose.NewComposeHelper(tc.dockerHelper) framework.ExpectNoError(err) - tc.f, err = setupDockerProvider(tc.initialDir+"/bin", "docker") + tc.f, err = setupDockerProvider(tc.initialDir + "/bin") framework.ExpectNoError(err) }) @@ -199,7 +199,7 @@ var _ = ginkgo.Describe( ) framework.ExpectNoError(err) - _, detail, err := tc.getAppContainer(ctx, workspace) + detail, err := tc.getAppContainer(ctx, workspace) framework.ExpectNoError(err) gomega.Expect(detail.Config.Entrypoint). NotTo(gomega.ContainElement("bash"), "overrides container entry point") @@ -336,7 +336,7 @@ var _ = ginkgo.Describe( ) framework.ExpectNoError(err) - _, detail, err := tc.getAppContainer(ctx, workspace) + detail, err := tc.getAppContainer(ctx, workspace) framework.ExpectNoError(err) gomega.Expect(detail.HostConfig.Privileged). To(gomega.BeTrue(), "container run with privileged true") @@ -349,7 +349,7 @@ var _ = ginkgo.Describe( ) framework.ExpectNoError(err) - _, detail, err := tc.getAppContainer(ctx, workspace) + detail, err := tc.getAppContainer(ctx, workspace) framework.ExpectNoError(err) gomega.Expect(detail.HostConfig.CapAdd). To(gomega.Or(gomega.ContainElement("SYS_PTRACE"), gomega.ContainElement("CAP_SYS_PTRACE")), @@ -366,7 +366,7 @@ var _ = ginkgo.Describe( ) framework.ExpectNoError(err) - _, detail, err := tc.getAppContainer(ctx, workspace) + detail, err := tc.getAppContainer(ctx, workspace) framework.ExpectNoError(err) gomega.Expect(detail.HostConfig.SecurityOpt). To(gomega.ContainElement("seccomp=unconfined"), "securityOpts contain seccomp=unconfined") @@ -577,7 +577,7 @@ var _ = ginkgo.Describe( framework.ExpectNoError(err) btc.f, err = setupDockerProvider( - filepath.Join(btc.initialDir, "bin"), "docker", + filepath.Join(btc.initialDir, "bin"), ) framework.ExpectNoError(err) }) diff --git a/e2e/tests/up-docker-compose/helper.go b/e2e/tests/up-docker-compose/helper.go index df147b0d4..ea489cab4 100644 --- a/e2e/tests/up-docker-compose/helper.go +++ b/e2e/tests/up-docker-compose/helper.go @@ -110,13 +110,12 @@ func (tc *testContext) setupAndStartWorkspace( func (tc *testContext) getAppContainer( ctx context.Context, workspace *provider2.Workspace, -) ([]string, *container.InspectResponse, error) { +) (*container.InspectResponse, error) { ids, err := findComposeContainer(ctx, tc.dockerHelper, tc.composeHelper, workspace.UID, "app") if err != nil || len(ids) == 0 { - return ids, nil, err + return nil, err } - detail, err := tc.inspectContainer(ctx, ids) - return ids, detail, err + return tc.inspectContainer(ctx, ids) } func (tc *testContext) findAppAndSidecar( @@ -158,7 +157,7 @@ func (tc *testContext) verifyWorkspaceMount( workspace *provider2.Workspace, tempDir string, ) error { - _, detail, err := tc.getAppContainer(ctx, workspace) + detail, err := tc.getAppContainer(ctx, workspace) if err != nil { return err } @@ -183,8 +182,8 @@ func setupWorkspace(testdataPath, initialDir string, f *framework.Framework) (st return tempDir, nil } -func setupDockerProvider(binDir, dockerPath string) (*framework.Framework, error) { - return framework.SetupDockerProvider(binDir, dockerPath) +func setupDockerProvider(binDir string) (*framework.Framework, error) { + return framework.SetupDockerProvider(binDir, "docker") } func findComposeContainer( diff --git a/e2e/tests/up-docker-compose/up_docker_compose.go b/e2e/tests/up-docker-compose/up_docker_compose.go index 290106d66..001515d43 100644 --- a/e2e/tests/up-docker-compose/up_docker_compose.go +++ b/e2e/tests/up-docker-compose/up_docker_compose.go @@ -40,7 +40,7 @@ var _ = ginkgo.Describe( tc.composeHelper, err = compose.NewComposeHelper(tc.dockerHelper) framework.ExpectNoError(err) - tc.f, err = setupDockerProvider(tc.initialDir+"/bin", "docker") + tc.f, err = setupDockerProvider(tc.initialDir + "/bin") framework.ExpectNoError(err) }) diff --git a/e2e/tests/up-features/helper.go b/e2e/tests/up-features/helper.go index 83796c5fb..8c27b0293 100644 --- a/e2e/tests/up-features/helper.go +++ b/e2e/tests/up-features/helper.go @@ -74,6 +74,7 @@ func addFileToTar(tarWriter *tar.Writer, filePath string) error { return err } +//nolint:unparam // dockerPath is "podman" for the windows-only caller in wsl.go func setupDockerProvider(binDir, dockerPath string) (*framework.Framework, error) { return framework.SetupDockerProvider(binDir, dockerPath) } diff --git a/pkg/copy/copy.go b/pkg/copy/copy.go index d8bd1cdf6..690dc4c0e 100644 --- a/pkg/copy/copy.go +++ b/pkg/copy/copy.go @@ -17,7 +17,7 @@ func Chown(path string, userName string) error { return nil } - uid, _ := parseUserSpec(userName) + uid := parseUserSpec(userName) userID, err := lookupUser(uid) if err != nil { return fmt.Errorf("lookup user: %w", err) @@ -33,7 +33,7 @@ func ChownR(path string, userName string) error { return nil } - uid, _ := parseUserSpec(userName) + uid := parseUserSpec(userName) userID, err := lookupUser(uid) if err != nil { return fmt.Errorf("lookup user: %w", err) @@ -215,10 +215,7 @@ func lookupUser(uid string) (*user.User, error) { return userID, err } -func parseUserSpec(userSpec string) (string, string) { +func parseUserSpec(userSpec string) string { parts := strings.SplitN(userSpec, ":", 2) - if len(parts) == 2 { - return parts[0], parts[1] - } - return parts[0], "" + return parts[0] } diff --git a/pkg/daemon/platform/daemon.go b/pkg/daemon/platform/daemon.go index fa7d9c8ba..0e3fdc2b1 100644 --- a/pkg/daemon/platform/daemon.go +++ b/pkg/daemon/platform/daemon.go @@ -60,10 +60,7 @@ func Init(ctx context.Context, config InitConfig) (*Daemon, error) { return nil, fmt.Errorf("create tailscale server: %w", err) } - localServer, err := newLocalServer(lc, config.PlatformClient, config.Context) - if err != nil { - return nil, fmt.Errorf("create local server: %w", err) - } + localServer := newLocalServer(lc, config.PlatformClient, config.Context) return &Daemon{ socketListener: socketListener, diff --git a/pkg/daemon/platform/local_server.go b/pkg/daemon/platform/local_server.go index 226935e96..aa0cd6812 100644 --- a/pkg/daemon/platform/local_server.go +++ b/pkg/daemon/platform/local_server.go @@ -88,7 +88,7 @@ func newLocalServer( lc *local.Client, pc platformclient.Client, devsyContext string, -) (*localServer, error) { +) *localServer { l := &localServer{ lc: lc, pc: pc, @@ -124,7 +124,7 @@ func newLocalServer( handler = handlers.LoggingHandler(log.Writer(log.LevelDebug), handler) l.httpServer = &http.Server{Handler: handler} - return l, nil + return l } type panicLogger struct{} @@ -137,8 +137,8 @@ func (l *localServer) ListenAndServe() error { errChan := make(chan error, 1) go func() { log.Info("Start config watcher") - err := l.watchPlatform(l.stopChan) - errChan <- err + l.watchPlatform(l.stopChan) + errChan <- nil }() go func() { err := l.httpServer.Serve(l.listener) @@ -168,7 +168,7 @@ func (l *localServer) Dial(ctx context.Context, network, addr string) (net.Conn, return l.listener.Dial(ctx, network, addr) } -func (l *localServer) watchPlatform(stopChan <-chan struct{}) error { +func (l *localServer) watchPlatform(stopChan <-chan struct{}) { for { log.Debug("Check platform status") @@ -185,7 +185,7 @@ func (l *localServer) watchPlatform(stopChan <-chan struct{}) error { select { case <-stopChan: - return nil + return case <-time.After(platformStatusCheckInterval): } } diff --git a/pkg/driver/kubernetes/init_container.go b/pkg/driver/kubernetes/init_container.go index 80bb821cf..a8f42b6f9 100644 --- a/pkg/driver/kubernetes/init_container.go +++ b/pkg/driver/kubernetes/init_container.go @@ -13,10 +13,10 @@ func (k *KubernetesDriver) getInitContainers( options *driver.RunOptions, pod *corev1.Pod, initialize bool, -) ([]corev1.Container, error) { +) []corev1.Container { if !initialize { // don't build init container and clean up existing one if defined - return filterOutInitContainer(pod.Spec.InitContainers), nil + return filterOutInitContainer(pod.Spec.InitContainers) } volumeMounts, commands := buildVolumeCopyCommands(options) @@ -25,7 +25,7 @@ func (k *KubernetesDriver) getInitContainers( // check if there is at least one mount if len(volumeMounts) == 0 { - return retContainers, nil + return retContainers } securityContext := &corev1.SecurityContext{ @@ -55,7 +55,7 @@ func (k *KubernetesDriver) getInitContainers( mergeContainer(&initContainer, existingInitContainer) retContainers = append(retContainers, initContainer) - return retContainers, nil + return retContainers } func filterOutInitContainer(containers []corev1.Container) []corev1.Container { diff --git a/pkg/driver/kubernetes/registry.go b/pkg/driver/kubernetes/registry.go index 44bb15394..70eddcbe3 100644 --- a/pkg/driver/kubernetes/registry.go +++ b/pkg/driver/kubernetes/registry.go @@ -15,10 +15,7 @@ func GetRegistryFromImageName(imageName string) (string, error) { return "", err } - repoInfo, err := newIndexInfo(reference.Domain(ref)) - if err != nil { - return "", err - } + repoInfo := newIndexInfo(reference.Domain(ref)) if !strings.ContainsRune(reference.FamiliarName(ref), '/') || repoInfo == "hub.docker.com" || repoInfo == "docker.io" { @@ -30,22 +27,16 @@ func GetRegistryFromImageName(imageName string) (string, error) { // validateIndexName validates an index name. It is used by the daemon to // validate the daemon configuration. -func validateIndexName(val string) (string, error) { +func validateIndexName(val string) string { // TODO: upstream this to check to reference package if val == "index.docker.io" { val = "docker.io" } - return val, nil + return val } // newIndexInfo returns IndexInfo configuration from indexName. -func newIndexInfo(indexName string) (string, error) { - var err error - indexName, err = validateIndexName(indexName) - if err != nil { - return "", err - } - +func newIndexInfo(indexName string) string { // Construct a non-configured index info. - return indexName, nil + return validateIndexName(indexName) } diff --git a/pkg/driver/kubernetes/run.go b/pkg/driver/kubernetes/run.go index 1c11b474a..bc6fc2e49 100644 --- a/pkg/driver/kubernetes/run.go +++ b/pkg/driver/kubernetes/run.go @@ -143,10 +143,7 @@ func (k *KubernetesDriver) buildPod( return nil, err } - initContainers, err := k.getInitContainers(options, pod, initialize) - if err != nil { - return nil, fmt.Errorf("build init container: %w", err) - } + initContainers := k.getInitContainers(options, pod, initialize) volumeMounts, tmpfsVolumes := buildVolumeMounts(mount, options) capabilities := buildCapabilities(options.CapAdd) diff --git a/pkg/platform/kubeconfig.go b/pkg/platform/kubeconfig.go index 574d2a14d..3fd8ed589 100644 --- a/pkg/platform/kubeconfig.go +++ b/pkg/platform/kubeconfig.go @@ -200,7 +200,6 @@ func directClusterEndpointKubeConfigForSpace( host, directClusterEndpointToken.Status.Token, p.spaceInstance.Spec.ClusterRef.Namespace, - true, ), nil } @@ -250,7 +249,7 @@ func kubeConfigViaAccessKey(p accessKeyKubeConfigParams) (*clientcmdapi.Config, p.resourceName, ) - return newKubeConfig(host, ownedAccessKey.Spec.Key, p.clusterRefNamespace, true), nil + return newKubeConfig(host, ownedAccessKey.Spec.Key, p.clusterRefNamespace), nil } func kubeConfigForVirtualClusterInstance( @@ -445,7 +444,6 @@ func directClusterEndpointKubeConfig( host, directClusterEndpointToken.Status.Token, req.instance.Spec.ClusterRef.Namespace, - true, ), nil } @@ -476,7 +474,7 @@ func findHostCluster( return managementv1.Cluster{}, nil } -func newKubeConfig(host, token, namespace string, insecure bool) *clientcmdapi.Config { +func newKubeConfig(host, token, namespace string) *clientcmdapi.Config { contextName := "loft" kubeConfig := clientcmdapi.NewConfig() kubeConfig.Contexts = map[string]*clientcmdapi.Context{ @@ -489,7 +487,7 @@ func newKubeConfig(host, token, namespace string, insecure bool) *clientcmdapi.C kubeConfig.Clusters = map[string]*clientcmdapi.Cluster{ contextName: { Server: host, - InsecureSkipTLSVerify: insecure, + InsecureSkipTLSVerify: true, }, } kubeConfig.AuthInfos = map[string]*clientcmdapi.AuthInfo{ diff --git a/pkg/telemetry/collect.go b/pkg/telemetry/collect.go index 142030af4..0029a4f96 100644 --- a/pkg/telemetry/collect.go +++ b/pkg/telemetry/collect.go @@ -72,11 +72,7 @@ func BootstrapCLI(cmd *cobra.Command) CLICollector { return &noopCollector{} } - collector, err := newCLICollector(cmd) - if err != nil { - log.Infof("telemetry: %s", err.Error()) - return &noopCollector{} - } + collector := newCLICollector(cmd) return collector } @@ -91,13 +87,11 @@ func ApplyCLIConfig(devsyConfig *config.Config, current CLICollector) CLICollect return current } -func newCLICollector(cmd *cobra.Command) (*cliCollector, error) { - defaultCollector := &cliCollector{ +func newCLICollector(cmd *cobra.Command) *cliCollector { + return &cliCollector{ analyticsClient: analytics.NewClient(), cmd: cmd, } - - return defaultCollector, nil } type cliCollector struct { diff --git a/pkg/workspace/list.go b/pkg/workspace/list.go index c4c3aa3ff..cfcdc6954 100644 --- a/pkg/workspace/list.go +++ b/pkg/workspace/list.go @@ -36,12 +36,9 @@ func List( proWorkspaces := []*providerpkg.Workspace{} if !skipPro { - proWorkspaces, localWorkspaces, err = reconcileProWorkspaces( + proWorkspaces, localWorkspaces = reconcileProWorkspaces( ctx, devsyConfig, localWorkspaces, owner, ) - if err != nil { - return nil, err - } } return mergeWorkspaces(localWorkspaces, proWorkspaces), nil @@ -52,11 +49,8 @@ func reconcileProWorkspaces( devsyConfig *config.Config, localWorkspaces []*providerpkg.Workspace, owner platform.OwnerFilter, -) ([]*providerpkg.Workspace, []*providerpkg.Workspace, error) { - proWorkspaceResults, err := listProWorkspaces(ctx, devsyConfig, owner) - if err != nil { - return nil, nil, err - } +) ([]*providerpkg.Workspace, []*providerpkg.Workspace) { + proWorkspaceResults := listProWorkspaces(ctx, devsyConfig, owner) proWorkspaces := []*providerpkg.Workspace{} for _, result := range proWorkspaceResults { @@ -76,7 +70,7 @@ func reconcileProWorkspaces( cleanedLocalWorkspaces = append(cleanedLocalWorkspaces, localWorkspace) } - return proWorkspaces, cleanedLocalWorkspaces, nil + return proWorkspaces, cleanedLocalWorkspaces } func deleteLocalWorkspace(devsyConfig *config.Config, localWorkspace *providerpkg.Workspace) { @@ -186,7 +180,7 @@ func listProWorkspaces( ctx context.Context, devsyConfig *config.Config, owner platform.OwnerFilter, -) (map[string]listProWorkspacesResult, error) { +) map[string]listProWorkspacesResult { results := map[string]listProWorkspacesResult{} // lock around `results` @@ -225,9 +219,9 @@ func listProWorkspaces( } }) } - wg.Wait() - return results, nil + wg.Wait() + return results } func listProWorkspacesForProvider( From d00b4c31bf58736d86be38622f7441c93c5dba60 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 7 Aug 2026 01:26:03 +0000 Subject: [PATCH 2/2] fix: address CodeRabbit review feedback on unparam PR - getAppContainer no longer returns (nil, nil) when the compose container lookup succeeds with zero IDs; it now falls through to inspectContainer, which already errors on an empty ID list. This fixes a latent nil-pointer panic in verifyWorkspaceMount and the container-detail assertions in config.go. - Update newIndexInfo's comment to reflect that it now returns a normalized registry name string, not an IndexInfo struct. --- e2e/tests/up-docker-compose/helper.go | 2 +- pkg/driver/kubernetes/registry.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/tests/up-docker-compose/helper.go b/e2e/tests/up-docker-compose/helper.go index ea489cab4..6343329f7 100644 --- a/e2e/tests/up-docker-compose/helper.go +++ b/e2e/tests/up-docker-compose/helper.go @@ -112,7 +112,7 @@ func (tc *testContext) getAppContainer( workspace *provider2.Workspace, ) (*container.InspectResponse, error) { ids, err := findComposeContainer(ctx, tc.dockerHelper, tc.composeHelper, workspace.UID, "app") - if err != nil || len(ids) == 0 { + if err != nil { return nil, err } return tc.inspectContainer(ctx, ids) diff --git a/pkg/driver/kubernetes/registry.go b/pkg/driver/kubernetes/registry.go index 70eddcbe3..a0cb8ba42 100644 --- a/pkg/driver/kubernetes/registry.go +++ b/pkg/driver/kubernetes/registry.go @@ -35,7 +35,7 @@ func validateIndexName(val string) string { return val } -// newIndexInfo returns IndexInfo configuration from indexName. +// newIndexInfo returns the normalized registry name for indexName. func newIndexInfo(indexName string) string { // Construct a non-configured index info. return validateIndexName(indexName)