Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

Refactor AWS account validation #70

Merged
merged 4 commits into from
Jan 6, 2018
Merged

Refactor AWS account validation #70

merged 4 commits into from
Jan 6, 2018

Conversation

svenwltr
Copy link
Member

@svenwltr svenwltr commented Jan 5, 2018

  • move AWS account queries into awsutil package to slim down cmd package
  • move account validation from Nuke to the actual config struct
  • add tests for account validation

@rebuy-de/prp-aws-nuke Please review.

This is a preparation for #66.

@svenwltr svenwltr self-assigned this Jan 5, 2018
t.Fatal(err)
}

config, err := LoadConfig(path.Join(cwd, "..", "config", "example.yaml"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Somewhat conflicted about using an example file as a test input. It does make sure that the example is valid to some degree. On the other hand it is really unexpected to have an example be part of a test case.

I do not have much experience with generated documentation, but I feel like this would be another potential use case for it (besides the documentation of supported AWS resources).

Copy link
Member Author

Choose a reason for hiding this comment

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

I had the same doubts. I added another test config for the tests.

cmd/config.go Outdated
@@ -51,6 +53,40 @@ func (c *NukeConfig) InBlacklist(searchID string) bool {
return false
}

func (c *NukeConfig) ValidateAccount(accountID string, aliases []string) (*NukeConfigAccount, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Imho, I liked the old signature of ValidateAccount, where it only validates and returns error(s).

Copy link
Member Author

Choose a reason for hiding this comment

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

That's very true. Fixed.

@svenwltr svenwltr merged commit 07ca11d into master Jan 6, 2018
@svenwltr svenwltr deleted the refactor-aws branch January 6, 2018 21:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants