Fix #8168 - AWS secrets should not be exposed while running tests

[mpryc]

Thank you for contributing to Velero!

Please add a summary of your change

Changed the tests to use mocked function that will not read actual secrets from env variables nor AWS config file that may be on the system that is running tests.

As a second guard against exposed secrets comparison for the values does not shows the actual values for the AWS data. This is to prevent situation where programming error may still allow the test to read AWS config/env variables instead of using mocked function.

This change also fixes the test run on the system with AWS creds where the test was failing.

Does your change fix a particular issue?

Fixes #8168 8168

Please indicate you've done the following:

• Accepted the DCO. Commits without the DCO will delay acceptance.
• Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
• Updated the corresponding documentation in site/content/docs/main.

[mpryc]

/kind changelog-not-required

[kaovilai]

Made an attempt at simplifying here WDYT?
No mocking, less complicated compares.

[mpryc]

Made an attempt at simplifying here WDYT? No mocking, less complicated compares.

this won't solve problem where test is ran on the system where some AWS configs are defined, e.g. before sending PR:

$ cd pkg/repository/config/
$ export AWS_ACCESS_KEY_ID=exposed_creds
$ go test ./...


$ cat ~/.aws/credentials 
[default]
aws_access_key_id = exposed_key
aws_secret_access_key = exposed_access_key
$ go test ./...

[kaovilai]

We should make unit tests not read from local credentials at all IMO.

[mpryc]

As discussed offline on Slack with @kaovilai there may be an issue with running tests in parallel with this proposed change.

The TB.SetEnv function disables the test to run in parallel as written in the https://go-review.googlesource.com/c/go/+/260577.

I would like to know if that is an issue here?

[kaovilai]

probably not an issue. When Setenv is used, isEnvSet is true, and any call to t.Parallel() after will panic.

	if t.isEnvSet {
		panic("testing: t.Parallel called after t.Setenv; cannot set environment variables in parallel tests")
	}

When we make tests more parallel we'll figure it out then. This is still an enhancement from before.. better an enhancement than no enhancement.

[mpryc]

@kaovilai I could first check if those vars are set and only then use them, how about that? This will mitigate the problem a bit because in the container run scenario it should never execute TB.SetEnv

[kaovilai]

These are internal to go testing package and the check is already done for you if you check the link

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 59.09%. Comparing base (3408ffe) to head (6d0f726).
Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/repository/config/aws.go	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8169      +/-   ##
==========================================
+ Coverage   59.05%   59.09%   +0.03%     
==========================================
  Files         364      364              
  Lines       30324    30342      +18     
==========================================
+ Hits        17909    17931      +22     
+ Misses      10972    10968       -4     
  Partials     1443     1443              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kaovilai]

please fix lint

[kaovilai]

lint test fails