chore: Remove unneeded Go linter exceptions #3247

lantoli · 2025-04-04T06:48:31Z

Description

Remove unneeded Go linter exceptions. Only do exceptions when strictly needed so the codebase follows linter rules.

PR tests failing in encryption group with error CANNOT_DISABLE_ENCRYPTION_AT_REST_REQUIRE_PRIVATE_NETWORKING_WHILE_PRIVATE_ENDPOINTS_EXIST
adv_cluster test group: pass
adv_cluster_tpf group: pass

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

lantoli · 2025-04-04T06:50:31Z

internal/service/advancedcluster/resource_advanced_cluster_test.go

@@ -161,7 +161,7 @@ func TestAccMockableAdvancedCluster_tenantUpgrade(t *testing.T) {
 		projectID, clusterName = acc.ProjectIDExecutionWithCluster(t, 1)
 		defaultZoneName        = "Zone 1" // Uses backend default to avoid non-empty plan, see CLOUDP-294339
 	)
-	unit.CaptureOrMockTestCaseAndRun(t, mockConfig, &resource.TestCase{
+	unit.CaptureOrMockTestCaseAndRun(t, &mockConfig, &resource.TestCase{


fixes: hugeParam: config is heavy (112 bytes); consider passing it by pointer (gocritic)

lantoli · 2025-04-04T06:50:51Z

internal/service/advancedcluster/resource_advanced_cluster_test.go

 				customOpensslCipherConfigTLS12Str = fmt.Sprintf(
-					`custom_openssl_cipher_config_tls12 = ["%s"]`,


fixes: sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)

lantoli · 2025-04-04T06:51:18Z

internal/service/encryptionatrestprivateendpoint/data_source.go

@@ -1,4 +1,3 @@
-//nolint:gocritic


leftover, no needed anymore

lantoli · 2025-04-04T06:51:24Z

internal/service/encryptionatrestprivateendpoint/plural_data_source.go

@@ -1,4 +1,3 @@
-//nolint:gocritic


leftover, no needed anymore

lantoli · 2025-04-04T06:51:45Z

internal/testutil/unit/http_mocker.go

@@ -30,12 +30,6 @@ type MockHTTPDataConfig struct {
 	IsDiffMustSubstrings []string             // Only include diff request for specific substrings, for example /clusters (avoids project create requests)
 	QueryVars            []string             // Substitute this query vars. Useful when differentiating responses based on query args, for example ?providerName=AWS/AZURE returns different responses
 	AllowMissingRequests bool                 // When false will require all API calls to be made.
-	AllowOutOfOrder      bool                 // When true will allow a GET request returned after a POST to be returned before the POST.


I think AllowOutOfOrder is not being used anymore, can @EspenAlbert you please confirm?

Although not used anywhere in the code, it can be helpful while debugging. I would prefer to keep it

ok, keeping, adding comment

lantoli · 2025-04-04T06:52:18Z

internal/testutil/unit/http_mocker.go

@@ -53,7 +47,7 @@ func IsDataUpdate() bool {
 	return val
 }

-func CaptureOrMockTestCaseAndRun(t *testing.T, config MockHTTPDataConfig, testCase *resource.TestCase) { //nolint: gocritic // Want each test run to have its own config (hugeParam: config is heavy (112 bytes); consider passing it by pointer)


fixes: hugeParam: config is heavy (112 bytes); consider passing it by pointer (gocritic)

Want each test run to have its own config

I prefer keeping this as is

lantoli · 2025-04-04T06:52:36Z

internal/testutil/unit/http_mocker_data.go

@@ -56,7 +56,7 @@ type RequestInfo struct {
 }

 // Custom marshaling is necessary to use `flow` style only on response fields (text and responses.*.text)
-func (i RequestInfo) MarshalYAML() (any, error) { //nolint:gocritic // Using a pointer method leads to inconsistent dump results
+func (i *RequestInfo) MarshalYAML() (any, error) {


fixes: hugeParam: i is heavy (88 bytes); consider passing it by pointer (gocritic)

al other RequestInfo funcs are using pointers

internal/common/conversion/string_framework.go

lantoli · 2025-04-04T07:39:06Z

internal/testutil/unit/http_mocker_data.go

-	Version   string       `yaml:"version"`
-	Text      string       `yaml:"text"`
-	Responses []StatusText `yaml:"responses"`
+	Responses *[]StatusText `yaml:"responses"`


fixes: hugeParam: i is heavy (88 bytes); consider passing it by pointer (gocritic)

* master: feat: Support configuring BYOK encryption on search nodes (#3199) chore: Updates Atlas Go SDK (#3235) # Conflicts: # internal/testutil/acc/advanced_cluster.go

AgustinBettati · 2025-04-04T15:54:21Z

internal/testutil/unit/http_mocker.go

@@ -53,7 +47,7 @@ func IsDataUpdate() bool {
 	return val
 }

-func CaptureOrMockTestCaseAndRun(t *testing.T, config MockHTTPDataConfig, testCase *resource.TestCase) { //nolint: gocritic // Want each test run to have its own config (hugeParam: config is heavy (112 bytes); consider passing it by pointer)


we should double check with @EspenAlbert but it seems the pass by value is intentional and needed here?

thanks, I'll wait

This reverts commit a2be534.

* master: chore: Updates org_clean_test.go to delete stream instances & private endpoint services (#3252) chore: Updates organization.md `skip_default_alerts_settings` note format (#3246) doc: Update guidelines for external contributors to squash commits (#3243) chore: Updates CHANGELOG.md for #3199

EspenAlbert

Thank you!

lantoli · 2025-04-09T15:21:56Z

encryption test group is also failing in master, e.g. here

lantoli added 2 commits April 4, 2025 08:39

remove exceptions in tests

5268b11

remove exceptions in prod code

bd91fe7

lantoli commented Apr 4, 2025

View reviewed changes

internal/service/encryptionatrestprivateendpoint/data_source.go

@@ -1,4 +1,3 @@

//nolint:gocritic

Copy link

Member Author

lantoli Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover, no needed anymore

lantoli commented Apr 4, 2025

View reviewed changes

ToTFMapOfString

d9d55f9

lantoli commented Apr 4, 2025

View reviewed changes

internal/common/conversion/string_framework.go Show resolved Hide resolved

fix MarshalYAML

a2be534

lantoli commented Apr 4, 2025

View reviewed changes

lantoli added 2 commits April 4, 2025 17:27

JoinQuotedStrings

4c27f7d

Merge branch 'master' into remove_lint_exceptions

567812f

* master: feat: Support configuring BYOK encryption on search nodes (#3199) chore: Updates Atlas Go SDK (#3235) # Conflicts: # internal/testutil/acc/advanced_cluster.go

lantoli marked this pull request as ready for review April 4, 2025 15:33

lantoli requested a review from a team as a code owner April 4, 2025 15:33

AgustinBettati approved these changes Apr 4, 2025

View reviewed changes

Revert "fix MarshalYAML"

40022b5

This reverts commit a2be534.

lantoli added the not_stale Not stale issue or PR label Apr 8, 2025

lantoli added 4 commits April 8, 2025 15:27

revert http_mocker changes

873e88f

AllowOutOfOrder doc

6bc3ca7

fix linter

f394318

EspenAlbert approved these changes Apr 8, 2025

View reviewed changes

lantoli merged commit b88f2a9 into master Apr 9, 2025
39 of 40 checks passed

lantoli deleted the remove_lint_exceptions branch April 9, 2025 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Remove unneeded Go linter exceptions #3247

chore: Remove unneeded Go linter exceptions #3247

lantoli commented Apr 4, 2025 •

edited

Loading

lantoli Apr 4, 2025

lantoli Apr 4, 2025

lantoli Apr 4, 2025

lantoli Apr 4, 2025

lantoli Apr 4, 2025

EspenAlbert Apr 8, 2025

lantoli Apr 8, 2025

lantoli Apr 4, 2025

EspenAlbert Apr 8, 2025 •

edited

Loading

lantoli Apr 4, 2025 •

edited

Loading

lantoli Apr 4, 2025

AgustinBettati Apr 4, 2025

lantoli Apr 4, 2025

EspenAlbert left a comment

lantoli commented Apr 9, 2025

		customOpensslCipherConfigTLS12Str = fmt.Sprintf(
		`custom_openssl_cipher_config_tls12 = ["%s"]`,

chore: Remove unneeded Go linter exceptions #3247

chore: Remove unneeded Go linter exceptions #3247

Conversation

lantoli commented Apr 4, 2025 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EspenAlbert Apr 8, 2025 • edited Loading

Choose a reason for hiding this comment

lantoli Apr 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EspenAlbert left a comment

Choose a reason for hiding this comment

lantoli commented Apr 9, 2025

lantoli commented Apr 4, 2025 •

edited

Loading

EspenAlbert Apr 8, 2025 •

edited

Loading

lantoli Apr 4, 2025 •

edited

Loading