Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor AWS db mocks #30086

Merged
merged 3 commits into from Aug 8, 2023
Merged

Conversation

greedy52
Copy link
Contributor

@greedy52 greedy52 commented Aug 5, 2023

While working on UT for database URL checker, I found that I need to copy a lot of mock structs from db fetchers. Decided to make a separate change just for this refactor to make review easier.

Changes:

  • split lib/cloud/mocks/aws.go to individual files (lib/cloud/mocks/aws_xxx.go) one per AWS service type
  • moved making of sample AWS structs (e.g rds.DBInstance) from db fetch UTs to lib/cloud/mocks/aws_xxx.go
    • fixed a few sample URLs in these structs from like xxx.localhost to valid AWS endpoints
  • refactored RDS cluster fetcher and ElastiCache fetcher by combining multiple services.NewXXX to one.
  • added missing ElastiCache db fetcher reader endpoint UT

No logic change, just moving things around.

@greedy52 greedy52 added refactoring aws Used for AWS Related Issues. database-access Database access related issues and PRs labels Aug 5, 2023
@greedy52 greedy52 self-assigned this Aug 5, 2023
@@ -809,6 +809,60 @@ func NewDatabasesFromRDSClusterCustomEndpoints(cluster *rds.DBCluster) (types.Da
return databases, trace.NewAggregate(errors...)
}

// NewDatabasesFromRDSCluster creates all database resources from an RDS Aurora
// cluster.
func NewDatabasesFromRDSCluster(cluster *rds.DBCluster) (types.Databases, error) {
Copy link
Contributor Author

@greedy52 greedy52 Aug 5, 2023

Choose a reason for hiding this comment

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

Moved logic from db fetcher to services so it's easier for another package to do the same. no change.

@greedy52 greedy52 force-pushed the STeve/p471_discovered_db_url_validation_p15 branch from 5d84d04 to fd1238b Compare August 6, 2023 17:56
@greedy52 greedy52 added this pull request to the merge queue Aug 8, 2023
Merged via the queue into master with commit d9d12cc Aug 8, 2023
21 checks passed
@greedy52 greedy52 deleted the STeve/p471_discovered_db_url_validation_p15 branch August 8, 2023 12:24
greedy52 added a commit that referenced this pull request Aug 14, 2023
* Refactor AWS db mocks

* removew lib/services dependency

* fix rds instance url, add elasticache reader endpoint
github-merge-queue bot pushed a commit that referenced this pull request Aug 15, 2023
* Refactor AWS db mocks

* removew lib/services dependency

* fix rds instance url, add elasticache reader endpoint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws Used for AWS Related Issues. database-access Database access related issues and PRs discovery refactoring size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants