chore(linter): enable unparam

[ivankatliarchuk]

What does it do ?

initial failed job https://github.com/kubernetes-sigs/external-dns/actions/runs/21548777453/job/62094259356?pr=6160

Just enough changes to satisfy linter. Fixing this, I found quite few patterns that could have shared function. I'll provide refactoring/shared function in follow-up PR.

Current problems solved

1. Dead code / unnecessary complexity: Parameters that never vary indicate over-engineered APIs. If pageSize is always 50, why make callers pass it? It adds cognitive load without flexibility.

2. Incomplete implementations: Functions returning error that's always nil suggest error handling was planned but never implemented. Reviewers should ask: "Should this propagate errors from the operations inside?"

3. Unused return values: If nobody uses a return value, either the callers are missing something, or the function signature is wrong.

4. Test helper bloat: Test helpers with unused parameters (like msgAndArgs always nil, or recordType always "A") indicate copy-paste from other helpers without cleanup.

5. Refactoring leftovers: Parameters that became constant over time often indicate incomplete refactoring. The code evolved but the signature didn't.

Motivation

For reviewers specifically:

• Catches issues that are hard to spot manually (requires tracing all call sites)
• Flags places where the API promises flexibility it doesn't deliver
• Highlights potential bugs where callers should be passing different values but aren't
• Reduces future maintenance burden by keeping signatures honest

The Alibaba errors are a good example: 6 functions return error but always return nil. A reviewer should ask whether errors from the underlying API calls should bubble up.

More

• Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

[ivankatliarchuk]

❯❯ mk go-lint
golangci-lint config verify
gofmt -l -s -w .
golangci-lint run --timeout=30m --fix ./...
controller/execute_test.go:299:62: runExecuteSubprocess - result 1 (string) is never used (unparam)
func runExecuteSubprocess(t *testing.T, args []string) (int, string, error) {
                                                             ^
provider/alibabacloud/alibaba_cloud.go:346:36: getNextPageNumber - pageSize always receives defaultAlibabaCloudPageSize (50) (unparam)
func getNextPageNumber(pageNumber, pageSize, totalCount int64) int64 {
                                   ^
provider/alibabacloud/alibaba_cloud.go:547:106: (*AlibabaCloudProvider).createRecords - result 0 (error) is always nil (unparam)
func (p *AlibabaCloudProvider) createRecords(endpoints []*endpoint.Endpoint, hostedZoneDomains []string) error {
                                                                                                         ^
provider/alibabacloud/alibaba_cloud.go:594:116: (*AlibabaCloudProvider).deleteRecords - result 0 (error) is always nil (unparam)
func (p *AlibabaCloudProvider) deleteRecords(recordMap map[string][]alidns.Record, endpoints []*endpoint.Endpoint) error {
                                                                                                                   ^
provider/alibabacloud/alibaba_cloud.go:631:144: (*AlibabaCloudProvider).updateRecords - result 0 (error) is always nil (unparam)
func (p *AlibabaCloudProvider) updateRecords(recordMap map[string][]alidns.Record, endpoints []*endpoint.Endpoint, hostedZoneDomains []string) error {
                                                                                                                                               ^
provider/alibabacloud/alibaba_cloud.go:909:127: (*AlibabaCloudProvider).createPrivateZoneRecords - result 0 (error) is always nil (unparam)
func (p *AlibabaCloudProvider) createPrivateZoneRecords(zones map[string]*alibabaPrivateZone, endpoints []*endpoint.Endpoint) error {
                                                                                                                              ^
provider/alibabacloud/alibaba_cloud.go:937:127: (*AlibabaCloudProvider).deletePrivateZoneRecords - result 0 (error) is always nil (unparam)
func (p *AlibabaCloudProvider) deletePrivateZoneRecords(zones map[string]*alibabaPrivateZone, endpoints []*endpoint.Endpoint) error {
                                                                                                                              ^
provider/alibabacloud/alibaba_cloud.go:1024:127: (*AlibabaCloudProvider).updatePrivateZoneRecords - result 0 (error) is always nil (unparam)
func (p *AlibabaCloudProvider) updatePrivateZoneRecords(zones map[string]*alibabaPrivateZone, endpoints []*endpoint.Endpoint) error {
                                                                                                                              ^
provider/aws/aws_utils_test.go:145:26: unmarshalTestHelper - input always receives "/fixtures/160-plus-zones.yaml" (unparam)
func unmarshalTestHelper(input string, obj any, t *testing.T) {
                         ^
provider/azure/azure_privatedns_test.go:195:33: createPrivateMockRecordSet - name always receives "@" (unparam)
func createPrivateMockRecordSet(name, recordType string, values ...string) *privatedns.RecordSet {
                                ^
provider/azure/azure_privatedns_test.go:228:303: newMockedAzurePrivateDNSProvider - result 1 (error) is always nil (unparam)
func newMockedAzurePrivateDNSProvider(domainFilter *endpoint.DomainFilter, zoneNameFilter *endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, dryRun bool, resourceGroup string, zones []*privatedns.PrivateZone, recordSets []*privatedns.RecordSet, maxRetriesCount int) (*AzurePrivateDNSProvider, error) {
                                                                                                                                                                                                                                                                                                              ^
provider/azure/azure_test.go:241:336: newMockedAzureProvider - result 1 (error) is always nil (unparam)
func newMockedAzureProvider(domainFilter *endpoint.DomainFilter, zoneNameFilter *endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, dryRun bool, resourceGroup string, userAssignedIdentityClientID string, activeDirectoryAuthorityHost string, zones []*dns.Zone, recordSets []*dns.RecordSet, maxRetriesCount int) (*AzureProvider, error) {
                                                                                                                                                                                                                                                                                                                                               ^
provider/civo/civo_test.go:1179:52: elementsMatch - msgAndArgs always receives nil (unparam)
func elementsMatch(t *testing.T, listA, listB any, msgAndArgs ...any) bool {
                                                   ^
provider/coredns/coredns.go:293:58: findLabelInTargets - result 0 (string) is never used (unparam)
func findLabelInTargets(targets []string, label string) (string, bool) {
                                                         ^
provider/digitalocean/digital_ocean_test.go:197:52: elementsMatch - msgAndArgs always receives nil (unparam)
func elementsMatch(t *testing.T, listA, listB any, msgAndArgs ...any) bool {
                                                   ^
provider/google/google_test.go:679:162: newGoogleProviderZoneOverlap - dryRun always receives false (unparam)
func newGoogleProviderZoneOverlap(t *testing.T, domainFilter *endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, dryRun bool, _ []*endpoint.Endpoint) *GoogleProvider {
                                                                                                                                                                 ^
provider/pdns/pdns.go:271:85: (*PDNSProvider).convertRRSetToEndpoints - result 1 (error) is always nil (unparam)
func (p *PDNSProvider) convertRRSetToEndpoints(rr pgo.RrSet) ([]*endpoint.Endpoint, error) {
                                                                                    ^
registry/awssd/registry_test.go:169:56: newEndpointWithOwner - ownerID always receives "owner" (unparam)
func newEndpointWithOwner(dnsName, target, recordType, ownerID string) *endpoint.Endpoint {
                                                       ^
registry/txt/utils_test.go:33:58: newEndpointWithOwnerAndOwnedRecord - recordType always receives endpoint.RecordTypeTXT ("TXT") (unparam)
func newEndpointWithOwnerAndOwnedRecord(dnsName, target, recordType, ownerID, ownedRecord string) *endpoint.Endpoint {
                                                         ^
registry/txt/utils_test.go:51:52: newEndpointWithOwnerResource - recordType always receives endpoint.RecordTypeCNAME ("CNAME") (unparam)
func newEndpointWithOwnerResource(dnsName, target, recordType, ownerID, resource string) *endpoint.Endpoint {
                                                   ^
source/ambassador_host.go:192:123: (*ambassadorHostSource).endpointsFromHost - result 1 (error) is always nil (unparam)
func (sc *ambassadorHostSource) endpointsFromHost(host *ambassador.Host, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
                                                                                                                          ^
source/contour_httpproxy.go:209:111: (*httpProxySource).endpointsFromHTTPProxy - result 1 (error) is always nil (unparam)
func (sc *httpProxySource) endpointsFromHTTPProxy(httpProxy *projectcontour.HTTPProxy) ([]*endpoint.Endpoint, error) {
                                                                                                              ^
source/f5_transportserver.go:144:129: (*f5TransportServerSource).endpointsFromTransportServers - result 1 (error) is always nil (unparam)
func (ts *f5TransportServerSource) endpointsFromTransportServers(transportServers []*f5.TransportServer) ([]*endpoint.Endpoint, error) {
                                                                                                                                ^
source/f5_virtualserver.go:149:121: (*f5VirtualServerSource).endpointsFromVirtualServers - result 1 (error) is always nil (unparam)
func (vs *f5VirtualServerSource) endpointsFromVirtualServers(virtualServers []*f5.VirtualServer) ([]*endpoint.Endpoint, error) {
                                                                                                                        ^
source/gateway_httproute_test.go:119:31: newTestEndpoint - recordType always receives "A" (unparam)
func newTestEndpoint(dnsName, recordType string, targets ...string) *endpoint.Endpoint {
                              ^
source/istio_gateway.go:279:94: (*gatewaySource).hostNamesFromGateway - result 1 (error) is always nil (unparam)
func (sc *gatewaySource) hostNamesFromGateway(gateway *networkingv1beta1.Gateway) ([]string, error) {
                                                                                             ^
source/kong_tcpingress.go:171:130: (*kongTCPIngressSource).endpointsFromTCPIngress - result 1 (error) is always nil (unparam)
func (sc *kongTCPIngressSource) endpointsFromTCPIngress(tcpIngress *TCPIngress, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
                                                                                                                                 ^
source/node_test.go:655:36: (*nodeListBuilder).withNode - labels always receives nil (unparam)
func (b *nodeListBuilder) withNode(labels map[string]string) *nodeListBuilder {
                                   ^
source/service_test.go:5112:31: createTestServicesByType - namespace always receives namespace ("testns") (unparam)
func createTestServicesByType(namespace string, typeCounts map[v1.ServiceType]int) []*v1.Service {
                              ^
29 issues:
* unparam: 29
make: *** [go-lint] Error 1

[coveralls]

Pull Request Test Coverage Report for Build 22218536551

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 198 unchanged lines in 10 files lost coverage.
• Overall coverage increased (+0.06%) to 79.221%

---

Files with Coverage Reduction	New Missed Lines	%
f5_virtualserver.go	5	80.61%
f5_transportserver.go	5	78.65%
contour_httpproxy.go	7	84.67%
ambassador_host.go	7	84.03%
istio_gateway.go	9	88.76%
crypto.go	15	67.9%
pdns/pdns.go	15	71.19%
coredns/coredns.go	29	90.21%
kong_tcpingress.go	37	48.15%
alibabacloud/alibaba_cloud.go	69	63.85%

Totals
Change from base Build 21548441334:	0.06%
Covered Lines:	16032
Relevant Lines:	20237

---

💛 - Coveralls

[vflaux]

Why change to "txt."?

[ivankatliarchuk]

changed back

[vflaux]

Suggested change

	if source, err := newTestHTTPProxySource(); err != nil {
	require.NoError(t, err)
	} else if endpoints, err := source.endpointsFromHTTPProxy(ti.httpProxy.HTTPProxy()); err != nil {
	require.NoError(t, err)
	} else {
	endpoints := source.endpointsFromHTTPProxy(ti.httpProxy.HTTPProxy())
	validateEndpoints(t, endpoints, ti.expected)
	}
	source, err := newTestHTTPProxySource()
	require.NoError(t, err)
	endpoints := source.endpointsFromHTTPProxy(ti.httpProxy.HTTPProxy())
	validateEndpoints(t, endpoints, ti.expected)

[ivankatliarchuk]

resolved

[vflaux]

Is there a reason for not using "default" here like in other test cases?

[ivankatliarchuk]

Yes. Unapam linter think that the code only works with default and fail the lint.

[vflaux]

Why this change?

[ivankatliarchuk]

The way unaparam works - it detects cases with same values like nil in this case, and fail the lint. As there is no variation of the values, kinda either should be empty or have different values.

[ivankatliarchuk]

[vflaux]

/lgtm

[ivankatliarchuk]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivankatliarchuk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [ivankatliarchuk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment